Reviewers: Trevor Daniels,
Message:
On 2012/07/19 09:30:15, Trevor Daniels wrote:
Just nitpicking; can't comment substantively.
Trevor
http://codereview.appspot.com/6432047/diff/1/lily/phrasing-slur-engraver.cc
File lily/phrasing-slur-engraver.cc (right):
http://codereview.appspot.com/6432047/diff/1/lily/phrasing-slur-engraver.cc#newcode256
lily/phrasing-slur-engraver.cc:256: slurs_[j]->suicide ();
no tabs please
http://codereview.appspot.com/6432047/diff/1/lily/phrasing-slur-engraver.cc#newcode258
lily/phrasing-slur-engraver.cc:258: continue;
no tabs please
http://codereview.appspot.com/6432047/diff/1/lily/slur-engraver.cc
File lily/slur-engraver.cc (right):
http://codereview.appspot.com/6432047/diff/1/lily/slur-engraver.cc#newcode257
lily/slur-engraver.cc:257: slurs_[j]->suicide ();
no tabs please
http://codereview.appspot.com/6432047/diff/1/lily/slur-engraver.cc#newcode259
lily/slur-engraver.cc:259: continue;
no tabs please
Ran M-x untabify RET on both files, but won't recommit to testing for
just this spacing change.
Description:
reopened Issue 2584: please make partcombine merge slurs
This refrains from referencing a slur grob's direction field (often
inconclusive and not representative of the input), instead referencing
the direction of the causing event.
Please review this at http://codereview.appspot.com/6432047/
Affected files:
M lily/phrasing-slur-engraver.cc
M lily/slur-engraver.cc
Index: lily/phrasing-slur-engraver.cc
diff --git a/lily/phrasing-slur-engraver.cc b/lily/phrasing-slur-engraver.cc
index
ff1bd4e51d5d0581b4dc5a7cad08c9d6aac70262..b87d649e015d8d3e01ada65077fb8774d99f22ec
100644
--- a/lily/phrasing-slur-engraver.cc
+++ b/lily/phrasing-slur-engraver.cc
@@ -218,7 +218,7 @@ Phrasing_slur_engraver::process_music ()
Direction updown = to_dir (ev->get_property ("direction"));
bool completed;
- for (vsize j = 0; !(completed = (j == slurs_.size ())); j++)
+ for (vsize j = slurs_.size (); !(completed = (j-- == 0));)
{
// Check if we already have a slur with the same spanner-id.
if (id == robust_scm2string (slurs_[j]->get_property
("spanner-id"), ""))
@@ -234,31 +234,28 @@ Phrasing_slur_engraver::process_music ()
// 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.
+ // we can ignore it.
if (!updown)
break;
- // If the existing slur does not have a direction yet,
- // give it ours
+ Stream_event *c = unsmob_stream_event (slurs_[j]->get_property
("cause"));
+
+ if (!c) {
+ slurs_[j]->programming_error ("phrasing slur without a cause");
+ continue;
+ }
- Direction slur_dir = to_dir (slurs_[j]->get_property
("direction"));
+ Direction slur_dir = to_dir (c->get_property ("direction"));
+
+ // If the existing slur does not have a direction yet,
+ // we'd rather take the new one.
if (!slur_dir)
{
- set_grob_direction (slurs_[j], updown);
- break;
+ slurs_[j]->suicide ();
+ slurs_.erase (slurs_.begin () + j);
+ continue;
}
// If the existing slur has the same direction as ours, drop
ours
Index: lily/slur-engraver.cc
diff --git a/lily/slur-engraver.cc b/lily/slur-engraver.cc
index
fde4d9447def65123f93532dc8421a8420e9a634..d88ff4ea5776dd17ed4b32bc57ef95adb020f21f
100644
--- a/lily/slur-engraver.cc
+++ b/lily/slur-engraver.cc
@@ -219,7 +219,7 @@ Slur_engraver::process_music ()
Direction updown = to_dir (ev->get_property ("direction"));
bool completed;
- for (vsize j = 0; !(completed = (j == slurs_.size ())); j++)
+ for (vsize j = slurs_.size (); !(completed = (j-- == 0));)
{
// Check if we already have a slur with the same spanner-id.
if (id == robust_scm2string (slurs_[j]->get_property
("spanner-id"), ""))
@@ -235,31 +235,28 @@ Slur_engraver::process_music ()
// 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.
+ // we can ignore it.
if (!updown)
break;
- // If the existing slur does not have a direction yet,
- // give it ours
+ Stream_event *c = unsmob_stream_event (slurs_[j]->get_property
("cause"));
+
+ if (!c) {
+ slurs_[j]->programming_error ("slur without a cause");
+ continue;
+ }
- Direction slur_dir = to_dir (slurs_[j]->get_property
("direction"));
+ Direction slur_dir = to_dir (c->get_property ("direction"));
+
+ // If the existing slur does not have a direction yet,
+ // we'd rather take the new one.
if (!slur_dir)
{
- set_grob_direction (slurs_[j], updown);
- break;
+ slurs_[j]->suicide ();
+ slurs_.erase (slurs_.begin () + j);
+ continue;
}
// If the existing slur has the same direction as ours, drop
ours
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
