From fa268e0257ef87ee227c79f2ea0db93bb226b85d Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Tue, 7 Jul 2026 06:18:19 +0000 Subject: [PATCH] fix: preserve explicit on single-staff parts NoteWriter::setStaffAndVoice only wrote when the part had more than one staff, since staff number is otherwise redundant with containment (measure -> staves -> voices -> notes). But is legal MusicXML on a single-staff part too, and many real-world exporters write it unconditionally -- so a source's explicit was silently dropped on round-trip on every single-staff part, the single largest drop signature in the api round-trip corpus. Added NoteData::isStaffValueSpecified (mirrors the existing DirectionData::isStaffValueSpecified), set by NoteReader whenever the source note carried an explicit . NoteWriter now emits whenever the part is multi-staff (unchanged, structurally required) or the source had it explicitly, matching the same source-had-it-explicitly convention already used for (NoteData::userRequestedVoiceNumber). Closes #275 --- src/include/mx/api/NoteData.h | 7 +++ src/private/mx/api/NoteData.cpp | 5 +- src/private/mx/impl/NoteFunctions.cpp | 1 + src/private/mx/impl/NoteReader.cpp | 7 ++- src/private/mx/impl/NoteReader.h | 6 ++ src/private/mx/impl/NoteWriter.cpp | 2 +- src/private/mxtest/api/NoteDataTest.cpp | 60 +++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 5 ++ 8 files changed, 87 insertions(+), 6 deletions(-) diff --git a/src/include/mx/api/NoteData.h b/src/include/mx/api/NoteData.h index 19fc1001e..c953e7e71 100644 --- a/src/include/mx/api/NoteData.h +++ b/src/include/mx/api/NoteData.h @@ -135,6 +135,12 @@ class NoteData PitchData pitchData; // step, alter, octave, accidental, etc int userRequestedVoiceNumber; + // Whether the source note carried an explicit element. mx::api normally only + // writes when the part has more than one staff (where it is structurally + // required); this preserves a source's redundant-but-legal on a single-staff + // part instead of silently dropping it. Mirrors DirectionData::isStaffValueSpecified. + bool isStaffValueSpecified; + // The zero-based index of the staff on which this note is displayed, for the rare case // that it differs from the staff that contains the note (cross-staff notation, e.g. a // beamed piano run or a chord that dips onto the other staff). The note stays in its @@ -189,6 +195,7 @@ MXAPI_EQUALS_MEMBER(isCue) MXAPI_EQUALS_MEMBER(graceSlash) MXAPI_EQUALS_MEMBER(pitchData) MXAPI_EQUALS_MEMBER(userRequestedVoiceNumber) +MXAPI_EQUALS_MEMBER(isStaffValueSpecified) MXAPI_EQUALS_MEMBER(crossStaffIndex) MXAPI_EQUALS_MEMBER(stem) MXAPI_EQUALS_MEMBER(stemPositionData) diff --git a/src/private/mx/api/NoteData.cpp b/src/private/mx/api/NoteData.cpp index 20d51b4a4..7f7c9985d 100644 --- a/src/private/mx/api/NoteData.cpp +++ b/src/private/mx/api/NoteData.cpp @@ -11,8 +11,9 @@ 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}, pitchData{}, userRequestedVoiceNumber{VALUE_UNSPECIFIED}, stem{Stem::unspecified}, - tickTimePosition{0}, durationData{}, beams{}, positionData{}, printData{}, noteAttachmentData{}, lyrics{} + notehead{Notehead::normal}, pitchData{}, userRequestedVoiceNumber{VALUE_UNSPECIFIED}, + isStaffValueSpecified{false}, 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 d8a52bbe1..625d288bb 100644 --- a/src/private/mx/impl/NoteFunctions.cpp +++ b/src/private/mx/impl/NoteFunctions.cpp @@ -77,6 +77,7 @@ api::NoteData NoteFunctions::parseNote() const myOutNoteData.pitchData.octave = reader.getOctave(); myOutNoteData.userRequestedVoiceNumber = reader.getVoiceNumber(); + myOutNoteData.isStaffValueSpecified = reader.getIsStaffSpecified(); myOutNoteData.notehead = converter.convert(reader.getNoteheadValue()); diff --git a/src/private/mx/impl/NoteReader.cpp b/src/private/mx/impl/NoteReader.cpp index 4686e8c80..6516861a2 100644 --- a/src/private/mx/impl/NoteReader.cpp +++ b/src/private/mx/impl/NoteReader.cpp @@ -144,8 +144,8 @@ NoteReader::NoteReader(const core::Note &mxNote) : myNote(mxNote), myNoteChoice(myNote.choice()), myFullNoteGroup(findFullNoteGroup(myNoteChoice)), myIsNormal(false), myIsGrace(false), myIsCue(false), myIsRest(false), myIsChord(false), myIsMeasureRest(false), myIsUnpitched(false), myIsPitch(false), myIsDisplayStepOctaveSpecified(false), myDurationValue(0.0), - myStep(core::Step::c()), myAlter(0), myCents(0.0), myOctave(4), myStaffNumber(0), myVoiceNumber(0), - myNoteheadValue(core::NoteheadValue::normal()), myDurationType(core::NoteTypeValue::maxima()), + myStep(core::Step::c()), myAlter(0), myCents(0.0), myOctave(4), myStaffNumber(0), myIsStaffSpecified(false), + myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myDurationType(core::NoteTypeValue::maxima()), myIsDurationTypeSpecified(false), myNumDots(0), myBeams(), myTimeModificationActualNotes(-1), myTimeModificationNormalNotes(-1), myTimeModificationNormalType(core::NoteTypeValue::maxima()), myTimeModificationNormalTypeDots(0), myHasAccidental(false), myAccidental(core::AccidentalValue::natural()), @@ -304,7 +304,8 @@ void NoteReader::setChord() void NoteReader::setStaffNumber() { - if (myNote.staff().has_value()) + myIsStaffSpecified = myNote.staff().has_value(); + if (myIsStaffSpecified) { myStaffNumber = *myNote.staff(); } diff --git a/src/private/mx/impl/NoteReader.h b/src/private/mx/impl/NoteReader.h index 8ecc3f1a9..a5f51f501 100644 --- a/src/private/mx/impl/NoteReader.h +++ b/src/private/mx/impl/NoteReader.h @@ -117,6 +117,11 @@ class NoteReader return myStaffNumber; } + inline bool getIsStaffSpecified() const + { + return myIsStaffSpecified; + } + inline int getVoiceNumber() const { return myVoiceNumber; @@ -251,6 +256,7 @@ class NoteReader double myCents; int myOctave; int myStaffNumber; + bool myIsStaffSpecified; int myVoiceNumber; core::NoteheadValue myNoteheadValue; core::NoteTypeValue myDurationType; diff --git a/src/private/mx/impl/NoteWriter.cpp b/src/private/mx/impl/NoteWriter.cpp index 06b19d2f6..703e7c91c 100644 --- a/src/private/mx/impl/NoteWriter.cpp +++ b/src/private/mx/impl/NoteWriter.cpp @@ -427,7 +427,7 @@ void NoteWriter::setStaffAndVoice() const // position, but the displayed staff comes from the override (NoteData.h) myOutNote.setStaff(*myNoteData.crossStaffIndex + 1); } - else if (myCursor.getNumStaves() > 1 && myCursor.staffIndex >= 0) + else if (myCursor.staffIndex >= 0 && (myCursor.getNumStaves() > 1 || myNoteData.isStaffValueSpecified)) { myOutNote.setStaff(myCursor.staffIndex + 1); } diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index 805c2c569..7180459f7 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -12,6 +12,7 @@ #include "mx/core/generated/Document.h" #include "mx/core/generated/MusicDataChoice.h" #include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" #include "mxtest/file/Path.h" using namespace std; @@ -1480,4 +1481,63 @@ TEST(strongAccentDirection, NoteData) T_END; +TEST(explicitStaffOnSingleStaffPartRoundTrips, NoteData) +{ + const std::string xml = R"( + + + + x + + + + + + 1 + + + C4 + 1 + 1 + quarter + 1 + + + + +)"; + + const auto score = fromXml(xml); + const auto ¬es = score.parts.back().measures.back().staves.back().voices.at(0).notes; + REQUIRE(notes.size() == 1); + CHECK(notes.at(0).isStaffValueSpecified); + + const auto outXml = toXml(score); + CHECK(outXml.find("1") != std::string::npos); +} + +T_END; + +TEST(implicitStaffOnSingleStaffPartOmitsElement, NoteData) +{ + ScoreData score; + 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]; + + NoteData note; + note.tickTimePosition = 0; + note.durationData.durationTimeTicks = score.ticksPerQuarter; + note.durationData.durationName = DurationName::quarter; + voice.notes.push_back(note); + + const auto xml = toXml(score); + CHECK(xml.find("") == std::string::npos); +} + +T_END; + #endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index c4b19619e..2a5b83a94 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -298,3 +298,8 @@ musuite/testAccidentals2.xml # MeasureWriter: the field and its Converter existed but neither side ever # called it, so was silently dropped on round-trip. ksuite/k015a_System_Layout.xml + +# Unblocked by NoteData::isStaffValueSpecified: NoteWriter only wrote +# for multi-staff parts, dropping a source's explicit (but structurally +# redundant) on a single-staff part. +lysuite/ly24c_GraceNote_MeasureEnd.xml