Re: Fixes bad slur heights by limiting fit_factor to the interior of slurs. (issue4810072)

[k-ohara5a5a]

LGTM.

I had time to try it on several scores; it often helped and never hurt.

> If any of the above is incorrect,
> then maybe consider adding some
> comment(s) where you define the variables,

Now, Mike created none of these variables, nor does his added code use
them.  If he adds helpful comments, great, but he has no more duty to
add comments than do the rest of us who reviewed the code.



http://codereview.appspot.com/4810072/diff/7001/input/regression/slur-height-capping.ly
File input/regression/slur-height-capping.ly (right):

http://codereview.appspot.com/4810072/diff/7001/input/regression/slur-height-capping.ly#newcode5
input/regression/slur-height-capping.ly:5: towards the edges of the
slurs.  said objects are thus ignored.
"said objects are thus ignored" confuses me.
You should say something to let us easily check if the test passes;
maybe "the two slurs should be similar in shape"

http://codereview.appspot.com/4810072/diff/7001/input/regression/slur-height-capping.ly#newcode10
input/regression/slur-height-capping.ly:10: \relative c' {
Even after the bug-fix, this output looks odd enough that people might
waste time wondering if something is wrong.
I'd drop it; the second test is enough.

http://codereview.appspot.com/4810072/diff/7001/lily/slur-configuration.cc
File lily/slur-configuration.cc (right):

http://codereview.appspot.com/4810072/diff/7001/lily/slur-configuration.cc#newcode105
lily/slur-configuration.cc:105: if ((pext.is_empty ()
You might move your code, and
  if (close_to_edge) continue;
up to line 94, because your test is conceptually independent of the pext
computation.

http://codereview.appspot.com/4810072/