Permits all positive and zero-length buildings in skylines (issue 3161)

This allows point stencils to figure into vertical spacing, as shown
in the new regtest input/regression/skyline-point-extent.ly.

Some issues regarding floating point errors were raised while reviewing
this patch, but it was concluded that the math in this patch likely does
not risk triggering those errors with the way GCC compiles the code.

diff --git a/input/regression/skyline-point-extent.ly b/input/regression/skyline-point-extent.ly
new file mode 100644 (file)
index 0000000..ceb27b2
--- /dev/null
+++ b/input/regression/skyline-point-extent.ly
@@ -0,0 +1,22 @@
+\version "2.17.15"
+
+\header {
+  texidoc = "The @code{Script} grobs should follow the descending melody line,
+even though the @code{NoteHead} stencils are point stencils. The
+@code{Stem_engraver} is removed so that the only
+@code{side-support-element} is the @code{NoteHead}.
+"
+}
+
+\layout {
+  \context {
+    \Voice
+    \remove "Stem_engraver"
+  }
+}
+
+{
+  \override Script #'direction = #DOWN
+  \override NoteHead #'stencil = #point-stencil
+  c'2.-> b8-- a-- g1->
+}

diff --git a/lily/skyline.cc b/lily/skyline.cc
index bf95fe35418153ce65237a415a29c4d4553ecde0..9908c611d59efaea501e94e30c414a7c23ebc705 100644 (file)
--- a/lily/skyline.cc
+++ b/lily/skyline.cc
@@ -56,10 +56,6 @@
    but the distance routine does.
 */
 
-/* If we start including very thin buildings, numerical accuracy errors can
-   arise. Therefore, we ignore all buildings that are less than epsilon wide. */
-#define EPS 1e-5
-
 static void
 print_buildings (list<Building> const &b)
 {
@@ -301,8 +297,7 @@ Skyline::internal_merge_skyline (list<Building> *s1, list<Building> *s2,
           break;
         }
 
-      /* only include buildings wider than epsilon */
-      if (end > x + EPS)
+      if (end >= x)
         {
           b.leading_part (end);
           b.start_ = last_end;
@@ -329,20 +324,15 @@ empty_skyline (list<Building> *const ret)
 static void
 single_skyline (Building b, list<Building> *const ret)
 {
-  if (b.end_ > b.start_ + EPS)
-    {
-      if (b.start_ != -infinity_f)
-        ret->push_back (Building (-infinity_f, -infinity_f,
-                                  -infinity_f, b.start_));
-      ret->push_back (b);
-      if (b.end_ != infinity_f)
-        ret->push_back (Building (b.end_, -infinity_f,
-                                  -infinity_f, infinity_f));
-    }
-  else
-    {
-      empty_skyline (ret);
-    }
+  assert (b.end_ >= b.start_);
+
+  if (b.start_ != -infinity_f)
+    ret->push_back (Building (-infinity_f, -infinity_f,
+                              -infinity_f, b.start_));
+  ret->push_back (b);
+  if (b.end_ != infinity_f)
+    ret->push_back (Building (b.end_, -infinity_f,
+                              -infinity_f, infinity_f));
 }
 
 /* remove a non-overlapping set of boxes from BOXES and build a skyline
@@ -377,7 +367,7 @@ non_overlapping_skyline (list<Building> *const buildings)
           continue;
         }
 
-      if (x1 > last_end + EPS)
+      if (x1 >= last_end)
         result.push_back (Building (last_end, -infinity_f, -infinity_f, x1));
 
       result.push_back (*i);
@@ -476,20 +466,16 @@ Skyline::Skyline (Direction sky)
 /*
   Build skyline from a set of boxes.
 
-  Boxes should have fatness in the horizon_axis, otherwise they are ignored.
+  Boxes should be non-empty on both axes.  Otherwise, they will be ignored
  */
 Skyline::Skyline (vector<Box> const &boxes, Axis horizon_axis, Direction sky)
 {
   list<Building> buildings;
   sky_ = sky;
 
-  Axis vert_axis = other_axis (horizon_axis);
   for (vsize i = 0; i < boxes.size (); i++)
-    {
-      Interval iv = boxes[i][horizon_axis];
-      if (iv.length () > EPS && !boxes[i][vert_axis].is_empty ())
-        buildings.push_front (Building (boxes[i], horizon_axis, sky));
-    }
+    if (!boxes[i].is_empty ())
+      buildings.push_front (Building (boxes[i], horizon_axis, sky));
 
   buildings_ = internal_build_skyline (&buildings);
   normalize ();
@@ -498,7 +484,8 @@ Skyline::Skyline (vector<Box> const &boxes, Axis horizon_axis, Direction sky)
 /*
   build skyline from a set of line segments.
 
-  Buildings should have fatness in the horizon_axis, otherwise they are ignored.
+  Segments can be articulated from left to right or right to left.
+  In the case of the latter, they will be stored internally as left to right.
  */
 Skyline::Skyline (vector<Drul_array<Offset> > const &segments, Axis horizon_axis, Direction sky)
 {
@@ -518,7 +505,7 @@ Skyline::Skyline (vector<Drul_array<Offset> > const &segments, Axis horizon_axis
       Real y1 = left[other_axis (horizon_axis)] * sky;
       Real y2 = right[other_axis (horizon_axis)] * sky;
 
-      if (x1 + EPS < x2)
+      if (x1 <= x2)
         buildings.push_back (Building (x1, y1, y2, x2));
     }
 
@@ -594,8 +581,7 @@ Skyline::insert (Box const &b, Axis a)
     }
 
   /* do the same filtering as in Skyline (vector<Box> const&, etc.) */
-  Interval iv = b[a];
-  if (iv.length () <= EPS || b[other_axis (a)].is_empty ())
+  if (b.is_empty ())
     return;
 
   my_bld.splice (my_bld.begin (), buildings_);
@@ -654,6 +640,12 @@ Skyline::internal_distance (Skyline const &other, Real horizon_padding, Real *to
 Skyline
 Skyline::padded (Real horizon_padding) const
 {
+  if (horizon_padding < 0.0)
+    warning ("Cannot have negative horizon padding.  Junking.");
+
+  if (horizon_padding <= 0.0)
+    return *this;
+
   list<Building> pad_buildings;
   for (list<Building>::const_iterator i = buildings_.begin (); i != buildings_.end (); ++i)
     {

diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index effaf131faa22f1dce75d52d8738b4c4eeb71ae7..a65ff1782a8a1540ccd6ac837ead713345a4c5a0 100644 (file)
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -1994,7 +1994,7 @@
        (extra-spacing-height . ,pure-from-neighbor-interface::extra-spacing-height)
        ; we want this to be ignored, so empty, but the extra spacing height
        ; should preserve the span bar's presence for horizontal spacing
-       (Y-extent . ,pure-from-neighbor-interface::unobtrusive-height)
+       (Y-extent . ,pure-from-neighbor-interface::height-if-pure)
        (meta . ((class . Item)
                 (object-callbacks . ((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common)
                                      (pure-relevant-grobs . ,ly:pure-from-neighbor-interface::calc-pure-relevant-grobs)))

diff --git a/scm/lily-library.scm b/scm/lily-library.scm
index 736d0c38107fed33e615fd3614f56c17ee67e8c2..d87bca208ac8e370679f35d622c5bef5d8d906ca 100644 (file)
--- a/scm/lily-library.scm
+++ b/scm/lily-library.scm
@@ -57,6 +57,8 @@
 (define-safe-public DOUBLE-SHARP 1)
 (define-safe-public SEMI-TONE 1/2)
 
+(define-safe-public INFINITY-INT 1000000)
+
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; moments
 

diff --git a/scm/output-lib.scm b/scm/output-lib.scm
index 09f526b8af631dab96ac49bfff5ba4fcaf722278..f52203468eb42ec82d42512621bd7845d7f576bc 100644 (file)
--- a/scm/output-lib.scm
+++ b/scm/output-lib.scm
@@ -91,13 +91,6 @@
     ly:grob::horizontal-skylines-from-element-stencils
     ly:grob::pure-horizontal-skylines-from-element-stencils))
 
-;; Sometimes, in horizontal spacing, we want grobs to block other grobs.
-;; They thus need to have a non-empty height.  We give them a point height
-;; so that, minimally, they block grobs directly to the right of them.
-;; Often this is complimented by an extra-spacing-height.
-;; We don't, however, want these grobs to factor into vertical spacing
-;; decisions, so we make their unpure height #f.
-
 ;; Using this as a callback for a grob's Y-extent promises
 ;; that the grob's stencil does not depend on line-spacing.
 ;; We use this promise to figure the space required by Clefs
@@ -283,8 +276,8 @@
   (if (stem-stub::do-calculations grob)
       (let* ((dad (ly:grob-parent grob X))
              (refp (ly:grob-common-refpoint grob dad Y))
-             (stem_ph (ly:grob-pure-height dad refp 0 1000000))
-             (my_ph (ly:grob-pure-height grob refp 0 1000000))
+             (stem_ph (ly:grob-pure-height dad refp 0 INFINITY-INT))
+             (my_ph (ly:grob-pure-height grob refp 0 INFINITY-INT))
              ;; only account for distance if stem is on different staff than stub
              (dist (if (grob::has-interface refp 'hara-kiri-group-spanner-interface)
                        0
@@ -505,13 +498,13 @@ and duration-log @var{log}."
       (cons -0.1 0.1)))
 
 (define-public (pure-from-neighbor-interface::extra-spacing-height grob)
-  (let* ((height (ly:grob-pure-height grob grob 0 10000000))
+  (let* ((height (ly:grob-pure-height grob grob 0 INFINITY-INT))
          (from-neighbors (interval-union
                             height
                             (ly:axis-group-interface::pure-height
                               grob
                               0
-                              10000000))))
+                              INFINITY-INT))))
     (coord-operation - from-neighbors height)))
 
 ;; If there are neighbors, we place the height at their midpoint
@@ -526,13 +519,13 @@ and duration-log @var{log}."
   (let* ((height (ly:axis-group-interface::pure-height
                   grob
                   0
-                  10000000))
+                  INFINITY-INT))
          (c (interval-center height)))
     (if (interval-empty? height) empty-interval (cons c c))))
 
 ;; Minimizes the impact of the height on vertical spacing while allowing
 ;; it to appear in horizontal skylines of paper columns if necessary.
-(define-public pure-from-neighbor-interface::unobtrusive-height
+(define-public pure-from-neighbor-interface::height-if-pure
   (ly:make-unpure-pure-container #f pure-from-neighbor-interface::pure-height))
 
 (define-public (pure-from-neighbor-interface::account-for-span-bar grob)
@@ -915,8 +908,7 @@ and duration-log @var{log}."
                            (ly:grob-relative-coordinate spanner common-y Y)))
                      (interval-end
                       (ly:grob-robust-relative-extent dots common X))
-                     ;; TODO: use real infinity constant.
-                     -10000))))
+                     (- INFINITY-INT)))))
         (right-x (max (- (interval-start
                           (ly:grob-robust-relative-extent right-span common X))
                          padding)