From d59d32d6c3393bcd249f3f7d987147a22da88074 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Sun, 2 Aug 2026 07:07:23 -0500 Subject: [PATCH 1/2] feat: support SMuFL other marks in mx::api ## Human Summary Preserve MusicXML other-notation and other mark SMuFL data through mx::api. Compound dynamics such as ffz retain their ordered dynamic components instead of losing everything after the first child. ## Summary Add typed API payloads for standard and other dynamic components, including ordered compound dynamics, and carry the smufl attribute for other-articulation, other-dynamics, other-ornament, and other-technical marks. Add a symmetric other-notation payload for its text, type, number, smufl, id, position, and print data. Keep single standard dynamics on the existing MarkData representation for source compatibility. Multiple children of one MusicXML dynamics element are owned by one compound mark, avoiding neighbor inference and preserving their order on write. Text and smufl remain independent for other dynamics because MusicXML permits a fallback text value alongside the glyph name. Also correct generic print-object writing so an explicit no value is emitted. Retire anonymous namespaces in the touched implementation files for strict unity-build compatibility. Nine synthetic other-mark fixtures now pass the API round-trip gate and are pinned in roundtrip-baseline.txt. ## Testing - make api-test MX_RUNNING_IN_DOCKER=1 (500 test cases, 5510 assertions) - make api-roundtrip MX_RUNNING_IN_DOCKER=1 (305 pinned files, 0 failed) - make fmt-check MX_RUNNING_IN_DOCKER=1 - strict unity build of target mx - git diff --check --- docs/ai/design/mx-impl-port-plan.md | 9 + src/include/mx/api/ApiEquality.h | 5 +- src/include/mx/api/DynamicsData.h | 110 ++++++++++++ src/include/mx/api/MarkData.h | 5 + src/include/mx/api/MarkDataChoice.h | 66 ++++++- src/private/mx/api/DynamicsData.cpp | 65 +++++++ src/private/mx/api/MarkData.cpp | 20 ++- src/private/mx/api/MarkDataChoice.cpp | 66 +++++++ .../mx/impl/ArticulationsFunctions.cpp | 6 + src/private/mx/impl/Converter.cpp | 56 +++++- src/private/mx/impl/Converter.h | 8 + src/private/mx/impl/DynamicsReader.cpp | 53 +++++- src/private/mx/impl/DynamicsWriter.cpp | 37 +++- src/private/mx/impl/NotationsWriter.cpp | 49 +++++- src/private/mx/impl/NoteFunctions.cpp | 24 ++- src/private/mx/impl/OrnamentsFunctions.cpp | 21 +-- src/private/mx/impl/PrintFunctions.h | 13 +- src/private/mx/impl/TechnicalFunctions.cpp | 28 +-- src/private/mxtest/api/OtherMarksApiTest.cpp | 166 ++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 13 ++ 20 files changed, 755 insertions(+), 65 deletions(-) create mode 100644 src/include/mx/api/DynamicsData.h create mode 100644 src/private/mx/api/DynamicsData.cpp create mode 100644 src/private/mxtest/api/OtherMarksApiTest.cpp diff --git a/docs/ai/design/mx-impl-port-plan.md b/docs/ai/design/mx-impl-port-plan.md index 66c4f4e74..3efe71350 100644 --- a/docs/ai/design/mx-impl-port-plan.md +++ b/docs/ai/design/mx-impl-port-plan.md @@ -392,6 +392,15 @@ Open questions for the Phase-3 design session: both text and attribute), or attribute-only (clean migration, Komp updates in lockstep)? 4. Fate of `customAccentTenuto`/`getMarkTypeFromCustomString` and the `SMUFLKILL` TODOs. +Resolution: + +- Exact glyph names live in mark-specific `MarkDataChoice` payloads, not as another common + `MarkData` field. +- A compound dynamic owns its ordered standard and `other-dynamics` components; neighboring marks + are never interpreted as one dynamic. +- Text and `smufl` may coexist. mx does not promote legacy text to a SMuFL name automatically. +- The `customAccentTenuto` compatibility path remains unchanged and can be retired separately. + ## Appendix A: port checklist ### A.1 `src/private/mx/api/` (4 of 13 .cpp touch core/ezxml) diff --git a/src/include/mx/api/ApiEquality.h b/src/include/mx/api/ApiEquality.h index 68ebc9abe..84beb0b15 100644 --- a/src/include/mx/api/ApiEquality.h +++ b/src/include/mx/api/ApiEquality.h @@ -82,10 +82,7 @@ inline void streamComparisonUnequalMessage(const char *const inClassName, const #define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) \ streamComparisonUnequalMessage(XtheCurrentClassName, XmxapiMemberName); #else -#define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) \ - { \ - MX_API_UNUSED(XtheCurrentClassName) \ - } +#define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) {MX_API_UNUSED(XtheCurrentClassName)} #endif #define MXAPI_EQUALS_BEGIN(mxapiClassName) \ diff --git a/src/include/mx/api/DynamicsData.h b/src/include/mx/api/DynamicsData.h new file mode 100644 index 000000000..db58b83c1 --- /dev/null +++ b/src/include/mx/api/DynamicsData.h @@ -0,0 +1,110 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#pragma once + +#include "mx/api/ApiCommon.h" + +#include +#include +#include +#include + +namespace mx +{ +namespace api +{ + +// A standard dynamic abbreviation represented by a dedicated MusicXML element. +enum class StandardDynamic +{ + p, + pp, + ppp, + pppp, + ppppp, + pppppp, + f, + ff, + fff, + ffff, + fffff, + ffffff, + mp, + mf, + sf, + sfp, + sfpp, + fp, + rf, + rfz, + sfz, + sffz, + fz, + n, + pf, + sfzp +}; + +// A component of a dynamic mark that has no dedicated MusicXML dynamic element. text is the +// visible fallback; smufl, when present, names the exact glyph to draw. +struct OtherDynamicsData +{ + std::string text; + std::optional smufl; +}; + +MXAPI_EQUALS_BEGIN(OtherDynamicsData) +MXAPI_EQUALS_MEMBER(text) +MXAPI_EQUALS_MEMBER(smufl) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(OtherDynamicsData); + +// One ordered component of a compound dynamic mark: either a standard abbreviation or a custom +// component with fallback text and an optional SMuFL glyph name. +class DynamicsComponent +{ + public: + enum class Kind + { + standard, + other + }; + + DynamicsComponent(); + DynamicsComponent(StandardDynamic value); + DynamicsComponent(OtherDynamicsData value); + + Kind kind() const; + bool isStandard() const; + bool isOther() const; + + // Returns the standard dynamic, or p when this holds an other-dynamics component. + StandardDynamic standard() const; + + // Returns the custom component, or a default value when this holds a standard dynamic. + OtherDynamicsData other() const; + + bool operator==(const DynamicsComponent &other) const; + + private: + std::variant myValue; +}; + +MXAPI_NOT_EQUALS_AND_VECTORS(DynamicsComponent); + +// A dynamic mark assembled from multiple symbols in order, such as ff followed by z for ffz. +// MusicXML writes these as children of one element. +struct CompoundDynamicsData +{ + std::vector components; +}; + +MXAPI_EQUALS_BEGIN(CompoundDynamicsData) +MXAPI_EQUALS_MEMBER(components) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(CompoundDynamicsData); + +} // namespace api +} // namespace mx diff --git a/src/include/mx/api/MarkData.h b/src/include/mx/api/MarkData.h index b3a57b6dd..a173595e7 100644 --- a/src/include/mx/api/MarkData.h +++ b/src/include/mx/api/MarkData.h @@ -73,6 +73,7 @@ enum class MarkType pf, sfzp, otherDynamics, + compoundDynamics, ///< A single dynamic mark assembled from ordered components in MarkData::choice unknownDynamics, // ornaments @@ -218,6 +219,9 @@ enum class MarkType // nonArpeggiate nonArpeggiate, + // general notation extension + otherNotation, + // these are cust additions that will be written to, and read from, the // other-articulations (or other-*) elements. customErrorUnknown, // used to represent an error when parsing from a string @@ -236,6 +240,7 @@ bool isMarkDynamic(MarkType); bool isMarkFermata(MarkType); bool isMarkArpeggiate(MarkType); bool isMarkNonArpeggiate(MarkType); +bool isMarkOtherNotation(MarkType); bool isMarkCustom(MarkType); std::string getCustomMarkName(MarkType); diff --git a/src/include/mx/api/MarkDataChoice.h b/src/include/mx/api/MarkDataChoice.h index f801bb526..1da63d237 100644 --- a/src/include/mx/api/MarkDataChoice.h +++ b/src/include/mx/api/MarkDataChoice.h @@ -5,6 +5,7 @@ #pragma once #include "mx/api/ApiCommon.h" +#include "mx/api/DynamicsData.h" #include #include @@ -83,6 +84,44 @@ MXAPI_EQUALS_MEMBER(id) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(NonArpeggiateMarkData); +// The exact glyph used by an other-articulation, other-dynamics, other-ornament, or +// other-technical mark. The mark's visible fallback text remains in MarkData::name. +struct OtherMarkData +{ + std::optional smufl; +}; + +MXAPI_EQUALS_BEGIN(OtherMarkData) +MXAPI_EQUALS_MEMBER(smufl) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(OtherMarkData); + +// Whether an other-notation is a standalone symbol or one end of a multi-note notation. +enum class OtherNotationType +{ + start, + stop, + single +}; + +// Payload for MusicXML's general other-notation extension. The visible fallback text, position, +// and print appearance use MarkData's common fields. +struct OtherNotationMarkData +{ + OtherNotationType type = OtherNotationType::single; + std::optional number; + std::optional smufl; + std::optional id; +}; + +MXAPI_EQUALS_BEGIN(OtherNotationMarkData) +MXAPI_EQUALS_MEMBER(type) +MXAPI_EQUALS_MEMBER(number) +MXAPI_EQUALS_MEMBER(smufl) +MXAPI_EQUALS_MEMBER(id) +MXAPI_EQUALS_END; +MXAPI_NOT_EQUALS_AND_VECTORS(OtherNotationMarkData); + // A variant class that carries data for MarkType values whose payload does not fit MarkData's // common fields. // @@ -105,7 +144,10 @@ class MarkDataChoice none, tremolo, arpeggiate, - nonArpeggiate + nonArpeggiate, + otherMark, + compoundDynamics, + otherNotation }; MarkDataChoice(); @@ -116,11 +158,20 @@ class MarkDataChoice MarkDataChoice(NonArpeggiateMarkData value); + MarkDataChoice(OtherMarkData value); + + MarkDataChoice(CompoundDynamicsData value); + + MarkDataChoice(OtherNotationMarkData value); + Kind kind() const; bool isNone() const; bool isTremolo() const; bool isArpeggiate() const; bool isNonArpeggiate() const; + bool isOtherMark() const; + bool isCompoundDynamics() const; + bool isOtherNotation() const; // Returns a copy of the internally held TremoloMarkData. // @@ -140,10 +191,21 @@ class MarkDataChoice // constructed NonArpeggiateMarkData is returned. const NonArpeggiateMarkData nonArpeggiate() const; + // Returns a copy of the internally held OtherMarkData, or a default value for another kind. + const OtherMarkData otherMark() const; + + // Returns a copy of the internally held CompoundDynamicsData, or a default value for another kind. + const CompoundDynamicsData compoundDynamics() const; + + // Returns a copy of the internally held OtherNotationMarkData, or a default value for another kind. + const OtherNotationMarkData otherNotation() 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/DynamicsData.cpp b/src/private/mx/api/DynamicsData.cpp new file mode 100644 index 000000000..ffb9c79fb --- /dev/null +++ b/src/private/mx/api/DynamicsData.cpp @@ -0,0 +1,65 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#include "mx/api/DynamicsData.h" + +#include + +namespace mx +{ +namespace api +{ + +DynamicsComponent::DynamicsComponent() : myValue{StandardDynamic::p} +{ +} + +DynamicsComponent::DynamicsComponent(StandardDynamic value) : myValue{value} +{ +} + +DynamicsComponent::DynamicsComponent(OtherDynamicsData value) : myValue{std::move(value)} +{ +} + +DynamicsComponent::Kind DynamicsComponent::kind() const +{ + return isOther() ? Kind::other : Kind::standard; +} + +bool DynamicsComponent::isStandard() const +{ + return std::holds_alternative(myValue); +} + +bool DynamicsComponent::isOther() const +{ + return std::holds_alternative(myValue); +} + +StandardDynamic DynamicsComponent::standard() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return StandardDynamic::p; +} + +OtherDynamicsData DynamicsComponent::other() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return OtherDynamicsData{}; +} + +bool DynamicsComponent::operator==(const DynamicsComponent &other) const +{ + return myValue == other.myValue; +} + +} // namespace api +} // namespace mx diff --git a/src/private/mx/api/MarkData.cpp b/src/private/mx/api/MarkData.cpp index ca546eb77..06b33f067 100644 --- a/src/private/mx/api/MarkData.cpp +++ b/src/private/mx/api/MarkData.cpp @@ -11,11 +11,9 @@ namespace mx { namespace api { -namespace -{ // The wire literal of a dynamics alternative (the old core's // toString(DynamicsEnum); the new variant Kind carries no string). -std::string dynamicsKindToString(core::DynamicsChoice::Kind kind) +std::string markDataDynamicsKindToString(core::DynamicsChoice::Kind kind) { switch (kind) { @@ -75,7 +73,6 @@ std::string dynamicsKindToString(core::DynamicsChoice::Kind kind) return "other-dynamics"; } } -} // namespace bool isMarkDynamic(MarkType markType) { @@ -88,7 +85,7 @@ bool isMarkDynamic(MarkType markType) (markType == MarkType::fp) || (markType == MarkType::rf) || (markType == MarkType::rfz) || (markType == MarkType::sfz) || (markType == MarkType::sffz) || (markType == MarkType::fz) || (markType == MarkType::n) || (markType == MarkType::pf) || (markType == MarkType::sfzp) || - (markType == MarkType::otherDynamics); + (markType == MarkType::otherDynamics) || (markType == MarkType::compoundDynamics); } bool isMarkArpeggiate(MarkType markType) @@ -144,6 +141,11 @@ bool isMarkNonArpeggiate(MarkType markType) return (markType == MarkType::nonArpeggiate); } +bool isMarkOtherNotation(MarkType markType) +{ + return markType == MarkType::otherNotation; +} + bool isMarkPedal(MarkType markType) { return (markType == MarkType::pedal) || (markType == MarkType::damp); @@ -244,9 +246,9 @@ MarkData::MarkData(MarkType inMarkType) fingeringAlternate{Bool::unspecified}, choice{} { impl::Converter converter; - if (isMarkDynamic(markType)) + if (isMarkDynamic(markType) && markType != MarkType::compoundDynamics) { - name = dynamicsKindToString(converter.convertDynamic(markType)); + name = markDataDynamicsKindToString(converter.convertDynamic(markType)); } else if (isMarkArticulation(markType)) { @@ -266,9 +268,9 @@ MarkData::MarkData(Placement inPlacement, MarkType inMarkType) { positionData.placement = inPlacement; impl::Converter converter; - if (isMarkDynamic(markType)) + if (isMarkDynamic(markType) && markType != MarkType::compoundDynamics) { - name = dynamicsKindToString(converter.convertDynamic(markType)); + name = markDataDynamicsKindToString(converter.convertDynamic(markType)); } else if (isMarkArticulation(markType)) { diff --git a/src/private/mx/api/MarkDataChoice.cpp b/src/private/mx/api/MarkDataChoice.cpp index ad8e3d7b8..fdda8bd1e 100644 --- a/src/private/mx/api/MarkDataChoice.cpp +++ b/src/private/mx/api/MarkDataChoice.cpp @@ -27,6 +27,18 @@ MarkDataChoice::MarkDataChoice(NonArpeggiateMarkData value) : myValue{std::move( { } +MarkDataChoice::MarkDataChoice(OtherMarkData value) : myValue{std::move(value)} +{ +} + +MarkDataChoice::MarkDataChoice(CompoundDynamicsData value) : myValue{std::move(value)} +{ +} + +MarkDataChoice::MarkDataChoice(OtherNotationMarkData value) : myValue{std::move(value)} +{ +} + MarkDataChoice::Kind MarkDataChoice::kind() const { if (std::holds_alternative(myValue)) @@ -41,6 +53,18 @@ MarkDataChoice::Kind MarkDataChoice::kind() const { return Kind::nonArpeggiate; } + if (std::holds_alternative(myValue)) + { + return Kind::otherMark; + } + if (std::holds_alternative(myValue)) + { + return Kind::compoundDynamics; + } + if (std::holds_alternative(myValue)) + { + return Kind::otherNotation; + } return Kind::none; } @@ -91,6 +115,48 @@ const NonArpeggiateMarkData MarkDataChoice::nonArpeggiate() const return NonArpeggiateMarkData{}; } +bool MarkDataChoice::isOtherMark() const +{ + return std::holds_alternative(myValue); +} + +const OtherMarkData MarkDataChoice::otherMark() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return OtherMarkData{}; +} + +bool MarkDataChoice::isCompoundDynamics() const +{ + return std::holds_alternative(myValue); +} + +const CompoundDynamicsData MarkDataChoice::compoundDynamics() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return CompoundDynamicsData{}; +} + +bool MarkDataChoice::isOtherNotation() const +{ + return std::holds_alternative(myValue); +} + +const OtherNotationMarkData MarkDataChoice::otherNotation() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return OtherNotationMarkData{}; +} + bool MarkDataChoice::operator==(const MarkDataChoice &other) const { return myValue == other.myValue; diff --git a/src/private/mx/impl/ArticulationsFunctions.cpp b/src/private/mx/impl/ArticulationsFunctions.cpp index 2e5cdc1ca..7185c3d3d 100644 --- a/src/private/mx/impl/ArticulationsFunctions.cpp +++ b/src/private/mx/impl/ArticulationsFunctions.cpp @@ -151,6 +151,12 @@ void ArticulationsFunctions::parseArticulation(const core::ArticulationsChoice & const auto &oa = inArticulation.asOtherArticulation(); parseMarkDataAttributes(oa, outMark); outMark.name = oa.value(); + api::OtherMarkData payload; + if (oa.smufl().has_value()) + { + payload.smufl = oa.smufl()->toString(); + } + outMark.choice = std::move(payload); const auto possibleCustomMarkType = mx::api::getMarkTypeFromCustomString(outMark.name); if (possibleCustomMarkType != mx::api::MarkType::customErrorUnknown) diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index cdd118fdd..212994197 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -157,7 +157,6 @@ const Converter::EnumMap Converter::cssMap = { {core::CSSFontSize::xxLarge(), api::CssSize::xxLarge}, }; -// TODO - SMUFLKILL const Converter::EnumMap Converter::articulationsMap = { {core::ArticulationsChoice::Kind::accent, api::MarkType::accent}, {core::ArticulationsChoice::Kind::strongAccent, api::MarkType::strongAccent}, @@ -215,6 +214,41 @@ const Converter::EnumMap Converter::d {core::DynamicsChoice::Kind::otherDynamics, api::MarkType::otherDynamics}, }; +const Converter::EnumMap Converter::standardDynamicsMap = { + {core::DynamicsChoice::Kind::p, api::StandardDynamic::p}, + {core::DynamicsChoice::Kind::pp, api::StandardDynamic::pp}, + {core::DynamicsChoice::Kind::ppp, api::StandardDynamic::ppp}, + {core::DynamicsChoice::Kind::pppp, api::StandardDynamic::pppp}, + {core::DynamicsChoice::Kind::ppppp, api::StandardDynamic::ppppp}, + {core::DynamicsChoice::Kind::pppppp, api::StandardDynamic::pppppp}, + {core::DynamicsChoice::Kind::f, api::StandardDynamic::f}, + {core::DynamicsChoice::Kind::ff, api::StandardDynamic::ff}, + {core::DynamicsChoice::Kind::fff, api::StandardDynamic::fff}, + {core::DynamicsChoice::Kind::ffff, api::StandardDynamic::ffff}, + {core::DynamicsChoice::Kind::fffff, api::StandardDynamic::fffff}, + {core::DynamicsChoice::Kind::ffffff, api::StandardDynamic::ffffff}, + {core::DynamicsChoice::Kind::mp, api::StandardDynamic::mp}, + {core::DynamicsChoice::Kind::mf, api::StandardDynamic::mf}, + {core::DynamicsChoice::Kind::sf, api::StandardDynamic::sf}, + {core::DynamicsChoice::Kind::sfp, api::StandardDynamic::sfp}, + {core::DynamicsChoice::Kind::sfpp, api::StandardDynamic::sfpp}, + {core::DynamicsChoice::Kind::fp, api::StandardDynamic::fp}, + {core::DynamicsChoice::Kind::rf, api::StandardDynamic::rf}, + {core::DynamicsChoice::Kind::rfz, api::StandardDynamic::rfz}, + {core::DynamicsChoice::Kind::sfz, api::StandardDynamic::sfz}, + {core::DynamicsChoice::Kind::sffz, api::StandardDynamic::sffz}, + {core::DynamicsChoice::Kind::fz, api::StandardDynamic::fz}, + {core::DynamicsChoice::Kind::n, api::StandardDynamic::n}, + {core::DynamicsChoice::Kind::pf, api::StandardDynamic::pf}, + {core::DynamicsChoice::Kind::sfzp, api::StandardDynamic::sfzp}, +}; + +const Converter::EnumMap Converter::otherNotationTypeMap = { + {core::StartStopSingle::start(), api::OtherNotationType::start}, + {core::StartStopSingle::stop(), api::OtherNotationType::stop}, + {core::StartStopSingle::single(), api::OtherNotationType::single}, +}; + const Converter::EnumMap Converter::ornamentsMap = { {core::OrnamentsGroupChoice::Kind::trillMark, api::MarkType::trillMark}, {core::OrnamentsGroupChoice::Kind::turn, api::MarkType::turn}, @@ -1759,6 +1793,26 @@ api::MarkType Converter::convertDynamic(core::DynamicsChoice::Kind value) const return findApiItem(dynamicsMap, api::MarkType::unspecified, value); } +core::DynamicsChoice::Kind Converter::convert(api::StandardDynamic value) const +{ + return findCoreItem(standardDynamicsMap, core::DynamicsChoice::Kind::p, value); +} + +api::StandardDynamic Converter::convertStandardDynamic(core::DynamicsChoice::Kind value) const +{ + return findApiItem(standardDynamicsMap, api::StandardDynamic::p, value); +} + +core::StartStopSingle Converter::convert(api::OtherNotationType value) const +{ + return findCoreItem(otherNotationTypeMap, core::StartStopSingle::single(), value); +} + +api::OtherNotationType Converter::convert(core::StartStopSingle value) const +{ + return findApiItem(otherNotationTypeMap, api::OtherNotationType::single, value); +} + core::OrnamentsGroupChoice::Kind Converter::convertOrnament(api::MarkType value) const { // All tremolo variants map to Kind::tremolo; the specific slash count is encoded diff --git a/src/private/mx/impl/Converter.h b/src/private/mx/impl/Converter.h index d62f4b44f..331f65c62 100644 --- a/src/private/mx/impl/Converter.h +++ b/src/private/mx/impl/Converter.h @@ -47,6 +47,7 @@ #include "mx/core/generated/RightLeftMiddle.h" #include "mx/core/generated/SoundID.h" #include "mx/core/generated/StartStopDiscontinue.h" +#include "mx/core/generated/StartStopSingle.h" #include "mx/core/generated/StemValue.h" #include "mx/core/generated/Step.h" #include "mx/core/generated/StickLocation.h" @@ -128,6 +129,11 @@ class Converter core::DynamicsChoice::Kind convertDynamic(api::MarkType value) const; api::MarkType convertDynamic(core::DynamicsChoice::Kind value) const; + core::DynamicsChoice::Kind convert(api::StandardDynamic value) const; + api::StandardDynamic convertStandardDynamic(core::DynamicsChoice::Kind value) const; + + core::StartStopSingle convert(api::OtherNotationType value) const; + api::OtherNotationType convert(core::StartStopSingle value) const; core::OrnamentsGroupChoice::Kind convertOrnament(api::MarkType value) const; api::MarkType convertOrnament(core::OrnamentsGroupChoice::Kind value) const; @@ -259,6 +265,8 @@ class Converter const static EnumMap fontWeightMap; const static EnumMap articulationsMap; const static EnumMap dynamicsMap; + const static EnumMap standardDynamicsMap; + const static EnumMap otherNotationTypeMap; const static EnumMap ornamentsMap; const static EnumMap accidentalMarkMap; const static EnumMap technicalMarkMap; diff --git a/src/private/mx/impl/DynamicsReader.cpp b/src/private/mx/impl/DynamicsReader.cpp index 5d7bfe63d..8064bfd20 100644 --- a/src/private/mx/impl/DynamicsReader.cpp +++ b/src/private/mx/impl/DynamicsReader.cpp @@ -26,25 +26,60 @@ void DynamicsReader::parseDynamics(std::vector &outMarks) const return; } - const auto &firstChoice = choices.front(); - const auto kind = firstChoice.kind(); Converter converter; - const auto markType = converter.convertDynamic(kind); - auto markData = api::MarkData{}; - markData.markType = markType; markData.tickTimePosition = myCursor.tickTimePosition; + markData.positionData = impl::getPositionData(myDynamic); + markData.printData = impl::getPrintData(myDynamic); - if (kind == core::DynamicsChoice::Kind::otherDynamics) + if (choices.size() == 1) { - markData.name = firstChoice.asOtherDynamics().value(); + const auto &choice = choices.front(); + const auto kind = choice.kind(); + markData.markType = converter.convertDynamic(kind); + + if (kind == core::DynamicsChoice::Kind::otherDynamics) + { + const auto &other = choice.asOtherDynamics(); + markData.name = other.value(); + api::OtherMarkData payload; + if (other.smufl().has_value()) + { + payload.smufl = other.smufl()->toString(); + } + markData.choice = std::move(payload); + } + else + { + markData.name = dynamicsKindToName(kind); + } } else { - markData.name = dynamicsKindToName(kind); + markData.markType = api::MarkType::compoundDynamics; + api::CompoundDynamicsData compound; + compound.components.reserve(choices.size()); + for (const auto &choice : choices) + { + if (choice.kind() == core::DynamicsChoice::Kind::otherDynamics) + { + const auto &other = choice.asOtherDynamics(); + api::OtherDynamicsData component; + component.text = other.value(); + if (other.smufl().has_value()) + { + component.smufl = other.smufl()->toString(); + } + compound.components.emplace_back(std::move(component)); + } + else + { + compound.components.emplace_back(converter.convertStandardDynamic(choice.kind())); + } + } + markData.choice = std::move(compound); } - markData.positionData = impl::getPositionData(myDynamic); outMarks.emplace_back(std::move(markData)); } } // namespace impl diff --git a/src/private/mx/impl/DynamicsWriter.cpp b/src/private/mx/impl/DynamicsWriter.cpp index 352541d8d..0fb3814aa 100644 --- a/src/private/mx/impl/DynamicsWriter.cpp +++ b/src/private/mx/impl/DynamicsWriter.cpp @@ -7,6 +7,7 @@ #include "mx/core/generated/DynamicsChoice.h" #include "mx/core/generated/Empty.h" #include "mx/core/generated/OtherText.h" +#include "mx/core/generated/SmuflGlyphName.h" #include "mx/impl/MarkDataFunctions.h" #include "mx/utility/Throw.h" @@ -15,7 +16,8 @@ namespace mx namespace impl { -static core::DynamicsChoice makeDynamicsChoice(core::DynamicsChoice::Kind kind, const std::string &otherName) +core::DynamicsChoice dynamicsWriterMakeChoice(core::DynamicsChoice::Kind kind, const std::string &otherName, + const std::optional &smufl) { using K = core::DynamicsChoice::Kind; core::Empty empty{}; @@ -76,6 +78,10 @@ static core::DynamicsChoice makeDynamicsChoice(core::DynamicsChoice::Kind kind, case K::otherDynamics: { core::OtherText ot; ot.setValue(otherName); + if (smufl.has_value()) + { + ot.setSmufl(core::SmuflGlyphName{*smufl}); + } return core::DynamicsChoice::otherDynamics(ot); } default: @@ -98,12 +104,31 @@ DynamicsWriter::DynamicsWriter(const api::MarkData &inMark, impl::Cursor inCurso core::Dynamics DynamicsWriter::getDynamics() const { - const auto kind = myConverter.convertDynamic(myMarkData.markType); - const bool isOther = kind == core::DynamicsChoice::Kind::otherDynamics; - const auto &otherName = isOther ? myMarkData.name : std::string{}; - core::Dynamics dyn; - dyn.addChoice(makeDynamicsChoice(kind, otherName)); + if (myMarkData.markType == api::MarkType::compoundDynamics) + { + for (const auto &component : myMarkData.choice.compoundDynamics().components) + { + if (component.isOther()) + { + const auto other = component.other(); + dyn.addChoice( + dynamicsWriterMakeChoice(core::DynamicsChoice::Kind::otherDynamics, other.text, other.smufl)); + } + else + { + dyn.addChoice(dynamicsWriterMakeChoice(myConverter.convert(component.standard()), {}, {})); + } + } + } + else + { + const auto kind = myConverter.convertDynamic(myMarkData.markType); + const bool isOther = kind == core::DynamicsChoice::Kind::otherDynamics; + const auto &otherName = isOther ? myMarkData.name : std::string{}; + const auto smufl = isOther ? myMarkData.choice.otherMark().smufl : std::optional{}; + dyn.addChoice(dynamicsWriterMakeChoice(kind, otherName, smufl)); + } impl::setAttributesFromMarkData(myMarkData, dyn); return dyn; } diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index 49f1613ca..685764703 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/src/private/mx/impl/NotationsWriter.cpp @@ -39,6 +39,7 @@ #include "mx/core/generated/PlacementText.h" #include "mx/core/generated/ShowTuplet.h" #include "mx/core/generated/Slur.h" +#include "mx/core/generated/SmuflGlyphName.h" #include "mx/core/generated/String.h" #include "mx/core/generated/StringNumber.h" #include "mx/core/generated/StrongAccent.h" @@ -69,9 +70,7 @@ namespace mx { namespace impl { -namespace -{ -void setMordentSpecificAttributes(const api::MarkData &mark, core::Mordent &mordent) +void notationsWriterSetMordentSpecificAttributes(const api::MarkData &mark, core::Mordent &mordent) { Converter converter; @@ -90,7 +89,6 @@ void setMordentSpecificAttributes(const api::MarkData &mark, core::Mordent &mord mordent.setDeparture(converter.convert(mark.mordentDeparture)); } } -} // namespace NotationsWriter::NotationsWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter) @@ -414,6 +412,29 @@ core::Notations NotationsWriter::getNotations() const outNotations.addChoice(core::NotationsChoice::arpeggiate(arpeggiate)); } + else if (isMarkOtherNotation(mark.markType)) + { + core::OtherNotation other; + impl::setAttributesFromMarkData(mark, other); + other.setValue(mark.name); + + const auto payload = mark.choice.otherNotation(); + other.setType(myConverter.convert(payload.type)); + if (payload.number.has_value()) + { + other.setNumber(core::NumberLevel{*payload.number}); + } + if (payload.smufl.has_value()) + { + other.setSmufl(core::SmuflGlyphName{*payload.smufl}); + } + if (payload.id.has_value()) + { + other.setID(core::Token{*payload.id}); + } + + outNotations.addChoice(core::NotationsChoice::otherNotation(other)); + } } if (!articulations.choice().empty()) @@ -591,6 +612,7 @@ void NotationsWriter::addArticulation(const api::MarkData &mark, core::Articulat case core::ArticulationsChoice::Kind::otherArticulation: { core::OtherPlacementText opt; setAttributesFromPositionData(mark.positionData, opt); + setAttributesFromPrintData(mark.printData, opt); if (api::isMarkCustom(mark.markType)) { opt.setValue(api::getCustomMarkName(mark.markType)); @@ -599,6 +621,10 @@ void NotationsWriter::addArticulation(const api::MarkData &mark, core::Articulat { opt.setValue(mark.name); } + if (mark.choice.otherMark().smufl.has_value()) + { + opt.setSmufl(core::SmuflGlyphName{*mark.choice.otherMark().smufl}); + } outArticulations.addChoice(core::ArticulationsChoice::otherArticulation(opt)); break; } @@ -676,14 +702,14 @@ void NotationsWriter::addOrnament(const api::MarkData &mark, core::Ornaments &ou case core::OrnamentsGroupChoice::Kind::mordent: { core::Mordent m; setAttributesFromPositionData(mark.positionData, m); - setMordentSpecificAttributes(mark, m); + notationsWriterSetMordentSpecificAttributes(mark, m); group.setChoice(core::OrnamentsGroupChoice::mordent(m)); break; } case core::OrnamentsGroupChoice::Kind::invertedMordent: { core::Mordent m; setAttributesFromPositionData(mark.positionData, m); - setMordentSpecificAttributes(mark, m); + notationsWriterSetMordentSpecificAttributes(mark, m); group.setChoice(core::OrnamentsGroupChoice::invertedMordent(m)); break; } @@ -719,11 +745,15 @@ void NotationsWriter::addOrnament(const api::MarkData &mark, core::Ornaments &ou case core::OrnamentsGroupChoice::Kind::otherOrnament: { core::OtherPlacementText opt; setAttributesFromPositionData(mark.positionData, opt); + setAttributesFromPrintData(mark.printData, opt); if (!mark.name.empty()) { opt.setValue(mark.name); } - // TODO - SMUFLKILL - handle custom enum values? + if (mark.choice.otherMark().smufl.has_value()) + { + opt.setSmufl(core::SmuflGlyphName{*mark.choice.otherMark().smufl}); + } group.setChoice(core::OrnamentsGroupChoice::otherOrnament(opt)); break; } @@ -972,10 +1002,15 @@ void NotationsWriter::addTechnical(const api::MarkData &mark, core::Technical &o case core::TechnicalChoice::Kind::otherTechnical: { core::OtherPlacementText opt; setAttributesFromPositionData(mark.positionData, opt); + setAttributesFromPrintData(mark.printData, opt); if (!mark.name.empty()) { opt.setValue(mark.name); } + if (mark.choice.otherMark().smufl.has_value()) + { + opt.setSmufl(core::SmuflGlyphName{*mark.choice.otherMark().smufl}); + } outTechnical.addChoice(core::TechnicalChoice::otherTechnical(opt)); break; } diff --git a/src/private/mx/impl/NoteFunctions.cpp b/src/private/mx/impl/NoteFunctions.cpp index 1a85f87ba..8ca73c289 100644 --- a/src/private/mx/impl/NoteFunctions.cpp +++ b/src/private/mx/impl/NoteFunctions.cpp @@ -297,7 +297,29 @@ void NoteFunctions::parseNotations() const break; } case core::NotationsChoice::Kind::otherNotation: { - // TODO - import otherNotation + const auto &other = notationsChoice.asOtherNotation(); + api::MarkData mark{api::MarkType::otherNotation}; + mark.tickTimePosition = myCursor.tickTimePosition; + mark.name = other.value(); + parseMarkDataAttributes(other, mark); + + api::OtherNotationMarkData payload; + Converter converter; + payload.type = converter.convert(other.type()); + if (other.number().has_value()) + { + payload.number = other.number()->value(); + } + if (other.smufl().has_value()) + { + payload.smufl = other.smufl()->toString(); + } + if (other.id().has_value()) + { + payload.id = other.id()->value(); + } + mark.choice = std::move(payload); + myOutNoteData.noteAttachmentData.marks.emplace_back(std::move(mark)); break; } default: diff --git a/src/private/mx/impl/OrnamentsFunctions.cpp b/src/private/mx/impl/OrnamentsFunctions.cpp index e5176ae42..b7dce9382 100644 --- a/src/private/mx/impl/OrnamentsFunctions.cpp +++ b/src/private/mx/impl/OrnamentsFunctions.cpp @@ -16,9 +16,7 @@ namespace mx { namespace impl { -namespace -{ -void parseMordentSpecificAttributes(const core::Mordent &m, api::MarkData &outMark) +void ornamentsFunctionsParseMordentSpecificAttributes(const core::Mordent &m, api::MarkData &outMark) { Converter converter; @@ -40,7 +38,6 @@ void parseMordentSpecificAttributes(const core::Mordent &m, api::MarkData &outMa outMark.mordentDeparture = converter.convert(*m.departure()); } } -} // namespace OrnamentsFunctions::OrnamentsFunctions(const core::Ornaments &inOrnaments, impl::Cursor inCursor) : myOrnaments{inOrnaments}, myCursor{inCursor} @@ -65,12 +62,6 @@ void OrnamentsFunctions::parseOrnamentsSet(std::vector &outMarks) parseOrnament(choiceObj, markData); markData.tickTimePosition = myCursor.tickTimePosition; - if ((markData.markType == api::MarkType::otherOrnament) || - (markData.markType == api::MarkType::unknownOrnament)) - { - // TODO - SMUFLKILL - use the name to see if we have a custom enum value - } - if (markData.markType != api::MarkType::unknownOrnament) { outMarks.emplace_back(std::move(markData)); @@ -143,14 +134,14 @@ void OrnamentsFunctions::parseOrnament(const core::OrnamentsGroupChoice &choiceO outMark.name = "mordent"; const auto &m = choiceObj.asMordent(); parseMarkDataAttributes(m, outMark); - parseMordentSpecificAttributes(m, outMark); + ornamentsFunctionsParseMordentSpecificAttributes(m, outMark); break; } case core::OrnamentsGroupChoice::Kind::invertedMordent: { outMark.name = "inverted-mordent"; const auto &m = choiceObj.asInvertedMordent(); parseMarkDataAttributes(m, outMark); - parseMordentSpecificAttributes(m, outMark); + ornamentsFunctionsParseMordentSpecificAttributes(m, outMark); break; } case core::OrnamentsGroupChoice::Kind::schleifer: { @@ -220,6 +211,12 @@ void OrnamentsFunctions::parseOrnament(const core::OrnamentsGroupChoice &choiceO const auto &oa = choiceObj.asOtherOrnament(); parseMarkDataAttributes(oa, outMark); const auto &value = oa.value(); + api::OtherMarkData payload; + if (oa.smufl().has_value()) + { + payload.smufl = oa.smufl()->toString(); + } + outMark.choice = std::move(payload); if (value.empty()) { diff --git a/src/private/mx/impl/PrintFunctions.h b/src/private/mx/impl/PrintFunctions.h index 63f0b0333..544e6053a 100644 --- a/src/private/mx/impl/PrintFunctions.h +++ b/src/private/mx/impl/PrintFunctions.h @@ -91,9 +91,16 @@ MX_OPTIONAL_SET_VALUE_FUNC(color, setColor, Color); template void setPrintObject(const api::Bool &inPrintObject, ATTRIBUTES_TYPE &outAttributes) { - if (!lookForAndSetHasPrintObject(inPrintObject != api::Bool::unspecified, &outAttributes)) + if (inPrintObject == api::Bool::unspecified) { - lookForAndSetPrintObject(inPrintObject, &outAttributes); + lookForAndSetHasPrintObject(false, &outAttributes); + return; + } + + if (lookForAndSetHasPrintObject(true, &outAttributes)) + { + Converter converter; + lookForAndSetPrintObject(converter.convert(inPrintObject), &outAttributes); } } @@ -109,6 +116,8 @@ void setAttributesFromColorData(const api::ColorData &inColorData, ATTRIBUTES_TY template void setAttributesFromPrintData(const api::PrintData &inPrintData, ATTRIBUTES_TYPE &outAttributes) { + setPrintObject(inPrintData.printObject, outAttributes); + if (inPrintData.isColorSpecified) { lookForAndSetHasColor(true, &outAttributes); diff --git a/src/private/mx/impl/TechnicalFunctions.cpp b/src/private/mx/impl/TechnicalFunctions.cpp index e822ccb52..fa6abfc8d 100644 --- a/src/private/mx/impl/TechnicalFunctions.cpp +++ b/src/private/mx/impl/TechnicalFunctions.cpp @@ -19,9 +19,12 @@ #include "mx/impl/Converter.h" #include "mx/impl/MarkDataFunctions.h" -namespace +namespace mx { -std::string holeToSmuflName(const mx::core::Hole &hole) +namespace impl +{ + +std::string technicalFunctionsHoleToSmuflName(const mx::core::Hole &hole) { const auto closedValue = hole.holeClosed().value(); switch (closedValue.tag()) @@ -36,7 +39,7 @@ std::string holeToSmuflName(const mx::core::Hole &hole) } } -std::string arrowToSmuflName(const mx::core::Arrow &arrow) +std::string technicalFunctionsArrowToSmuflName(const mx::core::Arrow &arrow) { using Tag = mx::core::ArrowDirection::Tag; if (arrow.choice().kind() != mx::core::ArrowChoice::Kind::group) @@ -68,7 +71,7 @@ std::string arrowToSmuflName(const mx::core::Arrow &arrow) } } -std::string handbellToSmuflName(const mx::core::HandbellValue &value) +std::string technicalFunctionsHandbellToSmuflName(const mx::core::HandbellValue &value) { using Tag = mx::core::HandbellValue::Tag; switch (value.tag()) @@ -99,12 +102,7 @@ std::string handbellToSmuflName(const mx::core::HandbellValue &value) return "handbellsGyro"; } } -} // namespace -namespace mx -{ -namespace impl -{ TechnicalFunctions::TechnicalFunctions(std::span inTechincalChoiceSet, Cursor inCursor) : myTechincalChoiceSet{inTechincalChoiceSet}, myCursor{inCursor} { @@ -243,19 +241,19 @@ bool TechnicalFunctions::parseTechicalMark(const core::TechnicalChoice &techical case core::TechnicalChoice::Kind::hole: { const auto &hole = techicalChoice.asHole(); parseMarkDataAttributes(hole, outMarkData); - outMarkData.name = holeToSmuflName(hole); + outMarkData.name = technicalFunctionsHoleToSmuflName(hole); return true; } case core::TechnicalChoice::Kind::arrow: { const auto &arrow = techicalChoice.asArrow(); parseMarkDataAttributes(arrow, outMarkData); - outMarkData.name = arrowToSmuflName(arrow); + outMarkData.name = technicalFunctionsArrowToSmuflName(arrow); return true; } case core::TechnicalChoice::Kind::handbell: { const auto &handbell = techicalChoice.asHandbell(); parseMarkDataAttributes(handbell, outMarkData); - outMarkData.name = handbellToSmuflName(handbell.value()); + outMarkData.name = technicalFunctionsHandbellToSmuflName(handbell.value()); return true; } case core::TechnicalChoice::Kind::brassBend: { @@ -297,6 +295,12 @@ bool TechnicalFunctions::parseTechicalMark(const core::TechnicalChoice &techical const auto &oa = techicalChoice.asOtherTechnical(); parseMarkDataAttributes(oa, outMarkData); outMarkData.name = oa.value(); + api::OtherMarkData payload; + if (oa.smufl().has_value()) + { + payload.smufl = oa.smufl()->toString(); + } + outMarkData.choice = std::move(payload); return true; } default: diff --git a/src/private/mxtest/api/OtherMarksApiTest.cpp b/src/private/mxtest/api/OtherMarksApiTest.cpp new file mode 100644 index 000000000..3bd6183e5 --- /dev/null +++ b/src/private/mxtest/api/OtherMarksApiTest.cpp @@ -0,0 +1,166 @@ +// 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/MarkData.h" +#include "mx/api/ScoreData.h" +#include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" + +#include +#include + +using namespace mx::api; + +ScoreData otherMarksScoreWithNote() +{ + ScoreData score; + score.parts.emplace_back(); + score.parts.back().measures.emplace_back(); + score.parts.back().measures.back().staves.emplace_back(); + score.parts.back().measures.back().staves.back().voices[0].notes.emplace_back(); + return score; +} + +NoteData &otherMarksNote(ScoreData &score) +{ + return score.parts.back().measures.back().staves.back().voices[0].notes.back(); +} + +TEST(smuflOtherMarksRoundTrip, OtherMarksApi) +{ + auto score = otherMarksScoreWithNote(); + auto &marks = otherMarksNote(score).noteAttachmentData.marks; + + auto addOtherMark = [&](MarkType type, std::string text, std::string smufl) { + marks.emplace_back(type); + marks.back().name = std::move(text); + marks.back().choice = OtherMarkData{std::move(smufl)}; + }; + + addOtherMark(MarkType::otherArticulation, "articulation fallback", "articAccentAbove"); + addOtherMark(MarkType::otherTechnical, "technique fallback", "brassMuteClosed"); + addOtherMark(MarkType::otherOrnament, "ornament fallback", "ornamentTurnSlash"); + addOtherMark(MarkType::otherDynamics, "", "dynamicZ"); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("smufl=\"articAccentAbove\"") != std::string::npos); + CHECK(xml.find("smufl=\"brassMuteClosed\"") != std::string::npos); + CHECK(xml.find("smufl=\"ornamentTurnSlash\"") != std::string::npos); + CHECK(xml.find("smufl=\"dynamicZ\"") != std::string::npos); + + const auto out = mxtest::roundTrip(score); + const auto &outMarks = + out.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; + REQUIRE(outMarks.size() == 4); + auto smuflFor = [&](MarkType type) { + const auto it = + std::find_if(outMarks.begin(), outMarks.end(), [type](const auto &item) { return item.markType == type; }); + return it == outMarks.end() ? std::optional{} : it->choice.otherMark().smufl; + }; + CHECK(smuflFor(MarkType::otherArticulation) == std::optional{"articAccentAbove"}); + CHECK(smuflFor(MarkType::otherOrnament) == std::optional{"ornamentTurnSlash"}); + CHECK(smuflFor(MarkType::otherTechnical) == std::optional{"brassMuteClosed"}); + CHECK(smuflFor(MarkType::otherDynamics) == std::optional{"dynamicZ"}); + const auto dynamic = std::find_if(outMarks.begin(), outMarks.end(), + [](const auto &item) { return item.markType == MarkType::otherDynamics; }); + REQUIRE(dynamic != outMarks.end()); + CHECK(dynamic->name.empty()); +} + +T_END; + +TEST(markChoiceWrongKindFallbacks, OtherMarksApi) +{ + const MarkDataChoice choice; + CHECK(!choice.otherMark().smufl.has_value()); + CHECK(choice.compoundDynamics().components.empty()); + CHECK(choice.otherNotation().type == OtherNotationType::single); + + const DynamicsComponent standard{StandardDynamic::ff}; + CHECK(standard.other() == OtherDynamicsData{}); + const DynamicsComponent other{OtherDynamicsData{"z", std::string{"dynamicZ"}}}; + CHECK(other.standard() == StandardDynamic::p); +} + +T_END; + +TEST(compoundDynamicsRoundTrip, OtherMarksApi) +{ + auto score = otherMarksScoreWithNote(); + auto &mark = otherMarksNote(score).noteAttachmentData.marks.emplace_back(MarkType::compoundDynamics); + + CompoundDynamicsData compound; + compound.components.emplace_back(StandardDynamic::ff); + compound.components.emplace_back(OtherDynamicsData{"z", std::string{"dynamicZ"}}); + mark.choice = std::move(compound); + + const auto xml = mxtest::toXml(score); + const auto dynamicsPosition = xml.find("z", ffPosition); + REQUIRE(dynamicsPosition != std::string::npos); + REQUIRE(ffPosition != std::string::npos); + REQUIRE(zPosition != std::string::npos); + CHECK(ffPosition < zPosition); + + const auto out = mxtest::roundTrip(score); + const auto &outMarks = + out.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; + REQUIRE(outMarks.size() == 1); + CHECK(outMarks.front().markType == MarkType::compoundDynamics); + const auto outCompound = outMarks.front().choice.compoundDynamics(); + REQUIRE(outCompound.components.size() == 2); + CHECK(outCompound.components.at(0).standard() == StandardDynamic::ff); + CHECK(outCompound.components.at(1).other().text == "z"); + CHECK(outCompound.components.at(1).other().smufl == std::optional{"dynamicZ"}); +} + +T_END; + +TEST(otherNotationRoundTrip, OtherMarksApi) +{ + auto score = otherMarksScoreWithNote(); + auto &mark = otherMarksNote(score).noteAttachmentData.marks.emplace_back(MarkType::otherNotation); + mark.name = "custom notation"; + mark.positionData.placement = Placement::above; + mark.printData.printObject = Bool::no; + + OtherNotationMarkData notation; + notation.type = OtherNotationType::start; + notation.number = 2; + notation.smufl = "pluckedSnapPizzicatoAbove"; + notation.id = "notation-id"; + mark.choice = std::move(notation); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("{2}); + CHECK(outNotation.smufl == std::optional{"pluckedSnapPizzicatoAbove"}); + CHECK(outNotation.id == std::optional{"notation-id"}); +} + +T_END; + +#endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index d113e78d2..ca60855cc 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -409,6 +409,19 @@ synthetic/arpeggiate.4.0.xml synthetic/non-arpeggiate.3.0.xml synthetic/non-arpeggiate.3.1.xml +# Unblocked by exposing SMuFL payloads for other-* marks and the complete +# other-notation payload. The 3.0 fixtures exercise text and print attributes; +# the 3.1 fixtures additionally exercise the smufl attribute. +synthetic/other-articulation.3.0.xml +synthetic/other-articulation.3.1.xml +synthetic/other-dynamics.3.1.xml +synthetic/other-notation.3.0.xml +synthetic/other-notation.3.1.xml +synthetic/other-ornament.3.0.xml +synthetic/other-ornament.3.1.xml +synthetic/other-technical.3.0.xml +synthetic/other-technical.3.1.xml + # Unblocked by the caesura value fix: MarkType::caesura now round-trips the # common empty element form, and the caesuraNormal/Thick/Short/Curved/Single # variants carry an explicit text value. From 3a99c923c81727024ef604610df2c9e935e7c910 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Sun, 2 Aug 2026 14:23:16 -0500 Subject: [PATCH 2/2] chore: remove unrelated ApiEquality formatting change --- src/include/mx/api/ApiEquality.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/include/mx/api/ApiEquality.h b/src/include/mx/api/ApiEquality.h index 84beb0b15..68ebc9abe 100644 --- a/src/include/mx/api/ApiEquality.h +++ b/src/include/mx/api/ApiEquality.h @@ -82,7 +82,10 @@ inline void streamComparisonUnequalMessage(const char *const inClassName, const #define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) \ streamComparisonUnequalMessage(XtheCurrentClassName, XmxapiMemberName); #else -#define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) {MX_API_UNUSED(XtheCurrentClassName)} +#define MX_SHOW_UNEQUAL(XtheCurrentClassName, XmxapiMemberName) \ + { \ + MX_API_UNUSED(XtheCurrentClassName) \ + } #endif #define MXAPI_EQUALS_BEGIN(mxapiClassName) \