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/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
