Issue 15400049: Make sure slurs actually avoid stafflines.
|
Created:
11 years, 6 months ago by janek Modified:
11 years, 5 months ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionMake sure slurs actually avoid stafflines.
Until now, slurs were pretending that they were avoiding stafflines,
but the function was broken: it only moved slurs in one direction
(sometimes making the collision signifincantly worse).
This commit makes sure that the correction is applied in appropriate
direction and that slur thickness is taken into account. Additionally,
the amount of correction is now user-settable.
Patch Set 1 #
Total comments: 27
Patch Set 2 : fix the problems - sort of.. #
Total comments: 6
MessagesTotal messages: 15
LGTM! Thanks for working on this
Sign in to reply to this message.
LGTM, at least by inspection (I can't make at present) Understanding this complex area is impressive! Trevor
Sign in to reply to this message.
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#old... 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#new... 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#new... 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#new... 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#new... 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#new... 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#new... 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#new... 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"
Sign in to reply to this message.
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#new... lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3 * (t - (t * t))); this new code looks considerably more heuristic compared to the old one (it's full of random constants such as 0.6, 0.4, 3, etc.), and will be much more difficult to make sense of if anybody finds a problem with it in the future. Why not fix the problem that Keith pointed out exactly?
Sign in to reply to this message.
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#new... lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3 * (t - (t * t))); It is not too bad, and we just started review. 0.6 is one empirical constant from which 0.4 = 1.0 - 0.6 follows, 3 comes from the definition of cubic Bezier curves, etc.
Sign in to reply to this message.
On 2013/10/23 07:16:36, Keith wrote: > 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#new... > lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3 * (t - > (t * t))); > It is not too bad, and we just started review. > 0.6 is one empirical constant from which 0.4 = 1.0 - 0.6 follows, > 3 comes from the definition of cubic Bezier curves, etc. If we have a Bezier with start and end point at height 0 and control points at height 1, I get 0.75 for the height in the middle. Giving everything a slope does not change the value for t=0.5, but it's no longer the top of the curve then. It may be that 0.75 is some sort of upper limit on how for we may get the maximum to go from the direct line.
Sign in to reply to this message.
On 2013/10/23 04:22:22, Keith wrote: > 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#old... > 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. Uh, how had you came to this conclusion? It's absurd. Take this example: \relative c'' { c8( b a2) c8( b) \slurUp d,2( f) a2 c } Here's how these slurs look with the correcting function turned off altogether - none is crossing the staffline: http://lilypond.googlecode.com/issues/attachment?aid=36280006000&name=no-corr... They are just a bit too close to the staffline, that's all. They should be flattened, but the previous version of the algorithm decided that such slurs should have their curvature increased, which resulted in this: http://lilypond.googlecode.com/issues/attachment?aid=36280006001&name=previou... Can you tell me in what way this made any sense? Making the slurs even more curved would make them even more ugly. Also, please look at the example in our essay: http://lilypond.org/doc/v2.17/Documentation/essay/automated-engraving#improve... Note that Baerenreiter puts the slur in the 2nd measure under the staffline, just as i want it to be. On to the inline comments.
Sign in to reply to this message.
Thanks for the review. I'll post a new patch tomorrow. Janek 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#new... lily/slur-configuration.cc:60: // rather than decrease, because we want to avoid too flat slurs. On 2013/10/23 04:22:23, Keith wrote: > 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. ?? This patch increases curvature of all slurs that would by default have the belly above a staffline (so that they are at least min_gap_above_staffline away from the staffline), and decreases curvature of all slurs that would by default have the belly below a staffline (so that they are at least min_gap_below_staffline away) Precisely because min_gap_above_staffline > min_gap_below_staffline we are more likely to increase than decrease curvature. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:84: Real const slur_th = state.thickness_ * staff_th * 10; On 2013/10/23 04:22:23, Keith wrote: > Why 10? Is that the thickness of the fattest part of the slur? No, it's because staff_th is measured in staffspaces, while state.thickness_ appears to be measured in 1/10ths of staffline-thicknesses (i'm sure that it's not the real thickness of the slur). To get real slur thickness, we have to multiply these values, but not when they are measured in differrent units. For example, in \new Staff \with { \override StaffSymbol thickness = #5 } \relative c'' { a2( d) } state.thickness_ reports to be 0.12, and staff_th is 0.5. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:93: // TODO: to handle weird staves well, we should round to staffline positions. On 2013/10/23 04:22:23, Keith wrote: > 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. That would be problematic. Imagine a slur where norm_y = n+0.26 staffspaces, where n is an integer. I.e, the clearance between belly of the curve and staffline is 0.26ss - (slur thickness + staff thickness)/2, i.e. most likely 0.15ss or less. We want to correct such slurs, but if we rounded to positions, we would not be measuring this distance (because n+0.26 ss would get rounded to n+0.5 ss). If we want to support non-default staves, we need to write a function round_to_staffline. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:99: && Staff_symbol_referencer::on_staff_line (staff, int (2 * round))) On 2013/10/23 04:22:23, Keith wrote: > 'round' has a sign-change depending on the slur direction, so it is not right > for passing to on_staff_line(). Good catch! > 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. I'll see what i can do. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:114: b = P0*(1-t)^3 + P2*t*(1-t)^2 + P3*(1-t)*t^2 + P4*t^3 On 2013/10/23 04:22:23, Keith wrote: > Below, you number them P0 P1 P2 P3 oops, sorry! https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3 * (t - (t * t))); On 2013/10/23 04:22:23, Keith wrote: > 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. Indeed, i've encountered this problem today. > 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²) I would prefer an exact solution. Actually, i think that we should fix this by handling the slur more completely, i.e. by including slur-end staffline avoidance in this function and deciding on the slur based on its whole configuration. But implementing this will take time... https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:121: Real mid_pts_cor = 0.6 * correction / (3 * (t - (t * t))); On 2013/10/23 07:16:37, Keith wrote: > It is not too bad, and we just started review. > 0.6 is one empirical constant from which 0.4 = 1.0 - 0.6 follows, > 3 comes from the definition of cubic Bezier curves, etc. Indeed, it's like you wrote. 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" On 2013/10/23 04:22:23, Keith wrote: > "... to the staffline outside the slur" > not necessarily above. Done. https://codereview.appspot.com/15400049/diff/1/lily/slur.cc#newcode522 lily/slur.cc:522: "Minimum clearance to staffline below slur belly.\n" On 2013/10/23 04:22:23, Keith wrote: > "... to the staffline inside the slur" Done.
Sign in to reply to this message.
Hi Keith, 2013/10/23 <janek.lilypond@gmail.com>: > > On 2013/10/23 04:22:22, Keith wrote: >> 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. > > Uh, how had you came to this conclusion? It's absurd. Take this > example: > > \relative c'' { > c8( b a2) c8( b) > \slurUp > d,2( f) > a2 c > } > > Here's how these slurs look with the correcting function turned off > altogether - none is crossing the staffline: > http://lilypond.googlecode.com/issues/attachment?aid=36280006000&name=no-corr... > > They are just a bit too close to the staffline, that's all. They should > be flattened, but the previous version of the algorithm decided that > such slurs should have their curvature increased, which resulted in > this: > http://lilypond.googlecode.com/issues/attachment?aid=36280006001&name=previou... > > Can you tell me in what way this made any sense? Making the slurs even > more curved would make them even more ugly. > > Also, please look at the example in our essay: > http://lilypond.org/doc/v2.17/Documentation/essay/automated-engraving#improve... > Note that Baerenreiter puts the slur in the 2nd measure under the > staffline, just as i want it to be. I hope that you won't interpret the harsh tone of my email as a personal attack. I spent *8 hours* (measured with a watch) reading slur code and working on this patch. I got irritated when i saw your email, because it seemed to me that when i said that something was broken you didn't trust my words enough to double-check your doubts. I realize that you probably didn't mean it. Unfortunately this resonated with some of my other experiences from LilyPond development, when a) my questions quite often didn't get answered desipte being short and well-formulated, b) my plain, well-commented and carefully git-crafted patches sometimes don't get reviewed for months, c) my diagnoses and observations are quite often not trusted without a lot of disussion and necessity to provide lots of scanned examples, d) it seems that people generally don't care or notice things that i care about and notice (for example when i say "many slurs, beams and ties in LilyPond are really ugly" many people don't believe me - as if they trusted LilyPond's decisions more than my judgement - or at least so it seems to me). This is all really unfortunate; of course you're not responsible for this situation. But it made me frustrated nevertheless, so i decided to explain myself. Anyway, be sure that i *do appreciate* getting reviews, and i'm honored to see Han-Wen reviewing my patch! good night Janek
Sign in to reply to this message.
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#old... lily/slur-configuration.cc:62: On 2013/10/23 04:22:23, Keith wrote: > > but the function was broken: it only moved slurs in one direction We concluded the same thing about the old code. The old code intended to move slurs in one direction only. I was only confused by your comment. I interpreted 'broken' to mean "failing to to what was intended". 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#new... lily/slur-configuration.cc:60: // rather than decrease, because we want to avoid too flat slurs. On 2013/10/23 21:41:24, janek wrote: > > Precisely because min_gap_above_staffline > min_gap_below_staffline > we are more likely to increase than decrease curvature. Okay. So then the comment "The function is implemented so that we're more likely to increase curvature" refers to the default values. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:84: Real const slur_th = state.thickness_ * staff_th * 10; On 2013/10/23 21:41:24, janek wrote: > On 2013/10/23 04:22:23, Keith wrote: > > Why 10? Is that the thickness of the fattest part of the slur? > > No, it's because staff_th is measured in staffspaces, while state.thickness_ > appears to be measured in 1/10ths of staffline-thicknesses state.thickness_ already has the \layout{line-thickness} as a factor in its initialization in Slur_score_state::fill(). Staff_symbol_referencer::line_thickness() returns something that scales with \layout{line-thickness} So, it looks like you have two factors of \layout{line-thickness} and the *10 roughly cancels one of them.
Sign in to reply to this message.
fix the problems - sort of..
Sign in to reply to this message.
New patch set uploaded. I expect that it may not be very good - i'm tired and cannot think clearly. I have a rough idea of how this problem should be fixed ideally, but implementing such solution would require much more work. 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#old... lily/slur-configuration.cc:62: On 2013/10/24 03:33:43, Keith wrote: > On 2013/10/23 04:22:23, Keith wrote: > > > but the function was broken: it only moved slurs in one direction > > We concluded the same thing about the old code. The old code intended to move > slurs in one direction only. > > I was only confused by your comment. I interpreted 'broken' to mean "failing to > to what was intended". Ah, ok. For me the intent was to avoid the collision, and in this regard old code failed. 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#new... lily/slur-configuration.cc:69: // Get the point(s) where the curve is horizontal (i.e. the belly): On 2013/10/23 04:22:23, Keith wrote: > 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" Done. https://codereview.appspot.com/15400049/diff/1/lily/slur-configuration.cc#new... lily/slur-configuration.cc:84: Real const slur_th = state.thickness_ * staff_th * 10; On 2013/10/24 03:33:43, Keith wrote: > On 2013/10/23 21:41:24, janek wrote: > > On 2013/10/23 04:22:23, Keith wrote: > > > Why 10? Is that the thickness of the fattest part of the slur? > > > > No, it's because staff_th is measured in staffspaces, while state.thickness_ > > appears to be measured in 1/10ths of staffline-thicknesses > > state.thickness_ already has the \layout{line-thickness} as a factor in its > initialization in Slur_score_state::fill(). > > Staff_symbol_referencer::line_thickness() returns something that scales with > \layout{line-thickness} > > So, it looks like you have two factors of \layout{line-thickness} and the *10 > roughly cancels one of them. Indeed. So, should i change the definition of thickness_ in Slur_score_state::fill so that it includes staff thickness? Because right now it seems that info is inaccurate.
Sign in to reply to this message.
The behavior of the first patch was good, only the code was difficult to understand. If you are tired, you could take a short break, and then simplify the patch and it will be great. https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.cc File lily/slur-configuration.cc (right): https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... lily/slur-configuration.cc:91: Real fac = (t - t*t + 0.5) * 4/3; The effectiveness of the middle control points to move the horizontal part of the slur depends in the same way on 't'. You could let that varying effectiveness have its natural result, and then move the endpoints only 2/3 as much as you would to get the full gap. https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... lily/slur-configuration.cc:93: Interval const min_gap = Interval (fac * state.parameters_.min_gap_below_staffline_, The use of normalized directions and indexing UP for inside and DOWN for outside was very hard for me to follow. You could leave everything right-side-up and just lookup the single Real-valued min_gap that you need. https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... lily/slur-configuration.cc:101: // TODO: to handle weird staves well, we should round to staffline positions. The old code handled 4-line staves, so try to keep that. Making a function nearest_staff_line() would be nice, but looking at the code for on_staff_line() it seems it would not be simple. You can check the two nearest staff positions: Real pos = my_round(2*y); // cannot use norm_y if (!on_staff_line(pos)) { pos += (pos < 2*y)? +1.0: -1.0; if (!on_staff_line(pos)) return bez; } https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... lily/slur-configuration.cc:133: Real mid_pts_cor = (1 - factor) * correction / (3 * (t - (t * t))); This is (|t-0.5| - 0.5) / 3t(1-t) * correction so as t goes 0 to 0.5 to 1 it varies from 1/3 to 2/3 to 1/3 In the first patch, the variation was in the other direction: big-small-big. Maybe you could use mid_pts_cor = 4.0/3.0 * correction Then you have moved the horizontal point 4t(1-t) of the desired correction, which is the complete correction if t is in the middle, less toward the ends. Then the remaining correction can be done by moving the endpoints, but since you can allow the slur to be closer to a staff-line if the nearest point is near an endpoint, just do half the remaining correction. https://codereview.appspot.com/15400049/diff/120001/lily/slur.cc File lily/slur.cc (right): https://codereview.appspot.com/15400049/diff/120001/lily/slur.cc#newcode519 lily/slur.cc:519: "@item min-gap-above-staffline\n" I meant to suggest that the property names be min-gap-to-staffline-inside min-gap-to-staffline-outside
Sign in to reply to this message.
I suggest making the minimal changes to the structure of the old code (plus whatever comments you like) that make the change we all want in in behavior. https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.cc File lily/slur-configuration.cc (left): https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... lily/slur-configuration.cc:45: { Real t = ts[0]; Real y = bez.curve_point (t)[Y_AXIS]; Grob *staff = state.extremes_[LEFT].staff_; Real p = 2 * (y - staff->relative_coordinate (state.common_[Y_AXIS], Y_AXIS)) / state.staff_space_; Real const round = my_round (p); Real const frac = p - round; Real min_separation = 0.5 * state.thickness_ + (frac * state.dir_ > 0.0) ? state.parameters_.min_gap_inside_ : state.parameters_.min_gap_outside_; if (fabs (frac) < 2 * min_separation && Staff_symbol_referencer::on_staff_line (staff, int (round))) { Direction resolution_dir = (frac > 0.0) ? UP : DOWN; Real newp = round + resolution_dir * min (1.0, 2 * min_separation); Real dy = (newp - p) * state.staff_space_ / 2.0; bez.control_[1][Y_AXIS] += dy; bez.control_[2][Y_AXIS] += dy; // The horizontal part of the curve moves 3t-3t² as far // as the middle control-points, so the remaining correction is: dy -= dy * 3.0 * t * (1.0 - t); bez.control_[0][Y_AXIS] += dy; bez.control_[1][Y_AXIS] += dy; bez.control_[2][Y_AXIS] += dy; bez.control_[3][Y_AXIS] += dy; }
Sign in to reply to this message.
Hi Keith, I'm sorry for not replying earlier - i had to focus on other things, and i didn't have time to spare on the slur stuff. Thanks for all your suggestions; i'll get back to this when i finish one thing i have to do for Urs (hopefully after the weekend). best, JAenk 2013/11/12 <k-ohara5a5a@oco.net>: > I suggest making the minimal changes to the structure of the old code > (plus whatever comments you like) that make the change we all want in in > behavior. > > > https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.cc > File lily/slur-configuration.cc (left): > > https://codereview.appspot.com/15400049/diff/120001/lily/slur-configuration.c... > lily/slur-configuration.cc:45: { > Real t = ts[0]; > Real y = bez.curve_point (t)[Y_AXIS]; > > Grob *staff = state.extremes_[LEFT].staff_; > > Real p = 2 * (y - staff->relative_coordinate > (state.common_[Y_AXIS], Y_AXIS)) > / state.staff_space_; > > Real const round = my_round (p); > Real const frac = p - round; > Real min_separation = 0.5 * state.thickness_ > + (frac * state.dir_ > 0.0) > ? state.parameters_.min_gap_inside_ > : state.parameters_.min_gap_outside_; > > if (fabs (frac) < 2 * min_separation > && Staff_symbol_referencer::on_staff_line (staff, int > (round))) > { > Direction resolution_dir = (frac > 0.0) ? UP : DOWN; > > Real newp = round + resolution_dir * min (1.0, 2 * > min_separation); > > Real dy = (newp - p) * state.staff_space_ / 2.0; > > bez.control_[1][Y_AXIS] += dy; > bez.control_[2][Y_AXIS] += dy; > > // The horizontal part of the curve moves 3t-3t² as far > // as the middle control-points, so the remaining correction > is: > dy -= dy * 3.0 * t * (1.0 - t); > > bez.control_[0][Y_AXIS] += dy; > bez.control_[1][Y_AXIS] += dy; > bez.control_[2][Y_AXIS] += dy; > bez.control_[3][Y_AXIS] += dy; > } > > https://codereview.appspot.com/15400049/
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
