Re: Fix Issue 1290 (issue3832046)

[joeneeman]

I don't know if it's important, but it may be worth mentioning somewhere
that skyline-horizontal-padding works differently for VerticalAxisGroup
and System. (Because in VerticalAxisGroup, skyline-horizontal-padding
takes effect while the outside-staff-grobs are being placed).


http://codereview.appspot.com/3832046/diff/2001/lily/page-layout-problem.cc
File lily/page-layout-problem.cc (right):

http://codereview.appspot.com/3832046/diff/2001/lily/page-layout-problem.cc#newcode274
lily/page-layout-problem.cc:274: minimum_distance = (*sky)[UP].distance
(bottom_skyline_, scm_to_double (prob->get_property
("skyline-horizontal-padding")));
robust_scm2double is probably better

http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc
File lily/skyline.cc (right):

http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode396
lily/skyline.cc:396: if ((i->slope_ == 0) && !isinf (i->y_intercept_) &&
i->y_intercept_ >= 0)
Why restrict to y_intercept_ < 0?

http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode398
lily/skyline.cc:398: Interval (i->y_intercept_ < 0 ? i->y_intercept_ : 0
, i->y_intercept_)));
...and if you do restrict to y_intercept_ < 0, then this is redundant.

http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode399
lily/skyline.cc:399: Skyline padSky = Skyline (boxes, horizon_padding,
a, (Direction) 1);
pad_sky instead of padSky
UP instead of (Direction) 1
and perhaps a comment about why you need to use UP instead of sky_
(which would seem intuitive)

http://codereview.appspot.com/3832046/diff/2001/lily/skyline.cc#newcode498
lily/skyline.cc:498: const Skyline *padded_other = &other;
I'm not sure, but I think "Skyline const *" is more consistent with the
rest of the code.

http://codereview.appspot.com/3832046/