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

[Han-Wen Nienhuys]

LGTM

On Thu, Aug 4, 2011 at 9:20 PM,  <address@hidden> wrote:

> Instead of defining the middle of curve in terms of curve length, I use
> a constant number here.  It seems to do the trick better than a variable
> length.



8fdea86edb73b0a1fb4829e0a4fcf39932828a8f..083fc5935df3b626ea9a13a7207354f8497a13a2
> 100644
> --- a/lily/slur-configuration.cc
> +++ b/lily/slur-configuration.cc
> @@ -95,7 +95,10 @@ fit_factor (Offset dz_unit, Offset dz_perp,
>       Real eps = 0.01;
>       Interval pext = eps * Interval (-1, 1) + p[X_AXIS];
>       pext.intersect (curve_xext);
> -      if (pext.is_empty () || pext.length () <= 1.999 * eps)
> +      if ((pext.is_empty ()
> +          || pext.length () <= 1.999 * eps
> +          || (p[X_AXIS] - curve_xext[LEFT])  < 2.5
> +          || (curve_xext[RIGHT] - p[X_AXIS]) < 2.5))

Basic logic seems sane.  You may adopt a todo to make the 2.5
parameterized; whether or not the point is 'far' from the edge depends
on the shape and length of the curve, so it could theoretically be
calculated from a scale-free parameter.

Two requests:

* add a bool close_to_edge to make the condition easier to read.

* add a parameter rather than hardcoding.  I know we have too many
parameters, but given that this uses an arbitrary number 2.5,  it's
better to have that number together with other numbers.

-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen