Fix Issue 1290 (issue3832046)

[Carl . D . Sorensen]

Reviewers: ,

Message:
Here is a patch to fix issue 1290.

It works, but it may need to be cleaned up.  I'm not sure the code is as
elegant as it could be.  I'm not really comfortable with all of the C++
syntax used in lilypond.

Please review it carefully, and let me know how it can be improved.

Thanks,

Carl


Description:
Fix Issue 1290

Create an optional horizontal-padding parameter to Skyline::distance
When horizontal-padding is not zero, padding is added to both systems
when calculating the distance.

Get the System's skyline-horizontal-padding property, and pass
it to distance () in page-layout-problem.cc

Add regression test for this feature.

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

Affected files:
  A input/regression/skyline-horizontal-padding.ly
  M lily/include/skyline.hh
  M lily/page-layout-problem.cc
  M lily/skyline.cc


Index: input/regression/skyline-horizontal-padding.ly
diff --git a/input/regression/skyline-horizontal-padding.ly  
b/input/regression/skyline-horizontal-padding.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..c3e96211304a824c5d8304c2faf43d1b638aa24d
--- /dev/null
+++ b/input/regression/skyline-horizontal-padding.ly
@@ -0,0 +1,20 @@
+\version "2.13.45"
+
+\header {
+  texidoc = "
+The skyline-horizontal-padding property can be set for System
+in order to keep systems from being spaced too closely together.
+In this example, the low notes from a system should not be
+interleaved with the high notes from the next system.
+"
+}
+
+\score {
+\repeat unfold 80 { <c'''-1 e'''-3 g'''-5> c' <c,-1 e,-3 g,-5> c' }
+  \layout {
+    \context {
+      \Score
+      \override System #'skyline-horizontal-padding = #2
+    }
+  }
+}
Index: lily/include/skyline.hh
diff --git a/lily/include/skyline.hh b/lily/include/skyline.hh
index  
a11b21dde6ce27d06a3e54b177e32940da779589..93b6e1155605d4c03fb8d8b566c40bf8ce80bd6e  
100644
--- a/lily/include/skyline.hh
+++ b/lily/include/skyline.hh
@@ -53,7 +53,7 @@ class Skyline
 private:
   list<Building> buildings_;
   Direction sky_;
-
+
   void internal_merge_skyline (list<Building>*, list<Building>*,
                               list<Building> *const result);
   list<Building> internal_build_skyline (list<Box>*, Real, Axis,  
Direction);
@@ -63,10 +63,11 @@ private:
 public:
   Skyline ();
   Skyline (Skyline const &src);
+  Skyline (Skyline const &src, Real horizon_padding, Axis a);
   Skyline (Direction sky);
   Skyline (vector<Box> const &bldgs, Real horizon_padding, Axis a,  
Direction sky);
   Skyline (Box const &b, Real horizon_padding, Axis a, Direction sky);
-
+
   vector<Offset> to_points (Axis) const;
   void merge (Skyline const &);
   void insert (Box const &, Real horizon_padding, Axis);
@@ -74,7 +75,7 @@ public:
   void print_points () const;
   void raise (Real);
   void shift (Real);
-  Real distance (Skyline const &) const;
+  Real distance (Skyline const &, Real horizon_padding = 0) const;
   Real height (Real airplane) const;
   Real max_height () const;
   void set_minimum_height (Real height);
Index: lily/page-layout-problem.cc
diff --git a/lily/page-layout-problem.cc b/lily/page-layout-problem.cc
index  
673278f69c6fc9490ab118d406610531064b6d9e..c30f43d84a40464278e078313fe34ae0b3d89a9d  
100644
--- a/lily/page-layout-problem.cc
+++ b/lily/page-layout-problem.cc
@@ -271,7 +271,7 @@ Page_layout_problem::append_prob (Prob *prob, Spring  
const& spring, Real padding

   if (sky)
     {
-      minimum_distance = (*sky)[UP].distance (bottom_skyline_);
+      minimum_distance = (*sky)[UP].distance (bottom_skyline_,  
scm_to_double (prob->get_property ("skyline-horizontal-padding")));
       bottom_skyline_ = (*sky)[DOWN];
     }
   else if (Stencil *sten = unsmob_stencil (prob->get_property ("stencil")))
Index: lily/skyline.cc
diff --git a/lily/skyline.cc b/lily/skyline.cc
index  
d8105e669b910eaadc3fe8e34286ae9d613c8e50..5283cd25cee6f1bce634269eb0b5fa33e48acd90  
100644
--- a/lily/skyline.cc
+++ b/lily/skyline.cc
@@ -119,7 +119,7 @@ Building::precompute (Real start, Real start_height,  
Real end_height, Real end)
     y_intercept_ = start_height - slope_ * start;
 }

-Real
+Real
 Building::height (Real x) const
 {
   return isinf (x) ? y_intercept_ : slope_*x + y_intercept_;
@@ -248,7 +248,7 @@ single_skyline (Building b, Real start, Real  
horizon_padding, list<Building> *co
                               -infinity_f, infinity_f));
   if (sloped_neighbours)
     ret->push_front (b.sloped_neighbour (start, horizon_padding, RIGHT));
-
+
   if (b.end_ > start + EPS)
     ret->push_front (b);

@@ -367,7 +367,7 @@ Skyline::Skyline ()
 Skyline::Skyline (Skyline const &src)
 {
   sky_ = src.sky_;
-
+
   /* doesn't a list's copy constructor do this? -- jneem */
   for (list<Building>::const_iterator i = src.buildings_.begin ();
        i != src.buildings_.end (); i++)
@@ -383,6 +383,26 @@ Skyline::Skyline (Direction sky)
 }

 /*
+  build padded skyline from an existing skyline with padding
+  added to it.
+*/
+
+Skyline::Skyline (Skyline const &src, Real horizon_padding, Axis a)
+{
+  Real start = -infinity_f;
+  vector<Box> boxes;
+  list<Building>::const_iterator end = src.buildings_.end ();
+  for (list<Building>::const_iterator i = src.buildings_.begin (); i !=  
end; start=i->end_, i++ )
+    if ((i->slope_ == 0) && !isinf (i->y_intercept_) && i->y_intercept_ >=  
0)
+      boxes.push_back (Box (Interval (start, i->end_),
+	                    Interval (i->y_intercept_ < 0 ? i->y_intercept_ : 0 ,  
i->y_intercept_)));
+  Skyline padSky = Skyline (boxes, horizon_padding, a, (Direction) 1);
+  buildings_ = padSky.buildings_;
+  sky_ = src.sky_;
+}
+
+
+/*
   build skyline from a set of boxes. If horizon_padding > 0, expand all  
the boxes
   by that amount and add 45-degree sloped boxes to the edges of each box  
(of
   width horizon_padding). That is, the total amount of horizontal  
expansion is
@@ -404,7 +424,7 @@ Skyline::Skyline (vector<Box> const &boxes, Real  
horizon_padding, Axis horizon_a
       if (iv.length () > EPS && !boxes[i][vert_axis].is_empty ())
        filtered_boxes.push_front (boxes[i]);
     }
-
+
   buildings_ = internal_build_skyline (&filtered_boxes, horizon_padding,  
horizon_axis, sky);
 }

@@ -470,15 +490,25 @@ Skyline::shift (Real s)
 }

 Real
-Skyline::distance (Skyline const &other) const
+Skyline::distance (Skyline const &other, Real horizon_padding) const
 {
   assert (sky_ == -other.sky_);
-  list<Building>::const_iterator i = buildings_.begin ();
-  list<Building>::const_iterator j = other.buildings_.begin ();
+
+  const Skyline *padded_this = this;
+  const Skyline *padded_other = &other;
+
+  if (horizon_padding != 0.0)
+    {
+      padded_this = new Skyline (*padded_this, horizon_padding, X_AXIS);
+      padded_other = new Skyline (*padded_other, horizon_padding, X_AXIS);
+    }
+
+  list<Building>::const_iterator i = padded_this->buildings_.begin ();
+  list<Building>::const_iterator j = padded_other->buildings_.begin ();

   Real dist = -infinity_f;
   Real start = -infinity_f;
-  while (i != buildings_.end () && j != other.buildings_.end ())
+  while (i != padded_this->buildings_.end () && j !=  
padded_other->buildings_.end ())
     {
       Real end = min (i->end_, j->end_);
       Real start_dist = i->height (start) + j->height (start);