From 33aead49e2f453cf98924187761be1204d1ffa8b Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Tue, 7 Jul 2026 16:44:48 +0000 Subject: [PATCH] feat: support multi-note (measured) tremolos in mx::api Add MarkType::tremoloStart/tremoloStop plus MarkData::tremoloMarks (the slash count, 0-8) so a pair spanning two adjacent notes can be authored and round-tripped. The existing tremoloSingleOne..Five glyph marks are unchanged. Pairing across notes is left to the api consumer via document order, matching the isTieStart/ isTieStop precedent -- MusicXML's has no `number` attribute to key off of. Closes #329. --- src/include/mx/api/MarkData.h | 10 +++ src/private/mx/api/MarkData.cpp | 10 +-- src/private/mx/impl/NotationsWriter.cpp | 13 +++- src/private/mx/impl/OrnamentsFunctions.cpp | 16 ++++- src/private/mxtest/api/NoteDataTest.cpp | 74 ++++++++++++++++++++++ 5 files changed, 115 insertions(+), 8 deletions(-) diff --git a/src/include/mx/api/MarkData.h b/src/include/mx/api/MarkData.h index c205736e8..cf7adb9c2 100644 --- a/src/include/mx/api/MarkData.h +++ b/src/include/mx/api/MarkData.h @@ -7,6 +7,7 @@ #include "mx/api/PositionData.h" #include "mx/api/PrintData.h" +#include #include namespace mx @@ -87,6 +88,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 MarkData::tremoloMarks + tremoloStop, ///< The second note of a measured (two-note) tremolo; slash count is MarkData::tremoloMarks otherOrnament, ///< MusicXML's 'other-ornament' value unknownOrnament, ///< Error state @@ -252,6 +255,12 @@ struct MarkData Bool fingeringSubstitution; Bool fingeringAlternate; + // The tremolo mark count (MusicXML text value, 0-8 slashes). Only meaningful + // when markType is tremoloStart or tremoloStop; absent means "not specified" (the writer + // falls back to a default). The tremoloSingle* mark types encode their slash count in the + // enumerator itself and leave this absent. + std::optional tremoloMarks; + MarkData(); MarkData(MarkType inMarkType); MarkData(Placement inPlacement, MarkType inMarkType); @@ -271,6 +280,7 @@ MXAPI_EQUALS_MEMBER(mordentDeparture) MXAPI_EQUALS_MEMBER(hasMordentDeparture) MXAPI_EQUALS_MEMBER(fingeringSubstitution) MXAPI_EQUALS_MEMBER(fingeringAlternate) +MXAPI_EQUALS_MEMBER(tremoloMarks) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(MarkData); } // namespace api diff --git a/src/private/mx/api/MarkData.cpp b/src/private/mx/api/MarkData.cpp index 02cf373bf..f79f6d606 100644 --- a/src/private/mx/api/MarkData.cpp +++ b/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,7 +230,7 @@ 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}, tremoloMarks{} { } @@ -236,7 +238,7 @@ 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}, tremoloMarks{} { 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}, tremoloMarks{} { positionData.placement = inPlacement; impl::Converter converter; diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index 97f8cc8b6..17a7831cd 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/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.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; } diff --git a/src/private/mx/impl/OrnamentsFunctions.cpp b/src/private/mx/impl/OrnamentsFunctions.cpp index dfd673d5b..4800f80af 100644 --- a/src/private/mx/impl/OrnamentsFunctions.cpp +++ b/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.tremoloMarks = 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; diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index 805c2c569..9ad1f835c 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -615,6 +615,80 @@ 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.tremoloMarks = 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.tremoloMarks = 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).tremoloMarks.has_value()); + CHECK_EQUAL(3, *oFirstMarks.at(0).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).tremoloMarks.has_value()); + CHECK_EQUAL(3, *oSecondMarks.at(0).tremoloMarks); +} + +T_END; + +// Parse the synthetic tremolo file (a 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).tremoloMarks.has_value()); + CHECK_EQUAL(1, *marks.at(0).tremoloMarks); +} + +T_END; + TEST(miscFields, NoteData) { ScoreData score;