Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/114002/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/114002/lily/skyline.cc#newcode111
lily/skyline.cc:111: if (start_height != end_height)
Does anyone know the reason for this change?
For some unusual input (issue 2836)
{ \override NoteHead #'stencil = ##f b2 b }
both _heights can be NaNs, due to some other change in this patch.
This re-arrangement of logic looks like it /might/ have been intended to
set slope_=0 for NaNs on input, but in fact we get slope_=NaN and are
trapped at the assertion below. (My reading of the standard says
NaN==NaN is false, NaN!=NaN is true, so gcc seems to comply.)
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 29 août 2012, at 06:18, Keith OHara k-ohara5...@oco.net wrote: On Tue, 28 Aug 2012 01:00:39 -0700, m...@mikesolomon.org m...@mikesolomon.org wrote: On 28 août 2012, at 06:08, k-ohara5...@oco.net wrote: \override Accidental #'vertical-skylines = #'() ... (you should set them to ly:grob::simple-vertical-skylines-from-stencil instead of '()). You added code to make that substitution for me, so I typed the shorter '() Ah, righto...jolly good. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 28 août 2012, at 06:08, k-ohara5...@oco.net wrote: Still works well, still the same speed, and now the code makes much more sense. The 25% extra time required to set a short score goes down if I \override Accidental #'vertical-skylines = #'() \override TextScript #'vertical-skylines = #'() \override TextScript #'horizontal-skylines = #'() \override Script #'vertical-skylines = #'() ... seemingly in proportion to the number of objects for which I give up detailed skylines. It may be worth it from a UI perspective to come up with a \cruderVerticalSkylineApproximationsForFasterCompilation command (or something of that ilk). I am bad at UI stuff, so I'll let someone else think of a good thing to call this and good vertical-skylines to override (you should set them to ly:grob::simple-vertical-skylines-from-stencil instead of '()). The list you have above looks like a good start - I'd add LyricText to it as well. Janek would be able to come up w/ an exhaustive list. After the simplification, many functions in skyline- and skyline-pair.cc are unused, and thus untested : left(), right(), intersects(), smallest_shift(), is_singleton(), invert(). Got rid of the cruft in staging. Most utility functions are a dime a dozen. Using them intelligently is what's hard. So people can just rewrite these things if they need them. http://codereview.appspot.com/5626052/ Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
m...@mikesolomon.org m...@mikesolomon.org writes: On 28 août 2012, at 06:08, k-ohara5...@oco.net wrote: Still works well, still the same speed, and now the code makes much more sense. The 25% extra time required to set a short score goes down if I \override Accidental #'vertical-skylines = #'() \override TextScript #'vertical-skylines = #'() \override TextScript #'horizontal-skylines = #'() \override Script #'vertical-skylines = #'() ... seemingly in proportion to the number of objects for which I give up detailed skylines. It may be worth it from a UI perspective to come up with a \cruderVerticalSkylineApproximationsForFasterCompilation command (or something of that ilk). I am bad at UI stuff, so I'll let someone else think of a good thing to call this and good vertical-skylines to override (you should set them to ly:grob::simple-vertical-skylines-from-stencil instead of '()). The list you have above looks like a good start - I'd add LyricText to it as well. Janek would be able to come up w/ an exhaustive list. After the simplification, many functions in skyline- and skyline-pair.cc are unused, and thus untested : left(), right(), intersects(), smallest_shift(), is_singleton(), invert(). Got rid of the cruft in staging. Most utility functions are a dime a dozen. Using them intelligently is what's hard. So people can just rewrite these things if they need them. The art is to create one that can do the job of a dozen, in an understandable way. A strictly limited number of versatile building blocks both conceptually simple as well as simple to use and providing completely coverage are what constitutes good APIs, and we should strive to have a set of those available from the Scheme side. People should program a simple C++ function if they need it is not being fair to our high-level users. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Tue, 28 Aug 2012 01:00:39 -0700, m...@mikesolomon.org m...@mikesolomon.org wrote: On 28 août 2012, at 06:08, k-ohara5...@oco.net wrote: \override Accidental #'vertical-skylines = #'() ... (you should set them to ly:grob::simple-vertical-skylines-from-stencil instead of '()). You added code to make that substitution for me, so I typed the shorter '() ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Still works well, still the same speed, and now the code makes much more sense. The 25% extra time required to set a short score goes down if I \override Accidental #'vertical-skylines = #'() \override TextScript #'vertical-skylines = #'() \override TextScript #'horizontal-skylines = #'() \override Script #'vertical-skylines = #'() ... seemingly in proportion to the number of objects for which I give up detailed skylines. After the simplification, many functions in skyline- and skyline-pair.cc are unused, and thus untested : left(), right(), intersects(), smallest_shift(), is_singleton(), invert(). http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/08/17 19:25:29, Keith wrote: On Fri, 17 Aug 2012 10:16:25 -0700, mailto:mts...@gmail.com wrote: http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:780: while (dirty); On 2012/08/17 08:12:56, Keith wrote: 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 awhile. 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] ) I am not adverse to using distance if possible - my trouble case is the following. A B Image that object A above is placed first and LilyPond is now considering whether it should leave B where it is or place it above A. The distance function would dictate that it needs to be shifted above A because its DOWN skyline is below A's UP. We need not let the distance function between A[UP] and B[DOWN] dictate, because having positive distance between A[DOWN] and B[UP] is another solution. Object AA, a member of the forest, sets two constraints floor = padding - AA[UP] ~distance~ BB[DOWN] ceiling = AA[DOWN] ~distance~ BB[UP] - padding and we are checking that a trial location satisfies (trial = floor) OR (trial = ceiling) I suppose you show BB having been placed just above the staff, and that it still lies below the ceiling set by AA, so this trial position satisfies the first test in the OR above, so we win. All these distances can be computed once, before raising the skylines at all, because they tell you right away what trial values of raising BB will clear AA. That would be much simpler for people like me to understand than computing the four intersects() conditions, after having moved the skylines by zero or more bump(s). I've implemented this method in dev/jneem-skylines and it looks pretty neat. Next, padding... http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/08/18 10:12:00, MikeSol wrote: Agreed. I'd love for this to go faster. The goal is not speed, but comprehensibility of code. The code is much shorter with direct use of distance() so there is less for the human reader to hold in his head. There was, however, no measurable difference in speed on my piano-music test case. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/08/21 07:32:15, Keith wrote: There was, however, no measurable difference in speed on my piano-music test case. at least, no speed difference from my attempt at simplification http://codereview.appspot.com/6462087/ I haven't tried Joe's. http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode804 lily/axis-group-interface.cc:804: bool use_separate_constructor_skyline = (horizon_padding != 0) (padding != 0); Should this AND be an OR ? I am guessing you want a separate copy to alter with padding, of either type, the way you put padding in the skylines themselves. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/08/18 10:12:00, MikeSol wrote:
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\override TupletNumber #'font-size = #5
\times 2/3 { a4\trill a\trill^foo a\trill }
}
I added this as a regtest. If you comment out the rider business,
you'll see that foo is printed on top of the tuplet number.
You gave TupletBracket a priority, but not TupletNumber. Wouldn't it
make more sense to have
\override TupletNumber #'outside-staff-priority = #1
as well ?
If I strip out the special-case code
http://codereview.appspot.com/6462087 then all the Grobs honor the
priorities I set for them.
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 21 août 2012, at 03:42, k-ohara5...@oco.net wrote:
On 2012/08/18 10:12:00, MikeSol wrote:
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\override TupletNumber #'font-size = #5
\times 2/3 { a4\trill a\trill^foo a\trill }
}
I added this as a regtest. If you comment out the rider business,
you'll see that foo is printed on top of the tuplet number.
You gave TupletBracket a priority, but not TupletNumber. Wouldn't it
make more sense to have
\override TupletNumber #'outside-staff-priority = #1
as well ?
If I strip out the special-case code
http://codereview.appspot.com/6462087 then all the Grobs honor the
priorities I set for them.
http://codereview.appspot.com/5626052/
It would make more sense, but I'm trying to cover all scenarios. In the
regtest above, I think that foo should never collide w/ the tuplet number
because of outside-staff-priority being set for either TupletBracket or
TupletNumber.
Cheers,
MS
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/08/21 07:42:38, Keith wrote:
On 2012/08/18 10:12:00, MikeSol wrote:
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\override TupletNumber #'font-size = #5
\times 2/3 { a4\trill a\trill^foo a\trill }
}
I added this as a regtest. If you comment out the rider business,
you'll see that foo is printed on top of the tuplet number.
You gave TupletBracket a priority, but not TupletNumber. Wouldn't it
make more
sense to have
\override TupletNumber #'outside-staff-priority = #1
as well ?
If I strip out the special-case code
http://codereview.appspot.com/6462087
then all the Grobs honor the priorities I set for them.
Have you tried setting TupletNumber's priority to be smaller than
TupletBracket? It results TupletNumber moving twice as far as it should
(making it collide with foo), because first it translates itself and
then it gets moved again because TupletBracket moves, and its position
is measured relative to TupletBracket.
It would also be possible for some grob to insert itself between the
TupletBracket and the TupletNumber, which isn't particularly nice
either. So I think some form of rider functionality is necessary. I do
it a little differently in my branch, which even allows the TupletNumber
to affect the position of the TupletBracket.
Cheers,
Joe
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Works nice.
But so far the density of problems in the code as been pretty high.
These problems seem to be in corner cases for which the code is
(unnecessarily?) complex. It is not clear why the _use_ of skylines has
to change so much at the same time as the _shapes_ of skylines change.
Maybe Joe or someone will pick out the good parts. Otherwise, if we push
it to master we can probably fix any further problems as they are
discovered. My guess is there will be fewer problems from this than
there were from 'pure-from-neighbor', or the MIDI changes just before
2.14.
On 2012/08/21 09:43:51, mike7 wrote:
On 21 août 2012, at 03:42, mailto:k-ohara5...@oco.net wrote:
On 2012/08/18 10:12:00, MikeSol wrote:
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\override TupletNumber #'font-size = #5
\times 2/3 { a4\trill a\trill^foo a\trill }
}
Wouldn't it make more sense to have
\override TupletNumber #'outside-staff-priority = #1
as well ?
It would make more sense, but I'm trying to cover all scenarios.
In the regtest above, I think that foo should never collide
w/ the tuplet number because of outside-staff-priority being
set for either TupletBracket or TupletNumber.
You are entitled to your opinion, but if you leave out the foo the
TupletNumber collides with the trill. I think I should be able to do
something about that. If I do the sensible thing and give the
TupletNumber the same outside-staff-priority as the bracket, your new
code says can't have that and continues to collide them.
Maybe we can compromise and give a priority-less child the priority of
his parent ... and of course drop all the book-keeping about 'riders'.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode980
lily/axis-group-interface.cc:980: vertical_skyline_forest[d].push_back
(doctored_skylines);
This selects only the outer skyline of the on-staff objects, so if you
get Russian-egg-doll packing you won't notice.
The tucking of dynamics inside slurs in 'slur-dynamics.ly' looks good,
but with a slight change we see it is really a bug :
\relative c' { b( d\( f\)\p b,) g( d'\(\ d\)\! g,) }
We can get dynamics to tuck under slurs safely, if we
\override Slur #'outside-staff-priority = #1
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Tue, 21 Aug 2012 20:43:03 -0700, joenee...@gmail.com wrote: Have you tried setting TupletNumber's priority to be smaller than TupletBracket? It results TupletNumber moving twice as far as it should That is a problem with the simple code. What is the desired output, though, if someone specifically asks for TupletNumbers to be placed before TupletBrackets ? It would also be possible for some grob to insert itself between the TupletBracket and the TupletNumber, which isn't particularly nice either. But if a user over-rode the priorities so that another object has priority between that of the Bracket and the Number, maybe he did think that would be nice. So I think some form of rider functionality is necessary. I do it a little differently in my branch, which even allows the TupletNumber to affect the position of the TupletBracket. That looks reasonably simple. And allowing space for the 'rider' while placing the parent is the right thing to do. (I had /thought/ setting the child's outside-staff-priority to #f would push outside-staff items away from where the child was, before getting moved with the parent. So I suggested just inheriting the priority of the parent.) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Thanks for the performance tests! I have no problem changing the
function avoid_outside_staff_collisions to be faster - it's just that I
haven't wrapped my head around how distance alone can do it.
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#newcode945
lily/axis-group-interface.cc:945: feature present in
tuplet-number-outside-staff-priority.
On 2012/08/18 02:42:37, Keith wrote:
On 2012/08/17 17:16:25, MikeSol wrote:
On 2012/08/17 08:12:56, Keith wrote:
If you comment out the rider business, the vertical-skylines will
not be
correct
for axis-group-interface.
That is very subtle, very minor, changes only the debug output, not
what would
normally be printed from that example, and is different from the
comment
indicates.
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\override TupletNumber #'font-size = #5
\times 2/3 { a4\trill a\trill^foo a\trill }
}
I added this as a regtest. If you comment out the rider business,
you'll see that foo is printed on top of the tuplet number.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode697
lily/axis-group-interface.cc:697: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance ((*pair)[-d]));
On 2012/08/18 02:42:37, Keith wrote:
When d = dir, the direction we intend to move, this tells us how far
we need to
move, but when d = -dir, this tells us by how far we have encroached
into the
next obstacle, which we will eventually need to hop over.
What you really want to put on the bumps list is how far we need to
move to hope
over this next obstacle. But we only tested the relevant skylines for
'intersection' ... if only we had stached them distance().
I'm not sure what you mean when you say we only tested the relevant
skylines for intersection ... if only we had stached them distance() -
one because I don't know what stached means in this context (from urban
dictionary: The art of placing a ficticious mustache on an individual or
oneself, drawing laughter and humor to this occurance.) and two because
every intersection call is followed by a distance call, so there is
bumping going on in this case.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode701
lily/axis-group-interface.cc:701: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance (to_flip));
On 2012/08/18 02:42:37, Keith wrote:
This finds the distance to move such that the top skyline of the new
object just
encloses, Russian egg doll style, one of the already-placed skylines.
We never
want that to be the final position, so don't put it on the bumps list.
That is a possible scenario, but Russian egg doll style enclosing is
tested for below, so even if this does happen it'd be rectified on the
next pass.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode732
lily/axis-group-interface.cc:732: pair-raise (min_bump);
On 2012/08/18 02:42:37, Keith wrote:
printf(bumping %.3f\n, min_bump);
shows that for a simple case
\relative f {f'^mouse c'' f,,^K d''}
we bump around quite a lot:
mouse
bumping 0.567
bumping 1.894
EEEK
bumping 0.443
bumping 0.048
bumping 0.095
bumping 0.191
bumping 0.382
bumping 0.459
bumping 1.344
bumping 0.161
bumping 0.322
bumping 0.578
Agreed. I'd love for this to go faster.
I am for getting this into current master ASAP so that people can work
on speeding it up - I'm not at all adverse to the idea of using distance
the way you're talking about, but I don't completely understand how it'd
work yet and it seems like something that could be done in a separate
commit. What'd be nice about having this in master is that it'd be
easier to spot in the regtests if efficiency changes to the algorithm
have a layout impact. Of course, one could run regtests with this patch
as a baseline, but it is a pain.
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
patch
real17m55.095s
user17m10.848s
sys 0m11.381s
git master
real16m16.228s
user15m30.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/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
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#newcode651 lily/axis-group-interface.cc:651: ---/ I've experimented with disabling the horizontal skylines, and it didn't actually make any difference to the regtests. Could you add something that exercises this? http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode658 lily/axis-group-interface.cc:658: are avoided Have you measured the costs of not having exempt? http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 17 août 2012, at 07:19, joenee...@gmail.com wrote:http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.ccFile lily/axis-group-interface.cc (right):http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode651lily/axis-group-interface.cc:651: ---/I've experimented with disabling the horizontal skylines, and it didn'tactually make any difference to the regtests. Could you add somethingthat exercises this?I could - it'll take me a bit of time to cook it up. Essentially the horizontal stuff exists to differentiate between the two images in the attached file. The top one shouldn't shifted whereas the bottom should and the only way to know is via the horizontal skylines.http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode658lily/axis-group-interface.cc:658: are avoidedHave you measured the costs of not having exempt?Not yet - I'm sure there is a minimal speedup for certain scores, but we can certainly delete it down the line if people think it's more trouble than it's worth.http://codereview.appspot.com/5626052/___lilypond-devel mailing listlilypond-devel@gnu.orghttps://lists.gnu.org/mailman/listinfo/lilypond-devel___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 17 août 2012, at 10:21, m...@mikesolomon.org m...@mikesolomon.org wrote:
On 17 août 2012, at 07:19, joenee...@gmail.com wrote:
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#newcode651
lily/axis-group-interface.cc:651: ---/
I've experimented with disabling the horizontal skylines, and it didn't
actually make any difference to the regtests. Could you add something
that exercises this?
I could - it'll take me a bit of time to cook it up. Essentially the
horizontal stuff exists to differentiate between the two images in the
attached file. The top one shouldn't shifted whereas the bottom should and
the only way to know is via the horizontal skylines.
http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode658
lily/axis-group-interface.cc:658: are avoided
Have you measured the costs of not having exempt?
Not yet - I'm sure there is a minimal speedup for certain scores, but we can
certainly delete it down the line if people think it's more trouble than it's
worth.
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
horizontalcollision.png
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I cooked up a quick test and it's not giving me the results I want - the
algorithm shouldn't shift the second image above the first but it does. I'll
look into it...
\version 2.15.30
foo = \markup \translate #'(0 . 6)
\path #0.1 #'((moveto 0 0)
(lineto 0 6)
(lineto 8 6)
(lineto 8 5)
(lineto 4 5)
(lineto 4 1)
(lineto 8 1)
(lineto 8 0)
(lineto 0 0))
bar = \markup \translate #'(6 . 0)
\path #0.1 #'((moveto 0 0)
(lineto 0 10)
(lineto -8 10)
(lineto -8 9)
(lineto -1 9)
(lineto -1 4)
(lineto -3 4)
(lineto -3 3)
(lineto -1 3)
(lineto -1 0)
(lineto 0 0))
\relative c'' {
a^\foo
a^\bar
}___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
I've forgotten why I added the horizontal skyline stuff so I've taken it
out in a new version that I'll post after regtests.
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#newcode658
lily/axis-group-interface.cc:658: are avoided
On 2012/08/17 11:19:26, joeneeman wrote:
Have you measured the costs of not having exempt?
I've decided to scrap exempt in a new version - will upload soon.
http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780
lily/axis-group-interface.cc:780: while (dirty);
On 2012/08/17 08:12:56, Keith wrote:
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.
Hey Keith,
I am not adverse to using distance if possible - my trouble case is the
following.
A
B
Image that object A above is placed first and LilyPond is now
considering whether it should leave B where it is or place it above A.
The distance function would dictate that it needs to be shifted above A
because its DOWN skyline is below A's UP. However, it never intersects,
so there's no need to shift it. This is why I'm looking for
intersections. Maybe I'm thinking of it incorrectly, though?
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.
On 2012/08/17 08:12:56, Keith wrote:
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 ?
\version 2.17.0
#(ly:set-option 'debug-skylines #t)
\relative c'' {
\override TupletBracket #'outside-staff-priority = #1
\times 2/3 { a4\trill a\trill a\trill } }
If you comment out the rider business, the vertical-skylines will not be
correct for axis-group-interface. Comment it back in and they will.
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Fri, 17 Aug 2012 10:16:25 -0700, mts...@gmail.com wrote: http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:780: while (dirty); On 2012/08/17 08:12:56, Keith wrote: 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 awhile. 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] ) I am not adverse to using distance if possible - my trouble case is the following. A B Image that object A above is placed first and LilyPond is now considering whether it should leave B where it is or place it above A. The distance function would dictate that it needs to be shifted above A because its DOWN skyline is below A's UP. We need not let the distance function between A[UP] and B[DOWN] dictate, because having positive distance between A[DOWN] and B[UP] is another solution. Object AA, a member of the forest, sets two constraints floor = padding - AA[UP] ~distance~ BB[DOWN] ceiling = AA[DOWN] ~distance~ BB[UP] - padding and we are checking that a trial location satisfies (trial = floor) OR (trial = ceiling) I suppose you show BB having been placed just above the staff, and that it still lies below the ceiling set by AA, so this trial position satisfies the first test in the OR above, so we win. All these distances can be computed once, before raising the skylines at all, because they tell you right away what trial values of raising BB will clear AA. That would be much simpler for people like me to understand than computing the four intersects() conditions, after having moved the skylines by zero or more bump(s). On the question of where to store padding, simple padding was previously added by the code using distance(). That seems best because it is a simple addition and different applications might have different padding to the same objects. Then you don't have to worry about adding padding multiple times to a skyline, deciding which of two skylines to pad, etc. The other thing is 'horizon-padding' which fattens the buildings in the skyline themselves, and has in the past been implemented by reshaping the Skyline object. The user-variables skyline-(vertical|horizontal)-padding apply so far to symmetric situations, and the code simply horizon-pads both skylines so you get the effect of twice the number you enter. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Sat, Aug 18, 2012 at 5:34 AM, Keith OHara k-ohara5...@oco.net wrote: On Fri, 17 Aug 2012 10:16:25 -0700, mts...@gmail.com wrote: http://codereview.appspot.com/**5626052/diff/106004/lily/axis-** group-interface.cc#newcode780http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:**780: while (dirty); On 2012/08/17 08:12:56, Keith wrote: 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 awhile. 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] ) I am not adverse to using distance if possible - my trouble case is the following. A B Image that object A above is placed first and LilyPond is now considering whether it should leave B where it is or place it above A. The distance function would dictate that it needs to be shifted above A because its DOWN skyline is below A's UP. We need not let the distance function between A[UP] and B[DOWN] dictate, because having positive distance between A[DOWN] and B[UP] is another solution. If you check out the dev/jneem-skylines (which is a simplified but not (yet) feature-compatible version of Mike's branch), you'll see that I'm using this method under the name 'bool Skyline_pair::intersects.' Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 17 août 2012, at 16:57, Joe Neeman joenee...@gmail.com wrote: On Sat, Aug 18, 2012 at 5:34 AM, Keith OHara k-ohara5...@oco.net wrote: On Fri, 17 Aug 2012 10:16:25 -0700, mts...@gmail.com wrote: http://codereview.appspot.com/5626052/diff/106004/lily/axis-group-interface.cc#newcode780 lily/axis-group-interface.cc:780: while (dirty); On 2012/08/17 08:12:56, Keith wrote: 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 awhile. 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] ) I am not adverse to using distance if possible - my trouble case is the following. A B Image that object A above is placed first and LilyPond is now considering whether it should leave B where it is or place it above A. The distance function would dictate that it needs to be shifted above A because its DOWN skyline is below A's UP. We need not let the distance function between A[UP] and B[DOWN] dictate, because having positive distance between A[DOWN] and B[UP] is another solution. If you check out the dev/jneem-skylines (which is a simplified but not (yet) feature-compatible version of Mike's branch), you'll see that I'm using this method under the name 'bool Skyline_pair::intersects.' Cheers, Joe This is great, Joe. If you can get it to a point where it is feature compatible such that it eliminates the calls to both intersection and distance, lemme know. I'll have time to work on it together in the AM for the next few days. I am starting to understand how this solution works but I don't think I could write it from scratch w/o doing more thinking so if you have something almost functional I'd rather go from that. Cheers, MS___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Joe Neeman joeneeman at gmail.com writes: We need not let the distance function between A[UP] and B[DOWN] dictate, because having positive distance between A[DOWN] and B[UP] is another solution. If you check out the dev/jneem-skylines (which is a simplified but not (yet) feature-compatible version of Mike's branch), you'll see that I'm using this method under the name 'bool Skyline_pair::intersects.' I see. That makes intersects() mean simply do the outlined regions overlap? which is helpful concept. The function intersects(), however, is not a very good tool for Mike's job. It tells us we need to move an object. How far? Ask distance(). Did we run into anything else? Ask intersects(). How far do we need to move now? Ask distance(). The Grobs bounce around like the ball in a pinball machine. I'm away this weekend, but will eventually convince Mike to simply build a list of distances where the Grob to be placed starts to overlap an existing Grob, and and a list of distances where it stops ovelapping, and search those distances for the first satisfactory home for the Grob. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
This is barely slower than master on an orchestral score and parts
patch master
real16m16.228sreal 15m54.212s
user15m30.543suser 15m5.920s
sys 0m10.624s sys 0m10.649s
but piano music (Chopin op45) takes 25% longer
real0m16.245s real 0m12.862s
user0m15.573s user 0m12.232s
sys 0m0.254s sys 0m0.217s
patch master
real16m16.228sreal 15m54.212s
user15m30.543suser 15m5.920s
sys 0m10.624s sys 0m10.649s
real0m16.245s real 0m12.862s
user0m15.573s user 0m12.232s
sys 0m0.254s sys 0m0.217s
We should clean up the code, most importantly because having operations
that do not progress toward the goal is quite confusing, and
time-consuming, for any code maintainer.
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#newcode945
lily/axis-group-interface.cc:945: feature present in
tuplet-number-outside-staff-priority.
On 2012/08/17 17:16:25, MikeSol wrote:
On 2012/08/17 08:12:56, Keith wrote:
If you comment out the rider business, the vertical-skylines will not
be correct
for axis-group-interface.
That is very subtle, very minor, changes only the debug output, not what
would normally be printed from that example, and is different from the
comment indicates.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode697
lily/axis-group-interface.cc:697: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance ((*pair)[-d]));
When d = dir, the direction we intend to move, this tells us how far we
need to move, but when d = -dir, this tells us by how far we have
encroached into the next obstacle, which we will eventually need to hop
over.
What you really want to put on the bumps list is how far we need to move
to hope over this next obstacle. But we only tested the relevant
skylines for 'intersection' ... if only we had stached them distance().
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode701
lily/axis-group-interface.cc:701: bumps.push_back (dir *
vertical_skyline_forest[dir][j][d].distance (to_flip));
This finds the distance to move such that the top skyline of the new
object just encloses, Russian egg doll style, one of the already-placed
skylines. We never want that to be the final position, so don't put it
on the bumps list.
http://codereview.appspot.com/5626052/diff/104124/lily/axis-group-interface.cc#newcode732
lily/axis-group-interface.cc:732: pair-raise (min_bump);
printf(bumping %.3f\n, min_bump);
shows that for a simple case
\relative f {f'^mouse c'' f,,^K d''}
we bump around quite a lot:
mouse
bumping 0.567
bumping 1.894
EEEK
bumping 0.443
bumping 0.048
bumping 0.095
bumping 0.191
bumping 0.382
bumping 0.459
bumping 1.344
bumping 0.161
bumping 0.322
bumping 0.578
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode778 lily/axis-group-interface.cc:778: Note we only use 75% of padding and apply the full compliment only if On 2012/08/14 04:21:33, Keith wrote: It looks like you add 50% padding here, then pass to avoid_outside_staff_collision() which removes 37.5% of padding away. Probably not what you meant to do. The verbs 'use' and 'apply' are vague enough that the comment didn't help me see what you meant to do. Sorry, I see the problem now (I missed the 0.75). I brought back the logic of the comment above it looks like it obviates the need for this whole 50%-of-padding thing. Running regtests now. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Thanks for the review!
http://codereview.appspot.com/5626052/diff/105001/input/regression/text-script-vertical-skylines.ly
File input/regression/text-script-vertical-skylines.ly (right):
http://codereview.appspot.com/5626052/diff/105001/input/regression/text-script-vertical-skylines.ly#newcode5
input/regression/text-script-vertical-skylines.ly:5: kerning.
On 2012/08/14 04:21:33, Keith wrote:
It is not obvious we want this for TextScripts,
{b'^Élever b'^brusquement}
so don't enshrine the requirement in a regtest.
Done.
http://codereview.appspot.com/5626052/diff/105001/input/regression/volta-bracket-vertical-skylines.ly
File input/regression/volta-bracket-vertical-skylines.ly (right):
http://codereview.appspot.com/5626052/diff/105001/input/regression/volta-bracket-vertical-skylines.ly#newcode4
input/regression/volta-bracket-vertical-skylines.ly:4: texidoc = Volta
brackets are vertically kerned to objects below them.
On 2012/08/14 04:21:33, Keith wrote:
Good, but fit not kerned
To me, 'to kern' includes making many aesthetic judgments to come up
with
individual adjustments for each fitting pair.
Done.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode629
lily/axis-group-interface.cc:629: Three rounds:
On 2012/08/14 04:21:33, Keith wrote:
I don't understand the comment, but what I needed to figure out was :
Given pre-padded vertical skylines 'pair' for an outside-staff Grob,
this
figures out the vertical position of the Grob. It raises the skylines
in 'pair'
and shifts (along the skyline) 'horizontal_skylines' by the same
amount.
Using horizontal_skylines (with the buildings pointing horizontally)
to
determine vertical position, is a new concept worth mentioning.
I'm still figuring out 'exempt', '*_forest', etc.
Better comment added.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode658
lily/axis-group-interface.cc:658: check for horizontal edges jinnying up
On 2012/08/14 04:21:33, Keith wrote:
I learned the local dialect 200 miles north, so I don't know what this
means. Is
this the case where the edges lie side a by each, or kitty corner ?
Elaine Gould uses the phrase jinnying up in behind bars, and given
that we are using that as one of our base texts, I'm ok using the phrase
here.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode728
lily/axis-group-interface.cc:728: We need to take the padding away now
that it has been shifted. Otherwise
On 2012/08/14 04:21:33, Keith wrote:
I can't find where the padding was ever added.
The call to grow in add_grobs_of_one_priority.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode751
lily/axis-group-interface.cc:751: edge of the just-placed grob).
Otherwise, we skip it until the next pass.
On 2012/08/14 04:21:33, Keith wrote:
This comment describes the old way of solving the problem you saw with
'midi-dynamics.pdf' but you removed the old code. Maybe you want to
use the old
method, but make code and comment agree one way or another.
Excellent observation - I'd completely forgotten this. Perhaps this
takes care of the midi-dynamics.ly problem altogether...will
investigate.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode778
lily/axis-group-interface.cc:778: Note we only use 75% of padding and
apply the full compliment only if
On 2012/08/14 04:21:33, Keith wrote:
It looks like you add 50% padding here, then pass to
avoid_outside_staff_collision() which removes 37.5% of padding away.
Probably
not what you meant to do.
The verbs 'use' and 'apply' are vague enough that the comment didn't
help me see
what you meant to do.
I think that both remove 50%, no?
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode843
lily/axis-group-interface.cc:843: Three rounds:
On 2012/08/14 04:21:33, Keith wrote:
Does this part of the comment apply to the code below ?
The comment's blech...I'll redo it in a later patchset (remind me if I
don't).
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode890
lily/axis-group-interface.cc:890: feature present in one regrest :(
On 2012/08/14 04:21:33, Keith wrote:
You tease us. Which regression test ?
Done.
http://codereview.appspot.com/5626052/diff/105001/lily/beam.cc
File lily/beam.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/beam.cc#newcode1368
lily/beam.cc:1368: shift += 0.01;
On 2012/08/14 04:21:33, Keith wrote:
* beamdir ?
Done.
http://codereview.appspot.com/5626052/diff/105001/lily/skyline-pair.cc
File lily/skyline-pair.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/skyline-pair.cc#newcode103
lily/skyline-pair.cc:103: // other[UP] then we will not intersect.
On
Re: Gets vertical skylines from grob stencils (issue 5626052)
Worked nicely on some piano music and a Dvorak symphony.
The code is bewildering. So far I've mostly read the comments.
The bit with the activation-factor is pretty ugly.
What was the size of the problem ? Did the script that should have fit
lack space of padding, 0.5*padding, or epsilon ?
http://codereview.appspot.com/5626052/diff/105001/input/regression/text-script-vertical-skylines.ly
File input/regression/text-script-vertical-skylines.ly (right):
http://codereview.appspot.com/5626052/diff/105001/input/regression/text-script-vertical-skylines.ly#newcode5
input/regression/text-script-vertical-skylines.ly:5: kerning.
It is not obvious we want this for TextScripts,
{b'^Élever b'^brusquement}
so don't enshrine the requirement in a regtest.
http://codereview.appspot.com/5626052/diff/105001/input/regression/volta-bracket-vertical-skylines.ly
File input/regression/volta-bracket-vertical-skylines.ly (right):
http://codereview.appspot.com/5626052/diff/105001/input/regression/volta-bracket-vertical-skylines.ly#newcode4
input/regression/volta-bracket-vertical-skylines.ly:4: texidoc = Volta
brackets are vertically kerned to objects below them.
Good, but fit not kerned
To me, 'to kern' includes making many aesthetic judgments to come up
with individual adjustments for each fitting pair.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc
File lily/axis-group-interface.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode629
lily/axis-group-interface.cc:629: Three rounds:
I don't understand the comment, but what I needed to figure out was :
Given pre-padded vertical skylines 'pair' for an outside-staff Grob,
this figures out the vertical position of the Grob. It raises the
skylines in 'pair' and shifts (along the skyline) 'horizontal_skylines'
by the same amount.
Using horizontal_skylines (with the buildings pointing horizontally) to
determine vertical position, is a new concept worth mentioning.
I'm still figuring out 'exempt', '*_forest', etc.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode658
lily/axis-group-interface.cc:658: check for horizontal edges jinnying up
I learned the local dialect 200 miles north, so I don't know what this
means. Is this the case where the edges lie side a by each, or kitty
corner ?
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode728
lily/axis-group-interface.cc:728: We need to take the padding away now
that it has been shifted. Otherwise
I can't find where the padding was ever added.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode751
lily/axis-group-interface.cc:751: edge of the just-placed grob).
Otherwise, we skip it until the next pass.
This comment describes the old way of solving the problem you saw with
'midi-dynamics.pdf' but you removed the old code. Maybe you want to use
the old method, but make code and comment agree one way or another.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode778
lily/axis-group-interface.cc:778: Note we only use 75% of padding and
apply the full compliment only if
It looks like you add 50% padding here, then pass to
avoid_outside_staff_collision() which removes 37.5% of padding away.
Probably not what you meant to do.
The verbs 'use' and 'apply' are vague enough that the comment didn't
help me see what you meant to do.
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode843
lily/axis-group-interface.cc:843: Three rounds:
Does this part of the comment apply to the code below ?
http://codereview.appspot.com/5626052/diff/105001/lily/axis-group-interface.cc#newcode890
lily/axis-group-interface.cc:890: feature present in one regrest :(
You tease us. Which regression test ?
http://codereview.appspot.com/5626052/diff/105001/lily/beam.cc
File lily/beam.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/beam.cc#newcode1368
lily/beam.cc:1368: shift += 0.01;
* beamdir ?
http://codereview.appspot.com/5626052/diff/105001/lily/skyline-pair.cc
File lily/skyline-pair.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/skyline-pair.cc#newcode103
lily/skyline-pair.cc:103: // other[UP] then we will not intersect.
Worth mentioning that the shift direction d is LEFT RIGHT for vertical
skylines, all axes reversed for horizontal.
http://codereview.appspot.com/5626052/diff/105001/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/105001/lily/skyline.cc#newcode157
lily/skyline.cc:157: Real ret = (y - y_intercept_ - slope_ * x) /
slope_;
(y - height(x))
http://codereview.appspot.com/5626052/diff/105001/lily/skyline.cc#newcode287
lily/skyline.cc:287: // Since a skyline should always be normalized, we
can
I think you mean this function requires the skyline to be normalized,
so that ...
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Sat, Jul 14, 2012 at 8:33 PM, mts...@gmail.com wrote: http://codereview.appspot.com/**5626052/diff/85004/lily/** skyline.cc#newcode302http://codereview.appspot.com/5626052/diff/85004/lily/skyline.cc#newcode302 lily/skyline.cc:302: while (dirty); On 2012/07/14 14:01:08, joeneeman wrote: I don't understand this do while (dirty) part. It seems to me that unless you have two empty segments in a row, you only need one pass. On the other hand, if you have two empty segments in a row then this function will fail anyway because you will end up setting center-slope_ and center-y_intercept_ both to -infinity, which is bound to start creating NaN sooner or later. I'll rewrite this so that it combines all interior skylines with an infinite y intercept and then only does one pass. This sounds like a good idea. If you put the skyline combination part in a separate function, we could just use it after merges, etc, so that the property of not having adjacent empty buildings is always maintained. http://codereview.appspot.com/**5626052/diff/90001/lily/** include/skyline.hhhttp://codereview.appspot.com/5626052/diff/90001/lily/include/skyline.hh File lily/include/skyline.hh (right): http://codereview.appspot.com/**5626052/diff/90001/lily/** include/skyline.hh#newcode58http://codereview.appspot.com/5626052/diff/90001/lily/include/skyline.hh#newcode58 lily/include/skyline.hh:58: NOT_ENOUGH_INFO On 2012/07/14 14:01:08, joeneeman wrote: I wish I could convince you to think of a skyline as a region instead of just the boundary of that region. Once you think of it that way, it becomes clear that this information can be easily obtained from the Skyline_pair, using the existing distance function. Suppose s and t are Skyline_pairs. max(s[UP].distance(t[DOWN]), s[DOWN].distance(t[UP])) = -infinity means the objects don't overlap horizontally at all, so it's meaningless to talk about which one is higher (I think this is what you're calling NOT_ENOUGH_INFO). min(s[UP].distance(t[DOWN]), s[DOWN].distance(t[UP])) 0 means that the objects intersect. s[UP].distance(t[DOWN]) = 0 and s[DOWN].distance(t[UP]) 0 means that s is below t t[UP].distance(s[DOWN]) = 0 and t[DOWN].distance(s[UP]) 0 means that t is below s This logic uses two calls to distance, which is expensive, whereas using the e-num gets this info from one call to distance. I can use the method you propose, but don't you think that'd cause a performance hit? Won't you need two skyline comparisons anyway? If T's up skyline is always above S's down skyline, then S and T may intersect but you won't know until you've compared T's down skyline with S's up skyline. Anyway, I'm always in favor of writing simple code first and then optimizing once things have been measured. Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
I actually only wanted to get rid of the whitespace errors plagueing every test of this patch, but one programming error jumped out so badly at me that I had to flag it as well. http://codereview.appspot.com/5626052/diff/79059/lily/axis-group-interface.cc File lily/axis-group-interface.cc (right): http://codereview.appspot.com/5626052/diff/79059/lily/axis-group-interface.cc#newcode726 lily/axis-group-interface.cc:726: Whitespace in empty line. http://codereview.appspot.com/5626052/diff/79059/lily/axis-group-interface.cc#newcode767 lily/axis-group-interface.cc:767: Whitespace in empty line. http://codereview.appspot.com/5626052/diff/79059/lily/axis-group-interface.cc#newcode798 lily/axis-group-interface.cc:798: Whitespace in empty line. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode281 lily/skyline.cc:281: bool dirty = false; Trailing whitespace at end of line, but actually the whole line seems like it should be deleted, or otherwise the loop will always execute exactly once since the variable `dirty' that is being set in the loop is not the same as the variable `dirty' that is checked at its end. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode583 lily/skyline.cc:583: Whitespace in empty line. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode710 lily/skyline.cc:710: end = i-start_; Trailing whitespace. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode711 lily/skyline.cc:711: pad_buildings.push_back (Building (start, height, height, end)); Trailing whitespace. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode728 lily/skyline.cc:728: pad_buildings.push_back (Building (start, height, height - horizon_padding, end)); Trailing whitespace. http://codereview.appspot.com/5626052/diff/79059/lily/skyline.cc#newcode834 lily/skyline.cc:834: return NOT_ENOUGH_INFO; Trailing whitespace. http://codereview.appspot.com/5626052/diff/79059/lily/system.cc File lily/system.cc (right): http://codereview.appspot.com/5626052/diff/79059/lily/system.cc#newcode426 lily/system.cc:426: return grobs_scm; Trailing whitespace. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 4 juil. 2012, at 21:58, Janek Warchoł wrote: On Wed, Jul 4, 2012 at 3:36 PM, m...@apollinemike.com m...@apollinemike.com wrote: No idea when i'll have time to finish Tie Report, but as tie shapes are screwed in general, i wouldn't care much about this change. New shape isn't worse than previous one, i'd say. I think that there's supposed to be a bit of space in there, no? What does Gould say? Definitely, current output is wrong. But as i said, i'm not sure if it's worth bothering as ties are screwed in general. What's important is to figure out why this change is happening. As ties only ever use Skylines made from boxes (and not from stencil integrals), there must be some weird subtlety about spacing somewhere (or perhaps the new Skyline code) that is causing this. Indeed, whatever the output is it shouldn't change because of new skylines. Hmm. I have another idea, which would be more difficult but also extremely awesome, and it would be very handy for area spacing (see http://lists.gnu.org/archive/html/lilypond-devel/2012-03/msg00507.html): make the skyline shape magnetic. See illustration here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/magnetic_skylines_demo.pdf What do you think? It wouldn't be too bad from a coding perspective. The function deholify already does this for single skylines: try the skyline of a large font-size string f _ f or something like that and the holes should be plugged with a sloped building. When there are multiple grobs involved, what becomes tricky is knowing which ones should allow for snug-fitting and which ones not. I think that, as time goes on, we'll realize where the snug spacing is too close and will modify it w/ padding values. Do you mean that padding should influence skylines' magnetism? Something like this: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705597/magnetic_skylines_-_padding.pdf (i.e. there would be only one setting, which would affect both the padding and magnetism of the skylines)? I think it might be a good idea. cheers, Janek Hey all, New patch set up that fixes all of the problems Janek pointed out and the major problem Keith pointed out. If any of the minor ones still exist in any of your scores, lemme know. Minimal examples help! Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Jul 5, 2012 at 5:23 PM, m...@apollinemike.com
m...@apollinemike.com wrote:
New patch set up that fixes all of the problems Janek pointed out
I confirm, but the fun doesn't end yet :P
\relative f' { c4( d e f) g1 }
\addlyrics { bl lah }
\relative f'' { c4( b a g) f1 }
\addlyrics { bl lah }
\relative f'' { c8( a ) }
\addlyrics { blaa, }
\relative f'' { c8([ a )] }
\addlyrics { blaa, }
-
http://lilypond-stuff.1065243.n5.nabble.com/file/n5705598/weird-melismas_skylines.pdf
LOL
hth,
Janek
(and script-stack-order.ly is still wrong)
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mon, Jul 2, 2012 at 1:03 AM, k-ohara5...@oco.net wrote: I did a quick test on some music. http://k-ohara.oco.net/Lilypond/ Ledger lines might not be getting the space they need. In rare cases, all the systems were overlaid on the top of the page. Several objects might need an increase in their default padding. At some moment my scores experienced similar effect. I think it was around patchset 35 (36 was fine). hth, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 3 juil. 2012, at 10:18, Janek Warchoł wrote: Hi all, my favorite patch returned! Yay! I did some testing and the results are here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705594/patchset37-results.tar.gz Thanks Janek! Very valuable stuff. (sorry, too big for an attachment - 200 kB) In short: - staffswitch lines are broken This is because VoiceFollower is not set as (cross-staff . #t) in define-grobs.scm. I'm not sure why...this seems like a good idea irrespective of how the skyline work pans out. - barnumbers collide with ties This came from a rare problem that arose from the tie threading through the bar number. Fixed. - some ties change shape This will take me some time to figure out...I'm gonna have to look at all the parameters going into tie scoring and see which ones are changing. I'll likely push something to staging the weekend or next week in the style of Tie_formatting_problem::spew () that gives tons of specs and then run the output through python to pick up on changes. Of course, if any tie gurus have intuition about this, lemme know! - some things move too close to each other, as if padding was nixed Should be fixed. - some skylines are really weird This is likely a directioning problem - all upwards pointing skylines seem to come out just fine. The culprit would most likely be in stencil-integral.cc: the notion of UP must be hardcoded somewhere that it shouldn't be. - sometimes objects interlock too much. This was the case in the lyric example you sent. I fixed it by adding light skyline-horizontal-padding to lyrics. hope this helps! Janek I won't post a new patchset until all of the problems are fixed, but know that most of them are. I very much appreciate the research! Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Wed, Jul 4, 2012 at 11:31 AM, m...@apollinemike.com m...@apollinemike.com wrote: Thanks Janek! Very valuable stuff. :) (sorry, too big for an attachment - 200 kB) In short: - staffswitch lines are broken This is because VoiceFollower is not set as (cross-staff . #t) in define-grobs.scm. I'm not sure why...this seems like a good idea irrespective of how the skyline work pans out. I find it even more surprising that Glissandos work perfectly. Aren't they related to VoiceFollowers? - some ties change shape This will take me some time to figure out...I'm gonna have to look at all the parameters going into tie scoring and see which ones are changing. I'll likely push something to staging the weekend or next week in the style of Tie_formatting_problem::spew () that gives tons of specs and then run the output through python to pick up on changes. Of course, if any tie gurus have intuition about this, lemme know! No idea when i'll have time to finish Tie Report, but as tie shapes are screwed in general, i wouldn't care much about this change. New shape isn't worse than previous one, i'd say. - some skylines are really weird This is likely a directioning problem - all upwards pointing skylines seem to come out just fine. The culprit would most likely be in stencil-integral.cc: the notion of UP must be hardcoded somewhere that it shouldn't be. If you have a moment to test out weird-skylines.ly on different versions of the patchset, I'd appreciate it! Tenuto skyline seems to be wrong since the beginning - i've tested patchsets 36, 34, 32 and 30, it looked similar everywhere. Sharp and forte skylines seemed ok in patchset 36, see here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/weird-skylines_patchset-36.pdf http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/weird-skylines_patchset-37.pdf - sometimes objects interlock too much. This was the case in the lyric example you sent. I fixed it by adding light skyline-horizontal-padding to lyrics. Hmm. I have another idea, which would be more difficult but also extremely awesome, and it would be very handy for area spacing (see http://lists.gnu.org/archive/html/lilypond-devel/2012-03/msg00507.html): make the skyline shape magnetic. See illustration here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/magnetic_skylines_demo.pdf What do you think? cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 4 juil. 2012, at 15:17, Janek Warchoł wrote: On Wed, Jul 4, 2012 at 11:31 AM, m...@apollinemike.com m...@apollinemike.com wrote: Thanks Janek! Very valuable stuff. :) (sorry, too big for an attachment - 200 kB) In short: - staffswitch lines are broken This is because VoiceFollower is not set as (cross-staff . #t) in define-grobs.scm. I'm not sure why...this seems like a good idea irrespective of how the skyline work pans out. I find it even more surprising that Glissandos work perfectly. Aren't they related to VoiceFollowers? They're set to cross-staff when they are cross-staff, but VoiceFollowers are by definition always cross staff. - some ties change shape This will take me some time to figure out...I'm gonna have to look at all the parameters going into tie scoring and see which ones are changing. I'll likely push something to staging the weekend or next week in the style of Tie_formatting_problem::spew () that gives tons of specs and then run the output through python to pick up on changes. Of course, if any tie gurus have intuition about this, lemme know! No idea when i'll have time to finish Tie Report, but as tie shapes are screwed in general, i wouldn't care much about this change. New shape isn't worse than previous one, i'd say. I think that there's supposed to be a bit of space in there, no? What does Gould say? What's important is to figure out why this change is happening. As ties only ever use Skylines made from boxes (and not from stencil integrals), there must be some weird subtlety about spacing somewhere (or perhaps the new Skyline code) that is causing this. - some skylines are really weird This is likely a directioning problem - all upwards pointing skylines seem to come out just fine. The culprit would most likely be in stencil-integral.cc: the notion of UP must be hardcoded somewhere that it shouldn't be. If you have a moment to test out weird-skylines.ly on different versions of the patchset, I'd appreciate it! Tenuto skyline seems to be wrong since the beginning - i've tested patchsets 36, 34, 32 and 30, it looked similar everywhere. Sharp and forte skylines seemed ok in patchset 36, see here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/weird-skylines_patchset-36.pdf http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/weird-skylines_patchset-37.pdf The tenuto is important, then. There must be something about it (maybe its symmetry) that confuses the the code. The sharp and forte are also funky...it seems that certain regions may have too short estimations. I'll investigate. - sometimes objects interlock too much. This was the case in the lyric example you sent. I fixed it by adding light skyline-horizontal-padding to lyrics. Hmm. I have another idea, which would be more difficult but also extremely awesome, and it would be very handy for area spacing (see http://lists.gnu.org/archive/html/lilypond-devel/2012-03/msg00507.html): make the skyline shape magnetic. See illustration here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/magnetic_skylines_demo.pdf What do you think? It wouldn't be too bad from a coding perspective. The function deholify already does this for single skylines: try the skyline of a large font-size string f _ f or something like that and the holes should be plugged with a sloped building. When there are multiple grobs involved, what becomes tricky is knowing which ones should allow for snug-fitting and which ones not. I think that, as time goes on, we'll realize where the snug spacing is too close and will modify it w/ padding values. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
- Original Message - From: m...@apollinemike.com To: Janek Warchoł janek.lilyp...@gmail.com In short: - staffswitch lines are broken This is because VoiceFollower is not set as (cross-staff . #t) in define-grobs.scm. I'm not sure why...this seems like a good idea irrespective of how the skyline work pans out. I've not been following the thread fully, but my understanding is that these should only be displayed when they're positively switched on - e.g. with \showStaffSwitch. -- Phil Holmes ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Wed, Jul 4, 2012 at 3:36 PM, m...@apollinemike.com m...@apollinemike.com wrote: No idea when i'll have time to finish Tie Report, but as tie shapes are screwed in general, i wouldn't care much about this change. New shape isn't worse than previous one, i'd say. I think that there's supposed to be a bit of space in there, no? What does Gould say? Definitely, current output is wrong. But as i said, i'm not sure if it's worth bothering as ties are screwed in general. What's important is to figure out why this change is happening. As ties only ever use Skylines made from boxes (and not from stencil integrals), there must be some weird subtlety about spacing somewhere (or perhaps the new Skyline code) that is causing this. Indeed, whatever the output is it shouldn't change because of new skylines. Hmm. I have another idea, which would be more difficult but also extremely awesome, and it would be very handy for area spacing (see http://lists.gnu.org/archive/html/lilypond-devel/2012-03/msg00507.html): make the skyline shape magnetic. See illustration here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705595/magnetic_skylines_demo.pdf What do you think? It wouldn't be too bad from a coding perspective. The function deholify already does this for single skylines: try the skyline of a large font-size string f _ f or something like that and the holes should be plugged with a sloped building. When there are multiple grobs involved, what becomes tricky is knowing which ones should allow for snug-fitting and which ones not. I think that, as time goes on, we'll realize where the snug spacing is too close and will modify it w/ padding values. Do you mean that padding should influence skylines' magnetism? Something like this: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705597/magnetic_skylines_-_padding.pdf (i.e. there would be only one setting, which would affect both the padding and magnetism of the skylines)? I think it might be a good idea. cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hi all, my favorite patch returned! Yay! I did some testing and the results are here: http://lilypond-stuff.1065243.n5.nabble.com/file/n5705594/patchset37-results.tar.gz (sorry, too big for an attachment - 200 kB) In short: - staffswitch lines are broken - barnumbers collide with ties - some ties change shape - some things move too close to each other, as if padding was nixed - some skylines are really weird - sometimes objects interlock too much. hope this helps! Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2 juil. 2012, at 01:03, k-ohara5...@oco.net wrote: I like the direction in which this goes, making things closer, because it is easier for users to add padding when needed than to persuade LilyPond to space things more closely. I did a quick test on some music. http://k-ohara.oco.net/Lilypond/ Ledger lines might not be getting the space they need. In rare cases, all the systems were overlaid on the top of the page. Several objects might need an increase in their default padding. Hey Keith, Could you tell me the page and the document where the system overlay happens? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Tue, 03 Jul 2012 10:51:05 -0700, m...@apollinemike.com m...@apollinemike.com wrote: Could you tell me the page and the document where the system overlay happens? Second page of the bassoon part. http://k-ohara.oco.net/Lilypond/TightSkylines/woods-Bassoon1.pdf The source is one directory up, but it's a big source. You might just look at the 2.15.40 output in the parallel directory /2.15.40/ to see if you recognize the situation that is fooling your patch. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
It's back... The only thing that doesn't the work the way I'd expect it is Skyline::padded. It seems to not be adding the padding correctly (Joe - any ideas)? This causes tighter than desired spacing in alignment-order.ly. Other than that it is all reviewable! For those who were keeping score before, files like size16.ly, size20.ly etc. were a major nuisance and are now kept at bay. Cheers, MS http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
mts...@gmail.com writes: It's back... The only thing that doesn't the work the way I'd expect it is Skyline::padded. It seems to not be adding the padding correctly (Joe - any ideas)? This causes tighter than desired spacing in alignment-order.ly. Other than that it is all reviewable! For those who were keeping score before, files like size16.ly, size20.ly etc. were a major nuisance and are now kept at bay. Tell you what: _that_ would be a good reason to make another release candidate, and if it does not get through, branch off a 2.16 prerelease branch to cherry-pick into _before_ merging the skyline stuff. And if, against all pessimistic expectations, the skyline stuff turns out not giving us any relevant regressions in the next months, why, we can have a short release cycle for 2.18 for a change. But this is the sort of thing I definitely want to see in the development versions, and that I definitely don't want to see in 2.16 release candidates at the current point of time. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 1 juil. 2012, at 17:08, David Kastrup wrote: mts...@gmail.com writes: It's back... The only thing that doesn't the work the way I'd expect it is Skyline::padded. It seems to not be adding the padding correctly (Joe - any ideas)? This causes tighter than desired spacing in alignment-order.ly. Other than that it is all reviewable! For those who were keeping score before, files like size16.ly, size20.ly etc. were a major nuisance and are now kept at bay. Tell you what: _that_ would be a good reason to make another release candidate, and if it does not get through, branch off a 2.16 prerelease branch to cherry-pick into _before_ merging the skyline stuff. And if, against all pessimistic expectations, the skyline stuff turns out not giving us any relevant regressions in the next months, why, we can have a short release cycle for 2.18 for a change. But this is the sort of thing I definitely want to see in the development versions, and that I definitely don't want to see in 2.16 release candidates at the current point of time. Couldn't agree more - this shouldn't be part of 2.16. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
I like the direction in which this goes, making things closer, because it is easier for users to add padding when needed than to persuade LilyPond to space things more closely. I did a quick test on some music. http://k-ohara.oco.net/Lilypond/ Ledger lines might not be getting the space they need. In rare cases, all the systems were overlaid on the top of the page. Several objects might need an increase in their default padding. On 2012/07/01 14:58:08, MikeSol wrote: The only thing that doesn't the work the way I'd expect it is Skyline::padded. Maybe the bit with last_end, noted below, will fix it; but what was wrong with the horizon_padding in the old code ? http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc#newcode693 lily/skyline.cc:693: Skyline::padded (Real horizon_padding) const The horizon_padding built in to Skyline::Skyline had nice beveled corners; does this result in nice beveled corners ? http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc#newcode699 lily/skyline.cc:699: if (last_end -infinity_f) I don't see any assignments to 'last_end' so this seems to be always false. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc
File lily/skyline.cc (right):
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc#newcode693
lily/skyline.cc:693: Skyline::padded (Real horizon_padding) const
On 2012/07/01 23:04:17, Keith wrote:
The horizon_padding built in to Skyline::Skyline had nice beveled
corners; does
this result in nice beveled corners ?
It did - I want to get Joe's feedback first before moving back large
chunks of the old code, as he was the one who created a lot of this new
Skyline stuff and I want to make sure I'm understanding it correctly.
http://codereview.appspot.com/5626052/diff/78001/lily/skyline.cc#newcode699
lily/skyline.cc:699: if (last_end -infinity_f)
On 2012/07/01 23:04:17, Keith wrote:
I don't see any assignments to 'last_end' so this seems to be always
false.
True - that's one of the problems. The other is that height is always
an empty interval. I've rewritten it as:
Skyline
Skyline::padded (Real horizon_padding) const
{
vectorBox pad_boxes;
Real last_end = -infinity_f;
for (listBuilding::const_iterator i = buildings_.begin (); i !=
buildings_.end (); ++i)
{
if (last_end -infinity_f)
{
// Add the box that pads the left side of the current
building.
Real start = last_end - horizon_padding;
Real height = i-height (last_end);
if (height -infinity_f)
pad_boxes.push_back (Box (Interval (start, last_end),
Interval (min (height - 0.01,
height), max (height - 0.01, height;
}
if (i-end_ infinity_f)
{
// Add the box that pads the right side of the current
building.
Real start = i-end_;
Real end = start + horizon_padding;
Real height = i-height (start);
if (height -infinity_f)
pad_boxes.push_back (Box (Interval (start, end),
Interval (min (height - 0.01,
height), max (height - 0.01, height;
}
}
// We created pad_boxes assuming that sky_ is UP. To get padded to
use
// the UP side of the boxes, we need to create it pointing UP even if
// it doesn't really.
Skyline padded (pad_boxes, X_AXIS, UP);
padded.sky_ = sky_;
padded.merge (*this);
return padded;
}
and still no result as expected...
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hey all, An update on this patch. After having correctly skylined dynamics, I'm realizing that serifed fonts and italicized fonts are posing a problem for snug vertical alignment. Joe and I have been back and forth about a solution and he has graciously accepted to tag-team the problem with me, but it'll likely require several rounds of back and forth between he and I to get it up to snuff. So, you won't be seeing a new patch for a bit, but when you do, it'll lead to better results. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
2012/3/6 Janek Warchoł janek.lilyp...@gmail.com On Tue, Mar 6, 2012 at 11:48 PM, Joe Neeman joenee...@gmail.com wrote: 2012/3/6 janek.lilyp...@gmail.com Mike all, i did a quick compile with patchset 36 and unfortunately didn't notice significant speedups from previous version. Could you try the dev/jneem branch in git? It has some optimizations. If that doesn't help, could you please send me some of the worst files? They look better indeed time-wise. As for the output, i've only did a quick look and noticed this: why you deleted horizontal padding? This results in mf at the bottom right of 1st page of Tota pulchra to jump into above lyrics. The way skyline padding was being done made the code very messy. Also, padding was being added in multiple places, so some things were getting extra padding. However, the capability for doing padding is still there (for example, with the outside-staff-horizon-padding property). I agree, though, that the defaults will need to be revisited. Actually, small/zero horizontal padding might give better results in some places, but that needs thorough investigation. Something that, FWIW, never happened with the previous defaults. A bunch of the lines I deleted had comments like // TODO: figure out what horizon_padding should be Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
2012/3/7 Janek Warchoł janek.lilyp...@gmail.com On Wed, Mar 7, 2012 at 11:43 AM, Joe Neeman joenee...@gmail.com wrote: 2012/3/6 Janek Warchoł janek.lilyp...@gmail.com Actually, small/zero horizontal padding might give better results in some places, but that needs thorough investigation. Something that, FWIW, never happened with the previous defaults. do you mean that there was no padding previously? No, I mean that the previous default values were never investigated. When the code was written, values were hard-coded with comments like TODO: what's the proper value? Years later, those comments were still there... Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Wed, Mar 7, 2012 at 10:50 PM, Joe Neeman joenee...@gmail.com wrote: 2012/3/7 Janek Warchoł janek.lilyp...@gmail.com On Wed, Mar 7, 2012 at 11:43 AM, Joe Neeman joenee...@gmail.com wrote: 2012/3/6 Janek Warchoł janek.lilyp...@gmail.com Actually, small/zero horizontal padding might give better results in some places, but that needs thorough investigation. Something that, FWIW, never happened with the previous defaults. do you mean that there was no padding previously? No, I mean that the previous default values were never investigated. When the code was written, values were hard-coded with comments like TODO: what's the proper value? Years later, those comments were still there... ah, ok, i understand now. thanks, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Mike all, i did a quick compile with patchset 36 and unfortunately didn't notice significant speedups from previous version. Compilation time differences vary widely depending on music (from 5% to 100% longer) PROCESSING ../dynamics.ly WITH MASTER: -- real0m0.762s user0m0.656s sys 0m0.076s PROCESSING ../dynamics.ly WITH SKYLINES: real0m0.799s user0m0.692s sys 0m0.080s PROCESSING ../eja-mater/Eja-Mater.ly WITH MASTER: -- real0m7.324s user0m6.488s sys 0m0.528s PROCESSING ../eja-mater/Eja-Mater.ly WITH SKYLINES: real0m14.040s user0m12.185s sys 0m1.132s PROCESSING ../Epifania/Eja_Mater_klarnet.ly WITH MASTER: -- real0m2.462s user0m2.120s sys 0m0.192s PROCESSING ../Epifania/Eja_Mater_klarnet.ly WITH SKYLINES: real0m3.314s user0m2.888s sys 0m0.256s PROCESSING ../Epifania/Gabriel's_Oboe/choir.ly WITH MASTER: -- real0m2.539s user0m2.200s sys 0m0.200s PROCESSING ../Epifania/Gabriel's_Oboe/choir.ly WITH SKYLINES: real0m4.081s user0m3.460s sys 0m0.344s PROCESSING ../Epifania/He_trusted_in_God/choir.ly WITH MASTER: -- real0m5.468s user0m4.848s sys 0m0.348s PROCESSING ../Epifania/He_trusted_in_God/choir.ly WITH SKYLINES: real0m11.347s user0m9.621s sys 0m0.988s PROCESSING ../Epifania/He_trusted_in_God/continuo.ly WITH MASTER: -- real0m2.201s user0m1.920s sys 0m0.184s PROCESSING ../Epifania/He_trusted_in_God/continuo.ly WITH SKYLINES: real0m2.440s user0m2.168s sys 0m0.176s PROCESSING ../Epifania/He_trusted_in_God/full_score.ly WITH MASTER: -- real0m8.185s user0m7.492s sys 0m0.412s PROCESSING ../Epifania/He_trusted_in_God/full_score.ly WITH SKYLINES: real0m14.932s user0m12.901s sys 0m1.248s PROCESSING ../Epifania/He_trusted_in_God/viola.ly WITH MASTER: -- real0m1.779s user0m1.552s sys 0m0.164s PROCESSING ../Epifania/He_trusted_in_God/viola.ly WITH SKYLINES: real0m1.970s user0m1.748s sys 0m0.136s PROCESSING ../Epifania/He_trusted_in_God/violin_I.ly WITH MASTER: -- real0m1.582s user0m1.372s sys 0m0.152s PROCESSING ../Epifania/He_trusted_in_God/violin_I.ly WITH SKYLINES: real0m1.796s user0m1.536s sys 0m0.184s PROCESSING ../Epifania/He_trusted_in_God/violin_II.ly WITH MASTER: -- real0m1.699s user0m1.520s sys 0m0.124s PROCESSING ../Epifania/He_trusted_in_God/violin_II.ly WITH SKYLINES: real0m1.954s user0m1.708s sys 0m0.172s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/choir.ly WITH MASTER: -- real0m2.416s user0m2.152s sys 0m0.140s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/choir.ly WITH SKYLINES: real0m4.625s user0m3.616s sys 0m0.468s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/full_score.ly WITH MASTER: -- real0m0.684s user0m0.580s sys 0m0.056s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/full_score.ly WITH SKYLINES: real0m0.680s user0m0.552s sys 0m0.068s PROCESSING ../Epifania/Nad_Betlejem_w_ciemną_noc/Nad_Betlejem_w._orkiestrowa_3_systemy.ly WITH MASTER: -- real0m2.039s user0m1.700s sys 0m0.152s PROCESSING ../Epifania/Nad_Betlejem_w_ciemną_noc/Nad_Betlejem_w._orkiestrowa_3_systemy.ly WITH SKYLINES: real0m3.326s user0m2.820s sys 0m0.260s PROCESSING ../Epifania/O_Matko_miłościwa/choir.ly WITH MASTER: -- real0m4.176s user0m3.688s sys 0m0.296s PROCESSING ../Epifania/O_Matko_miłościwa/choir.ly WITH SKYLINES: real0m8.384s user0m7.144s sys 0m0.780s PROCESSING ../Epifania/O_Matko_miłościwa/continuo.ly WITH MASTER: -- real0m0.861s user0m0.752s sys 0m0.068s PROCESSING ../Epifania/O_Matko_miłościwa/continuo.ly WITH SKYLINES: real0m0.873s user0m0.740s sys 0m0.076s PROCESSING ../Epifania/O_Matko_miłościwa/full_score.ly WITH MASTER: -- real0m4.027s user0m3.652s sys 0m0.248s PROCESSING ../Epifania/O_Matko_miłościwa/full_score.ly WITH SKYLINES: real0m8.267s user0m7.156s sys 0m0.668s PROCESSING ../Epifania/O_Matko_miłościwa/viola.ly WITH MASTER: -- real0m0.878s user0m0.768s sys 0m0.072s PROCESSING ../Epifania/O_Matko_miłościwa/viola.ly WITH SKYLINES: real0m0.869s user0m0.772s sys 0m0.060s PROCESSING ../Epifania/O_Matko_miłościwa/violin_I.ly WITH MASTER: -- real0m0.880s user0m0.756s sys 0m0.084s PROCESSING
Re: Gets vertical skylines from grob stencils (issue 5626052)
2012/3/6 janek.lilyp...@gmail.com Mike all, i did a quick compile with patchset 36 and unfortunately didn't notice significant speedups from previous version. Could you try the dev/jneem branch in git? It has some optimizations. If that doesn't help, could you please send me some of the worst files? Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Tue, Mar 6, 2012 at 11:48 PM, Joe Neeman joenee...@gmail.com wrote: 2012/3/6 janek.lilyp...@gmail.com Mike all, i did a quick compile with patchset 36 and unfortunately didn't notice significant speedups from previous version. Could you try the dev/jneem branch in git? It has some optimizations. If that doesn't help, could you please send me some of the worst files? They look better indeed time-wise. As for the output, i've only did a quick look and noticed this: why you deleted horizontal padding? This results in mf at the bottom right of 1st page of Tota pulchra to jump into above lyrics. Actually, small/zero horizontal padding might give better results in some places, but that needs thorough investigation. Compilation times: PROCESSING ../dynamics.ly WITH MASTER: -- real0m0.752s user0m0.632s sys 0m0.096s PROCESSING ../dynamics.ly WITH SKYLINES: real0m1.002s user0m0.656s sys 0m0.092s PROCESSING ../eja-mater/Eja-Mater.ly WITH MASTER: -- real0m7.195s user0m6.320s sys 0m0.456s PROCESSING ../eja-mater/Eja-Mater.ly WITH SKYLINES: real0m10.792s user0m9.145s sys 0m0.816s PROCESSING ../hairpins.ly WITH MASTER: -- real0m1.100s user0m0.772s sys 0m0.072s PROCESSING ../hairpins.ly WITH SKYLINES: real0m0.866s user0m0.772s sys 0m0.064s PROCESSING ../lyrics.ly WITH MASTER: -- real0m0.906s user0m0.752s sys 0m0.084s PROCESSING ../lyrics.ly WITH SKYLINES: real0m0.955s user0m0.824s sys 0m0.100s PROCESSING ../psalm94-reubke/psalm94.ly WITH MASTER: -- real0m46.639s user0m42.651s sys 0m2.232s PROCESSING ../psalm94-reubke/psalm94.ly WITH SKYLINES: ^C janek@lilydev2 ~$ ./skytest PROCESSING ../dynamics.ly WITH MASTER: -- real0m0.760s user0m0.652s sys 0m0.076s PROCESSING ../dynamics.ly WITH SKYLINES: real0m0.807s user0m0.692s sys 0m0.080s PROCESSING ../eja-mater/Eja-Mater.ly WITH MASTER: -- real0m7.295s user0m6.408s sys 0m0.528s PROCESSING ../eja-mater/Eja-Mater.ly WITH SKYLINES: real0m10.482s user0m9.161s sys 0m0.800s PROCESSING ../hairpins.ly WITH MASTER: -- real0m0.830s user0m0.736s sys 0m0.064s PROCESSING ../hairpins.ly WITH SKYLINES: real0m0.811s user0m0.696s sys 0m0.088s PROCESSING ../lyrics.ly WITH MASTER: -- real0m0.852s user0m0.704s sys 0m0.116s PROCESSING ../lyrics.ly WITH SKYLINES: real0m0.968s user0m0.844s sys 0m0.084s PROCESSING ../psalm94-reubke/psalm94.ly WITH MASTER: -- real0m45.944s user0m42.799s sys 0m2.144s PROCESSING ../psalm94-reubke/psalm94.ly WITH SKYLINES: real1m9.338s user0m48.959s sys 0m3.320s PROCESSING ../slanted-trillspan.ly WITH MASTER: -- Unbound variable: ly:grob::vertical-skylines-from-stencil real0m2.852s user0m0.724s sys 0m0.176s PROCESSING ../slanted-trillspan.ly WITH SKYLINES: real0m1.361s user0m0.800s sys 0m0.128s PROCESSING ../tota-pulchra/tota-pulchra.ly WITH MASTER: -- real0m8.149s user0m7.416s sys 0m0.360s PROCESSING ../tota-pulchra/tota-pulchra.ly WITH SKYLINES: real0m10.730s user0m9.625s sys 0m0.672s cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Fri, Mar 2, 2012 at 7:38 AM, m...@apollinemike.com m...@apollinemike.com wrote: Not a prob, Janek, and thank you _very_ much for your help. You've put as much time as I have into this thing, and I couldn't have gotten it to where it is w/o your feedback. Thanks! :) Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
I couldn't resist the temptation of compiling new patchset. Here are the compilation times i've got (no time to check how output looks like, though): PROCESSING ../dynamics.ly WITH MASTER: -- real0m0.732s PROCESSING ../dynamics.ly WITH SKYLINES: real0m0.787s PROCESSING ../eja-mater/Eja-Mater.ly WITH MASTER: -- real0m7.144s PROCESSING ../eja-mater/Eja-Mater.ly WITH SKYLINES: real0m14.459s PROCESSING ../Epifania/Eja_Mater_klarnet.ly WITH MASTER: -- real0m2.330s PROCESSING ../Epifania/Eja_Mater_klarnet.ly WITH SKYLINES: real0m3.219s PROCESSING ../Epifania/Gabriel's_Oboe/choir.ly WITH MASTER: -- real0m2.411s PROCESSING ../Epifania/Gabriel's_Oboe/choir.ly WITH SKYLINES: real0m3.980s PROCESSING ../Epifania/He_trusted_in_God/choir.ly WITH MASTER: -- real0m5.155s PROCESSING ../Epifania/He_trusted_in_God/choir.ly WITH SKYLINES: real0m11.485s PROCESSING ../Epifania/He_trusted_in_God/continuo.ly WITH MASTER: -- real0m2.056s PROCESSING ../Epifania/He_trusted_in_God/continuo.ly WITH SKYLINES: real0m2.452s PROCESSING ../Epifania/He_trusted_in_God/full_score.ly WITH MASTER: -- real0m7.878s user0m7.200s sys 0m0.424s PROCESSING ../Epifania/He_trusted_in_God/full_score.ly WITH SKYLINES: real0m15.219s user0m13.337s sys 0m1.140s PROCESSING ../Epifania/He_trusted_in_God/viola.ly WITH MASTER: -- real0m1.707s user0m1.552s sys 0m0.100s PROCESSING ../Epifania/He_trusted_in_God/viola.ly WITH SKYLINES: real0m1.912s user0m1.708s sys 0m0.132s PROCESSING ../Epifania/He_trusted_in_God/violin_I.ly WITH MASTER: -- real0m1.484s user0m1.324s sys 0m0.108s PROCESSING ../Epifania/He_trusted_in_God/violin_I.ly WITH SKYLINES: real0m1.790s user0m1.580s sys 0m0.124s PROCESSING ../Epifania/He_trusted_in_God/violin_II.ly WITH MASTER: -- real0m1.611s user0m1.448s sys 0m0.108s PROCESSING ../Epifania/He_trusted_in_God/violin_II.ly WITH SKYLINES: real0m1.904s user0m1.680s sys 0m0.144s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/choir.ly WITH MASTER: -- real0m2.347s user0m1.996s sys 0m0.220s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/choir.ly WITH SKYLINES: real0m4.208s user0m3.504s sys 0m0.416s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/full_score.ly WITH MASTER: -- real0m0.586s user0m0.520s sys 0m0.040s PROCESSING ../Epifania/Jesus_bleibet_meine_Freude/full_score.ly WITH SKYLINES: real0m0.608s user0m0.548s sys 0m0.032s PROCESSING ../Epifania/Nad_Betlejem_w_ciemną_noc/Nad_Betlejem_w._orkiestrowa_3_systemy.ly WITH MASTER: -- real0m1.848s user0m1.588s sys 0m0.180s PROCESSING ../Epifania/Nad_Betlejem_w_ciemną_noc/Nad_Betlejem_w._orkiestrowa_3_systemy.ly WITH SKYLINES: real0m3.139s user0m2.684s sys 0m0.260s PROCESSING ../Epifania/O_Matko_miłościwa/choir.ly WITH MASTER: -- real0m3.950s user0m3.560s sys 0m0.240s PROCESSING ../Epifania/O_Matko_miłościwa/choir.ly WITH SKYLINES: real0m8.374s user0m7.160s sys 0m0.760s PROCESSING ../Epifania/O_Matko_miłościwa/continuo.ly WITH MASTER: -- real0m0.818s user0m0.712s sys 0m0.068s PROCESSING ../Epifania/O_Matko_miłościwa/continuo.ly WITH SKYLINES: real0m0.836s user0m0.716s sys 0m0.084s PROCESSING ../Epifania/O_Matko_miłościwa/full_score.ly WITH MASTER: -- real0m3.915s user0m3.644s sys 0m0.148s PROCESSING ../Epifania/O_Matko_miłościwa/full_score.ly WITH SKYLINES: real0m8.306s user0m7.188s sys 0m0.696s PROCESSING ../Epifania/O_Matko_miłościwa/viola.ly WITH MASTER: -- real0m0.827s user0m0.700s sys 0m0.080s PROCESSING ../Epifania/O_Matko_miłościwa/viola.ly WITH SKYLINES: real0m0.840s user0m0.732s sys 0m0.068s PROCESSING ../Epifania/O_Matko_miłościwa/violin_I.ly WITH MASTER: -- real0m0.840s user0m0.732s sys 0m0.068s PROCESSING ../Epifania/O_Matko_miłościwa/violin_I.ly WITH SKYLINES: real0m0.898s user0m0.780s sys 0m0.076s PROCESSING ../Epifania/O_Matko_miłościwa/violin_II.ly WITH MASTER: -- real0m0.811s user0m0.676s sys 0m0.096s PROCESSING ../Epifania/O_Matko_miłościwa/violin_II.ly WITH SKYLINES: real0m0.838s user0m0.708s sys 0m0.092s PROCESSING ../hairpins.ly WITH MASTER: -- real0m0.775s user0m0.672s sys
Re: Gets vertical skylines from grob stencils (issue 5626052)
From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I second this. I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. I still think that the `bounding outline' should be defined in the Metafont code for Emmentaler glyphs. However, this should be rather easy to add later on in case we try this route. Caching glyph outlines certainly helps, BTW. Werner ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
2012/3/1 Janek Warchoł janek.lilyp...@gmail.com: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? You could look into some heuristic that limits the number of boxes depending on their shapes, so it creates a single box for most glyphs. For example, you could take the box enclosing the glyph and compare its area with that of the union of the boxes, and revert to one box if the difference is less than X percent. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Han-Wen Nienhuys hanw...@gmail.com writes: 2012/3/1 Janek Warchoł janek.lilyp...@gmail.com: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? Well, it is again an issue of incest tabu where the details of the combining skylines make best sense for combining _heterogenous_ elements: for arranging text with text, you don't want to have things get too closely or even interleaved. But having a single high letter interlock with a note stem can improve the overall arrangement. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Mar 1, 2012 at 7:57 AM, David Kastrup d...@gnu.org wrote: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? Well, it is again an issue of incest tabu where the details of the combining skylines make best sense for combining _heterogenous_ elements: for arranging text with text, you don't want to have things get too closely or even interleaved. But having a single high letter interlock with a note stem can improve the overall arrangement. Exactly my point: you can fix the single letter case with a single bbox per glyph. Why do you need more accuracy than single box per glyph? -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mar 1, 2012, at 11:52 AM, Han-Wen Nienhuys wrote: 2012/3/1 Janek Warchoł janek.lilyp...@gmail.com: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? Janek had sent out a couple e-mails before (I can't find them). It makes a significant difference for DynamicText, and I know he has a few examples of lyrics as well where it makes a difference. David's also right about the single high letter and note stem (this was one of Janek's examples). You could look into some heuristic that limits the number of boxes depending on their shapes, so it creates a single box for most glyphs. I agree - there's a lot of room for heuristicking. For example, you could take the box enclosing the glyph and compare its area with that of the union of the boxes, and revert to one box if the difference is less than X percent. Remembering that 81.2% of all statistics are fabricated on the spot, I'd say that 70% of speed-up work could be done in skyline.cc. I think stencil-integral.cc is as fast as its gonna get. It may help to cache a FT_Face in the Pango_font class as I'm not sure how long it takes to look it up. I'm not convinced after testing that caching font outlines significantly speeds things up - there used to be some code for this, but I've removed it. It can be put back in later if it turns out that after profiling the calls to the functions in freetype.cc sap a lot of time. What I really need help on is profiling. People have so far offered great suggestions on how to do it, but I'm not too good at reading the summaries from profilers to know exactly where the resources are being consumed. If someone sent me a really detailed chart with what resources were consumed on what, that'd help immensely. The only thing that was clear to me was that LilyPond is spending a lot of time in skyline-related functions, but I'm not sure which ones. The skyline class in general looks like it needs a look-over from someone who knows something about CS and optimization. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Han-Wen Nienhuys hanw...@gmail.com writes: On Thu, Mar 1, 2012 at 7:57 AM, David Kastrup d...@gnu.org wrote: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? Well, it is again an issue of incest tabu where the details of the combining skylines make best sense for combining _heterogenous_ elements: for arranging text with text, you don't want to have things get too closely or even interleaved. But having a single high letter interlock with a note stem can improve the overall arrangement. Exactly my point: you can fix the single letter case with a single bbox per glyph. Why do you need more accuracy than single box per glyph? Because a q does not have its descender stick out in the middle over the full breadth. For collision avoidance, it is good for at least two boxes. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Janek Warchoł wrote Thursday, March 01, 2012 12:01 AM The bad news is that for scores containing a lot of lyrics (like my SATB pieces) compilation times are now 3-4 times longer than wiith master. For instrumental scores the situation look better, it's 1.5-2 times longer. I can live with these compilation times, but others probably won't like it... I wouldn't like it for one. Could either the precise skylines for glyphs be optionally activated, or applied only to dynamic text, not lyrics? Trevor ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Mar 1, 2012 at 8:01 AM, David Kastrup d...@gnu.org wrote: Han-Wen Nienhuys hanw...@gmail.com writes: 2012/3/1 Janek Warchoł janek.lilyp...@gmail.com: From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? You could look into some heuristic that limits the number of boxes depending on their shapes, so it creates a single box for most glyphs. For example, you could take the box enclosing the glyph and compare its area with that of the union of the boxes, and revert to one box if the difference is less than X percent. Percentage of area differences sounds like a recipe for disaster. Once one box gets large enough, it will eat every small box in its vicinity because any single one will not make a large percentual difference. Well, perhaps there is another method. I just want to point out that it is a waste to represent the extent of the letter 'n' with 10 boxes, and we should be able to do better. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mar 1, 2012, at 12:17 PM, Trevor Daniels wrote: Janek Warchoł wrote Thursday, March 01, 2012 12:01 AM The bad news is that for scores containing a lot of lyrics (like my SATB pieces) compilation times are now 3-4 times longer than wiith master. For instrumental scores the situation look better, it's 1.5-2 times longer. I can live with these compilation times, but others probably won't like it... I wouldn't like it for one. Could either the precise skylines for glyphs be optionally activated, or applied only to dynamic text, not lyrics? The good news is that a simple property activation causes this to kick in, so before pushing the patch, we can decide what does and doesn't get this feature as default behavior. Furthermore, we can create a file precision-spacing.ly that sets tons of grobs to the more time-costly alternative for those who want to put their finished score in a final pass through the vertical-skyline functions. Now, though, it's important to develop the with everything at maximum complexity to see what type of toll it takes and get as many bugs out as possible. I still think there's a lot of room for speed up - as I said in my previous e-mail, I just need help with some profiling. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mar 1, 2012, at 12:23 PM, Han-Wen Nienhuys wrote: Well, perhaps there is another method. I just want to point out that it is a waste to represent the extent of the letter 'n' with 10 boxes, and we should be able to do better. I agree that it's excessive. To me, do better means maximally optimizing the code and then making decisions about what should get fine-tuned skylines at the end of the patch development process. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Mar 1, 2012 at 12:00 PM, m...@apollinemike.com m...@apollinemike.com wrote: On Mar 1, 2012, at 11:52 AM, Han-Wen Nienhuys wrote: It's neat that you are generating such precise skylines, but can you show places where this makes an appreciable difference for texts? Janek had sent out a couple e-mails before (I can't find them). It makes a significant difference for DynamicText, and I know he has a few examples of lyrics as well where it makes a difference. David's also right about the single high letter and note stem (this was one of Janek's examples). I attach sources of my test files. I will send pdfs compiled with both master and Mike's patch, with some of the differences highlighted, privately (4 snippets and 2 real-life scores, 4 MB). I hope you'll like it :) thanks, Janek skylines-tests-sources.tar.gz Description: GNU Zip compressed data ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
As for compilation times with patchset 32, SATB choral files compile two times longer than master, and instrumentals 1.0-1.5 times longer than master. Mike, i am very sorry but i have to pause my involvement in this issue a bit. I'm very short on time and testing takes a lot of time :/ cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mar 2, 2012, at 6:28 AM, Janek Warchoł wrote: As for compilation times with patchset 32, SATB choral files compile two times longer than master, and instrumentals 1.0-1.5 times longer than master. Mike, i am very sorry but i have to pause my involvement in this issue a bit. I'm very short on time and testing takes a lot of time :/ cheers, Janek Not a prob, Janek, and thank you _very_ much for your help. You've put as much time as I have into this thing, and I couldn't have gotten it to where it is w/o your feedback. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Feb 29, 2012, at 2:05 AM, Joe Neeman wrote: On Tue, Feb 28, 2012 at 1:32 PM, m...@apollinemike.com m...@apollinemike.com wrote: Hey all, I've put up a new version that doesn't issue warnings. Any change of getting an updated git branch, too? Done. A lot of the too-close-spacing issues are now cleared up. Cheers, MS___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hey Mike, I did a quick check of newest patchset (30) and the results look great! There are some flaws, but i'd say they're not your patch's fault. Rather, it seems that the new super-precise skylines unveil imperfections in vertical spacing code. The bad news is that for scores containing a lot of lyrics (like my SATB pieces) compilation times are now 3-4 times longer than wiith master. For instrumental scores the situation look better, it's 1.5-2 times longer. I can live with these compilation times, but others probably won't like it... cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Mar 1, 2012, at 1:01 AM, Janek Warchoł wrote: Hey Mike, I did a quick check of newest patchset (30) and the results look great! There are some flaws, but i'd say they're not your patch's fault. Rather, it seems that the new super-precise skylines unveil imperfections in vertical spacing code. I'm interested to see what they are - send them over if you can! The bad news is that for scores containing a lot of lyrics (like my SATB pieces) compilation times are now 3-4 times longer than wiith master. For instrumental scores the situation look better, it's 1.5-2 times longer. I can live with these compilation times, but others probably won't like it... The precision takes a big toll. I'm not sure the best places to speed up the code. Perhaps caching the FT_Face for Pango_font objects? I'd need help from others to help me spot places to speed this up. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Mar 1, 2012 at 8:14 AM, m...@apollinemike.com m...@apollinemike.com wrote: On Mar 1, 2012, at 1:01 AM, Janek Warchoł wrote: I did a quick check of newest patchset (30) and the results look great! There are some flaws, but i'd say they're not your patch's fault. Rather, it seems that the new super-precise skylines unveil imperfections in vertical spacing code. I'm interested to see what they are - send them over if you can! Sure. Files are big, so i'll send them in private mail. The bad news is that for scores containing a lot of lyrics (like my SATB pieces) compilation times are now 3-4 times longer than wiith master. For instrumental scores the situation look better, it's 1.5-2 times longer. I can live with these compilation times, but others probably won't like it... The precision takes a big toll. I'm not sure the best places to speed up the code. Perhaps caching the FT_Face for Pango_font objects? I'd need help from others to help me spot places to speed this up. From what i see, the skylines are now more precise than they need to be - every glyph has a skyline of 10 or so boxes, even if it's a single letter! (see attached) I think the proper solution would be to: a) set minimal step size to 0.2 staffspace (or more in case of bigger objects) b) change outlines from stairs to glued lines (what Joe suggested). This would allow for even less fragments for each skyline. maybe caching will help, too. cheers, Janek attachment: super-precise-skylines.png___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hey all, I have a new patch up that simplifies the old one a great deal and uses freetype to get outlines. The only problem with it (that I know of) exists for some non-standard fonts. If you run input/regression/utf-8.ly through this, you'll see LilyPond get very angry about not being able to find the glyph in the font. This is because I am stashing a font at creation time in pango-font.cc (check out the constructor - you'll see a new member font_ being set) instead of extracting the font from the glyphs. In certain cases, this means that the FT_Face created in the pango_item_string_stencil of my patch is different from that created in current master. I have no clue why this is the case - does anyone have any intuition about this? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hey all, I've put up a new version that doesn't issue warnings. The only problem I'm running into now is that the outlines aren't always 100% exact. This causes really close kerning and sometimes even overlap (check out figured-bass-implicit.ly). An easy (albeit kludgy) solution would be to add a bit of padding around all glyphs that come from a pango font. Does this seem like a reasonable way of going about it? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Tue, Feb 28, 2012 at 1:32 PM, m...@apollinemike.com m...@apollinemike.com wrote: Hey all, I've put up a new version that doesn't issue warnings. Any change of getting an updated git branch, too? ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Fri, Feb 24, 2012 at 12:50 AM, m...@apollinemike.com m...@apollinemike.com wrote: On Feb 24, 2012, at 12:07 AM, Janek Warchoł wrote: dynamic texts aren't skylined, their outlines are still rectangles. The issue is that dynamics are created via Pango and thus I can only use rectangles for the time being. A separate issue in the tracker can be opened for redoing dynamics so that they used a series of named-glyph stencils. Hmm. But what about custom dynamics created using make-dynamic-script? (http://www.lilypond.org/doc/v2.15/Documentation/notation/expressive-marks-attached-to-notes#new-dynamic-marks) I mean, isn't Pango necessary to create them? If so, it seems to me that what you suggest will result in having two types of dynamics: simple stencil dynamics and advanced Pango dynamics. This doesn't sound good to me. I think the question is: should we rather try to break inside Pango to be able to create precise outlines for everything that is processed by Pango? The advantage of this approach would be that lyrics and markups would be skylined even better. cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Feb 26, 2012, at 1:08 PM, Janek Warchoł wrote: On Fri, Feb 24, 2012 at 12:50 AM, m...@apollinemike.com m...@apollinemike.com wrote: On Feb 24, 2012, at 12:07 AM, Janek Warchoł wrote: dynamic texts aren't skylined, their outlines are still rectangles. The issue is that dynamics are created via Pango and thus I can only use rectangles for the time being. A separate issue in the tracker can be opened for redoing dynamics so that they used a series of named-glyph stencils. should we rather try to break inside Pango to be able to create precise outlines for everything that is processed by Pango? The advantage of this approach would be that lyrics and markups would be skylined even better. I'd like to do this...I'm just not sure if it's possible. Joe (or someone who knows fonts) - is there a way to get font outlines of pango fonts? Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
m...@apollinemike.com m...@apollinemike.com writes: On Feb 26, 2012, at 1:08 PM, Janek Warchoł wrote: On Fri, Feb 24, 2012 at 12:50 AM, m...@apollinemike.com m...@apollinemike.com wrote: On Feb 24, 2012, at 12:07 AM, Janek Warchoł wrote: dynamic texts aren't skylined, their outlines are still rectangles. The issue is that dynamics are created via Pango and thus I can only use rectangles for the time being. A separate issue in the tracker can be opened for redoing dynamics so that they used a series of named-glyph stencils. should we rather try to break inside Pango to be able to create precise outlines for everything that is processed by Pango? The advantage of this approach would be that lyrics and markups would be skylined even better. I'd like to do this...I'm just not sure if it's possible. Joe (or someone who knows fonts) - is there a way to get font outlines of pango fonts? The reason we are using Pango in the first place is because it is good at composing Unicode (with kerning, ligatures, accents, writing direction etc etc). So we don't want the outlines of its fonts but rather the outlines of the composed result. Not that I would have a clue about the answer to either your question or the changed question. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
should we rather try to break inside Pango to be able to create precise outlines for everything that is processed by Pango? The advantage of this approach would be that lyrics and markups would be skylined even better. The reason we are using Pango in the first place is because it is good at composing Unicode (with kerning, ligatures, accents, writing direction etc etc). So we don't want the outlines of its fonts but rather the outlines of the composed result. AFAIK, Pango returns a list of glyph indices and coordinates to position glyphs. It's straightforward to feed those indices directly into FreeType's `FT_Load_Glyph' function which is able to return an `FT_Outline' structure holding the glyph's outline. Werner ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Feb 26, 2012, at 7:37 PM, Werner LEMBERG wrote: AFAIK, Pango returns a list of glyph indices and coordinates to position glyphs. It's straightforward to feed those indices directly into FreeType's `FT_Load_Glyph' function which is able to return an `FT_Outline' structure holding the glyph's outline. This is great Werner - thank you! I just checked out FT_Outline. It's really straightforward, which is great. Joe - this likely means that the build system work you were thinking about doing w/ glyph outlines is no longer necessary and that I can scrap all of the .scm stuff I wrote and replace it with calls to this. I'll have time Wednesday morning to check this out. Cheers, MS___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Sun, Feb 26, 2012 at 11:25 AM, m...@apollinemike.com m...@apollinemike.com wrote: On Feb 26, 2012, at 7:37 PM, Werner LEMBERG wrote: AFAIK, Pango returns a list of glyph indices and coordinates to position glyphs. It's straightforward to feed those indices directly into FreeType's `FT_Load_Glyph' function which is able to return an `FT_Outline' structure holding the glyph's outline. This is great Werner - thank you! I just checked out FT_Outline. It's really straightforward, which is great. Joe - this likely means that the build system work you were thinking about doing w/ glyph outlines is no longer necessary and that I can scrap all of the .scm stuff I wrote and replace it with calls to this. I'll have time Wednesday morning to check this out. Sounds good! ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Sun, Feb 26, 2012 at 8:25 PM, m...@apollinemike.com m...@apollinemike.com wrote: On Feb 26, 2012, at 7:37 PM, Werner LEMBERG wrote: AFAIK, Pango returns a list of glyph indices and coordinates to position glyphs. It's straightforward to feed those indices directly into FreeType's `FT_Load_Glyph' function which is able to return an `FT_Outline' structure holding the glyph's outline. This is great Werner - thank you! I just checked out FT_Outline. It's really straightforward, which is great. Wonderful! Joe - this likely means that the build system work you were thinking about doing w/ glyph outlines is no longer necessary and that I can scrap all of the .scm stuff I wrote and replace it with calls to this. I'll have time Wednesday morning to check this out. I can't wait! That's incredible! cheers, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hey all, The newest patch set implements several of Joe's suggestions but takes a time hit because I am using the fancy skylines for Scripts. This makes script-accidental-collision.ly look better (as well as a few other regtests) but it slows things down even more. I think that the coding is sound, though, and it is as fast as possible given what I know how to do. I could maybe eliminate one or two loops here or there (i.e. transforming buildings to points and back to buildings for the skyline constructor that accepts skyline pairs) but this seems like chump change. Each successive slow down comes from the skylining of a new grob, which is linear (there's tons of scripts, so tons of subtleties in the skylines). Of course, any speed-up suggestions are welcome. I know that people have suggested being pickier about what grobs have tight skylines, but I want to include a rather large range, as with each new grob the regtests look better and better. Janek - all of the problems you pointed out before should be fixed in this newest patch set - thank you very much for spotting them. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Feb 23, 2012 at 1:35 AM, m...@apollinemike.com m...@apollinemike.com wrote: Hey all, The newest patch set implements several of Joe's suggestions but takes a time hit because I am using the fancy skylines for Scripts. This makes script-accidental-collision.ly look better (as well as a few other regtests) but it slows things down even more. I think that the coding is sound, though, and it is as fast as possible given what I know how to do. I could maybe eliminate one or two loops here or there (i.e. transforming buildings to points and back to buildings for the skyline constructor that accepts skyline pairs) but this seems like chump change. Each successive slow down comes from the skylining of a new grob, which is linear (there's tons of scripts, so tons of subtleties in the skylines). Of course, any speed-up suggestions are welcome. I don't see any obvious things to speed up... can you update your git branch? Then I'll profile it and take a closer look. Cheers, Joe ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc#newcode362 lily/skyline.cc:362: result.push_front (Building (last_end, -infinity_f, -infinity_f, iv[LEFT] - 2 * horizon_padding)); push_back is constant time for STL lists. No need to push_front and then reverse. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc#newcode393 lily/skyline.cc:393: Skyline::Skyline (Building b, Real start, Axis horizon_axis, Direction sky) On 2012/02/22 09:34:43, joeneeman wrote: This isn't quite what I had in mind (for one thing, it means that the caller has to be aware of buildings, calculating their slope, etc.) what about Skyline::Skyline (vectorpairPoint, Point const segments, Axis, Direction)? it works similarly to Skyline::Skyline(vectorBox...) except that the resulting skyline shows the outline of the given set of line segments. Done. http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc#newcode647 lily/skyline.cc:647: out.merge (to_merge); On 2012/02/22 09:34:43, joeneeman wrote: merge is linear, so this loop is quadratic. It should now be n*log(n). http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc#newcode362 lily/skyline.cc:362: result.push_front (Building (last_end, -infinity_f, -infinity_f, iv[LEFT] - 2 * horizon_padding)); On 2012/02/23 11:11:47, joeneeman wrote: push_back is constant time for STL lists. No need to push_front and then reverse. I'm not not in favor of this, but why is there a reverse in the other non_overlapping_skyline function? I tried to copy it as closely as possible. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); On 2012/02/23 12:29:06, dak wrote: Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. The problem is TabStaff and TabVoice. In both contexts, many grobs are set to transparent as a default. These are grobs that are never supposed to factor into a vertical skyline. I'd be interested in seeing what happens if these were set to ##f instead of transparent, but that'd be a new project. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/02/23 12:37:04, MikeSol wrote: http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); On 2012/02/23 12:29:06, dak wrote: Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. The problem is TabStaff and TabVoice. In both contexts, many grobs are set to transparent as a default. These are grobs that are never supposed to factor into a vertical skyline. I'd be interested in seeing what happens if these were set to ##f instead of transparent, but that'd be a new project. a) the behavior is expected and consistent b) http://code.google.com/p/lilypond/issues/detail?id=2085 is fixed and verified Setting that stuff to transparent is merely an ugly workaround used for historical reasons. We should not create inconsistent and complex semantics to half-heartedly support it. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/02/23 13:35:29, dak wrote: On 2012/02/23 12:37:04, MikeSol wrote: http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); On 2012/02/23 12:29:06, dak wrote: Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. The problem is TabStaff and TabVoice. In both contexts, many grobs are set to transparent as a default. These are grobs that are never supposed to factor into a vertical skyline. I'd be interested in seeing what happens if these were set to ##f instead of transparent, but that'd be a new project. a) the behavior is expected and consistent b) http://code.google.com/p/lilypond/issues/detail?id=2085 is fixed and verified Setting that stuff to transparent is merely an ugly workaround used for historical reasons. We should not create inconsistent and complex semantics to half-heartedly support it. So, just to be clear, you're saying that the TabVoice/TabStaff everything-is-transparent-when-it-should-be-##f issue is blocking this patch? If so, I'll open up an issue for that and mark this as blocked. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Am 23.02.2012 14:35, schrieb d...@gnu.org: On 2012/02/23 12:37:04, MikeSol wrote: http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); On 2012/02/23 12:29:06, dak wrote: Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. The problem is TabStaff and TabVoice. In both contexts, many grobs are set to transparent as a default. These are grobs that are never supposed to factor into a vertical skyline. I'd be interested in seeing what happens if these were set to ##f instead of transparent, but that'd be a new project. a) the behavior is expected and consistent b) http://code.google.com/p/lilypond/issues/detail?id=2085 is fixed and verified Setting that stuff to transparent is merely an ugly workaround used for historical reasons. We should not create inconsistent and complex semantics to half-heartedly support it. As the person who wrote a lot of stuff for tablature and removed the stems and stuff by default, I second that it is an ugly workaround. Ideally, I had removed the stem/flag engraver completely from TabVoice, but then again, the slur engraver (and some others as well IIRC) complains when there is no stem to be taken into account when calculating the control points. I haven't tested this in detail, though, and tablature.scm and the tablature stuff in ly/engraver-init.ly exist for quite a long time now, so my poor knowledge just may be outdated. Probably opening a new issue for this is the right way to go, IMHO. Regards, Marc http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On 2012/02/23 14:09:12, MikeSol wrote: On 2012/02/23 13:35:29, dak wrote: On 2012/02/23 12:37:04, MikeSol wrote: http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc File lily/stencil-integral.cc (right): http://codereview.appspot.com/5626052/diff/40001/lily/stencil-integral.cc#newcode932 lily/stencil-integral.cc:932: MAKE_SCHEME_CALLBACK (Grob, simple_vertical_skylines_from_possibly_transparent_stencil, 1); On 2012/02/23 12:29:06, dak wrote: Why do we need this? I think that a _transparent_ stencil is not supposed to create a different skyline. That's the point of setting transparency rather than just #f-ing the stencil. The problem is TabStaff and TabVoice. In both contexts, many grobs are set to transparent as a default. These are grobs that are never supposed to factor into a vertical skyline. I'd be interested in seeing what happens if these were set to ##f instead of transparent, but that'd be a new project. a) the behavior is expected and consistent b) http://code.google.com/p/lilypond/issues/detail?id=2085 is fixed and verified Setting that stuff to transparent is merely an ugly workaround used for historical reasons. We should not create inconsistent and complex semantics to half-heartedly support it. So, just to be clear, you're saying that the TabVoice/TabStaff everything-is-transparent-when-it-should-be-##f issue is blocking this patch? If so, I'll open up an issue for that and mark this as blocked. No, it isn't blocking this patch. Ugly before - ugly afterwards is not a regression. In particular not Ugly before because of historically motivated user-code level workarounds with expected effects - ugly afterwards because of historically motivated user-code level workarounds with expected effects. One can't even call them sideeffects. You can open an issue for fixing all the transparent examples in our code base and possibly detecting more cases where #f does not work as expected. But it is not blocking this patch. It is orthogonal to this patch, and you should keep it that way instead of complicating your patch by introducing weird stuff for fixing an unrelated problem in the user-code level code base. http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Marc Hohl m...@hohlart.de writes: As the person who wrote a lot of stuff for tablature and removed the stems and stuff by default, made them transparent I second that it is an ugly workaround. Ideally, I had removed the stem/flag engraver completely from TabVoice, but then again, the slur engraver (and some others as well IIRC) complains when there is no stem to be taken into account when calculating the control points. Sounds to me like you did what you had to in order to get results at your comfort level with the code. But we really should straighten this out at the right place. I don't think that transparency should play a role in skyline calculations. Transparent items are _exactly_ needed to make room for stuff that otherwise would not fit. Not for circumventing code that has not yet catered for every sensible use case. I appreciate it that Mike is a remarkably gifted and clever programmer, but his code should not need to be running circles around old code. The code base should make sense as a whole. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hi Mike,
I've tested the latest patch on a few real-world scores and there
appears to be a bit of weirdness going on with TupletNumbers when a
DynamicText's nearby:
\relative c' {
\times 2/3 { c8^\p c c }
}
Cheers,
Neil
http://codereview.appspot.com/5626052/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Am 23.02.2012 15:59, schrieb David Kastrup: Marc Hohlm...@hohlart.de writes: As the person who wrote a lot of stuff for tablature and removed the stems and stuff by default, made them transparent Yes, of course ;-) I second that it is an ugly workaround. Ideally, I had removed the stem/flag engraver completely from TabVoice, but then again, the slur engraver (and some others as well IIRC) complains when there is no stem to be taken into account when calculating the control points. Sounds to me like you did what you had to in order to get results at your comfort level with the code. Yes, more or less – I was not very familiar with the code, but I got a lot of help from the community, and after all, my patches got accepted. But we really should straighten this out at the right place. I don't think that transparency should play a role in skyline calculations. Transparent items are _exactly_ needed to make room for stuff that otherwise would not fit. Not for circumventing code that has not yet catered for every sensible use case. Of course. If I had more insight into the engraver stuff (and more time), I would try to remove the dependencies between stem/flag and slur engraver. Look at the stems in the definition of TabVoice - they are transparent and as short as possible, but if you do some hideNote/unHideNote actions, the small stubs get visible, so this is definitely a rather clumsy approach. I appreciate it that Mike is a remarkably gifted and clever programmer, but his code should not need to be running circles around old code. The code base should make sense as a whole. +1 Regards, Marc ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Thu, Feb 23, 2012 at 3:13 AM, mts...@gmail.com wrote: http://codereview.appspot.com/**5626052/diff/30003/lily/**skyline.cchttp://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/**5626052/diff/30003/lily/** skyline.cc#newcode393http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc#newcode393 lily/skyline.cc:393: Skyline::Skyline (Building b, Real start, Axis horizon_axis, Direction sky) On 2012/02/22 09:34:43, joeneeman wrote: This isn't quite what I had in mind (for one thing, it means that the caller has to be aware of buildings, calculating their slope, etc.) what about Skyline::Skyline (vectorpairPoint, Point const segments, Axis, Direction)? it works similarly to Skyline::Skyline(vectorBox..**.) except that the resulting skyline shows the outline of the given set of line segments. Done. http://codereview.appspot.com/**5626052/diff/30003/lily/** skyline.cc#newcode647http://codereview.appspot.com/5626052/diff/30003/lily/skyline.cc#newcode647 lily/skyline.cc:647: out.merge (to_merge); On 2012/02/22 09:34:43, joeneeman wrote: merge is linear, so this loop is quadratic. It should now be n*log(n). http://codereview.appspot.com/**5626052/diff/34001/lily/**skyline.cchttp://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/**5626052/diff/34001/lily/** skyline.cc#newcode362http://codereview.appspot.com/5626052/diff/34001/lily/skyline.cc#newcode362 lily/skyline.cc:362: result.push_front (Building (last_end, -infinity_f, -infinity_f, iv[LEFT] - 2 * horizon_padding)); On 2012/02/23 11:11:47, joeneeman wrote: push_back is constant time for STL lists. No need to push_front and then reverse. I'm not not in favor of this, but why is there a reverse in the other non_overlapping_skyline function? Good question... ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Feb 24, 2012, at 12:07 AM, Janek Warchoł wrote: Hey Mike, On Thu, Feb 23, 2012 at 10:35 AM, m...@apollinemike.com m...@apollinemike.com wrote: Janek - all of the problems you pointed out before should be fixed in this newest patch set - thank you very much for spotting them. Actually, there's one problem left: dynamic texts aren't skylined, their outlines are still rectangles. See attachment. thanks, Janek Hey Janek, The issue is that dynamics are created via Pango and thus I can only use rectangles for the time being. A separate issue in the tracker can be opened for redoing dynamics so that they used a series of named-glyph stencils. Cheers, MS ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
Hi, sorry, Mike - i got confused and forgotten that a patchset newer than what i had is here, on Rietveld. I see that you've fixed hairpins and added skylines to accidentals, flags and beams - great! Unfortunately I've spotted some problems with DynamicSigns - they are too high (as of patchset 21). I'll send you pdf files privately. As for compilation times, they are better in case of bigger scores: dynamics.ly: master 0m0.859s previous skylines 0m1.520s skylines from patchset 21: 0m2.994s Eja-Mater.ly: master 0m7.373s previous skylines 0m13.331s skylines from patchset 21: 0m11.387s tota-pulchra.ly: master 0m8.400s previous skylines 0m15.649s skylines from patchset 21: 0m15.649s HTH, Janek http://codereview.appspot.com/5626052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Gets vertical skylines from grob stencils (issue 5626052)
On Wed, Feb 22, 2012 at 9:04 AM, janek.lilyp...@gmail.com wrote: tota-pulchra.ly: master 0m8.400s previous skylines 0m15.649s skylines from patchset 21: 0m15.649s sorry, this should read 0m12.030s ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
