On Sat, Oct 21, 2017 at 5:15 PM, David Kastrup <d...@gnu.org> wrote:

> Joe Neeman <joenee...@gmail.com> writes:
>
> > I have a hunch: once upon a time (IIRC), the staves of each StaffGroup
> > were contained in some container grob (maybe VerticalAxisGroup?). The
> > vertical spacing was done heirarchically, so the VerticalAxisGroup was
> > in charge of spacing the staves of a StaffGroup, and then the System
> > would space those groups. This led to poor vertical spacing, and so
> > the intermediate VerticalAxisGroups were scrapped.
> >
> > So what does this have to do with Mark_engraver? Well my hunch is that
> > a long time ago, if you had moved Mark_engraver to the \StaffGroup
> > context then those marks would have ended up living inside the
> > VerticalAxisGroup belonging to those staves, so it would have shown up
> > in the right place.  With the intermediate VerticalAxisGroup gone,
> > there's no intermediate grob to take parentage of the mark, and so it
> > bubbles all the way up to the System.
> >
> > I'll try to find some time this weekend to investigate this hunch...
>

It looks like I might have been right (at least about what's happening now
-- I didn't delve into the history). I've whipped up a quick patch (not
well-tested) that seems to fix it.

That would be great!  The doc string of Mark_engraver suggests that
> Mark_engraver should only be moved together with Staff_grouping_engraver
> (?) but It appears to me that this scheme could be troublesome if you
> don't want to keep the bar numbers where they are?
>

At least on the test I'm using (which I got from the bug report), the bar
numbers seem to stay where they are without problems...

Best,
Joe
From 06d8fe8d624a89f9117997251ffc0c31c6c271f7 Mon Sep 17 00:00:00 2001
From: Joe Neeman <joenee...@gmail.com>
Date: Sat, 21 Oct 2017 22:40:39 -0500
Subject: [PATCH] Change the heuristic in move_to_extremal_staff.

---
 lily/side-position-interface.cc | 89 ++++++++++++++++++++++++++++++++---------
 1 file changed, 71 insertions(+), 18 deletions(-)

diff --git a/lily/side-position-interface.cc b/lily/side-position-interface.cc
index 52551e8b52..b0d717a775 100644
--- a/lily/side-position-interface.cc
+++ b/lily/side-position-interface.cc
@@ -32,6 +32,7 @@ using namespace std;
 #include "directional-element-interface.hh"
 #include "grob.hh"
 #include "grob-array.hh"
+#include "hara-kiri-group-spanner.hh"
 #include "international.hh"
 #include "item.hh"
 #include "main.hh"
@@ -435,28 +436,85 @@ Side_position_interface::get_axis (Grob *me)
   return NO_AXES;
 }
 
+Grob *
+their_child_my_ancestor(Grob *me, Grob *them)
+{
+  Grob *parent = me->get_parent (Y_AXIS);
+  if (!parent)
+    return 0;
+  if (parent == them)
+    return me;
+  return their_child_my_ancestor(parent, them);
+}
+
+// Some side-positioned grobs (particularly those that are positioned above
+// or below a staff) would really like to participate in the outside-staff
+// collision avoidance routines. In order for that to happen, these grobs
+// need to have a VerticalAxisGroup as their Y_AXIS parent. It's a little
+// tricky to decide which VerticalAxisGroup should be the parent; figuring
+// that out is the purpose of this callback.
+//
+// This callback is usually called as an after-line-breaking callback,
+// at which point we know which staves will be visible. To choose this
+// grob's new parent, we look at all of its side-support-elements, and
+// find the top-most (or bottom-most, if 'direction is DOWN)
+// VerticalAxisGroup containing one of those side-support-elements.
 MAKE_SCHEME_CALLBACK (Side_position_interface, move_to_extremal_staff, 1);
 SCM
 Side_position_interface::move_to_extremal_staff (SCM smob)
 {
   Grob *me = unsmob<Grob> (smob);
+  Grob_array *ga = unsmob<Grob_array> (me->get_object ("side-support-elements"));
+  if (!ga)
+    return SCM_BOOL_F;
+
   System *sys = dynamic_cast<System *> (me->get_system ());
+  Grob *valign = unsmob<Grob> (sys->get_object("vertical-alignment"));
+  if (!valign)
+    return SCM_BOOL_F;
+  Grob_array *vaxis_groups = unsmob<Grob_array> (valign->get_object("elements"));
+  if (!vaxis_groups)
+    return SCM_BOOL_F;
+
   Direction dir = get_grob_direction (me);
   if (dir != DOWN)
     dir = UP;
 
   Interval iv = me->extent (sys, X_AXIS);
   iv.widen (1.0);
-  Grob *top_staff = sys->get_extremal_staff (dir, iv);
 
-  if (!top_staff)
-    return SCM_BOOL_F;
+  // Find out which of my side-support-elements belongs to the uppermost staff.
+  // (We filter eligible staves to only consider those whose X-extents overlap
+  // with mine; this way, we avoid reparenting ourselves to a staff that only
+  // takes up a small part of the line.)
+  vector<Grob *> const &elts = ga->array ();
+  vector<Grob *> const &all_groups = vaxis_groups->array ();
+  Grob *top_staff = 0;
+  vsize top_staff_idx = 0;
+  for (vsize i = 0; i < elts.size (); ++i)
+    {
+      Grob *staff_vaxis_group = their_child_my_ancestor(elts[i], valign);
 
-  // Only move this grob if it is a direct child of the system.  We
-  // are not interested in moving marks from other staves to the top
-  // staff; we only want to move marks from the system to the top
-  // staff.
-  if (sys != me->get_parent (Y_AXIS))
+      if (has_interface<Hara_kiri_group_spanner> (elts[i]))
+        Hara_kiri_group_spanner::consider_suicide (elts[i]);
+
+      Interval intersection = elts[i]->extent (sys, X_AXIS);
+      intersection.intersect (iv);
+      if (elts[i]->is_live () && !intersection.is_empty ())
+        {
+          // The current staff is a candidate. Check if it's the extremal
+          // candidate in the desired direction.
+          vsize idx = find(all_groups.begin(), all_groups.end(), top_staff)
+              - all_groups.begin();
+          if (!top_staff || ((int) idx) * dir < ((int) top_staff_idx) * dir)
+            {
+              top_staff = staff_vaxis_group;
+              top_staff_idx = idx;
+            }
+        }
+    }
+
+  if (!top_staff)
     return SCM_BOOL_F;
 
   me->set_parent (top_staff, Y_AXIS);
@@ -464,18 +522,13 @@ Side_position_interface::move_to_extremal_staff (SCM smob)
   Axis_group_interface::add_element (top_staff, me);
 
   // Remove any cross-staff side-support dependencies
-  Grob_array *ga = unsmob<Grob_array> (me->get_object ("side-support-elements"));
-  if (ga)
+  vector<Grob *> new_elts;
+  for (vsize i = 0; i < elts.size (); ++i)
     {
-      vector<Grob *> const &elts = ga->array ();
-      vector<Grob *> new_elts;
-      for (vsize i = 0; i < elts.size (); ++i)
-        {
-          if (me->common_refpoint (elts[i], Y_AXIS) == top_staff)
-            new_elts.push_back (elts[i]);
-        }
-      ga->set_array (new_elts);
+      if (me->common_refpoint (elts[i], Y_AXIS) == top_staff)
+        new_elts.push_back (elts[i]);
     }
+  ga->set_array (new_elts);
   return SCM_BOOL_T;
 }
 
-- 
2.14.2

_______________________________________________
lilypond-user mailing list
lilypond-user@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-user

Reply via email to