Am Sonntag, den 22.11.2020, 21:12 +0100 schrieb Han-Wen Nienhuys: > On Sun, Nov 22, 2020 at 1:55 PM Jonas Hahnfeld <[email protected]> > wrote: > > footnote-volta-spanner.ly was added by > > commit c1f44faf958aafe7802f432a5ae0f4c74a8c0146 > > Author: Han-Wen Nienhuys <[email protected]> > > Date: Tue Jun 16 08:59:35 2020 +0200 > > > > For footnotes on spanners, copy bounds from underlying spanner. > > > > This is likely more correct for spanners that run from non-musical > > columns (eg. volta brackets) or are created retroactively > > (eg. automatic beams). > > > > Demonstrate this by adding a regression test that puts a footnote on a > > VoltaBracket. > > > > Without adjustments of Footnote_engraver, the test produces the > > following: > > > > programming error: Spanner `FootnoteSpanner' is not fully contained > > in parent spanner. Ignoring orphaned part > > continuing, cross fingers > > programming error: bounds of this piece aren't breakable. > > continuing, cross fingers > > > > (https://gitlab.com/lilypond/lilypond/-/merge_requests/138). During > > review, David had some concerns about Spanners that were dismissed on > > the basis that there was no test case that showed wrong results (is > > that really our standard for claiming that a change works correctly?!). > > > > I tried reverting that commit and all other differences also went away. > > Does this mean the change is indeed flawed?
Nope, that was a red herring: The reason is that the footnote creation
process in footnote-volta-spanner.ly messes with (point-stencil), which
is used by skyline-point-extent.ly and also the BendSpanner. As far as
I understand the Stencil class, its objects are not immutable...
One of the following two changes fixes the problem in my local test
scenario:
---
diff --git a/ly/music-functions-init.ly b/ly/music-functions-init.ly
index 13f5c37811..17f14b1f07 100644
--- a/ly/music-functions-init.ly
+++ b/ly/music-functions-init.ly
@@ -538,7 +538,7 @@ to the preceding note or rest as a post-event with
@code{-}.")
'X-offset (car offset)
'Y-offset (cdr offset)
'automatically-numbered (not mark)
- 'text (or mark (make-null-markup))
+ 'text (or mark (ly:make-stencil "" '(0 . 0) '(0 . 0)))
'footnote-text footnote)))
(once (propertyTweak 'footnote-music mus item))))
diff --git a/scm/define-markup-commands.scm b/scm/define-markup-commands.scm
index f7280f7a58..1ab6eaafe3 100644
--- a/scm/define-markup-commands.scm
+++ b/scm/define-markup-commands.scm
@@ -1379,7 +1379,8 @@ An empty markup with extents of a single point.
\\null
}
@end lilypond"
- point-stencil)
+ ;; Create a new point-stencil every time, it might get modified...
+ (ly:make-stencil "" '(0 . 0) '(0 . 0)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; basic formatting.
---
My gut feeling is that we should fix null-markup because nothing should
ever translate / rotate / modify the global point-stencil, thoughts?
I'll try to have a look this week (likely Friday). It may be prudent to
revert the change if 2.22 is imminent.
Actually CG says that whatever code compiled with a beta release must
work with later releases of that series. I think that makes sense and
we should stick to it...
Jonas
signature.asc
Description: This is a digitally signed message part
