Reviewers: lemzwerg, Message: no, it's in this commit. I actually folded in https://codereview.appspot.com/555770044/ , but maybe it will be submitted separately.
https://codereview.appspot.com/553980043/diff/549960044/lily/figured-bass-position-engraver.cc File lily/figured-bass-position-engraver.cc (right): https://codereview.appspot.com/553980043/diff/549960044/lily/figured-bass-position-engraver.cc#newcode99 lily/figured-bass-position-engraver.cc:99: support_.push_back (info.grob ()); here https://codereview.appspot.com/553980043/diff/549960044/scm/define-grobs.scm File scm/define-grobs.scm (right): https://codereview.appspot.com/553980043/diff/549960044/scm/define-grobs.scm#newcode319 scm/define-grobs.scm:319: (add-stem-support . #t) and here Description: Calculate skylines only once. Before, Axis_group_interface::skyline_spacing() would call the function add_interior_skylines(), which recursed into VerticalAxisGroups. This caused staff-skylines to be computed twice: once as part of the Staff's VerticalAxisGroup, and once to compute the skyline for System. Instead, in Axis_group_interface::skyline_spacing(), compute the vertical-skylines for all constituent elements. Since the property is subject to caching, the Staff skyline is only computed once. To make this work * Add flags to the NoteColumn using Axis_group_interface::add_element * add vertical-skylines callbacks for NoteColumn and NoteCollision, which are also X,Y Axis groups. * declare add-stem-support for bass figures (or the digits are meshed in with stems that stick out of the staff.) * calculate a skyline for BassFigureAlignment, otherwise, the skyline is computed from the extent of the alignment, which is inaccurate if some bass figures have accidentals. Formatting impact: * ottava-edge.ly - the ottava bracket meshes better with the stem, leading to tighter spacing. Timing impact ac49229cdf - Calculate skylines only once. baseline: eaf40071f5 Use vectors rather than lists for skylines. args: -I carver MSDM memory: med diff 1916 (stddevs 103 135, n=5) memory: med diff 0.2 % (ac49229cdf is fatter) time: med diff -0.37 (stddevs 0.08 0.14, n=5) time: med diff -0.8 % (ac49229cdf is faster) Please review this at https://codereview.appspot.com/553980043/ Affected files (+43, -34 lines): M lily/axis-group-interface.cc M lily/figured-bass-position-engraver.cc M lily/rhythmic-column-engraver.cc M scm/define-grobs.scm Index: lily/axis-group-interface.cc diff --git a/lily/axis-group-interface.cc b/lily/axis-group-interface.cc index 68453459f37eade1aef3e08e9d24835647e697dd..9a4a47067cb8d04d140dac3b838aa0d3b09fb7d8 100644 --- a/lily/axis-group-interface.cc +++ b/lily/axis-group-interface.cc @@ -655,28 +655,6 @@ pure_staff_priority_less (Grob *const &g1, Grob *const &g2) return priority_1 < priority_2; } -static void -add_interior_skylines (Grob *me, Grob *x_common, Grob *y_common, vector<Skyline_pair> *skylines) -{ - if (Grob_array *elements = unsmob<Grob_array> (get_object (me, "elements"))) - { - for (vsize i = 0; i < elements->size (); i++) - add_interior_skylines (elements->grob (i), x_common, y_common, skylines); - } - else if (!scm_is_number (get_property (me, "outside-staff-priority")) - && !to_boolean (get_property (me, "cross-staff"))) - { - Skyline_pair *maybe_pair = unsmob<Skyline_pair> (get_property (me, "vertical-skylines")); - if (!maybe_pair) - return; - if (maybe_pair->is_empty ()) - return; - skylines->push_back (Skyline_pair (*maybe_pair)); - skylines->back ().shift (me->relative_coordinate (x_common, X_AXIS)); - skylines->back ().raise (me->relative_coordinate (y_common, Y_AXIS)); - } -} - // Raises the grob elt (whose skylines are given by h_skyline // and v_skyline) so that it doesn't intersect with staff_skyline, // or with anything in other_h_skylines and other_v_skylines. @@ -880,7 +858,12 @@ Axis_group_interface::outside_staff_ancestor (Grob *me) Skyline_pair Axis_group_interface::skyline_spacing (Grob *me) { - extract_grob_set (me, unsmob<Grob_array> (get_object (me, "vertical-skyline-elements")) ? "vertical-skyline-elements" : "elements", fakeelements); + extract_grob_set ( + me, + unsmob<Grob_array> (get_object (me, "vertical-skyline-elements")) + ? "vertical-skyline-elements" + : "elements", + fakeelements); vector<Grob *> elements (fakeelements); for (vsize i = 0; i < elements.size (); i++) /* @@ -922,15 +905,28 @@ Axis_group_interface::skyline_spacing (Grob *me) vsize i = 0; vector<Skyline_pair> inside_staff_skylines; - for (i = 0; i < elements.size () - && !scm_is_number (get_property (elements[i], "outside-staff-priority")); i++) + for (i = 0; + i < elements.size () + && !scm_is_number (get_property (elements[i], "outside-staff-priority")); + i++) { Grob *elt = elements[i]; - Grob *ancestor = outside_staff_ancestor (elt); - if (!(to_boolean (get_property (elt, "cross-staff")) || ancestor)) - add_interior_skylines (elt, x_common, y_common, &inside_staff_skylines); - if (ancestor) + if (Grob *ancestor = outside_staff_ancestor (elt)) riders.insert (pair<Grob *, Grob *> (ancestor, elt)); + else if (!to_boolean (get_property (elt, "cross-staff"))) + { + Skyline_pair *maybe_pair + = unsmob<Skyline_pair> (get_property (elt, "vertical-skylines")); + if (!maybe_pair) + continue; + if (maybe_pair->is_empty ()) + continue; + inside_staff_skylines.push_back (Skyline_pair (*maybe_pair)); + inside_staff_skylines.back ().shift ( + me->relative_coordinate (x_common, X_AXIS)); + inside_staff_skylines.back ().raise ( + me->relative_coordinate (y_common, Y_AXIS)); + } } Skyline_pair skylines (inside_staff_skylines); Index: lily/figured-bass-position-engraver.cc diff --git a/lily/figured-bass-position-engraver.cc b/lily/figured-bass-position-engraver.cc index 0fcca7216642bc859f28dfc23d975259b76a8031..1156a1bcc4c1227c33c091748329793534feff4b 100644 --- a/lily/figured-bass-position-engraver.cc +++ b/lily/figured-bass-position-engraver.cc @@ -40,6 +40,7 @@ class Figured_bass_position_engraver : public Engraver protected: void acknowledge_note_column (Grob_info); void acknowledge_slur (Grob_info); + void acknowledge_stem (Grob_info); void acknowledge_end_slur (Grob_info); void acknowledge_end_tie (Grob_info); void acknowledge_bass_figure_alignment (Grob_info); @@ -92,6 +93,12 @@ Figured_bass_position_engraver::acknowledge_note_column (Grob_info info) support_.push_back (info.grob ()); } +void +Figured_bass_position_engraver::acknowledge_stem (Grob_info info) +{ + support_.push_back (info.grob ()); +} + void Figured_bass_position_engraver::acknowledge_end_slur (Grob_info info) { @@ -146,6 +153,7 @@ Figured_bass_position_engraver::boot () { ADD_ACKNOWLEDGER (Figured_bass_position_engraver, note_column); ADD_ACKNOWLEDGER (Figured_bass_position_engraver, slur); + ADD_ACKNOWLEDGER (Figured_bass_position_engraver, stem); ADD_END_ACKNOWLEDGER (Figured_bass_position_engraver, slur); ADD_END_ACKNOWLEDGER (Figured_bass_position_engraver, tie); ADD_ACKNOWLEDGER (Figured_bass_position_engraver, bass_figure_alignment); Index: lily/rhythmic-column-engraver.cc diff --git a/lily/rhythmic-column-engraver.cc b/lily/rhythmic-column-engraver.cc index 189c99d551d42ad121c0a430e1d254826553c2b3..f4e5b12daefe3b8daee5a5cd75a11ea44d923b51 100644 --- a/lily/rhythmic-column-engraver.cc +++ b/lily/rhythmic-column-engraver.cc @@ -17,13 +17,14 @@ along with LilyPond. If not, see <http://www.gnu.org/licenses/>. */ +#include "axis-group-interface.hh" +#include "dot-column.hh" #include "engraver.hh" -#include "rhythmic-head.hh" -#include "stem.hh" -#include "note-column.hh" #include "item.hh" -#include "dot-column.hh" +#include "note-column.hh" #include "pointer-group-interface.hh" +#include "rhythmic-head.hh" +#include "stem.hh" #include "translator.icc" @@ -105,7 +106,7 @@ Rhythmic_column_engraver::process_acknowledged () if (flag_) { - Pointer_group_interface::add_grob (note_column_, ly_symbol2scm ("elements"), flag_); + Axis_group_interface::add_element (note_column_, flag_); flag_ = 0; } } Index: scm/define-grobs.scm diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm index 391cbafe930ba9ef9f730b581b9ffc6dead2ee17..25fd272eba75884b4384451df1756d880d772d97 100644 --- a/scm/define-grobs.scm +++ b/scm/define-grobs.scm @@ -302,6 +302,7 @@ (stacking-dir . ,DOWN) (X-extent . ,ly:axis-group-interface::width) (Y-extent . ,axis-group-interface::height) + (vertical-skylines . ,ly:axis-group-interface::calc-skylines) (meta . ((class . Spanner) (object-callbacks . ((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common) (pure-relevant-grobs . ,ly:axis-group-interface::calc-pure-relevant-grobs))) @@ -315,6 +316,7 @@ (direction . ,UP) (padding . 0.5) (side-axis . ,Y) + (add-stem-support . #t) (staff-padding . 1.0) (X-extent . ,ly:axis-group-interface::width) (Y-extent . ,axis-group-interface::height) @@ -1680,6 +1682,7 @@ (prefer-dotted-right . #t) (X-extent . ,ly:axis-group-interface::width) (Y-extent . ,axis-group-interface::height) + (vertical-skylines . ,ly:axis-group-interface::calc-skylines) (meta . ((class . Item) (object-callbacks . ((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common) (pure-relevant-grobs . ,ly:axis-group-interface::calc-pure-relevant-grobs))) @@ -1694,6 +1697,7 @@ (skyline-vertical-padding . 0.15) (X-extent . ,ly:axis-group-interface::width) (Y-extent . ,axis-group-interface::height) + (vertical-skylines . ,ly:axis-group-interface::calc-skylines) (meta . ((class . Item) (object-callbacks . ((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common) (pure-relevant-grobs . ,ly:axis-group-interface::calc-pure-relevant-grobs)))
