Removes kludges in stencil-based skyline code.

We used to create infinite skylines for certain corner cases like SystemStartBrackets (horizontal) and BassFigurePositioning (vertical).

We no longer do this by making all SystemStartX cross-staff, which makes their horizontal skylines extend infinitely in either direction.  This is conceptually what we want, as we don't want them slipping to the right of any objects, and as we can't estimate their actual height until way downstream, this doesn't sacrifice any spacing precision.

For BassFigurePositioning, we just give X-extents to BassFigureAlignment and BassFigureLine as well as BassFigurePositioning using ly:axis-group-interface::width.  This way, correct skyline positioning kicks in.

[Keith, 2013-03-16 03:26:58]

good enough

https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):

https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm#newcode312
scm/define-grobs.scm:312: (vertical-skylines .
,grob::simple-vertical-skylines-from-y-extent)
I'm confused, because the fall-back for vertical-skylines seems to do exactly
the same thing for Spanners, and it looks like this is a Spanner.

https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm#newcode2237
scm/define-grobs.scm:2237: (horizontal-skylines .
,grob::simple-horizontal-skylines-from-x-extent)
The fall-back would be the same if this Grob were marked cross-staff = #t  
Would it be more smarter to mark this and similar grobs cross-staff ?

[mike7, 2013-03-16 07:36:38]

On 16 mars 2013, at 04:26, k-ohara5a5a@oco.net wrote:

> good enough
> 
> 
> https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm
> File scm/define-grobs.scm (right):
> 
> https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm#newcode312
> scm/define-grobs.scm:312: (vertical-skylines .
> ,grob::simple-vertical-skylines-from-y-extent)
> I'm confused, because the fall-back for vertical-skylines seems to do
> exactly the same thing for Spanners, and it looks like this is a
> Spanner.
> 

That's only in the pure equivalent.  The unpure function doesn't have this.

>
https://codereview.appspot.com/7516048/diff/1/scm/define-grobs.scm#newcode2237
> scm/define-grobs.scm:2237: (horizontal-skylines .
> ,grob::simple-horizontal-skylines-from-x-extent)
> The fall-back would be the same if this Grob were marked cross-staff =
> #t   Would it be more smarter to mark this and similar grobs cross-staff
> ?

Running regtests.

Cheers,
MS

[MikeSol, 2013-03-16 08:00:04]

Response to Keith's comments

[MikeSol, 2013-03-17 08:01:45]

Removes evil code from C++, replaces with happy defaults in Scheme

[janek, 2013-03-19 11:49:12]

Mike,

i've read the patch 3 times and i don't understand how it solves the problem.

I see that you remove some weird c++ code, but why can you do this?
Also, why marking some grobs as cross-staff helps with solving issue 3242, which
is about manual beams over rests?
What figured bass has to do with this at all?

> Creates more possibilities for vertical and horizontal skyline functions.

I think this sentence applied to first version of the patch; it seems unrelated
to current patchset.

I'm sorry to say this, but i think this issue is unreviewable :/

[MikeSol, 2013-03-19 14:39:40]

Adds comments

[mike7, 2013-03-19 14:41:01]

On 19 mars 2013, at 12:49, janek.lilypond@gmail.com wrote:

> Mike,
> 
> i've read the patch 3 times and i don't understand how it solves the
> problem.
> 
> I see that you remove some weird c++ code, but why can you do this?
> Also, why marking some grobs as cross-staff helps with solving issue
> 3242, which is about manual beams over rests?
> What figured bass has to do with this at all?
> 
>> Creates more possibilities for vertical and horizontal skyline
> functions.
> 
> I think this sentence applied to first version of the patch; it seems
> unrelated to current patchset.
> 
> I'm sorry to say this, but i think this issue is unreviewable :/

Thanks for the feedback!  I've changed the message on Rietveld to be more in
line with the current patch - you're right that it's bounced around quite a bit.
 I've also added a couple comments.

Cheers,
MS

[janek, 2013-03-19 21:57:10]

LGTM :)

[Keith, 2013-03-30 04:52:12]

https://codereview.appspot.com/7516048/diff/13001/lily/side-position-interfac...
File lily/side-position-interface.cc (left):

https://codereview.appspot.com/7516048/diff/13001/lily/side-position-interfac...
lily/side-position-interface.cc:317: // this seems kinda kludgy, as there is no
apparent logic to it
Probably not so kludgy if we requested staff-padding on something placed in a
Dynamics context.

I suggest that in  if (include_staff) {} above we make staff_extents at least
the interval (0 . 0)