note-collison.cc: Scale shifts by width of note at left; issue 1713

note-collison.cc: Shift based on actual note-extents; issue 1713

[Keith, 2012-05-06 00:20:16]

http://codereview.appspot.com/6189048/diff/1/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/1/lily/note-collision.cc#newcode543
lily/note-collision.cc:543: ->get_property ("stem-nest"), 0.2);
adjustable property requested in issue 2192

http://codereview.appspot.com/6189048/diff/1/lily/note-collision.cc#newcode548
lily/note-collision.cc:548: Interval oppwidth = clash_groups[d][0]
oops,   clash_groups[-d][0]

but this is just a preview of future plans

http://codereview.appspot.com/6189048/diff/2001/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/2001/lily/note-collision.cc#newcod...
lily/note-collision.cc:298: If the downstem note is being moved right, however,
its motion needs ot be
oops, "... to be".  fixed in my copy.

Later, I plan to straighten out the multiplies as in Patch Set 1, so this
comment will no longer be needed.  But that is for a later commit.

[Keith, 2012-05-09 06:02:48]

http://codereview.appspot.com/6189048/diff/10001/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/10001/lily/note-collision.cc#newco...
lily/note-collision.cc:303: shift_amount *= (extent_up[RIGHT] -
extent_down[LEFT]) / extent_down.length ();
For the issue 1546 example, the up-stem note is a breve, which has one side-bar
left of the reference point, and has its head and the other side-bar to the
right of the reference point.  To clear it, we need to shift over the stuff on
the right, not quite the full length of the breve.

[janek, 2012-05-11 00:21:54]

http://codereview.appspot.com/6189048/diff/10001/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/10001/lily/note-collision.cc#newco...
lily/note-collision.cc:301: of the note heads on the sides that interfere. */
So, should the offsets really be multiplied?  I'd rather think not.

http://codereview.appspot.com/6189048/diff/10001/lily/note-collision.cc#newco...
lily/note-collision.cc:303: shift_amount *= (extent_up[RIGHT] -
extent_down[LEFT]) / extent_down.length ();
The interesting thing is that the order of voices matters:

<< 
  \override Staff.NoteHead #'style = #'altdefault
  { c''\breve*1/2 b'1 }
  \\
  { b'1 c''\breve*1/2 }
>>

the placement of the notes should be the same in both measures, but it isn't. 
Moreover, it's the opposite of current master.

[Keith, 2012-05-11 03:46:12]

On Thu, 10 May 2012 17:21:54 -0700, <janek.lilypond@gmail.com> wrote:

> The interesting thing is that the order of voices matters:
> [....]
> the placement of the notes should be the same in both measures,but it isn't.

The order of voices produces different placement for half notes.

<<
   \override Staff.NoteHead #'style = #'altdefault
   { c''\breve*1/2 b'1 c''2 b' }
   \\
   { b'1 c''\breve*1/2 b'2 c'' } >>

Do we need another special case for breves ?

[janek, 2012-05-15 06:48:09]

LGTM
I apologize that you had to wait.  Thank you for your work!!

http://codereview.appspot.com/6189048/diff/9003/input/regression/collision-wh...
File input/regression/collision-whole.ly (right):

http://codereview.appspot.com/6189048/diff/9003/input/regression/collision-wh...
input/regression/collision-whole.ly:16: }
I suggest to use this as regression test:

\paper { indent = 0 }

\markup "collisions - quarter note in upper voice"

\new Staff << 
  { \repeat unfold 12 { b'4*4 } }
  \\
  { 
    \override Staff.NoteHead #'style = #'altdefault
    c''4*4        b' a'
    c''2*2        b' a' 
    c''1          b' a' 
    c''\breve*1/2 b' a' 
  }
>>

\markup "collisions - half note in upper voice"

\new Staff << 
  { \repeat unfold 12 { b'2*2 } }
  \\
  { 
    \override Staff.NoteHead #'style = #'altdefault
    c''4*4        b' a'
    c''2*2        b' a' 
    c''1          b' a' 
    c''\breve*1/2 b' a' 
  }
>>

\markup "collisions - whole note in upper voice"

\new Staff << 
  { \repeat unfold 12 { b'1 } }
  \\
  { 
    \override Staff.NoteHead #'style = #'altdefault
    c''4*4        b' a'
    c''2*2        b' a' 
    c''1          b' a' 
    c''\breve*1/2 b' a' 
  }
>>

\markup "collisions - breve note in upper voice"

\new Staff << 
  { 
    \override Staff.NoteHead #'style = #'altdefault
    \repeat unfold 12 { b'\breve*1/2 } 
  }
  \\
  { 
    \override Staff.NoteHead #'style = #'altdefault
    c''4*4        b' a'
    c''2*2        b' a' 
    c''1          b' a' 
    c''\breve*1/2 b' a' 
  }
>>

http://codereview.appspot.com/6189048/diff/9003/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/9003/lily/note-collision.cc#newcod...
lily/note-collision.cc:181: && (up_ball_type <= 0 || down_ball_type <= 0));
Excellent solution!

I was wondering "how the hell Lily can know which noteheads are rectangular
(non-oval)" - your solution is great!  In the future we may want to use some
shape recognition algorithms (or simply adapt Mike's skylines for use in
horizontal spacing), but for now this is perfect.

But i think the comment should read "for breves and longer"?
Actually, i would rephrase that comment to
"breves and longer notes are rectangular in shape, so if they are present we
shift columns as in full collision".

http://codereview.appspot.com/6189048/diff/9003/lily/note-collision.cc#newcod...
lily/note-collision.cc:301: in calc_positioning_done(), by the width of the
downstem note.
Why don't we get gid of that multiplication?  It seems to be complicating things
and happens in an unexpected (by me at least) place - is there a reason for it?

[Keith, 2012-05-15 07:31:09]

http://codereview.appspot.com/6189048/diff/9003/lily/note-collision.cc
File lily/note-collision.cc (right):

http://codereview.appspot.com/6189048/diff/9003/lily/note-collision.cc#newcod...
lily/note-collision.cc:301: in calc_positioning_done(), by the width of the
downstem note.
On 2012/05/15 06:48:09, janek wrote:
> Why don't we get gid of that multiplication?  

That is my plan.  See Patch Set 1 for a preview.  All note-collision spacing is
scaled by the first down-stem note, so each path through the collision code
needs to use a smarter replacement width, before that multiplication can go.

[janek, 2012-05-15 17:26:51]

On 2012/05/15 07:31:09, Keith wrote:
> On 2012/05/15 06:48:09, janek wrote:
> > Why don't we get gid of that multiplication?  
> 
> That is my plan.  See Patch Set 1 for a preview.  All note-collision spacing
is
> scaled by the first down-stem note, so each path through the collision code
> needs to use a smarter replacement width, before that multiplication can go.

Ah, i missed it, sorry.  I've also missed the discussion in tracker, but i'm
catching up now.
Good to see that you are dealing with this thoroughly!

thanks,
Janek