From: Mike Solomon Date: Thu, 18 Apr 2013 04:19:50 +0000 (+0200) Subject: Adds arpeggio to conditional item grob array. X-Git-Tag: release/2.17.17-1~27^2~3 X-Git-Url: https://git.donarmstrong.com/?a=commitdiff_plain;h=5ab8335d106d736335698245af3c1b2b2455aed6;p=lilypond.git 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 4f4a22f4b4..5fccbca3a6 100644 --- 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." \arpeggio \arpeggio \arpeggio + << { \arpeggio } \\ { \arpeggio } >> } diff --git a/lily/arpeggio-engraver.cc b/lily/arpeggio-engraver.cc index 19f635f7f1..2a6d3771d7 100644 --- 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 874b542075..e39edf840f 100644 --- 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 7f9bac8eca..b98b4c68f8 100644 --- 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 8ff894d8fa..cdbb2e8b00 100644 --- 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 e4e78ecee6..1955498fc4 100644 --- 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 8a32363eb4..ebee8d6bf3 100644 --- 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 Separation_item::boxes (Grob *me, Grob *left) @@ -121,7 +124,19 @@ Separation_item::boxes (Grob *me, Grob *left) vector elts; if (left) - elts = Accidental_placement::get_relevant_accidentals (read_only_elts, left); + { + vector accidental_elts; + vector 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 61ebd16dfe..049f4f2d65 100644 --- 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 arpeggios_; + vector 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 faf082bec1..b4861729b7 100644 --- 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.")