api: preserve an explicit or omitted staff on notes

src/include/mx/api/NoteData.h

@@ -135,6 +135,15 @@ class NoteData
     PitchData pitchData; // step, alter, octave, accidental, etc
     int userRequestedVoiceNumber;
 
+    // Most users can ignore this; leave it unspecified. It only controls whether the note's
+    // optional <staff> element is written. unspecified (the default) applies the right rule
+    // automatically: omit <staff> on a single-staff part (where 1 is implied) and include it
+    // otherwise. yes/no force the element on or off, except that no is ignored for a note that
+    // is not on the first staff (omitting <staff> there would move the note to staff 1). It
+    // exists for round-trip fidelity - reading a file sets yes/no only when the source diverged
+    // from the automatic rule. Same convention as ClefData::writeStaffNumber.
+    Bool writeStaffNumber;
+
     // 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 +198,7 @@ MXAPI_EQUALS_MEMBER(isCue)
 MXAPI_EQUALS_MEMBER(graceSlash)
 MXAPI_EQUALS_MEMBER(pitchData)
 MXAPI_EQUALS_MEMBER(userRequestedVoiceNumber)
+MXAPI_EQUALS_MEMBER(writeStaffNumber)
 MXAPI_EQUALS_MEMBER(crossStaffIndex)
 MXAPI_EQUALS_MEMBER(stem)
 MXAPI_EQUALS_MEMBER(stemPositionData)

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},
+      writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified}, tickTimePosition{0}, durationData{}, beams{},
+      positionData{}, printData{}, noteAttachmentData{}, lyrics{}
 {
 }
 } // namespace api

src/private/mx/impl/NoteFunctions.cpp

@@ -78,6 +78,19 @@ api::NoteData NoteFunctions::parseNote() const
     myOutNoteData.pitchData.octave = reader.getOctave();
     myOutNoteData.userRequestedVoiceNumber = reader.getVoiceNumber();
 
+    // Auto rule (see NoteData::writeStaffNumber): <staff> is included on a multi-staff part and
+    // omitted on a single-staff part. Record an override only when the source diverges from that
+    // rule, so the common case stays unspecified.
+    const bool staffAutoIncludes = myCursor.getNumStaves() > 1;
+    if (reader.getIsStaffSpecified() && !staffAutoIncludes)
+    {
+        myOutNoteData.writeStaffNumber = api::Bool::yes;
+    }
+    else if (!reader.getIsStaffSpecified() && staffAutoIncludes)
+    {
+        myOutNoteData.writeStaffNumber = api::Bool::no;
+    }
+
     myOutNoteData.notehead = converter.convert(reader.getNoteheadValue());
 
     if (reader.getIsDurationTypeSpecified())

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();
     }

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;

src/private/mx/impl/NoteWriter.cpp

@@ -427,9 +427,25 @@ 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)
     {
-        myOutNote.setStaff(myCursor.staffIndex + 1);
+        // Auto rule: <staff> is structurally required on a multi-staff part and implied (1) on a
+        // single-staff part. writeStaffNumber forces the decision either way (round-trip
+        // fidelity), except that no cannot suppress <staff> off the first staff - MusicXML would
+        // read the omission as staff 1.
+        bool includeStaff = myCursor.getNumStaves() > 1;
+        if (myNoteData.writeStaffNumber == api::Bool::yes)
+        {
+            includeStaff = true;
+        }
+        else if (myNoteData.writeStaffNumber == api::Bool::no && myCursor.staffIndex == 0)
+        {
+            includeStaff = false;
+        }
+        if (includeStaff)
+        {
+            myOutNote.setStaff(myCursor.staffIndex + 1);
+        }
     }
 
     const bool sourceHadVoice = myNoteData.userRequestedVoiceNumber != api::VALUE_UNSPECIFIED;

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;
@@ -1557,4 +1558,137 @@ TEST(strongAccentDirection, NoteData)
 
 T_END;
 
+TEST(explicitStaffOnSingleStaffPartRoundTrips, NoteData)
+{
+    const std::string xml = R"(<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<score-partwise version="3.0">
+  <part-list>
+    <score-part id="P1">
+      <part-name>x</part-name>
+    </score-part>
+  </part-list>
+  <part id="P1">
+    <measure number="1">
+      <attributes>
+        <divisions>1</divisions>
+      </attributes>
+      <note>
+        <pitch><step>C</step><octave>4</octave></pitch>
+        <duration>1</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+        <staff>1</staff>
+      </note>
+    </measure>
+  </part>
+</score-partwise>
+)";
+
+    const auto score = fromXml(xml);
+    const auto &notes = score.parts.back().measures.back().staves.back().voices.at(0).notes;
+    REQUIRE(notes.size() == 1);
+    CHECK(notes.at(0).writeStaffNumber == Bool::yes);
+
+    const auto outXml = toXml(score);
+    CHECK(outXml.find("<staff>1</staff>") != std::string::npos);
+}
+
+T_END;
+
+TEST(omittedStaffOnMultiStaffPartRoundTrips, NoteData)
+{
+    const std::string xml = R"(<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<score-partwise version="3.0">
+  <part-list>
+    <score-part id="P1">
+      <part-name>x</part-name>
+    </score-part>
+  </part-list>
+  <part id="P1">
+    <measure number="1">
+      <attributes>
+        <divisions>1</divisions>
+        <staves>2</staves>
+      </attributes>
+      <note>
+        <pitch><step>C</step><octave>5</octave></pitch>
+        <duration>1</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+      <backup><duration>1</duration></backup>
+      <note>
+        <pitch><step>C</step><octave>3</octave></pitch>
+        <duration>1</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+        <staff>2</staff>
+      </note>
+    </measure>
+  </part>
+</score-partwise>
+)";
+
+    const auto score = fromXml(xml);
+    const auto &staves = score.parts.back().measures.back().staves;
+    REQUIRE(staves.size() == 2);
+    const auto &firstStaffNotes = staves.at(0).voices.at(0).notes;
+    const auto &secondStaffNotes = staves.at(1).voices.at(0).notes;
+    REQUIRE(firstStaffNotes.size() == 1);
+    REQUIRE(secondStaffNotes.size() == 1);
+    CHECK(firstStaffNotes.at(0).writeStaffNumber == Bool::no);
+    CHECK(secondStaffNotes.at(0).writeStaffNumber == Bool::unspecified);
+
+    const auto outXml = toXml(score);
+    CHECK(outXml.find("<staff>1</staff>") == std::string::npos);
+    CHECK(outXml.find("<staff>2</staff>") != std::string::npos);
+}
+
+T_END;
+
+TEST(writeStaffNumberNoIsIgnoredOffFirstStaff, 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();
+    measure.staves.emplace_back();
+
+    NoteData note;
+    note.tickTimePosition = 0;
+    note.durationData.durationTimeTicks = score.ticksPerQuarter;
+    note.durationData.durationName = DurationName::quarter;
+    note.writeStaffNumber = Bool::no;
+    measure.staves.at(1).voices[0].notes.push_back(note);
+
+    const auto xml = toXml(score);
+    CHECK(xml.find("<staff>2</staff>") != 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("<staff>") == std::string::npos);
+}
+
+T_END;
+
 #endif

src/private/mxtest/api/roundtrip-baseline.txt

@@ -318,6 +318,24 @@ lysuite/ly11h_TimeSignatures_SenzaMisura.xml
 # composite time-signature round-trip in isolation from those.
 rpatters1/timesigs_composite-ref-modified.musicxml
 
+# Unblocked by NoteData::writeStaffNumber: NoteWriter only wrote <staff>
+# for multi-staff parts, dropping a source's explicit (but structurally
+# redundant) <staff> on a single-staff part.
+lysuite/ly24c_GraceNote_MeasureEnd.xml
+
+# Also unblocked by NoteData::writeStaffNumber, in the other direction: a
+# two-staff part whose last note legally omits <staff> (implied 1). The
+# writer used to inject <staff>1</staff> there; the reader now records
+# writeStaffNumber = no and the omission round-trips.
+lysuite/ly02e_Rests_NoType.xml
+
+# Found already passing when discovery was re-run for the writeStaffNumber
+# work; unlocked by earlier merged changes and never pinned. Pinned here to
+# defend them.
+custom/systems-and-pages.xml
+ksuite/k008a_Beaming.xml
+ksuite/k014a_Fermatas.xml
+
 # The original of the file above, now unblocked by PartData::groups
 # (<score-part>/<group>), MeasureData's measureNumbering multiple-rest-*/
 # system-relation attributes, and SoundData::swing (<sound>/<swing>). Its