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

[address@hidden]

On 18 nov. 2012, at 00:55, address@hidden wrote:

> I haven't gotten so far to see the main point, yet.
> 

The main point is:

1) Unify all side spacing into one algorithm (this patch).  In addition to 
fixing several fingering bugs, this helps...
2) Eventually eliminate the translate axis call used to position outside_staff 
grobs, which will then help...
3) Do multiple-pass vertical spacing to help take things like cross-staff slurs 
and beams into account.


> 
> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc
> File lily/box-quarantine.cc (right):
> 
> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode24
> lily/box-quarantine.cc:24: Box_quarantine::Box_quarantine (Real padding,
> Axis a)
> So far it looks like this handles fingering only.  Other similar
> problems are handled with a XX_Collision object, so maybe just call this
> Fingering_Collision ?

Will do.

> 
> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode56
> lily/box-quarantine.cc:56: the beam cannot start.  it iterates through
> these boxes,
> Stray comment.  This routine is not used for anything to do with beams.
> 

Will remove.

> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode70
> lily/box-quarantine.cc:70: return fabs (ii0.index_ - mid) < fabs
> (ii1.index_ - mid);
> C provides subtraction on unsigned integers, so you can do
> hardware-oriented things like 0xFFFE - 3 => 0xFFEA  and have the result
> come out automatically as an unsigned type.
> 
> Here, index_ is unsigned, so the type of the result of the subtraction
> is unsigned, so sort_towards_middle() --- which is really only the
> comparison function 'closer_to_middle()' --- gives you not the order you
> intended.  For example, try
>  <c-1 d-2 e-3 f-4 g-5 a-6 b-7>2
> 
> However, I think performing the arrangement from the middle out does
> help in the algorithm below.  It seems to work just as well without the
> sort.
> 

This is a bit over my head - do you recommend I just remove the sorting?  If I 
do this, then I think the fingerings will fan up or down instead of fanning out 
from their middle?  But I could be wrong...

> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode86
> lily/box-quarantine.cc:86: scratch.intersect (boxes_to_quarantine_[i +
> 1]);
> If you look only at adjacent boxes, and work from middle out, then with
> boxes   A B C D E F G   you might push box B clear beyond A before even
> looking for an A-B collision.
> 
> Maybe you intended to widen by padding_, otherwise the resulting boxes
> do not get moved if they are closer to each other than padding_.
> 

I'll have to look at this more closely...basically, I want an algorithm that 
looks at how things are grouped and moves them around along one axis so that 
there are no more collisions while maintaining the general grouping.  I'm sure 
someone who knows something about CS can suggest something more efficient, but 
for now, this seems to work for all of the fingering regtests.

> http://codereview.appspot.com/6827072/diff/11002/lily/box-quarantine.cc#newcode102
> lily/box-quarantine.cc:102: while (dirty);
> Can be quite a long loop.  If you have three overlapping boxes A B C,
> you move B-A apart to clear, worsening the overlap B-C, then move B-C
> apart to clear, restoring most of the A-B overlap.  Convergence is in
> the style of Xeno's paradox, sped along somewhat by epsilon and
> padding.
> 
> Additional overlapping boxes D add additional nesting layers to the
> convergence process.

True.  I stand by what I say above - anyone who can see a better way to 
implement this can and should!  It's just a first step.

> 
> http://codereview.appspot.com/6827072/diff/11002/lily/script-engraver.cc
> File lily/script-engraver.cc (right):
> 
> http://codereview.appspot.com/6827072/diff/11002/lily/script-engraver.cc#newcode270
> lily/script-engraver.cc:270: // we never want Script grobs to be tucked
> down next to stems
> The patch changes  'chord-scripts.ly'  in a way inconsistent with this
> comment at the moment.
> 
> However, I can imagine that in dense music someone would want
> 'add-stem-support=##f for their Scripts, so that a staccato can tuck in
> next to a stem, much like we often do with Fingering in piano music.
> 

I'm still wondering the best way to implement this is.  For me, the cleanest 
thing to do is to either use or not use the NoteColumn as a support (see my 
previous response to you on this subject).  When used, there's no tucking, and 
when not used, there is.

My schedule is swamped so I'll have to put this patch to the side for 2-ish 
weeks, but I appreciate your responses and will try to chip away at it whenever 
I have time!

Cheers,
MS