Menu
▾
▴
#3338 spacing after 2.17.13 is much looser
Originally created by: *anonymous
Originally created by: Elu...@gmail.com
Originally owned by: k-ohara5...@oco.net
as reported in http://lilypond.1069038.n5.nabble.com/Bad-vertical-spacing-with-instantiated-staves-td145151.html by Timothy Lanfear the spacing has changed significantly after 2.17.13
was this intended or not?
the answers of David K. on http://lilypond.1069038.n5.nabble.com/looser-spacing-after-2-17-13-td145152.html seem to doubt the intention
Originally posted by: k-ohara5...@oco.net
A short ossia staff is now getting space, not just for the line on which it appears, but for all further lines in the score. This was not intended.
The patch for issue 3160 was trying to be consistent about when we remove the space reservation for staves, only when requested by remove-empty, but accidentally kept the reservation for an ossia staff when it is not merely empty, but when the staff context is finished.
Labels: -Type-Other Type-Defect Regression
Originally posted by: k-ohara5...@oco.net
Workaround: make the ossia staff with the remove-empty option
\new Staff \with {\override VerticalAxisGroup #'remove-empty = ##t} c''1
Originally posted by: k-ohara5...@oco.net
This bug will ruin any score with a temporary staff, and we can revert the patch labeled for issue 3160 and issue 3160 remains fixed (comments there explain why) so
http://codereview.appspot.com/9026044
Labels: -Type-Defect Type-Critical patch-new
Originally posted by: m...@mikesolomon.org
Does your patch cause issue 1669 to resurface?
Originally posted by: k-ohara5...@oco.net
The 'patch' linked above for automated checking is not my patch; it is merely a revert.
After the revert issue 1669 comes back (not with the original example, but with the revised example in comment 10 on that issue). Note that issue 1669 was found by me while debugging, not in any realistic use of the program, while this issue ruins page breaking for any score using a temporary staff.
Originally posted by: k-ohara5...@oco.net
(No comment was entered for this change.)
Owner: k-ohara5...@oco.net
Originally posted by: dak@gnu.org
Patchy the autobot says: passes tests. The only flagged regtest is spacing-to-empty-barline. It is not quite clear from the image whether it has become more compact or less, but the second system is supposedly at a different vertical position.
Labels: -patch-new Patch-review
Originally posted by: mts...@gmail.com
Rather than reverting, can we modify this fix so that the previous fix holds.
Originally posted by: m...@mikesolomon.org
For example, can the Hara_kiri function return true if the requested range is after the end of an ossia staff?
Originally posted by: k-ohara5...@oco.net
I first tried amending the patch linked to issue 3160, but did not see a way to distinguish between
1) a temporary staff being absent on a particular line-broken system (this issue)
2) a dynamics context completely empty on a particular system (issue 1669)
Each of these can re-appear on later lines, with another forte mark or \context Staff="ossia"{..}
The patch linked to issue 3160 includes the requested baseline-skip allowance for each staff or dynamics line, even when empty. We do not want that allowance for the ossia staff, and neither for the dynamics in issue 1669, actually.
Probably better to fix 1669 in the other direction: consistently skip the spacing allowance for lines that have no graphical elements at all. I do have an updated patch linked to issue 1669, that has all the good simplifications we found when working on the patch linked to issue 3160.
Originally posted by: dak@gnu.org
Issue 3330 introduces a distinction between "emptiness" of stencils in X and Y direction. Can (stencil-empty? stencil Y) drive the decision whether a line should be separating skylines or not? (stencil-empty? stencil X) conveys the additional information whether this "line" has any material requiring padding or other space reservations of its own.
Of course, a large redesign of the whole vertical spacing mechanisms before 2.18 is not really something to look forward to. The question is what a solution would look like that causes the least amount of tears and upheavals.
But maybe this offers some way to signal the condition Mike is talking about in #9.
Originally posted by: k-ohara5...@oco.net
Neither the ossia staff, nor the dynamics line from issue 1669, have any graphical objects at all in the systems where they are having space allowed inconsistently. So there are no stencils to have directional emptiness.
Also, such lines do not 'separate' skylines if there is gap. In general, notes can protrude through a line of dynamics
<<\new Staff {g'4 g' g g'}
\new Dynamics {s4\p s s s\f}
\new Staff {\clef bass g4 g' g g} >>
The question is whether the empty line should advance the ideal position of staves to follow, according to the the staff-spacing parameters. Mike and I independently decided empty lines should count for advancing the ideal staff position, but maybe only due to the sneaky way they were being skipped in (some places in) the code. The example of this issue makes it clear that we should *not* advance staff postiions after an ossia staff when it is absent, and skipping the advancement would work for issue 1669 as well, if it is done consistently.
Originally posted by: m...@mikesolomon.org
Just to play devil's advocate, if it is true that ossia staves behave fundamentally differently than dynamics staves in that one, when empty, shouldn't reserve space and one should, then shouldn't this difference be encoded into the LilyPond document? For example, if we made:
\new OssiaStaff
and defined OssiaStaff as exactly the same as staff but with
\override VerticalAxisGroup #'remove-empty = ##t
in addition to one or two other common ossia overrides, then it'd be a lot more lilypondish in that input reflects intention and the program responds accordingly. Of course, it'd screw up many an existing score, but if the syntax is more coherent, then it is worth it in my opinion.
Originally posted by: dak@gnu.org
@Mike: why should it screw up existing scores?
Originally posted by: m...@mikesolomon.org
Because we would have no way with a convert-ly rule to figure out whether or not people intended for staves to be an OssiaStaff, so there is no way to logically convert scores to follow this rule. The best we could do would be to document it.
Originally posted by: pkx1...@gmail.com
I have no idea on this one, am leaving it for review until next countdown.
Originally posted by: k-ohara5...@oco.net
I think the discussion is settled, so will try to put this on countdown.
The patch is a simple revert to the patch from issue 3160, which solves this regression, leaves issue 3160 correct (as another cleanup patch solved it) and also removes the major symptom from issue 3342 (for me, so I'd like the user who reported the problem to be able to confirm).
The simplification that we liked about the patch-to-be-reverted is now in a patch linked to issue 1669.
Comment #14 imagines what to do *if* we want ossia staves to affect spacing differently from dynamics when empty, but the relevant issues (this one and 1669) want to act the same way: both should take no space on systems where they have no graphical output.
Labels: -Patch-review Patch-countdown
Originally posted by: k-ohara5...@oco.net
Original example set on A6 paper, ragged-bottom.
Originally posted by: mts...@gmail.com
Ok, I dig, all LGTM!
Originally posted by: pkx1...@gmail.com
Patch counted down - please push
Labels: -Patch-countdown Patch-push
Originally posted by: k-ohara5...@oco.net
commit [r96b7c6e45ec82d720e565c2f361e593f016eb3fa]
Labels: -Patch-push Fixed_2_17_18
Status: Fixed
Originally posted by: k-ohara5...@oco.net
Setting the example from the email at the top of this issue on A6 paper
\paper {ragged-bottom = ##t #(set-paper-size "a6") }
{ c''1
<< c''1
\new Staff c''1 >>
\repeat unfold 120 c''1 }
should now put about seven staves on each page.
Originally posted by: Elu...@gmail.com
(No comment was entered for this change.)
Status: Verified