possible tuplet bug?
Hi, I just noticed something fishy about the interaction of Tuplet.connect-to-neighbor and Tuplet.break-overshoot. The property 'break-overshoot is meant to apply to broken spanners, but here it is having an effect on unbroken "connected" tuplet brackets. (Break-overshoot is described as "How much does a broken spanner stick out of its bounds?") The following snippet demonstrates the problem (see attached image): %% \version "2.17.97" \layout { ragged-right = ##t } \relative c' { \times 2/3 { c4 d e } \times 2/3 { d4 e c } \times 2/3 { c'4 d e } \times 2/3 { d4 e c } \bar "||" %This "fixes" the problem: %\override TupletBracket.break-overshoot = #'(0 . 0) \override TupletBracket.connect-to-neighbor = #'(#t . #t) \times 2/3 { c,4 d e } \times 2/3 { d4 e c } \times 2/3 { c'4 d e } \times 2/3 { d4 e c } } A default value of '(-0.5 . 0.0) is being applied to the tuplet brackets affected by the override of 'break-overshoot. Thus, the left side is shifted to the right. (The tuplet number is also moved from its position in the ordinary tuplets.) In the "patch" below, I've suggested a modification of the relevant function in tuplet-bracket.cc which appears to fix the problem. (The replacement is above the commented-out section of the original.) You can see the results in the second image. I hesitate to submit a formal patch because I haven't worked with C++ in LilyPond before, and I'd like to get some feedback from someone who understands this aspect of the codebase to make sure I'm not missing something big! (It does compile--the image uses the patch.) Thanks, David from tuplet-bracket.cc: MAKE_SCHEME_CALLBACK (Tuplet_bracket, calc_x_positions, 1) SCM Tuplet_bracket::calc_x_positions (SCM smob) { Spanner *me = unsmob_spanner (smob); extract_grob_set (me, "note-columns", columns); Grob *commonx = get_common_x (me); Direction dir = get_grob_direction (me); Drul_array bounds; bounds[LEFT] = get_x_bound_item (me, LEFT, dir); bounds[RIGHT] = get_x_bound_item (me, RIGHT, dir); Drul_array connect_to_other = robust_scm2booldrul (me->get_property ("connect-to-neighbor"), Drul_array (false, false)); Interval x_span; for (LEFT_and_RIGHT (d)) { x_span[d] = Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[d]; if (connect_to_other[d] && bounds[d]->break_status_dir()) { Interval overshoot (robust_scm2drul (me->get_property ("break-overshoot"), Interval (-0.5, 0.0))); if (d == RIGHT) x_span[d] += d * overshoot[d]; else x_span[d] = Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[-d] - overshoot[LEFT]; } /* if (connect_to_other[d]) { Interval overshoot (robust_scm2drul (me->get_property ("break-overshoot"), Interval (-0.5, 0.0))); if (d == RIGHT) x_span[d] += d * overshoot[d]; else x_span[d] = (bounds[d]->break_status_dir () ? Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[-d] : robust_relative_extent (bounds[d], commonx, X_AXIS)[-d]) - overshoot[LEFT]; } */ else if (d == RIGHT && (columns.empty () || (bounds[d]->get_column () != dynamic_cast (columns.back ())->get_column ( { /* We're connecting to a column, for the last bit of a broken fullLength bracket. */ Real padding = robust_scm2double (me->get_property ("full-length-padding"), 1.0); if (bounds[d]->break_status_dir ()) padding = 0.0; Real coord = bounds[d]->relative_coordinate (commonx, X_AXIS); if (to_boolean (me->get_property ("full-length-to-extent"))) coord = robust_relative_extent (bounds[d], commonx, X_AXIS)[LEFT]; coord = max (coord, x_span[LEFT]); x_span[d] = coord - padding; } } return ly_interval2scm (x_span - me->get_bound (LEFT)->relative_coordinate (commonx, X_AXIS)); } <>___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
Hi again, On Wed, Dec 25, 2013 at 12:41 PM, David Nalesnik wrote: > > > The following snippet demonstrates the problem (see attached image): > > And here's the image of the default... <>___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
David Nalesnik writes: > In the "patch" below, I've suggested a modification of the relevant > function in tuplet-bracket.cc which appears to fix the problem. (The > replacement is above the commented-out section of the original.) You can > see the results in the second image. > > > I hesitate to submit a formal patch because I haven't worked with C++ in > LilyPond before, Formal or not, trying to figure out what you changed from just a quote of code is a major nuisance and unreliable. So even if you don't want to propose a "formal" commit, using "git diff" in order to produce a properly readable patch that you can attach to a mail will make people's life easier. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
David, On Wed, Dec 25, 2013 at 12:56 PM, David Kastrup wrote: > David Nalesnik writes: > > > > > I hesitate to submit a formal patch because I haven't worked with C++ in > > LilyPond before, > > Formal or not, trying to figure out what you changed from just a quote > of code is a major nuisance and unreliable. > > So even if you don't want to propose a "formal" commit, using "git diff" > in order to produce a properly readable patch that you can attach to a > mail will make people's life easier. > > Ah, OK, will do. --David ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
On Wed, Dec 25, 2013 at 12:58 PM, David Nalesnik wrote: > David, > > On Wed, Dec 25, 2013 at 12:56 PM, David Kastrup wrote: > > So even if you don't want to propose a "formal" commit, using "git diff" >> in order to produce a properly readable patch that you can attach to a >> mail will make people's life easier. >> >> > Ah, OK, will do. > See attached. I apologize for any inconvenience. -David From 059ff4586a1e6a1fda27f40372a87db972be378c Mon Sep 17 00:00:00 2001 From: David Nalesnik Date: Wed, 25 Dec 2013 09:41:39 -0600 Subject: [PATCH] fix problem with connect-to-neighbor and break-overshoot in unbroken tuplets --- lily/tuplet-bracket.cc |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/lily/tuplet-bracket.cc b/lily/tuplet-bracket.cc index 1eccaab..7171e2a 100644 --- a/lily/tuplet-bracket.cc +++ b/lily/tuplet-bracket.cc @@ -202,18 +202,15 @@ Tuplet_bracket::calc_x_positions (SCM smob) for (LEFT_and_RIGHT (d)) { x_span[d] = Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[d]; - - if (connect_to_other[d]) + + if (connect_to_other[d] && bounds[d]->break_status_dir()) { Interval overshoot (robust_scm2drul (me->get_property ("break-overshoot"), Interval (-0.5, 0.0))); - if (d == RIGHT) x_span[d] += d * overshoot[d]; else -x_span[d] = (bounds[d]->break_status_dir () - ? Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[-d] - : robust_relative_extent (bounds[d], commonx, X_AXIS)[-d]) +x_span[d] = Axis_group_interface::generic_bound_extent (bounds[d], commonx, X_AXIS)[-d] - overshoot[LEFT]; } -- 1.7.0.4 ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
David Nalesnik writes: > See attached. I apologize for any inconvenience. > > -David > From 059ff4586a1e6a1fda27f40372a87db972be378c Mon Sep 17 00:00:00 2001 > From: David Nalesnik > Date: Wed, 25 Dec 2013 09:41:39 -0600 > Subject: [PATCH] fix problem with connect-to-neighbor and break-overshoot in > unbroken tuplets > > --- > lily/tuplet-bracket.cc |9 +++-- > 1 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/lily/tuplet-bracket.cc b/lily/tuplet-bracket.cc > index 1eccaab..7171e2a 100644 > --- a/lily/tuplet-bracket.cc > +++ b/lily/tuplet-bracket.cc > @@ -202,18 +202,15 @@ Tuplet_bracket::calc_x_positions (SCM smob) >for (LEFT_and_RIGHT (d)) > { >x_span[d] = Axis_group_interface::generic_bound_extent (bounds[d], > commonx, X_AXIS)[d]; > - > - if (connect_to_other[d]) > + > + if (connect_to_other[d] && bounds[d]->break_status_dir()) > { >Interval overshoot (robust_scm2drul (me->get_property > ("break-overshoot"), > Interval (-0.5, 0.0))); > - >if (d == RIGHT) > x_span[d] += d * overshoot[d]; >else > -x_span[d] = (bounds[d]->break_status_dir () > - ? Axis_group_interface::generic_bound_extent > (bounds[d], commonx, X_AXIS)[-d] > - : robust_relative_extent (bounds[d], commonx, > X_AXIS)[-d]) Actually, commenting inline in the mail is very much like working on a Rietveld review, just with less of a record for it. If it makes you feel more comfortable doing it in this "informal" way, we should try figuring out whether it is "just you" or a general phenomenon. At any rate: here is what triggers my discomfort sensors while not actually having any clue about what the code does: robust_relative_extent (bounds[d], commonx, X_AXIS)[-d] served as a fallback value for some cases previously. It is not used at all now. That seems strange. > +x_span[d] = Axis_group_interface::generic_bound_extent > (bounds[d], commonx, X_AXIS)[-d] > - overshoot[LEFT]; > } -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: possible tuplet bug?
Hi David(s, especially N) ;-) 2013/12/25 David Nalesnik : > [tuplet problem] > See attached. I apologize for any inconvenience. It's great to see you working on C++ stuff! I'm sorry that i don't have time to investigate this, but i'm rooting for you! One piece of advice that i have: you may try using 'git blame' to find the commit which originally introduced the problematic code (you will probably have to use it repeatedly because of code reformattings that happened since this was written). There is no guarantee that you'll find any explanation, but it's worth trying. Also, try running the regression tests to see if your change has any negative side-effects. best, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel