Reviewers: Neil Puttock,

http://codereview.appspot.com/6453/diff/1/3
File lily/arpeggio.cc (right):

http://codereview.appspot.com/6453/diff/1/3#newcode108
Line 108: y += rint (squiggle.extent (Y_AXIS).length ()))
while the original is obviously wrong for the .ly you show, the fix
looks fishy to me as you would be accumulating roundoff errors. The loop
should be

while (mol.extent(Y_AXIS) <  heads[RIGHT])
 { add_at_edge }


perhaps to prevent the problem, there could be something like

while (mol.extent(Y_AXIS) + 1.0 <  heads[RIGHT])
 { add_at_edge }

(where 1.0 is intelligently chosen.)

Description:
* Fix arpeggio overshoot for chords which reach centre line.
This patch fixes a rounding error in Arpeggio::print () that occurs when
a
chord reaches the centre line, resulting in an extra squiggle being
added.

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

Affected files:
  A input/regression/arpeggio-no-overshoot.ly
  M lily/arpeggio.cc


Index: input/regression/arpeggio-no-overshoot.ly
diff --git a/input/regression/arpeggio-no-overshoot.ly b/input/regression/arpeggio-no-overshoot.ly
new file mode 100644
index 0000000000000000000000000000000000000000..10a8debce8d67c612838c0d2e2f1d2d41d2707c7
--- /dev/null
+++ b/input/regression/arpeggio-no-overshoot.ly
@@ -0,0 +1,15 @@
+\version "2.11.61"
+
+\header {
+  texidoc = "Arpeggios do not overshoot the highest note head.
+The first chord in this example simulates overshoot using
[EMAIL PROTECTED]'positions} for comparison with the correct behaviour."
+}
+
+\relative c' {
+  % simulate overshoot for comparison
+  \once \override Arpeggio #'positions = #'(-3 . 1)
+  <c e g b>1\arpeggio
+  <c e g b>1\arpeggio
+  <f a c>2\arpeggio <g b d f>\arpeggio
+}
Index: lily/arpeggio.cc
diff --git a/lily/arpeggio.cc b/lily/arpeggio.cc
index 80858426bc6eb43bc7f12e2f77a83aec4351d8b3..5b4f8e338b28ab4423b7071cd7e974790cb4bb0b 100644
--- a/lily/arpeggio.cc
+++ b/lily/arpeggio.cc
@@ -105,7 +105,7 @@ Arpeggio::print (SCM smob)
     }

   for (Real y = heads[LEFT]; y < heads[RIGHT];
-       y += squiggle.extent (Y_AXIS).length ())
+       y += rint (squiggle.extent (Y_AXIS).length ()))
     mol.add_at_edge (Y_AXIS, UP, squiggle, 0.0);

   mol.translate_axis (heads[LEFT], Y_AXIS);




_______________________________________________
lilypond-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/lilypond-devel

Reply via email to