lilypond-devel
[Top][All Lists]
Advanced

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

Fix calculation of vertical offset when 'staff-padding is set (issue4489


From: tdanielsmusic
Subject: Fix calculation of vertical offset when 'staff-padding is set (issue4489042)
Date: Fri, 06 May 2011 22:22:19 +0000

Reviewers: ,

Message:
This is a fix for issue 877, "Ottava clefs may not look good".  See
  http://code.google.com/p/lilypond/issues/detail?id=877

The regtests run clean except for
  cue-clef-octavation.ly
  skiptypesetting-multimeasurerest.ly
both of which are improved, and 1 pixel differences in
  clef-oct.ly

Rather surprisingly this is a bug in
  Side_position_interface::aligned_side ()
but it only appears to affect clefs.

Please review.

Trevor

Description:
Fix calculation of vertical offset when 'staff-padding is set

 - fix 877: Ottava clefs may not look good

 - the previous code incorrectly calculated the offset from
   the staff rather than the parent; this happened to coincide
   with the correct value for most parents but gave an incorrect
   offset for OctavateEight above the F-clef

 - add regression test for this case

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

Affected files:
  A input/regression/clef-octavation.ly
  M lily/side-position-interface.cc


Index: input/regression/clef-octavation.ly
diff --git a/input/regression/clef-octavation.ly b/input/regression/clef-octavation.ly
new file mode 100644
index 0000000000000000000000000000000000000000..fe1fd61e4586c5e7c18d811c19a42148873da346
--- /dev/null
+++ b/input/regression/clef-octavation.ly
@@ -0,0 +1,19 @@
+\version "2.15.0"
+
+\header{
+
+  texidoc=" Octavate symbols should be correctly positioned close to
+the parent clef."
+
+}
+\score {
+  <<
+      \new Staff { \clef "G^8" g''1 }
+      \new Staff { \clef "F^8" c'1 }
+      \new Staff { \clef "C^8" c''1 }
+      \new Staff { \clef "G_8" g1 }
+      \new Staff { \clef "F_8" c,1 }
+      \new Staff { \clef "C_8" c1 }
+  >>
+  \layout { ragged-right = ##t }
+}
Index: lily/side-position-interface.cc
diff --git a/lily/side-position-interface.cc b/lily/side-position-interface.cc index 4fe16e510b46ad871fd8e59a01e0b2f668889e2d..0e60b0533483ce7f6f0088644a31bb5f6dcbc213 100644
--- a/lily/side-position-interface.cc
+++ b/lily/side-position-interface.cc
@@ -62,7 +62,7 @@ Side_position_interface::general_side_position (Grob *me, Axis a, bool use_exten

   Grob *common = common_refpoint_of_array (support, me->get_parent (a), a);
   Grob *staff_symbol = Staff_symbol_referencer::get_staff_symbol (me);
-  bool include_staff =
+  bool include_staff =
     staff_symbol
     && a == Y_AXIS
     && scm_is_number (me->get_property ("staff-padding"))
@@ -92,7 +92,7 @@ Side_position_interface::general_side_position (Grob *me, Axis a, bool use_exten
          && Stem::has_interface (e)
          && dir == - get_grob_direction (e))
        continue;
-
+
       if (e)
        {
          if (use_extents)
@@ -136,8 +136,8 @@ Side_position_interface::general_side_position (Grob *me, Axis a, bool use_exten
   if (current_offset)
     total_off = dir * max (dir * total_off,
                           dir * (*current_offset));
-
-
+
+
   /* FIXME: 1000 should relate to paper size.  */
   if (fabs (total_off) > 1000)
     {
@@ -149,6 +149,7 @@ Side_position_interface::general_side_position (Grob *me, Axis a, bool use_exten
       if (strict_infinity_checking)
        scm_misc_error (__FUNCTION__, "Improbable offset.", SCM_EOL);
     }
+
   return scm_from_double (total_off);
 }

@@ -157,15 +158,15 @@ MAKE_SCHEME_CALLBACK (Side_position_interface, y_aligned_on_support_refpoints, 1
 SCM
 Side_position_interface::y_aligned_on_support_refpoints (SCM smob)
 {
- return general_side_position (unsmob_grob (smob), Y_AXIS, false, false, false, 0, 0, 0); + return general_side_position (unsmob_grob (smob), Y_AXIS, false, false, false, 0, 0, 0);
 }

MAKE_SCHEME_CALLBACK (Side_position_interface, pure_y_aligned_on_support_refpoints, 3);
 SCM
Side_position_interface::pure_y_aligned_on_support_refpoints (SCM smob, SCM start, SCM end)
 {
-  return general_side_position (unsmob_grob (smob), Y_AXIS, false, false,
-                               true, scm_to_int (start), scm_to_int (end), 0);
+  return general_side_position (unsmob_grob (smob), Y_AXIS, false, false,
+                               true, scm_to_int (start), scm_to_int (end), 0);
 }


@@ -180,9 +181,9 @@ axis_aligned_side_helper (SCM smob, Axis a, bool pure, int start, int end, SCM c
   if (scm_is_number (current_off_scm))
     {
       r = scm_to_double (current_off_scm);
-      current_off_ptr = &r;
+      current_off_ptr = &r;
     }
-
+
return Side_position_interface::aligned_side (unsmob_grob (smob), a, pure, start, end, current_off_ptr);
 }

@@ -255,7 +256,7 @@ Side_position_interface::aligned_side (Grob *me, Axis a, bool pure, int start, i if (fabs (position) <= 2 * Staff_symbol_referencer::staff_radius (me) + 1 /* In case of a ledger lines, quantize even if we're outside the staff. */
              || (Note_head::has_interface (head)
-               
+
&& abs (Staff_symbol_referencer::get_position (head)) > abs (position)))
            {
              o += (rounded - position) * 0.5 * ss;
@@ -266,16 +267,20 @@ Side_position_interface::aligned_side (Grob *me, Axis a, bool pure, int start, i
       else if (scm_is_number (me->get_property ("staff-padding")) && dir)
        {
          Interval iv = me->maybe_pure_extent (me, a, pure, start, end);
-       
-         Real padding
+
+         Real staff_padding
            = Staff_symbol_referencer::staff_space (me)
            * scm_to_double (me->get_property ("staff-padding"));

-         Grob *common = me->common_refpoint (staff, Y_AXIS);
-
- Interval staff_size = staff->maybe_pure_extent (common, Y_AXIS, pure, start, end);
-         Real diff = dir*staff_size[dir] + padding - dir * (o + iv[-dir]);
-         o += dir * max (diff, 0.0);
+          Grob *parent = me->get_parent (Y_AXIS);
+          Grob *common = me->common_refpoint (staff, Y_AXIS);
+ Real parent_position = parent->maybe_pure_coordinate (common, Y_AXIS, pure, start, end); + Real staff_position = staff->maybe_pure_coordinate (common, Y_AXIS, pure, start, end); + Interval staff_extent = staff->maybe_pure_extent (staff, a, pure, start, end);
+          Real diff = dir * staff_extent[dir] + staff_padding
+                      - dir * (o + iv[-dir])
+                      + dir * (staff_position - parent_position);
+          o += dir * max (diff, 0.0);
        }
     }
   return scm_from_double (o);
@@ -300,7 +305,7 @@ Side_position_interface::get_axis (Grob *me)
 {
   if (scm_is_number (me->get_property ("side-axis")))
     return Axis (scm_to_int (me->get_property ("side-axis")));
-
+
string msg = String_convert::form_string ("side-axis not set for grob %s.",
                                            me->name ().c_str ());
   me->programming_error (msg);





reply via email to

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