From: Reinhold Kainhofer Date: Fri, 24 Jun 2011 20:43:55 +0000 (+0200) Subject: Fix 1259/1433: linebreaks with \breakDynamicSpan or spanners with style=#'none X-Git-Tag: release/2.15.7-1~16 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=cf3642858a2340bb39ee56739f34c799946d4454;p=lilypond.git Fix 1259/1433: linebreaks with \breakDynamicSpan or spanners with style=#'none This patch changes the way DynamicLineSpanners are handled when \breakDynamicSpan is called or when the DynamicTextSpanner's style=#'none (i.e. no line should be used and thus the spanner should also not unneccessarily shift the dynamic text spanner up/down). So far, this was handled by simply ending the DynamicLineSpanner prematurely, while leaving its children at their full length. As a consequence, the child spanners were no longer fully contained in the DynamicLineSpanner, which caused several problem at line breaks. This patch changes it as follows: -) All spanner breaks are handled by a flag set on the spanner itself -) The breakDynamicSpan events are handled by the dynamic engraver, which will set that flag (no longer handled by the dynamic span engraver!). This allows to obey the order of \breakDynamicSpan and \> (i.e. if the break was before the \< then break the previous spanner, if it was placed afterwards then end the newly created spanner) -) When a DynamicTextSpanner with style=#'none or a \breakDynamicSpan is encountered, I also set that flag immediately after spanner creation -) From then on, no new dynamics are added to the line spanner and no new support points are added that would otherwise shift the spanner if there is a very high/low note or articulation. If the spanner creates a grob (like a hairpin), that grob would still be shifted as a grob to prevent collisions, though. -) When the current child spanner is ended, we also end the DynamicLineSpanner (and store it in a temporary variable so that we can properly end it in stop_translation_timestep). If a new dynamic is encountered, it will then create a completely new DynamicLineSpanner, which provides the independent alignment that we want. This fixes both issues 1259 and 1433. --- diff --git a/input/regression/dynamics-alignment-breaker-linebreak.ly b/input/regression/dynamics-alignment-breaker-linebreak.ly new file mode 100644 index 0000000000..b4e9f2df4a --- /dev/null +++ b/input/regression/dynamics-alignment-breaker-linebreak.ly @@ -0,0 +1,17 @@ +\version "2.15.7" + +\header { + texidoc = "@code{\\breakDynamicSpan} shall also work if a +dynamic spanner crosses a line break. +" +} + +\relative c' { + % spanner really crosses linebreak: + c1\<\breakDynamicSpan c'' \break + c,,1 + % new spanner immediately after linebreak (with broken spanner): + c''1\>\breakDynamicSpan \break + c,,1\< + f,1\breakDynamicSpan\p +} diff --git a/input/regression/dynamics-alignment-breaker-order.ly b/input/regression/dynamics-alignment-breaker-order.ly new file mode 100644 index 0000000000..1647b204c4 --- /dev/null +++ b/input/regression/dynamics-alignment-breaker-order.ly @@ -0,0 +1,18 @@ +\version "2.15.7" + +\header { + texidoc = "@code{\\breakDynamicSpan} work whether it is placed together +with the start or the end of a spanner. Both lines should be identical. +" +} + +\relative c { + c1\< c'' + % break directly before and after \> : + c,1\breakDynamicSpan\>\breakDynamicSpan + f,1\p \break + + c1\<\breakDynamicSpan c'' + c,1\> + f,1\breakDynamicSpan\p +} diff --git a/input/regression/dynamics-alignment-breaker-subsequent-spanner.ly b/input/regression/dynamics-alignment-breaker-subsequent-spanner.ly new file mode 100644 index 0000000000..f2a397188c --- /dev/null +++ b/input/regression/dynamics-alignment-breaker-subsequent-spanner.ly @@ -0,0 +1,15 @@ +\version "2.15.7" + +\header { + texidoc = "@code{\\breakDynamicSpan} shall only have an effect on the current +spanner, not on subsequent spanners. +" +} + +\relative c' { + % Check that the effect of \breakDynamic span is only for the current + % spanner and not for the following spanners, too. + c1\<\breakDynamicSpan c'' + c,,1\> + f,1\p % <= the \> and the \p should be aligned! +} diff --git a/input/regression/dynamics-alignment-no-line-linebreak.ly b/input/regression/dynamics-alignment-no-line-linebreak.ly new file mode 100644 index 0000000000..28dbdce532 --- /dev/null +++ b/input/regression/dynamics-alignment-no-line-linebreak.ly @@ -0,0 +1,14 @@ +\version "2.15.6" + +\header { + texidoc = "Setting the style of a @code{DynamicTextSpanner} to @code{'none} +to hide the line altogether should also work over line breaks. +" +} + +\relative c'' { + \override DynamicTextSpanner #'style = #'none + c2\cresc g,2 + \break + g2 c'2\f +} diff --git a/lily/dynamic-align-engraver.cc b/lily/dynamic-align-engraver.cc index f62c45f0dc..591ffee733 100644 --- a/lily/dynamic-align-engraver.cc +++ b/lily/dynamic-align-engraver.cc @@ -34,7 +34,6 @@ class Dynamic_align_engraver : public Engraver { TRANSLATOR_DECLARATIONS (Dynamic_align_engraver); - DECLARE_TRANSLATOR_LISTENER (break_span); DECLARE_ACKNOWLEDGER (note_column); DECLARE_ACKNOWLEDGER (dynamic); DECLARE_ACKNOWLEDGER (footnote_spanner); @@ -45,21 +44,23 @@ protected: private: void create_line_spanner (Stream_event *cause); + void set_spanner_bounds (Spanner *line, bool end); Spanner *line_; + Spanner *ended_line_; // Spanner manually broken, don't use it for new grobs + Spanner *current_dynamic_spanner_; vector ended_; vector started_; vector scripts_; vector support_; set running_; - - bool early_end_; }; Dynamic_align_engraver::Dynamic_align_engraver () { line_ = 0; - early_end_ = false; + ended_line_ = 0; + current_dynamic_spanner_ = 0; } ADD_ACKNOWLEDGER (Dynamic_align_engraver, dynamic); @@ -80,6 +81,20 @@ Dynamic_align_engraver::acknowledge_end_dynamic (Grob_info info) { if (Spanner::has_interface (info.grob ())) ended_.push_back (info.spanner ()); + + /* If the break flag is set, store the current spanner and let new dynamics + * create a new spanner + */ + bool spanner_broken = current_dynamic_spanner_ == info.spanner () && + to_boolean (current_dynamic_spanner_->get_property ("spanner-broken")); + if (spanner_broken && line_) + { + if (ended_line_) + programming_error ("already have a force-ended DynamicLineSpanner."); + ended_line_ = line_; + line_ = 0; + current_dynamic_spanner_ = 0; + } } void @@ -97,14 +112,6 @@ Dynamic_align_engraver::acknowledge_note_column (Grob_info info) support_.push_back (info.grob ()); } -IMPLEMENT_TRANSLATOR_LISTENER (Dynamic_align_engraver, break_span); -void -Dynamic_align_engraver::listen_break_span (Stream_event *event) -{ - if (event->in_event_class ("break-dynamic-span-event")) - early_end_ = true; -} - void Dynamic_align_engraver::acknowledge_dynamic (Grob_info info) { @@ -113,14 +120,7 @@ Dynamic_align_engraver::acknowledge_dynamic (Grob_info info) if (Spanner::has_interface (info.grob ())) { started_.push_back (info.spanner ()); - /* - If we are using text spans instead of hairpins and the line - is hidden, end the alignment spanner early: this allows dynamics - to be spaced individually instead of being linked together. - */ - if (info.grob ()->internal_has_interface (ly_symbol2scm ("dynamic-text-spanner-interface")) - && (info.grob ()->get_property ("style") == ly_symbol2scm ("none"))) - early_end_ = true; + current_dynamic_spanner_ = info.spanner (); } else if (info.item ()) scripts_.push_back (info.item ()); @@ -131,35 +131,24 @@ Dynamic_align_engraver::acknowledge_dynamic (Grob_info info) if (cause) { + // TODO: Compare the direction of the existing spanner with + // the new one and if they differ, create a new line + // spanner. if (Direction d = to_dir (cause->get_property ("direction"))) set_grob_direction (line_, d); } } void -Dynamic_align_engraver::stop_translation_timestep () +Dynamic_align_engraver::set_spanner_bounds (Spanner *line, bool end) { - for (vsize i = 0; i < started_.size (); i++) - running_.insert (started_[i]); - for (vsize i = 0; i < ended_.size (); i++) - { - Spanner *sp = ended_[i]; - - set::iterator it = running_.find (sp); - if (it != running_.end ()) - running_.erase (it); - else - started_[i]->programming_error ("lost track of this dynamic spanner"); - } - - bool end = line_ && (running_.empty () - || early_end_); + if (!line) + return; Direction d = LEFT; do { - if (line_ - && ((d == LEFT && !line_->get_bound (LEFT)) - || (end && d == RIGHT && !line_->get_bound (RIGHT)))) + if ((d == LEFT && !line->get_bound (LEFT)) || + (end && d == RIGHT && !line->get_bound (RIGHT))) { vector const &spanners = (d == LEFT) ? started_ : ended_; @@ -172,24 +161,51 @@ Dynamic_align_engraver::stop_translation_timestep () else { bound = unsmob_grob (get_property ("currentMusicalColumn")); - if (!early_end_) - programming_error ("started DynamicLineSpanner but have no left bound"); + programming_error ("started DynamicLineSpanner but have no left bound"); } - line_->set_bound (d, bound); + line->set_bound (d, bound); } } while (flip (&d) != LEFT); +} + +void +Dynamic_align_engraver::stop_translation_timestep () +{ + for (vsize i = 0; i < started_.size (); i++) + running_.insert (started_[i]); + for (vsize i = 0; i < ended_.size (); i++) + { + Spanner *sp = ended_[i]; + + set::iterator it = running_.find (sp); + if (it != running_.end ()) + running_.erase (it); + else + started_[i]->programming_error ("lost track of this dynamic spanner"); + } - for (vsize i = 0; line_ && i < support_.size (); i++) + bool end = line_ && running_.empty (); + + // Set the proper bounds for the current spanner and for a spanner that + // is ended now + set_spanner_bounds (ended_line_, true); + set_spanner_bounds (line_, end); + // If the flag is set to break the spanner after the current child, don't + // add any more support points (needed e.g. for style=none, where the + // invisible spanner should NOT be shifted since we don't have a line). + bool spanner_broken = current_dynamic_spanner_ && + to_boolean (current_dynamic_spanner_->get_property ("spanner-broken")); + for (vsize i = 0; line_ && !spanner_broken && i < support_.size (); i++) Side_position_interface::add_support (line_, support_[i]); if (end) { line_ = 0; - early_end_ = false; } + ended_line_ = 0; ended_.clear (); started_.clear (); scripts_.clear (); diff --git a/lily/new-dynamic-engraver.cc b/lily/new-dynamic-engraver.cc index e225d97f1f..331d605616 100644 --- a/lily/new-dynamic-engraver.cc +++ b/lily/new-dynamic-engraver.cc @@ -36,6 +36,7 @@ class New_dynamic_engraver : public Engraver DECLARE_ACKNOWLEDGER (note_column); DECLARE_TRANSLATOR_LISTENER (absolute_dynamic); DECLARE_TRANSLATOR_LISTENER (span_dynamic); + DECLARE_TRANSLATOR_LISTENER (break_span); protected: virtual void process_music (); @@ -54,6 +55,7 @@ private: Item *script_; Stream_event *script_event_; Stream_event *current_span_event_; + bool end_new_spanner_; }; New_dynamic_engraver::New_dynamic_engraver () @@ -64,6 +66,7 @@ New_dynamic_engraver::New_dynamic_engraver () finished_spanner_ = 0; current_spanner_ = 0; accepted_spanevents_drul_.set (0, 0); + end_new_spanner_ = false; } IMPLEMENT_TRANSLATOR_LISTENER (New_dynamic_engraver, absolute_dynamic); @@ -82,6 +85,22 @@ New_dynamic_engraver::listen_span_dynamic (Stream_event *ev) ASSIGN_EVENT_ONCE (accepted_spanevents_drul_[d], ev); } +IMPLEMENT_TRANSLATOR_LISTENER (New_dynamic_engraver, break_span); +void +New_dynamic_engraver::listen_break_span (Stream_event *event) +{ + if (event->in_event_class ("break-dynamic-span-event")) + { + // Case 1: Already have a start dynamic event -> break applies to new + // spanner (created later) -> set a flag + // Case 2: no new spanner, but spanner already active -> break it now + if (accepted_spanevents_drul_[START]) + end_new_spanner_ = true; + else if (current_spanner_) + current_spanner_->set_property ("spanner-broken", SCM_BOOL_T); + } +} + SCM New_dynamic_engraver::get_property_setting (Stream_event *evt, char const *evprop, @@ -132,6 +151,13 @@ New_dynamic_engraver::process_music () (start_type + "Text").c_str ()); if (Text_interface::is_markup (text)) current_spanner_->set_property ("text", text); + /* + If the line of a text spanner is hidden, end the alignment spanner + early: this allows dynamics to be spaced individually instead of + being linked together. + */ + if (current_spanner_->get_property ("style") == ly_symbol2scm ("none")) + current_spanner_->set_property ("spanner-broken", SCM_BOOL_T); } else { @@ -144,6 +170,12 @@ New_dynamic_engraver::process_music () current_spanner_ = make_spanner ("Hairpin", current_span_event_->self_scm ()); } + // if we have a break-dynamic-span event right after the start dynamic, break the new spanner immediately + if (end_new_spanner_) + { + current_spanner_->set_property ("spanner-broken", SCM_BOOL_T); + end_new_spanner_ = false; + } if (finished_spanner_) { if (Hairpin::has_interface (finished_spanner_)) @@ -186,6 +218,7 @@ New_dynamic_engraver::stop_translation_timestep () script_event_ = 0; accepted_spanevents_drul_.set (0, 0); finished_spanner_ = 0; + end_new_spanner_ = false; } void diff --git a/lily/spanner.cc b/lily/spanner.cc index 22282aa231..18a7e645f5 100644 --- a/lily/spanner.cc +++ b/lily/spanner.cc @@ -547,6 +547,7 @@ ADD_INTERFACE (Spanner, /* properties */ "normalized-endpoints " "minimum-length " + "spanner-broken " "spanner-id " "to-barline " ); diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm index 40732d0e9d..3f9da21f6d 100644 --- a/scm/define-grob-properties.scm +++ b/scm/define-grob-properties.scm @@ -1058,6 +1058,8 @@ grobs.") (spacing-wishes ,ly:grob-array? "An array of note spacing or staff spacing objects.") (span-start ,boolean? "Is the note head at the start of a spanner?") + (spanner-broken ,boolean? "Indicates whether spanner +alignment should be broken after the current spanner.") (spanner-placement ,ly:dir? "The place of an annotation on a spanner. LEFT is for the first spanner, and RIGHT is for the last. CENTER will place it on the broken spanner that falls closest to the center of the length