From c60ec58569cbc6db6f484b89335c9c123e4c67a9 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 23 Aug 2026 16:09:20 +0000 Subject: [PATCH 1/4] feat: model note playback velocity in mx::api MusicXML lets a carry its own playback velocity in the dynamics and end-dynamics attributes, the MIDI Note On and Note Off velocities expressed as percentages of the default forte level. The api had no home for either, so the reader dropped them and an author could not state them. NoteData gains velocityStart and velocityStop. Both are optional doubles that default to empty, so a note that says nothing about playback still writes no attribute. The names avoid "dynamics", which in the api already means the printed marking (a MarkData in noteAttachmentData) rather than how hard the note is played. Pins the thirteen foundsuite Inventions in the api round-trip baseline. Each one carried a per-note velocity that was the only thing it lost on write. --- src/include/mx/api/NoteData.h | 15 +++ src/private/mx/api/NoteData.cpp | 7 +- src/private/mx/impl/NoteFunctions.cpp | 11 +++ src/private/mx/impl/NoteWriter.cpp | 12 +++ .../mxtest/api/NoteVelocityApiTest.cpp | 93 +++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 17 ++++ 6 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 src/private/mxtest/api/NoteVelocityApiTest.cpp diff --git a/src/include/mx/api/NoteData.h b/src/include/mx/api/NoteData.h index 28074a6a1..63d911b09 100644 --- a/src/include/mx/api/NoteData.h +++ b/src/include/mx/api/NoteData.h @@ -160,6 +160,19 @@ class NoteData std::optional noteheadSmufl; PitchData pitchData; // step, alter, octave, accidental, etc + + // How hard this note is struck on playback, as a percentage of the default forte level: 100 + // is that default and 50 is half as hard. This is MusicXML's dynamics attribute on , + // and MIDI's Note On velocity. It describes one performance of this note. The dynamic + // marking printed above the staff is a separate thing -- a MarkData in noteAttachmentData -- + // so leave this empty and playback follows the printed marking. + std::optional velocityStart; + + // How hard this note is released on playback, on the same percentage scale as velocityStart. + // This is MusicXML's end-dynamics attribute on , and MIDI's Note Off velocity. Few + // instruments respond to it, so leave it empty unless the release really matters. + std::optional velocityStop; + int userRequestedVoiceNumber; // Most users can ignore this; leave it unspecified. It only controls whether the note's @@ -230,6 +243,8 @@ MXAPI_EQUALS_MEMBER(notehead) MXAPI_EQUALS_MEMBER(noteheadFilled) MXAPI_EQUALS_MEMBER(noteheadSmufl) MXAPI_EQUALS_MEMBER(pitchData) +MXAPI_EQUALS_MEMBER(velocityStart) +MXAPI_EQUALS_MEMBER(velocityStop) MXAPI_EQUALS_MEMBER(userRequestedVoiceNumber) MXAPI_EQUALS_MEMBER(writeStaffNumber) MXAPI_EQUALS_MEMBER(crossStaffIndex) diff --git a/src/private/mx/api/NoteData.cpp b/src/private/mx/api/NoteData.cpp index 31fbc42a2..8055653a8 100644 --- a/src/private/mx/api/NoteData.cpp +++ b/src/private/mx/api/NoteData.cpp @@ -11,9 +11,10 @@ namespace api NoteData::NoteData() : isRest{false}, isMeasureRest{false}, isUnpitched{false}, isDisplayStepOctaveSpecified{false}, isChord{false}, isTieStart{false}, isTieStop{false}, tieLetRing{}, isGrace{false}, graceSlash{Bool::unspecified}, isCue{false}, - notehead{Notehead::normal}, noteheadFilled{Bool::unspecified}, noteheadSmufl{}, pitchData{}, - userRequestedVoiceNumber{VALUE_UNSPECIFIED}, writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified}, - tickTimePosition{0}, durationData{}, beams{}, positionData{}, printData{}, noteAttachmentData{}, lyrics{} + notehead{Notehead::normal}, noteheadFilled{Bool::unspecified}, noteheadSmufl{}, pitchData{}, velocityStart{}, + velocityStop{}, userRequestedVoiceNumber{VALUE_UNSPECIFIED}, writeStaffNumber{Bool::unspecified}, + stem{Stem::unspecified}, tickTimePosition{0}, durationData{}, beams{}, positionData{}, printData{}, + noteAttachmentData{}, lyrics{} { } } // namespace api diff --git a/src/private/mx/impl/NoteFunctions.cpp b/src/private/mx/impl/NoteFunctions.cpp index c98fa382f..0e735899d 100644 --- a/src/private/mx/impl/NoteFunctions.cpp +++ b/src/private/mx/impl/NoteFunctions.cpp @@ -77,6 +77,17 @@ api::NoteData NoteFunctions::parseNote() const } myOutNoteData.pitchData.octave = reader.getOctave(); + + if (myNote.dynamics().has_value()) + { + myOutNoteData.velocityStart = myNote.dynamics()->value().value(); + } + + if (myNote.endDynamics().has_value()) + { + myOutNoteData.velocityStop = myNote.endDynamics()->value().value(); + } + myOutNoteData.userRequestedVoiceNumber = reader.getVoiceNumber(); // Auto rule (see NoteData::writeStaffNumber): is included on a multi-staff part and diff --git a/src/private/mx/impl/NoteWriter.cpp b/src/private/mx/impl/NoteWriter.cpp index 9ed71c384..7316d411e 100644 --- a/src/private/mx/impl/NoteWriter.cpp +++ b/src/private/mx/impl/NoteWriter.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT License #include "mx/impl/NoteWriter.h" +#include "mx/core/Decimal.h" #include "mx/core/NameToken.h" #include "mx/core/generated/Accidental.h" #include "mx/core/generated/BeamLevel.h" @@ -17,6 +18,7 @@ #include "mx/core/generated/GraceNoteGroup.h" #include "mx/core/generated/LyricChoice.h" #include "mx/core/generated/LyricTextGroup.h" +#include "mx/core/generated/NonNegativeDecimal.h" #include "mx/core/generated/NormalNoteGroup.h" #include "mx/core/generated/Pitch.h" #include "mx/core/generated/Rest.h" @@ -94,6 +96,16 @@ core::Note NoteWriter::getNote(bool isStartOfChord) const myOutNote.setPrintObject(myConverter.convert(myNoteData.printData.printObject)); } + if (myNoteData.velocityStart.has_value()) + { + myOutNote.setDynamics(core::NonNegativeDecimal{core::Decimal{*myNoteData.velocityStart}}); + } + + if (myNoteData.velocityStop.has_value()) + { + myOutNote.setEndDynamics(core::NonNegativeDecimal{core::Decimal{*myNoteData.velocityStop}}); + } + // The tie come first (as in the old writer, where they were // created during setNoteChoiceAndFullNoteGroup). if (!myOutTieNotationsChoices.empty()) diff --git a/src/private/mxtest/api/NoteVelocityApiTest.cpp b/src/private/mxtest/api/NoteVelocityApiTest.cpp new file mode 100644 index 000000000..1acc21352 --- /dev/null +++ b/src/private/mxtest/api/NoteVelocityApiTest.cpp @@ -0,0 +1,93 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mxtest/control/CompileControl.h" +#ifdef MX_COMPILE_API_TESTS + +#include "cpul/cpulTestHarness.h" +#include "mx/api/DocumentManager.h" +#include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" + +using namespace std; +using namespace mx::api; +using namespace mxtest; + +// A one-measure, one-note score. The caller fills in the note's velocities. +ScoreData noteVelocityMakeScore() +{ + 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(); + auto ¬e = voice.notes.back(); + note.durationData.durationName = DurationName::quarter; + note.durationData.durationTimeTicks = 4; + return score; +} + +const NoteData ¬eVelocityFirstNote(const ScoreData &score) +{ + return score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0); +} + +TEST(velocitiesSurviveRoundTrip, NoteVelocity) +{ + auto score = noteVelocityMakeScore(); + auto ¬e = score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0); + note.velocityStart = 71.5; + note.velocityStop = 40.0; + + const auto out = roundTrip(score); + const auto &outNote = noteVelocityFirstNote(out); + REQUIRE(outNote.velocityStart.has_value()); + REQUIRE(outNote.velocityStop.has_value()); + CHECK_DOUBLES_EQUAL(71.5, *outNote.velocityStart, 0.0001); + CHECK_DOUBLES_EQUAL(40.0, *outNote.velocityStop, 0.0001); +} + +TEST(velocitiesWriteMusicXmlAttributes, NoteVelocity) +{ + auto score = noteVelocityMakeScore(); + auto ¬e = score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0); + note.velocityStart = 100.0; + note.velocityStop = 60.0; + + const auto xml = toXml(score); + CHECK(xml.find("dynamics=\"100\"") != std::string::npos); + CHECK(xml.find("end-dynamics=\"60\"") != std::string::npos); +} + +TEST(absentVelocitiesWriteNothing, NoteVelocity) +{ + const auto score = noteVelocityMakeScore(); + const auto xml = toXml(score); + CHECK(xml.find("dynamics=") == std::string::npos); + + const auto out = roundTrip(score); + const auto &outNote = noteVelocityFirstNote(out); + CHECK(!outNote.velocityStart.has_value()); + CHECK(!outNote.velocityStop.has_value()); +} + +TEST(velocitiesAreReadFromMusicXml, NoteVelocity) +{ + auto score = noteVelocityMakeScore(); + auto ¬e = score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0); + note.velocityStart = 12.5; + + const auto in = fromXml(toXml(score)); + const auto &inNote = noteVelocityFirstNote(in); + REQUIRE(inNote.velocityStart.has_value()); + CHECK_DOUBLES_EQUAL(12.5, *inNote.velocityStart, 0.0001); + CHECK(!inNote.velocityStop.has_value()); +} + +#endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 2dacdc781..33789b08a 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -669,3 +669,20 @@ synthetic/wavy-line.4.0.xml # earlier work and simply had not been pinned yet. lysuite/ly71c_ChordsFrets.xml musuite/testHarmony3.xml + +# Unblocked by note playback velocity: the api had no home for the dynamics +# attribute, so a note that named its own playback velocity lost it on write. It is +# now NoteData::velocityStart (with velocityStop for end-dynamics). +foundsuite/Invention 2.xml +foundsuite/Invention 3.xml +foundsuite/Invention_10.xml +foundsuite/Invention_11.xml +foundsuite/Invention_12.xml +foundsuite/Invention_13.xml +foundsuite/Invention_14.xml +foundsuite/Invention_15.xml +foundsuite/Invention_4.xml +foundsuite/Invention_6.xml +foundsuite/Invention_7.xml +foundsuite/Invention_8.xml +foundsuite/Invention_9.xml From f10a1203b1444d04e123120ead4ad10f41736aa5 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 23 Aug 2026 16:28:04 +0000 Subject: [PATCH 2/4] feat: expose the identification source in mx::api MusicXML's / names the edition, manuscript, or other publication a score was made from. The api had nowhere to keep it, so the reader discarded it and every write dropped the element. ScoreData gains an optional source string. Optional rather than a plain string because is legal and means something different from saying nothing at all. Pins five more files in the api round-trip baseline whose only divergence was the dropped element. --- src/include/mx/api/ScoreData.h | 6 ++ src/private/mx/api/ScoreData.cpp | 2 +- src/private/mx/impl/ScoreReader.cpp | 2 + src/private/mx/impl/ScoreWriter.cpp | 6 ++ .../api/IdentificationSourceApiTest.cpp | 69 +++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 9 +++ 6 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/private/mxtest/api/IdentificationSourceApiTest.cpp diff --git a/src/include/mx/api/ScoreData.h b/src/include/mx/api/ScoreData.h index 81852c679..fab4c4f77 100644 --- a/src/include/mx/api/ScoreData.h +++ b/src/include/mx/api/ScoreData.h @@ -66,6 +66,11 @@ class ScoreData // The type attribute for `copyright` (above). std::optional copyrightType; + // Where the music came from: the edition, manuscript, or other publication this score was + // made from. This is MusicXML's element. It names the origin of the music, not of + // the file. Leave it empty when the score does not name one. + std::optional source; + EncodingData encoding; std::vector pageTextItems; @@ -118,6 +123,7 @@ MXAPI_EQUALS_MEMBER(arranger) MXAPI_EQUALS_MEMBER(publisher) MXAPI_EQUALS_MEMBER(copyright) MXAPI_EQUALS_MEMBER(copyrightType) +MXAPI_EQUALS_MEMBER(source) MXAPI_EQUALS_MEMBER(encoding) MXAPI_EQUALS_MEMBER(pageTextItems) MXAPI_EQUALS_MEMBER(pageImageItems) diff --git a/src/private/mx/api/ScoreData.cpp b/src/private/mx/api/ScoreData.cpp index 8790b6cb9..625219eff 100644 --- a/src/private/mx/api/ScoreData.cpp +++ b/src/private/mx/api/ScoreData.cpp @@ -14,7 +14,7 @@ namespace api ScoreData::ScoreData() : musicXmlVersion{api::MusicXmlVersion::unspecified}, declaredMusicXmlVersion{}, musicXmlType{"partwise"}, workTitle{}, workNumber{}, movementTitle{}, movementNumber{}, composer{}, lyricist{}, copyright{}, - copyrightType{"copyright"}, encoding{}, pageTextItems{}, defaults{}, parts{}, partGroups{}, + copyrightType{"copyright"}, source{}, encoding{}, pageTextItems{}, defaults{}, parts{}, partGroups{}, ticksPerQuarter{DEFAULT_TICKS_PER_QUARTER} { } diff --git a/src/private/mx/impl/ScoreReader.cpp b/src/private/mx/impl/ScoreReader.cpp index 7c5d79b9c..cbc10de32 100644 --- a/src/private/mx/impl/ScoreReader.cpp +++ b/src/private/mx/impl/ScoreReader.cpp @@ -234,6 +234,8 @@ api::ScoreData ScoreReader::getScoreData() const myOutScoreData.copyrightType = std::nullopt; } } + myOutScoreData.source = ident.source(); + api::EncodingData encodingData; if (ident.encoding().has_value()) diff --git a/src/private/mx/impl/ScoreWriter.cpp b/src/private/mx/impl/ScoreWriter.cpp index 5186266e9..d179360ad 100644 --- a/src/private/mx/impl/ScoreWriter.cpp +++ b/src/private/mx/impl/ScoreWriter.cpp @@ -144,6 +144,12 @@ core::ScorePartwise ScoreWriter::getScorePartwise() const hasIdentification = true; } + if (myScoreData.source.has_value()) + { + identification.setSource(myScoreData.source); + hasIdentification = true; + } + if (hasIdentification) { header.setIdentification(identification); diff --git a/src/private/mxtest/api/IdentificationSourceApiTest.cpp b/src/private/mxtest/api/IdentificationSourceApiTest.cpp new file mode 100644 index 000000000..1a72b67be --- /dev/null +++ b/src/private/mxtest/api/IdentificationSourceApiTest.cpp @@ -0,0 +1,69 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mxtest/control/CompileControl.h" +#ifdef MX_COMPILE_API_TESTS + +#include "cpul/cpulTestHarness.h" +#include "mx/api/DocumentManager.h" +#include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" + +using namespace std; +using namespace mx::api; +using namespace mxtest; + +// An otherwise empty one-part score, so the only thing under test is the header. +ScoreData identificationSourceMakeScore() +{ + ScoreData score; + score.parts.emplace_back(); + score.parts.back().uniqueId = "P1"; + score.parts.back().measures.emplace_back(); + score.parts.back().measures.back().staves.emplace_back(); + return score; +} + +TEST(sourceSurvivesRoundTrip, IdentificationSource) +{ + auto score = identificationSourceMakeScore(); + score.source = "Bach-Gesellschaft Ausgabe, Band 3"; + + const auto out = roundTrip(score); + REQUIRE(out.source.has_value()); + CHECK_EQUAL("Bach-Gesellschaft Ausgabe, Band 3", *out.source); +} + +TEST(sourceWritesTheMusicXmlElement, IdentificationSource) +{ + auto score = identificationSourceMakeScore(); + score.source = "Urtext"; + + const auto xml = toXml(score); + CHECK(xml.find("Urtext") != std::string::npos); +} + +TEST(anEmptySourceWritesNothing, IdentificationSource) +{ + const auto score = identificationSourceMakeScore(); + const auto xml = toXml(score); + CHECK(xml.find("") == std::string::npos); + + const auto out = roundTrip(score); + CHECK(!out.source.has_value()); +} + +// MusicXML allows , which says the score named a source and left it blank. +// That is a different fact from saying nothing at all, so both have to survive the round trip. +TEST(aBlankSourceIsNotTheSameAsNoSource, IdentificationSource) +{ + auto score = identificationSourceMakeScore(); + score.source = ""; + + const auto out = roundTrip(score); + REQUIRE(out.source.has_value()); + CHECK_EQUAL("", *out.source); +} + +#endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 33789b08a..04c6675de 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -686,3 +686,12 @@ foundsuite/Invention_6.xml foundsuite/Invention_7.xml foundsuite/Invention_8.xml foundsuite/Invention_9.xml + +# Unblocked by ScoreData::source: / names the edition or +# manuscript a score was made from, and the api had nowhere to keep it, so the +# element was dropped on write. +musetrainer/Canon_in_D_easy.xml +musetrainer/Carol_of_the_Bells_easy_piano.xml +musetrainer/Lacrimosa_-_Requiem.xml +musetrainer/Prelude_I_in_C_major_BWV_846_-_Well_Tempered_Clavier_First_Book.xml +synthetic/source.3.0.xml From b95ae179f4391ca4079993d20fdb8c3ec105eb23 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 23 Aug 2026 16:46:23 +0000 Subject: [PATCH 3/4] feat: support the direction directive attribute in mx::api aligns a direction with the measure's time signature instead of with the note it is anchored to, which is how a tempo or style marking at the head of a movement is usually placed. The api had no field for it, so the attribute was dropped on write. DirectionData gains a tri-state directive following the api's Bool convention: unspecified does not write an attribute, yes and no are written verbatim. does not carry the attribute, so only reads and writes it. Pins three more files in the api round-trip baseline. --- src/include/mx/api/DirectionData.h | 16 ++++-- src/private/mx/impl/DirectionReader.cpp | 11 ++++ src/private/mx/impl/DirectionReader.h | 1 + src/private/mx/impl/DirectionWriter.cpp | 5 ++ src/private/mxtest/api/roundtrip-baseline.txt | 7 +++ .../mxtest/impl/DirectionWriterTest.cpp | 52 +++++++++++++++++++ 6 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/include/mx/api/DirectionData.h b/src/include/mx/api/DirectionData.h index b1adcd8d9..809dee75b 100644 --- a/src/include/mx/api/DirectionData.h +++ b/src/include/mx/api/DirectionData.h @@ -38,6 +38,15 @@ struct DirectionData // and ::alsoBottom exist only for measure numbering; either one here writes no attribute. SystemRelation systemRelation; + // Whether this direction lines up with the measure's time signature rather than with the + // note it is anchored to. A tempo or style marking at the head of a movement is usually set + // this way: yes puts its left edge at the left edge of the time signature, or at the first + // note in the measure when there is no time signature. A default-x, justify, or halign on + // the direction's content overrides it. Leave it unspecified for a direction that belongs + // over its note. Harmony chord symbols do not carry this; MusicXML puts it on + // alone. + Bool directive; + // The source's , in divisions, or absent when it had none. An nudges only // where the direction is *drawn*, shifting it away from the note it is anchored to; it does not // move that anchor. tickTimePosition holds the anchor -- the musical location the direction @@ -86,9 +95,9 @@ struct DirectionData std::optional id; DirectionData() - : tickTimePosition{0}, placement{Placement::unspecified}, systemRelation{SystemRelation::unspecified}, offset{}, - voice{VALUE_UNSPECIFIED}, isStaffValueSpecified{true}, isSoundDataSpecified{false}, soundData{}, - directionTypes{}, chords{}, figuredBasses{}, id{} + : tickTimePosition{0}, placement{Placement::unspecified}, systemRelation{SystemRelation::unspecified}, + directive{Bool::unspecified}, offset{}, voice{VALUE_UNSPECIFIED}, isStaffValueSpecified{true}, + isSoundDataSpecified{false}, soundData{}, directionTypes{}, chords{}, figuredBasses{}, id{} { } }; @@ -103,6 +112,7 @@ MXAPI_EQUALS_BEGIN(DirectionData) MXAPI_EQUALS_MEMBER(tickTimePosition) MXAPI_EQUALS_MEMBER(placement) MXAPI_EQUALS_MEMBER(systemRelation) +MXAPI_EQUALS_MEMBER(directive) MXAPI_EQUALS_MEMBER(offset) MXAPI_EQUALS_MEMBER(voice) MXAPI_EQUALS_MEMBER(isStaffValueSpecified) diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index 8172b90f6..38a4781dd 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -114,6 +114,7 @@ api::DirectionData DirectionReader::getDirectionData() parseOffset(); parsePlacement(); parseSystemRelation(); + parseDirective(); parseValues(); return returnData(); } @@ -196,6 +197,16 @@ void DirectionReader::parseSystemRelation() } } +// aligns the direction with the measure's time signature. Only +// has the attribute; does not. +void DirectionReader::parseDirective() +{ + if (myDirection && myDirection->directive().has_value()) + { + myOutDirectionData.directive = myConverter.convert(*myDirection->directive()); + } +} + void DirectionReader::parseValues() { if (myDirection) diff --git a/src/private/mx/impl/DirectionReader.h b/src/private/mx/impl/DirectionReader.h index 7b252224d..392f86e8e 100644 --- a/src/private/mx/impl/DirectionReader.h +++ b/src/private/mx/impl/DirectionReader.h @@ -41,6 +41,7 @@ class DirectionReader void parseOffset(); void parsePlacement(); void parseSystemRelation(); + void parseDirective(); void parseValues(); mx::api::DirectionData returnData(); void parseStaffIndex(); diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index 773d3bd97..f5e0a6283 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -167,6 +167,11 @@ std::vector DirectionWriter::getDirectionLikeThings() // nullopt for unspecified, and for the bottom-of-system values that only measure numbering has. // carries the same attribute; createHarmonyElements writes it there too. direction.setSystem(myConverter.convertDirectionSystemRelation(myDirectionData.systemRelation)); + + if (myDirectionData.directive != api::Bool::unspecified) + { + direction.setDirective(myConverter.convert(myDirectionData.directive)); + } setId(myDirectionData.id, direction); if (myDirectionData.isStaffValueSpecified || myCursor.staffIndex != 0) diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 04c6675de..36787ec58 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -695,3 +695,10 @@ musetrainer/Carol_of_the_Bells_easy_piano.xml musetrainer/Lacrimosa_-_Requiem.xml musetrainer/Prelude_I_in_C_major_BWV_846_-_Well_Tempered_Clavier_First_Book.xml synthetic/source.3.0.xml + +# Unblocked by DirectionData::directive: the api had no field for 's +# directive attribute, which aligns a direction with the measure's time signature, +# so it was dropped on write. +recsuite/Chant.xml +synthetic/direction.3.1.xml +synthetic/direction.4.0.xml diff --git a/src/private/mxtest/impl/DirectionWriterTest.cpp b/src/private/mxtest/impl/DirectionWriterTest.cpp index af8d16fb8..385272b9f 100644 --- a/src/private/mxtest/impl/DirectionWriterTest.cpp +++ b/src/private/mxtest/impl/DirectionWriterTest.cpp @@ -236,4 +236,56 @@ TEST(rehearsalRoundTrip, DirectionWriter) T_END +// aligns the direction with the measure's time signature. The +// attribute is tri-state: unspecified does not write anything, and yes/no are written verbatim. +TEST(directiveRoundTrip, DirectionWriter) +{ + api::RehearsalData rehearsal; + rehearsal.text = "A"; + + api::DirectionData directionData; + directionData.directionTypes.emplace_back(api::DirectionChoice{rehearsal}); + directionData.directive = api::Bool::yes; + + Cursor cursor{1, 100}; + SpannerResolver spannerResolver; + DirectionWriter writer{directionData, cursor, spannerResolver}; + const auto mdcSet = writer.getDirectionLikeThings(); + REQUIRE(mdcSet.size() >= 1); + REQUIRE(mdcSet.front().isDirection()); + const auto &direction = mdcSet.front().asDirection(); + REQUIRE(direction.directive().has_value()); + CHECK(core::YesNo::Tag::yes == direction.directive()->tag()); + + DirectionReader reader{direction, cursor}; + CHECK(api::Bool::yes == reader.getDirectionData().directive); +} + +T_END + +// An unspecified directive leaves the attribute off the written entirely, so a +// document that never mentioned it does not gain one. +TEST(directiveUnspecifiedWritesNoAttribute, DirectionWriter) +{ + api::RehearsalData rehearsal; + rehearsal.text = "A"; + + api::DirectionData directionData; + directionData.directionTypes.emplace_back(api::DirectionChoice{rehearsal}); + + Cursor cursor{1, 100}; + SpannerResolver spannerResolver; + DirectionWriter writer{directionData, cursor, spannerResolver}; + const auto mdcSet = writer.getDirectionLikeThings(); + REQUIRE(mdcSet.size() >= 1); + REQUIRE(mdcSet.front().isDirection()); + const auto &direction = mdcSet.front().asDirection(); + CHECK(!direction.directive().has_value()); + + DirectionReader reader{direction, cursor}; + CHECK(api::Bool::unspecified == reader.getDirectionData().directive); +} + +T_END + #endif From 19b9d570b6ee4acc30773e1c2805a5370168bcfe Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 23 Aug 2026 17:04:37 +0000 Subject: [PATCH 4/4] feat: round-trip a redundant staves element in mx::api MusicXML states a part's staff count in , and a part that omits the element has one staff. mx wrote the element only when the part had more than one staff, so a source that spelled out the redundant 1 lost it. PartData gains writeStaffCount, defaulting to false. It only affects a single-staff part; a part with more than one staff writes its count either way, since without it every note would land on staff one. The reader sets it when the source spelled the element out. Pins three more files in the api round-trip baseline. --- src/include/mx/api/PartData.h | 8 ++ src/private/mx/impl/MeasureWriter.cpp | 5 +- src/private/mx/impl/PartReader.cpp | 12 ++- src/private/mx/impl/PartReader.h | 5 +- src/private/mxtest/api/StaffCountApiTest.cpp | 77 +++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 7 ++ 6 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 src/private/mxtest/api/StaffCountApiTest.cpp diff --git a/src/include/mx/api/PartData.h b/src/include/mx/api/PartData.h index eed1e575e..8eb37a053 100644 --- a/src/include/mx/api/PartData.h +++ b/src/include/mx/api/PartData.h @@ -210,6 +210,13 @@ class PartData /// the part. Subsequent transposition changes are not currently supported by mx::api. std::optional transposition; + // Most users can ignore this; leave it false. MusicXML states a part's staff count in + // , which it needs only when the part has more than one staff -- a part with no + // has one. mx writes the element whenever the part has more than one staff, so + // this only controls the redundant 1 on a single-staff part. Reading a + // file sets it when the source spelled that out. Same idea as ClefData::writeStaffNumber. + bool writeStaffCount = false; + std::vector measures; // MusicXML requires , but real files often leave it empty and @@ -312,6 +319,7 @@ MXAPI_EQUALS_MEMBER(displayAbbreviationPositionData) MXAPI_EQUALS_MEMBER(groups) MXAPI_EQUALS_MEMBER(instrumentData) MXAPI_EQUALS_MEMBER(transposition) +MXAPI_EQUALS_MEMBER(writeStaffCount) MXAPI_EQUALS_MEMBER(measures) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(PartData); diff --git a/src/private/mx/impl/MeasureWriter.cpp b/src/private/mx/impl/MeasureWriter.cpp index 3df64a875..782d0f83c 100644 --- a/src/private/mx/impl/MeasureWriter.cpp +++ b/src/private/mx/impl/MeasureWriter.cpp @@ -136,7 +136,10 @@ void MeasureWriter::writeMeasureGlobals() myPropertiesWriter->writeDivisions(myHistory.getCursor().getGlobalTicksPerQuarter()); } - if (myMeasureData.staves.size() > 1) + // is needed only when the part has more than one staff. PartData::writeStaffCount + // adds the redundant 1 back for a source that spelled it out. + const bool isStaffCountRequested = myScoreWriter.getPart(myHistory.getCursor().partIndex).writeStaffCount; + if (myMeasureData.staves.size() > 1 || isStaffCountRequested) { myPropertiesWriter->writeNumStaves(static_cast(myMeasureData.staves.size())); } diff --git a/src/private/mx/impl/PartReader.cpp b/src/private/mx/impl/PartReader.cpp index 748dc74b0..b406da912 100644 --- a/src/private/mx/impl/PartReader.cpp +++ b/src/private/mx/impl/PartReader.cpp @@ -78,7 +78,7 @@ void readNameDisplay(const core::PartName &nameElement, const std::optional= 0); myPartIndex = partIndex; - myNumStaves = calculateNumStaves(); + myNumStaves = calculateNumStaves(myIsStavesElementPresent); } api::PartData PartReader::getPartData() @@ -100,6 +100,10 @@ api::PartData PartReader::getPartData() myOutPartData = api::PartData{}; parseScorePart(); + // A single-staff part does not need , so mx omits it there. Record an override only + // when the source spelled the redundant element out. + myOutPartData.writeStaffCount = myIsStavesElementPresent && myNumStaves == 1; + myCurrentCursor = MeasureCursor{myNumStaves, myGlobalTicksPerMeasure}; myCurrentCursor.partIndex = myPartIndex; @@ -142,8 +146,9 @@ MeasureCursor PartReader::getCursor() const return myCurrentCursor; } -int PartReader::calculateNumStaves() const +int PartReader::calculateNumStaves(bool &outIsStavesElementPresent) const { + outIsStavesElementPresent = false; int numStaves = 1; for (const auto &measure : myPartwisePart.measure()) @@ -168,6 +173,7 @@ int PartReader::calculateNumStaves() const const auto &attributes = mdc.asAttributes(); if (attributes.staves().has_value()) { + outIsStavesElementPresent = true; int temp = *attributes.staves(); if (temp > numStaves) { diff --git a/src/private/mx/impl/PartReader.h b/src/private/mx/impl/PartReader.h index c0cec2d8a..2081f34a5 100644 --- a/src/private/mx/impl/PartReader.h +++ b/src/private/mx/impl/PartReader.h @@ -38,6 +38,7 @@ class PartReader const core::PartwisePart &myPartwisePart; const core::ScorePart &myScorePart; int myNumStaves; + bool myIsStavesElementPresent; const int myGlobalTicksPerMeasure; const core::ScorePartwise &myScore; int myPartIndex; @@ -49,7 +50,9 @@ class PartReader mutable std::mutex myMutex; mutable api::PartData myOutPartData; - int calculateNumStaves() const; + // Returns the part's staff count. outIsStavesElementPresent reports whether the source said + // so with a element, which the count alone cannot tell you when it is one. + int calculateNumStaves(bool &outIsStavesElementPresent) const; void parseScorePart() const; void parseScoreInstrument(const core::ScoreInstrument &scoreInstrument) const; void parseVirtualInstrument(const core::VirtualInstrument &virtualInstrument) const; diff --git a/src/private/mxtest/api/StaffCountApiTest.cpp b/src/private/mxtest/api/StaffCountApiTest.cpp new file mode 100644 index 000000000..e2af19c00 --- /dev/null +++ b/src/private/mxtest/api/StaffCountApiTest.cpp @@ -0,0 +1,77 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mxtest/control/CompileControl.h" +#ifdef MX_COMPILE_API_TESTS + +#include "cpul/cpulTestHarness.h" +#include "mx/api/DocumentManager.h" +#include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" + +using namespace std; +using namespace mx::api; +using namespace mxtest; + +// A one-measure score whose single part has the requested number of staves, each holding one +// quarter note so the staves are real. +ScoreData staffCountMakeScore(int numStaves) +{ + ScoreData score; + score.ticksPerQuarter = 4; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.uniqueId = "P1"; + part.measures.emplace_back(); + auto &measure = part.measures.back(); + + for (int i = 0; i < numStaves; ++i) + { + measure.staves.emplace_back(); + auto &voice = measure.staves.back().voices[0]; + voice.notes.emplace_back(); + voice.notes.back().durationData.durationName = DurationName::quarter; + voice.notes.back().durationData.durationTimeTicks = 4; + } + + return score; +} + +TEST(aSingleStaffPartOmitsTheStaffCount, StaffCount) +{ + const auto score = staffCountMakeScore(1); + const auto xml = toXml(score); + CHECK(xml.find("") == std::string::npos); + + const auto out = roundTrip(score); + CHECK(!out.parts.at(0).writeStaffCount); +} + +TEST(aSingleStaffPartCanAskForTheStaffCount, StaffCount) +{ + auto score = staffCountMakeScore(1); + score.parts.at(0).writeStaffCount = true; + + const auto xml = toXml(score); + CHECK(xml.find("1") != std::string::npos); + + const auto out = roundTrip(score); + CHECK(out.parts.at(0).writeStaffCount); + REQUIRE(out.parts.at(0).measures.at(0).staves.size() == 1); +} + +// A part with more than one staff needs no matter what the field says, since without it +// every note would land on staff one. +TEST(aMultiStaffPartAlwaysWritesTheStaffCount, StaffCount) +{ + const auto score = staffCountMakeScore(2); + const auto xml = toXml(score); + CHECK(xml.find("2") != std::string::npos); + + const auto out = roundTrip(score); + CHECK(!out.parts.at(0).writeStaffCount); + REQUIRE(out.parts.at(0).measures.at(0).staves.size() == 2); +} + +#endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 36787ec58..839341b06 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -702,3 +702,10 @@ synthetic/source.3.0.xml recsuite/Chant.xml synthetic/direction.3.1.xml synthetic/direction.4.0.xml + +# Unblocked by PartData::writeStaffCount: a single-staff part does not need +# , so mx omitted it, and these three sources spell out the redundant +# 1. +lysuite/ly02b_Rests_PitchedRests.xml +lysuite/ly32b_Articulations_Texts.xml +lysuite/ly33b_Spanners_Tie.xml