From 8a57f497f6c4f9f00c17040d3c41d30eb2d1b765 Mon Sep 17 00:00:00 2001 From: David Kastrup Date: Wed, 6 Jun 2012 15:38:23 +0200 Subject: [PATCH] Issue 2584 (redo 1967): please make partcombine merge slurs The partcombiner does not really bother about keeping the number of generated start and end slur events matched, so this attempts to cope by implementing the following behavior: a) multiple slur starts on the same moment are not an error but the same as one. b) multiple slur ends on the same moment are not an error but the same as one. a2) there will be a slur with direction UP if there is at least one such start event, and there will be a slur with direction DOWN if there is at least one such end event. This might imply a double slur, but for ending it, a single slur end is sufficient. Consequently, a^(_( c)) (the second closing paren not required, just added for prettiness) will add a double slur. --- input/regression/phrasing-slur-multiple.ly | 14 ++-- input/regression/slur-multiple.ly | 14 ++-- lily/phrasing-slur-engraver.cc | 80 ++++++++++++++++++---- lily/slur-engraver.cc | 77 +++++++++++++++++---- 4 files changed, 146 insertions(+), 39 deletions(-) diff --git a/input/regression/phrasing-slur-multiple.ly b/input/regression/phrasing-slur-multiple.ly index f9ccca61b2..dc1edfc44a 100644 --- a/input/regression/phrasing-slur-multiple.ly +++ b/input/regression/phrasing-slur-multiple.ly @@ -1,8 +1,9 @@ -\version "2.15.4" +\version "2.15.40" #(ly:set-option 'warning-as-error #f) #(ly:expect-warning (_ "already have phrasing slur")) #(ly:expect-warning (_ "cannot end phrasing slur")) +#(ly:expect-warning (_ "unterminated phrasing slur")) \header { texidoc = "LilyPond does not support multiple concurrent phrasing slurs with the @@ -11,13 +12,14 @@ slur will not be generated. However, one can can create a second slur with a different spanner-id." } -altPhSlur = #(make-music 'PhrasingSlurEvent 'span-direction START 'spanner-id "alt") -altPhSlurEnd = #(make-music 'PhrasingSlurEvent 'span-direction STOP 'spanner-id "alt") +sp=#(define-event-function (parser location n e) (index? ly:event?) + (set! (ly:music-property e 'spanner-id) (format "sp~a" n)) + e) \relative c'' { % This will give warnings ("Already have phrasing slur" and "Cannot end phrasing slur") - c4\(\( d4\)\( e4\) f\) | - % This will give two overlapping slurs: - d\( d\altPhSlur e\) f\altPhSlurEnd | + c4\(\(\sp1\( d4\)\(\sp1\( e4\) f\) | + % This will give two overlapping slurs and "unterminated phrasing slur" from above + d\( d\sp2\( e\) f\sp2\) | } diff --git a/input/regression/slur-multiple.ly b/input/regression/slur-multiple.ly index f73d93a316..42c859afcd 100644 --- a/input/regression/slur-multiple.ly +++ b/input/regression/slur-multiple.ly @@ -1,8 +1,9 @@ -\version "2.15.5" +\version "2.15.40" #(ly:set-option 'warning-as-error #f) #(ly:expect-warning (_ "already have slur")) #(ly:expect-warning (_ "cannot end slur")) +#(ly:expect-warning (_ "unterminated slur")) \header { texidoc = "LilyPond does not support multiple concurrent slurs with the @@ -11,13 +12,14 @@ slur will not be generated. However, one can can create a second slur with a different spanner-id." } -altSlur = #(make-music 'SlurEvent 'span-direction START 'spanner-id "alt") -altSlurEnd = #(make-music 'SlurEvent 'span-direction STOP 'spanner-id "alt") +sp=#(define-event-function (parser location n e) (index? ly:event?) + (set! (ly:music-property e 'spanner-id) (format "sp~a" n)) + e) \relative c'' { % This will give warnings ("Already have slur" and "Cannot end slur") - c4(( d4)( e4) f) | - % This will give two overlapping slurs: - d( d\altSlur e) f\altSlurEnd | + c4((\sp1( d4)(\sp1( e4) f) | + % This will give two overlapping slurs and "unterminated slur" from above + d( d\sp2( e) f\sp2) | } diff --git a/lily/phrasing-slur-engraver.cc b/lily/phrasing-slur-engraver.cc index a3b10d027a..ea2439534c 100644 --- a/lily/phrasing-slur-engraver.cc +++ b/lily/phrasing-slur-engraver.cc @@ -175,7 +175,7 @@ Phrasing_slur_engraver::process_music () Stream_event *ev = stop_events_[i]; string id = robust_scm2string (ev->get_property ("spanner-id"), ""); - // Find the slur that is ended with this event (by checking the spanner-id) + // Find the slurs that are ended with this event (by checking the spanner-id) bool ended = false; for (vsize j = slurs_.size (); j--;) { @@ -186,26 +186,80 @@ Phrasing_slur_engraver::process_music () slurs_.erase (slurs_.begin () + j); } } - if (!ended) + if (ended) + { + // Ignore redundant stop events for this id + for (vsize j = stop_events_.size (); --j > i;) + { + if (id == robust_scm2string (stop_events_[j]->get_property ("spanner-id"), "")) + stop_events_.erase (stop_events_.begin() + j); + } + } + else ev->origin ()->warning (_ ("cannot end phrasing slur")); } - for (vsize i = 0; i < start_events_.size (); i++) + vsize old_slurs = slurs_.size (); + for (vsize i = start_events_.size (); i--;) { Stream_event *ev = start_events_[i]; string id = robust_scm2string (ev->get_property ("spanner-id"), ""); - bool have_slur = false; - // Check if we already have a slur with the same spanner-id. - // In that case, don't create a new slur, but print a warning - for (vsize i = 0; i < slurs_.size (); i++) - have_slur = have_slur || (id == robust_scm2string (slurs_[i]->get_property ("spanner-id"), "")); - - if (have_slur) - ev->origin ()->warning (_ ("already have phrasing slur")); - else + Direction updown = to_dir (ev->get_property ("direction")); + + bool completed; + for (vsize j = 0; !(completed = (j == slurs_.size ())); j++) + { + // Check if we already have a slur with the same spanner-id. + if (id == robust_scm2string (slurs_[j]->get_property ("spanner-id"), "")) + { + if (j < old_slurs) + { + // We already have an old slur, so give a warning + // and completely ignore the new slur. + ev->origin ()->warning (_ ("already have phrasing slur")); + start_events_.erase (start_events_.begin () + i); + break; + } + + // If this slur event has no direction, it will not + // contribute anything new to the existing slur(s), so + // we can ignore it. This is not entirely accurate: + // tweaks or context properties like those set with + // \slurUp can still override a neutral direction, so + // when encountering a slur event with "opposite" + // direction first, then one with neutral direction, we + // only let the "opposite" direction remain, while if + // the order is the other way round, a double slur + // results since the direction of the first slur is no + // longer attributable to a "neutral" slur event. A + // mixture of neutral and directed events is nothing + // that the partcombiner should crank out, and it would + // be decidedly strange for manual input. + + if (!updown) + break; + + // If the existing slur does not have a direction yet, + // give it ours + + Direction slur_dir = to_dir (slurs_[j]->get_property ("direction")); + + if (!slur_dir) + { + set_grob_direction (slurs_[j], updown); + break; + } + + // If the existing slur has the same direction as ours, drop ours + + if (slur_dir == updown) + break; + } + } + // If the loop completed, our slur is new + if (completed) { Grob *slur = make_spanner ("PhrasingSlur", ev->self_scm ()); - Direction updown = to_dir (ev->get_property ("direction")); slur->set_property ("spanner-id", ly_string2scm (id)); if (updown) set_grob_direction (slur, updown); diff --git a/lily/slur-engraver.cc b/lily/slur-engraver.cc index 40b3b6bd63..e496285288 100644 --- a/lily/slur-engraver.cc +++ b/lily/slur-engraver.cc @@ -176,7 +176,7 @@ Slur_engraver::process_music () Stream_event *ev = stop_events_[i]; string id = robust_scm2string (ev->get_property ("spanner-id"), ""); - // Find the slur that is ended with this event (by checking the spanner-id) + // Find the slurs that are ended with this event (by checking the spanner-id) bool ended = false; for (vsize j = slurs_.size (); j--;) { @@ -187,31 +187,80 @@ Slur_engraver::process_music () slurs_.erase (slurs_.begin () + j); } } - if (!ended) + if (ended) + { + // Ignore redundant stop events for this id + for (vsize j = stop_events_.size (); --j > i;) + { + if (id == robust_scm2string (stop_events_[j]->get_property ("spanner-id"), "")) + stop_events_.erase (stop_events_.begin() + j); + } + } + else ev->origin ()->warning (_ ("cannot end slur")); } + vsize old_slurs = slurs_.size (); for (vsize i = start_events_.size (); i--;) { Stream_event *ev = start_events_[i]; string id = robust_scm2string (ev->get_property ("spanner-id"), ""); - bool have_slur = false; - // Check if we already have a slur with the same spanner-id. - // In that case, don't create a new slur, but print a warning - for (vsize j = 0; j < slurs_.size (); j++) - have_slur = have_slur || (id == robust_scm2string (slurs_[j]->get_property ("spanner-id"), "")); + Direction updown = to_dir (ev->get_property ("direction")); - if (have_slur) + bool completed; + for (vsize j = 0; !(completed = (j == slurs_.size ())); j++) { - // We already have a slur, so give a warning and completely ignore - // the new slur. - ev->origin ()->warning (_ ("already have slur")); - start_events_.erase (start_events_.begin () + i); + // Check if we already have a slur with the same spanner-id. + if (id == robust_scm2string (slurs_[j]->get_property ("spanner-id"), "")) + { + if (j < old_slurs) + { + // We already have an old slur, so give a warning + // and completely ignore the new slur. + ev->origin ()->warning (_ ("already have slur")); + start_events_.erase (start_events_.begin () + i); + break; + } + + // If this slur event has no direction, it will not + // contribute anything new to the existing slur(s), so + // we can ignore it. This is not entirely accurate: + // tweaks or context properties like those set with + // \slurUp can still override a neutral direction, so + // when encountering a slur event with "opposite" + // direction first, then one with neutral direction, we + // only let the "opposite" direction remain, while if + // the order is the other way round, a double slur + // results since the direction of the first slur is no + // longer attributable to a "neutral" slur event. A + // mixture of neutral and directed events is nothing + // that the partcombiner should crank out, and it would + // be decidedly strange for manual input. + + if (!updown) + break; + + // If the existing slur does not have a direction yet, + // give it ours + + Direction slur_dir = to_dir (slurs_[j]->get_property ("direction")); + + if (!slur_dir) + { + set_grob_direction (slurs_[j], updown); + break; + } + + // If the existing slur has the same direction as ours, drop ours + + if (slur_dir == updown) + break; + } } - else + // If the loop completed, our slur is new + if (completed) { Grob *slur = make_spanner ("Slur", ev->self_scm ()); - Direction updown = to_dir (ev->get_property ("direction")); slur->set_property ("spanner-id", ly_string2scm (id)); if (updown) set_grob_direction (slur, updown); -- 2.39.2