Re: Approximates cross-staff slurs in VerticalAxisGroup vertical-skylines. (issue 6498077)

[address@hidden]

On 10 sept. 2012, at 23:26, address@hidden wrote:

> On 2012/09/07 16:23:21, mike7 wrote:
>> On 7 sept. 2012, at 09:34, mailto:address@hidden wrote:
> 
>>> Do you still think it possible to use just the real Slurs ?...
>>> 
>>> 1) setting tentative control points using pre-line-breaking
>>> estimates of heights (which are later replaced when the Slurs
>>> go through their post-line-breaking shaping-and-scoring cycle).
> 
>> This doesn't work because all of this skyline stuff happens
>> pre-line breaking but post-vertical spacing.
> 
> Did you mean "after line-breaking but before vertical-spacing"?

Yes, was tired.

> 
>> And furthermore, there needs to be a SlurStub for
>> each axis group traversed.
> 
> I didn't know that.  I thought one skyline per Slur would suffice, since
> it is applied for spacing the Systems.

I thought this too when I started working on this bit of code but I learned 
otherwise, ergo the approach.  Check out 
Page_layout_problem::build_system_skyline.  The system skyline used for spacing 
is actually built from the vertical axis groups.

> 
>>> 2) Determining their extremal-side-only skylines, either through
>>> callbacks on a property other than the real "vertical-skylines", > >
> or not as a callback at all but through a direct function call.

I'm wary of direct function calls to control placement - I like the use of 
properties and callbacks whenever possible.

> 
>> Even if this were done, it couldn't be applied to the Slur proper.
>> For example, if you have slur S that cross staves X Y and Z from
>> bottom to top, the SlurStub on X would only have a bottom skyline,
>> the SlurStub on Y would have no skyline and the slur stub on Z
>> would have an upper skyline.  If the Slur had these two skylines
>> via some sort of pre-skyline callback, LilyPond would think that
>> the VerticalAxisGroup were the height of the Slur and would space
>> it way far apart from its upper or lower neighbor.  That's why
>> separate stubs need to be in each axis group.
> 
> Then we don't store the extremal-side-only skylines as a property of the
> Slur.  We merely use the data in the Slur to compute extremal-side-only
> skylines, and merge them with the System skylines for system-system
> spacing.

This results in a fair bit of extra code - I had a version of the patchset like 
this, but it leads to funniness like, for example, the skylines not appearing 
when debug-skylines is set to #t because the skylines are not part of the 
vertical axis group.  In general, the way of doing things in the pushed 
patchset (creating grobs, assigning them properties and letting them get placed 
in skylines and such via the normal flow of things) is more lilypondish and 
leads to more maintainable code because it is more predictable.

> 
> But, that does seem more trouble than it is worth, given that the
> estimated slur shapes are only accurate enough to resolve about 50% of
> the collisions.
> 

I think this accuracy can be improved - I stand by what I stated before, which 
is that improvements can come later if the method in place is sound.

> 
> 
> http://codereview.appspot.com/6498077/diff/29/input/regression/cross-staff-slur-vertical-spacing.ly
> File input/regression/cross-staff-slur-vertical-spacing.ly (right):
> 
> http://codereview.appspot.com/6498077/diff/29/input/regression/cross-staff-slur-vertical-spacing.ly#newcode61
> input/regression/cross-staff-slur-vertical-spacing.ly:61: <g'' c''>8 )
> <g'' c''>8 )(
> 
> http://codereview.appspot.com/6498077/diff/29/input/regression/cross-staff-slur-vertical-spacing.ly#newcode63
> input/regression/cross-staff-slur-vertical-spacing.ly:63: e8 dis e
> e8 dis e)
> 
> http://codereview.appspot.com/6498077/diff/29/input/regression/cross-staff-slur-vertical-spacing.ly#newcode70
> input/regression/cross-staff-slur-vertical-spacing.ly:70: a8 a8 a8 a8 a8
> a8 a8_\markup \column { "f" "o" "o" } a8 a8 a8
> 
> Stubs for down-sloped slurs are not helping.
> 
> http://codereview.appspot.com/6498077/diff/29/lily/script-column.cc
> File lily/script-column.cc (right):
> 
> http://codereview.appspot.com/6498077/diff/29/lily/script-column.cc#newcode58
> lily/script-column.cc:58: /*
> This seems entirely unrelated to spacing of systems.  It looks like a
> sensible fix to issue 2589, but it causes a collision in the Chopin
> Op.45 measure 85, for reasons I cannot quite figure out.  One fix for
> that score is
> \once\override Script #'avoid-slur = #'inside

I'll try to have a look at it today or tomorrow.

> 
> I'd rather have a message in debug builds than a collision in user
> scores. (Maybe that message should be a warning rather than 'programming
> error', since LilyPond allows users to make inconsistent spacing
> requests like I did with Fingering inside and Accents avoiding slurs,
> yet fingering outside the accent when they occur together.)
> 
> http://codereview.appspot.com/6498077/