From a019b9866bdd8d54bf5a95f742a59879f4c92c72 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 07:35:44 -0500 Subject: [PATCH] Add direction and text formatting attributes to mx::api - WordsData, SymbolData and RehearsalData gain justify, mapping the attribute of the same name on , and . - DirectionData gains systemRelation, mapping the system attribute on and . - Enclosure in ApiCommon.h replaces RehearsalEnclosure and PercussionEnclosure, completing the enclosure-shape vocabulary and collapsing three conversion paths into one Converter table. The two old enum names are removed; the vocabularies are unchanged. --- src/include/mx/api/ApiCommon.h | 33 +++- src/include/mx/api/DirectionData.h | 15 +- src/include/mx/api/PercussionData.h | 29 +--- src/include/mx/api/RehearsalData.h | 30 ++-- src/include/mx/api/SymbolData.h | 18 +- src/include/mx/api/WordsData.h | 18 +- src/private/mx/impl/Converter.cpp | 64 ++++--- src/private/mx/impl/Converter.h | 15 +- src/private/mx/impl/DirectionReader.cpp | 64 +++---- src/private/mx/impl/DirectionReader.h | 1 + src/private/mx/impl/DirectionWriter.cpp | 63 ++++--- src/private/mxtest/api/DirectionDataTest.cpp | 91 +++++++++- .../api/DirectionMarksRoundTripTest.cpp | 162 +++++++++++++++++- .../mxtest/api/HarmonyExtrasApiTest.cpp | 34 ++++ .../mxtest/impl/DirectionWriterTest.cpp | 2 +- 15 files changed, 488 insertions(+), 151 deletions(-) diff --git a/src/include/mx/api/ApiCommon.h b/src/include/mx/api/ApiCommon.h index 7e03d6101..4827c916a 100644 --- a/src/include/mx/api/ApiCommon.h +++ b/src/include/mx/api/ApiCommon.h @@ -71,9 +71,11 @@ enum class MeasureNumbering system }; -// The attribute: whether this part's measure numbers are also, -// or only, associated with the system (as opposed to just this part). unspecified means the -// attribute is absent. +// Whether an item is associated with the system rather than only with the part it appears in -- +// the system attribute of (MeasureData) and of and +// (DirectionData). only... means the item is drawn on the top or bottom part of the system instead +// of this part; also... means it is drawn on both. unspecified means the attribute is absent. +// onlyBottom and alsoBottom are measure-numbering only; MusicXML has no bottom-of-system direction. enum class SystemRelation { unspecified, @@ -83,6 +85,31 @@ enum class SystemRelation alsoTop, alsoBottom }; + +// The shape drawn around a piece of text or a symbol -- MusicXML's enclosure attribute, carried by +// RehearsalData, WordsData, SymbolData and PercussionData. unspecified means the attribute is +// absent, which draws no enclosure; none states explicitly that there is none. A bracket is a +// rectangle with the bottom line missing, as is common in jazz notation, and an invertedBracket is +// one with the top line missing. +enum class Enclosure +{ + unspecified, + rectangle, + square, + oval, + circle, + bracket, + invertedBracket, + triangle, + diamond, + pentagon, + hexagon, + heptagon, + octagon, + nonagon, + decagon, + none +}; } // namespace api } // namespace mx diff --git a/src/include/mx/api/DirectionData.h b/src/include/mx/api/DirectionData.h index dea7b9465..7980f12dd 100644 --- a/src/include/mx/api/DirectionData.h +++ b/src/include/mx/api/DirectionData.h @@ -28,6 +28,14 @@ struct DirectionData int tickTimePosition; Placement placement; + // The system attribute of and : content that belongs to the whole system + // rather than to this part alone, such as a tempo mark drawn once above the top staff. onlyTop + // draws it only on the top part of the system, alsoTop on both this part and the top part, and + // none states explicitly that it belongs to this part alone. One value covers the direction + // types and the chords held here; both elements are written with it. SystemRelation::onlyBottom + // and ::alsoBottom exist only for measure numbering; either one here writes no attribute. + SystemRelation systemRelation; + // The source's , in divisions, or absent when it had none. An nudges only // where the direction is *drawn*, shifting it away from the note it is anchored to; it does not // move that anchor. tickTimePosition holds the anchor -- the musical location the direction @@ -73,9 +81,9 @@ struct DirectionData std::vector figuredBasses; DirectionData() - : tickTimePosition{0}, placement{Placement::unspecified}, offset{}, voice{VALUE_UNSPECIFIED}, - isStaffValueSpecified{true}, isSoundDataSpecified{false}, soundData{}, directionTypes{}, chords{}, - figuredBasses{} + : tickTimePosition{0}, placement{Placement::unspecified}, systemRelation{SystemRelation::unspecified}, offset{}, + voice{VALUE_UNSPECIFIED}, isStaffValueSpecified{true}, isSoundDataSpecified{false}, soundData{}, + directionTypes{}, chords{}, figuredBasses{} { } }; @@ -89,6 +97,7 @@ inline bool isDirectionDataEmpty(const DirectionData &directionData) MXAPI_EQUALS_BEGIN(DirectionData) MXAPI_EQUALS_MEMBER(tickTimePosition) MXAPI_EQUALS_MEMBER(placement) +MXAPI_EQUALS_MEMBER(systemRelation) MXAPI_EQUALS_MEMBER(offset) MXAPI_EQUALS_MEMBER(voice) MXAPI_EQUALS_MEMBER(isStaffValueSpecified) diff --git a/src/include/mx/api/PercussionData.h b/src/include/mx/api/PercussionData.h index 46ee9fb35..8fc25f55d 100644 --- a/src/include/mx/api/PercussionData.h +++ b/src/include/mx/api/PercussionData.h @@ -464,27 +464,6 @@ class PercussionDataChoice MXAPI_NOT_EQUALS_AND_VECTORS(PercussionDataChoice); -// How a percussion pictogram is enclosed, MusicXML's enclosure attribute on . -enum class PercussionEnclosure -{ - unspecified, - rectangle, - square, - oval, - circle, - bracket, - invertedBracket, - triangle, - diamond, - pentagon, - hexagon, - heptagon, - octagon, - nonagon, - decagon, - none -}; - // A percussion pictogram, MusicXML's element: a symbol for an unpitched instrument // or the implement striking it, used in percussion parts and legends. Several PercussionData in // one direction read as a group (for example a membrane pictogram followed by a beater). @@ -495,13 +474,17 @@ class PercussionData { public: PercussionDataChoice choice; - PercussionEnclosure enclosure; + + // A shape drawn around the pictogram. Enclosure::unspecified draws no enclosure; + // Enclosure::none states explicitly that there is none. + Enclosure enclosure; + PositionData positionData; FontData fontData; std::optional color; std::optional id; - PercussionData() : choice{}, enclosure{PercussionEnclosure::unspecified}, positionData{}, fontData{}, color{}, id{} + PercussionData() : choice{}, enclosure{Enclosure::unspecified}, positionData{}, fontData{}, color{}, id{} { } }; diff --git a/src/include/mx/api/RehearsalData.h b/src/include/mx/api/RehearsalData.h index e45f90166..73685bcfd 100644 --- a/src/include/mx/api/RehearsalData.h +++ b/src/include/mx/api/RehearsalData.h @@ -6,25 +6,13 @@ #include "mx/api/ApiCommon.h" #include "mx/api/ColorData.h" +#include "mx/api/FontData.h" #include "mx/api/PositionData.h" namespace mx { namespace api { -enum class RehearsalEnclosure -{ - unspecified, - rectangle, - square, - oval, - circle, - bracket, - triangle, - diamond, - none -}; - class RehearsalData { public: @@ -33,11 +21,20 @@ class RehearsalData bool isColorSpecified; ColorData colorData; FontData fontData; - RehearsalEnclosure enclosure; + + // A shape drawn around the text. Enclosure::unspecified draws no enclosure; Enclosure::none + // states explicitly that there is none. + Enclosure enclosure; + + // The `justify` attribute of ``: how the mark's lines sit relative to each other + // when its text runs to more than one line. `unspecified` means the attribute is absent. + // Distinct from the `halign` attribute, which is carried in `positionData.horizontalAlignment` + // and says which edge of the text the position refers to; MusicXML defines both. + HorizontalAlignment justify; RehearsalData() - : text{}, positionData{}, isColorSpecified{false}, colorData{}, fontData{}, - enclosure{RehearsalEnclosure::unspecified} + : text{}, positionData{}, isColorSpecified{false}, colorData{}, fontData{}, enclosure{Enclosure::unspecified}, + justify{HorizontalAlignment::unspecified} { } }; @@ -49,6 +46,7 @@ MXAPI_EQUALS_MEMBER(isColorSpecified) MXAPI_EQUALS_MEMBER(colorData) MXAPI_EQUALS_MEMBER(fontData) MXAPI_EQUALS_MEMBER(enclosure) +MXAPI_EQUALS_MEMBER(justify) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(RehearsalData); } // namespace api diff --git a/src/include/mx/api/SymbolData.h b/src/include/mx/api/SymbolData.h index ad403a60b..8b6d88742 100644 --- a/src/include/mx/api/SymbolData.h +++ b/src/include/mx/api/SymbolData.h @@ -8,7 +8,6 @@ #include "mx/api/ColorData.h" #include "mx/api/FontData.h" #include "mx/api/PositionData.h" -#include "mx/api/RehearsalData.h" #include #include @@ -32,11 +31,19 @@ class SymbolData FontData fontData; std::optional color; - // A shape drawn around the symbol. RehearsalEnclosure::unspecified draws no enclosure; - // RehearsalEnclosure::none states explicitly that there is none. - RehearsalEnclosure enclosure; + // A shape drawn around the symbol. Enclosure::unspecified draws no enclosure; Enclosure::none + // states explicitly that there is none. + Enclosure enclosure; - SymbolData() : smufl{}, positionData{}, fontData{}, color{}, enclosure{RehearsalEnclosure::unspecified} + // The `justify` attribute of ``. `unspecified` means the attribute is absent. Distinct + // from the `halign` attribute, which is carried in `positionData.horizontalAlignment` and says + // which edge of the glyph the position refers to; MusicXML defines both on ``, whose + // symbol-formatting attributes mirror the text-formatting ones on ``. + HorizontalAlignment justify; + + SymbolData() + : smufl{}, positionData{}, fontData{}, color{}, enclosure{Enclosure::unspecified}, + justify{HorizontalAlignment::unspecified} { } }; @@ -47,6 +54,7 @@ MXAPI_EQUALS_MEMBER(positionData) MXAPI_EQUALS_MEMBER(fontData) MXAPI_EQUALS_MEMBER(color) MXAPI_EQUALS_MEMBER(enclosure) +MXAPI_EQUALS_MEMBER(justify) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(SymbolData); } // namespace api diff --git a/src/include/mx/api/WordsData.h b/src/include/mx/api/WordsData.h index 055419239..80e558f38 100644 --- a/src/include/mx/api/WordsData.h +++ b/src/include/mx/api/WordsData.h @@ -8,7 +8,6 @@ #include "mx/api/ColorData.h" #include "mx/api/FontData.h" #include "mx/api/PositionData.h" -#include "mx/api/RehearsalData.h" namespace mx { @@ -26,13 +25,19 @@ class WordsData bool isColorSpecified; ColorData colorData; - // A shape drawn around the text. RehearsalEnclosure::unspecified draws no enclosure; - // RehearsalEnclosure::none states explicitly that there is none. - RehearsalEnclosure enclosure; + // A shape drawn around the text. Enclosure::unspecified draws no enclosure; Enclosure::none + // states explicitly that there is none. + Enclosure enclosure; + + // The `justify` attribute of ``: how the text lines up within itself when it spans + // more than one line. `unspecified` means the attribute is absent. Distinct from the `halign` + // attribute, which is carried in `positionData.horizontalAlignment` and aligns the whole text + // block against its anchor point; MusicXML defines both attributes on ``. + HorizontalAlignment justify; WordsData() - : text{}, positionData{}, fontData{}, isColorSpecified{false}, colorData{}, - enclosure{RehearsalEnclosure::unspecified} + : text{}, positionData{}, fontData{}, isColorSpecified{false}, colorData{}, enclosure{Enclosure::unspecified}, + justify{HorizontalAlignment::unspecified} { } }; @@ -44,6 +49,7 @@ MXAPI_EQUALS_MEMBER(fontData) MXAPI_EQUALS_MEMBER(isColorSpecified) MXAPI_EQUALS_MEMBER(colorData) MXAPI_EQUALS_MEMBER(enclosure) +MXAPI_EQUALS_MEMBER(justify) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(WordsData); } // namespace api diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index f8334dfc6..84357f7cb 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -300,6 +300,12 @@ const Converter::EnumMap Conver {core::SystemRelationNumber::alsoBottom(), api::SystemRelation::alsoBottom}, }; +const Converter::EnumMap Converter::directionSystemRelationMap = { + {core::SystemRelation::none(), api::SystemRelation::none}, + {core::SystemRelation::onlyTop(), api::SystemRelation::onlyTop}, + {core::SystemRelation::alsoTop(), api::SystemRelation::alsoTop}, +}; + const Converter::EnumMap Converter::technicalMarkMap = { // { core::TechnicalChoice::Kind::technical, // api::MarkType::unspecified }, @@ -1581,22 +1587,22 @@ const Converter::EnumMap Converter::tipDi {core::TipDirection::southwest(), api::TipDirection::southwest}, }; -const Converter::EnumMap Converter::percussionEnclosureMap = { - {core::EnclosureShape::rectangle(), api::PercussionEnclosure::rectangle}, - {core::EnclosureShape::square(), api::PercussionEnclosure::square}, - {core::EnclosureShape::oval(), api::PercussionEnclosure::oval}, - {core::EnclosureShape::circle(), api::PercussionEnclosure::circle}, - {core::EnclosureShape::bracket(), api::PercussionEnclosure::bracket}, - {core::EnclosureShape::invertedBracket(), api::PercussionEnclosure::invertedBracket}, - {core::EnclosureShape::triangle(), api::PercussionEnclosure::triangle}, - {core::EnclosureShape::diamond(), api::PercussionEnclosure::diamond}, - {core::EnclosureShape::pentagon(), api::PercussionEnclosure::pentagon}, - {core::EnclosureShape::hexagon(), api::PercussionEnclosure::hexagon}, - {core::EnclosureShape::heptagon(), api::PercussionEnclosure::heptagon}, - {core::EnclosureShape::octagon(), api::PercussionEnclosure::octagon}, - {core::EnclosureShape::nonagon(), api::PercussionEnclosure::nonagon}, - {core::EnclosureShape::decagon(), api::PercussionEnclosure::decagon}, - {core::EnclosureShape::none(), api::PercussionEnclosure::none}, +const Converter::EnumMap Converter::enclosureMap = { + {core::EnclosureShape::rectangle(), api::Enclosure::rectangle}, + {core::EnclosureShape::square(), api::Enclosure::square}, + {core::EnclosureShape::oval(), api::Enclosure::oval}, + {core::EnclosureShape::circle(), api::Enclosure::circle}, + {core::EnclosureShape::bracket(), api::Enclosure::bracket}, + {core::EnclosureShape::invertedBracket(), api::Enclosure::invertedBracket}, + {core::EnclosureShape::triangle(), api::Enclosure::triangle}, + {core::EnclosureShape::diamond(), api::Enclosure::diamond}, + {core::EnclosureShape::pentagon(), api::Enclosure::pentagon}, + {core::EnclosureShape::hexagon(), api::Enclosure::hexagon}, + {core::EnclosureShape::heptagon(), api::Enclosure::heptagon}, + {core::EnclosureShape::octagon(), api::Enclosure::octagon}, + {core::EnclosureShape::nonagon(), api::Enclosure::nonagon}, + {core::EnclosureShape::decagon(), api::Enclosure::decagon}, + {core::EnclosureShape::none(), api::Enclosure::none}, }; api::Step Converter::convert(core::Step inStep) const @@ -1805,6 +1811,24 @@ api::SystemRelation Converter::convertSystemRelation(core::SystemRelationNumber return findApiItem(systemRelationMap, api::SystemRelation::unspecified, value); } +std::optional Converter::convertDirectionSystemRelation(api::SystemRelation value) const +{ + const auto compare = [&value](const std::pair &v) { + return v.second == value; + }; + const auto it = std::find_if(directionSystemRelationMap.cbegin(), directionSystemRelationMap.cend(), compare); + if (it == directionSystemRelationMap.cend()) + { + return std::nullopt; + } + return it->first; +} + +api::SystemRelation Converter::convertDirectionSystemRelation(core::SystemRelation value) const +{ + return findApiItem(directionSystemRelationMap, api::SystemRelation::unspecified, value); +} + core::StemValue Converter::convert(api::Stem value) const { return findCoreItem(stemMap, core::StemValue::up(), value); @@ -2095,14 +2119,14 @@ core::TipDirection Converter::convert(api::TipDirection value) const return findCoreItem(tipDirectionMap, core::TipDirection::up(), value); } -api::PercussionEnclosure Converter::convert(core::EnclosureShape value) const +api::Enclosure Converter::convert(core::EnclosureShape value) const { - return findApiItem(percussionEnclosureMap, api::PercussionEnclosure::unspecified, value); + return findApiItem(enclosureMap, api::Enclosure::unspecified, value); } -core::EnclosureShape Converter::convert(api::PercussionEnclosure value) const +core::EnclosureShape Converter::convert(api::Enclosure value) const { - return findCoreItem(percussionEnclosureMap, core::EnclosureShape::none(), value); + return findCoreItem(enclosureMap, core::EnclosureShape::none(), value); } double Converter::convertToAlter(int semitones, double cents) diff --git a/src/private/mx/impl/Converter.h b/src/private/mx/impl/Converter.h index 83a2e5114..2d2629f5e 100644 --- a/src/private/mx/impl/Converter.h +++ b/src/private/mx/impl/Converter.h @@ -51,6 +51,7 @@ #include "mx/core/generated/StickLocation.h" #include "mx/core/generated/StickMaterial.h" #include "mx/core/generated/StickType.h" +#include "mx/core/generated/SystemRelation.h" #include "mx/core/generated/SystemRelationNumber.h" #include "mx/core/generated/TechnicalChoice.h" #include "mx/core/generated/TimeRelation.h" @@ -65,6 +66,7 @@ #include "mx/core/generated/YesNo.h" #include +#include #include #include @@ -143,6 +145,12 @@ class Converter core::SystemRelationNumber convertSystemRelation(api::SystemRelation value) const; api::SystemRelation convertSystemRelation(core::SystemRelationNumber value) const; + // uses a narrower vocabulary than : + // only-top, also-top and none. api::SystemRelation::onlyBottom and ::alsoBottom have no + // direction equivalent and, like unspecified, yield nullopt so the caller writes no attribute. + std::optional convertDirectionSystemRelation(api::SystemRelation value) const; + api::SystemRelation convertDirectionSystemRelation(core::SystemRelation value) const; + core::StemValue convert(api::Stem value) const; api::Stem convert(core::StemValue value) const; @@ -227,8 +235,8 @@ class Converter core::StickLocation convert(api::StickLocation value) const; api::TipDirection convert(core::TipDirection value) const; core::TipDirection convert(api::TipDirection value) const; - api::PercussionEnclosure convert(core::EnclosureShape value) const; - core::EnclosureShape convert(api::PercussionEnclosure value) const; + api::Enclosure convert(core::EnclosureShape value) const; + core::EnclosureShape convert(api::Enclosure value) const; static double convertToAlter(int semitones, double cents); static std::pair convertToSemitonesAndCents(double alter); @@ -256,6 +264,7 @@ class Converter const static EnumMap technicalMarkMap; const static EnumMap measureNumberingMap; const static EnumMap systemRelationMap; + const static EnumMap directionSystemRelationMap; const static EnumMap stemMap; const static EnumMap lineType; const static EnumMap wedgeMap; @@ -286,7 +295,7 @@ class Converter const static EnumMap stickMaterialMap; const static EnumMap stickLocationMap; const static EnumMap tipDirectionMap; - const static EnumMap percussionEnclosureMap; + const static EnumMap enclosureMap; private: template diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index 64f6f6c5b..564809845 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -112,6 +112,7 @@ api::DirectionData DirectionReader::getDirectionData() myOutDirectionData = initializeData(); parseOffset(); parsePlacement(); + parseSystemRelation(); parseValues(); return returnData(); } @@ -175,6 +176,24 @@ void DirectionReader::parsePlacement() } } +void DirectionReader::parseSystemRelation() +{ + if (myDirection) + { + if (myDirection->system().has_value()) + { + myOutDirectionData.systemRelation = myConverter.convertDirectionSystemRelation(*myDirection->system()); + } + } + else if (myHarmony) + { + if (myHarmony->system().has_value()) + { + myOutDirectionData.systemRelation = myConverter.convertDirectionSystemRelation(*myHarmony->system()); + } + } +} + void DirectionReader::parseValues() { if (myDirection) @@ -221,33 +240,6 @@ mx::api::DirectionData DirectionReader::returnData() return temp; } -// Maps a core enclosure-shape attribute to the api enum shared by rehearsals, words, and -// symbols. -api::RehearsalEnclosure directionReaderEnclosure(core::EnclosureShape::Tag tag) -{ - switch (tag) - { - case core::EnclosureShape::Tag::rectangle: - return api::RehearsalEnclosure::rectangle; - case core::EnclosureShape::Tag::square: - return api::RehearsalEnclosure::square; - case core::EnclosureShape::Tag::oval: - return api::RehearsalEnclosure::oval; - case core::EnclosureShape::Tag::circle: - return api::RehearsalEnclosure::circle; - case core::EnclosureShape::Tag::bracket: - return api::RehearsalEnclosure::bracket; - case core::EnclosureShape::Tag::triangle: - return api::RehearsalEnclosure::triangle; - case core::EnclosureShape::Tag::diamond: - return api::RehearsalEnclosure::diamond; - case core::EnclosureShape::Tag::none: - return api::RehearsalEnclosure::none; - default: - return api::RehearsalEnclosure::unspecified; - } -} - void DirectionReader::parseDirectionType(const core::DirectionType &directionType) { const auto &ch = directionType.choice(); @@ -369,7 +361,11 @@ void DirectionReader::parseRehearsal(const core::DirectionType &directionType) } if (rehearsal.enclosure().has_value()) { - outRehearsal.enclosure = directionReaderEnclosure(rehearsal.enclosure()->tag()); + outRehearsal.enclosure = myConverter.convert(*rehearsal.enclosure()); + } + if (rehearsal.justify().has_value()) + { + outRehearsal.justify = myConverter.convert(*rehearsal.justify()); } myOutDirectionData.directionTypes.emplace_back(api::DirectionChoice{std::move(outRehearsal)}); } @@ -426,7 +422,11 @@ void DirectionReader::parseWordsRun(const core::DirectionType &directionType) } if (symbolEl.enclosure().has_value()) { - outSymbol.enclosure = directionReaderEnclosure(symbolEl.enclosure()->tag()); + outSymbol.enclosure = myConverter.convert(*symbolEl.enclosure()); + } + if (symbolEl.justify().has_value()) + { + outSymbol.justify = myConverter.convert(*symbolEl.justify()); } run.emplace_back(std::move(outSymbol)); continue; @@ -443,7 +443,11 @@ void DirectionReader::parseWordsRun(const core::DirectionType &directionType) outWords.fontData = getFontData(wordEl); if (wordEl.enclosure().has_value()) { - outWords.enclosure = directionReaderEnclosure(wordEl.enclosure()->tag()); + outWords.enclosure = myConverter.convert(*wordEl.enclosure()); + } + if (wordEl.justify().has_value()) + { + outWords.justify = myConverter.convert(*wordEl.justify()); } run.emplace_back(std::move(outWords)); } diff --git a/src/private/mx/impl/DirectionReader.h b/src/private/mx/impl/DirectionReader.h index e052c4fac..7b252224d 100644 --- a/src/private/mx/impl/DirectionReader.h +++ b/src/private/mx/impl/DirectionReader.h @@ -40,6 +40,7 @@ class DirectionReader mx::api::DirectionData initializeData(); void parseOffset(); void parsePlacement(); + void parseSystemRelation(); void parseValues(); mx::api::DirectionData returnData(); void parseStaffIndex(); diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index d528dc722..704c2fca1 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -162,6 +162,10 @@ std::vector DirectionWriter::getDirectionLikeThings() direction.setPlacement(myConverter.convert(myDirectionData.placement)); } + // nullopt for unspecified, and for the bottom-of-system values that only measure numbering has. + // carries the same attribute; createHarmonyElements writes it there too. + direction.setSystem(myConverter.convertDirectionSystemRelation(myDirectionData.systemRelation)); + if (myDirectionData.isStaffValueSpecified || myCursor.staffIndex != 0) { direction.setStaff(myCursor.staffIndex + 1); @@ -710,34 +714,6 @@ void DirectionWriter::emitTempo(const api::TempoData &tempo, core::Direction &di addDirectionType(std::move(dt), direction); } -// Maps the api enclosure enum shared by rehearsals, words, and symbols to the core -// enclosure-shape attribute; nullopt (for unspecified) emits no attribute. -static std::optional directionWriterEnclosure(api::RehearsalEnclosure enclosure) -{ - switch (enclosure) - { - case api::RehearsalEnclosure::rectangle: - return core::EnclosureShape::rectangle(); - case api::RehearsalEnclosure::square: - return core::EnclosureShape::square(); - case api::RehearsalEnclosure::oval: - return core::EnclosureShape::oval(); - case api::RehearsalEnclosure::circle: - return core::EnclosureShape::circle(); - case api::RehearsalEnclosure::bracket: - return core::EnclosureShape::bracket(); - case api::RehearsalEnclosure::triangle: - return core::EnclosureShape::triangle(); - case api::RehearsalEnclosure::diamond: - return core::EnclosureShape::diamond(); - case api::RehearsalEnclosure::none: - return core::EnclosureShape::none(); - case api::RehearsalEnclosure::unspecified: - default: - return std::nullopt; - } -} - void DirectionWriter::emitWordsRun(const std::vector &inRun, core::Direction &direction) { // An empty run cannot be expressed (the schema requires at least one words or symbol) and is @@ -765,7 +741,14 @@ void DirectionWriter::emitWordsRun(const std::vector &inRun, c { setAttributesFromColorData(*symbolData.color, outSymbol); } - outSymbol.setEnclosure(directionWriterEnclosure(symbolData.enclosure)); + if (symbolData.enclosure != api::Enclosure::unspecified) + { + outSymbol.setEnclosure(myConverter.convert(symbolData.enclosure)); + } + if (symbolData.justify != api::HorizontalAlignment::unspecified) + { + outSymbol.setJustify(myConverter.convert(symbolData.justify)); + } choiceItem = core::DirectionTypeChoiceChoice::symbol(std::move(outSymbol)); } else @@ -779,7 +762,14 @@ void DirectionWriter::emitWordsRun(const std::vector &inRun, c { setAttributesFromColorData(wordsData.colorData, outWords); } - outWords.setEnclosure(directionWriterEnclosure(wordsData.enclosure)); + if (wordsData.enclosure != api::Enclosure::unspecified) + { + outWords.setEnclosure(myConverter.convert(wordsData.enclosure)); + } + if (wordsData.justify != api::HorizontalAlignment::unspecified) + { + outWords.setJustify(myConverter.convert(wordsData.justify)); + } choiceItem = core::DirectionTypeChoiceChoice::words(std::move(outWords)); } @@ -853,7 +843,14 @@ void DirectionWriter::emitRehearsal(const api::RehearsalData &item, core::Direct { setAttributesFromColorData(item.colorData, rehearsal); } - rehearsal.setEnclosure(directionWriterEnclosure(item.enclosure)); + if (item.enclosure != api::Enclosure::unspecified) + { + rehearsal.setEnclosure(myConverter.convert(item.enclosure)); + } + if (item.justify != api::HorizontalAlignment::unspecified) + { + rehearsal.setJustify(myConverter.convert(item.justify)); + } core::DirectionType dt{}; dt.setChoice(core::DirectionTypeChoice::rehearsal(core::OneOrMore{std::move(rehearsal)})); addDirectionType(std::move(dt), direction); @@ -1280,7 +1277,7 @@ void DirectionWriter::emitPercussion(const api::PercussionData &item, core::Dire { core::Percussion percussion{}; percussion.setChoice(createPercussionChoice(item.choice)); - if (item.enclosure != api::PercussionEnclosure::unspecified) + if (item.enclosure != api::Enclosure::unspecified) { percussion.setEnclosure(myConverter.convert(item.enclosure)); } @@ -1452,6 +1449,8 @@ std::vector DirectionWriter::createHarmonyElements(int in harmony.setStaff(myCursor.staffIndex + 1); } + harmony.setSystem(myConverter.convertDirectionSystemRelation(myDirectionData.systemRelation)); + const auto &chords = myDirectionData.chords; auto chordIter = chords.cbegin(); diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index 1730b16d7..2ba101dfb 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -263,7 +263,7 @@ TEST(RehearsalSyntheticFileRead, DirectionData) REQUIRE(directions.front().directionTypes.front().isRehearsal()); const auto rehearsal = directions.front().directionTypes.front().rehearsal(); CHECK_EQUAL("x", rehearsal.text); - CHECK(RehearsalEnclosure::rectangle == rehearsal.enclosure); + CHECK(Enclosure::rectangle == rehearsal.enclosure); } T_END; @@ -291,7 +291,7 @@ TEST(RehearsalRoundTripXml, DirectionData) RehearsalData rehearsal; rehearsal.text = "B"; - rehearsal.enclosure = RehearsalEnclosure::rectangle; + rehearsal.enclosure = Enclosure::rectangle; rehearsal.fontData.fontFamily = {"Times New Roman"}; rehearsal.fontData.style = FontStyle::normal; rehearsal.fontData.weight = FontWeight::bold; @@ -315,13 +315,13 @@ TEST(RehearsalRoundTripXml, DirectionData) REQUIRE(rdirections.front().directionTypes.front().isRehearsal()); const auto outRehearsal = rdirections.front().directionTypes.front().rehearsal(); CHECK_EQUAL("B", outRehearsal.text); - CHECK(RehearsalEnclosure::rectangle == outRehearsal.enclosure); + CHECK(Enclosure::rectangle == outRehearsal.enclosure); CHECK(FontWeight::bold == outRehearsal.fontData.weight); } T_END; -// Verify that a rehearsal with no enclosure set (RehearsalEnclosure::unspecified) does not emit +// Verify that a rehearsal with no enclosure set (Enclosure::unspecified) does not emit // an enclosure attribute in the serialized XML, and that the field round-trips as unspecified. TEST(RehearsalUnspecifiedEnclosureNoPhantomAttribute, DirectionData) { @@ -358,7 +358,88 @@ TEST(RehearsalUnspecifiedEnclosureNoPhantomAttribute, DirectionData) REQUIRE(rdirections.size() == 1); REQUIRE(rdirections.front().directionTypes.size() == 1); REQUIRE(rdirections.front().directionTypes.front().isRehearsal()); - CHECK(RehearsalEnclosure::unspecified == rdirections.front().directionTypes.front().rehearsal().enclosure); + CHECK(Enclosure::unspecified == rdirections.front().directionTypes.front().rehearsal().enclosure); +} + +T_END; + +// A rehearsal mark's justify attribute round-trips and stays independent of halign, which lives in +// positionData. An unspecified justify emits no attribute. +TEST(RehearsalJustify, DirectionData) +{ + ScoreData oscore; + oscore.ticksPerQuarter = 10; + oscore.parts.emplace_back(); + auto &opart = oscore.parts.back(); + opart.measures.emplace_back(); + auto &omeasure = opart.measures.back(); + omeasure.staves.emplace_back(); + auto &ostaff = omeasure.staves.back(); + auto &ovoice = ostaff.voices[0]; + + NoteData onote{}; + onote.tickTimePosition = 0; + onote.durationData.durationTimeTicks = 10; + onote.durationData.durationName = DurationName::quarter; + ovoice.notes.push_back(onote); + + RehearsalData rehearsal; + rehearsal.text = "D"; + rehearsal.justify = HorizontalAlignment::center; + rehearsal.positionData.horizontalAlignment = HorizontalAlignment::right; + + DirectionData directionData; + directionData.tickTimePosition = 0; + directionData.directionTypes.emplace_back(DirectionChoice{rehearsal}); + ostaff.directions.push_back(directionData); + + const auto rscore = mxtest::roundTrip(oscore); + const auto &rdirections = rscore.parts.front().measures.front().staves.front().directions; + REQUIRE(rdirections.size() == 1); + REQUIRE(rdirections.front().directionTypes.size() == 1); + REQUIRE(rdirections.front().directionTypes.front().isRehearsal()); + const auto outRehearsal = rdirections.front().directionTypes.front().rehearsal(); + CHECK(HorizontalAlignment::center == outRehearsal.justify); + CHECK(HorizontalAlignment::right == outRehearsal.positionData.horizontalAlignment); +} + +T_END; + +TEST(RehearsalUnspecifiedJustifyNoPhantomAttribute, DirectionData) +{ + ScoreData oscore; + oscore.ticksPerQuarter = 10; + oscore.parts.emplace_back(); + auto &opart = oscore.parts.back(); + opart.measures.emplace_back(); + auto &omeasure = opart.measures.back(); + omeasure.staves.emplace_back(); + auto &ostaff = omeasure.staves.back(); + auto &ovoice = ostaff.voices[0]; + + NoteData onote{}; + onote.tickTimePosition = 0; + onote.durationData.durationTimeTicks = 10; + onote.durationData.durationName = DurationName::quarter; + ovoice.notes.push_back(onote); + + RehearsalData rehearsal; + rehearsal.text = "E"; + + DirectionData directionData; + directionData.tickTimePosition = 0; + directionData.directionTypes.emplace_back(DirectionChoice{rehearsal}); + ostaff.directions.push_back(directionData); + + const auto xml = mxtest::toXml(oscore); + CHECK(xml.find("justify") == std::string::npos); + + const auto rscore = mxtest::roundTrip(oscore); + const auto &rdirections = rscore.parts.front().measures.front().staves.front().directions; + REQUIRE(rdirections.size() == 1); + REQUIRE(rdirections.front().directionTypes.size() == 1); + REQUIRE(rdirections.front().directionTypes.front().isRehearsal()); + CHECK(HorizontalAlignment::unspecified == rdirections.front().directionTypes.front().rehearsal().justify); } T_END; diff --git a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp index 1db723550..05ff00539 100644 --- a/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp +++ b/src/private/mxtest/api/DirectionMarksRoundTripTest.cpp @@ -343,7 +343,7 @@ TEST(PercussionGlass, DirectionMarksRoundTrip) GlassPercussion glass; glass.value = GlassInstrument::windChimes; percussion.choice = PercussionDataChoice{glass}; - percussion.enclosure = PercussionEnclosure::rectangle; + percussion.enclosure = Enclosure::rectangle; direction.directionTypes.emplace_back(DirectionChoice{percussion}); const auto directions = roundTripDirectionData(direction); REQUIRE(directions.size() == 1); @@ -352,7 +352,7 @@ TEST(PercussionGlass, DirectionMarksRoundTrip) const auto out = directions.front().directionTypes.front().percussion(); REQUIRE(out.choice.isGlass()); CHECK(out.choice.glass().value == GlassInstrument::windChimes); - CHECK(out.enclosure == PercussionEnclosure::rectangle); + CHECK(out.enclosure == Enclosure::rectangle); } T_END; @@ -588,7 +588,7 @@ TEST(WordsEnclosure, DirectionMarksRoundTrip) DirectionData direction; WordsData words; words.text = "boxed"; - words.enclosure = RehearsalEnclosure::rectangle; + words.enclosure = Enclosure::rectangle; direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); const auto directions = roundTripDirectionData(direction); REQUIRE(directions.size() == 1); @@ -597,7 +597,161 @@ TEST(WordsEnclosure, DirectionMarksRoundTrip) const auto outRun = directions.front().directionTypes.front().wordsRun(); REQUIRE(outRun.size() == 1); REQUIRE(outRun.front().isWords()); - CHECK(outRun.front().words().enclosure == RehearsalEnclosure::rectangle); + CHECK(outRun.front().words().enclosure == Enclosure::rectangle); +} + +T_END; + +// Every shape in the enclosure vocabulary reaches the wire and comes back. Keep this list complete: +// a value the conversion switches do not map is silently dropped, which is what it guards against. +TEST(WordsEnclosureAllShapes, DirectionMarksRoundTrip) +{ + const std::vector shapes{ + Enclosure::rectangle, Enclosure::square, Enclosure::oval, Enclosure::circle, Enclosure::bracket, + Enclosure::invertedBracket, Enclosure::triangle, Enclosure::diamond, Enclosure::pentagon, Enclosure::hexagon, + Enclosure::heptagon, Enclosure::octagon, Enclosure::nonagon, Enclosure::decagon, Enclosure::none}; + for (const auto shape : shapes) + { + DirectionData direction; + WordsData words; + words.text = "boxed"; + words.enclosure = shape; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().directionTypes.size() == 1); + REQUIRE(directions.front().directionTypes.front().isWordsRun()); + const auto outRun = directions.front().directionTypes.front().wordsRun(); + REQUIRE(outRun.size() == 1); + REQUIRE(outRun.front().isWords()); + CHECK(outRun.front().words().enclosure == shape); + } +} + +T_END; + +// The justify attribute of round-trips, and is independent of halign. +TEST(WordsJustify, DirectionMarksRoundTrip) +{ + DirectionData direction; + WordsData words; + words.text = "a tempo"; + words.justify = HorizontalAlignment::center; + words.positionData.horizontalAlignment = HorizontalAlignment::right; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().directionTypes.size() == 1); + REQUIRE(directions.front().directionTypes.front().isWordsRun()); + const auto outRun = directions.front().directionTypes.front().wordsRun(); + REQUIRE(outRun.size() == 1); + REQUIRE(outRun.front().isWords()); + CHECK(outRun.front().words().justify == HorizontalAlignment::center); + CHECK(outRun.front().words().positionData.horizontalAlignment == HorizontalAlignment::right); +} + +T_END; + +// carries justify and enclosure just as does; both survive within a run. +TEST(SymbolJustify, DirectionMarksRoundTrip) +{ + DirectionData direction; + SymbolData symbol; + symbol.smufl = "arrowBlackUp"; + symbol.justify = HorizontalAlignment::right; + symbol.enclosure = Enclosure::hexagon; + symbol.positionData.horizontalAlignment = HorizontalAlignment::left; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{symbol}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().directionTypes.size() == 1); + REQUIRE(directions.front().directionTypes.front().isWordsRun()); + const auto outRun = directions.front().directionTypes.front().wordsRun(); + REQUIRE(outRun.size() == 1); + REQUIRE(outRun.front().isSymbol()); + const auto outSymbol = outRun.front().symbol(); + CHECK_EQUAL("arrowBlackUp", outSymbol.smufl); + CHECK(outSymbol.justify == HorizontalAlignment::right); + CHECK(outSymbol.enclosure == Enclosure::hexagon); + CHECK(outSymbol.positionData.horizontalAlignment == HorizontalAlignment::left); +} + +T_END; + +// An unspecified justify emits no attribute and reads back as unspecified, for words and symbols. +TEST(SymbolJustifyUnspecified, DirectionMarksRoundTrip) +{ + DirectionData direction; + SymbolData symbol; + symbol.smufl = "arrowBlackUp"; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{symbol}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().directionTypes.size() == 1); + REQUIRE(directions.front().directionTypes.front().isWordsRun()); + const auto outRun = directions.front().directionTypes.front().wordsRun(); + REQUIRE(outRun.size() == 1); + REQUIRE(outRun.front().isSymbol()); + CHECK(outRun.front().symbol().justify == HorizontalAlignment::unspecified); +} + +T_END; + +// An unspecified justify emits no attribute and reads back as unspecified. +TEST(WordsJustifyUnspecified, DirectionMarksRoundTrip) +{ + DirectionData direction; + WordsData words; + words.text = "a tempo"; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().directionTypes.size() == 1); + REQUIRE(directions.front().directionTypes.front().isWordsRun()); + const auto outRun = directions.front().directionTypes.front().wordsRun(); + REQUIRE(outRun.size() == 1); + REQUIRE(outRun.front().isWords()); + CHECK(outRun.front().words().justify == HorizontalAlignment::unspecified); +} + +T_END; + +// The attribute round-trips for each value the direction vocabulary has. +TEST(DirectionSystemRelation, DirectionMarksRoundTrip) +{ + const std::vector values{SystemRelation::onlyTop, SystemRelation::alsoTop, SystemRelation::none}; + for (const auto value : values) + { + DirectionData direction; + direction.systemRelation = value; + WordsData words; + words.text = "Allegro"; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + CHECK(directions.front().systemRelation == value); + } +} + +T_END; + +// A direction left at the default writes no system attribute; the measure-numbering-only bottom +// values have no direction equivalent and are dropped rather than written as something else. +TEST(DirectionSystemRelationUnwritable, DirectionMarksRoundTrip) +{ + const std::vector values{SystemRelation::unspecified, SystemRelation::onlyBottom, + SystemRelation::alsoBottom}; + for (const auto value : values) + { + DirectionData direction; + direction.systemRelation = value; + WordsData words; + words.text = "Allegro"; + direction.directionTypes.emplace_back(DirectionChoice{std::vector{WordsChoice{words}}}); + const auto directions = roundTripDirectionData(direction); + REQUIRE(directions.size() == 1); + CHECK(directions.front().systemRelation == SystemRelation::unspecified); + } } T_END; diff --git a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp index 40135a495..96647a783 100644 --- a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp +++ b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp @@ -111,4 +111,38 @@ TEST(harmonyNumeralRoundTrip, HarmonyExtrasApi) T_END; +// carries the same system attribute as ; a harmony-only direction writes it +// and reads it back. +TEST(harmonySystemRelationRoundTrip, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::major; + auto &direction = score.parts.front().measures.front().staves.front().directions.front(); + direction.systemRelation = SystemRelation::alsoTop; + + const auto out = mxtest::roundTrip(score); + const auto &outDirection = out.parts.front().measures.front().staves.front().directions.front(); + CHECK(SystemRelation::alsoTop == outDirection.systemRelation); + CHECK(Step::c == firstChord(out).root); +} + +T_END; + +// A harmony-only direction left at the default writes no system attribute. +TEST(harmonySystemRelationUnspecified, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::major; + + const auto out = mxtest::roundTrip(score); + const auto &outDirection = out.parts.front().measures.front().staves.front().directions.front(); + CHECK(SystemRelation::unspecified == outDirection.systemRelation); +} + +T_END; + #endif diff --git a/src/private/mxtest/impl/DirectionWriterTest.cpp b/src/private/mxtest/impl/DirectionWriterTest.cpp index 28410e8a2..4b41145f5 100644 --- a/src/private/mxtest/impl/DirectionWriterTest.cpp +++ b/src/private/mxtest/impl/DirectionWriterTest.cpp @@ -212,7 +212,7 @@ TEST(rehearsalRoundTrip, DirectionWriter) rehearsal.colorData.blue = 0; rehearsal.colorData.isAlphaSpecified = true; rehearsal.colorData.alpha = 255; - rehearsal.enclosure = api::RehearsalEnclosure::square; + rehearsal.enclosure = api::Enclosure::square; api::DirectionData directionData; directionData.directionTypes.emplace_back(api::DirectionChoice{rehearsal});