lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Make sure slurs actually avoid stafflines. (issue 15400049)


From: k-ohara5a5a
Subject: Re: Make sure slurs actually avoid stafflines. (issue 15400049)
Date: Wed, 23 Oct 2013 04:22:23 +0000


https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc
File lily/slur-configuration.cc (left):

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#oldcode62
lily/slur-configuration.cc:62:
but the function was broken: it only moved slurs in one direction

The commit message makes it sound like there was a blunder in the
direction that the old code moved the slur.  I don't see any such
problem.

It looks like the old code intentionally moved the middle of the slur
always in the direction of the slur, so the curvature would increase.
It simply didn't move far enough, because the curve does not move as far
as the control points.

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc
File lily/slur-configuration.cc (right):

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode60
lily/slur-configuration.cc:60: // rather than decrease, because we want
to avoid too flat slurs.
I can't find that feature in the implementation.
In fact, the required clearance is greater outside the slur than inside,
so you flatten more than your fatten.

Maybe figure how far you would have to move in each direction, in order
to get the required gap, and choose 'which_side' depending on which
motion is smaller.

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode69
lily/slur-configuration.cc:69: // Get the point(s) where the curve is
horizontal (i.e. the belly):
For slurs on a slope, the spot where the slur is horizontal might not
look like its belly.  I would just call it "the horizontal part"

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode84
lily/slur-configuration.cc:84: Real const slur_th = state.thickness_ *
staff_th * 10;
Why 10?  Is that the thickness of the fattest part of the slur?

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode93
lily/slur-configuration.cc:93: // TODO: to handle weird staves well, we
should round to staffline positions.
Staves with an even number of lines are fairly common.  The old code
found the nearest staff-position, as opposed to staff-line, and moved
the slur if there was a staff-line drawn there and the slur would be too
close. Maybe restore that aspect of the old code.

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode99
lily/slur-configuration.cc:99: && Staff_symbol_referencer::on_staff_line
(staff, int (2 * round)))
'round' has a sign-change depending on the slur direction, so it is not
right for passing to on_staff_line().

Maybe keep the Real quantities with signs meaning UP/DOWN and use only
'which_side' to keep track of whether the nearest potential staff-line
is inside or outside the slur.

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode114
lily/slur-configuration.cc:114: b = P0*(1-t)^3 + P2*t*(1-t)^2 +
P3*(1-t)*t^2 + P4*t^3
Below, you number them P0 P1 P2 P3

https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#newcode121
lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3
* (t - (t * t)));
This could still move the control points quite a lot, and make a
funny-shaped slur, if the horizontal part is near one end of the slur.
Then t=0 or t=1 nearly.

Maybe try first to move only the middle control points, but limit their
motion to half a staff space
mid_pts_cor = min(correction/(3t-3t²)*state.dir_, 0.5) *state.dir_
all_pts_cor = correction - mid_pts_cor * (3t-3t²)

https://codereview.appspot.com/15400049/diff/1/lily/slur.cc
File lily/slur.cc (right):

https://codereview.appspot.com/15400049/diff/1/lily/slur.cc#newcode520
lily/slur.cc:520: "Minimum clearance to staffline above slur belly.\n"
"... to the staffline outside the slur"
not necessarily above.

https://codereview.appspot.com/15400049/diff/1/lily/slur.cc#newcode522
lily/slur.cc:522: "Minimum clearance to staffline below slur belly.\n"
"... to the staffline inside the slur"

https://codereview.appspot.com/15400049/

reply via email to

[Prev in Thread] Current Thread [Next in Thread]