Re: beam.cc:pure-rest-collision-callback Place on staff lines; issue 2468 (issue 6035053)

[k-ohara5a5a]

The function being patched is the pure-alternative to a
chained-offset-callback to the original Y-position
callback for rests (Rest::y_offset_callback).
I cannot follow the data between C and Scheme well enough to understand
what this function should return if it needs to bail out early.

The problem with patch set 2 (included in 2.15.37) was that I returned
'prev_offset' on early returns.  Sometimes, however, 'prev_offset'
contains unrealistically large values, and returning them fools the
page-breaker (issue 2486).  I haven't yet found where the large values
come from.

Returning 0.0 seems to work most of the time, but caused minor problems
like that mentioned on the tracker
 << {g''8[ r8. g''16]}\\{r8 <g' a' b'>8. r16} >>

This patch returns 0.0 when it returns early, but avoids returning early
when it doesn't need to.


http://codereview.appspot.com/6035053/diff/16001/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/6035053/diff/16001/lily/beam.cc#newcode1323
lily/beam.cc:1323: return scm_from_double (0.0);
I'm confused.  There is a remaining problem with dots on
 << {g''8[ r8. g''16]}\\{r8 <g' a' b'>8. r16} >>
which I thought was caused by this code returning 0.0 on its early
returns.  It is a minor bug, but a regression since 2.14.

This is the pure-alternative to a chained-callback to the original
Y-position callback for rests, Rest::y_offset_callback

http://codereview.appspot.com/6035053/diff/16001/lily/beam.cc#newcode1328
lily/beam.cc:1328: return scm_from_double (0.0);
It would seem we do not want an early return in this case; if the beam
direction is not yet know, we would prefer to estimate the rest position
to returning the wrong value.
But this condition was added recently, with the fix for issue 774.

http://codereview.appspot.com/6035053/diff/21001/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/6035053/diff/21001/lily/beam.cc#newcode1323
lily/beam.cc:1323: return scm_from_double (0.0);
or possibly
   return scm_from_double (previous);

http://codereview.appspot.com/6035053/diff/21001/lily/beam.cc#newcode1327
lily/beam.cc:1327: return scm_from_double (0.0);
or possibly
   return scm_from_double (previous);

http://codereview.appspot.com/6035053/diff/21001/lily/beam.cc#newcode1359
lily/beam.cc:1359: return scm_from_double (0.0);
or possibly
   return scm_from_double (previous);

http://codereview.appspot.com/6035053/