Re: Gets vertical skylines from grob stencils (issue 5626052)

[k-ohara5a5a]

patch
real    17m55.095s
user    17m10.848s
sys     0m11.381s
git master
real    16m16.228s
user    15m30.543s
sys     0m10.624s



http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode636
lily/axis-group-interface.cc:636: overlap whereas the horizontal
skylines do.
Really?  In the example you draw below, the lower vertical skyline of
(obj B) would intersect the upper vertical skyline of (obj A) because
the skyline fills in the concavity in (obj B)

Protect ASCII art comments with a first-column *.  Similar comments in
the note-collision code were destroyed when someone ran the emacs
auto-indenter over the files.

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode706
lily/axis-group-interface.cc:706: do
// ... while dirty

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode746
lily/axis-group-interface.cc:746: egg doll style, there will be no
"intersection" because they never
Skyline:distance() would report this case, with a negative distance.

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780
lily/axis-group-interface.cc:780: while (dirty);
I am beginning to understand the new code. Would you consider again
using distance() instead of the repeated calls to intersects() in the
while(dirty) loop ?  It might look simpler now that you've been away
from the code for a while.

The Skyline::distance()s have all the information you want.  If you are
trying to place, by moving UP, an object with skylines 'pair' among a
set of skylines 'forest[j]' then the distance you need to move has
several constraints

floors[j] = padding - forest[j][UP]~distance~pair[DOWN]
ceilings[j] = forest[j][DOWN]~distance~pair[UP]

You know the solution will be to move an amount equal to one of the
floors[j], and you want to find the smallest that fits, so make
  trials = sort (floors)
and find the smallest among trials that satisfies
   trials[n] > 0
&&  for all j (trials[n] > floors[j]
               || trials[n] < ceilings[j] )

Somebody might complain that this is quadratic in the number of objects
to place, but the problem placing N objects while looking for holes left
in earlier placements is a quadratic task.  But now the inner loop is
simple comparisons, rather than re-checking of skyline intersections and
distances with the while(dirty) method.

In any case, the number N seems to be sufficiently limited in practice
(thanks maybe to 'exempt') based on reported score compilation times.

http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode945
lily/axis-group-interface.cc:945: feature present in
tuplet-number-outside-staff-priority.
If I remove all additions to the 'riders' array,
'tuplet-number-outside-staff-priority.ly' stays the same.

I tried, but failed, to create a case where keeping track of 'riders'
helps.
  \relative c'' {
    \override TupletBracket #'outside-staff-priority = #1
    \times 2/3 { a8\trill a\trill a\trill } }
What case did you use ?

http://codereview.appspot.com/5626052/diff/106004/lily/skyline.cc
File lily/skyline.cc (right):

http://codereview.appspot.com/5626052/diff/106004/lily/skyline.cc#newcode793
lily/skyline.cc:793: For systems, padding is not added at creation time.
 Padding is
Stray comment?

http://codereview.appspot.com/5626052/