Re: [Lilypond-auto] Issue 3385 in lilypond: Ottava with cross-staff messes up beams

[lilypond]

Comment #31 on issue 3385 by address@hidden: Ottava with cross-staff  messes  
up beams
http://code.google.com/p/lilypond/issues/detail?id=3385

You are assuming that there is a "problematic container".  But the patch is  
a conglomeration of lots of things.

diff --git a/lily/axis-group-interface.cc b/lily/axis-group-interface.cc
index 1ed1f7f..0d786c3 100644
--- a/lily/axis-group-interface.cc
+++ b/lily/axis-group-interface.cc
@@ -258,6 +258,9 @@ Axis_group_interface::adjacent_pure_heights (SCM smob)
           && !has_interface (g))
         continue;

+      if (!g->is_live ())
+        continue;
+
       bool outside_staff = scm_is_number (g->get_property  
("outside-staff-prior
       Real padding = robust_scm2double (g->get_property  
("outside-staff-padding

has nothing to do at all with unpure/pure containers.  It looks like  
typical "oh, something stopped working, let's not investigate but place a  
bandaid" material.  Why should that even be relevant to unpure/pure  
containers?

Then there is stuff like
@@ -573,7 +576,7 @@ Axis_group_interface::pure_group_height (Grob *me, int  
start
       programming_error ("no pure Y common refpoint");
       return Interval ();
     }
-  Real my_coord = me->relative_coordinate (common, Y_AXIS);
+  Real my_coord = me->pure_relative_y_coordinate (common, start, end);
   Interval r (relative_pure_height (me, start, end));

   return r - my_coord;

which does not use a different way to declare a pure/unpure container or  
callback but suddenly uses a different function.  While this has something  
to do with pure/unpure, it has nothing to do with a different way of  
declaring pure/unpure.  There are several of those.  Again, this looks like  
bandaids that should not be necessary if the actual idea of the patch were  
sound.

Similarly
@@ -488,10 +495,7 @@ Interval
 Grob::pure_height (Grob *refp, int start, int end)
 {
   SCM iv_scm = get_pure_property ("Y-extent", start, end);
-  // TODO: Why is this Interval (0,0)
-  // Shouldn't it just be an empty interval?
-  // 0,0 puts an arbitrary point at 0,0 which will influence spacing
-  Interval iv = robust_scm2interval (iv_scm, Interval (0, 0));
+  Interval iv = robust_scm2interval (iv_scm, Interval ());
   Real offset = pure_relative_y_coordinate (refp, start, end);

   SCM min_ext = get_property ("minimum-Y-extent");

Then there is more unrelated stuff like
+/*
+  Used to be in stop_translation_timestep, but grobs can't
+  be created here.
+*/
 void
-Melody_engraver::stop_translation_timestep ()
+Melody_engraver::process_acknowledged ()
 {
   if (stem_
       && !is_direction (stem_->get_property_data ("neutral-direction")))
@@ -66,6 +71,11 @@ Melody_engraver::stop_translation_timestep ()
           Melody_spanner::add_stem (melody_item_, stem_);
         }
     }
+}
+
+void
+Melody_engraver::stop_translation_timestep ()
+{
   stem_ = 0;
 }


or
--- a/lily/separation-item.cc
+++ b/lily/separation-item.cc
@@ -150,10 +150,19 @@ Separation_item::boxes (Grob *me, Grob *left)
       Interval extra_height = robust_scm2interval (elts[i]->get_property  
("extr
                                                    Interval (0.0, 0.0));

-      x[LEFT] += extra_width[LEFT];
-      x[RIGHT] += extra_width[RIGHT];
-      y[DOWN] += extra_height[DOWN];
-      y[UP] += extra_height[UP];
+      // The conventional empty extent is (+inf.0 . -inf.0)
+      //  but (-inf.0 . +inf.0) is used as extra-spacing-height
+      //  on items that must not overlap other note-columns.
+      // If these two uses of inf combine, leave the empty extent.
+
+      if (!isinf (x[LEFT]))
+        x[LEFT] += extra_width[LEFT];
+      if (!isinf (x[RIGHT]))
+        x[RIGHT] += extra_width[RIGHT];
+      if (!isinf (y[DOWN]))
+        y[DOWN] += extra_height[DOWN];
+      if (!isinf (y[UP]))
+        y[UP] += extra_height[UP];

       if (!x.is_empty () && !y.is_empty ())
         out.push_back (Box (x, y));


Then there is stuff like get_maybe_pure_property.  Again, that does not  
seem at all related to using a different interface for declaring pure  
callbacks.

And the list of pure-functions contains just 7 elements.  If we take a  
look, say, at laisser-vibrer::print, then it has been declared as a pure  
function previously.  But it's not like it now is an unpure_pure container  
or anything.  It's just unchanged and still calls ly:tie::print which does  
not seem to be wrapped into anything unpure/pure.

So there is a _lot_ more in the line of splitting this into several  
independent commits, and in verifying that there were no oversights, than  
the 7 pure-functions would suggest.

--
You received this message because this project is configured to send all  
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings