diff --git a/src/include/mx/api/MarkDataChoice.h b/src/include/mx/api/MarkDataChoice.h index 44a8e0de..f801bb52 100644 --- a/src/include/mx/api/MarkDataChoice.h +++ b/src/include/mx/api/MarkDataChoice.h @@ -7,6 +7,7 @@ #include "mx/api/ApiCommon.h" #include +#include #include namespace mx @@ -28,6 +29,60 @@ MXAPI_EQUALS_MEMBER(tremoloMarks) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(TremoloMarkData); +// Payload for the arpeggiate mark types (MarkType::arpeggiate / arpeggiateUp / arpeggiateDown). +// The up/down direction of the arpeggio's wavy line is encoded in the MarkType itself. +struct ArpeggiateMarkData +{ + // Distinguishes overlapping arpeggios: notes sharing a number belong to one arpeggio (e.g. + // an arpeggio spanning both staves of a keyboard part) while different numbers arpeggiate + // independently. Corresponds to the `number` attribute. + std::optional number; + + // yes: the arpeggio is drawn as one unbroken line across staves instead of one line per + // staff. Corresponds to the `unbroken` attribute (MusicXML 4.0). + Bool unbroken = Bool::unspecified; + + // The element's `id` attribute (MusicXML 3.1). + std::optional id; +}; + +MXAPI_EQUALS_BEGIN(ArpeggiateMarkData) +MXAPI_EQUALS_MEMBER(number) +MXAPI_EQUALS_MEMBER(unbroken) +MXAPI_EQUALS_MEMBER(id) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(ArpeggiateMarkData); + +// Which end of a non-arpeggiate bracket a note carries: the bracket's top or bottom. +enum class NonArpeggiatePlacement +{ + top, + bottom, +}; + +// Payload for MarkType::nonArpeggiate: a bracket indicating that a chord is NOT to be +// arpeggiated. MusicXML marks only the top and bottom notes of the chord, each carrying a +// element that states which end of the bracket it is. +struct NonArpeggiateMarkData +{ + // Which end of the bracket this note carries. Corresponds to the required `type` attribute. + NonArpeggiatePlacement placement = NonArpeggiatePlacement::top; + + // Distinguishes overlapping brackets, like ArpeggiateMarkData::number. Corresponds to the + // `number` attribute. + std::optional number; + + // The element's `id` attribute (MusicXML 3.1). + std::optional id; +}; + +MXAPI_EQUALS_BEGIN(NonArpeggiateMarkData) +MXAPI_EQUALS_MEMBER(placement) +MXAPI_EQUALS_MEMBER(number) +MXAPI_EQUALS_MEMBER(id) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(NonArpeggiateMarkData); + // A variant class that carries data for MarkType values whose payload does not fit MarkData's // common fields. // @@ -48,16 +103,24 @@ class MarkDataChoice enum class Kind { none, - tremolo + tremolo, + arpeggiate, + nonArpeggiate }; MarkDataChoice(); MarkDataChoice(TremoloMarkData value); + MarkDataChoice(ArpeggiateMarkData value); + + MarkDataChoice(NonArpeggiateMarkData value); + Kind kind() const; bool isNone() const; bool isTremolo() const; + bool isArpeggiate() const; + bool isNonArpeggiate() const; // Returns a copy of the internally held TremoloMarkData. // @@ -65,10 +128,22 @@ class MarkDataChoice // TremoloMarkData is returned. const TremoloMarkData tremolo() const; + // Returns a copy of the internally held ArpeggiateMarkData. + // + // Check isArpeggiate() first. If this is not an arpeggiate payload, a default constructed + // ArpeggiateMarkData is returned. + const ArpeggiateMarkData arpeggiate() const; + + // Returns a copy of the internally held NonArpeggiateMarkData. + // + // Check isNonArpeggiate() first. If this is not a non-arpeggiate payload, a default + // constructed NonArpeggiateMarkData is returned. + const NonArpeggiateMarkData nonArpeggiate() const; + bool operator==(const MarkDataChoice &other) const; private: - std::variant myValue; + std::variant myValue; }; MXAPI_NOT_EQUALS_AND_VECTORS(MarkDataChoice); diff --git a/src/private/mx/api/MarkDataChoice.cpp b/src/private/mx/api/MarkDataChoice.cpp index 2e429628..ad8e3d7b 100644 --- a/src/private/mx/api/MarkDataChoice.cpp +++ b/src/private/mx/api/MarkDataChoice.cpp @@ -19,9 +19,29 @@ MarkDataChoice::MarkDataChoice(TremoloMarkData value) : myValue{std::move(value) { } +MarkDataChoice::MarkDataChoice(ArpeggiateMarkData value) : myValue{std::move(value)} +{ +} + +MarkDataChoice::MarkDataChoice(NonArpeggiateMarkData value) : myValue{std::move(value)} +{ +} + MarkDataChoice::Kind MarkDataChoice::kind() const { - return std::holds_alternative(myValue) ? Kind::tremolo : Kind::none; + if (std::holds_alternative(myValue)) + { + return Kind::tremolo; + } + if (std::holds_alternative(myValue)) + { + return Kind::arpeggiate; + } + if (std::holds_alternative(myValue)) + { + return Kind::nonArpeggiate; + } + return Kind::none; } bool MarkDataChoice::isNone() const @@ -43,6 +63,34 @@ const TremoloMarkData MarkDataChoice::tremolo() const return TremoloMarkData{}; } +bool MarkDataChoice::isArpeggiate() const +{ + return std::holds_alternative(myValue); +} + +const ArpeggiateMarkData MarkDataChoice::arpeggiate() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return ArpeggiateMarkData{}; +} + +bool MarkDataChoice::isNonArpeggiate() const +{ + return std::holds_alternative(myValue); +} + +const NonArpeggiateMarkData MarkDataChoice::nonArpeggiate() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return NonArpeggiateMarkData{}; +} + bool MarkDataChoice::operator==(const MarkDataChoice &other) const { return myValue == other.myValue; diff --git a/src/private/mx/impl/ArpeggiateFunctions.cpp b/src/private/mx/impl/ArpeggiateFunctions.cpp index 0d121690..79984af4 100644 --- a/src/private/mx/impl/ArpeggiateFunctions.cpp +++ b/src/private/mx/impl/ArpeggiateFunctions.cpp @@ -4,6 +4,7 @@ #include "mx/impl/ArpeggiateFunctions.h" #include "mx/core/generated/Arpeggiate.h" +#include "mx/impl/Converter.h" #include "mx/impl/MarkDataFunctions.h" namespace mx @@ -36,6 +37,22 @@ api::MarkData ArpeggiateFunctions::parseArpeggiate() const impl::parseMarkDataAttributes(myArpeggiate, markData); markData.tickTimePosition = myCursor.tickTimePosition; + api::ArpeggiateMarkData arpeggiateData{}; + if (myArpeggiate.number().has_value()) + { + arpeggiateData.number = myArpeggiate.number()->value(); + } + if (myArpeggiate.unbroken().has_value()) + { + Converter converter; + arpeggiateData.unbroken = converter.convert(*myArpeggiate.unbroken()); + } + if (myArpeggiate.id().has_value()) + { + arpeggiateData.id = myArpeggiate.id()->value(); + } + markData.choice = arpeggiateData; + return markData; } } // namespace impl diff --git a/src/private/mx/impl/NonArpeggiateFunctions.cpp b/src/private/mx/impl/NonArpeggiateFunctions.cpp index 70eb137b..c78e957a 100644 --- a/src/private/mx/impl/NonArpeggiateFunctions.cpp +++ b/src/private/mx/impl/NonArpeggiateFunctions.cpp @@ -21,6 +21,21 @@ api::MarkData NonArpeggiateFunctions::parseNonArpeggiate() const api::MarkData markData{api::MarkType::nonArpeggiate}; impl::parseMarkDataAttributes(myNonArpeggiate, markData); markData.tickTimePosition = myCursor.tickTimePosition; + + api::NonArpeggiateMarkData nonArpeggiateData{}; + nonArpeggiateData.placement = myNonArpeggiate.type().tag() == core::TopBottom::Tag::bottom + ? api::NonArpeggiatePlacement::bottom + : api::NonArpeggiatePlacement::top; + if (myNonArpeggiate.number().has_value()) + { + nonArpeggiateData.number = myNonArpeggiate.number()->value(); + } + if (myNonArpeggiate.id().has_value()) + { + nonArpeggiateData.id = myNonArpeggiate.id()->value(); + } + markData.choice = nonArpeggiateData; + return markData; } } // namespace impl diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index aec8f86a..cf7312ec 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/src/private/mx/impl/NotationsWriter.cpp @@ -3,6 +3,7 @@ // Distributed under the MIT License #include "mx/impl/NotationsWriter.h" +#include "mx/core/Token.h" #include "mx/core/generated/Arpeggiate.h" #include "mx/core/generated/ArrowChoice.h" #include "mx/core/generated/ArrowChoiceGroup.h" @@ -30,6 +31,7 @@ #include "mx/core/generated/Mordent.h" #include "mx/core/generated/NonArpeggiate.h" #include "mx/core/generated/NotationsChoice.h" +#include "mx/core/generated/NumberLevel.h" #include "mx/core/generated/OrnamentsGroup.h" #include "mx/core/generated/OrnamentsGroupChoice.h" #include "mx/core/generated/OtherPlacementText.h" @@ -41,6 +43,7 @@ #include "mx/core/generated/StrongAccent.h" #include "mx/core/generated/TechnicalChoice.h" #include "mx/core/generated/Tied.h" +#include "mx/core/generated/TopBottom.h" #include "mx/core/generated/Tremolo.h" #include "mx/core/generated/TremoloMarks.h" #include "mx/core/generated/TremoloType.h" @@ -52,6 +55,7 @@ #include "mx/core/generated/UpDown.h" #include "mx/core/generated/UprightInverted.h" #include "mx/core/generated/WavyLine.h" +#include "mx/impl/Converter.h" #include "mx/impl/CurveFunctions.h" #include "mx/impl/DynamicsWriter.h" #include "mx/impl/MarkDataFunctions.h" @@ -358,6 +362,20 @@ core::Notations NotationsWriter::getNotations() const { core::NonArpeggiate nonArpeggiate; impl::setAttributesFromMarkData(mark, nonArpeggiate); + + const auto nonArpeggiateData = mark.choice.nonArpeggiate(); + nonArpeggiate.setType(nonArpeggiateData.placement == api::NonArpeggiatePlacement::bottom + ? core::TopBottom::bottom() + : core::TopBottom::top()); + if (nonArpeggiateData.number.has_value()) + { + nonArpeggiate.setNumber(core::NumberLevel{*nonArpeggiateData.number}); + } + if (nonArpeggiateData.id.has_value()) + { + nonArpeggiate.setID(core::Token{*nonArpeggiateData.id}); + } + outNotations.addChoice(core::NotationsChoice::nonArpeggiate(nonArpeggiate)); } else if (isMarkArpeggiate(mark.markType)) @@ -378,6 +396,21 @@ core::Notations NotationsWriter::getNotations() const arpeggiate.setDirection(core::UpDown::down()); } + const auto arpeggiateData = mark.choice.arpeggiate(); + if (arpeggiateData.number.has_value()) + { + arpeggiate.setNumber(core::NumberLevel{*arpeggiateData.number}); + } + if (arpeggiateData.unbroken != api::Bool::unspecified) + { + Converter converter; + arpeggiate.setUnbroken(converter.convert(arpeggiateData.unbroken)); + } + if (arpeggiateData.id.has_value()) + { + arpeggiate.setID(core::Token{*arpeggiateData.id}); + } + outNotations.addChoice(core::NotationsChoice::arpeggiate(arpeggiate)); } } diff --git a/src/private/mxtest/api/MarkRoundTripTest.cpp b/src/private/mxtest/api/MarkRoundTripTest.cpp index 1a519868..de76fcc1 100644 --- a/src/private/mxtest/api/MarkRoundTripTest.cpp +++ b/src/private/mxtest/api/MarkRoundTripTest.cpp @@ -19,7 +19,7 @@ namespace // Round-trips a single note-attached mark of the given MarkType through the // full serialize -> deserialize path and returns the marks read back from the // first note. Used by the enum data-loss regression tests below. -std::vector roundTripMark(MarkType inMarkType) +std::vector roundTripMarkData(const MarkData &inMarkData) { ScoreData score; score.parts.emplace_back(); @@ -31,7 +31,7 @@ std::vector roundTripMark(MarkType inMarkType) auto &voice = staff.voices[0]; voice.notes.emplace_back(); auto ¬e = voice.notes.back(); - note.noteAttachmentData.marks.emplace_back(Placement::unspecified, inMarkType); + note.noteAttachmentData.marks.push_back(inMarkData); auto &mgr = DocumentManager::getInstance(); const auto r1 = mgr.createFromScore(score); @@ -58,6 +58,11 @@ std::vector roundTripMark(MarkType inMarkType) return onote.noteAttachmentData.marks; } +std::vector roundTripMark(MarkType inMarkType) +{ + return roundTripMarkData(MarkData{Placement::unspecified, inMarkType}); +} + bool hasMark(const std::vector &marks, MarkType inMarkType) { for (const auto &m : marks) @@ -227,4 +232,40 @@ TEST(FermataCurlew, MarkRoundTrip) T_END; +TEST(ArpeggiateAttributes, MarkRoundTrip) +{ + MarkData mark{Placement::unspecified, MarkType::arpeggiateUp}; + ArpeggiateMarkData payload{}; + payload.number = 2; + payload.unbroken = Bool::yes; + payload.id = "arp-1"; + mark.choice = payload; + + const auto marks = roundTripMarkData(mark); + REQUIRE(marks.size() == 1); + CHECK(marks.front().markType == MarkType::arpeggiateUp); + REQUIRE(marks.front().choice.isArpeggiate()); + CHECK(payload == marks.front().choice.arpeggiate()); +} + +T_END; + +TEST(NonArpeggiateAttributes, MarkRoundTrip) +{ + MarkData mark{Placement::unspecified, MarkType::nonArpeggiate}; + NonArpeggiateMarkData payload{}; + payload.placement = NonArpeggiatePlacement::bottom; + payload.number = 1; + payload.id = "nonarp-1"; + mark.choice = payload; + + const auto marks = roundTripMarkData(mark); + REQUIRE(marks.size() == 1); + CHECK(marks.front().markType == MarkType::nonArpeggiate); + REQUIRE(marks.front().choice.isNonArpeggiate()); + CHECK(payload == marks.front().choice.nonArpeggiate()); +} + +T_END; + #endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 64395d60..c21fbe8c 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -398,3 +398,12 @@ synthetic/voice.3.0.xml synthetic/word-font.3.0.xml synthetic/work-number.3.0.xml synthetic/work-title.3.0.xml + +# Unblocked by the arpeggiate/non-arpeggiate attribute payloads +# (ArpeggiateMarkData / NonArpeggiateMarkData in MarkDataChoice): number, +# unbroken, id, and the non-arpeggiate top/bottom type now round-trip. +synthetic/arpeggiate.3.0.xml +synthetic/arpeggiate.3.1.xml +synthetic/arpeggiate.4.0.xml +synthetic/non-arpeggiate.3.0.xml +synthetic/non-arpeggiate.3.1.xml