Re: Standardizes use of empty extents in pure heights and skylines. (issue 7310075)

[k-ohara5a5a]

https://codereview.appspot.com/7310075/diff/10/input/regression/skyline-point-extent.ly
File input/regression/skyline-point-extent.ly (right):

https://codereview.appspot.com/7310075/diff/10/input/regression/skyline-point-extent.ly#newcode7
input/regression/skyline-point-extent.ly:7: }
... The accents should follow the descending melody line, even though
the note-head stencils are empty."}
{ \override NoteHead #'stencil = #point-stencil
  c'2.-> b8-- a-- g1-> }
#(ly:set-option 'debug-skylines)

https://codereview.appspot.com/7310075/diff/10/input/regression/skyline-point-extent.ly#newcode12
input/regression/skyline-point-extent.ly:12: \type Engraver_group
You don't need a custom context to test this, so there's no reason to
make anyone who might investigate a change in this output try to figure
all this out.

https://codereview.appspot.com/7310075/diff/10/input/regression/skyline-point-extent.ly#newcode20
input/regression/skyline-point-extent.ly:20: \override VerticalAxisGroup
#'default-staff-staff-spacing = #'((basic_distance . 0)
(minimum_distance . 10) (padding . 6) (stretchability . 0))
minimum_distance is misspelled, so it has no effect.
if minimum-distance is 10, your patch has no effect on the output.

https://codereview.appspot.com/7310075/diff/10/lily/grob.cc
File lily/grob.cc (right):

https://codereview.appspot.com/7310075/diff/10/lily/grob.cc#newcode488
lily/grob.cc:488: Interval iv = robust_scm2interval (iv_scm, Interval
());
I assume you will apply this change and the change in separation-item.cc
in a separate commit, the "empty extents in pure-heights" part of the
patch set.

https://codereview.appspot.com/7310075/diff/10/lily/separation-item.cc
File lily/separation-item.cc (right):

https://codereview.appspot.com/7310075/diff/10/lily/separation-item.cc#newcode153
lily/separation-item.cc:153: x[LEFT] += extra_width[LEFT];
// The conventional empty extent is (+inf.0 . -inf.0)
//  but (-inf.0 . +inf.0) is used as extra-spacing-height
//  on items that must not overlap other note-columns.
// If these two uses of inf combine, leave the empty extent.

if (!isinf(x[LEFT]) x[LEFT] += extra_width[LEFT];
if (!isinf(x[RIGHT]) x[RIGHT] += extra_width[RIGHT];

https://codereview.appspot.com/7310075/diff/10/lily/separation-item.cc#newcode159
lily/separation-item.cc:159: // empty interval with infinity at either
end
Other than the addition above, what other 'operation' can produce a NaN
?

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc
File lily/skyline.cc (right):

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode59
lily/skyline.cc:59: /* If we start including very thin buildings,
numerical accuracy errors can
Did you find where these numerical accuracy errors occured?   I don't
see anything obvious.  The most likely seems to be the way we step
through the two skylines in internal_merge_skylines();

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode61
lily/skyline.cc:61: than epsilon wide. In merges, we omit them.
Any such buildings are created in merge_skyline() are omitted from the
merged result.

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode104
lily/skyline.cc:104: Interval start_end = fatten_skinny_buildings
(start, end);
The old-fashioned way would be
  // Buildings all have width at least EPS
  end = min(end, start + EPS);
and the same in other constructors, but I know how you hate
code-duplication.

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode119
lily/skyline.cc:119:
// Buildings all have width at least EPS
end = min(end, start + EPS);

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode497
lily/skyline.cc:497: Boxes should have fatness in the horizon_axis,
otherwise they are ignored.
This comment would no longer belong here

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode519
lily/skyline.cc:519: Buildings should have fatness in the horizon_axis,
otherwise they are ignored.
This comment would no longer belong here

https://codereview.appspot.com/7310075/