Re: Potential fix for issue 1504. (issue4237057)

[hanwenn]

lgtm


http://codereview.appspot.com/4237057/diff/13002/lily/beam.cc
File lily/beam.cc (right):

http://codereview.appspot.com/4237057/diff/13002/lily/beam.cc#newcode593
lily/beam.cc:593: Interval placements = robust_scm2interval
(me->get_property ("normalized-endpoints"), Interval (0.0,0.0));
space after ,

http://codereview.appspot.com/4237057/diff/13002/lily/beam.cc#newcode624
lily/beam.cc:624: weights[RIGHT] = 1 - multiplier;
weights.swap() ?

http://codereview.appspot.com/4237057/diff/13002/lily/beam.cc#newcode636
lily/beam.cc:636: + beam_dy * segments[extreme].vertical_count_;

how about

 int idx = {i, extreme}
 int translations[2]
 for (j = 0;j<2;j++)
   translations[i] = slope * (segments[idx[j]]  .. etc)

http://codereview.appspot.com/4237057/diff/13002/lily/spanner.cc
File lily/spanner.cc (right):

http://codereview.appspot.com/4237057/diff/13002/lily/spanner.cc#newcode406
lily/spanner.cc:406: SCM ret = scm_cons (scm_from_double (0.0),
scm_from_double (0.0));
i personally like 'result' better, almost as short, and a full word.

You could init to SCM_EOL instead?

http://codereview.appspot.com/4237057/diff/13002/lily/spanner.cc#newcode433
lily/spanner.cc:433: SCM t = ly_interval2scm (Interval
(unnormalized_endpoints[i][LEFT] / total_width,
unnormalized_endpoints[i][RIGHT] / total_width));
try using interval * 1/width (or 1/width * interval).  The operator
overloading for interval is hit and miss.  You can add a operator/ if
you want (separate commit)

http://codereview.appspot.com/4237057/