Re: Sketch for DotColumn not triggering vertical alignment. (issue 5538049)

[mtsolo]

Reviewers: lemzwerg, carl.d.sorensen_gmail.com, Keith,


http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc
File lily/staff-symbol-referencer.cc (right):

http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc#newcode95
lily/staff-symbol-referencer.cc:95: Real y = (pure
On 2012/01/12 11:02:13, lemzwerg wrote:
> The outermost parentheses have no effect.  I suggest to remove them.

They're for code alignment purposes: it's so that the ? and : don't
align vertically with the -.  There are a couple other places in the
code that use this convention (forget where).

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc
File lily/dot-column.cc (right):

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc#newcode216
lily/dot-column.cc:216: // do the X-offset properly.
On 2012/01/15 21:45:27, Keith wrote:
> Might this comment now be obsolete?  It is about the fix to issue
1088.  If so,
> we could unravel some of this complication, later.

It very well could be.  After this patch is pushed, remind me to
investigate this (I'm writing myself a reminder as well).

Description:
Sketch for DotColumn not triggering vertical alignment.

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

Affected files:
  A input/regression/dot-column-vertical-positioning.ly
  M lily/dot-column.cc
  M lily/include/staff-symbol-referencer.hh
  M lily/staff-symbol-referencer.cc


Index: input/regression/dot-column-vertical-positioning.ly
diff --git a/input/regression/dot-column-vertical-positioning.ly  
b/input/regression/dot-column-vertical-positioning.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..e609c7b4d03b6642f8352127501cb3e2ae8f208d
--- /dev/null
+++ b/input/regression/dot-column-vertical-positioning.ly
@@ -0,0 +1,13 @@
+\version "2.15.24"
+
+\header {
+  texidoc = "Dot columns should not trigger vertical spacing before
+line breaking.  If the regtest issues a programming_error saying that
+vertical spacing has been called before line breaking, it has failed.
+"
+}
+
+\context Staff <<
+  \new Voice { \voiceOne f''8.[ e''16] }
+  \new Voice { \voiceThree r8. a'16}
+>>
Index: lily/dot-column.cc
diff --git a/lily/dot-column.cc b/lily/dot-column.cc
index  
2292d2fe0999b295623f8f9a064d7b96d90f1560..03f368d76b50703cd49055d54820417ec3248f63  
100644
--- a/lily/dot-column.cc
+++ b/lily/dot-column.cc
@@ -143,7 +143,14 @@ Dot_column::calc_positioning_done (SCM smob)
         }
     }

-  vector_sort (dots, position_less);
+  /*
+    The use of pure_position_less and pure_get_rounded_position below
+    are due to the fact that this callback is called before line breaking
+    occurs.  Because dots' actual Y posiitons may be linked to that of
+    beams (dots are attached to rests, which are shifted to avoid beams),
+    we instead must use their pure Y positions.
+  */
+  vector_sort (dots, pure_position_less);
   for (vsize i = dots.size (); i--;)
     {
       if (!dots[i]->is_live ())
@@ -170,7 +177,7 @@ Dot_column::calc_positioning_done (SCM smob)
           dp.x_extent_ = note->extent (commonx, X_AXIS);
         }

-      int p = Staff_symbol_referencer::get_rounded_position (dp.dot_);
+      int p = Staff_symbol_referencer::pure_get_rounded_position (dp.dot_);

       /* icky, since this should go via a Staff_symbol_referencer
          offset callback but adding a dot overwrites Y-offset. */
@@ -191,7 +198,7 @@ Dot_column::calc_positioning_done (SCM smob)
       /*
         Junkme?
        */
-      Staff_symbol_referencer::set_position (i->second.dot_, i->first);
+      Staff_symbol_referencer::pure_set_position (i->second.dot_,  
i->first);
     }

   me->translate_axis (cfg.x_offset () - me->relative_coordinate (commonx,  
X_AXIS),
Index: lily/include/staff-symbol-referencer.hh
diff --git a/lily/include/staff-symbol-referencer.hh  
b/lily/include/staff-symbol-referencer.hh
index  
a3dd915c9ab85a96aed0986850f2091fdfbda3bb..179bae828f6905339b16ffa2757e67b9fb87fa12  
100644
--- a/lily/include/staff-symbol-referencer.hh
+++ b/lily/include/staff-symbol-referencer.hh
@@ -33,6 +33,7 @@ public:
   DECLARE_GROB_INTERFACE ();
   static bool ugly_hack (Grob *);
   static void set_position (Grob *, Real);
+  static void pure_set_position (Grob *, Real);
   DECLARE_SCHEME_CALLBACK (callback, (SCM element));

   /**
@@ -46,12 +47,19 @@ public:
   static bool on_staff_line (Grob *, int);
   static int line_count (Grob *);
   static Real get_position (Grob *);
+  static Real pure_get_position (Grob *);
   static Real staff_radius (Grob *);
   static int get_rounded_position (Grob *);
+  static int pure_get_rounded_position (Grob *);
   static Interval extent_in_staff (Grob *);
+
+private:
+  static void internal_set_position (Grob *, Real, bool);
+  static Real internal_get_position (Grob *, bool);
 };

 int compare_position (Grob *const &, Grob *const &);
 bool position_less (Grob *const &, Grob *const &);
+bool pure_position_less (Grob *const &, Grob *const &);
 #endif /* STAFF_SYMBOL_REFERENCER_HH */

Index: lily/staff-symbol-referencer.cc
diff --git a/lily/staff-symbol-referencer.cc  
b/lily/staff-symbol-referencer.cc
index  
1cda85f2a755d6eef8eede44a65653e1825708f2..74c2c448c61c2407bb14778050b62c354f91b3ab  
100644
--- a/lily/staff-symbol-referencer.cc
+++ b/lily/staff-symbol-referencer.cc
@@ -75,12 +75,26 @@ Staff_symbol_referencer::line_thickness (Grob *me)
 Real
 Staff_symbol_referencer::get_position (Grob *me)
 {
+  return internal_get_position (me, false);
+}
+
+Real
+Staff_symbol_referencer::pure_get_position (Grob *me)
+{
+  return internal_get_position (me, true);
+}
+
+Real
+Staff_symbol_referencer::internal_get_position (Grob *me, bool pure)
+{
   Real p = 0.0;
   Grob *st = get_staff_symbol (me);
   Grob *c = st ? me->common_refpoint (st, Y_AXIS) : 0;
   if (st && c)
     {
-      Real y = me->relative_coordinate (c, Y_AXIS)
+      Real y = (pure
+                ? me->pure_relative_y_coordinate (c, 0, INT_MAX)
+                : me->relative_coordinate (c, Y_AXIS))
                - st->relative_coordinate (c, Y_AXIS);
       Real space = Staff_symbol::staff_space (st);
       p = (space == 0) ? 0 : 2.0 * y / space;
@@ -113,6 +127,12 @@ Staff_symbol_referencer::get_rounded_position (Grob  
*me)
   return int (rint (get_position (me)));
 }

+int
+Staff_symbol_referencer::pure_get_rounded_position (Grob *me)
+{
+  return int (rint (pure_get_position (me)));
+}
+
 MAKE_SCHEME_CALLBACK (Staff_symbol_referencer, callback, 1);
 SCM
 Staff_symbol_referencer::callback (SCM smob)
@@ -145,11 +165,23 @@ will be extracted from staff-position */
 void
 Staff_symbol_referencer::set_position (Grob *me, Real p)
 {
+  internal_set_position (me, p, false);
+}
+
+void
+Staff_symbol_referencer::pure_set_position (Grob *me, Real p)
+{
+  internal_set_position (me, p, true);
+}
+
+void
+Staff_symbol_referencer::internal_set_position (Grob *me, Real p, bool  
pure)
+{
   Grob *st = get_staff_symbol (me);
   Real oldpos = 0.0;
   if (st && me->common_refpoint (st, Y_AXIS))
     {
-      oldpos = get_position (me);
+      oldpos = pure ? pure_get_position (me) : get_position (me);
     }

   Real ss = Staff_symbol_referencer::staff_space (me);
@@ -177,6 +209,13 @@ position_less (Grob *const &a, Grob *const &b)
          < Staff_symbol_referencer::get_position (b);
 }

+bool
+pure_position_less (Grob *const &a, Grob *const &b)
+{
+  return Staff_symbol_referencer::pure_get_position (a)
+         < Staff_symbol_referencer::pure_get_position (b);
+}
+
 ADD_INTERFACE (Staff_symbol_referencer,
                "An object whose address@hidden is meant relative to a  
staff"
                " symbol.  These usually"