diff --git a/.claude/skills/mx-api-doctrine/SKILL.md b/.claude/skills/mx-api-doctrine/SKILL.md index c18f4b4fb..24c801358 100644 --- a/.claude/skills/mx-api-doctrine/SKILL.md +++ b/.claude/skills/mx-api-doctrine/SKILL.md @@ -40,8 +40,9 @@ Responses to wrong api usage, in order of preference: 1. Unrepresentable: shape the type so the wrong state cannot be expressed (choice types below; merged fields, principle 3). 2. Defined fallback: document a harmless result and return it. A wrong-kind choice accessor - returns a default-constructed copy; the writer emits nothing for a meaningless combination - (cue-note ties are silently dropped). No signal to the caller. + returns a default-constructed copy; the writer drops the half of an encoding that is + meaningless for the note it is on (a tie on a silent cue note is written as `` + notation only, never as a sound-level ``). No signal to the caller. 3. `Result` (`Result.h`): the error channel of last resort. It exists for the `DocumentManager` I/O boundary, where failure is real (unreadable file, unparseable XML). Do not spread it into the data model. diff --git a/data/corpus.xml b/data/corpus.xml index 2e23e3030..7496c6c9a 100644 --- a/data/corpus.xml +++ b/data/corpus.xml @@ -4,13 +4,13 @@ usage (descending). Do not edit by hand; regenerate with the tool. --> - + - + - + @@ -32,7 +32,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -386,6 +386,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -426,9 +427,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -445,7 +446,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -799,6 +800,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -839,7 +841,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -855,7 +857,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -1209,6 +1211,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -1249,7 +1252,7 @@ synthetic/work-title.3.0.xml - + @@ -1278,7 +1281,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -1632,6 +1635,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -1672,9 +1676,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -1691,7 +1695,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2045,6 +2049,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -2085,9 +2090,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -2104,7 +2109,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2458,6 +2463,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -2498,7 +2504,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2514,7 +2520,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -2529,10 +2535,11 @@ synthetic/segno.3.0.xml synthetic/segno.3.1.xml synthetic/suffix.3.0.xml + synthetic/tied.cue.4.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2548,7 +2555,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/beat-repeat.3.0.xml synthetic/beat-type.3.0.xml synthetic/beats.3.0.xml @@ -2585,6 +2592,7 @@ synthetic/slash.3.0.xml synthetic/staff-size.4.0.xml synthetic/staff-type.3.0.xml + synthetic/tied.cue.4.0.xml synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml @@ -2593,7 +2601,7 @@ synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2609,7 +2617,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -2618,10 +2626,11 @@ synthetic/key-accidental.smufl.3.1.xml synthetic/segno.3.0.xml synthetic/segno.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/words-symbol.4.0.xml - + @@ -2660,7 +2669,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2860,6 +2869,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -2978,7 +2988,7 @@ synthetic/words-symbol.4.0.xml - + @@ -2997,7 +3007,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -3006,6 +3016,7 @@ synthetic/key-accidental.smufl.3.1.xml synthetic/segno.3.0.xml synthetic/segno.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/words-symbol.4.0.xml @@ -3115,7 +3126,7 @@ synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3131,7 +3142,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3407,6 +3418,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/timpani.3.0.xml synthetic/timpani.4.0.xml synthetic/toe.3.0.xml @@ -3441,7 +3453,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3457,7 +3469,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3657,6 +3669,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -3684,7 +3697,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3700,7 +3713,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3900,6 +3913,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -3927,7 +3941,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3943,7 +3957,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -4143,6 +4157,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -4597,7 +4612,7 @@ - + @@ -4617,7 +4632,7 @@ foundsuite/Invention_10.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -4720,6 +4735,7 @@ synthetic/thumb-position.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -5750,9 +5766,9 @@ - + - + @@ -5787,16 +5803,17 @@ foundsuite/Invention_12.xml - + synthetic/notations.3.0.xml synthetic/notations.3.1.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml - + - + @@ -5814,8 +5831,9 @@ foundsuite/Invention_12.xml - + synthetic/tie.3.0.xml + synthetic/tied.cue.4.0.xml @@ -6439,7 +6457,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6459,7 +6477,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6483,7 +6501,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6505,7 +6523,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6524,7 +6542,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6565,7 +6583,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6603,7 +6621,7 @@ foundsuite/PepAiraSco.xml foundsuite/PezR44Sco.xml foundsuite/RonCLunSco.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml kiritan_singing/kiritan_singing_01.xml kiritan_singing/kiritan_singing_02.xml kiritan_singing/kiritan_singing_03.xml @@ -6970,7 +6988,7 @@ foundsuite/O_Holy_Night-Adam-1871.xml foundsuite/O_Holy_Night.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/Bombe.xml ksuite/k011a_Tuplets.xml @@ -6990,7 +7008,7 @@ foundsuite/O_Holy_Night-Adam-1871.xml foundsuite/O_Holy_Night.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/Bombe.xml ksuite/k011a_Tuplets.xml @@ -7013,7 +7031,7 @@ foundsuite/RonCLunSco.xml foundsuite/Silent_Night-Hartwig.xml foundsuite/Silent_Night_Young_1.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k006a_Header_Scaling_Decimals.xml ksuite/k013a_OrchestralScoreFinale.xml @@ -7033,7 +7051,7 @@ foundsuite/O_Holy_Night-Adam-1871.xml foundsuite/O_Holy_Night.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/Bombe.xml ksuite/k011a_Tuplets.xml @@ -7110,7 +7128,7 @@ foundsuite/O_Holy_Night-Adam-1871.xml foundsuite/O_Holy_Night.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/Bombe.xml ksuite/k011a_Tuplets.xml @@ -7312,7 +7330,7 @@ - + @@ -7334,7 +7352,7 @@ lysuite/ly33f_Trill_EndingOnGraceNote.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -7527,6 +7545,7 @@ synthetic/tie.3.0.xml synthetic/tied.3.0.xml synthetic/tied.3.1.xml + synthetic/tied.cue.4.0.xml synthetic/toe.3.0.xml synthetic/tremolo.3.0.xml synthetic/tremolo.3.1.xml @@ -7705,7 +7724,7 @@ foundsuite/Berlioz_Le_Corsaire.xml foundsuite/PezR44Sco.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013a_OrchestralScoreFinale.xml ksuite/k013b_OrchestralScoreSibelius.xml logicpro/logic01a_homoSapiens.xml @@ -8117,7 +8136,7 @@ foundsuite/Berlioz_Le_Corsaire.xml foundsuite/PezR44Sco.xml foundsuite/SCHUBERT An die Sonne.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013a_OrchestralScoreFinale.xml ksuite/k013b_OrchestralScoreSibelius.xml logicpro/logic01a_homoSapiens.xml @@ -8179,7 +8198,7 @@ foundsuite/Berlioz_Le_Corsaire.xml foundsuite/RonCLunSco.xml foundsuite/Schubert_der_Mueller.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013b_OrchestralScoreSibelius.xml ksuite/k015a_System_Layout.xml lysuite/ly14a_StaffDetails_LineChanges.xml @@ -8283,7 +8302,7 @@ foundsuite/Berlioz_Le_Corsaire.xml foundsuite/Black Note Study Op 10 no 5.xml foundsuite/Moments Musicaux Op16 No4.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/Bombe.xml ksuite/k011a_Tuplets.xml lysuite/ly23c_Tuplet_Display_NonStandard.xml @@ -9164,7 +9183,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k001b_Articulations_Above.xml ksuite/k001c_Articulations_Below.xml ksuite/k004a_Technical.xml @@ -9291,7 +9310,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k001b_Articulations_Above.xml ksuite/k001c_Articulations_Below.xml ksuite/k004a_Technical.xml @@ -9407,7 +9426,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013a_OrchestralScoreFinale.xml ksuite/k013b_OrchestralScoreSibelius.xml lysuite/ly41c_StaffGroups.xml @@ -9610,7 +9629,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013a_OrchestralScoreFinale.xml ksuite/k013b_OrchestralScoreSibelius.xml lysuite/ly41i_PartNameDisplay_Override.xml @@ -10589,21 +10608,22 @@ synthetic/coda.3.1.xml - + mjbsuite/krz_v40.xml recsuite/ActorPreludeSample.xml recsuite/MahlFaGe4Sample.xml recsuite/Telemann.xml - + synthetic/grace-cue.4.0.xml + synthetic/tied.cue.4.0.xml foundsuite/Schubert_der_Mueller.xml - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k006a_Header_Scaling_Decimals.xml ksuite/k013b_OrchestralScoreSibelius.xml @@ -11098,7 +11118,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k006a_Header_Scaling_Decimals.xml ksuite/k013b_OrchestralScoreSibelius.xml @@ -11227,7 +11247,7 @@ - foundsuite/Απτάλικο.xml + foundsuite/Απτάλικο.xml ksuite/k013b_OrchestralScoreSibelius.xml recsuite/Echigo_Jishi.xml diff --git a/data/synthetic/tied.cue.4.0.features.xml b/data/synthetic/tied.cue.4.0.features.xml new file mode 100644 index 000000000..05d27c403 --- /dev/null +++ b/data/synthetic/tied.cue.4.0.features.xml @@ -0,0 +1,87 @@ + + + + synthetic/tied.cue.4.0.xml + 4.0 + + + attributes + + + cue + + + divisions + + + duration + + + grace + + + measure + + number + + + + notations + + + note + + + octave + + + part + + id + + + + part-list + + + part-name + + + pitch + + + score-part + + id + + + + score-partwise + + version + + + + step + + + tie + + type + + + + tied + + type + + + + type + + + voice + + + diff --git a/data/synthetic/tied.cue.4.0.xml b/data/synthetic/tied.cue.4.0.xml new file mode 100644 index 000000000..961dfcaf6 --- /dev/null +++ b/data/synthetic/tied.cue.4.0.xml @@ -0,0 +1,104 @@ + + + + + x + + + + + + 4 + + + + + C + 4 + + 4 + 1 + quarter + + + + + + + + C + 4 + + 4 + 1 + quarter + + + + + + + + + D + 4 + + 1 + eighth + + + + + + + + D + 4 + + 8 + 1 + half + + + + + + + + + + F + 4 + + + 1 + eighth + + + + + + + F + 4 + + 8 + + 1 + half + + + + + + + E + 4 + + 8 + 1 + half + + + + diff --git a/src/include/mx/api/NoteData.h b/src/include/mx/api/NoteData.h index 488105000..79129b482 100644 --- a/src/include/mx/api/NoteData.h +++ b/src/include/mx/api/NoteData.h @@ -112,8 +112,11 @@ class NoteData // tag, but subsequent chord notes do have the tag). bool isChord; - // One field, two encodings: on write these emit both (sound) and - // (notation), so the two can never contradict each other. + // One field, two encodings: MusicXML states a tie twice, once for sound + // () and once for notation (), so setting one of these emits + // both and the two can never contradict each other. On a cue or grace-cue + // note only the notation is emitted -- those notes are silent, and the + // schema gives them no -- so the tie is visible but not played. bool isTieStart; bool isTieStop; @@ -125,7 +128,8 @@ class NoteData // are the four combinations of these two independent flags. Schema facts: // a grace note carries no on the wire (durationTimeTicks reads // as 0 and is ignored on write), and cue notes -- including grace-cue - // notes -- cannot carry (ties on them are silently dropped on write). + // notes -- cannot carry , so a tie on one is notation only (see + // isTieStart / isTieStop). bool isGrace; // 's slash attribute. Only meaningful when isGrace is true. Bool graceSlash; diff --git a/src/private/mx/api/ScoreData.cpp b/src/private/mx/api/ScoreData.cpp index 0fcecb921..8790b6cb9 100644 --- a/src/private/mx/api/ScoreData.cpp +++ b/src/private/mx/api/ScoreData.cpp @@ -4,6 +4,8 @@ #include "mx/api/ScoreData.h" +#include + namespace mx { namespace api @@ -45,6 +47,11 @@ int ScoreData::getNumStavesPerSystem() const return numStaves; } +// Sorts by tick position only, so items sharing a tick keep the order they were +// given. That order is meaningful and must not be disturbed: the members of a +// chord all sit at one tick, and MusicXML encodes a chord by omitting +// from its first note. A plain std::sort is unstable, so the chord's spelling +// would depend on the standard library implementation. void ScoreData::sort() { for (auto &part : parts) @@ -54,25 +61,25 @@ void ScoreData::sort() for (auto &staff : measure.staves) { - const auto clefCompare = [&](ClefData &inLeft, ClefData &inRight) { + const auto clefCompare = [](const ClefData &inLeft, const ClefData &inRight) { return inLeft.tickTimePosition < inRight.tickTimePosition; }; - std::sort(std::begin(staff.clefs), std::end(staff.clefs), clefCompare); + std::stable_sort(std::begin(staff.clefs), std::end(staff.clefs), clefCompare); - const auto directionCompare = [&](DirectionData &inLeft, DirectionData &inRight) { + const auto directionCompare = [](const DirectionData &inLeft, const DirectionData &inRight) { return inLeft.tickTimePosition < inRight.tickTimePosition; }; - std::sort(std::begin(staff.directions), std::end(staff.directions), directionCompare); + std::stable_sort(std::begin(staff.directions), std::end(staff.directions), directionCompare); for (auto &voice : staff.voices) { - const auto noteCompare = [&](NoteData &inLeft, NoteData &inRight) { + const auto noteCompare = [](const NoteData &inLeft, const NoteData &inRight) { return inLeft.tickTimePosition < inRight.tickTimePosition; }; - std::sort(std::begin(voice.second.notes), std::end(voice.second.notes), noteCompare); + std::stable_sort(std::begin(voice.second.notes), std::end(voice.second.notes), noteCompare); } } } diff --git a/src/private/mx/impl/NoteReader.cpp b/src/private/mx/impl/NoteReader.cpp index 25b3796f7..e47d577a8 100644 --- a/src/private/mx/impl/NoteReader.cpp +++ b/src/private/mx/impl/NoteReader.cpp @@ -8,8 +8,11 @@ #include "mx/core/generated/LyricChoice.h" #include "mx/core/generated/LyricSyllableGroup.h" #include "mx/core/generated/LyricTextGroup.h" +#include "mx/core/generated/Notations.h" +#include "mx/core/generated/NotationsChoice.h" #include "mx/core/generated/Syllabic.h" #include "mx/core/generated/TextElementData.h" +#include "mx/core/generated/Tied.h" #include "mx/impl/FontFunctions.h" #include "mx/impl/PositionFunctions.h" #include "mx/impl/PrintFunctions.h" @@ -221,8 +224,10 @@ void NoteReader::setNormalGraceCueItems() else { // + : a grace note inside a cue passage. The - // grace-cue group carries no in the schema. + // grace-cue group has no slot in the schema, so the tie can + // only be stated as a notation. myIsCue = true; + setTieFromNotations(); } break; } @@ -230,6 +235,8 @@ void NoteReader::setNormalGraceCueItems() myIsCue = true; const auto ¬eGuts = myNoteChoice.asCueNoteGroup(); myDurationValue = noteGuts.duration().value().value(); + // A cue note is silent and has no slot either; same as grace-cue. + setTieFromNotations(); break; } default: @@ -478,6 +485,35 @@ void NoteReader::setTie(std::span tieSet) } } +// Cue and grace-cue notes have no slot in the schema, so their tie can +// only be stated as a notation. Reads the start/stop flags from there. +// (A lone is a different thing and is read elsewhere, +// into NoteData::tieLetRing.) +void NoteReader::setTieFromNotations() +{ + for (const auto ¬ations : myNote.notations()) + { + for (const auto ¬ationsChoice : notations.choice()) + { + if (notationsChoice.kind() != core::NotationsChoice::Kind::tied) + { + continue; + } + + const auto type = notationsChoice.asTied().type(); + + if (type == core::TiedType::start()) + { + myIsTieStart = true; + } + else if (type == core::TiedType::stop()) + { + myIsTieStop = true; + } + } + } +} + void NoteReader::setLyric() { const auto lyricSet = myNote.lyric(); diff --git a/src/private/mx/impl/NoteReader.h b/src/private/mx/impl/NoteReader.h index 413dd7abc..a4591b06f 100644 --- a/src/private/mx/impl/NoteReader.h +++ b/src/private/mx/impl/NoteReader.h @@ -311,6 +311,7 @@ class NoteReader void setAccidental(); void setStem(); void setTie(std::span tieSet); + void setTieFromNotations(); void setLyric(); }; } // namespace impl diff --git a/src/private/mx/impl/NoteWriter.cpp b/src/private/mx/impl/NoteWriter.cpp index fcef3c10b..8e3381b5e 100644 --- a/src/private/mx/impl/NoteWriter.cpp +++ b/src/private/mx/impl/NoteWriter.cpp @@ -34,6 +34,7 @@ #include "mx/impl/WriteRefusal.h" #include "mx/utility/Throw.h" +#include #include namespace mx @@ -270,37 +271,60 @@ core::Note NoteWriter::getNote(bool isStartOfChord) const return myOutNote; } -// Records a tie (for the note choice) and the matching tied notation. Old -// behavior preserved: a single element collects the tied -// choices, stop before start when both are present. +// True when the note's curve vectors already carry a tie curve in the given +// direction. Those curves are written by NotationsWriter with their full +// attributes, so the bare synthesized here would be a duplicate. +bool NoteWriter::hasTieCurve(bool isStart) const +{ + if (isStart) + { + const auto &curves = myNoteData.noteAttachmentData.curveStarts; + return std::any_of(curves.cbegin(), curves.cend(), + [](const api::CurveStart &curve) { return curve.curveType == api::CurveType::tie; }); + } + + const auto &curves = myNoteData.noteAttachmentData.curveStops; + return std::any_of(curves.cbegin(), curves.cend(), + [](const api::CurveStop &curve) { return curve.curveType == api::CurveType::tie; }); +} + +// Records a tie in both of MusicXML's encodings: the sound-level (for the +// note choice) and the matching notation. A single element +// collects the tied choices, stop before start when both are present. void NoteWriter::addTie(bool isStart) const { - core::Tie tie; - tie.setType(isStart ? core::StartStop::start() : core::StartStop::stop()); - myOutTies.push_back(std::move(tie)); + // lives inside the note choice, and the schema gives it a slot in only + // two of the four note flavors: normal and grace-normal. Cue and grace-cue + // notes are silent, so a sound-level tie is meaningless on them. + if (!myNoteData.isCue) + { + core::Tie tie; + tie.setType(isStart ? core::StartStop::start() : core::StartStop::stop()); + myOutTies.push_back(std::move(tie)); + } - core::Tied tied; - tied.setType(isStart ? core::TiedType::start() : core::TiedType::stop()); - myOutTieNotationsChoices.push_back(core::NotationsChoice::tied(std::move(tied))); + // is a notation and sits outside the note choice, so it + // is legal on all four flavors -- cue and grace-cue notes included. + if (!hasTieCurve(isStart)) + { + core::Tied tied; + tied.setType(isStart ? core::TiedType::start() : core::TiedType::stop()); + myOutTieNotationsChoices.push_back(core::NotationsChoice::tied(std::move(tied))); + } } void NoteWriter::setNoteChoiceAndFullNoteGroup(bool isStartOfChord) const { myOutFullNoteGroup.setChord(myCursor.isChordActive && myIsPreviousNoteAChordMember && !isStartOfChord); - // The schema has no on cue and grace-cue notes, so ties on them are - // silently dropped. Normal and grace-normal notes carry their ties. - if (!myNoteData.isCue) + if (myNoteData.isTieStop) { - if (myNoteData.isTieStop) - { - addTie(false); - } + addTie(false); + } - if (myNoteData.isTieStart) - { - addTie(true); - } + if (myNoteData.isTieStart) + { + addTie(true); } } @@ -324,7 +348,8 @@ void NoteWriter::assembleNoteChoice() const } if (myNoteData.isCue) { - // + : the grace-cue group carries no . + // + : the grace-cue group has no slot, so + // myOutTies is empty here; the tie survives as a notation. core::GraceCueNoteGroup inner; inner.setFullNote(myOutFullNoteGroup); choiceObj.setGraceNoteChoice(core::GraceNoteChoice::graceCueNoteGroup(std::move(inner))); diff --git a/src/private/mx/impl/NoteWriter.h b/src/private/mx/impl/NoteWriter.h index 78ca6ac0c..1073e83bf 100644 --- a/src/private/mx/impl/NoteWriter.h +++ b/src/private/mx/impl/NoteWriter.h @@ -44,6 +44,7 @@ class NoteWriter mutable std::vector myOutTieNotationsChoices; private: + bool hasTieCurve(bool isStart) const; void addTie(bool isStart) const; void setNoteChoiceAndFullNoteGroup(bool isStartOfChord) const; void assembleNoteChoice() const; diff --git a/src/private/mxtest/api/GraceCueApiTest.cpp b/src/private/mxtest/api/GraceCueApiTest.cpp index e6acfd18e..3cb32e734 100644 --- a/src/private/mxtest/api/GraceCueApiTest.cpp +++ b/src/private/mxtest/api/GraceCueApiTest.cpp @@ -193,7 +193,9 @@ TEST(writeGraceCueElements, GraceCue) T_END; -TEST(cueNoteTiesAreDropped, GraceCue) +// cue and grace-cue notes are silent and the schema gives them no , so a +// tie on one is notation only: the is written, the is not +TEST(cueNoteTiesAreNotationOnly, GraceCue) { ScoreData score; score.ticksPerQuarter = 4; @@ -205,8 +207,6 @@ TEST(cueNoteTiesAreDropped, GraceCue) auto &staff = measure.staves.back(); auto &voice = staff.voices[0]; - // a cue note and a grace-cue note, both (illegally) marked as tie starts; - // the schema has no for them, so the writer silently drops the ties voice.notes.emplace_back(); voice.notes.back().isCue = true; voice.notes.back().isTieStart = true; @@ -216,20 +216,53 @@ TEST(cueNoteTiesAreDropped, GraceCue) voice.notes.emplace_back(); voice.notes.back().isGrace = true; voice.notes.back().isCue = true; - voice.notes.back().isTieStart = true; + voice.notes.back().isTieStop = true; voice.notes.back().tickTimePosition = 4; voice.notes.back().durationData.durationName = DurationName::eighth; voice.notes.back().durationData.durationTimeTicks = 0; const auto xml = toXml(score); CHECK(xml.find(" notation is written exactly once, whether it comes from the +// isTieStart / isTieStop flags or from an attribute-bearing tie curve +TEST(tiedNotationIsNotDuplicated, GraceCue) +{ + ScoreData score; + score.ticksPerQuarter = 4; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(); + auto &staff = measure.staves.back(); + auto &voice = staff.voices[0]; + + voice.notes.emplace_back(); + voice.notes.back().isTieStart = true; + voice.notes.back().durationData.durationName = DurationName::quarter; + voice.notes.back().durationData.durationTimeTicks = 4; + voice.notes.back().noteAttachmentData.curveStarts.emplace_back(CurveType::tie); + + const auto xml = toXml(score); + const auto first = xml.find(" from its first note, so their given order +// decides how the chord is spelled on the wire. An unstable sort would let the +// standard library implementation pick that order -- which it did: libc++ left +// these notes alone while libstdc++ permuted them, so ly32d_Arpeggio round-tripped +// on macOS and failed on Linux. +// +// The run is deliberately longer than the threshold at which an introsort stops +// insertion-sorting and starts partitioning, since a short run can come out +// ordered by luck. +TEST(sortKeepsSameTickNotesInOrder, ScoreDataSort) +{ + ScoreData score; + score.ticksPerQuarter = 4; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(); + auto &voice = measure.staves.back().voices[0]; + + constexpr int chordSize = 64; + + for (int i = 0; i < chordSize; ++i) + { + voice.notes.emplace_back(); + auto ¬e = voice.notes.back(); + note.isChord = true; + note.tickTimePosition = 0; + note.durationData.durationName = DurationName::quarter; + note.durationData.durationTimeTicks = 4; + // octave is the identity we check for; step alone would repeat + note.pitchData.octave = i; + } + + score.sort(); + + REQUIRE(voice.notes.size() == static_cast(chordSize)); + + for (int i = 0; i < chordSize; ++i) + { + CHECK(voice.notes.at(static_cast(i)).pitchData.octave == i); + } +} + +T_END; + +// The same guarantee for a voice that really does need reordering. The notes are +// interleaved across four ticks so the input is not already in order, which is +// what it takes to make an unstable sort actually permute the equal runs -- a +// long run that is already in order can survive std::sort by luck. +TEST(sortOrdersTicksAndKeepsEachTickInOrder, ScoreDataSort) +{ + ScoreData score; + score.ticksPerQuarter = 4; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(); + auto &voice = measure.staves.back().voices[0]; + + constexpr int noteCount = 64; + constexpr int tickCount = 4; + const auto tickOf = [](int index) { return (index * 7) % tickCount; }; + + for (int i = 0; i < noteCount; ++i) + { + voice.notes.emplace_back(); + auto ¬e = voice.notes.back(); + note.tickTimePosition = tickOf(i); + note.durationData.durationName = DurationName::quarter; + note.durationData.durationTimeTicks = 4; + // octave carries the note's original index, so the given order is readable + // back out of the sorted result + note.pitchData.octave = i; + } + + score.sort(); + + // ticks ascending; within a tick, the indices in the order they were given + std::vector expectedOctaves; + for (int tick = 0; tick < tickCount; ++tick) + { + for (int i = 0; i < noteCount; ++i) + { + if (tickOf(i) == tick) + { + expectedOctaves.push_back(i); + } + } + } + + REQUIRE(voice.notes.size() == expectedOctaves.size()); + + for (size_t i = 0; i < expectedOctaves.size(); ++i) + { + CHECK(voice.notes.at(i).pitchData.octave == expectedOctaves.at(i)); + } +} + +T_END; + +#endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index d113e78d2..685c286e9 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -538,3 +538,73 @@ lysuite/ly03d_Rhythm_DottedDurations_Factors.xml musuite/testMultiMeasureRest1.xml musuite/testMultiMeasureRest2.xml musuite/testMultiMeasureRest3.xml +synthetic/multiple-rest.3.0.xml + +# Passed on macOS but not on Linux until ScoreData::sort() was made stable. It is +# 14 chords, and every member of a chord shares one tick, so an unstable sort let +# the standard library choose which one came first -- and MusicXML spells a chord +# by omitting from its first note. +lysuite/ly32d_Arpeggio.xml + +# MusicXML states a tie twice: for sound and for notation. Two fixes +# here. (1) The that NoteWriter synthesizes from isTieStart/isTieStop is now +# suppressed when the note's curve vectors already carry a tie curve, which +# NotationsWriter writes with its full attributes -- previously every note whose +# source had both elements emitted twice. (2) Cue and grace-cue notes get no +# slot in the schema (they are silent), but sits outside the note +# choice, so is legal on them; the writer now emits it and the reader reads +# isTieStart/isTieStop back from it for those two flavors. +kiritan_singing/kiritan_singing_01.xml +kiritan_singing/kiritan_singing_02.xml +kiritan_singing/kiritan_singing_03.xml +kiritan_singing/kiritan_singing_04.xml +kiritan_singing/kiritan_singing_05.xml +kiritan_singing/kiritan_singing_06.xml +kiritan_singing/kiritan_singing_07.xml +kiritan_singing/kiritan_singing_08.xml +kiritan_singing/kiritan_singing_09.xml +kiritan_singing/kiritan_singing_10.xml +kiritan_singing/kiritan_singing_11.xml +kiritan_singing/kiritan_singing_12.xml +kiritan_singing/kiritan_singing_13.xml +kiritan_singing/kiritan_singing_14.xml +kiritan_singing/kiritan_singing_16.xml +kiritan_singing/kiritan_singing_17.xml +kiritan_singing/kiritan_singing_18.xml +kiritan_singing/kiritan_singing_19.xml +kiritan_singing/kiritan_singing_20.xml +kiritan_singing/kiritan_singing_21.xml +kiritan_singing/kiritan_singing_22.xml +kiritan_singing/kiritan_singing_23.xml +kiritan_singing/kiritan_singing_25.xml +kiritan_singing/kiritan_singing_26.xml +kiritan_singing/kiritan_singing_27.xml +kiritan_singing/kiritan_singing_28.xml +kiritan_singing/kiritan_singing_29.xml +kiritan_singing/kiritan_singing_30.xml +kiritan_singing/kiritan_singing_31.xml +kiritan_singing/kiritan_singing_32.xml +kiritan_singing/kiritan_singing_33.xml +kiritan_singing/kiritan_singing_34.xml +kiritan_singing/kiritan_singing_35.xml +kiritan_singing/kiritan_singing_36.xml +kiritan_singing/kiritan_singing_37.xml +kiritan_singing/kiritan_singing_38.xml +kiritan_singing/kiritan_singing_39.xml +kiritan_singing/kiritan_singing_40.xml +kiritan_singing/kiritan_singing_41.xml +kiritan_singing/kiritan_singing_42.xml +kiritan_singing/kiritan_singing_43.xml +kiritan_singing/kiritan_singing_44.xml +kiritan_singing/kiritan_singing_45.xml +kiritan_singing/kiritan_singing_46.xml +kiritan_singing/kiritan_singing_47.xml +kiritan_singing/kiritan_singing_48.xml +kiritan_singing/kiritan_singing_49.xml +kiritan_singing/kiritan_singing_50.xml +lysuite/ly11a_TimeSignatures.xml +lysuite/ly24a_GraceNotes.xml +lysuite/ly33i_Ties_NotEnded.xml +lysuite/ly61d_Lyrics_Melisma.xml +lysuite/ly61f_Lyrics_GracedNotes.xml +synthetic/tied.cue.4.0.xml diff --git a/src/private/mxtest/corert/CoreRoundtripTest.cpp b/src/private/mxtest/corert/CoreRoundtripTest.cpp index e1ad70216..0608f8e76 100644 --- a/src/private/mxtest/corert/CoreRoundtripTest.cpp +++ b/src/private/mxtest/corert/CoreRoundtripTest.cpp @@ -127,12 +127,12 @@ const CoreRoundtripRegistrar g_coreRoundtripRegistrar; } // namespace -// Pinned counts: 836 eligible files, none skipped. Count drift is a failure +// Pinned counts: 837 eligible files, none skipped. Count drift is a failure // even with zero individual fails, so a corpus or version-gate change is a // conscious decision, not silent decay. Registered last (registration is // discovery order; "zz" keeps it last alphabetically for shuffled runs too). TEST_CASE("zz-corert-pinned-counts", "[core-roundtrip]") { - CHECK(mxtest::corert::discoverInputFiles().size() == 836); + CHECK(mxtest::corert::discoverInputFiles().size() == 837); CHECK(g_skippedCount == 0); }