From 884466c370689a43d16b5740207922db2ae85d56 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Sun, 2 Aug 2026 07:07:23 -0500 Subject: [PATCH 1/3] 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 764a30748..92dd8aed9 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}, @@ -1765,6 +1799,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 d018af6f6..1bdf67822 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" @@ -130,6 +131,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; @@ -267,6 +273,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 685c286e9..755e8c689 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 30aaa6fda1178a154769a529328243e5486a7bbd Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Sun, 2 Aug 2026 14:23:16 -0500 Subject: [PATCH 2/3] 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) \ From 52cec9b8f2001f8fbf40572b4552bb6aab234f7c Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Tue, 4 Aug 2026 17:16:46 -0500 Subject: [PATCH 3/3] refactor: one dynamics vocabulary in mx::api ## Human Summary There were two enums for the same 26 dynamic markings: the dynamics block of `MarkType`, and `StandardDynamic`, which the previous commit added for compound dynamics. They are now one. `MarkType` keeps a single `dynamics` value and the symbol itself moves into `MarkDataChoice`, so `` has one shape in the api instead of three. Writing a dynamic is unchanged in length -- `MarkData{StandardDynamic::ff}` -- and `ffz` now spells itself in `MarkData::name`, which it could not do before. ## Summary `StandardDynamic` won and the 29 dynamics values in `MarkType` were removed (`p` through `sfzp`, `otherDynamics`, `compoundDynamics`, `unknownDynamics`), replaced by one `dynamics` value. `StandardDynamic` was chosen despite being the newer type because it had not shipped -- it exists only on this branch -- while removing `MarkType::ff` and friends would break every downstream caller. The `Enclosure` unification in #378 is the precedent for accepting a breaking api change where the model improves. The symbol lives in `MarkDataChoice`, which gains `Kind::dynamic` holding a `StandardDynamic` alongside the existing `Kind::compoundDynamics`. `MarkDataChoice(CompoundDynamicsData)` collapses a lone standard component to `Kind::dynamic`, following `TimeChoice(ComplexTimeSignature)`. The collapse is what makes two alternatives safe rather than confusing: `ff` has exactly one representation however it was built, so code reading a plain dynamic never has to look inside a compound for it. A lone other-dynamics does not collapse -- it has no dedicated MusicXML element, so it stays a compound of one, which keeps `Kind::dynamic` meaning exactly "a standard abbreviation" and lets `dynamic()` return a bare `StandardDynamic`. Before this change one `` element had three api shapes: `markType == ff`; or `markType == otherDynamics` with the text in `MarkData::name` and the glyph in `choice.otherMark().smufl`; or `markType == compoundDynamics` with both facts restated inside `DynamicsComponent`. All three collapse to one component list. `DynamicsReader` loses its single-child special case and `DynamicsWriter` loses its entire non-compound branch. `MarkData` gains `MarkData(StandardDynamic)` and `MarkData(CompoundDynamicsData)`, which set `markType`, `choice`, and `name` together. Dynamics is the one mark family where a payload determines its mark type uniquely -- `Kind::tremolo` covers `tremoloStart`/`tremoloStop`, `Kind::arpeggiate` covers three mark types -- so it is the one family where a constructor can set both without guessing. Every dynamics mark in the tree, including the reader, now goes through these constructors; there are no direct `choice = CompoundDynamicsData{...}` assignments left. `MarkData::name` keeps working. It spells the whole mark -- "ff", "z", or "ffz" -- derived by one rule with no special cases, the way articulations and fermatas already name themselves. The writer ignores it for dynamics, so it cannot contradict the output. A single standard component yields what the old reader yielded, and a lone other-dynamics yields its text, so compatibility falls out of the general rule rather than being special-cased. `name` is documented for the first time, including that a self-naming mark derives it at construction and that replacing `choice` afterwards means re-deriving it. Mapping tables went from two to one, and the survivor is public. `markDataDynamicsKindToString` and `dynamicsKindToName` were near-identical private Kind-to-string switches differing only in their default arm; both are deleted, along with `Converter::dynamicsMap` and both `convertDynamic` overloads. `standardDynamicsMap` remains as the only dynamics map. In their place `DynamicsData.h` exports `toString(StandardDynamic)`, `toString(const DynamicsComponent &)` and `toString(const CompoundDynamicsData &)`, so clients can spell a mark they built rather than reaching for `name`. The core cannot supply these strings: it hard-codes the element literals inline in the generated parse and serialize routines with no accessor, and `mx::impl` never sees the XML -- by the time `DynamicsReader` runs, `` is already `DynamicsChoice::Kind::ff`. `isMarkDynamic` is now a single comparison and no longer has its duplicated `MarkType::p` test. Dispatch written as `if (isMarkDynamic(...))` keeps working verbatim, so notations and directions are untouched, as are `NoteAttachmentData::marks`, `DirectionChoice`, and every other mark family. MusicXML output is unchanged; this is an api-shape change only. ## Breaking changes - The 29 dynamics values of `MarkType` are gone. Use `MarkType::dynamics` with `MarkData(StandardDynamic)` or `MarkData(CompoundDynamicsData)`. Every site is a compile error. - `MarkData::name` is no longer the other-dynamics text on the write path; that text is `OtherDynamicsData::text`. The reader still fills `name` in, so read-only clients are unaffected. - Building a `CompoundDynamicsData` with a single standard component yields `Kind::dynamic`, not `Kind::compoundDynamics`. ## Testing - make test-all MX_RUNNING_IN_DOCKER=1 (837 + 41 + 502 test cases, 5541 assertions; 305 pinned round-trip files, 0 failed) - strict unity build of target mx (CMAKE_UNITY_BUILD_BATCH_SIZE=0) - git diff --check - make fmt MX_RUNNING_IN_DOCKER=1. Formatting was verified against the host clang-format only; Docker was unavailable, so CI's clang-format has not seen these files. ApiEquality.h is deliberately left as-is (host/Docker version drift, see 3a99c92). --- src/include/mx/api/DynamicsData.h | 10 ++ src/include/mx/api/MarkData.h | 48 ++++---- src/include/mx/api/MarkDataChoice.h | 12 +- src/private/mx/api/DynamicsData.cpp | 75 +++++++++++++ src/private/mx/api/MarkData.cpp | 103 ++++-------------- src/private/mx/api/MarkDataChoice.cpp | 35 +++++- src/private/mx/impl/Converter.cpp | 40 ------- src/private/mx/impl/Converter.h | 3 - src/private/mx/impl/DynamicsReader.cpp | 56 +++------- src/private/mx/impl/DynamicsReader.h | 71 ------------ src/private/mx/impl/DynamicsWriter.cpp | 14 +-- src/private/mxtest/api/ApiK007aScoreData.h | 74 ++++++------- src/private/mxtest/api/ApiK007cScoreData.h | 77 ++++++------- src/private/mxtest/api/ApiLy43eScoreData.h | 8 +- src/private/mxtest/api/DirectionDataTest.cpp | 4 +- .../api/DirectionMarksRoundTripTest.cpp | 8 +- src/private/mxtest/api/FreezingRoundTrip.cpp | 2 +- src/private/mxtest/api/MarkRoundTripTest.cpp | 31 ++++-- src/private/mxtest/api/OtherMarksApiTest.cpp | 99 +++++++++++++++-- 19 files changed, 378 insertions(+), 392 deletions(-) diff --git a/src/include/mx/api/DynamicsData.h b/src/include/mx/api/DynamicsData.h index db58b83c1..c61e97cf2 100644 --- a/src/include/mx/api/DynamicsData.h +++ b/src/include/mx/api/DynamicsData.h @@ -47,6 +47,9 @@ enum class StandardDynamic sfzp }; +// The letters of the symbol, e.g. "ff" -- also the name of the MusicXML element that carries it. +std::string toString(StandardDynamic value); + // 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 @@ -94,6 +97,10 @@ class DynamicsComponent MXAPI_NOT_EQUALS_AND_VECTORS(DynamicsComponent); +// The component's letters: the symbol for a standard component, the fallback text for one that has +// no dedicated MusicXML element. +std::string toString(const DynamicsComponent &value); + // 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 @@ -106,5 +113,8 @@ MXAPI_EQUALS_MEMBER(components) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(CompoundDynamicsData); +// The letters of the whole mark, its components run together -- "ffz" for ff followed by z. +std::string toString(const CompoundDynamicsData &value); + } // namespace api } // namespace mx diff --git a/src/include/mx/api/MarkData.h b/src/include/mx/api/MarkData.h index a173595e7..3f8b03cd0 100644 --- a/src/include/mx/api/MarkData.h +++ b/src/include/mx/api/MarkData.h @@ -4,6 +4,7 @@ #pragma once +#include "mx/api/DynamicsData.h" #include "mx/api/MarkDataChoice.h" #include "mx/api/PositionData.h" #include "mx/api/PrintData.h" @@ -46,35 +47,8 @@ enum class MarkType otherArticulation, // dynamics - 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, - otherDynamics, - compoundDynamics, ///< A single dynamic mark assembled from ordered components in MarkData::choice - unknownDynamics, + dynamics, ///< The symbol itself is in MarkData::choice -- a StandardDynamic such as ff, or a + ///< CompoundDynamicsData for marks like ffz that MusicXML spells with several symbols // ornaments trillMark, @@ -251,6 +225,13 @@ struct MarkData { // Fields common to (nearly) every mark, regardless of markType. MarkType markType; + + // The mark's text. For marks whose text is the data -- fingering, pluck, fret, string, and the + // other-* marks -- this is what gets written. For marks that name themselves -- articulations, + // fermatas, dynamics -- it spells the mark out ("ff", "ffz") and the writer ignores it, + // emitting whatever markType and choice say. A self-naming mark fills this in when it is + // constructed, so if you replace choice afterwards, re-derive it: toString() in DynamicsData.h + // spells a dynamic. std::string name; int tickTimePosition; PrintData printData; @@ -286,6 +267,15 @@ struct MarkData MarkData(); MarkData(MarkType inMarkType); MarkData(Placement inPlacement, MarkType inMarkType); + + // Builds a dynamic mark: markType is MarkType::dynamics, choice holds the symbol, and name is + // its letters. + MarkData(StandardDynamic inDynamic); + + // Builds a dynamic mark spelled with several symbols, such as ff followed by z for ffz. name + // becomes the letters of the whole mark. A lone standard symbol collapses to the same mark the + // StandardDynamic constructor builds. + MarkData(CompoundDynamicsData inDynamics); }; MXAPI_EQUALS_BEGIN(MarkData) diff --git a/src/include/mx/api/MarkDataChoice.h b/src/include/mx/api/MarkDataChoice.h index 1da63d237..54be32ac5 100644 --- a/src/include/mx/api/MarkDataChoice.h +++ b/src/include/mx/api/MarkDataChoice.h @@ -146,6 +146,7 @@ class MarkDataChoice arpeggiate, nonArpeggiate, otherMark, + dynamic, compoundDynamics, otherNotation }; @@ -160,6 +161,11 @@ class MarkDataChoice MarkDataChoice(OtherMarkData value); + MarkDataChoice(StandardDynamic value); + + // Builds a compound dynamic, unless the value is a single standard symbol, in which case the + // result is a Kind::dynamic choice (auto-collapse). A lone other-dynamics symbol does not + // collapse -- it has no dedicated MusicXML element, so it stays a compound of one. MarkDataChoice(CompoundDynamicsData value); MarkDataChoice(OtherNotationMarkData value); @@ -170,6 +176,7 @@ class MarkDataChoice bool isArpeggiate() const; bool isNonArpeggiate() const; bool isOtherMark() const; + bool isDynamic() const; bool isCompoundDynamics() const; bool isOtherNotation() const; @@ -194,6 +201,9 @@ class MarkDataChoice // Returns a copy of the internally held OtherMarkData, or a default value for another kind. const OtherMarkData otherMark() const; + // Returns the standard dynamic symbol, or p for another kind. + StandardDynamic dynamic() const; + // Returns a copy of the internally held CompoundDynamicsData, or a default value for another kind. const CompoundDynamicsData compoundDynamics() const; @@ -204,7 +214,7 @@ class MarkDataChoice private: std::variant + StandardDynamic, CompoundDynamicsData, OtherNotationMarkData> myValue; }; diff --git a/src/private/mx/api/DynamicsData.cpp b/src/private/mx/api/DynamicsData.cpp index ffb9c79fb..1d6c517ae 100644 --- a/src/private/mx/api/DynamicsData.cpp +++ b/src/private/mx/api/DynamicsData.cpp @@ -11,6 +11,66 @@ namespace mx namespace api { +std::string toString(StandardDynamic value) +{ + switch (value) + { + case StandardDynamic::p: + return "p"; + case StandardDynamic::pp: + return "pp"; + case StandardDynamic::ppp: + return "ppp"; + case StandardDynamic::pppp: + return "pppp"; + case StandardDynamic::ppppp: + return "ppppp"; + case StandardDynamic::pppppp: + return "pppppp"; + case StandardDynamic::f: + return "f"; + case StandardDynamic::ff: + return "ff"; + case StandardDynamic::fff: + return "fff"; + case StandardDynamic::ffff: + return "ffff"; + case StandardDynamic::fffff: + return "fffff"; + case StandardDynamic::ffffff: + return "ffffff"; + case StandardDynamic::mp: + return "mp"; + case StandardDynamic::mf: + return "mf"; + case StandardDynamic::sf: + return "sf"; + case StandardDynamic::sfp: + return "sfp"; + case StandardDynamic::sfpp: + return "sfpp"; + case StandardDynamic::fp: + return "fp"; + case StandardDynamic::rf: + return "rf"; + case StandardDynamic::rfz: + return "rfz"; + case StandardDynamic::sfz: + return "sfz"; + case StandardDynamic::sffz: + return "sffz"; + case StandardDynamic::fz: + return "fz"; + case StandardDynamic::n: + return "n"; + case StandardDynamic::pf: + return "pf"; + case StandardDynamic::sfzp: + return "sfzp"; + } + return "p"; +} + DynamicsComponent::DynamicsComponent() : myValue{StandardDynamic::p} { } @@ -61,5 +121,20 @@ bool DynamicsComponent::operator==(const DynamicsComponent &other) const return myValue == other.myValue; } +std::string toString(const DynamicsComponent &value) +{ + return value.isOther() ? value.other().text : toString(value.standard()); +} + +std::string toString(const CompoundDynamicsData &value) +{ + std::string result; + for (const auto &component : value.components) + { + result += toString(component); + } + return result; +} + } // namespace api } // namespace mx diff --git a/src/private/mx/api/MarkData.cpp b/src/private/mx/api/MarkData.cpp index 06b33f067..8f1536dcb 100644 --- a/src/private/mx/api/MarkData.cpp +++ b/src/private/mx/api/MarkData.cpp @@ -3,7 +3,6 @@ // Distributed under the MIT License #include "mx/api/MarkData.h" -#include "mx/core/generated/DynamicsChoice.h" #include "mx/core/generated/FermataShape.h" #include "mx/impl/Converter.h" @@ -11,81 +10,9 @@ namespace mx { namespace api { -// The wire literal of a dynamics alternative (the old core's -// toString(DynamicsEnum); the new variant Kind carries no string). -std::string markDataDynamicsKindToString(core::DynamicsChoice::Kind kind) -{ - switch (kind) - { - case core::DynamicsChoice::Kind::p: - return "p"; - case core::DynamicsChoice::Kind::pp: - return "pp"; - case core::DynamicsChoice::Kind::ppp: - return "ppp"; - case core::DynamicsChoice::Kind::pppp: - return "pppp"; - case core::DynamicsChoice::Kind::ppppp: - return "ppppp"; - case core::DynamicsChoice::Kind::pppppp: - return "pppppp"; - case core::DynamicsChoice::Kind::f: - return "f"; - case core::DynamicsChoice::Kind::ff: - return "ff"; - case core::DynamicsChoice::Kind::fff: - return "fff"; - case core::DynamicsChoice::Kind::ffff: - return "ffff"; - case core::DynamicsChoice::Kind::fffff: - return "fffff"; - case core::DynamicsChoice::Kind::ffffff: - return "ffffff"; - case core::DynamicsChoice::Kind::mp: - return "mp"; - case core::DynamicsChoice::Kind::mf: - return "mf"; - case core::DynamicsChoice::Kind::sf: - return "sf"; - case core::DynamicsChoice::Kind::sfp: - return "sfp"; - case core::DynamicsChoice::Kind::sfpp: - return "sfpp"; - case core::DynamicsChoice::Kind::fp: - return "fp"; - case core::DynamicsChoice::Kind::rf: - return "rf"; - case core::DynamicsChoice::Kind::rfz: - return "rfz"; - case core::DynamicsChoice::Kind::sfz: - return "sfz"; - case core::DynamicsChoice::Kind::sffz: - return "sffz"; - case core::DynamicsChoice::Kind::fz: - return "fz"; - case core::DynamicsChoice::Kind::n: - return "n"; - case core::DynamicsChoice::Kind::pf: - return "pf"; - case core::DynamicsChoice::Kind::sfzp: - return "sfzp"; - default: - return "other-dynamics"; - } -} - bool isMarkDynamic(MarkType markType) { - return (markType == MarkType::p) || (markType == MarkType::p) || (markType == MarkType::pp) || - (markType == MarkType::ppp) || (markType == MarkType::pppp) || (markType == MarkType::ppppp) || - (markType == MarkType::pppppp) || (markType == MarkType::f) || (markType == MarkType::ff) || - (markType == MarkType::fff) || (markType == MarkType::ffff) || (markType == MarkType::fffff) || - (markType == MarkType::ffffff) || (markType == MarkType::mp) || (markType == MarkType::mf) || - (markType == MarkType::sf) || (markType == MarkType::sfp) || (markType == MarkType::sfpp) || - (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::compoundDynamics); + return markType == MarkType::dynamics; } bool isMarkArpeggiate(MarkType markType) @@ -246,11 +173,7 @@ MarkData::MarkData(MarkType inMarkType) fingeringAlternate{Bool::unspecified}, choice{} { impl::Converter converter; - if (isMarkDynamic(markType) && markType != MarkType::compoundDynamics) - { - name = markDataDynamicsKindToString(converter.convertDynamic(markType)); - } - else if (isMarkArticulation(markType)) + if (isMarkArticulation(markType)) { name = "articulation"; } @@ -268,11 +191,7 @@ MarkData::MarkData(Placement inPlacement, MarkType inMarkType) { positionData.placement = inPlacement; impl::Converter converter; - if (isMarkDynamic(markType) && markType != MarkType::compoundDynamics) - { - name = markDataDynamicsKindToString(converter.convertDynamic(markType)); - } - else if (isMarkArticulation(markType)) + if (isMarkArticulation(markType)) { name = "articulation"; } @@ -281,5 +200,21 @@ MarkData::MarkData(Placement inPlacement, MarkType inMarkType) name = std::string{converter.convertFermata(markType).toString()}; } } + +MarkData::MarkData(StandardDynamic inDynamic) + : markType(MarkType::dynamics), name{toString(inDynamic)}, 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}, choice{inDynamic} +{ +} + +MarkData::MarkData(CompoundDynamicsData inDynamics) + : markType(MarkType::dynamics), name{toString(inDynamics)}, 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}, choice{std::move(inDynamics)} +{ +} } // namespace api } // namespace mx diff --git a/src/private/mx/api/MarkDataChoice.cpp b/src/private/mx/api/MarkDataChoice.cpp index fdda8bd1e..3303459a8 100644 --- a/src/private/mx/api/MarkDataChoice.cpp +++ b/src/private/mx/api/MarkDataChoice.cpp @@ -31,10 +31,25 @@ MarkDataChoice::MarkDataChoice(OtherMarkData value) : myValue{std::move(value)} { } -MarkDataChoice::MarkDataChoice(CompoundDynamicsData value) : myValue{std::move(value)} +MarkDataChoice::MarkDataChoice(StandardDynamic value) : myValue{value} { } +// A single standard symbol is the same mark whether it was built as a compound or not, so it is +// always stored as Kind::dynamic. That keeps one representation per mark: code reading a plain ff +// never has to look inside a compound for it. +MarkDataChoice::MarkDataChoice(CompoundDynamicsData value) +{ + if (value.components.size() == 1 && value.components.front().isStandard()) + { + myValue = value.components.front().standard(); + } + else + { + myValue = std::move(value); + } +} + MarkDataChoice::MarkDataChoice(OtherNotationMarkData value) : myValue{std::move(value)} { } @@ -57,6 +72,10 @@ MarkDataChoice::Kind MarkDataChoice::kind() const { return Kind::otherMark; } + if (std::holds_alternative(myValue)) + { + return Kind::dynamic; + } if (std::holds_alternative(myValue)) { return Kind::compoundDynamics; @@ -129,6 +148,20 @@ const OtherMarkData MarkDataChoice::otherMark() const return OtherMarkData{}; } +bool MarkDataChoice::isDynamic() const +{ + return std::holds_alternative(myValue); +} + +StandardDynamic MarkDataChoice::dynamic() const +{ + if (const auto *value = std::get_if(&myValue)) + { + return *value; + } + return StandardDynamic::p; +} + bool MarkDataChoice::isCompoundDynamics() const { return std::holds_alternative(myValue); diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index 92dd8aed9..59f5346df 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -184,36 +184,6 @@ const Converter::EnumMap Convert {core::ArticulationsChoice::Kind::otherArticulation, api::MarkType::otherArticulation}, }; -const Converter::EnumMap Converter::dynamicsMap = { - {core::DynamicsChoice::Kind::p, api::MarkType::p}, - {core::DynamicsChoice::Kind::pp, api::MarkType::pp}, - {core::DynamicsChoice::Kind::ppp, api::MarkType::ppp}, - {core::DynamicsChoice::Kind::pppp, api::MarkType::pppp}, - {core::DynamicsChoice::Kind::ppppp, api::MarkType::ppppp}, - {core::DynamicsChoice::Kind::pppppp, api::MarkType::pppppp}, - {core::DynamicsChoice::Kind::f, api::MarkType::f}, - {core::DynamicsChoice::Kind::ff, api::MarkType::ff}, - {core::DynamicsChoice::Kind::fff, api::MarkType::fff}, - {core::DynamicsChoice::Kind::ffff, api::MarkType::ffff}, - {core::DynamicsChoice::Kind::fffff, api::MarkType::fffff}, - {core::DynamicsChoice::Kind::ffffff, api::MarkType::ffffff}, - {core::DynamicsChoice::Kind::mp, api::MarkType::mp}, - {core::DynamicsChoice::Kind::mf, api::MarkType::mf}, - {core::DynamicsChoice::Kind::sf, api::MarkType::sf}, - {core::DynamicsChoice::Kind::sfp, api::MarkType::sfp}, - {core::DynamicsChoice::Kind::sfpp, api::MarkType::sfpp}, - {core::DynamicsChoice::Kind::fp, api::MarkType::fp}, - {core::DynamicsChoice::Kind::rf, api::MarkType::rf}, - {core::DynamicsChoice::Kind::rfz, api::MarkType::rfz}, - {core::DynamicsChoice::Kind::sfz, api::MarkType::sfz}, - {core::DynamicsChoice::Kind::sffz, api::MarkType::sffz}, - {core::DynamicsChoice::Kind::fz, api::MarkType::fz}, - {core::DynamicsChoice::Kind::n, api::MarkType::n}, - {core::DynamicsChoice::Kind::pf, api::MarkType::pf}, - {core::DynamicsChoice::Kind::sfzp, api::MarkType::sfzp}, - {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}, @@ -1789,16 +1759,6 @@ api::MarkType Converter::convertArticulation(core::ArticulationsChoice::Kind val return findApiItem(articulationsMap, api::MarkType::unspecified, value); } -core::DynamicsChoice::Kind Converter::convertDynamic(api::MarkType value) const -{ - return findCoreItem(dynamicsMap, core::DynamicsChoice::Kind::otherDynamics, value); -} - -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); diff --git a/src/private/mx/impl/Converter.h b/src/private/mx/impl/Converter.h index 1bdf67822..a558edabb 100644 --- a/src/private/mx/impl/Converter.h +++ b/src/private/mx/impl/Converter.h @@ -129,8 +129,6 @@ class Converter core::ArticulationsChoice::Kind convertArticulation(api::MarkType value) const; api::MarkType convertArticulation(core::ArticulationsChoice::Kind value) const; - 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; @@ -272,7 +270,6 @@ class Converter const static EnumMap fontStyleMap; const static EnumMap fontWeightMap; const static EnumMap articulationsMap; - const static EnumMap dynamicsMap; const static EnumMap standardDynamicsMap; const static EnumMap otherNotationTypeMap; const static EnumMap ornamentsMap; diff --git a/src/private/mx/impl/DynamicsReader.cpp b/src/private/mx/impl/DynamicsReader.cpp index 8064bfd20..a04a48476 100644 --- a/src/private/mx/impl/DynamicsReader.cpp +++ b/src/private/mx/impl/DynamicsReader.cpp @@ -27,58 +27,32 @@ void DynamicsReader::parseDynamics(std::vector &outMarks) const } Converter converter; - auto markData = api::MarkData{}; - markData.tickTimePosition = myCursor.tickTimePosition; - markData.positionData = impl::getPositionData(myDynamic); - markData.printData = impl::getPrintData(myDynamic); - - if (choices.size() == 1) + api::CompoundDynamicsData compound; + compound.components.reserve(choices.size()); + for (const auto &choice : choices) { - const auto &choice = choices.front(); - const auto kind = choice.kind(); - markData.markType = converter.convertDynamic(kind); - - if (kind == core::DynamicsChoice::Kind::otherDynamics) + if (choice.kind() == core::DynamicsChoice::Kind::otherDynamics) { const auto &other = choice.asOtherDynamics(); - markData.name = other.value(); - api::OtherMarkData payload; + api::OtherDynamicsData component; + component.text = other.value(); if (other.smufl().has_value()) { - payload.smufl = other.smufl()->toString(); + component.smufl = other.smufl()->toString(); } - markData.choice = std::move(payload); + compound.components.emplace_back(std::move(component)); } else { - markData.name = dynamicsKindToName(kind); + compound.components.emplace_back(converter.convertStandardDynamic(choice.kind())); } } - else - { - 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); - } + + // The constructor spells the mark into name and collapses a lone standard symbol. + auto markData = api::MarkData{std::move(compound)}; + markData.tickTimePosition = myCursor.tickTimePosition; + markData.positionData = impl::getPositionData(myDynamic); + markData.printData = impl::getPrintData(myDynamic); outMarks.emplace_back(std::move(markData)); } diff --git a/src/private/mx/impl/DynamicsReader.h b/src/private/mx/impl/DynamicsReader.h index fa79a5287..c437426d7 100644 --- a/src/private/mx/impl/DynamicsReader.h +++ b/src/private/mx/impl/DynamicsReader.h @@ -5,11 +5,8 @@ #pragma once #include "mx/api/MarkData.h" -#include "mx/core/generated/DynamicsChoice.h" #include "mx/impl/Cursor.h" -#include - namespace mx { namespace core @@ -20,74 +17,6 @@ class Dynamics; namespace impl { -/// Returns the MusicXML element-name string for a dynamics kind -/// (e.g. Kind::mf → "mf", Kind::otherDynamics → "other-dynamics"). -/// Exposed here so test-data helpers can use it without duplicating -/// the mapping. -inline std::string dynamicsKindToName(core::DynamicsChoice::Kind kind) -{ - using K = core::DynamicsChoice::Kind; - switch (kind) - { - case K::p: - return "p"; - case K::pp: - return "pp"; - case K::ppp: - return "ppp"; - case K::pppp: - return "pppp"; - case K::ppppp: - return "ppppp"; - case K::pppppp: - return "pppppp"; - case K::f: - return "f"; - case K::ff: - return "ff"; - case K::fff: - return "fff"; - case K::ffff: - return "ffff"; - case K::fffff: - return "fffff"; - case K::ffffff: - return "ffffff"; - case K::mp: - return "mp"; - case K::mf: - return "mf"; - case K::sf: - return "sf"; - case K::sfp: - return "sfp"; - case K::sfpp: - return "sfpp"; - case K::fp: - return "fp"; - case K::rf: - return "rf"; - case K::rfz: - return "rfz"; - case K::sfz: - return "sfz"; - case K::sffz: - return "sffz"; - case K::fz: - return "fz"; - case K::n: - return "n"; - case K::pf: - return "pf"; - case K::sfzp: - return "sfzp"; - case K::otherDynamics: - return "other-dynamics"; - default: - return "p"; - } -} - class DynamicsReader { public: diff --git a/src/private/mx/impl/DynamicsWriter.cpp b/src/private/mx/impl/DynamicsWriter.cpp index 0fb3814aa..219022cd0 100644 --- a/src/private/mx/impl/DynamicsWriter.cpp +++ b/src/private/mx/impl/DynamicsWriter.cpp @@ -105,7 +105,11 @@ DynamicsWriter::DynamicsWriter(const api::MarkData &inMark, impl::Cursor inCurso core::Dynamics DynamicsWriter::getDynamics() const { core::Dynamics dyn; - if (myMarkData.markType == api::MarkType::compoundDynamics) + if (myMarkData.choice.isDynamic()) + { + dyn.addChoice(dynamicsWriterMakeChoice(myConverter.convert(myMarkData.choice.dynamic()), {}, {})); + } + else { for (const auto &component : myMarkData.choice.compoundDynamics().components) { @@ -121,14 +125,6 @@ core::Dynamics DynamicsWriter::getDynamics() const } } } - 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/mxtest/api/ApiK007aScoreData.h b/src/private/mxtest/api/ApiK007aScoreData.h index 241b6be7c..d912cfbc5 100644 --- a/src/private/mxtest/api/ApiK007aScoreData.h +++ b/src/private/mxtest/api/ApiK007aScoreData.h @@ -5,13 +5,13 @@ #pragma once #include "mx/api/ScoreData.h" -#include "mx/impl/Converter.h" -#include "mx/impl/DynamicsReader.h" + +#include namespace { -inline void addNoteToMeasure(mx::api::MarkType markType, mx::api::MeasureData *measureP) +inline void addNoteToMeasure(mx::api::MarkData dynamic, mx::api::MeasureData *measureP) { using namespace mx::api; auto staffP = &measureP->staves.at(0); @@ -26,26 +26,21 @@ inline void addNoteToMeasure(mx::api::MarkType markType, mx::api::MeasureData *m noteP->pitchData.accidental = Accidental::none; noteP->durationData.durationName = DurationName::whole; noteP->durationData.durationTimeTicks = 8; - noteP->noteAttachmentData.marks.emplace_back(MarkData{}); - auto &markData = noteP->noteAttachmentData.marks.back(); - markData.markType = markType; - mx::impl::Converter converter; - const auto d = converter.convertDynamic(markType); - markData.name = mx::impl::dynamicsKindToName(d); + auto &markData = noteP->noteAttachmentData.marks.emplace_back(std::move(dynamic)); markData.tickTimePosition = noteP->tickTimePosition; markData.positionData.placement = Placement::below; } -inline void addMeasureWithNote(mx::api::MarkType markType, mx::api::PartData &outPartData) +inline void addMeasureWithNote(mx::api::MarkData dynamic, mx::api::PartData &outPartData) { using namespace mx::api; outPartData.measures.emplace_back(MeasureData{}); auto measureP = &outPartData.measures.back(); measureP->staves.emplace_back(StaffData{}); - addNoteToMeasure(markType, measureP); + addNoteToMeasure(std::move(dynamic), measureP); } -inline void addFirstMeasureWithNote(mx::api::MarkType markType, mx::api::PartData &outPartData) +inline void addFirstMeasureWithNote(mx::api::MarkData dynamic, mx::api::PartData &outPartData) { using namespace mx::api; outPartData.measures.emplace_back(MeasureData{}); @@ -58,7 +53,7 @@ inline void addFirstMeasureWithNote(mx::api::MarkType markType, mx::api::PartDat auto clefP = &staffP->clefs.back(); clefP->symbol = ClefSymbol::g; clefP->line = 2; - addNoteToMeasure(markType, measureP); + addNoteToMeasure(std::move(dynamic), measureP); } } // namespace @@ -75,35 +70,30 @@ inline mx::api::ScoreData apiK007aScoreData() part.uniqueId = "P1"; part.name = "Dynamics"; - addFirstMeasureWithNote(MarkType::p, part); - addMeasureWithNote(MarkType::pp, part); - addMeasureWithNote(MarkType::ppp, part); - addMeasureWithNote(MarkType::pppp, part); - addMeasureWithNote(MarkType::ppppp, part); - addMeasureWithNote(MarkType::pppppp, part); - addMeasureWithNote(MarkType::f, part); - addMeasureWithNote(MarkType::ff, part); - addMeasureWithNote(MarkType::fff, part); - addMeasureWithNote(MarkType::ffff, part); - addMeasureWithNote(MarkType::fffff, part); - addMeasureWithNote(MarkType::ffffff, part); - addMeasureWithNote(MarkType::mp, part); - addMeasureWithNote(MarkType::mf, part); - addMeasureWithNote(MarkType::sf, part); - addMeasureWithNote(MarkType::sfp, part); - addMeasureWithNote(MarkType::sfpp, part); - addMeasureWithNote(MarkType::fp, part); - addMeasureWithNote(MarkType::rf, part); - addMeasureWithNote(MarkType::rfz, part); - addMeasureWithNote(MarkType::sfz, part); - addMeasureWithNote(MarkType::sffz, part); - addMeasureWithNote(MarkType::fz, part); - addMeasureWithNote(MarkType::otherDynamics, part); - - auto &lastMeasure = part.measures.back(); - auto &lastDynamic = lastMeasure.staves.back().voices[0].notes.back().noteAttachmentData.marks.back(); - const std::string name = "dynamicNiente"; - lastDynamic.name = name; + addFirstMeasureWithNote(StandardDynamic::p, part); + addMeasureWithNote(StandardDynamic::pp, part); + addMeasureWithNote(StandardDynamic::ppp, part); + addMeasureWithNote(StandardDynamic::pppp, part); + addMeasureWithNote(StandardDynamic::ppppp, part); + addMeasureWithNote(StandardDynamic::pppppp, part); + addMeasureWithNote(StandardDynamic::f, part); + addMeasureWithNote(StandardDynamic::ff, part); + addMeasureWithNote(StandardDynamic::fff, part); + addMeasureWithNote(StandardDynamic::ffff, part); + addMeasureWithNote(StandardDynamic::fffff, part); + addMeasureWithNote(StandardDynamic::ffffff, part); + addMeasureWithNote(StandardDynamic::mp, part); + addMeasureWithNote(StandardDynamic::mf, part); + addMeasureWithNote(StandardDynamic::sf, part); + addMeasureWithNote(StandardDynamic::sfp, part); + addMeasureWithNote(StandardDynamic::sfpp, part); + addMeasureWithNote(StandardDynamic::fp, part); + addMeasureWithNote(StandardDynamic::rf, part); + addMeasureWithNote(StandardDynamic::rfz, part); + addMeasureWithNote(StandardDynamic::sfz, part); + addMeasureWithNote(StandardDynamic::sffz, part); + addMeasureWithNote(StandardDynamic::fz, part); + addMeasureWithNote(CompoundDynamicsData{{OtherDynamicsData{"dynamicNiente"}}}, part); return score; } diff --git a/src/private/mxtest/api/ApiK007cScoreData.h b/src/private/mxtest/api/ApiK007cScoreData.h index e4bd7195d..66b59871d 100644 --- a/src/private/mxtest/api/ApiK007cScoreData.h +++ b/src/private/mxtest/api/ApiK007cScoreData.h @@ -5,12 +5,12 @@ #pragma once #include "mx/api/ScoreData.h" -#include "mx/impl/Converter.h" -#include "mx/impl/DynamicsReader.h" + +#include namespace { -inline void addNoteToMeasure(mx::api::MarkType markType, mx::api::MeasureData *measureP) +inline void addNoteToMeasure(mx::api::MarkData dynamic, mx::api::MeasureData *measureP) { using namespace mx::api; auto staffP = &measureP->staves.at(0); @@ -29,27 +29,22 @@ inline void addNoteToMeasure(mx::api::MarkType markType, mx::api::MeasureData *m staffP->directions.emplace_back(DirectionData{}); auto &direction = staffP->directions.back(); - MarkData mark{}; - mark.markType = markType; - mx::impl::Converter converter; - const auto d = converter.convertDynamic(markType); - mark.name = mx::impl::dynamicsKindToName(d); - mark.positionData.placement = Placement::below; + dynamic.positionData.placement = Placement::below; direction.tickTimePosition = noteP->tickTimePosition; direction.isStaffValueSpecified = false; - direction.directionTypes.emplace_back(DirectionChoice{mark}); + direction.directionTypes.emplace_back(DirectionChoice{std::move(dynamic)}); } -inline void addMeasureWithNote(mx::api::MarkType markType, mx::api::PartData &outPartData) +inline void addMeasureWithNote(mx::api::MarkData dynamic, mx::api::PartData &outPartData) { using namespace mx::api; outPartData.measures.emplace_back(MeasureData{}); auto measureP = &outPartData.measures.back(); measureP->staves.emplace_back(StaffData{}); - addNoteToMeasure(markType, measureP); + addNoteToMeasure(std::move(dynamic), measureP); } -inline void addFirstMeasureWithNote(mx::api::MarkType markType, mx::api::PartData &outPartData) +inline void addFirstMeasureWithNote(mx::api::MarkData dynamic, mx::api::PartData &outPartData) { using namespace mx::api; outPartData.measures.emplace_back(MeasureData{}); @@ -62,7 +57,7 @@ inline void addFirstMeasureWithNote(mx::api::MarkType markType, mx::api::PartDat auto clefP = &staffP->clefs.back(); clefP->symbol = ClefSymbol::g; clefP->line = 2; - addNoteToMeasure(markType, measureP); + addNoteToMeasure(std::move(dynamic), measureP); } } // namespace @@ -79,36 +74,30 @@ inline mx::api::ScoreData apiK007cScoreData() part.uniqueId = "P1"; part.name = "Dynamics"; - addFirstMeasureWithNote(MarkType::p, part); - addMeasureWithNote(MarkType::pp, part); - addMeasureWithNote(MarkType::ppp, part); - addMeasureWithNote(MarkType::pppp, part); - addMeasureWithNote(MarkType::ppppp, part); - addMeasureWithNote(MarkType::pppppp, part); - addMeasureWithNote(MarkType::f, part); - addMeasureWithNote(MarkType::ff, part); - addMeasureWithNote(MarkType::fff, part); - addMeasureWithNote(MarkType::ffff, part); - addMeasureWithNote(MarkType::fffff, part); - addMeasureWithNote(MarkType::ffffff, part); - addMeasureWithNote(MarkType::mp, part); - addMeasureWithNote(MarkType::mf, part); - addMeasureWithNote(MarkType::sf, part); - addMeasureWithNote(MarkType::sfp, part); - addMeasureWithNote(MarkType::sfpp, part); - addMeasureWithNote(MarkType::fp, part); - addMeasureWithNote(MarkType::rf, part); - addMeasureWithNote(MarkType::rfz, part); - addMeasureWithNote(MarkType::sfz, part); - addMeasureWithNote(MarkType::sffz, part); - addMeasureWithNote(MarkType::fz, part); - addMeasureWithNote(MarkType::otherDynamics, part); - - auto &lastMeasure = part.measures.back(); - auto &lastDirection = lastMeasure.staves.back().directions.back(); - auto lastDynamic = lastDirection.directionTypes.back().mark(); - lastDynamic.name = "dynamicNiente"; - lastDirection.directionTypes.back() = DirectionChoice{lastDynamic}; + addFirstMeasureWithNote(StandardDynamic::p, part); + addMeasureWithNote(StandardDynamic::pp, part); + addMeasureWithNote(StandardDynamic::ppp, part); + addMeasureWithNote(StandardDynamic::pppp, part); + addMeasureWithNote(StandardDynamic::ppppp, part); + addMeasureWithNote(StandardDynamic::pppppp, part); + addMeasureWithNote(StandardDynamic::f, part); + addMeasureWithNote(StandardDynamic::ff, part); + addMeasureWithNote(StandardDynamic::fff, part); + addMeasureWithNote(StandardDynamic::ffff, part); + addMeasureWithNote(StandardDynamic::fffff, part); + addMeasureWithNote(StandardDynamic::ffffff, part); + addMeasureWithNote(StandardDynamic::mp, part); + addMeasureWithNote(StandardDynamic::mf, part); + addMeasureWithNote(StandardDynamic::sf, part); + addMeasureWithNote(StandardDynamic::sfp, part); + addMeasureWithNote(StandardDynamic::sfpp, part); + addMeasureWithNote(StandardDynamic::fp, part); + addMeasureWithNote(StandardDynamic::rf, part); + addMeasureWithNote(StandardDynamic::rfz, part); + addMeasureWithNote(StandardDynamic::sfz, part); + addMeasureWithNote(StandardDynamic::sffz, part); + addMeasureWithNote(StandardDynamic::fz, part); + addMeasureWithNote(CompoundDynamicsData{{OtherDynamicsData{"dynamicNiente"}}}, part); return score; } diff --git a/src/private/mxtest/api/ApiLy43eScoreData.h b/src/private/mxtest/api/ApiLy43eScoreData.h index c031e2e91..1f47bb7a1 100644 --- a/src/private/mxtest/api/ApiLy43eScoreData.h +++ b/src/private/mxtest/api/ApiLy43eScoreData.h @@ -42,9 +42,7 @@ inline mx::api::ScoreData apiLy43eScoreData() staff1P->directions.emplace_back(DirectionData{}); auto directionP = &staff1P->directions.back(); directionP->placement = Placement::below; - MarkData directionMark{}; - directionMark.markType = MarkType::ffff; - directionMark.name = "ffff"; + MarkData directionMark{StandardDynamic::ffff}; directionMark.tickTimePosition = 0; directionP->directionTypes.emplace_back(DirectionChoice{directionMark}); @@ -101,10 +99,8 @@ inline mx::api::ScoreData apiLy43eScoreData() // directionP->offset = 1; directionP->placement = Placement::below; directionP->isStaffValueSpecified = true; - directionMark = MarkData{}; + directionMark = MarkData{StandardDynamic::p}; directionMark.tickTimePosition = directionP->tickTimePosition; - directionMark.markType = MarkType::p; - directionMark.name = "p"; directionP->directionTypes.emplace_back(DirectionChoice{directionMark}); voice = 1; diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index 2ba101dfb..e34f523a7 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -56,7 +56,7 @@ TEST(OutOfOrderDoesntThrow, DirectionData) ovoice.notes.push_back(onote); DirectionData directionData{}; - MarkData mark{MarkType::f}; + MarkData mark{StandardDynamic::f}; int tickTime = 10; mark.tickTimePosition = tickTime; @@ -190,7 +190,7 @@ TEST(OutOfOrderTorture, DirectionData) ovoice.notes.push_back(onote); DirectionData directionData{}; - MarkData mark{MarkType::f}; + MarkData mark{StandardDynamic::f}; mark.tickTimePosition = dur0tick; directionData.tickTimePosition = dur0tick; diff --git a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp index 05ff00539..7da61896c 100644 --- a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp +++ b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp @@ -529,7 +529,7 @@ TEST(WordsSymbolInterleaved, DirectionMarksRoundTrip) run.emplace_back(after); direction.directionTypes.emplace_back(DirectionChoice{run}); - MarkData dynamic{MarkType::ff}; + MarkData dynamic{StandardDynamic::ff}; direction.directionTypes.emplace_back(DirectionChoice{dynamic}); const auto directions = roundTripDirectionData(direction); @@ -547,7 +547,7 @@ TEST(WordsSymbolInterleaved, DirectionMarksRoundTrip) CHECK_EQUAL(" al niente", outRun.at(2).words().text); REQUIRE(directions.front().directionTypes.back().isMark()); - CHECK(directions.front().directionTypes.back().mark().markType == MarkType::ff); + CHECK(directions.front().directionTypes.back().mark().choice.dynamic() == StandardDynamic::ff); } T_END; @@ -562,7 +562,7 @@ TEST(WordsBeforeDynamicsOrderPreserved, DirectionMarksRoundTrip) WordsData piu; piu.text = "più"; direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{piu}}}); - direction.directionTypes.emplace_back(DirectionChoice{MarkData{MarkType::f}}); + direction.directionTypes.emplace_back(DirectionChoice{MarkData{StandardDynamic::f}}); WordsData troppo; troppo.text = "ma non troppo"; direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{troppo}}}); @@ -574,7 +574,7 @@ TEST(WordsBeforeDynamicsOrderPreserved, DirectionMarksRoundTrip) REQUIRE(directions.front().directionTypes.at(0).wordsRun().size() == 1); CHECK_EQUAL("più", directions.front().directionTypes.at(0).wordsRun().front().words().text); REQUIRE(directions.front().directionTypes.at(1).isMark()); - CHECK(directions.front().directionTypes.at(1).mark().markType == MarkType::f); + CHECK(directions.front().directionTypes.at(1).mark().choice.dynamic() == StandardDynamic::f); REQUIRE(directions.front().directionTypes.at(2).isWordsRun()); REQUIRE(directions.front().directionTypes.at(2).wordsRun().size() == 1); CHECK_EQUAL("ma non troppo", directions.front().directionTypes.at(2).wordsRun().front().words().text); diff --git a/src/private/mxtest/api/FreezingRoundTrip.cpp b/src/private/mxtest/api/FreezingRoundTrip.cpp index 9cbd576fd..ad3ac27e4 100644 --- a/src/private/mxtest/api/FreezingRoundTrip.cpp +++ b/src/private/mxtest/api/FreezingRoundTrip.cpp @@ -242,7 +242,7 @@ TEST(roundTripViolaDynamicWrongTime, Freezing) const auto findDirectionLambda = [&](const DirectionData &inDirection) { if (inDirection.directionTypes.size() == 1 && inDirection.directionTypes.front().isMark()) { - if (inDirection.directionTypes.front().mark().markType == MarkType::pp) + if (inDirection.directionTypes.front().mark().choice.dynamic() == StandardDynamic::pp) { return true; } diff --git a/src/private/mxtest/api/MarkRoundTripTest.cpp b/src/private/mxtest/api/MarkRoundTripTest.cpp index 191a90e79..8f43f465e 100644 --- a/src/private/mxtest/api/MarkRoundTripTest.cpp +++ b/src/private/mxtest/api/MarkRoundTripTest.cpp @@ -63,6 +63,23 @@ std::vector roundTripMark(MarkType inMarkType) return roundTripMarkData(MarkData{Placement::unspecified, inMarkType}); } +std::vector roundTripDynamic(StandardDynamic inDynamic) +{ + return roundTripMarkData(MarkData{inDynamic}); +} + +bool hasDynamic(const std::vector &marks, StandardDynamic inDynamic) +{ + for (const auto &m : marks) + { + if (m.markType == MarkType::dynamics && m.choice.isDynamic() && m.choice.dynamic() == inDynamic) + { + return true; + } + } + return false; +} + bool hasMark(const std::vector &marks, MarkType inMarkType) { for (const auto &m : marks) @@ -77,27 +94,27 @@ bool hasMark(const std::vector &marks, MarkType inMarkType) } // namespace // #193 - dynamics n, pf, sfzp were silently dropped (fell back to unspecified) -// because the api enum lacked the members and dynamicsMap lacked the rows. +// because the api enum lacked the members and standardDynamicsMap lacked the rows. TEST(DynamicsN, MarkRoundTrip) { - const auto marks = roundTripMark(MarkType::n); - CHECK(hasMark(marks, MarkType::n)); + const auto marks = roundTripDynamic(StandardDynamic::n); + CHECK(hasDynamic(marks, StandardDynamic::n)); } T_END; TEST(DynamicsPf, MarkRoundTrip) { - const auto marks = roundTripMark(MarkType::pf); - CHECK(hasMark(marks, MarkType::pf)); + const auto marks = roundTripDynamic(StandardDynamic::pf); + CHECK(hasDynamic(marks, StandardDynamic::pf)); } T_END; TEST(DynamicsSfzp, MarkRoundTrip) { - const auto marks = roundTripMark(MarkType::sfzp); - CHECK(hasMark(marks, MarkType::sfzp)); + const auto marks = roundTripDynamic(StandardDynamic::sfzp); + CHECK(hasDynamic(marks, StandardDynamic::sfzp)); } T_END; diff --git a/src/private/mxtest/api/OtherMarksApiTest.cpp b/src/private/mxtest/api/OtherMarksApiTest.cpp index 3bd6183e5..41a620876 100644 --- a/src/private/mxtest/api/OtherMarksApiTest.cpp +++ b/src/private/mxtest/api/OtherMarksApiTest.cpp @@ -46,7 +46,9 @@ TEST(smuflOtherMarksRoundTrip, OtherMarksApi) addOtherMark(MarkType::otherArticulation, "articulation fallback", "articAccentAbove"); addOtherMark(MarkType::otherTechnical, "technique fallback", "brassMuteClosed"); addOtherMark(MarkType::otherOrnament, "ornament fallback", "ornamentTurnSlash"); - addOtherMark(MarkType::otherDynamics, "", "dynamicZ"); + + // An other-dynamics has no dedicated element, so it is a compound of one component. + marks.emplace_back(CompoundDynamicsData{{OtherDynamicsData{"", std::string{"dynamicZ"}}}}); const auto xml = mxtest::toXml(score); CHECK(xml.find("smufl=\"articAccentAbove\"") != std::string::npos); @@ -66,10 +68,15 @@ TEST(smuflOtherMarksRoundTrip, OtherMarksApi) 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; }); + [](const auto &item) { return item.markType == MarkType::dynamics; }); REQUIRE(dynamic != outMarks.end()); + REQUIRE(dynamic->choice.isCompoundDynamics()); + const auto components = dynamic->choice.compoundDynamics().components; + REQUIRE(components.size() == 1); + CHECK(components.front().other().smufl == std::optional{"dynamicZ"}); + CHECK(components.front().other().text.empty()); CHECK(dynamic->name.empty()); } @@ -80,6 +87,7 @@ TEST(markChoiceWrongKindFallbacks, OtherMarksApi) const MarkDataChoice choice; CHECK(!choice.otherMark().smufl.has_value()); CHECK(choice.compoundDynamics().components.empty()); + CHECK(choice.dynamic() == StandardDynamic::p); CHECK(choice.otherNotation().type == OtherNotationType::single); const DynamicsComponent standard{StandardDynamic::ff}; @@ -90,15 +98,91 @@ TEST(markChoiceWrongKindFallbacks, OtherMarksApi) T_END; -TEST(compoundDynamicsRoundTrip, OtherMarksApi) +// A lone standard symbol is one mark however it was built, so it is always stored as +// Kind::dynamic. A lone other-dynamics has no dedicated element and stays a compound of one. +TEST(singleStandardDynamicCollapses, OtherMarksApi) +{ + const MarkDataChoice fromCompound{CompoundDynamicsData{{StandardDynamic::ff}}}; + CHECK(fromCompound.kind() == MarkDataChoice::Kind::dynamic); + CHECK(fromCompound.dynamic() == StandardDynamic::ff); + CHECK(fromCompound == MarkDataChoice{StandardDynamic::ff}); + + const MarkDataChoice loneOther{CompoundDynamicsData{{OtherDynamicsData{"z", std::string{"dynamicZ"}}}}}; + CHECK(loneOther.kind() == MarkDataChoice::Kind::compoundDynamics); + REQUIRE(loneOther.compoundDynamics().components.size() == 1); + + const MarkDataChoice twoStandard{CompoundDynamicsData{{StandardDynamic::ff, StandardDynamic::p}}}; + CHECK(twoStandard.kind() == MarkDataChoice::Kind::compoundDynamics); + + const MarkData mark{StandardDynamic::ff}; + CHECK(mark.markType == MarkType::dynamics); + CHECK(mark.choice.dynamic() == StandardDynamic::ff); +} + +T_END; + +// name spells out the whole mark, the way articulations and fermatas name themselves. The writer +// ignores it -- the symbols that get written are the ones in choice. +TEST(dynamicNameSpellsTheMark, OtherMarksApi) { + CHECK_EQUAL("ff", toString(StandardDynamic::ff)); + CHECK_EQUAL("sfzp", toString(StandardDynamic::sfzp)); + CHECK_EQUAL("ff", MarkData{StandardDynamic::ff}.name); + CHECK_EQUAL("z", toString(DynamicsComponent{OtherDynamicsData{"z", std::string{"dynamicZ"}}})); + CHECK_EQUAL("ffz", + toString(CompoundDynamicsData{{StandardDynamic::ff, OtherDynamicsData{"z", std::string{"dynamicZ"}}}})); + auto score = otherMarksScoreWithNote(); - auto &mark = otherMarksNote(score).noteAttachmentData.marks.emplace_back(MarkType::compoundDynamics); + otherMarksNote(score).noteAttachmentData.marks.emplace_back(StandardDynamic::ffff); + 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().choice.dynamic() == StandardDynamic::ffff); + CHECK_EQUAL("ffff", outMarks.front().name); + + // A lone other-dynamics echoes its text, as it did before the symbol moved into choice. + auto otherScore = otherMarksScoreWithNote(); + otherMarksNote(otherScore) + .noteAttachmentData.marks.emplace_back(CompoundDynamicsData{{OtherDynamicsData{"z", std::string{"dynamicZ"}}}}); + const auto otherOut = mxtest::roundTrip(otherScore); + const auto &otherOutMarks = + otherOut.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; + REQUIRE(otherOutMarks.size() == 1); + CHECK_EQUAL("z", otherOutMarks.front().name); + REQUIRE(otherOutMarks.front().choice.isCompoundDynamics()); + CHECK_EQUAL("z", otherOutMarks.front().choice.compoundDynamics().components.front().other().text); + + // The CompoundDynamicsData constructor spells the mark out the same way the reader does, and a + // lone standard symbol reaches the same mark as the StandardDynamic constructor. + const MarkData built{CompoundDynamicsData{{StandardDynamic::ff, OtherDynamicsData{"z", std::string{"dynamicZ"}}}}}; + CHECK(built.markType == MarkType::dynamics); + CHECK_EQUAL("ffz", built.name); + CHECK(built.choice.isCompoundDynamics()); + CHECK(MarkData{CompoundDynamicsData{{StandardDynamic::ff}}} == MarkData{StandardDynamic::ff}); + + // A compound spells out every component in order. + auto compoundScore = otherMarksScoreWithNote(); + otherMarksNote(compoundScore) + .noteAttachmentData.marks.emplace_back( + CompoundDynamicsData{{StandardDynamic::ff, OtherDynamicsData{"z", std::string{"dynamicZ"}}}}); + const auto compoundOut = mxtest::roundTrip(compoundScore); + const auto &compoundMarks = + compoundOut.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteAttachmentData.marks; + REQUIRE(compoundMarks.size() == 1); + CHECK_EQUAL("ffz", compoundMarks.front().name); +} + +T_END; + +TEST(compoundDynamicsRoundTrip, OtherMarksApi) +{ + auto score = otherMarksScoreWithNote(); CompoundDynamicsData compound; compound.components.emplace_back(StandardDynamic::ff); compound.components.emplace_back(OtherDynamicsData{"z", std::string{"dynamicZ"}}); - mark.choice = std::move(compound); + otherMarksNote(score).noteAttachmentData.marks.emplace_back(std::move(compound)); const auto xml = mxtest::toXml(score); const auto dynamicsPosition = xml.find("