feat: preserve source voice names in mx::api

src/include/mx/api/VoiceData.h

@@ -16,12 +16,19 @@ namespace api
 class VoiceData
 {
   public:
+    // What MusicXML calls this voice, the text of the <voice> element on each of its notes.
+    // Voices are normally numbered and the api keys them by index, so leave this empty and each
+    // voice is written with its own number. Set it when the name is not that number: a part
+    // whose voices are numbered straight through its staves instead of restarting on each one,
+    // or a program that writes words such as "FirstVoice" rather than numbers.
+    std::string label;
+
     std::vector<NoteData> notes;
 };
 
 inline bool operator==(const VoiceData &lhs, const VoiceData &rhs)
 {
-    return areVectorsEqual(lhs.notes, rhs.notes);
+    return lhs.label == rhs.label && areVectorsEqual(lhs.notes, rhs.notes);
 }
 
 MXAPI_NOT_EQUALS_AND_VECTORS(VoiceData);

src/private/mx/impl/MeasureReader.cpp

@@ -431,7 +431,8 @@ void MeasureReader::parseNote(const core::Note &inMxNote, const core::Note *next
     MX_ASSERT(bucketStaffIndex >= 0);
     MX_ASSERT(static_cast<size_t>(bucketStaffIndex) < myOutMeasureData.staves.size());
     myPreviousNoteBucketStaffIndex = bucketStaffIndex;
-    insertNoteData(std::move(noteData), myCurrentCursor.staffIndex, myCurrentCursor.voiceIndex);
+    insertNoteData(std::move(noteData), myCurrentCursor.staffIndex, myCurrentCursor.voiceIndex,
+                   noteReader.getVoiceLabel());
 }
 
 void MeasureReader::scanForCrossStaffBeamGroups() const
@@ -1158,7 +1159,7 @@ void MeasureReader::importClef(const core::Clef &inClef) const
     insertClef(std::move(clefData), celfStaffIndex);
 }
 
-void MeasureReader::insertNoteData(api::NoteData &&noteData, int staff, int voice) const
+void MeasureReader::insertNoteData(api::NoteData &&noteData, int staff, int voice, const std::string &voiceLabel) const
 {
     MX_ASSERT(staff >= 0);
     MX_ASSERT(static_cast<size_t>(staff) < myOutMeasureData.staves.size());
@@ -1169,6 +1170,14 @@ void MeasureReader::insertNoteData(api::NoteData &&noteData, int staff, int voic
         staffRef.voices[voice] = api::VoiceData{};
     }
     auto &voiceRef = staffRef.voices[voice];
+
+    // The first note that names the voice names it for all of them. Notes without a <voice> get
+    // the name of the voice they land in.
+    if (voiceRef.label.empty())
+    {
+        voiceRef.label = voiceLabel;
+    }
+
     voiceRef.notes.emplace_back(std::move(noteData));
 }
 
@@ -1193,6 +1202,25 @@ void MeasureReader::consolidateVoicesForAllStaves() const
             collapseVoicesAutomatically(staff);
         }
     }
+
+    dropRedundantVoiceLabels();
+}
+
+// A voice whose name is the number the writer would give it anyway does not need to carry one.
+// Clearing those keeps VoiceData::label empty in the common case, where it stands for a voice
+// name the index cannot express.
+void MeasureReader::dropRedundantVoiceLabels() const
+{
+    for (auto &staff : myOutMeasureData.staves)
+    {
+        for (auto &voicePair : staff.voices)
+        {
+            if (voicePair.second.label == std::to_string(voicePair.first + 1))
+            {
+                voicePair.second.label.clear();
+            }
+        }
+    }
 }
 
 void MeasureReader::takeUserRequestedVoiceNumbers(api::StaffData &staff) const

src/private/mx/impl/MeasureReader.h

@@ -113,9 +113,10 @@ class MeasureReader
     void importStaffDetails(const core::Attributes &inMxAttributes) const;
     void importClefs(std::span<const core::Clef> inClefs) const;
     void importClef(const core::Clef &inClef) const;
-    void insertNoteData(api::NoteData &&noteData, int staff, int voice) const;
+    void insertNoteData(api::NoteData &&noteData, int staff, int voice, const std::string &voiceLabel) const;
     void insertClef(api::ClefData &&clefData, int staff) const;
     void consolidateVoicesForAllStaves() const;
+    void dropRedundantVoiceLabels() const;
     void takeUserRequestedVoiceNumbers(api::StaffData &staff) const;
     void collapseVoicesAutomatically(api::StaffData &staff) const;
     bool isUserRequestedVoiceNumberConsistent(const api::VoiceData &voiceData) const;

src/private/mx/impl/MeasureWriter.cpp

@@ -602,7 +602,8 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff)
                               myPreviousCursor.isChordActive,
                               voice.second.notes,
                               noteIndex,
-                              numVoices};
+                              numVoices,
+                              voice.second.label};
             myOutMeasure.addMusicData(core::MusicDataChoice::note(writer.getNote(isStartOfChord)));
             myHistory.log("addNote cursorTime " + std::to_string(myHistory.getCursor().tickTimePosition) +
                           ", noteTime " + std::to_string(apiNote.tickTimePosition));

src/private/mx/impl/NoteReader.cpp

@@ -154,9 +154,9 @@ NoteReader::NoteReader(const core::Note &mxNote)
       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), myIsStaffSpecified(false),
-      myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myNoteheadFilled{}, myNoteheadSmufl{},
-      myDurationType(core::NoteTypeValue::maxima()), myIsDurationTypeSpecified(false), myNumDots(0), myBeams(),
-      myTimeModificationActualNotes(-1), myTimeModificationNormalNotes(-1),
+      myVoiceNumber(0), myVoiceLabel{}, myNoteheadValue(core::NoteheadValue::normal()), myNoteheadFilled{},
+      myNoteheadSmufl{}, 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()), myIsAccidentalParenthetical(false),
       myIsAccidentalCautionary{false}, myIsAccidentalEditorial{false}, myIsAccidentalBracketed{false},
@@ -328,13 +328,15 @@ void NoteReader::setStaffNumber()
 void NoteReader::setVoiceNumber()
 {
     myVoiceNumber = api::VALUE_UNSPECIFIED;
+    myVoiceLabel.clear();
 
     if (!myNote.editorialVoice().voice().has_value())
     {
         return;
     }
 
-    utility::stringToInt(myNote.editorialVoice().voice()->c_str(), myVoiceNumber);
+    myVoiceLabel = *myNote.editorialVoice().voice();
+    utility::stringToInt(myVoiceLabel.c_str(), myVoiceNumber);
 }
 
 void NoteReader::setNoteheadItems()

src/private/mx/impl/NoteReader.h

@@ -129,6 +129,13 @@ class NoteReader
         return myVoiceNumber;
     }
 
+    // The text of the note's <voice> element, empty when it had none. A voice name does not have
+    // to be a number, so getVoiceNumber above cannot stand in for it.
+    inline const std::string &getVoiceLabel() const
+    {
+        return myVoiceLabel;
+    }
+
     inline core::NoteheadValue getNoteheadValue() const
     {
         return myNoteheadValue;
@@ -270,6 +277,7 @@ class NoteReader
     int myStaffNumber;
     bool myIsStaffSpecified;
     int myVoiceNumber;
+    std::string myVoiceLabel;
     core::NoteheadValue myNoteheadValue;
     std::optional<core::YesNo> myNoteheadFilled;
     std::optional<std::string> myNoteheadSmufl;

src/private/mx/impl/NoteWriter.cpp

@@ -69,11 +69,11 @@ core::Syllabic convertLyricSyllabicForNoteWriter(api::LyricSyllabic value)
 
 NoteWriter::NoteWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter,
                        bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes,
-                       int inNoteIndex, int inNumVoices)
+                       int inNoteIndex, int inNumVoices, const std::string &inVoiceLabel)
     : myNoteData{inNoteData}, myCursor{inCursor}, myScoreWriter{inScoreWriter}, myConverter{},
       myIsPreviousNoteAChordMember{isPreviousNoteAChordMember}, mySiblingNotes{inSiblingNotes},
-      myNoteIndex{inNoteIndex}, myNumVoices{inNumVoices}, myOutNote{}, myOutFullNoteGroup{}, myOutTies{},
-      myOutTieNotationsChoices{}
+      myNoteIndex{inNoteIndex}, myNumVoices{inNumVoices}, myVoiceLabel{inVoiceLabel}, myOutNote{}, myOutFullNoteGroup{},
+      myOutTies{}, myOutTieNotationsChoices{}
 {
 }
 
@@ -492,13 +492,24 @@ void NoteWriter::setStaffAndVoice() const
         }
     }
 
+    // VoiceData::label decides what goes in the <voice> element, replacing the voice's number
+    // when the number cannot express its name. Which notes get the element is otherwise
+    // unchanged, so a source that left <voice> off a chord member still does. The exception is a
+    // voice where no note carried a number: there the label is the only thing asking for the
+    // element, so it is what puts one there.
+    const bool hasVoiceLabel = !myVoiceLabel.empty();
     const bool sourceHadVoice = myNoteData.userRequestedVoiceNumber != api::VALUE_UNSPECIFIED;
     const bool isNonDefaultVoice = myCursor.voiceIndex > 0;
     const bool isMultiVoiceStaff = myNumVoices > 1;
-    if (myCursor.voiceIndex >= 0 && (sourceHadVoice || isNonDefaultVoice || isMultiVoiceStaff))
+    const bool isVoiceNumbered = std::any_of(mySiblingNotes.cbegin(), mySiblingNotes.cend(), [](const auto &note) {
+        return note.userRequestedVoiceNumber != api::VALUE_UNSPECIFIED;
+    });
+    const bool labelAsksForVoice = hasVoiceLabel && !isVoiceNumbered;
+
+    if (myCursor.voiceIndex >= 0 && (sourceHadVoice || isNonDefaultVoice || isMultiVoiceStaff || labelAsksForVoice))
     {
         auto editorialVoice = myOutNote.editorialVoice();
-        editorialVoice.setVoice(std::to_string(myCursor.voiceIndex + 1));
+        editorialVoice.setVoice(hasVoiceLabel ? myVoiceLabel : std::to_string(myCursor.voiceIndex + 1));
         myOutNote.setEditorialVoice(std::move(editorialVoice));
     }
 }

src/private/mx/impl/NoteWriter.h

@@ -12,6 +12,7 @@
 #include "mx/impl/Converter.h"
 #include "mx/impl/MeasureCursor.h"
 
+#include <string>
 #include <vector>
 
 namespace mx
@@ -25,7 +26,7 @@ class NoteWriter
   public:
     NoteWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter,
                bool isPreviousNoteAChordMember, const std::vector<mx::api::NoteData> &inSiblingNotes, int inNoteIndex,
-               int inNumVoices);
+               int inNumVoices, const std::string &inVoiceLabel);
 
     core::Note getNote(bool isStartOfChord) const;
 
@@ -38,6 +39,7 @@ class NoteWriter
     const std::vector<mx::api::NoteData> &mySiblingNotes;
     const int myNoteIndex;
     const int myNumVoices;
+    const std::string &myVoiceLabel;
     mutable core::Note myOutNote;
     mutable core::FullNoteGroup myOutFullNoteGroup;
     mutable std::vector<core::Tie> myOutTies;

src/private/mxtest/api/VoiceLabelApiTest.cpp

@@ -0,0 +1,108 @@
+// 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-staff score with the requested number of voices, each holding a whole note
+// so the voices overlap and all of them are written.
+ScoreData voiceLabelMakeScore(int numVoices)
+{
+    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();
+    measure.staves.emplace_back();
+
+    for (int i = 0; i < numVoices; ++i)
+    {
+        auto &voice = measure.staves.back().voices[i];
+        voice.notes.emplace_back();
+        voice.notes.back().durationData.durationName = DurationName::whole;
+        voice.notes.back().durationData.durationTimeTicks = 16;
+    }
+
+    return score;
+}
+
+TEST(anUnlabeledVoiceIsWrittenWithItsNumber, VoiceLabel)
+{
+    const auto score = voiceLabelMakeScore(2);
+    const auto xml = toXml(score);
+    CHECK(xml.find("<voice>1</voice>") != std::string::npos);
+    CHECK(xml.find("<voice>2</voice>") != std::string::npos);
+
+    const auto out = roundTrip(score);
+    const auto &voices = out.parts.at(0).measures.at(0).staves.at(0).voices;
+    REQUIRE(voices.size() == 2);
+    CHECK_EQUAL("", voices.at(0).label);
+    CHECK_EQUAL("", voices.at(1).label);
+}
+
+// MusicXML does not require a voice to be named with a number, and some programs write words.
+// The number the api would otherwise give the voice cannot express that.
+TEST(aNamedVoiceKeepsItsName, VoiceLabel)
+{
+    auto score = voiceLabelMakeScore(2);
+    score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).label = "FirstVoice";
+    score.parts.at(0).measures.at(0).staves.at(0).voices.at(1).label = "SecondVoice";
+
+    const auto xml = toXml(score);
+    CHECK(xml.find("<voice>FirstVoice</voice>") != std::string::npos);
+    CHECK(xml.find("<voice>SecondVoice</voice>") != std::string::npos);
+
+    const auto out = roundTrip(score);
+    const auto &voices = out.parts.at(0).measures.at(0).staves.at(0).voices;
+    REQUIRE(voices.size() == 2);
+    CHECK_EQUAL("FirstVoice", voices.at(0).label);
+    CHECK_EQUAL("SecondVoice", voices.at(1).label);
+}
+
+// A lone voice named 5 is written as voice 5 rather than renumbered to 1. Reading it back turns
+// the number into the voice's index, so the name is no longer needed and the label comes back
+// empty. Writing that again still gives voice 5.
+TEST(aVoiceNamedPastTheStaffVoiceCountKeepsItsNumber, VoiceLabel)
+{
+    auto score = voiceLabelMakeScore(1);
+    score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).label = "5";
+
+    const auto xml = toXml(score);
+    CHECK(xml.find("<voice>5</voice>") != std::string::npos);
+
+    const auto out = roundTrip(score);
+    const auto &voices = out.parts.at(0).measures.at(0).staves.at(0).voices;
+    REQUIRE(voices.size() == 1);
+    REQUIRE(voices.count(4) == 1);
+    CHECK_EQUAL("", voices.at(4).label);
+    CHECK(toXml(out).find("<voice>5</voice>") != std::string::npos);
+}
+
+// Reading drops a name that is just the number the writer would have used anyway, so the common
+// case comes back with an empty label.
+TEST(aNameThatMatchesTheVoiceNumberIsDropped, VoiceLabel)
+{
+    auto score = voiceLabelMakeScore(2);
+    score.parts.at(0).measures.at(0).staves.at(0).voices.at(0).label = "1";
+    score.parts.at(0).measures.at(0).staves.at(0).voices.at(1).label = "2";
+
+    const auto out = roundTrip(score);
+    const auto &voices = out.parts.at(0).measures.at(0).staves.at(0).voices;
+    REQUIRE(voices.size() == 2);
+    CHECK_EQUAL("", voices.at(0).label);
+    CHECK_EQUAL("", voices.at(1).label);
+}
+
+#endif

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

@@ -717,3 +717,10 @@ synthetic/lyric.3.0.xml
 lysuite/ly02b_Rests_PitchedRests.xml
 lysuite/ly32b_Articulations_Texts.xml
 lysuite/ly33b_Spanners_Tie.xml
+
+# Unblocked by VoiceData::label: mx renumbered every voice from the position of
+# its container, so a voice whose <voice> element said something else -- a word
+# rather than a number, or a number the api's own index does not reproduce --
+# was written under the wrong name.
+musuite/testChordNoVoice.xml
+musuite/testStringVoiceName.xml