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