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

[mtsolo]

On 2011/08/05 01:50:15, hanwenn wrote:
> LGTM

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

> > Instead of defining the middle of curve in terms of curve length, I
use
> > a constant number here. &nbsp;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,
> > &nbsp; &nbsp; &nbsp; Real eps = 0.01;
> > &nbsp; &nbsp; &nbsp; Interval pext = eps * Interval (-1, 1) +
p[X_AXIS];
> > &nbsp; &nbsp; &nbsp; pext.intersect (curve_xext);
> > - &nbsp; &nbsp; &nbsp;if (pext.is_empty () || pext.length () <=
1.999 * eps)
> > + &nbsp; &nbsp; &nbsp;if ((pext.is_empty ()
> > + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;|| pext.length () <= 1.999 * eps
> > + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;|| (p[X_AXIS] -
curve_xext[LEFT]) &nbsp;< 2.5
> > + &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;|| (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 - mailto:address@hidden -
http://www.xs4all.nl/%7Ehanwen

New patch w/ Han-Wen's suggestions & a regtest online.

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