Re: Emit not-quite-cross-staff beams in the right context. (issue4564041)

[joeneeman]

Reviewers: Keith,

Message:
On 2011/05/28 18:43:19, Keith wrote:
> I would love to test, but don't know enough of the internals to see
what this
> does.

If you take your example from comment 5 of issue 1043, without manual
beaming, the Beam's Y-parent will be the VerticalAxisGroup of the top
staff and Beam::calc_cross_staff will return true. With manual beaming
(or with this patch), the Beam's Y-parent will be the VerticalAxisGroup
of the bottom staff and Beam::calc_cross_staff will return false.


Description:
Emit not-quite-cross-staff beams in the right context.

This is related to 1043 and possibly other bugs.  Previously,
if a staff change happened immediately after the termination of
an auto-engraved cross-staff beam, then the beam was parented
to the wrong staff.  Now, every beam is parented to the context
in which it began.

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

Affected files:
  M lily/auto-beam-engraver.cc
  M lily/engraver-group.cc
  M lily/engraver.cc
  M lily/grob-info.cc
  M lily/include/engraver.hh
  M lily/include/grob-info.hh


Index: lily/auto-beam-engraver.cc
diff --git a/lily/auto-beam-engraver.cc b/lily/auto-beam-engraver.cc
index  
66b79fe008c59b0c2612c03425c2871af991c9cd..3fb76af9b6e7928e9f8f997c99cb0adde0cd0646  
100644
--- a/lily/auto-beam-engraver.cc
+++ b/lily/auto-beam-engraver.cc
@@ -80,6 +80,7 @@ private:
   Moment extend_mom_;
   Moment beam_start_moment_;
   Moment beam_start_location_;
+  Context *beam_start_context_;

   // We act as if beam were created, and start a grouping anyway.
   Beaming_pattern *grouping_;
@@ -219,7 +220,9 @@ Auto_beam_engraver::create_beam ()
   for (vsize i = 0; i < stems_->size (); i++)
     Beam::add_stem (beam, (*stems_)[i]);

-  announce_grob (beam, (*stems_)[0]->self_scm ());
+  Grob_info i = make_grob_info (beam, (*stems_)[0]->self_scm ());
+  i.rerouting_daddy_context_ = beam_start_context_;
+  announce_grob (i);

   return beam;
 }
@@ -238,6 +241,7 @@ Auto_beam_engraver::begin_beam ()
   beaming_options_.from_context (context ());
   beam_settings_ = updated_grob_properties (context (), ly_symbol2scm  
("Beam"));

+  beam_start_context_ = context ();
   beam_start_moment_ = now_mom ();
   beam_start_location_
     = robust_scm2moment (get_property ("measurePosition"), Moment (0));
@@ -269,7 +273,10 @@ Auto_beam_engraver::end_beam ()

       if (finished_beam_)
        {
-         announce_end_grob (finished_beam_, SCM_EOL);
+         Grob_info i = make_grob_info (finished_beam_, SCM_EOL);
+         i.rerouting_daddy_context_ = beam_start_context_;
+
+         announce_end_grob (i);
          finished_grouping_ = grouping_;
          finished_beaming_options_ = beaming_options_;
        }
Index: lily/engraver-group.cc
diff --git a/lily/engraver-group.cc b/lily/engraver-group.cc
index  
07a0ef50914b4f235093ae222e3c8637e604964d..a5782d23b0b8fbbcb8fa5272fb2cbdef77b389d2  
100644
--- a/lily/engraver-group.cc
+++ b/lily/engraver-group.cc
@@ -70,9 +70,16 @@ Engraver_group::announce_grob (Grob_info info)
 {
   announce_infos_.push_back (info);

+  Context *dad_con = context_->get_parent_context ();
+  if (info.rerouting_daddy_context_)
+    {
+      dad_con = info.rerouting_daddy_context_;
+      info.rerouting_daddy_context_ = 0;
+    }
+
   Engraver_group *dad_eng
-    = context_->get_parent_context ()
-    ? dynamic_cast<Engraver_group *> (context_->get_parent_context  
()->implementation ())
+    = dad_con
+    ? dynamic_cast<Engraver_group *> (dad_con->implementation ())
     : 0;

   if (dad_eng)
Index: lily/engraver.cc
diff --git a/lily/engraver.cc b/lily/engraver.cc
index  
8e49280fd8f928413eb810b4b1428da66e806691..12615eb8d6cbc780fc96a8e95cdee2fab6984bc8  
100644
--- a/lily/engraver.cc
+++ b/lily/engraver.cc
@@ -43,29 +43,33 @@ Engraver::announce_grob (Grob_info inf)
 void
 Engraver::announce_end_grob (Grob_info inf)
 {
+  inf.start_end_ = STOP;
   get_daddy_engraver ()->announce_grob (inf);
 }

-/*
-  CAUSE is the object (typically a Stream_event object)  that
-  was the reason for making E.
-*/
-void
-Engraver::announce_grob (Grob *e, SCM cause)
+Grob_info
+Engraver::make_grob_info(Grob *e, SCM cause)
 {
   /* TODO: Remove Music code when it's no longer needed */
   if (Music *m = unsmob_music (cause))
     {
       cause = m->to_event ()->unprotect ();
     }
-  if (unsmob_stream_event (cause) || unsmob_grob (cause))
+  if (e->get_property ("cause") == SCM_EOL
+      && (unsmob_stream_event (cause) || unsmob_grob (cause)))
     e->set_property ("cause", cause);

-  Grob_info i (this, e);
+  return Grob_info (this, e);
+}

-  Engraver_group *g = get_daddy_engraver ();
-  if (g)
-    g->announce_grob (i);
+/*
+  CAUSE is the object (typically a Stream_event object)  that
+  was the reason for making E.
+*/
+void
+Engraver::announce_grob (Grob *e, SCM cause)
+{
+  announce_grob (make_grob_info (e, cause));
 }


@@ -75,21 +79,7 @@ Engraver::announce_grob (Grob *e, SCM cause)
 void
 Engraver::announce_end_grob (Grob *e, SCM cause)
 {
-  /* TODO: Remove Music code when it's no longer needed */
-  if (Music *m = unsmob_music (cause))
-    {
-      cause = m->to_event ()->unprotect ();
-    }
-  if (e->get_property ("cause") == SCM_EOL
-      && (unsmob_stream_event (cause) || unsmob_grob (cause)))
-    e->set_property ("cause", cause);
-
-  Grob_info i (this, e);
-
-  i.start_end_ = STOP;
-  Engraver_group *g = get_daddy_engraver ();
-  if (g)
-    g->announce_grob (i);
+  announce_end_grob (make_grob_info (e, cause));
 }


Index: lily/grob-info.cc
diff --git a/lily/grob-info.cc b/lily/grob-info.cc
index  
f02e836e75389b80d601e7ddb9c12981682fb46d..b710477a8e4073ef21a2bc0812e198fa99ba8ab9  
100644
--- a/lily/grob-info.cc
+++ b/lily/grob-info.cc
@@ -30,6 +30,7 @@ Grob_info::Grob_info (Translator *t, Grob *g)
   origin_trans_ = t;
   grob_ = g;
   start_end_ = START;
+  rerouting_daddy_context_ = 0;

   /*
     assert here, because this is easier to debug.
@@ -42,6 +43,7 @@ Grob_info::Grob_info ()
   grob_ = 0;
   start_end_ = START;
   origin_trans_ = 0;
+  rerouting_daddy_context_ = 0;
 }

 Stream_event *
Index: lily/include/engraver.hh
diff --git a/lily/include/engraver.hh b/lily/include/engraver.hh
index  
1488d78c75f00a8f3768bfcba0c2e6dc65e1c70d..162724b808cb4980f9a6ae0ba87b64d8f3e47090  
100644
--- a/lily/include/engraver.hh
+++ b/lily/include/engraver.hh
@@ -52,6 +52,8 @@ public:
   void announce_grob (Grob *, SCM cause);
   void announce_end_grob (Grob *, SCM cause);

+  Grob_info make_grob_info (Grob *, SCM cause);
+
   Item *internal_make_item (SCM sym, SCM cause, char const *name,
                            char const *f, int l, char const *fun);
   Spanner *internal_make_spanner (SCM sym, SCM cause, char const *name,
Index: lily/include/grob-info.hh
diff --git a/lily/include/grob-info.hh b/lily/include/grob-info.hh
index  
f9390fff446d16cede3c5345c766c97db75cbd2e..56a49170a4bd1d52cfe2bcecbc1140730b87ef0a  
100644
--- a/lily/include/grob-info.hh
+++ b/lily/include/grob-info.hh
@@ -32,6 +32,7 @@ class Grob_info
   Translator *origin_trans_;
   Grob *grob_;
   Direction start_end_;
+

   friend class Engraver;
 public:
@@ -49,6 +50,14 @@ public:
   Item *item () const;
   Spanner *spanner () const;
   static bool less (Grob_info i, Grob_info j);
+
+  /*
+    For contexts that change staves, it may be desirable to emit a
+    grob into a staff other than the current one.  If this is non-null,
+    this grob should be announced in this context instead of the
+    daddy_context_.
+  */
+  Context *rerouting_daddy_context_;
 };

 #endif // STAFFELEMINFO_HH