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/
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
