feat: round-trip metronome attributes and non-numeric per-minute - #362
Merged
Conversation
The <metronome> element carried only its parentheses attribute and a
numeric per-minute; every other attribute was silently dropped. Model
the full print-style-align set (position, font, color, halign/valign)
plus justify, print-object, and id on TempoData, populated by the reader
and emitted by the writer.
per-minute is xs:string in MusicXML ("120", "ca. 76", a range), but the
api held it as an int with a lossy tempoText fallback that discarded the
beat-unit and was never written back. BeatsPerMinute::beatsPerMinute is
now a std::string kept verbatim, so a mark like "quarter = ca. 76"
round-trips. The now-unreachable TempoType::tempoText enumerator and the
TempoText struct are removed. A player needing a numeric playback tempo
should read SoundData::tempo, which is numeric and expressed in quarter
notes per minute.
Round-trip baseline 272 -> 275 (the synthetic metronome kitchen-sink
fixtures). Real-world files that reported an attribute mismatch at the
metronome now progress past it to their next divergence.
First metronome slice of #324.
4 tasks
webern
added a commit
that referenced
this pull request
Jul 19, 2026
## Human Summary Unfortunately this is quite complicated, but it appears to correctly model the various forms of metronome markings that MusicXML supports. ## Summary Second and final metronome slice of #324. `TempoData`'s enum-plus-parallel-payload shape is replaced by a `TempoChoice` choice class (the `TimeChoice` / `PercussionDataChoice` pattern) over the three `<metronome>` bodies: beats-per-minute, metric modulation, and the note-relation form. The unreachable `unspecified` tempo sentinel is gone — a default/empty tempo is skipped by the writer on a content check, so nothing throws. The note-relation form — metric relationships drawn with note figures (`metronome-note` and `metronome-relation`, plus per-figure `metronome-beam`, `metronome-dot`, `metronome-tied`, and `metronome-tuplet`, and the `metronome-arrows` flag) — is modeled in a new `NoteRelationData.h` with dedicated mini-structs. They reuse only the leaf enums (`DurationName`, `Beam`); a metronome note figure has none of the pitch/stem/staff/voice machinery of a real note, so `NoteData` is not reused. `beat-unit-tied` (a beat-unit tied to the preceding one, e.g. "quarter + eighth = 120") is modeled on both the beats-per-minute and metric-modulation forms. Every part of `<metronome>` now round-trips except per-minute's own font override, which is a documented drop — the font belongs on the metronome element, and no real-world file uses the per-minute font. Breaking: `TempoData::tempoType` and the parallel `beatsPerMinute` / `metricModulation` fields are replaced by `TempoData::choice`. ## Testing - [x] New `roundTripNoteRelation` (strong whole-body equality check), `roundTripBeatUnitTied`, plus `roundTripMetricModulation`/`roundTripBpm`/attributes migrated to the choice API - [x] Full unit suite passes (5105 assertions in 449 test cases) - [x] Round-trip regression 284/284 pinned, no regressions - [x] Round-trip baseline 275 -> 284 (all eight metronome-note fixtures plus `beat-unit-tied`) ## References - Progresses #324 - Stacked on #362
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
1 participant
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
I am reluctantly making the interface more and more complicated as we hill climb against the test corpus. This one makes metronome beats per minute a string (it was an int) and adds support for its attributes.
Summary
The
<metronome>element only round-tripped itsparenthesesattribute and a numeric per-minute; every other attribute was silently dropped, and a non-numeric per-minute discarded the beat-unit and was never written back.This models the metronome's full attribute surface on
TempoData— theprint-style-aligngroup (position, font, color, halign/valign) plusjustify,print-object, andid— populated byMetronomeReaderand emitted byDirectionWriter.per-minute is
xs:stringin MusicXML ("120", "ca. 76", a range).BeatsPerMinute::beatsPerMinutechanges frominttostd::stringand is kept verbatim, so "quarter = ca. 76" now round-trips. The unreachableTempoType::tempoTextenumerator and theTempoTextstruct are removed. A numeric playback tempo is available separately onSoundData::tempo(quarter notes per minute).Breaking:
BeatsPerMinute::beatsPerMinuteis now a string, andTempoType::tempoText/TempoTextare gone. The metronome-note form and per-minute's own font are still not modeled (the note form is the next slice; per-minute's font is a documented drop).Testing
roundTripMetronomeAttributesandnonNumericPerMinuteRoundTripsunit testsReferences