Repository navigation
fix: back up to a staff's leading direction instead of inventing an offset - #471
Merged
Merged
Conversation
5 tasks done
6 tasks done
webern
marked this pull request as ready for review
October 30, 2026 07:59
…ffset Each staff starts writing where the previous staff ended. A direction at the start of a later staff was written there, before the <backup>, and DirectionWriter added an <offset sound="yes"> to reach its real tick. Harmony and a standalone <sound> got the same <offset>. A figured-bass has no <offset>, so it only came out in the wrong order. When the first pending direction of a staff is behind the write position and not later than the staff's first note, the writer now backs up to it first, so it follows the <backup> as it does in the source. A direction later than the first note, or exactly where the previous staff ended, is written as before. A middle barline at the end of the previous staff is still written ahead of the <backup>. lysuite/ly43e_Multistaff_ClefDynamics.xml, musetrainer/Canon_in_D_3.xml, musetrainer/J._S._Bach_-_Air_on_the_G_String_Piano_arrangement.xml and musetrainer/Maple_Leaf_Rag_Scott_Joplin.xml now round-trip and are pinned in the baseline.
webern
force-pushed
the
m/mxdev-dirbackup
branch
from
October 30, 2026 08:15
1d4fce9 to
002c51e
Compare
webern
added a commit
that referenced
this pull request
Nov 1, 2026
## Summary A figured-bass `<figure>` can end with `<extend/>`, the continuation line that carries a figure across the bass notes that follow. `mx::api` dropped it on read, so a figure holding only an extend came back as an empty `<figure/>`. `FigureData` gains `std::optional<FigureExtend> extend`. Absent means no line. `FigureExtend::type` is a `FigureExtendType` whose `unspecified` value is a bare `<extend/>`; `start`, `continue_` and `stop` write the matching type attribute. Additive; existing callers are unaffected. Why this shape: "no line" and "a line with no type" are different facts (every extend in the corpus file is the bare kind), AGENTS.md rules out `std::optional<SomeEnum>`, and a `hasExtend` + type pair like `LyricData`'s can contradict itself. The one-field struct leaves room for the `<extend>` position and color attributes, which are not modeled yet (none appear in the corpus). `FigureExtendType` repeats `LyricExtendType`'s values rather than reusing a lyric-named type. `musuite/testFiguredBass2.xml` now round-trips and is pinned. It also needs #471, which puts a later staff's leading `<figured-bass>` after the `<backup>`. ## Testing - [x] New `FiguredBassApiTest` cases: round trip, extend-only figure, all four types, written XML for every form plus no extend, and equality - [x] `make api-test` passes (6271 assertions in 735 test cases) - [x] `make api-roundtrip` passes, 452 of 452 pinned files - [x] `make api-roundtrip-discover`: 452 pass, against 451 on the commit below; no file lost its pass - [x] `make fmt-check` passes ## References - Stacked on #471 - Follows #190 (figured-bass in `mx::api`)
webern
added a commit
that referenced
this pull request
Nov 1, 2026
## Summary A `<credit>` with several `<credit-words>` (for example a title followed by "BWV Anhang 114" in a different font style) kept only the first run; the rest of the credit's text was dropped. `PageTextData` gains `std::vector<PageTextSegment> continuations`, following `LyricData::continuations` / `LyricTextSegment`. `PageTextData`'s own fields describe the first run; each later `<credit-words>` becomes a `PageTextSegment` (text, position, font, justify, enclosure). The list is empty in the common single-run case, so the change is additive. Each run stores only the attributes it states, rather than resolved values: font, justify and enclosure carry over from earlier runs in MusicXML, position does not. That keeps output reproducible; a caller who wants a run's effective font looks back through the earlier runs. The reader collects every `<credit-words>` in order and skips `<credit-symbol>`, links and bookmarks. A credit that starts with a symbol now gets its words in `text` (before, they were dropped); the symbol itself is still not modeled. Two corpus files now round-trip and are pinned: - musetrainer/Bach_Minuet_in_G_Major_BWV_Anh._114.xml (also needs #470) - musetrainer/Canon_in_D.xml (also needs #471 and #473) Other multi-run credits in the corpus still fail on run attributes the api does not model (`xml:space`, `xml:lang`, `dir`, `rotation`, `letter-spacing`, `line-height`, `color`, `id`). ## Testing - [x] New `CreditRoundTripTest` cases: continuations survive, single run has none, read from XML, equality, a credit-symbol between words is not a segment, and words after a leading symbol become the text - [x] `make api-test` passes (6362 assertions in 746 test cases) - [x] `make api-roundtrip` passes, 456 of 456 pinned files - [x] `make api-roundtrip-discover`: 456 pass, against 454 on the commit below; no file lost its pass - [x] Unity build (`CMAKE_UNITY_BUILD_BATCH_SIZE=0`) of the `mx` target compiles - [x] `make fmt-check` passes ## References - Stacked on #473 - Related to #187 (earlier credit gaps) and #407 (credit id)
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.
Summary
MeasureWriterwrites each staff starting where the previous staff ended. A direction at the startof a later staff was written there, before the
<backup>, andDirectionWriteradded an invented<offset sound="yes">(e.g.-32) to reach its real tick. Harmony and a standalone<sound>gotthe same offset; a figured-bass, which has no
<offset>, simply came out in the wrong order.When the first pending direction of a staff is behind the write position and not later than the
staff's first note, the writer now backs up to it first, so it follows the
<backup>as it does inthe source. Writer-only; no api change.
Deliberately left as before:
start it belongs to)
Four corpus files now round-trip and are pinned:
Across the failing corpus, files with an invented
add:offsetdrop from 34 to 13 and files withreorder:measurefrom 49 to 29.Testing
MeasureWriterTestcases: direction, harmony and figured-bass at a later staff's start,a single-note staff, a middle barline at the previous staff's end, plus two that pin the cases
left alone (direction at the previous staff's end, direction after the first note)
make api-testpasses (6224 assertions in 730 test cases)make api-roundtrippasses, 451 of 451 pinned filesmake api-roundtrip-discover: 451 pass, against 447 on the commit below; no file lost its passmake fmt-checkpassesReferences