diff --git a/src/include/mx/api/VoiceData.h b/src/include/mx/api/VoiceData.h index b919edfe8..384a2e37e 100644 --- a/src/include/mx/api/VoiceData.h +++ b/src/include/mx/api/VoiceData.h @@ -16,12 +16,19 @@ namespace api class VoiceData { public: + // What MusicXML calls this voice, the text of the 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 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); diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index 8adc66065..27d0bfa79 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/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(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 &¬eData, int staff, int voice) const +void MeasureReader::insertNoteData(api::NoteData &¬eData, int staff, int voice, const std::string &voiceLabel) const { MX_ASSERT(staff >= 0); MX_ASSERT(static_cast(staff) < myOutMeasureData.staves.size()); @@ -1169,6 +1170,14 @@ void MeasureReader::insertNoteData(api::NoteData &¬eData, 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 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 diff --git a/src/private/mx/impl/MeasureReader.h b/src/private/mx/impl/MeasureReader.h index d1f756ee9..5edbc05d7 100644 --- a/src/private/mx/impl/MeasureReader.h +++ b/src/private/mx/impl/MeasureReader.h @@ -113,9 +113,10 @@ class MeasureReader void importStaffDetails(const core::Attributes &inMxAttributes) const; void importClefs(std::span inClefs) const; void importClef(const core::Clef &inClef) const; - void insertNoteData(api::NoteData &¬eData, int staff, int voice) const; + void insertNoteData(api::NoteData &¬eData, 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; diff --git a/src/private/mx/impl/MeasureWriter.cpp b/src/private/mx/impl/MeasureWriter.cpp index 8fb7d7e5e..e03f071de 100644 --- a/src/private/mx/impl/MeasureWriter.cpp +++ b/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)); diff --git a/src/private/mx/impl/NoteReader.cpp b/src/private/mx/impl/NoteReader.cpp index 47c567195..191d0b83b 100644 --- a/src/private/mx/impl/NoteReader.cpp +++ b/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() diff --git a/src/private/mx/impl/NoteReader.h b/src/private/mx/impl/NoteReader.h index a4591b06f..44e560f3a 100644 --- a/src/private/mx/impl/NoteReader.h +++ b/src/private/mx/impl/NoteReader.h @@ -129,6 +129,13 @@ class NoteReader return myVoiceNumber; } + // The text of the note's 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 myNoteheadFilled; std::optional myNoteheadSmufl; diff --git a/src/private/mx/impl/NoteWriter.cpp b/src/private/mx/impl/NoteWriter.cpp index 24deff2ea..f82ab9ed9 100644 --- a/src/private/mx/impl/NoteWriter.cpp +++ b/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 &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 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 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 ¬e) { + 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)); } } diff --git a/src/private/mx/impl/NoteWriter.h b/src/private/mx/impl/NoteWriter.h index 1073e83bf..6b961ecde 100644 --- a/src/private/mx/impl/NoteWriter.h +++ b/src/private/mx/impl/NoteWriter.h @@ -12,6 +12,7 @@ #include "mx/impl/Converter.h" #include "mx/impl/MeasureCursor.h" +#include #include 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 &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 &mySiblingNotes; const int myNoteIndex; const int myNumVoices; + const std::string &myVoiceLabel; mutable core::Note myOutNote; mutable core::FullNoteGroup myOutFullNoteGroup; mutable std::vector myOutTies; diff --git a/src/private/mxtest/api/VoiceLabelApiTest.cpp b/src/private/mxtest/api/VoiceLabelApiTest.cpp new file mode 100644 index 000000000..a60e5b213 --- /dev/null +++ b/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("1") != std::string::npos); + CHECK(xml.find("2") != 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("FirstVoice") != std::string::npos); + CHECK(xml.find("SecondVoice") != 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("5") != 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("5") != 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 diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 112214d07..61bf8738b 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -726,3 +726,10 @@ lysuite/ly33b_Spanners_Tie.xml # the ordering is fixed, but the writer restates / # on a start whose source omits them.) rpatters1/gliss_to_rest-ref.xml + +# Unblocked by VoiceData::label: mx renumbered every voice from the position of +# its container, so a voice whose 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