Menu
▾
▴
1 Attachments
#3160 chord names pushed into the staff
Originally created by: *anonymous
Originally created by: k-ohara5...@oco.net
Originally owned by: mts...@gmail.com
\version "2.17.11"
n = 30
<< \new ChordNames \chordmode { \repeat unfold \n c1 }
\new Staff {\clef alto \repeat unfold \n c'1 }
\addlyrics { \repeat unfold \n "a" }
\addlyrics { \repeat unfold \n "a" }
\addlyrics { \repeat unfold \n "" }
\addlyrics { \repeat unfold \n "" } >>
This failed in 2.14, was fixed as issue 2276, and worked in 2.16.
Now it fails again.
Originally posted by: k-ohara5...@oco.net
reported at
<https://lists.gnu.org/archive/html/bug-lilypond/2013-02/msg00019.html>
Originally posted by: k-ohara5...@oco.net
(No comment was entered for this change.)
Blocking: lilypond:3156
Originally posted by: k-ohara5...@oco.net
The collision starts with the patch for 2148, commit [r28f3294954eff1f263d3b2e3de1c520f4d2fbdfc]
I cannot find which piece of that huge commit caused the change, but the change in behavior is that the lines of Lyrics containing "" now get empty skylines, where they did not before. This leads to issue 1669 for the Lyrics, because the line is empty in the sense of taking no space, but not in the sense of \removeEmptyStaves. The code I tried to remove in the patch for issue 1669 gets confused in this situation.
So the suggestion of issue 3156 seems reasonable: any line that takes no space would be removed in the sense of \removeEmptyStaves
Related
Issues:
#1669Issues: #3156
Originally posted by: dak@gnu.org
I think that an explicit removal is the wrong thing to do: a zero-height line is a "neutral element" when stacking boxes (without padding, though). Now we are stacking skylines, and empty lyrics again should correspond to a "neutral element". For skylines, the difference between a zero-height box and nothing at all is very significant. Instead of juggling around with workarounds like \removeEmptyStaves, we need to make sure that skylines have "neutral elements" that don't change a contour, and that those neutral elements are produced by code supposed to have no visible output.
That's the fundamental issue that I see in need of addressing, and not addressing that will lead to a growing heap of various workarounds piling up in various areas of LilyPond. I think that it is very important to get consistent and predictable behavior from skylines first, and then make any callers work with that consistent and predictable behavior rather than all callers working around inconsistent and only loosely predictable behavior.
Originally posted by: mts...@gmail.com
In the text interface, the "" symbol has an empty extent (so negative height), not zero-height. I agree with you that a zero-height element should stay. However, by definition, an empty skyline has nothing in it, which means that measurements cannot be taken between it and adjacent skylines.
I agree with Keith, therefore, that 3156 is the way to go for empty skylines. If, however, you want to redefine "" so as to be a point stencil instead of an empty interval, then that's a trivial change and can be proposed in a separate patch.
Originally posted by: k-ohara5...@oco.net
My plan to resolve issue 1669 was to treat empty lines of Lyrics and Dynamics by leaving them in the conceptual stack of lines, so they advance the baseline of the next line according to the minimum-distance setting, and act transparently to the skylines of lines above and below. (Possibly what was meant by "neutral element" above.)
It is perfectly possible to mark off baselines, and remember the padded skylines of previous lines, in the presence of intermediate lines with empty skylines.
However, during the planning of page-layout, we do not want to count the minimum-distance of an empty Lyrics or Dynamics line, if the user has requested \removeEmptyStaves for that line. The rules for guessing which lines to ignore are not quite consistent, but the discussion on the patch for issue 1669 might lead to a consistent solution.
Related
Issues:
#1669Originally posted by: mts...@gmail.com
Conceptually, things that implement the axis-group-interface have widths, heights, and skylines that are 100% linked to the elements they contain. I think it's important to maintain this distinction. So, for example, you could create a transparent stub grob that causes the VerticalAxisGroup of Dynamics to always have a skyline. But this would pose the issue of where to put said baseline. For example, in the case:
<<
\Staff { c,,,,,4 }
\Lyrics { }
\Staff { [r2] c'''''4 }
>>
where is the baseline established and how long is it? If it is at the same X position as one of the two notes, it will cause radically different spacing then if it is between them. That is why I think these axis groups should be removed all together if they have no elements - it is a conceptually cleaner approach.
To me, removeEmpty means "if there is no musical information, remove." That is, if there are no notes. There may be, however, rests, time signatures, etc.. However, if there are no grobs period, then the containing axis group is empty and as it has no height apart from its elements. Thus, a baseline cannot be established to it because it is "solid" nowhere.
Originally posted by: k-ohara5...@oco.net
> I think it's important to maintain this distinction.
You did not describe the distinct things, between which to maintain "this distinction".
I suggest that the two things are
1) graphical objects, having outlines stored in a skyline, for which we allow space including the requested padding
2) the conceptual presence of a line, whether empty or not, which we use to determine how many baseline-skips to apply, based on the basic-distance and minimum-distance property of the line.
LilyPond keeps these concepts mostly distinct. For example we have the option remove-empty to control if an absence of events on a line should remove the entire line. Some the page-spacing code, however, assumes that graphically empty lines are to be conceptually removed, while other code does not, raising issue 1669.
> So, for example, you could create a transparent stub grob that causes the
> VerticalAxisGroup of Dynamics to always have a skyline.
Probably we do not want this (and if I understand correctly you bring it up to show why you do not want it). Notes and protrusions from the Staff above and below often protrude through gaps in an intervening Lyrics or Dynamics line.
> Thus, a baseline cannot be established to it because it is "solid" nowhere.
Well, we can and do space lines based on their 'basic-distance properties when the skylines of these lines are well clear of each other. In several cases, a graphically empty line has its baseline spaced this way---for example, the way version 2.16 sets the input at the top of this issue.
The only place I see where the spacing code completely skip lines that are graphically empty is in align-interface.cc:get_skylines(). From the comments on issue 1669 we see that the reason for this skipping was to estimate page heights better, assuming empty lines would be removed under the remove-empty mechanism. It would seem better to test under the actual remove-empty rules, which
Hara_kiri_group_spanner::request_suicide (Grob *me, int start, int end)
will do for us, without actually removing the line, so we can get the correct answers for hypothetical layouts during the line-and-page-breaking stage.
Related
Issues:
#1669Originally posted by: mts...@gmail.com
Just to make sure, then, the idea for vertical axis groups with empty skylines that do not have remove-empty set is to omit them but tack on their basic distance (and other spacing info) to the next vertical axis group in the queue (unless it is the bottommost one)? I can propose a patch with this change...
Originally posted by: k-ohara5...@oco.net
> the idea for vertical axis groups with empty skylines
> that do not have remove-empty set is to omit them but ...
Well, I would think it simpler to leave lines in place, if remove-empty is false.
That is, leave the VerticalAxisGroup data structure in place, and let the spacing engine simply notice that there are no collisions with the empty line, so insert a spacing spring (rod) as usual with the basic(minimum)-distance.
But you're the programmer so you can implement as you like.
Originally posted by: mts...@gmail.com
Ok. Once the solution to the EPS problem in skyline.cc is pushed I'll come back to this.
Originally posted by: k-ohara5...@oco.net
The part of the patch for issue 2148, that caused the change in the example at the top of this issue, was the removal of 'horizon_padding' from skylines. Without this padding, empty lyrics like "" have zero width (less than EPS) and are removed from the skyline, causing the skyline to be empty, causing the same confusion as in issue 1669.
Therefore the "solution to the EPS problem" in issue 3161 will also remove the collision here.
Related
Issues:
#1669Issues:
#2148Issues:
#3161Originally posted by: mts...@gmail.com
Assuming that we want to preserve space between the vertical axis groups, the issue is that Align_interface::get_minimum_translations is returning no translation between the full lyrics and the empty lyrics. This causes the minimum distance of the spring representing a system to be too short. This translation is, however, read into the spacing spec for the chord names (and the phantom lyrics, although we can't see it because its empty), forcing them down on the next staff.
The solution will likely be in getting get_minimum_translations to report better minimum translations, but it'll take me time to figure that out...
Originally posted by: mts...@gmail.com
I think I found the culprit. The function get_skylines in align interface has a side effect that removes elements with empty skylines from the elements list. Side effects are evil...
Running regtests.
Originally posted by: mts...@gmail.com
Includes empty skylines in minimum translation calculations
http://codereview.appspot.com/7304068
Labels: Patch-new
Owner: mts...@gmail.com
Status: Started
Originally posted by: mts...@gmail.com
The patch that I've uploaded breaks stanza numbers lining up in a column...ignore it for now...
Originally posted by: mts...@gmail.com
Fixes StanzaNumber
http://codereview.appspot.com/7304068
Originally posted by: mts...@gmail.com
Should work now...
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc.
Labels: -Patch-new Patch-review
Originally posted by: dak@gnu.org
This one looks rather confined and straightforward: should be easy to review (the use of horizon-padding seems awkward, though). I'm putting this on countdown for 2013/02/13 16:00 UTC.
Labels: -Patch-review Patch-countdown
Originally posted by: mts...@gmail.com
Avoids over-estimating pure heights of systems with removable vertical axis groups.
http://codereview.appspot.com/7304068
Labels: -Patch-countdown Patch-new
Originally posted by: mts...@gmail.com
Restores previous translates for empty elements
http://codereview.appspot.com/7304068
Originally posted by: pkx1...@gmail.com
Passes Make, Make test and full make doc.
Reg test diffs here:
https://www.yousendit.com/download/UW13eFljNnlEbUxsZThUQw
Labels: -Patch-new Patch-review
Originally posted by: pkx1...@gmail.com
Keith has made some comments on patch
Labels: -Patch-review Patch-needs_work
Originally posted by: k-ohara5...@oco.net
Also, the regression test differences show that non-staff lines like Lyrics are not getting their space allowance.
Probably we should let the patch for issue 3161 fix this issue as well, and continue the cleanup of confusion between remove-empty lines and lines that just look empty, with issue 1669, where the patch is already further along toward that goal.
Related
Issues:
#1669Issues:
#3161