Re: Uses single algorithm for side-position spacing. (issue 6827072)

[mike]

> Re: Uses single algorithm for side-position spacing. (issue 6827072)
> 
> {\clef bass
>  <g^3 a^5>2..->
>  << r16 \\ r \\ r  \\ r \\ >> eeses'16
>  \set fingeringOrientations = #'(right)
>   <c e>8-1-4 <c^1 e^4> <g,-3 b,-4> r
>   r2 }
> 

Beautiful ugly test case.  Even with current master it is atrocious. I've fixed 
everything but the double-flat on rest collision, which'd require a separate 
patch.

> 
> http://codereview.appspot.com/6827072/diff/1/input/regression/dynamics-avoid-cross-staff-stem.ly
> 
> File input/regression/dynamics-avoid-cross-staff-stem.ly (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/input/regression/dynamics-avoid-cross-staff-stem.ly#newcode14
> 
> input/regression/dynamics-avoid-cross-staff-stem.ly:14: a8 \p [ \change
> Staff = "PnRH" \stemDown gis'8 \fff ]
> Before your patch, the dynamics below the staff clear each other; after
> your patch, they collide.
> 

Fixed.

> 
> http://codereview.appspot.com/6827072/diff/1/lily/axis-group-interface.cc
> 
> File lily/axis-group-interface.cc (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/axis-group-interface.cc#newcode403
> 
> lily/axis-group-interface.cc:403: Axis_group_interface::cross_staff (SCM
> smob)
> For what situation?   Which object that supports axis-group-interface
> (PianoPedalSpanner, DynamicLineSpanner) should be potentially considered
> a cross-staff object?

NoteColumn

> http://codereview.appspot.com/6827072/diff/1/lily/box-quarantine.cc
> 
> File lily/box-quarantine.cc (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/box-quarantine.cc#newcode70
> 
> lily/box-quarantine.cc:70: return abs (ii0.index_ - mid) < abs
> (ii1.index_ - mid);
> gcc 4.5.1 complains that he cannot determine which version of the
> overloaded abs() should be called.  (Polymorphism is l33t, isn't it.)  I
> just removed the abs() calls to get it to compile, because index_ is an
> unsigned type so the arguments are always positive anyway.

index_ is always positive and mid_ is always positive but index_ - mid_ will be 
positive or negative depending on which one is larger. Changing to fabs.

> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/script-engraver.cc
> 
> File lily/script-engraver.cc (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/script-engraver.cc#newcode270
> 
> lily/script-engraver.cc:270: // we never want scripts to be tucked down
> next to stems
> This disagrees with   
> input/regression/finger-chords.ly

True - I mean Script grobs.  I'll make that clearer.

> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/stencil-integral.cc
> 
> File lily/stencil-integral.cc (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/lily/stencil-integral.cc#newcode988
> 
> lily/stencil-integral.cc:988: pure = Rest::has_interface (me) ? true :
> pure;
> Worse than losing ledgers, the 'pure' extent is in the wrong position
> for those rests that might have been moved.
> 
> At this point in determining the layout, shouldn't you ensure that the
> rests are positioned, by testing 'positioning-done, before figuring
> their skylines for use in note-spacing ?
> 

Even this doesn't work.  It fails maybe every 5th time I compile, so it must be 
one of those uniq () sorting problems that I run into from time to time.  The 
solution for now is just to use ly:grob::vertical-skylines-from-stencil on the 
rests, which don't muddle with extents or positioning and just check the glyph. 
 But this is me going into problem-avoidance mode.  There are comments all over 
that part of the code base warning about RestColumn positioning, but a big fat 
"THIS IS A TICKING CIRCULAR DEPENDENCY TIME BOMB" comment somewhere in there 
wouldn't hurt.

> 
> http://codereview.appspot.com/6827072/diff/1/scm/define-grob-properties.scm
> 
> File scm/define-grob-properties.scm (right):
> 
> 
> http://codereview.appspot.com/6827072/diff/1/scm/define-grob-properties.scm#newcode668
> 
> scm/define-grob-properties.scm:668: (other-axis-padding ,number? "The
> amount to pad the axis
> traditionally called horizon-padding, which I suppose makes sense in the
> metaphor of a city skyline, as this padding is in the direction of the
> horizon.
> 
> 

Fixed.

Thanks for the review!  Will upload a new patch set in the not-too-distant 
future.

Cheers,
MS