feat: support multi-note tremolos in mx::api

src/include/mx/api/MarkData.h

@@ -4,9 +4,11 @@
 
 #pragma once
 
+#include "mx/api/MarkDataChoice.h"
 #include "mx/api/PositionData.h"
 #include "mx/api/PrintData.h"
 
+#include <optional>
 #include <string>
 
 namespace mx
@@ -87,6 +89,8 @@ enum class MarkType
     tremoloSingleThree, ///< A tremolo on a single note (a glyph, not a spanner) with 3 slashes
     tremoloSingleFour,  ///< A tremolo on a single note (a glyph, not a spanner) with 4 slashes
     tremoloSingleFive,  ///< A tremolo on a single note (a glyph, not a spanner) with 5 slashes
+    tremoloStart,       ///< The first note of a measured (two-note) tremolo; slash count is in MarkData::choice
+    tremoloStop,        ///< The second note of a measured (two-note) tremolo; slash count is in MarkData::choice
     otherOrnament,      ///< MusicXML's 'other-ornament' value
     unknownOrnament,    ///< Error state
 
@@ -235,23 +239,40 @@ int numTremoloSlashes(MarkType);
 
 struct MarkData
 {
+    // Fields common to (nearly) every mark, regardless of markType.
     MarkType markType;
     std::string name;
     int tickTimePosition;
     PrintData printData;
     PositionData positionData;
+
+    // Fields below apply only to specific mark types, kept as direct fields for backward
+    // compatibility (see AGENTS.md "mx::api conventions" / issue #249; these are not migrated to
+    // 'choice'). Do not follow this shape for new mark-specific data -- add a new MarkDataChoice
+    // alternative instead (see 'choice' below).
+
+    // Mordent payload attributes (MusicXML <mordent> 'long', 'approach', 'departure'). Only
+    // meaningful when markType == MarkType::mordent or MarkType::invertedMordent.
     Bool mordentLong;
     bool hasMordentLong;
     Placement mordentApproach;
     bool hasMordentApproach;
     Placement mordentDeparture;
     bool hasMordentDeparture;
+
     // Fingering payload attributes (MusicXML <fingering> 'substitution' and
     // 'alternate'). Only meaningful when markType == MarkType::fingering. The
     // fingering text itself (e.g. "1", "2-3") is carried in 'name'.
     Bool fingeringSubstitution;
     Bool fingeringAlternate;
 
+    // Payload for mark types whose data does not fit the common fields above. Its Kind SHOULD
+    // correspond to markType (e.g. MarkDataChoice::Kind::tremolo pairs with
+    // MarkType::tremoloStart/tremoloStop), but this is a convention that MarkData does not
+    // enforce. New mark-specific data belongs here, as a new MarkDataChoice alternative -- see
+    // MarkDataChoice.h.
+    MarkDataChoice choice;
+
     MarkData();
     MarkData(MarkType inMarkType);
     MarkData(Placement inPlacement, MarkType inMarkType);
@@ -271,6 +292,7 @@ MXAPI_EQUALS_MEMBER(mordentDeparture)
 MXAPI_EQUALS_MEMBER(hasMordentDeparture)
 MXAPI_EQUALS_MEMBER(fingeringSubstitution)
 MXAPI_EQUALS_MEMBER(fingeringAlternate)
+MXAPI_EQUALS_MEMBER(choice)
 MXAPI_EQUALS_END;
 MXAPI_NOT_EQUALS_AND_VECTORS(MarkData);
 } // namespace api

src/include/mx/api/MarkDataChoice.h

@@ -0,0 +1,77 @@
+// MusicXML Class Library
+// Copyright (c) by Matthew James Briggs
+// Distributed under the MIT License
+
+#pragma once
+
+#include "mx/api/ApiCommon.h"
+
+#include <optional>
+#include <variant>
+
+namespace mx
+{
+namespace api
+{
+
+// Payload for MarkType::tremoloStart / MarkType::tremoloStop: the measured-tremolo slash count
+// (MusicXML <tremolo> text value, 0-8). Absent means "not specified" (the writer falls back to a
+// default). The tremoloSingle* mark types encode their slash count in the enumerator itself and do
+// not use this payload.
+struct TremoloMarkData
+{
+    std::optional<int> tremoloMarks;
+};
+
+MXAPI_EQUALS_BEGIN(TremoloMarkData)
+MXAPI_EQUALS_MEMBER(tremoloMarks)
+MXAPI_EQUALS_END;
+MXAPI_NOT_EQUALS_AND_VECTORS(TremoloMarkData);
+
+// A variant class that carries data for MarkType values whose payload does not fit MarkData's
+// common fields.
+//
+// Justification: MarkData represents every kind of MusicXML mark (articulation, ornament,
+// technical, fermata, dynamic, ...) with a single struct. Most of MarkData's fields are common to
+// (nearly) every mark. A minority of fields only make sense for one specific mark, or a narrow
+// family of marks; adding those directly to MarkData as ad hoc fields does not scale and leaves it
+// unclear which fields apply to which mark. New mark-specific payloads belong here, as a new
+// alternative, rather than as a new direct field on MarkData.
+//
+// The choice's Kind SHOULD correspond to MarkData::markType (e.g. Kind::tremolo pairs with
+// MarkType::tremoloStart/tremoloStop), but this is a convention that this class does not enforce.
+//
+// Defaults to none (no mark-specific payload).
+class MarkDataChoice
+{
+  public:
+    enum class Kind
+    {
+        none,
+        tremolo
+    };
+
+    MarkDataChoice();
+
+    MarkDataChoice(TremoloMarkData value);
+
+    Kind kind() const;
+    bool isNone() const;
+    bool isTremolo() const;
+
+    // Returns a copy of the internally held TremoloMarkData.
+    //
+    // Check isTremolo() first. If this is not a tremolo payload, a default constructed
+    // TremoloMarkData is returned.
+    const TremoloMarkData tremolo() const;
+
+    bool operator==(const MarkDataChoice &other) const;
+
+  private:
+    std::variant<std::monostate, TremoloMarkData> myValue;
+};
+
+MXAPI_NOT_EQUALS_AND_VECTORS(MarkDataChoice);
+
+} // namespace api
+} // namespace mx

src/private/mx/api/MarkData.cpp

@@ -119,6 +119,7 @@ bool isMarkOrnament(MarkType markType)
            (markType == MarkType::schleifer) || (markType == MarkType::tremoloSingleOne) ||
            (markType == MarkType::tremoloSingleTwo) || (markType == MarkType::tremoloSingleThree) ||
            (markType == MarkType::tremoloSingleFour) || (markType == MarkType::tremoloSingleFive) ||
+           (markType == MarkType::tremoloStart) || (markType == MarkType::tremoloStop) ||
            (markType == MarkType::otherOrnament) || (markType == MarkType::unknownOrnament);
 }
 
@@ -163,7 +164,8 @@ bool isMarkTremolo(MarkType markType)
 {
     return (markType == MarkType::tremoloSingleOne) || (markType == MarkType::tremoloSingleTwo) ||
            (markType == MarkType::tremoloSingleThree) || (markType == MarkType::tremoloSingleFour) ||
-           (markType == MarkType::tremoloSingleFive);
+           (markType == MarkType::tremoloSingleFive) || (markType == MarkType::tremoloStart) ||
+           (markType == MarkType::tremoloStop);
 }
 
 bool isMarkCustom(MarkType markType)
@@ -228,15 +230,15 @@ MarkData::MarkData()
     : markType(MarkType::unspecified), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no},
       hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false},
       mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified},
-      fingeringAlternate{Bool::unspecified}
+      fingeringAlternate{Bool::unspecified}, choice{}
 {
 }
 
 MarkData::MarkData(MarkType inMarkType)
     : markType(inMarkType), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no},
       hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false},
       mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified},
-      fingeringAlternate{Bool::unspecified}
+      fingeringAlternate{Bool::unspecified}, choice{}
 {
     impl::Converter converter;
     if (isMarkDynamic(markType))
@@ -257,7 +259,7 @@ MarkData::MarkData(Placement inPlacement, MarkType inMarkType)
     : markType(inMarkType), name{}, tickTimePosition{0}, printData{}, positionData{}, mordentLong{Bool::no},
       hasMordentLong{false}, mordentApproach{Placement::unspecified}, hasMordentApproach{false},
       mordentDeparture{Placement::unspecified}, hasMordentDeparture{false}, fingeringSubstitution{Bool::unspecified},
-      fingeringAlternate{Bool::unspecified}
+      fingeringAlternate{Bool::unspecified}, choice{}
 {
     positionData.placement = inPlacement;
     impl::Converter converter;

src/private/mx/api/MarkDataChoice.cpp

@@ -0,0 +1,52 @@
+// MusicXML Class Library
+// Copyright (c) by Matthew James Briggs
+// Distributed under the MIT License
+
+#include "mx/api/MarkDataChoice.h"
+
+#include <utility>
+
+namespace mx
+{
+namespace api
+{
+
+MarkDataChoice::MarkDataChoice() : myValue{std::monostate{}}
+{
+}
+
+MarkDataChoice::MarkDataChoice(TremoloMarkData value) : myValue{std::move(value)}
+{
+}
+
+MarkDataChoice::Kind MarkDataChoice::kind() const
+{
+    return std::holds_alternative<TremoloMarkData>(myValue) ? Kind::tremolo : Kind::none;
+}
+
+bool MarkDataChoice::isNone() const
+{
+    return std::holds_alternative<std::monostate>(myValue);
+}
+
+bool MarkDataChoice::isTremolo() const
+{
+    return std::holds_alternative<TremoloMarkData>(myValue);
+}
+
+const TremoloMarkData MarkDataChoice::tremolo() const
+{
+    if (const auto *value = std::get_if<TremoloMarkData>(&myValue))
+    {
+        return *value;
+    }
+    return TremoloMarkData{};
+}
+
+bool MarkDataChoice::operator==(const MarkDataChoice &other) const
+{
+    return myValue == other.myValue;
+}
+
+} // namespace api
+} // namespace mx

src/private/mx/impl/NotationsWriter.cpp

@@ -619,9 +619,18 @@ void NotationsWriter::addOrnament(const api::MarkData &mark, core::Ornaments &ou
     }
     case core::OrnamentsGroupChoice::Kind::tremolo: {
         core::Tremolo tremolo;
-        tremolo.setType(core::TremoloType::single());
         setAttributesFromPositionData(mark.positionData, tremolo);
-        tremolo.setValue(core::TremoloMarks{api::numTremoloSlashes(mark.markType)});
+        if (mark.markType == api::MarkType::tremoloStart || mark.markType == api::MarkType::tremoloStop)
+        {
+            tremolo.setType(mark.markType == api::MarkType::tremoloStart ? core::TremoloType::start()
+                                                                         : core::TremoloType::stop());
+            tremolo.setValue(core::TremoloMarks{mark.choice.tremolo().tremoloMarks.value_or(3)});
+        }
+        else
+        {
+            tremolo.setType(core::TremoloType::single());
+            tremolo.setValue(core::TremoloMarks{api::numTremoloSlashes(mark.markType)});
+        }
         group.setChoice(core::OrnamentsGroupChoice::tremolo(tremolo));
         break;
     }

src/private/mx/impl/OrnamentsFunctions.cpp

@@ -160,9 +160,21 @@ void OrnamentsFunctions::parseOrnament(const core::OrnamentsGroupChoice &choiceO
     }
     case core::OrnamentsGroupChoice::Kind::tremolo: {
         const auto &tremolo = choiceObj.asTremolo();
-        const bool isSingle = !tremolo.type().has_value() || *tremolo.type() == core::TremoloType::single();
-        if (!isSingle)
+        const auto type = tremolo.type().value_or(core::TremoloType::single()).tag();
+
+        if (type == core::TremoloType::Tag::start || type == core::TremoloType::Tag::stop)
+        {
+            outMark.name = "tremolo";
+            parseMarkDataAttributes(tremolo, outMark);
+            outMark.markType =
+                (type == core::TremoloType::Tag::start) ? api::MarkType::tremoloStart : api::MarkType::tremoloStop;
+            outMark.choice = api::TremoloMarkData{tremolo.value().value()};
+            break;
+        }
+
+        if (type != core::TremoloType::Tag::single)
         {
+            // unmeasured tremolo -- not yet representable
             outMark.name = "this tremolo is not a mark";
             outMark.markType = api::MarkType::unknownOrnament;
             return;

src/private/mxtest/api/NoteDataTest.cpp

@@ -615,6 +615,83 @@ TEST(tremolos, NoteData)
 
 T_END;
 
+TEST(measuredTremolo, NoteData)
+{
+    ScoreData score;
+    score.ticksPerQuarter = 1;
+    score.parts.emplace_back();
+    auto &part = score.parts.back();
+    part.measures.emplace_back();
+    auto &measure = part.measures.back();
+    measure.staves.emplace_back();
+    auto &staff = measure.staves.back();
+    auto &voice = staff.voices[0];
+
+    voice.notes.emplace_back();
+    auto &firstNote = voice.notes.back();
+    firstNote.durationData.durationName = DurationName::quarter;
+    firstNote.durationData.durationTimeTicks = 1;
+    firstNote.tickTimePosition = 0;
+    MarkData startMark{MarkType::tremoloStart};
+    startMark.choice = TremoloMarkData{3};
+    firstNote.noteAttachmentData.marks.emplace_back(startMark);
+
+    voice.notes.emplace_back();
+    auto &secondNote = voice.notes.back();
+    secondNote.durationData.durationName = DurationName::quarter;
+    secondNote.durationData.durationTimeTicks = 1;
+    secondNote.tickTimePosition = 1;
+    MarkData stopMark{MarkType::tremoloStop};
+    stopMark.choice = TremoloMarkData{3};
+    secondNote.noteAttachmentData.marks.emplace_back(stopMark);
+
+    const auto out = mxtest::roundTrip(score);
+
+    const auto &ovoice = out.parts.back().measures.back().staves.back().voices.at(0);
+    REQUIRE(2 == ovoice.notes.size());
+
+    const auto &oFirstMarks = ovoice.notes.at(0).noteAttachmentData.marks;
+    REQUIRE(1 == oFirstMarks.size());
+    CHECK(MarkType::tremoloStart == oFirstMarks.at(0).markType);
+    REQUIRE(oFirstMarks.at(0).choice.isTremolo());
+    REQUIRE(oFirstMarks.at(0).choice.tremolo().tremoloMarks.has_value());
+    CHECK_EQUAL(3, *oFirstMarks.at(0).choice.tremolo().tremoloMarks);
+
+    const auto &oSecondMarks = ovoice.notes.at(1).noteAttachmentData.marks;
+    REQUIRE(1 == oSecondMarks.size());
+    CHECK(MarkType::tremoloStop == oSecondMarks.at(0).markType);
+    REQUIRE(oSecondMarks.at(0).choice.isTremolo());
+    REQUIRE(oSecondMarks.at(0).choice.tremolo().tremoloMarks.has_value());
+    CHECK_EQUAL(3, *oSecondMarks.at(0).choice.tremolo().tremoloMarks);
+}
+
+T_END;
+
+// Parse the synthetic tremolo file (a <tremolo type="start"> with one slash) and confirm mx::api
+// surfaces it as a tremoloStart mark. This pins the core -> api read path for measured tremolos.
+TEST(measuredTremoloFromSyntheticFile, NoteData)
+{
+    const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/tremolo.3.0.xml";
+    auto &docMgr = DocumentManager::getInstance();
+    const auto docIdResult = docMgr.createFromFile(path);
+    REQUIRE(docIdResult.ok());
+    const int docId = docIdResult.value();
+    const auto scoreResult = docMgr.getData(docId);
+    docMgr.destroyDocument(docId);
+    REQUIRE(scoreResult.ok());
+    const auto &score = scoreResult.value();
+
+    const auto &marks =
+        score.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks;
+    REQUIRE(1 == marks.size());
+    CHECK(MarkType::tremoloStart == marks.at(0).markType);
+    REQUIRE(marks.at(0).choice.isTremolo());
+    REQUIRE(marks.at(0).choice.tremolo().tremoloMarks.has_value());
+    CHECK_EQUAL(1, *marks.at(0).choice.tremolo().tremoloMarks);
+}
+
+T_END;
+
 TEST(miscFields, NoteData)
 {
     ScoreData score;