Re: Better pure heights for slurs (issue 5431065)

[k-ohara5a5a]

I like the direction of this change.

The code has your trademark incomprehensibility, so I don't know if it
does what you want based on your introduction.  Maybe just decide that
you want what it does.


http://codereview.appspot.com/5431065/diff/1/lily/slur.cc
File lily/slur.cc (right):

http://codereview.appspot.com/5431065/diff/1/lily/slur.cc#newcode102
lily/slur.cc:102: height *= 0.5;
Not too harmful here, but in general redefining variables with
meaningful names causes other programmers to scan lines 77 through 101
for use of the original height, and then lines 103 through 118 for the
new use of height, to try to determine why it changed halfway through.

If the conceptual "height" didn't change, just leave 'height' alone.

Oh, the concept is actually "height-limit". Maybe the original variable
name wasn't perfect.  Since ret.length() is also an estimate of the
actual height of the slur, I'd take the time to change the variable
names.

http://codereview.appspot.com/5431065/diff/1/lily/slur.cc#newcode107
lily/slur.cc:107: between two columns
Trop de mots,
und ein, dass ich nicht in meinem Wörterbuch gefunden haben.

http://codereview.appspot.com/5431065/diff/1/lily/slur.cc#newcode115
lily/slur.cc:115: ret[dir] += dir * (sqrt (height * height / ((s * s) +
1)) + 0.5);
Reduces to abs(0.5*height_limit) / sqrt(s²+1).
Simplifying the math, it looks like the result is nearly this:
 Real encompassing_height = max(1.0, ret.length());
 // Coarse estimate of how far the slur will bow upwards
 ret[dir] += dir * no_elts * height_limit / encompassing_height;
 ret += 0.5 * dir;

http://codereview.appspot.com/5431065/