lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Creates pure conversion for Flag::calc_y_offset (issue 5934050)


From: mtsolo
Subject: Re: Creates pure conversion for Flag::calc_y_offset (issue 5934050)
Date: Thu, 29 Mar 2012 06:27:14 +0000

Reviewers: Keith,

Message:
There is a nasty bug in LilyPond that this patch only exacerbates - as a
rule of thumb, X positioning functions should not set Y positioning
variables and vice versa.  note-collision.cc, which works from an X_AXIS
callback, sets the stem attachment, which is a both X and Y axis value.
This is a recipe for trouble - there even if it means logic / code dup,
these should be separated out into two functions: one that sets the X
value and one that sets the Y value.  As a result, I have to trigger the
X extent in Stem::internal_stem_begin_position to get the Y position set
correctly.  It may be worth it to fix this now: it'd delay 2.16 further,
but it seems like a sound architecture decision that'd prevent this
sorta problem from happening in the future.

Description:
Creates pure conversion for Flag::calc_y_offset

Please review this at http://codereview.appspot.com/5934050/

Affected files:
  A input/regression/stem-begin-position.ly
  M lily/stem.cc


Index: input/regression/stem-begin-position.ly
diff --git a/input/regression/stem-begin-position.ly b/input/regression/stem-begin-position.ly
new file mode 100644
index 0000000000000000000000000000000000000000..30bd3ff1e3865c53f50a3ab7e03630b62fff5e1f
--- /dev/null
+++ b/input/regression/stem-begin-position.ly
@@ -0,0 +1,10 @@
+\version "2.15.36"
+
+\header {
+  texidoc = "Stems reach correct begin points of merged noteheads.
+"
+}
+
+<< { \aikenHeads f'8 }  \\ { \aikenHeads f'8 } >>
+<< { \aikenHeads f'4:32 }  \\ { \aikenHeads f' } >>
+<< { \aikenHeads e'8 f' s4 }  \\ { \aikenHeads e'8 f' s4 } >>
Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index f662f3cf1dec75637680e3c85f9bcd9fd31a158c..0e8c13cdaa1ff7edc67ffe930c4496fbd841b243 100644
--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -787,6 +787,14 @@ Stem::internal_calc_stem_begin_position (Grob *me, bool calc_beam)
   if (Grob *head = support_head (me))
     {
       Interval head_height = head->extent (head, Y_AXIS);
+      // trigger positioning of stems in note column in case of merge
+      if (me->get_parent (X_AXIS)
+          && me->get_parent (X_AXIS)->get_parent (X_AXIS))
+        (void) me->get_parent (X_AXIS)
+                 ->extent (me->get_parent (X_AXIS)
+                             ->get_parent (X_AXIS), X_AXIS);
+      else
+ me->programming_error ("Stem doesn't belong to NoteColumn or PaperColumn.");
       Real y_attach = Note_head::stem_attachment_coordinate (head, Y_AXIS);

       y_attach = head_height.linear_combination (y_attach);





reply via email to

[Prev in Thread] Current Thread [Next in Thread]