Sketch for DotColumn not triggering vertical alignment.

Sketch for DotColumn not triggering vertical alignment.

[lemzwerg, 2012-01-12 11:02:13]

LGTM, but not time to test...

http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc
File lily/staff-symbol-referencer.cc (right):

http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc#...
lily/staff-symbol-referencer.cc:95: Real y = (pure
The outermost parentheses have no effect.  I suggest to remove them.

[Carl, 2012-01-13 01:09:34]

A few comments.

Thanks,

Carl

http://codereview.appspot.com/5538049/diff/1/input/regression/dot-column-vert...
File input/regression/dot-column-vertical-positioning.ly (right):

http://codereview.appspot.com/5538049/diff/1/input/regression/dot-column-vert...
input/regression/dot-column-vertical-positioning.ly:5: line breaking.
What should the output look like?  How do I know if the regtest passes?

http://codereview.appspot.com/5538049/diff/1/lily/include/staff-symbol-refere...
File lily/include/staff-symbol-referencer.hh (right):

http://codereview.appspot.com/5538049/diff/1/lily/include/staff-symbol-refere...
lily/include/staff-symbol-referencer.hh:37: static void internal_set_position
(Grob *, Real, bool);
shouldn't internal_set_position be private, and be called by pure_set_position
and set_position, which are the public functions?

http://codereview.appspot.com/5538049/diff/1/lily/include/staff-symbol-refere...
lily/include/staff-symbol-referencer.hh:52: static Real internal_get_position
(Grob *, bool);
Ditto here -- I think internal_get_postion should be private.

[Keith, 2012-01-15 21:45:27]

LGTM.

Now I see why this bug appeared.  The recent changes to the beam configuration
code need to know the note-spacing, which depends on the horizontal dot
positions. So, dot configuration may not wait for beaming.

The code configuring dots was (indirectly and wrongly) causing rest-collision to
run, which (in rare cases) needs to know the beam configuration so as to avoid a
stem.

If I get the point, a 'pure' (read "before line-breaking") estimate of rest
position is always sufficient for purposes of dot configuration. (Code comment
would be nice.)

If so, dot-column may run on rests before rest-collision sets their final
positions.  This would be good, because then rest-collision would be free to
avoid beams and stems. (Comments in that code warn us to "try not to look at the
stem".)

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc
File lily/dot-column.cc (right):

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc#newcode216
lily/dot-column.cc:216: // do the X-offset properly.
Might this comment now be obsolete?  It is about the fix to issue 1088.  If so,
we could unravel some of this complication, later.

[MikeSol, 2012-01-16 07:16:54]

http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc
File lily/staff-symbol-referencer.cc (right):

http://codereview.appspot.com/5538049/diff/1/lily/staff-symbol-referencer.cc#...
lily/staff-symbol-referencer.cc:95: Real y = (pure
On 2012/01/12 11:02:13, lemzwerg wrote:
> The outermost parentheses have no effect.  I suggest to remove them.

They're for code alignment purposes: it's so that the ? and : don't align
vertically with the -.  There are a couple other places in the code that use
this convention (forget where).

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc
File lily/dot-column.cc (right):

http://codereview.appspot.com/5538049/diff/4002/lily/dot-column.cc#newcode216
lily/dot-column.cc:216: // do the X-offset properly.
On 2012/01/15 21:45:27, Keith wrote:
> Might this comment now be obsolete?  It is about the fix to issue 1088.  If
so,
> we could unravel some of this complication, later.

It very well could be.  After this patch is pushed, remind me to investigate
this (I'm writing myself a reminder as well).