fix: write normal-type from DurationData instead of inferring it from sibling notes - #433
Merged
Merged
Conversation
… sibling notes NoteWriter scanned sibling notes for a TupletStart/TupletStop pair and recomputed <normal-type> from their tick distance, writing the element on every note of every tuplet and ignoring DurationData::timeModificationNormalType entirely. Nested tuplets broke the sibling search and lost the element altogether. The writer now emits <normal-type> (and <normal-dot>) exactly when timeModificationNormalType is set, and omits it when unspecified. On the read side, NoteReader records whether the source <time-modification> carried a <normal-type>, and NoteFunctions maps absence to unspecified instead of guessing from a dots-and-type heuristic that dropped explicit undotted values. Closes #428
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
I am not quite sure what I was going for when I had the writer attempting to derive normal-note from the tick time positions. I think I wa trying to make a more fully automatic tuplet experience, but it definitely did not work correctly. This implements the writer in the way that the api struct implies which should fix the problem. In other words, you are now expected to fill normal-note yourself when it is needed and the api has no opinion about whether you are using it correctly.
I created #434 to track the idea of a tuplet helper.
Summary
NoteWriterignoredDurationData::timeModificationNormalTypeand instead recomputednormal-typeby scanning sibling notes for aTupletStart/TupletStoppair and measuring the tick distance between them. That wrote the element on every note of every tuplet (253 written for 11 requested in the issue's test document), and a note that opens two tuplets broke the sibling search so nothing was written no matter what the author asked for.The writer now treats the field as the author's statement: it emits
normal-type(andnormal-dot) exactly whentimeModificationNormalTypeis set and omits it when unspecified, which MusicXML reads as the note's own type. The sibling scan,findNormalNameAndDots, and the writer's note-index plumbing are deleted. (NoteWriterstill receives the voice's notes, which the<voice>-element logic from #422 uses.)For the round trip,
NoteReadernow records whether the sourcetime-modificationactually carried anormal-type, andNoteFunctionsmaps absence tounspecified. This replaces a heuristic (dots > 0 && type != note type) that dropped an explicit undottednormal-typeon read, which the writer's inference then papered over on write.Everything is in the
mx::impllayer; no public header changed. Behavior of written output changes: an author who never setstimeModificationNormalTypeno longer gets an inferrednormal-typein the output.The two
FreezingRoundTriptests that pinned the old behavior now assert presence parity: a saved note carriesnormal-typeexactly where the source did, with the same value. ThePreserveTimeModificationNormalType.xmlcase also checks that a meaningfulnormal-type(differing from the note's own type) survives, and that an absent one is not invented.I created issue #434 to track the idea of a tuplet helper.
Testing
checkMissingNormalTypeSimpleasserts the fix: the source'snormal-typesurvives and an absent one is not fabricatedmake api-testpasses (5463 assertions in 597 test cases)make api-roundtripregression gate passes (411/411 pinned files)make fmtapplied;make fmt-checkpassesReferences