Menu
▾
▴
#2272 Long monosyllabic words collide with barlines
Originally created by: *anonymous
Originally created by: colingh...@gmail.com
Originally owned by: mts...@gmail.com
Reported by Patrick
http://article.gmane.org/gmane.comp.gnu.lilypond.bugs/32939
This seems to be related to https://code.google.com/p/lilypond/issues/detail?id=2061
I'll confirm this later today when I have access to 2.15.27.
Originally posted by: PhilEHol...@googlemail.com
(No comment was entered for this change.)
Status: Accepted
Originally posted by: k-ohara5...@oco.net
\version "2.15.33"
\new StaffGroup <<
\new Staff { [r1] e'2 e'2 }
\addlyrics { Straight through }
\new Staff { [r1]*2 } >>
First bad commit : [r4f49b000d6e257724e311b406e2346b8388c1f0e]
Author: Mike Solomon <mike@apollinemike.com>
Date: Sat Oct 1 16:40:28 2011 +0200
Improves horizontal spacing of vertical axis groups that span bars traverse.
Fixes issue 1846.
This patch introduces the idea of a pure-from-neighbor-interface.
The interface provides support for grobs whose pure height is difficult
to estimate (any grob that crosses staffs likely suffers from this)
by allowing for the creation of spacing "stubs" in the crossed
vertical axis groups. These stubs take their pure heights from their
neighbors. So long as one knows that a grob fully crosses a vertical
axis group and the grob should take up the full vertical space of this
axis group for its rank, using approximations of pure height's from the
grob's neighbors ensures that it will be as tall as the rest of the
axis group without triggering a vertical alignment.
Seems the same as issue 2061.
Somehow, the old span bars worked, except for issue 910. If there is another way to fix 910 we could go back to the old machinery.
Labels: -Type-Ugly Type-Critical Regression
Related
Issues:
#1846Issues:
#2061Issues:
#910Originally posted by: dak@gnu.org
"Go back to the old machinery"? I recommend you look at that commit.
input/regression/span-bar-allow-span-bar.ly | 59 +++++++++++
lily/grob.cc | 76 ++++++++++++++
lily/include/grob.hh | 6 +
lily/include/pure-from-neighbor-interface.hh | 34 ++++++
lily/pure-from-neighbor-engraver.cc | 104 +++++++++++++++++++
lily/pure-from-neighbor-interface.cc | 57 ++++++++++
lily/span-bar-engraver.cc | 27 ++++--
lily/span-bar-stub-engraver.cc | 143 ++++++++++++++++++++++++++
lily/span-bar.cc | 2 -
ly/engraver-init.ly | 2 +
scm/define-grob-properties.scm | 1 +
scm/define-grobs.scm | 15 ++-
scm/output-lib.scm | 6 +
13 files changed, 520 insertions(+), 12 deletions(-)
Much fun figuring out just what part of this commit is actually implementing the span-bar stuff, and what part is doing something else that later commits rely on.
Originally posted by: k-ohara5...@oco.net
> "Go back to the old machinery"? I recommend you look at that commit.
The span-bar.cc had only minor changes. In define-grobs.scm we need to restore
Span Bar
(Y-extent . ,ly:axis-group-interface::height)
(meta . ((class . Item)
(object-callbacks . ((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common)
(pure-relevant-grobs . ,ly:axis-group-interface::calc-pure-relevant-grobs)))
and then remove the problematic (issue 2000 issue 2056 issue 2061 issue 2356) engravers "Span_bar_stub_engraver" and "Pure_from_neighbor_engraver" from the StaffGroup and Lyrics contexts.
Then to stop notes from moving over barlines, we need to restore a magic number
BarLine (extra-spacing-height . (-1.1 . 1.1)) ;; reach the first ledger line
as that magic number now lives in pure-from-neighbor-interface::account-for-span-bar grob in output-lib.scm.
Related
Issues:
#2000Issues:
#2056Issues:
#2061Issues:
#2356Originally posted by: mts...@gmail.com
Axis group interface ignores column rank for pure-from-neighbor-interface
http://codereview.appspot.com/5843063
Labels: Patch-new
Originally posted by: mts...@gmail.com
It was just a minor fix that was necessary - by definition, the pure heights of all elements in the "neighbors" grob array factor into the pure height of the grob, whereas axis-group-interface pure height only counts columns in the grob's span rank.
Originally posted by: mts...@gmail.com
Axis group interface ignores column rank for pure-from-neighbor-interface
http://codereview.appspot.com/5843063
Originally posted by: dak@gnu.org
Patchy the autobot says: LGTM. There is a somewhat surprising increase from 1539186 to 2500694 cells in the profile of tuplet-no-stems for which I can see no obvious reason. spanner-alignment drops from 3244804 to 2246305. So the total is about the same; perhaps some bleedover in a multifile job due to conservative garbage collection artifacts.
Labels: Patch-review
Originally posted by: mts...@gmail.com
Axis group interface ignores column rank for pure-from-neighbor-interface
http://codereview.appspot.com/5843063
Labels: Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: LGTM. Interesting spacing changes in accidental-broken-tie-spacing and accidental-tie: in both cases, accidentals move somewhat closer after a line break. Not enough of a difference to look significantly better or worse, but I thought I'd mention it.
Labels: Patch-review
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Owner: mts...@gmail.com
Originally posted by: k-ohara5...@oco.net
> Interesting spacing changes in accidental-broken-tie-spacing
This was worth mentioning, because in fact the current patch defeats the code that 'accidental-broken-tie-spacing.ly' was meant to test. It looks very easy to fix the patch.
An intervening bug-fix, issue 1785, made the failure mode of this regression test less obvious than it was when the test was written.
Related
Issues: #1785
Originally posted by: mts...@gmail.com
Axis group interface ignores column rank for pure-from-neighbor-interface
http://codereview.appspot.com/5843063
Labels: Patch-new
Originally posted by: dak@gnu.org
Regarding comment #13: well, there must be _some_ advantage in having the regtests being run by overqualified personnel.
Can you think of a way to make the failure mode of this regtest become obvious enough to reliably trigger the attention of less paranoid reviewers?
Originally posted by: mts...@gmail.com
It'd be great if there were a sort of collision-assertion feature. That is, for any two grobs, be able to:
a) Connect them somehow; and
b) Assert that their extents (X or Y or both) do not overlap.
This can be hacked in Scheme by setting the grobs' IDs to 'foo and 'bar and overriding something like color (or something that is triggered way downstream) to go up to the system, get the elements list, find grobs with ids foo and bar, and check to see that they don't overlap.
This should be a separate project, though.
Cheers,
MS
Originally posted by: dak@gnu.org
Patchy the autobot says: LGTM.
Labels: Patch-review
Originally posted by: k-ohara5...@oco.net
> a way to make the failure mode of this regtest become obvious
I'll make 'accidental-broken-tie-spacing' test set an accidental both with and without a tie, because it was intended to check that the spacing is the same in both cases. If it breaks then, one line would change and another not, below a description that will say "these should be spaced the same". I can push that along with a comment pointing out where the relevant code is a bit fragile.
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Originally posted by: mts...@gmail.com
Pushed as [r469c53f8ce7c9e6a4e120be130bef0aaac271422].
Sorry for the early push, but I have several out-of-town gigs over the next few days and it seems like there's been consensus on the LGTM-ness of this patch for 2-ish days. The changes Keith is talking about don't have to do w/ this patch in its current form but rather a regtest.
Originally posted by: mts...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-countdown Patch-push Fixed_2_15_35
Status: Fixed
Originally posted by: ma...@gregoriana.sk
(No comment was entered for this change.)
Status: Verified
.