Adds arpeggio to conditional item grob array.

The actual function of conditional elements was not correctly reflected
in the comment above Separation_item::boxes.  This comment led one to believe
that conditional elements were only used when notes with accidentals
had ties coming to them.  This is not true.  Conditional elements are always
used for right columns when there is something to the left (see
Spacing_interface::skylines).  They are omitted _only_ when they are
accidentals with ties coming to them.

So why do we want an arpeggio to be a conditional element?  There is
nothing conditional about it (it will always be printed, unlike accidentals
with ties going to them).  It is because conditional elements have the
double duty of being conditional (i.e. accidentals) AND being factored
into springs in note spacing (see the long comment in
Note_spacing::get_spacing) in the calculation of a spring's ideal distance.
Other elements to the left of a note column, like scripts and fingerings,
are only factored into the minimum distance. We want arpeggios to factor
into the ideal distance because otherwise they will be too close to
left note-columns in tight spacing situations.

Lastly, there was no reason in the code base to keep a pointer to the
arpeggio in the note column, so it is removed here.

diff --git a/input/regression/arpeggio-collision.ly b/input/regression/arpeggio-collision.ly
index 4f4a22f4b42ca3c0bbcab44e4a37945d87ffb521..5fccbca3a6af6aaf416b3d101905f901e1297249 100644 (file)
--- a/input/regression/arpeggio-collision.ly
+++ b/input/regression/arpeggio-collision.ly
@@ -1,4 +1,4 @@
-\version "2.16.0"
+\version "2.17.17"
 
 \header  {
 texidoc = "Arpeggio stays clear of accidentals and flipped note heads."
@@ -8,6 +8,7 @@ texidoc = "Arpeggio stays clear of accidentals and flipped note heads."
   \context{
     \Staff
     connectArpeggios = ##t
+    \consists "Span_arpeggio_engraver"
   }
 }
 
@@ -17,4 +18,5 @@ texidoc = "Arpeggio stays clear of accidentals and flipped note heads."
   <fis'' g d a>\arpeggio
   <fis, g d a>\arpeggio
   <fis'' g d a>\arpeggio
+  << { <e' a>\arpeggio } \\ { <g cis>\arpeggio } >>
 }

diff --git a/lily/arpeggio-engraver.cc b/lily/arpeggio-engraver.cc
index 19f635f7f1f28f9829880f618bfddebd838a2ff9..2a6d3771d79ac364584aac5c0ff778eac8f9db8c 100644 (file)
--- a/lily/arpeggio-engraver.cc
+++ b/lily/arpeggio-engraver.cc
@@ -19,14 +19,15 @@
 
 #include "engraver.hh"
 
-#include "pointer-group-interface.hh"
 #include "arpeggio.hh"
-#include "stem.hh"
+#include "item.hh"
+#include "note-column.hh"
+#include "pointer-group-interface.hh"
 #include "rhythmic-head.hh"
+#include "separation-item.hh"
 #include "side-position-interface.hh"
+#include "stem.hh"
 #include "stream-event.hh"
-#include "note-column.hh"
-#include "item.hh"
 
 #include "translator.icc"
 
@@ -37,6 +38,8 @@ public:
 
   void acknowledge_stem (Grob_info);
   void acknowledge_rhythmic_head (Grob_info);
+  void acknowledge_note_column (Grob_info);
+
 protected:
   void process_music ();
   void stop_translation_timestep ();
@@ -83,6 +86,13 @@ Arpeggio_engraver::acknowledge_rhythmic_head (Grob_info info)
     Side_position_interface::add_support (arpeggio_, info.grob ());
 }
 
+void
+Arpeggio_engraver::acknowledge_note_column (Grob_info info)
+{
+  if (arpeggio_)
+    Separation_item::add_conditional_item (info.grob (), arpeggio_);
+}
+
 void
 Arpeggio_engraver::process_music ()
 {
@@ -101,6 +111,7 @@ Arpeggio_engraver::stop_translation_timestep ()
 
 ADD_ACKNOWLEDGER (Arpeggio_engraver, stem);
 ADD_ACKNOWLEDGER (Arpeggio_engraver, rhythmic_head);
+ADD_ACKNOWLEDGER (Arpeggio_engraver, note_column);
 
 ADD_TRANSLATOR (Arpeggio_engraver,
                 /* doc */

diff --git a/lily/include/note-column.hh b/lily/include/note-column.hh
index 874b54207539a8238cfcd7d777a74c0c14e3e516..e39edf840f070086be168fade1a6d96d91795af2 100644 (file)
--- a/lily/include/note-column.hh
+++ b/lily/include/note-column.hh
@@ -34,7 +34,6 @@ public:
   static bool shift_less (Grob *const &, Grob *const &);
   static Direction dir (Grob *me);
   static Grob *accidentals (Grob *me);
-  static Grob *arpeggio (Grob *me);
   static Slice head_positions_interval (Grob *me);
   static Grob *first_head (Grob *me);
   static Grob *get_rest (Grob *me);

diff --git a/lily/note-column.cc b/lily/note-column.cc
index 7f9bac8eca3bb4305b68278b8eaa07cb9a2f3e96..b98b4c68f8079ce1178da7c2cc2883afc978eacf 100644 (file)
--- a/lily/note-column.cc
+++ b/lily/note-column.cc
@@ -221,12 +221,6 @@ Note_column::dot_column (Grob *me)
   return 0;
 }
 
-Grob *
-Note_column::arpeggio (Grob *me)
-{
-  return unsmob_grob (me->get_object ("arpeggio"));
-}
-
 /* If a note-column contains a cross-staff stem then
    nc->extent (Y_AXIS, refp) will not consider the extent of the stem.
    If you want the extent of the stem to be included (and you are safe
@@ -245,7 +239,6 @@ ADD_INTERFACE (Note_column,
                "Stem and noteheads combined.",
 
                /* properties */
-               "arpeggio "
                "force-hshift "
                "horizontal-shift "
                "ignore-collision "

diff --git a/lily/paper-column-engraver.cc b/lily/paper-column-engraver.cc
index 8ff894d8facbc9f402af3d33d2834125d95d97d6..cdbb2e8b004ba5435f800baf781bc081306f3888 100644 (file)
--- a/lily/paper-column-engraver.cc
+++ b/lily/paper-column-engraver.cc
@@ -22,6 +22,7 @@
 #include "international.hh"
 #include "accidental-placement.hh"
 #include "accidental-interface.hh"
+#include "arpeggio.hh"
 #include "axis-group-interface.hh"
 #include "context.hh"
 #include "note-spacing.hh"
@@ -241,7 +242,8 @@ Paper_column_engraver::stop_translation_timestep ()
       if (!unsmob_grob (elem->get_object ("axis-group-parent-X")))
         elem->set_object ("axis-group-parent-X", col->self_scm ());
 
-      if (Accidental_placement::has_interface (elem))
+      if (Accidental_placement::has_interface (elem)
+          || Arpeggio::has_interface (elem))
         Separation_item::add_conditional_item (col, elem);
       else if (!Accidental_interface::has_interface (elem))
         Separation_item::add_item (col, elem);

diff --git a/lily/rhythmic-column-engraver.cc b/lily/rhythmic-column-engraver.cc
index e4e78ecee6a2ad06b99441fe44f4957552e46999..1955498fc43a1e80b6291f91405e2b3dc64324fc 100644 (file)
--- a/lily/rhythmic-column-engraver.cc
+++ b/lily/rhythmic-column-engraver.cc
@@ -57,7 +57,6 @@ class Rhythmic_column_engraver : public Engraver
   Grob *stem_;
   Grob *flag_;
   Grob *note_column_;
-  Grob *arpeggio_;
 
   TRANSLATOR_DECLARATIONS (Rhythmic_column_engraver);
 protected:
@@ -65,7 +64,6 @@ protected:
   DECLARE_ACKNOWLEDGER (stem);
   DECLARE_ACKNOWLEDGER (flag);
   DECLARE_ACKNOWLEDGER (rhythmic_head);
-  DECLARE_ACKNOWLEDGER (arpeggio);
   void process_acknowledged ();
   void stop_translation_timestep ();
 };
@@ -76,7 +74,6 @@ Rhythmic_column_engraver::Rhythmic_column_engraver ()
   stem_ = 0;
   flag_ = 0;
   note_column_ = 0;
-  arpeggio_ = 0;
 }
 
 void
@@ -103,13 +100,11 @@ Rhythmic_column_engraver::process_acknowledged ()
           stem_ = 0;
         }
 
-      if (arpeggio_)
+      if (flag_)
         {
-          Pointer_group_interface::add_grob (note_column_, ly_symbol2scm ("elements"), arpeggio_);
-          note_column_->set_object ("arpeggio", arpeggio_->self_scm ());
+          Pointer_group_interface::add_grob (note_column_, ly_symbol2scm ("elements"), flag_);
+          flag_ = 0;
         }
-      if (flag_)
-        Pointer_group_interface::add_grob (note_column_, ly_symbol2scm ("elements"), flag_);
     }
 }
 
@@ -131,25 +126,17 @@ Rhythmic_column_engraver::acknowledge_rhythmic_head (Grob_info i)
   rheads_.push_back (i.grob ());
 }
 
-void
-Rhythmic_column_engraver::acknowledge_arpeggio (Grob_info i)
-{
-  arpeggio_ = i.grob ();
-}
-
 void
 Rhythmic_column_engraver::stop_translation_timestep ()
 {
   note_column_ = 0;
   stem_ = 0;
-  arpeggio_ = 0;
   flag_ = 0;
 }
 
 ADD_ACKNOWLEDGER (Rhythmic_column_engraver, stem);
 ADD_ACKNOWLEDGER (Rhythmic_column_engraver, flag);
 ADD_ACKNOWLEDGER (Rhythmic_column_engraver, rhythmic_head);
-ADD_ACKNOWLEDGER (Rhythmic_column_engraver, arpeggio);
 
 ADD_TRANSLATOR (Rhythmic_column_engraver,
                 /* doc */

diff --git a/lily/separation-item.cc b/lily/separation-item.cc
index 8a32363eb4d2b0d8c6df9c321f429cbee8886e8d..ebee8d6bf3f0c53413232cf6dfbc52e40d611c9a 100644 (file)
--- a/lily/separation-item.cc
+++ b/lily/separation-item.cc
@@ -104,10 +104,13 @@ Separation_item::calc_skylines (SCM smob)
   return sp.smobbed_copy ();
 }
 
-/* if left is non-NULL, get the boxes corresponding to the
-   conditional-elements (conditioned on the grob LEFT). This
-   sounds more general than it is: conditional-elements are
-   always accidentals attached to a tied note.
+/*
+   If left is non-NULL, get the boxes corresponding to the
+   conditional-elements (conditioned on the grob LEFT).
+   Conditional elements are, for now, arpeggios and accidental
+   placements.  Based on the left grob, the accidentals will
+   be printed or not, so we filter using
+   Accidental_placement::get_relevant_accidentals.
 */
 vector<Box>
 Separation_item::boxes (Grob *me, Grob *left)
@@ -121,7 +124,19 @@ Separation_item::boxes (Grob *me, Grob *left)
   vector<Grob *> elts;
 
   if (left)
-    elts = Accidental_placement::get_relevant_accidentals (read_only_elts, left);
+    {
+      vector<Grob *> accidental_elts;
+      vector<Grob *> other_elts; // for now only arpeggios
+      for (vsize i = 0; i < read_only_elts.size (); i++)
+        {
+          if (Accidental_placement::has_interface (read_only_elts[i]))
+            accidental_elts.push_back (read_only_elts[i]);
+          else
+            other_elts.push_back (read_only_elts[i]);
+        }
+      elts = Accidental_placement::get_relevant_accidentals (accidental_elts, left);
+      elts.insert (elts.end (), other_elts.begin (), other_elts.end ());
+    }
   else
     elts = read_only_elts;
 

diff --git a/lily/span-arpeggio-engraver.cc b/lily/span-arpeggio-engraver.cc
index 61ebd16dfe0d5c191b338a5e3bf7759b4ef2cafb..049f4f2d65fbc13df23c9ace32b684c99537532c 100644 (file)
--- a/lily/span-arpeggio-engraver.cc
+++ b/lily/span-arpeggio-engraver.cc
@@ -21,10 +21,11 @@
 
 #include "engraver.hh"
 #include "arpeggio.hh"
+#include "item.hh"
 #include "pointer-group-interface.hh"
+#include "separation-item.hh"
 #include "side-position-interface.hh"
 #include "staff-symbol-referencer.hh"
-#include "item.hh"
 
 /**
    Make arpeggios that span multiple staves.  Catch arpeggios, and span a
@@ -35,6 +36,7 @@ class Span_arpeggio_engraver : public Engraver
 public:
   TRANSLATOR_DECLARATIONS (Span_arpeggio_engraver);
   DECLARE_ACKNOWLEDGER (arpeggio);
+  DECLARE_ACKNOWLEDGER (note_column);
 
 protected:
   void process_acknowledged ();
@@ -43,6 +45,7 @@ protected:
 private:
   Item *span_arpeggio_;
   vector<Grob *> arpeggios_;
+  vector<Grob *> note_columns_;
 };
 
 Span_arpeggio_engraver::Span_arpeggio_engraver ()
@@ -57,6 +60,12 @@ Span_arpeggio_engraver::acknowledge_arpeggio (Grob_info info)
     arpeggios_.push_back (info.grob ());
 }
 
+void
+Span_arpeggio_engraver::acknowledge_note_column (Grob_info info)
+{
+  note_columns_.push_back (info.grob ());
+}
+
 void
 Span_arpeggio_engraver::process_acknowledged ()
 {
@@ -73,6 +82,12 @@ Span_arpeggio_engraver::process_acknowledged ()
       span_arpeggio_ = make_item ("Arpeggio", SCM_EOL);
       span_arpeggio_->set_property ("cross-staff", SCM_BOOL_T);
     }
+  if (span_arpeggio_)
+    {
+      for (vsize i = 0; i < note_columns_.size (); i++)
+        Separation_item::add_conditional_item (note_columns_[i], span_arpeggio_);
+      note_columns_.clear ();
+    }
 }
 
 void
@@ -107,11 +122,13 @@ Span_arpeggio_engraver::stop_translation_timestep ()
       span_arpeggio_ = 0;
     }
   arpeggios_.clear ();
+  note_columns_.clear ();
 }
 
 #include "translator.icc"
 
 ADD_ACKNOWLEDGER (Span_arpeggio_engraver, arpeggio);
+ADD_ACKNOWLEDGER (Span_arpeggio_engraver, note_column);
 ADD_TRANSLATOR (Span_arpeggio_engraver,
                 /* doc */
                 "Make arpeggios that span multiple staves.",

diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index faf082bec1f5667d26215e2a414f5fe9c313e006..b4861729b7788cc19424ca0c4f3adc33592f34c1 100644 (file)
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -1062,7 +1062,6 @@ constructed from a whole number of squiggles.")
 dynamic spanners.")
      (all-elements ,ly:grob-array? "An array of all grobs in this line.  Its
 function is to protect objects from being garbage collected.")
-     (arpeggio ,ly:grob? "A pointer to an @code{Arpeggio} object.")
      (axis-group-parent-X ,ly:grob? "Containing X@tie{}axis group.")
      (axis-group-parent-Y ,ly:grob? "Containing Y@tie{}axis group.")