Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
LGTM https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc File lily/spanner.cc (right): https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc#newcode394 lily/spanner.cc:394: /* On 2015/02/06 21:47:11, lemzwerg wrote: Minor nit: Please use spaces, not tabs. Done. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc File lily/spanner.cc (right): https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc#newcode404 lily/spanner.cc:404: Rod r; Perhaps we should call this rod something else to underscore that it's not the same rod as the one declared above. Sure, it's fine because the last r was declared in a nested scope and 'r' is for rod!, but this is unnecessarily confusing (even after moving the declaration from where it was originally, which was really obscure). https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly File input/regression/minimum-length-after-break.ly (right): https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode15 input/regression/minimum-length-after-break.ly:15: \override Tie.minimum-length-after-break = 20 On 2015/02/05 23:34:22, thomasmorley651 wrote: On 2015/02/05 22:51:06, david.nalesnik wrote: On 2015/02/05 22:31:33, thomasmorley651 wrote: I'd use \once \override ... Though, that's only me I'm happy to change that. Would you use \once with the other overrides in the file? I'd always use \once. But again, it's only me, what do others think? I went ahead and changed to \once. The object being overridden changes, and the Scheme music representation isn't any shorter, but \once might prevent problems should the test be extended. (Done.) https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16 input/regression/minimum-length-after-break.ly:16: a1~ On 2015/02/05 23:34:22, thomasmorley651 wrote: On 2015/02/05 22:51:06, david.nalesnik wrote: On 2015/02/05 22:31:34, thomasmorley651 wrote: I assume it works for chords as well. I'd add at least one example with chords and Tie or Glissando Instead of the one regtest, I could have two. The first would show a tied chord, and I would show how you can use 'minimum-length and 'minimum-length-after-break in various combinations. The second would be this regtest, and I could take out the tie example so there's no overlap with the other test. What do you think? I can't see an advantage in having two regtests. I'd prefer to extend this one. After discovering an issue (corrected in patch set #2), I'm leaning towards a regtest specifically aimed at showing how minimum-distance and minimum-distance-after-break relate. They ought to produce the same results in the spanner piece after the break. The C++ code uses these values in a way that is not easy to understand, and I think there should be a check for future changes to the springs-and-rods callback. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc File lily/spanner.cc (right): https://codereview.appspot.com/201140043/diff/20001/lily/spanner.cc#newcode394 lily/spanner.cc:394: /* Minor nit: Please use spaces, not tabs. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
LGTM. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
Can't review the C++ part. Some nitpicks in the regtest. Will it be possible to tweak the first part of the Tie etc at line-end as well? With a combi of setting 'minimum-length-after-break' and 'minimum-length? If yes, I'd show an example in the regtest. At least it should be explained/demonstrated in the docs. https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly File input/regression/minimum-length-after-break.ly (right): https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode15 input/regression/minimum-length-after-break.ly:15: \override Tie.minimum-length-after-break = 20 I'd use \once \override ... Though, that's only me https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16 input/regression/minimum-length-after-break.ly:16: a1~ I assume it works for chords as well. I'd add at least one example with chords and Tie or Glissando https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly File input/regression/minimum-length-after-break.ly (right): https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode15 input/regression/minimum-length-after-break.ly:15: \override Tie.minimum-length-after-break = 20 On 2015/02/05 22:31:33, thomasmorley651 wrote: I'd use \once \override ... Though, that's only me I'm happy to change that. Would you use \once with the other overrides in the file? https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16 input/regression/minimum-length-after-break.ly:16: a1~ On 2015/02/05 22:31:34, thomasmorley651 wrote: I assume it works for chords as well. I'd add at least one example with chords and Tie or Glissando Instead of the one regtest, I could have two. The first would show a tied chord, and I would show how you can use 'minimum-length and 'minimum-length-after-break in various combinations. The second would be this regtest, and I could take out the tie example so there's no overlap with the other test. What do you think? https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
Reviewers: ,
Message:
Please review.
Description:
Allow independent adjustment of minimum length for spanner siblings
The property minimum-length affects both unbroken and broken spanners,
making
independent adjustment impossible. This patch adds a property,
minimum-length-after-break, which, if set, behaves similarly to
minimum-length,
but only affects siblings starting a line. If
minimum-length-after-break is
unset, minimum-length still controls all lengths.
A regtest demonstrates usage of the new property with a number of
spanners.
Please review this at https://codereview.appspot.com/201140043/
Affected files (+56, -1 lines):
A input/regression/minimum-length-after-break.ly
M lily/spanner.cc
M scm/define-grob-properties.scm
Index: input/regression/minimum-length-after-break.ly
diff --git a/input/regression/minimum-length-after-break.ly
b/input/regression/minimum-length-after-break.ly
new file mode 100644
index
..80c3f019afbca5523a5dae2a412b05529c15a69a
--- /dev/null
+++ b/input/regression/minimum-length-after-break.ly
@@ -0,0 +1,45 @@
+\version 2.19.16
+
+\header {
+ texidoc = The property @code{minimum-length-after-break} can be
+used to stretch broken spanners starting after a line break. The
+following example shows usage for a variety of spanners.
+
+}
+
+\layout {
+ ragged-right = ##t
+}
+
+{
+ \override Tie.minimum-length-after-break = 20
+ a1~
+ \break
+ a1
+
+ \override Slur.minimum-length-after-break = 20
+ a1(
+ \break
+ d'1)
+
+ \override TextSpanner.springs-and-rods = #ly:spanner::set-spacing-rods
+ \override TextSpanner.minimum-length-after-break = 20
+ a1\startTextSpan
+ \break
+ a1\stopTextSpan
+
+ \override Hairpin.after-line-breaking = ##t
+ \override Hairpin.to-barline = ##f
+ \override Hairpin.minimum-length-after-break = 20
+ a1\
+ \break
+ a1\!
+
+ \override Glissando.springs-and-rods = #ly:spanner::set-spacing-rods
+ \override Glissando.breakable = ##t
+ \override Glissando.after-line-breaking = ##t
+ \override Glissando.minimum-length-after-break = 20
+ a1\glissando
+ \break
+ d'1
+}
Index: lily/spanner.cc
diff --git a/lily/spanner.cc b/lily/spanner.cc
index
66b35dcd1b8b53df0438bcccddc62b96be71b134..bb0e78b02236c0aee834311eca82f54cd12a4722
100644
--- a/lily/spanner.cc
+++ b/lily/spanner.cc
@@ -366,7 +366,9 @@ Spanner::set_spacing_rods (SCM smob)
{
Grob *me = Grob::unsmob (smob);
SCM num_length = me-get_property (minimum-length);
- if (scm_is_number (num_length))
+ SCM broken_length = me-get_property (minimum-length-after-break);
+ if (scm_is_number (num_length)
+ || scm_is_number (broken_length))
{
Rod r;
Spanner *sp = dynamic_castSpanner * (me);
@@ -389,6 +391,8 @@ Spanner::set_spacing_rods (SCM smob)
r.item_drul_[LEFT] = cols.back ()-find_prebroken_piece (RIGHT);
r.item_drul_[RIGHT] = sp-get_bound (RIGHT);
+ if (scm_is_number (broken_length))
+r.distance_ = robust_scm2double (broken_length, 0);
r.add_to_cols ();
}
@@ -546,6 +550,7 @@ ADD_INTERFACE (Spanner,
/* properties */
normalized-endpoints
minimum-length
+ minimum-length-after-break
spanner-broken
spanner-id
to-barline
Index: scm/define-grob-properties.scm
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index
0d5eaa124a001d875af1f3f7019e34df1dd8e60d..92742f98d05d51c64ca2469c9b165a66e98d4b9a
100644
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -637,6 +637,11 @@ this long, normally in the horizontal direction. This
requires an
appropriate callback for the @code{springs-and-rods} property. If
added to a @code{Tie}, this sets the minimum distance between
noteheads.)
+ (minimum-length-after-break ,ly:dimension? If set, try to make
+a broken spanner starting a line this long. This requires an
+appropriate callback for the @code{springs-and-rods} property. If
+added to a @code{Tie}, this sets the minimum distance to the
+notehead.)
(minimum-length-fraction ,number? Minimum length of ledger line
as fraction of note head size.)
(minimum-space ,ly:dimension? Minimum distance that the victim
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)
https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly File input/regression/minimum-length-after-break.ly (right): https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode15 input/regression/minimum-length-after-break.ly:15: \override Tie.minimum-length-after-break = 20 On 2015/02/05 22:51:06, david.nalesnik wrote: On 2015/02/05 22:31:33, thomasmorley651 wrote: I'd use \once \override ... Though, that's only me I'm happy to change that. Would you use \once with the other overrides in the file? I'd always use \once. But again, it's only me, what do others think? https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16 input/regression/minimum-length-after-break.ly:16: a1~ On 2015/02/05 22:51:06, david.nalesnik wrote: On 2015/02/05 22:31:34, thomasmorley651 wrote: I assume it works for chords as well. I'd add at least one example with chords and Tie or Glissando Instead of the one regtest, I could have two. The first would show a tied chord, and I would show how you can use 'minimum-length and 'minimum-length-after-break in various combinations. The second would be this regtest, and I could take out the tie example so there's no overlap with the other test. What do you think? I can't see an advantage in having two regtests. I'd prefer to extend this one. https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
