Triggers X-extent calculation for NoteColumn before setting stem-begin-position

Triggers X-extent calculation for NoteColumn before setting stem-begin-position

[Keith, 2012-03-29 04:37:37]

LGTM,
but if the solution is to avoid looking at the stem extent, then there is
another case to solve with tremolo flags
 << { \aikenHeads f'4:32 }  \\ { \aikenHeads f' } >>
and then beams
 << { \aikenHeads e'8 f' s4 }  \\ { \aikenHeads e'8 f' s4 } >>
...

http://codereview.appspot.com/5934050/diff/1/input/regression/flag-stem-begin...
File input/regression/flag-stem-begin-position.ly (right):

http://codereview.appspot.com/5934050/diff/1/input/regression/flag-stem-begin...
input/regression/flag-stem-begin-position.ly:8: << { \aikenHeads f'8 }  \\ {
\aikenHeads f'8 } >>
We already have a very similar ‘collision-head-solfa-fa.ly’

[MikeSol, 2012-03-29 06:27:14]

There is a nasty bug in LilyPond that this patch only exacerbates - as a rule of
thumb, X positioning functions should not set Y positioning variables and vice
versa.  note-collision.cc, which works from an X_AXIS callback, sets the stem
attachment, which is a both X and Y axis value.  This is a recipe for trouble -
there even if it means logic / code dup, these should be separated out into two
functions: one that sets the X value and one that sets the Y value.  As a
result, I have to trigger the X extent in Stem::internal_stem_begin_position to
get the Y position set correctly.  It may be worth it to fix this now: it'd
delay 2.16 further, but it seems like a sound architecture decision that'd
prevent this sorta problem from happening in the future.

[Trevor Daniels, 2012-03-29 09:25:00]

Mike, I don't understand how the commit message and title relate to this change,
probably due to some misunderstanding on my part.  How does this create a pure
conversion for Flag::calc_y_offset?
Trevor

[mike_apollinemike.com, 2012-03-29 09:26:04]

On Mar 29, 2012, at 11:25 AM, tdanielsmusic@googlemail.com wrote:

> Mike, I don't understand how the commit message and title relate to this
> change, probably due to some misunderstanding on my part.  How does this
> create a pure conversion for Flag::calc_y_offset?
> Trevor
> 

Sorry...forgot to change the title.  That was what the first patch set did.
Will fix.

Cheers,
MS

[Keith, 2012-04-02 07:42:55]

Works good for me.

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc#newcode131
lily/stem.cc:131: Real stem_beg = internal_calc_stem_begin_position (me, false);
I'm assuming something within calc_stem_begin_position (me, calc_beam=false)
triggers note-collision resolution before setting the stem-begin position.

[MikeSol, 2012-04-02 07:48:11]

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc#newcode131
lily/stem.cc:131: Real stem_beg = internal_calc_stem_begin_position (me, false);
On 2012/04/02 07:42:55, Keith wrote:
> I'm assuming something within calc_stem_begin_position (me, calc_beam=false)
> triggers note-collision resolution before setting the stem-begin position.

Yup.  It allows calc_stem_position to skip all of the beam business and get to
the setting of note positions.

[Keith, 2012-04-04 04:02:47]

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/5934050/diff/7001/lily/stem.cc#newcode131
lily/stem.cc:131: Real stem_beg = internal_calc_stem_begin_position (me, false);
On 2012/04/02 07:48:11, MikeSol wrote:
> On 2012/04/02 07:42:55, Keith wrote:
> > I'm assuming something within calc_stem_begin_position 
> > triggers note-collision resolution 

That was nonsense.  Note-collision resolution happens in the "Preprocessing
graphical objects..." phase, which completes for all three scores in your
regtest before the "Fitting music on 1 page..." phase, which is when we pass the
code with the comment //trigger note collision 

> Yup.  It allows calc_stem_position to skip all of the beam business and get
to
> the setting of note positions.

You must mean something like "get to the business of /looking up/ stem
attachment points" that were set long ago.

Could you change the comment and commit message to whatever you thought I meant
when you said "Yup", because what I thought it meant isn't true.