lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Remove special case in staff-spacing (issue4188051)


From: k-ohara5a5a
Subject: Re: Remove special case in staff-spacing (issue4188051)
Date: Tue, 26 Jul 2011 04:30:44 +0000

Reviewers: J_lowe, joeneeman,

Message:
lily/include/spacing-spanner.hh:45: static Real cushion_;
I think this should be a grob property rather than a static variable.

I'm scared of the property-lookup costs.  I might be wiser to leave it
where it was in Springs::merge() , and just apply
full-measure-extra-space to the spring lengths *before* the call to
merge().

I see now that the 'fixed'-distance concept in
Staff_spacing::get_spacing() assumes Spring's compressibility still
works in a pre-2007 manner. I'll take this patch off review while I look
at some history to understand the intent of the code.

Description:
In note spacing after a breakable item,
add up all the contributions to the ideal length of the
spacing (the spring.distance) before checking the ideal
spacing for near-collisions.

This avoids unnecessary lengthening when later contributions
to the desired length would clear near-collisions anyway.


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

Affected files:
  M lily/include/spacing-spanner.hh
  M lily/spacing-spanner.cc
  M lily/spring.cc
  M lily/staff-spacing.cc
  M scm/define-grobs.scm


Index: lily/include/spacing-spanner.hh
diff --git a/lily/include/spacing-spanner.hh b/lily/include/spacing-spanner.hh index d469c3f4636a9069065caf0c688afe0b93700319..3a986ccecce9cae27a4433fb5af3183ec0b5d40e 100644
--- a/lily/include/spacing-spanner.hh
+++ b/lily/include/spacing-spanner.hh
@@ -42,6 +42,7 @@ private:
   static void set_implicit_neighbor_columns (vector<Grob*> const &cols);
static void generate_springs (Grob *me, vector<Grob*> const &cols, Spacing_options const *); static void musical_column_spacing (Grob *, Item *, Item *, Spacing_options const *);
+  static Real cushion_;
   static bool fills_measure (Grob *, Item *, Item *);
 public:
   static vector<Grob*> get_columns (Grob *me);
Index: lily/spacing-spanner.cc
diff --git a/lily/spacing-spanner.cc b/lily/spacing-spanner.cc
index 0395506bbcb79dbecc9e1423e08bcee9d821bdc0..18565b622b730f0d1a4e00f057460ba7eea80031 100644
--- a/lily/spacing-spanner.cc
+++ b/lily/spacing-spanner.cc
@@ -319,6 +319,11 @@ Spacing_spanner::generate_springs (Grob *me,
   set_column_rods (cols, padding);
 }

+/* Ensure that the ideal distance between notecolumns is greater than
+   the minimum distance by this amount:
+*/
+Real Spacing_spanner::cushion_ = 0.3;
+
 /*
   Generate the space between two musical columns LEFT_COL and RIGHT_COL.
 */
@@ -401,7 +406,12 @@ Spacing_spanner::musical_column_spacing (Grob *me,
            }
        }
       else
-       spring = merge_springs (springs);
+       {
+           spring = merge_springs (springs);
+           /* Leave a little headroom above the merged minimum distance */
+           if (spring.distance() < spring.min_distance () + cushion_)
+             spring.set_distance (spring.min_distance () + cushion_);
+       }
     }

   if (Paper_column::when_mom (right_col).grace_part_
@@ -542,13 +552,17 @@ Spacing_spanner::breakable_column_spacing (Grob *me, Item *l, Item *r,
       spring.set_distance (spring.distance () + full_measure_extra_space);
       spring.set_default_compress_strength ();
     }
-
+
   if (options->stretch_uniformly_ && l->break_status_dir () != RIGHT)
     {
       spring.set_min_distance (0.0);
       spring.set_default_strength ();
     }

+  /* Leave a little headroom above the merged minimum distance */
+  if (spring.distance() < spring.min_distance () + cushion_)
+    spring.set_distance (spring.min_distance () + cushion_);
+
   Spaceable_grob::add_spring (l, r, spring);
 }

Index: lily/spring.cc
diff --git a/lily/spring.cc b/lily/spring.cc
index c576da374329bf22b196982c3276343587b56fbb..38720b0bd2cbc3ce094e8dd33a66e44b3225baf9 100644
--- a/lily/spring.cc
+++ b/lily/spring.cc
@@ -80,8 +80,7 @@ Spring::operator> (Spring const &other) const
   return blocking_force_ > other.blocking_force_;
 }

-/* merge springs, basically by averaging them, but leave a little headroom
- above the largest minimum distance so that things don't get too cramped */
+/* merge springs, basically by averaging them */
 Spring
 merge_springs (vector<Spring> const &springs)
 {
@@ -101,7 +100,7 @@ merge_springs (vector<Spring> const &springs)
   avg_stretch /= springs.size ();
   avg_compress /= springs.size ();
   avg_distance /= springs.size ();
-  avg_distance = max (min_distance + 0.3, avg_distance);
+  avg_distance = max (min_distance, avg_distance);

   Spring ret = Spring (avg_distance, min_distance);
   ret.set_inverse_stretch_strength (avg_stretch);
Index: lily/staff-spacing.cc
diff --git a/lily/staff-spacing.cc b/lily/staff-spacing.cc
index ff064b5c0f6a83455b4b9fd43de4f33046c17b89..7a9e38b9d13af95efece335012c6e60dbbc65a87 100644
--- a/lily/staff-spacing.cc
+++ b/lily/staff-spacing.cc
@@ -195,16 +195,11 @@ Staff_spacing::get_spacing (Grob *me, Grob *right_col)
       ideal = fixed;
     }

-
   Real optical_correction = next_notes_correction (me, last_grob);
-  Real min_dist = Paper_column::minimum_distance (left_col, right_col);
+  fixed += optical_correction;
+  ideal += optical_correction;

- /* ensure that the "fixed" distance will leave a gap of at least 0.3 ss. */
-  Real min_dist_correction = max (0.0, 0.3 + min_dist - fixed);
-  Real correction = max (optical_correction, min_dist_correction);
-
-  fixed += correction;
-  ideal += correction;
+  Real min_dist = Paper_column::minimum_distance (left_col, right_col);

   Spring ret (ideal, min_dist);
   ret.set_inverse_stretch_strength (max (0.0, ideal - fixed));
Index: scm/define-grobs.scm
diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index a5c03a52755c7cc3bdfac07960833e799ba7730b..0a20b89f4f85649a64839b41e9cc7f9a4b7ba8c6 100644
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -527,7 +527,7 @@
                        (key-signature . (minimum-space . 3.5))
                        (time-signature . (minimum-space . 4.2))
                        (first-note . (minimum-fixed-space . 5.0))
-                       (next-note . (extra-space . 0.5))
+                       (next-note . (semi-fixed-space . 0.8))
                        (right-edge . (extra-space . 0.5))))
        (stencil . ,ly:clef::print)
        (Y-offset . ,ly:staff-symbol-referencer::callback)
@@ -593,7 +593,7 @@
                        (time-signature . (minimum-space . 4.2))
                        (custos . (minimum-space . 0.0))
                        (first-note . (minimum-fixed-space . 3.0))
-                       (next-note . (extra-space . 0.5))
+                       (next-note . (semi-fixed-space . 0.8))
                        (right-edge . (extra-space . 0.5))))
        (stencil . ,ly:clef::print)
        (Y-offset . ,ly:staff-symbol-referencer::callback)
@@ -620,7 +620,7 @@
                        (key-signature . (minimum-space . 3.5))
                        (time-signature . (minimum-space . 4.2))
                        (first-note . (minimum-fixed-space . 5.0))
-                       (next-note . (extra-space . 0.5))
+                       (next-note . (semi-fixed-space . 0.8))
                        (right-edge . (extra-space . 0.5))))
        (stencil . ,ly:clef::print)
        (Y-offset . ,ly:staff-symbol-referencer::callback)
@@ -2235,6 +2235,7 @@
        (break-align-anchor-alignment . ,LEFT)
        (break-visibility . ,all-visible)
        (extra-spacing-height . (-1.0 . 1.0))
+       (extra-spacing-width . (0.0 . 0.5))
        (non-musical . #t)
        (space-alist . (
                        (cue-clef . (extra-space . 1.5))





reply via email to

[Prev in Thread] Current Thread [Next in Thread]