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

[address@hidden]

On 4 sept. 2012, at 09:45, address@hidden wrote:

> Works for me.  16% slower than master.
> (I'll try make clean and make.)
> It makes no change for the Chopin; can you give an example where it
> helps?

In the Chopin, ragged-bottom is false so the difference can't really be seen.  
The piece isn't a good test case for how the patch changes engraving but it is 
an excellent test case for efficiency.

> 
> I still do not understand how creating separate SlurStubs helps.

Check out the regtest.  Also checkout les-nereides.ly (which is what the patch 
was created to fix (and does fix (why don't we have les-nereides.ly in the 
regtest comparison?))).

> At the
> time when we build system skylines, what information is in the stubs
> that is not also in the slurs themselves?

It is not that there is more information in the stubs - it's just different.  
They use an approximation of the vertical-skylines instead of the actual ones 
at a point in the code where skylines are needed but the actual ones cannot be 
used (to wit: Axis_group_interface::skyline_spacing).

If we calculated the actual control points for cross staff slurs at the time 
VerticalAxisGroup skylines were calculated, it would trigger a vertical 
alignment.  As an experiment, try removing the lines that weed out cross-staff 
grobs in Axis_group_interface::skyline_spacing and run all of the beam regtests 
- you'll see what I mean.

> Can you explain the need for
> stubs ?
> -- without using the word 'pure' because I do not know what you mean
> when you say that word -- you could mean 'with the boolean of that name
> set to true' in any one of dozens of functions.
> 

I need to use the word pure, but I'll use it judiciously :-)

SlurStubs run the control point calculations using not the actual heights and 
coordinates (which would trigger a vertical alignment) but rather pure heights 
and coordinates.  Checkout the changes to slur-scoring.cc: almost all of them 
are changing extent to maybe_pure_extent and stuff of that ilk.  In the case of 
stubs, this generates approximations of control points and therefore an 
approximation of the vertical skylines.  We feed this approximation into the 
vertical skyline calculations.

> 
> http://codereview.appspot.com/6498077/diff/4046/lily/axis-group-interface.cc
> File lily/axis-group-interface.cc (right):
> 
> http://codereview.appspot.com/6498077/diff/4046/lily/axis-group-interface.cc#newcode806
> lily/axis-group-interface.cc:806:
> Axis_group_interface::add_interior_skylines
> move this back up to line 616 for the sake of history
> 

ok

> http://codereview.appspot.com/6498077/diff/4046/lily/slur-engraver.cc
> File lily/slur-engraver.cc (right):
> 
> http://codereview.appspot.com/6498077/diff/4046/lily/slur-engraver.cc#newcode347
> lily/slur-engraver.cc:347: vector<Grob *>::const_iterator it =
> // Keep just one stub per Staff-like construct
> // Remove stubs if we have seen one already in the same vag,
> 
> ??  Do slurs that stay within a single staff produce stubs, and do we
> remove them here?

Ya.  There is no way to know if a slur will stay within a single staff until 
all its noteheads have been acknowledged.

http://codereview.appspot.com/6498077/