Re: Better pure height approximations for beamed rests. (issue4860043)

[reinhold . kainhofer]

I haven't actually tested it, but from reading the code it LGTM in
general. Please observe the comments, though.


http://codereview.appspot.com/4860043/diff/1/lily/rest.cc
File lily/rest.cc (right):

http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode239
lily/rest.cc:239: Interval y_ext (-line_count * ss / 2.0, line_count *
ss / 2.0);
Do you want to calculate the y-extents of the staff? Shouldn'tyou then
use (line_count-1)*ss, i.e. a staff with n lines only has n-1 staff
spaces...

http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode244
lily/rest.cc:244: Grob *stem = st;
Why do you need that second line at all? Shouldn't
  Grob *stem = unsmob_grob (..);
work?

http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode246
lily/rest.cc:246: return ly_interval2scm (ext);
Why don't you move that to the very top, before the variable definitions
of line_count, ss, y_ext and amount?

Then, everything needed for stemmed rests comes later and is placed
together in the source code.

http://codereview.appspot.com/4860043/diff/1/lily/rest.cc#newcode283
lily/rest.cc:283: amount = min (max (minmax (beamdir,
Stem::head_positions (left)[beamdir], Stem::head_positions
(right)[beamdir]) / 2.0, y_ext[DOWN]), y_ext[UP]);
Can you add some comments how you calculate this?
And this min(max(minmax(...))) is extremely hard to read.

http://codereview.appspot.com/4860043/