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

[mtsolo]

New patch set coming after I finish running the regtests.


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: }
On 2013/02/10 03:08:04, Keith wrote:
> ... 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)

Regtest changed.  I've removed the Stem_engraver in the regtest so that
the only possible side support element is the note head.

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
());
On 2013/02/10 03:08:04, Keith wrote:
> 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.

I'm not sure if these changes can be separated from the skyline stuff
without causing regtest havoc.

I can do this, but it'd take me more time (figure out what changes to
isolate, run regtests, etc.).  Is it necessary?

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];
On 2013/02/10 03:08:04, Keith wrote:
> // 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];

Done.

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

Comment eliminated with change above.

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
On 2013/02/10 03:08:04, Keith wrote:
> 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();

No idea...

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

Done.

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode104
lily/skyline.cc:104: Interval start_end = fatten_skinny_buildings
(start, end);
On 2013/02/10 03:08:04, Keith wrote:
> 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.

But this adds the EPS arbitrarily to the right instead of adding an
equal amount to the right and left...

And I hate code duplication.

https://codereview.appspot.com/7310075/diff/10/lily/skyline.cc#newcode119
lily/skyline.cc:119:
On 2013/02/10 03:08:04, Keith wrote:
> // Buildings all have width at least EPS
> end = min(end, start + EPS);

See above.

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.
On 2013/02/10 03:08:04, Keith wrote:
> This comment would no longer belong here

Changed to represent use of EPS as minimum fatness.

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.
On 2013/02/10 03:08:04, Keith wrote:
> This comment would no longer belong here

Changed to represent use of EPS as minimum fatness.

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