fix: write single-note spanner starts before their stops - #431
Merged
Conversation
tuplet_singletons-ref.xml and gliss_to_rest-ref.xml, attached by rpatters1 to issue #429: Finale's exports of a bar of two single-note tuplets and of a glissando and a slide drawn to a rest. Both place a spanner's start and stop on one note in start-then-stop order -- the shape NotationsWriter currently inverts. Pinned corert count 838 -> 840; audit artifacts regenerated.
NotationsWriter wrote every spanner stop before every start, which is right for a note that chains two spanners but backwards for a spanner contained in one note: a nested tuplet covering exactly one note, or a glissando or slide drawn to a rest, came out stop-then-start, closing a span that was never open (#429). Tuplets are decided locally: matching numberLevels on one note always mean a single-note tuplet, so each start is followed by its same-note stop and the remaining stops come last. Glissando, slide, and wavy line cannot be decided locally -- a chained note can carry the same number as a single-note span -- so the SpannerResolver's serialized walk now records the pairs (a stop that closes nothing open pairs with a same-bucket start on its own note) and the writer holds those stops back until right after their starts. Chained spanners keep the stop-before-start order of #139.
The #429 fixture passes the strict read -> write -> read comparison now that single-note spanners are written start-first, so it defends the ordering. tuplet_singletons-ref.xml is not pinned: its ordering is fixed too, but the writer restates tuplet-actual/tuplet-normal on a start whose source omits them, which is a separate fidelity gap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
When a spanner starts and stops on the same note, the start needs to be emitted before the stop, but mx had not considered single note spanners.
This looks like a solid fix for, at least the known and cited cases from #429. I will not close the issue so we can track future cases of out of order spanner start and stop problems.
The test file submitted in the issue has been added to the corpus and passes round trip now.
Summary
NotationsWriter wrote every spanner stop before every start. That is right for a note that chains two spanners (one ends where the next begins, #139), but backwards for a spanner contained in one note: a nested tuplet covering exactly one note, or a glissando or slide drawn from a note toward a rest, came out stop-then-start, so a reader saw a stop for a span that was never open followed by a start that never closes.
Tuplets are decided locally in NotationsWriter: matching numberLevels on one note always mean a single-note tuplet (two different tuplets cannot share a note, because a note carries only one time-modification), so each start is followed by its same-note stop and the remaining stops come last. The nested case now comes out as Finale writes it: start 2, stop 2, stop 1.
Glissando, slide, and wavy line cannot be decided locally: a chained note can carry the same number as a single-note span, and only the stream of earlier events tells them apart. The SpannerResolver's existing serialized walk now also records same-note pairs (a stop that closes nothing open pairs with a same-bucket start on its own note) and exposes them through sameNoteSpanPartner; the writer holds those stops back until right after their starts. Chained spanners keep the stop-before-start order of #139, covered by the existing GlissandoApiTest chain test.
The reporter's fixtures from the issue are added to the corpus as data/rpatters1/gliss_to_rest-ref.xml and tuplet_singletons-ref.xml (Finale's exports, both with the start-then-stop shape). The gliss fixture now survives the strict api roundtrip and is pinned in roundtrip-baseline.txt. The tuplet fixture is not pinned: its ordering is fixed, but the writer restates tuplet-actual/tuplet-normal on a start whose source omits them, which is a separate fidelity gap.
Testing
*_SingleNoteSpannertests fail before the fix (7 of 8 cases, all start/stop inversions), pass after (36 assertions in 8 test cases)References