feat: model cue-note ties as notation, stop duplicating <tied>, stabilize note sorting - #379
Open
rpatters1 wants to merge 2 commits into
Open
feat: model cue-note ties as notation, stop duplicating <tied>, stabilize note sorting#379rpatters1 wants to merge 2 commits into
rpatters1 wants to merge 2 commits into
Conversation
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
Closes a gap in cue-note ties through mx::api, allowing them to be emitted as notation-only ties.
Summary
MusicXML states a tie twice:
<tie>for sound and<tied>for notation. Two defects cameout of how
mx::implhandled that split. A third, unrelated fix is folded in below.Cue and grace-cue notes dropped their ties entirely. Half of that was right - the schema
gives
<tie>a slot in only two of the four note-choice branches (normal and grace-normal),because a cue note is silent and a sound-level tie on it is meaningless. But
<notations>sits outside the note choice, so
<tied>is legal on all four flavors, and it was beingdropped along with the
<tie>.NoteWriter::addTienow gates only thecore::Tiepush onisCueand always records the<tied>;NoteReaderreadsisTieStart/isTieStopbackfrom the
<tied>notation for those two flavors, since they have no<tie>to read from.Grace-normal notes already round-tripped ties and are unchanged.
The second defect:
NoteWritersynthesized a bare<tied>from the tie flags while thereader routed source
<tied>intonoteAttachmentData.curveStarts/curveStops, whichNotationsWriteralso emits with its full attributes. Any note whose source had bothelements wrote
<tied>twice - exact doubling, e.g.Adagio and Fugue in C minor, K.546.xmlwent from 52
<tied type="start">to 104.NoteWriter::hasTieCurvenow suppresses thesynthesized one when a curve already covers that direction. Authoring a flag alone still
emits both encodings, so the api keeps one field for one fact (doctrine principle 3).
data/synthetic/tied.cue.4.0.xmldefends both halves: measure 1 has cue and grace-cue tiepairs (notation only), measure 2 a grace-normal pair carrying both
<tie>and<tied>.cueNoteTiesAreDroppedpinned the old behavior, so it is nowcueNoteTiesAreNotationOnly,joined by
tiedNotationIsNotDuplicated. NewScoreDataSortTest.cppcovers sort stability. Themx-api-doctrineskill cited "cue-note ties are silently dropped" as its example of a defined fallback; that clause is now inaccurate and was rewritten.Separately:
ScoreWritercallsScoreData::sort(), which ordered notes, clefs, anddirections by
tickTimePositionalone usingstd::sort. Every member of a chord shares onetick, so they all compare equal, and
std::sortis not stable - their relative order wasleft to the standard library. libc++ happened to leave them alone; libstdc++ permutes them.
Since MusicXML spells a chord by omitting
<chord>from its first note, that rewrites thechord. Switched to
std::stable_sort; the comparators now takeconst&, whichstable_sortrequires and which was the right signature anyway.This one is pre-existing and platform-dependent, not fallout from the tie work. It surfaced because
ly32d_Arpeggio.xml- 14 chords - passed on macOS and was pinned on the strength of a macOS-only run, then failed the Linux CI job withtext mismatch at .../note/pitch/step: expected 'C', actual 'E'. Its baseline comment now records why.Not addressed: a normal note whose source has
<tie>but no<tied>still gains a<tied>on write (visible in
ly33b_Spanners_Tie.xml). That is pre-existing principle-3normalization, left alone deliberately.
Testing
make api-roundtrip: 351 passed, 0 failed - baseline grew 295 -> 351before/after pass-lists across a revert; every previously pinned file still passes
that already passed but were never pinned (
ly32d_Arpeggio.xml,multiple-rest.3.0.xml)make api-test: all tests pass (5231 assertions in 464 test cases)make core-roundtrip-test: all tests pass (838 test cases); pinned count bumped to 837and
make auditregeneratedcorpus.xmlplus the new sidecarcueNoteTiesAreNotationOnlyandtiedNotationIsNotDuplicatedfail before the fix,pass after
References
mx::api)