From 5fe0911757652c2dd992ac82f6bacc699fb77053 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 06:22:13 -0500 Subject: [PATCH 1/7] Add fields for `` and ` ` to `MeasureData`. --- src/include/mx/api/MeasureData.h | 16 ++++ src/private/mx/impl/MeasureReader.cpp | 11 +++ src/private/mx/impl/MeasureWriter.cpp | 13 +++ src/private/mxtest/api/MeasureDataTest.cpp | 106 +++++++++++++++++++++ 4 files changed, 146 insertions(+) diff --git a/src/include/mx/api/MeasureData.h b/src/include/mx/api/MeasureData.h index c7b029f2b..8a6cde682 100644 --- a/src/include/mx/api/MeasureData.h +++ b/src/include/mx/api/MeasureData.h @@ -13,6 +13,7 @@ #include "mx/api/TransposeData.h" #include +#include #include namespace mx @@ -51,6 +52,15 @@ class MeasureData // about measure numbering can be defined using the measure-numbering element. std::string number; + // The measure number as it should appear on the page, when that differs from the 'number' + // above. 'number' identifies the measure (and aligns it with the same measure in other parts), + // while this is purely what the engraver prints. Set it when the printed numbering does not + // follow the identifying numbering -- a second ending that restarts at 8, an editor's numbering + // like "12a", or a rehearsal-driven relabeling. Leave it absent and the printed number is the + // 'number' value. An empty string is not written; use implicit = Bool::yes to suppress the + // number entirely, which is also what makes engravers ignore this field. + std::optional displayedNumber; + // The measure-numbering-value type describes how measure numbers are displayed on this part: // no numbers, numbers every measure, or numbers every system. MeasureNumbering measureNumbering; @@ -65,6 +75,10 @@ class MeasureData // The attribute; see SystemRelation. SystemRelation measureNumberingSystemRelation; + // Which staff of the part the measure number is vertically positioned against, zero-based from + // the top staff. Meaningful only when measureNumbering != unspecified. Absent means the top staff. + std::optional measureNumberingStaffIndex; + // a number greater than zero indicates that this measure is the beginning of a mult-measure // rest that will last for the indicated number of measures. following measures will be affected // by this. @@ -112,10 +126,12 @@ MXAPI_EQUALS_MEMBER(staves) MXAPI_EQUALS_MEMBER(timeSignature) MXAPI_EQUALS_MEMBER(staffTimeSignatures) MXAPI_EQUALS_MEMBER(number) +MXAPI_EQUALS_MEMBER(displayedNumber) MXAPI_EQUALS_MEMBER(measureNumbering) MXAPI_EQUALS_MEMBER(measureNumberingMultipleRestAlways) MXAPI_EQUALS_MEMBER(measureNumberingMultipleRestRange) MXAPI_EQUALS_MEMBER(measureNumberingSystemRelation) +MXAPI_EQUALS_MEMBER(measureNumberingStaffIndex) MXAPI_EQUALS_MEMBER(multiMeasureRest) MXAPI_EQUALS_MEMBER(multiMeasureRestUseSymbols) MXAPI_EQUALS_MEMBER(implicit) diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index f6fa4f001..2041cd105 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/src/private/mx/impl/MeasureReader.cpp @@ -151,6 +151,11 @@ std::pair> MeasureReader::ge myOutMeasureData.number = ""; } + if (myPartwiseMeasure.text().has_value()) + { + myOutMeasureData.displayedNumber = myPartwiseMeasure.text()->value(); + } + if (myPartwiseMeasure.width().has_value()) { myOutMeasureData.width = static_cast(myPartwiseMeasure.width()->value().value()); @@ -883,6 +888,12 @@ void MeasureReader::parsePrint(const core::Print &inMxPrint) const myOutMeasureData.measureNumberingSystemRelation = myConverter.convertSystemRelation(*measureNumbering.system()); } + + if (measureNumbering.staff().has_value()) + { + // core staff numbers are one-based; the api index is zero-based. + myOutMeasureData.measureNumberingStaffIndex = measureNumbering.staff()->value() - 1; + } } } diff --git a/src/private/mx/impl/MeasureWriter.cpp b/src/private/mx/impl/MeasureWriter.cpp index d463896f0..0f651348f 100644 --- a/src/private/mx/impl/MeasureWriter.cpp +++ b/src/private/mx/impl/MeasureWriter.cpp @@ -16,6 +16,7 @@ #include "mx/core/generated/LeftRightMarginsGroup.h" #include "mx/core/generated/MarginType.h" #include "mx/core/generated/MeasureNumbering.h" +#include "mx/core/generated/MeasureText.h" #include "mx/core/generated/MusicDataChoice.h" #include "mx/core/generated/PageLayout.h" #include "mx/core/generated/PageLayoutGroup.h" @@ -88,6 +89,12 @@ void MeasureWriter::writeMeasureGlobals() myOutMeasure.setNumber(std::to_string(myHistory.getCursor().measureIndex + 1)); } + // is not legal MusicXML; an empty displayedNumber means "no override". + if (myMeasureData.displayedNumber.has_value() && !myMeasureData.displayedNumber->empty()) + { + myOutMeasure.setText(core::MeasureText{*myMeasureData.displayedNumber}); + } + if (myMeasureData.width >= 0.0) { myOutMeasure.setWidth(core::Tenths{core::Decimal{static_cast(myMeasureData.width)}}); @@ -453,6 +460,12 @@ void MeasureWriter::writeMeasureNumbering() outMeasureNumbering.setSystem(myConverter.convertSystemRelation(myMeasureData.measureNumberingSystemRelation)); } + if (myMeasureData.measureNumberingStaffIndex.has_value()) + { + // the api index is zero-based; core staff numbers are one-based. + outMeasureNumbering.setStaff(core::StaffNumber{*myMeasureData.measureNumberingStaffIndex + 1}); + } + outPrint.setMeasureNumbering(std::move(outMeasureNumbering)); if (printIndex >= 0) diff --git a/src/private/mxtest/api/MeasureDataTest.cpp b/src/private/mxtest/api/MeasureDataTest.cpp index 630d54d00..c1444cd9f 100644 --- a/src/private/mxtest/api/MeasureDataTest.cpp +++ b/src/private/mxtest/api/MeasureDataTest.cpp @@ -332,6 +332,112 @@ TEST(measureNumberingUnspecifiedOmitsElement, MeasureData) T_END; +TEST(measureNumberingStaffRoundTrip, MeasureData) +{ + ScoreData score; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.measureNumbering = MeasureNumbering::system; + measure.measureNumberingStaffIndex = 1; + measure.staves.emplace_back(); + measure.staves.back().voices[0].notes.emplace_back(); + measure.staves.emplace_back(); + measure.staves.back().voices[0].notes.emplace_back(); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("staff=\"2\"") != std::string::npos); + + const auto outScore = mxtest::fromXml(xml); + const auto &outMeasure = outScore.parts.front().measures.front(); + REQUIRE(outMeasure.measureNumberingStaffIndex.has_value()); + CHECK_EQUAL(1, *outMeasure.measureNumberingStaffIndex); +} + +T_END; + +TEST(measureNumberingStaffAbsentOmitsAttribute, MeasureData) +{ + ScoreData score; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.measureNumbering = MeasureNumbering::system; + measure.staves.emplace_back(); + measure.staves.back().voices[0].notes.emplace_back(); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find(" 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}); From 71d78b1e650fccc9d2a30e2a9c845fee777c8199 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 09:42:47 -0500 Subject: [PATCH 3/7] Add clef additional attribute and fix dropped after-barline in mx::api ClefData::location was read from clef@after-barline but never written. Adds Bool ClefData::additional and maps both attributes in both directions. --- src/include/mx/api/ClefData.h | 12 ++ src/private/mx/api/ClefData.cpp | 2 +- src/private/mx/impl/MeasureReader.cpp | 5 + src/private/mx/impl/PropertiesWriter.cpp | 16 +++ src/private/mxtest/api/ClefDataTest.cpp | 171 +++++++++++++++++++++++ 5 files changed, 205 insertions(+), 1 deletion(-) diff --git a/src/include/mx/api/ClefData.h b/src/include/mx/api/ClefData.h index 2f0fd89c6..c4feba6cd 100644 --- a/src/include/mx/api/ClefData.h +++ b/src/include/mx/api/ClefData.h @@ -21,6 +21,11 @@ enum class ClefSymbol jianpu }; +// Where a clef sits relative to its measure. A clef at the start of a measure normally prints +// before the barline; beforeBarline and afterBarline choose between the two, which matters for cue +// clefs and for the clef that resumes after a repeated section. midMeasure describes a clef change +// partway through the measure, positioned by tickTimePosition. unspecified leaves the choice to the +// notation program. enum class ClefLocation { unspecified, @@ -53,6 +58,12 @@ class ClefData bool isOctaveChangeSpecified; int tickTimePosition; ClefLocation location; + // Marks a supplementary clef placed on the staff alongside the regular one - a cue clef, or a + // second clef in effect at the same time. MusicXML writes this as clef@additional="yes"; such a + // clef sits at a non-standard line position, is not restated at the start of each system, and + // notation software disregards its line. unspecified (the default) omits the attribute, which + // describes an ordinary clef. + Bool additional; // Visibility of the clef via the MusicXML print-object attribute. // unspecified -> omit the attribute, yes/no -> write print-object verbatim. Bool printObject; @@ -86,6 +97,7 @@ MXAPI_EQUALS_MEMBER(octaveChange) MXAPI_EQUALS_MEMBER(isOctaveChangeSpecified) MXAPI_EQUALS_MEMBER(tickTimePosition) MXAPI_EQUALS_MEMBER(location) +MXAPI_EQUALS_MEMBER(additional) MXAPI_EQUALS_MEMBER(printObject) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(ClefData); diff --git a/src/private/mx/api/ClefData.cpp b/src/private/mx/api/ClefData.cpp index 4219b7dc6..ccd1e103f 100644 --- a/src/private/mx/api/ClefData.cpp +++ b/src/private/mx/api/ClefData.cpp @@ -12,7 +12,7 @@ namespace api ClefData::ClefData() : writeStaffNumber{Bool::unspecified}, symbol{DEFAULT_CLEF_SYMBOL}, line{DEFAULT_CLEF_LINE}, isLineSpecified{true}, octaveChange{DEFAULT_CLEF_OCTAVE_CHANGE}, isOctaveChangeSpecified{true}, tickTimePosition{0}, - location{ClefLocation::unspecified}, printObject{Bool::unspecified} + location{ClefLocation::unspecified}, additional{Bool::unspecified}, printObject{Bool::unspecified} { } diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index f6fa4f001..ff689f064 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/src/private/mx/impl/MeasureReader.cpp @@ -1114,6 +1114,11 @@ void MeasureReader::importClef(const core::Clef &inClef) const clefData.isOctaveChangeSpecified = false; } + if (inClef.additional().has_value()) + { + clefData.additional = converter.convert(*inClef.additional()); + } + if (inClef.printObject().has_value()) { clefData.printObject = converter.convert(*inClef.printObject()); diff --git a/src/private/mx/impl/PropertiesWriter.cpp b/src/private/mx/impl/PropertiesWriter.cpp index fdf165197..bd1ee6894 100644 --- a/src/private/mx/impl/PropertiesWriter.cpp +++ b/src/private/mx/impl/PropertiesWriter.cpp @@ -354,6 +354,22 @@ void PropertiesWriter::writeClef(int staffIndex, const api::ClefData &inClefData cg.setClefOctaveChange(inClefData.octaveChange); } + if (inClefData.additional != api::Bool::unspecified) + { + mxClef.setAdditional(converter.convert(inClefData.additional)); + } + + // after-barline states which side of the barline a start-of-measure clef prints on. MusicXML + // ignores it on mid-measure clefs, so those emit nothing. + if (inClefData.location == api::ClefLocation::afterBarline) + { + mxClef.setAfterBarline(core::YesNo::yes()); + } + else if (inClefData.location == api::ClefLocation::beforeBarline) + { + mxClef.setAfterBarline(core::YesNo::no()); + } + if (inClefData.printObject != api::Bool::unspecified) { mxClef.setPrintObject(converter.convert(inClefData.printObject)); diff --git a/src/private/mxtest/api/ClefDataTest.cpp b/src/private/mxtest/api/ClefDataTest.cpp index 38e7f069c..b2dba1316 100644 --- a/src/private/mxtest/api/ClefDataTest.cpp +++ b/src/private/mxtest/api/ClefDataTest.cpp @@ -31,6 +31,25 @@ ScoreData makeScoreWithClef(Bool printObject) staff.voices[0].notes.emplace_back(); return score; } + +ScoreData makeScoreWithAdditionalClef(Bool additional) +{ + ScoreData score = makeScoreWithClef(Bool::unspecified); + score.parts.front().measures.front().staves.front().clefs.front().additional = additional; + return score; +} + +ScoreData makeScoreWithClefLocation(ClefLocation location) +{ + ScoreData score = makeScoreWithClef(Bool::unspecified); + score.parts.front().measures.front().staves.front().clefs.front().location = location; + return score; +} + +const ClefData &firstClef(const ScoreData &score) +{ + return score.parts.front().measures.front().staves.front().clefs.front(); +} } // namespace mxtest::api::clef_data_test TEST(clefPrintObjectNoRoundTrip, ClefData) @@ -78,4 +97,156 @@ TEST(clefPrintObjectUnspecifiedIsOmitted, ClefData) T_END; +TEST(clefAdditionalYesRoundTrip, ClefData) +{ + const auto xml = mxtest::toXml(mxtest::api::clef_data_test::makeScoreWithAdditionalClef(Bool::yes)); + CHECK(xml.find("additional=\"yes\"") != std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.size()); + CHECK_EQUAL(1, out.parts.front().measures.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(Bool::yes == out.parts.front().measures.front().staves.front().clefs.front().additional); +} + +T_END; + +TEST(clefAdditionalNoRoundTrip, ClefData) +{ + const auto xml = mxtest::toXml(mxtest::api::clef_data_test::makeScoreWithAdditionalClef(Bool::no)); + CHECK(xml.find("additional=\"no\"") != std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.size()); + CHECK_EQUAL(1, out.parts.front().measures.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(Bool::no == out.parts.front().measures.front().staves.front().clefs.front().additional); +} + +T_END; + +TEST(clefAdditionalUnspecifiedIsOmitted, ClefData) +{ + const auto xml = mxtest::toXml(mxtest::api::clef_data_test::makeScoreWithAdditionalClef(Bool::unspecified)); + CHECK(xml.find("additional=") == std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.size()); + CHECK_EQUAL(1, out.parts.front().measures.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.size()); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(Bool::unspecified == out.parts.front().measures.front().staves.front().clefs.front().additional); +} + +T_END; + +TEST(clefAfterBarlineYesRoundTrip, ClefData) +{ + using namespace mxtest::api::clef_data_test; + const auto xml = mxtest::toXml(makeScoreWithClefLocation(ClefLocation::afterBarline)); + CHECK(xml.find("after-barline=\"yes\"") != std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(ClefLocation::afterBarline == firstClef(out).location); +} + +T_END; + +TEST(clefAfterBarlineNoRoundTrip, ClefData) +{ + using namespace mxtest::api::clef_data_test; + const auto xml = mxtest::toXml(makeScoreWithClefLocation(ClefLocation::beforeBarline)); + CHECK(xml.find("after-barline=\"no\"") != std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(ClefLocation::beforeBarline == firstClef(out).location); +} + +T_END; + +TEST(clefAfterBarlineUnspecifiedIsOmitted, ClefData) +{ + using namespace mxtest::api::clef_data_test; + const auto xml = mxtest::toXml(makeScoreWithClefLocation(ClefLocation::unspecified)); + CHECK(xml.find("after-barline=") == std::string::npos); + + const auto out = mxtest::fromXml(xml); + CHECK_EQUAL(1, out.parts.front().measures.front().staves.front().clefs.size()); + CHECK(ClefLocation::unspecified == firstClef(out).location); +} + +T_END; + +TEST(clefAllFieldsSurviveWrite, ClefData) +{ + using namespace mxtest::api::clef_data_test; + ScoreData score = makeScoreWithClef(Bool::unspecified); + auto &clef = score.parts.front().measures.front().staves.front().clefs.front(); + clef.writeStaffNumber = Bool::yes; + clef.symbol = ClefSymbol::f; + clef.line = 4; + clef.isLineSpecified = true; + clef.octaveChange = -1; + clef.isOctaveChangeSpecified = true; + clef.tickTimePosition = 0; + clef.location = ClefLocation::afterBarline; + clef.additional = Bool::yes; + clef.printObject = Bool::no; + const ClefData expected = clef; + + const auto out = mxtest::fromXml(mxtest::toXml(score)); + const auto &actual = firstClef(out); + CHECK(expected.writeStaffNumber == actual.writeStaffNumber); + CHECK(expected.symbol == actual.symbol); + CHECK_EQUAL(expected.line, actual.line); + CHECK_EQUAL(expected.isLineSpecified, actual.isLineSpecified); + CHECK_EQUAL(expected.octaveChange, actual.octaveChange); + CHECK_EQUAL(expected.isOctaveChangeSpecified, actual.isOctaveChangeSpecified); + CHECK_EQUAL(expected.tickTimePosition, actual.tickTimePosition); + CHECK(expected.location == actual.location); + CHECK(expected.additional == actual.additional); + CHECK(expected.printObject == actual.printObject); + CHECK(expected == actual); +} + +T_END; + +TEST(clefUnspecifiedElementsSurviveWrite, ClefData) +{ + using namespace mxtest::api::clef_data_test; + ScoreData score = makeScoreWithClef(Bool::unspecified); + auto &clef = score.parts.front().measures.front().staves.front().clefs.front(); + clef.symbol = ClefSymbol::c; + clef.isLineSpecified = false; + clef.line = 3; + clef.isOctaveChangeSpecified = false; + clef.octaveChange = 0; + clef.writeStaffNumber = Bool::no; + const ClefData expected = clef; + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("") == std::string::npos); + CHECK(xml.find("") == std::string::npos); + CHECK(xml.find("") != std::string::npos); + + const auto out = mxtest::fromXml(xml); + const auto &actual = firstClef(out); + CHECK(expected.symbol == actual.symbol); + CHECK_EQUAL(expected.line, actual.line); + CHECK_EQUAL(expected.isLineSpecified, actual.isLineSpecified); + CHECK_EQUAL(expected.octaveChange, actual.octaveChange); + CHECK_EQUAL(expected.isOctaveChangeSpecified, actual.isOctaveChangeSpecified); + + // writeStaffNumber records only a divergence from the automatic rule. On a single-staff part + // the automatic rule already omits the number, so no agrees with it and reads back as + // unspecified. The emitted XML is the same either way. + CHECK(Bool::unspecified == actual.writeStaffNumber); +} + +T_END; + #endif From 8698ee47c1fc6d640dad41b0676e3ef337e68c34 Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 10:41:06 -0500 Subject: [PATCH 4/7] Add stackDegrees and parenthesesDegrees to ChordData Expose the MusicXML kind stack-degrees and parentheses-degrees attributes, which control how a chord symbol's degree alterations are laid out. Both are api::Bool defaulting to unspecified, so the writer emits nothing unless the field is set. DirectionReader reads them off core::Kind; DirectionWriter writes them back. Adds round-trip, negative, and file-based tests in HarmonyExtrasApiTest, and registers data/synthetic/kind.3.0.xml with MxFileRepository. --- src/include/mx/api/ChordData.h | 9 ++++ src/private/mx/api/ChordData.cpp | 4 +- src/private/mx/impl/DirectionReader.cpp | 12 +++++ src/private/mx/impl/DirectionWriter.cpp | 12 +++++ .../mxtest/api/HarmonyExtrasApiTest.cpp | 52 +++++++++++++++++++ src/private/mxtest/file/MxFileRepositoy.cpp | 1 + 6 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/include/mx/api/ChordData.h b/src/include/mx/api/ChordData.h index cbfbcb476..cef16d76c 100644 --- a/src/include/mx/api/ChordData.h +++ b/src/include/mx/api/ChordData.h @@ -251,6 +251,13 @@ class ChordData ChordKind chordKind; std::string text; Bool useSymbols; + // How the chord's degree alterations (the 'extensions' below) are laid out next to the chord + // symbol. stackDegrees stacks them vertically instead of running them left to right; + // parenthesesDegrees wraps all of them in parentheses, e.g. C7(#9b13). These are the MusicXML + // stack-degrees and parentheses-degrees attributes; unspecified leaves the choice to the + // notation software rendering the score. + Bool stackDegrees; + Bool parenthesesDegrees; Step bass; int bassAlter; // The MusicXML (0 = root position, 1 = first inversion, ...). Present only when @@ -279,6 +286,8 @@ MXAPI_EQUALS_MEMBER(numeralMode) MXAPI_EQUALS_MEMBER(chordKind) MXAPI_EQUALS_MEMBER(text) MXAPI_EQUALS_MEMBER(useSymbols) +MXAPI_EQUALS_MEMBER(stackDegrees) +MXAPI_EQUALS_MEMBER(parenthesesDegrees) MXAPI_EQUALS_MEMBER(bass) MXAPI_EQUALS_MEMBER(bassAlter) MXAPI_EQUALS_MEMBER(inversion) diff --git a/src/private/mx/api/ChordData.cpp b/src/private/mx/api/ChordData.cpp index 2f9a92bdf..442ef181f 100644 --- a/src/private/mx/api/ChordData.cpp +++ b/src/private/mx/api/ChordData.cpp @@ -32,8 +32,8 @@ ChordData::ChordData() : harmonyChordSource{HarmonyChordSource::root}, root{Step::c}, rootAlter{0}, functionText{}, numeralRoot{0}, numeralRootText{}, numeralAlter{0}, hasNumeralAlter{false}, hasNumeralKey{false}, numeralKeyFifths{0}, numeralMode{NumeralMode::unspecified}, chordKind{ChordKind::unspecified}, text{}, useSymbols{Bool::unspecified}, - bass{Step::unspecified}, bassAlter{0}, inversion{0}, hasInversion{false}, extensions{}, miscData{}, - hasFrameData{false}, frameData{}, positionData{} + stackDegrees{Bool::unspecified}, parenthesesDegrees{Bool::unspecified}, bass{Step::unspecified}, bassAlter{0}, + inversion{0}, hasInversion{false}, extensions{}, miscData{}, hasFrameData{false}, frameData{}, positionData{} { } diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index 64f6f6c5b..cafaacf33 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -1285,6 +1285,18 @@ void DirectionReader::parseHarmony(const core::Harmony &inHarmony, const core::H } } + if (kind.stackDegrees().has_value()) + { + const bool isYes = kind.stackDegrees()->tag() == mx::core::YesNo::Tag::yes; + chord.stackDegrees = isYes ? api::Bool::yes : api::Bool::no; + } + + if (kind.parenthesesDegrees().has_value()) + { + const bool isYes = kind.parenthesesDegrees()->tag() == mx::core::YesNo::Tag::yes; + chord.parenthesesDegrees = isYes ? api::Bool::yes : api::Bool::no; + } + if (inGrp.bass().has_value()) { const auto &bass = *inGrp.bass(); diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index d528dc722..2990e1c62 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -1591,6 +1591,18 @@ std::vector DirectionWriter::createHarmonyElements(int in kind.setUseSymbols(chordIter->useSymbols == api::Bool::yes ? core::YesNo::yes() : core::YesNo::no()); } + if (chordIter->stackDegrees != api::Bool::unspecified) + { + const bool isYes = chordIter->stackDegrees == api::Bool::yes; + kind.setStackDegrees(isYes ? core::YesNo::yes() : core::YesNo::no()); + } + + if (chordIter->parenthesesDegrees != api::Bool::unspecified) + { + const bool isYes = chordIter->parenthesesDegrees == api::Bool::yes; + kind.setParenthesesDegrees(isYes ? core::YesNo::yes() : core::YesNo::no()); + } + grp.setKind(kind); for (const auto &extension : chordIter->extensions) diff --git a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp index 40135a495..5651b6549 100644 --- a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp +++ b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp @@ -8,6 +8,8 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/ScoreData.h" #include "mxtest/api/RoundTrip.h" +#include "mxtest/api/TestHelpers.h" +#include "mxtest/file/MxFileRepository.h" using namespace std; using namespace mx::api; @@ -48,6 +50,56 @@ const ChordData &firstChord(const ScoreData &score) } } // namespace +TEST(harmonyDegreeLayoutRoundTrip, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::dominantNinth; + chord.stackDegrees = Bool::yes; + chord.parenthesesDegrees = Bool::no; + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("stack-degrees=\"yes\"") != std::string::npos); + CHECK(xml.find("parentheses-degrees=\"no\"") != std::string::npos); + + const auto out = mxtest::roundTrip(score); + const auto &outChord = firstChord(out); + CHECK(Bool::yes == outChord.stackDegrees); + CHECK(Bool::no == outChord.parenthesesDegrees); +} + +T_END; + +TEST(harmonyDegreeLayoutUnspecifiedIsNotWritten, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::major; + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("stack-degrees") == std::string::npos); + CHECK(xml.find("parentheses-degrees") == std::string::npos); + + const auto out = mxtest::roundTrip(score); + const auto &outChord = firstChord(out); + CHECK(Bool::unspecified == outChord.stackDegrees); + CHECK(Bool::unspecified == outChord.parenthesesDegrees); +} + +T_END; + +TEST(harmonyDegreeLayoutFromFile, HarmonyExtrasApi) +{ + const auto score = mxtest::MxFileRepository::loadFile("kind.3.0.xml"); + const auto &outChord = firstChord(score); + CHECK(Bool::yes == outChord.stackDegrees); + CHECK(Bool::yes == outChord.parenthesesDegrees); +} + +T_END; + TEST(harmonyInversionRoundTrip, HarmonyExtrasApi) { auto score = makeScoreWithChord(); diff --git a/src/private/mxtest/file/MxFileRepositoy.cpp b/src/private/mxtest/file/MxFileRepositoy.cpp index 139052f4a..89a7e76ee 100644 --- a/src/private/mxtest/file/MxFileRepositoy.cpp +++ b/src/private/mxtest/file/MxFileRepositoy.cpp @@ -478,5 +478,6 @@ void MxFileRepository::initializeNameSubdirectoryMap() myNameSubdirectoryMap.emplace("segno.3.1.xml", "synthetic"); myNameSubdirectoryMap.emplace("coda.3.0.xml", "synthetic"); myNameSubdirectoryMap.emplace("coda.3.1.xml", "synthetic"); + myNameSubdirectoryMap.emplace("kind.3.0.xml", "synthetic"); } } // namespace mxtest From 6c083c50d57d19a8a99e68120396781d7c9ea30f Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 12:47:24 -0500 Subject: [PATCH 5/7] Write the degree print-object attribute DirectionWriter built each core::Degree with only degree-type, degree-value, and degree-alter, so api::Extension::printObject was dropped on write. The read side already populated it, meaning the field round-tripped to nothing. MusicXML marks a degree print-object="no" when the kind text attribute already spells it out; Finale exports suspended-fourth with a hidden added seventh. Without this the seventh prints twice. Covered by a round-trip test, a negative test, and a read of the three hidden degrees in recsuite/BrookeWestSample.xml. --- src/private/mx/impl/DirectionWriter.cpp | 7 ++ .../mxtest/api/HarmonyExtrasApiTest.cpp | 87 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index 2990e1c62..ba2109c0b 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -1683,6 +1683,13 @@ std::vector DirectionWriter::createHarmonyElements(int in degree.setDegreeType(degreeType); degree.setDegreeValue(degreeValue); degree.setDegreeAlter(degreeAlter); + + if (extension.printObject != api::Bool::unspecified) + { + const bool isYes = extension.printObject == api::Bool::yes; + degree.setPrintObject(isYes ? core::YesNo::yes() : core::YesNo::no()); + } + grp.addDegree(degree); } diff --git a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp index 5651b6549..f8c29831f 100644 --- a/src/private/mxtest/api/HarmonyExtrasApiTest.cpp +++ b/src/private/mxtest/api/HarmonyExtrasApiTest.cpp @@ -100,6 +100,93 @@ TEST(harmonyDegreeLayoutFromFile, HarmonyExtrasApi) T_END; +// A degree the text already spells out is marked print-object="no" so it is not printed +// twice. Finale exports exactly this: suspended-fourth plus a hidden +// seventh. +TEST(harmonyDegreePrintObjectRoundTrip, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::suspendedFourth; + chord.text = "7sus4"; + Extension seventh; + seventh.extensionType = ExtensionType::add; + seventh.extensionNumber = ExtensionNumber::seventh; + seventh.extensionAlter = ExtensionAlter::none; + seventh.printObject = Bool::no; + chord.extensions.push_back(seventh); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("") != std::string::npos); + + const auto out = mxtest::roundTrip(score); + const auto &outChord = firstChord(out); + REQUIRE(outChord.extensions.size() == 1); + CHECK(Bool::no == outChord.extensions.front().printObject); + CHECK(ExtensionNumber::seventh == outChord.extensions.front().extensionNumber); +} + +T_END; + +TEST(harmonyDegreePrintObjectUnspecifiedIsNotWritten, HarmonyExtrasApi) +{ + auto score = makeScoreWithChord(); + auto &chord = chordOf(score); + chord.root = Step::c; + chord.chordKind = ChordKind::major; + Extension ninth; + ninth.extensionType = ExtensionType::add; + ninth.extensionNumber = ExtensionNumber::ninth; + ninth.extensionAlter = ExtensionAlter::sharp; + chord.extensions.push_back(ninth); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("print-object") == std::string::npos); + + const auto out = mxtest::roundTrip(score); + const auto &outChord = firstChord(out); + REQUIRE(outChord.extensions.size() == 1); + CHECK(Bool::unspecified == outChord.extensions.front().printObject); +} + +T_END; + +// The read path against a real Finale export: three chord symbols in this file hide a degree the +// kind text already spells out. +TEST(harmonyDegreePrintObjectFromFinaleExport, HarmonyExtrasApi) +{ + const auto score = mxtest::MxFileRepository::loadFile("BrookeWestSample.xml"); + int hiddenDegreeCount = 0; + + for (const auto &part : score.parts) + { + for (const auto &measure : part.measures) + { + for (const auto &staff : measure.staves) + { + for (const auto &direction : staff.directions) + { + for (const auto &chord : direction.chords) + { + for (const auto &extension : chord.extensions) + { + if (extension.printObject == Bool::no) + { + ++hiddenDegreeCount; + } + } + } + } + } + } + } + + CHECK_EQUAL(3, hiddenDegreeCount); +} + +T_END; + TEST(harmonyInversionRoundTrip, HarmonyExtrasApi) { auto score = makeScoreWithChord(); From c0913283515fd6c9c9d3c0d9972dc9a28b6801ea Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 13:18:55 -0500 Subject: [PATCH 6/7] Add notehead filled and smufl to NoteData Expose the element's filled and smufl attributes, which mx::core already modeled but mx::api dropped in both directions. NoteWriter now emits when either is set, even for a normal notehead value. Also adds the missing MXAPI_EQUALS_MEMBER(notehead), which had never been in the equality block. Six tests in NoteDataTest; no roundtrip-baseline.txt additions. --- src/include/mx/api/NoteData.h | 20 ++++ src/private/mx/api/NoteData.cpp | 6 +- src/private/mx/impl/NoteFunctions.cpp | 7 ++ src/private/mx/impl/NoteReader.cpp | 32 ++++-- src/private/mx/impl/NoteReader.h | 16 ++- src/private/mx/impl/NoteWriter.cpp | 28 ++++- src/private/mxtest/api/NoteDataTest.cpp | 143 ++++++++++++++++++++++++ 7 files changed, 233 insertions(+), 19 deletions(-) diff --git a/src/include/mx/api/NoteData.h b/src/include/mx/api/NoteData.h index f5a8692a1..488105000 100644 --- a/src/include/mx/api/NoteData.h +++ b/src/include/mx/api/NoteData.h @@ -13,6 +13,7 @@ #include "mx/api/PrintData.h" #include +#include #include namespace mx @@ -132,6 +133,22 @@ class NoteData bool isCue; Notehead notehead; + + // The element's filled attribute: whether the notehead shape is drawn solid or + // hollow. Leave it unspecified to get MusicXML's default, which follows the note's duration + // (hollow for a half note and longer, solid for shorter). Set yes or no to override that, + // for example a solid whole note in a rhythm-notation part. + Bool noteheadFilled; + + // The element's smufl attribute: the canonical SMuFL glyph name to draw for this + // notehead, for example "noteheadSlashHorizontalEnds". It names a glyph directly, so a + // notation program can draw a notehead that MusicXML has no value for -- pair it with + // Notehead::other -- or narrow one that MusicXML names only broadly, such as + // Notehead::cluster. Note-name noteheads (the SMuFL ranges U+E150-U+E1AF and U+EEE0-U+EEFF) + // are not written this way; MusicXML spells those out in . An empty string + // names no glyph and is written the same as leaving this empty. + std::optional noteheadSmufl; + PitchData pitchData; // step, alter, octave, accidental, etc int userRequestedVoiceNumber; @@ -196,6 +213,9 @@ MXAPI_EQUALS_MEMBER(tieLetRing) MXAPI_EQUALS_MEMBER(isGrace) MXAPI_EQUALS_MEMBER(isCue) MXAPI_EQUALS_MEMBER(graceSlash) +MXAPI_EQUALS_MEMBER(notehead) +MXAPI_EQUALS_MEMBER(noteheadFilled) +MXAPI_EQUALS_MEMBER(noteheadSmufl) MXAPI_EQUALS_MEMBER(pitchData) MXAPI_EQUALS_MEMBER(userRequestedVoiceNumber) MXAPI_EQUALS_MEMBER(writeStaffNumber) diff --git a/src/private/mx/api/NoteData.cpp b/src/private/mx/api/NoteData.cpp index 619b23984..31fbc42a2 100644 --- a/src/private/mx/api/NoteData.cpp +++ b/src/private/mx/api/NoteData.cpp @@ -11,9 +11,9 @@ namespace api NoteData::NoteData() : isRest{false}, isMeasureRest{false}, isUnpitched{false}, isDisplayStepOctaveSpecified{false}, isChord{false}, isTieStart{false}, isTieStop{false}, tieLetRing{}, isGrace{false}, graceSlash{Bool::unspecified}, isCue{false}, - notehead{Notehead::normal}, pitchData{}, userRequestedVoiceNumber{VALUE_UNSPECIFIED}, - writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified}, tickTimePosition{0}, durationData{}, beams{}, - positionData{}, printData{}, noteAttachmentData{}, lyrics{} + notehead{Notehead::normal}, noteheadFilled{Bool::unspecified}, noteheadSmufl{}, pitchData{}, + userRequestedVoiceNumber{VALUE_UNSPECIFIED}, writeStaffNumber{Bool::unspecified}, stem{Stem::unspecified}, + tickTimePosition{0}, durationData{}, beams{}, positionData{}, printData{}, noteAttachmentData{}, lyrics{} { } } // namespace api diff --git a/src/private/mx/impl/NoteFunctions.cpp b/src/private/mx/impl/NoteFunctions.cpp index d93924242..1a85f87ba 100644 --- a/src/private/mx/impl/NoteFunctions.cpp +++ b/src/private/mx/impl/NoteFunctions.cpp @@ -93,6 +93,13 @@ api::NoteData NoteFunctions::parseNote() const myOutNoteData.notehead = converter.convert(reader.getNoteheadValue()); + if (reader.getNoteheadFilled().has_value()) + { + myOutNoteData.noteheadFilled = converter.convert(*reader.getNoteheadFilled()); + } + + myOutNoteData.noteheadSmufl = reader.getNoteheadSmufl(); + if (reader.getIsDurationTypeSpecified()) { myOutNoteData.durationData.durationName = converter.convert(reader.getDurationType()); diff --git a/src/private/mx/impl/NoteReader.cpp b/src/private/mx/impl/NoteReader.cpp index 1aa8006c6..25b3796f7 100644 --- a/src/private/mx/impl/NoteReader.cpp +++ b/src/private/mx/impl/NoteReader.cpp @@ -145,20 +145,21 @@ NoteReader::NoteReader(const core::Note &mxNote) myIsNormal(false), myIsGrace(false), myIsCue(false), myIsRest(false), myIsChord(false), myIsMeasureRest(false), myIsUnpitched(false), myIsPitch(false), myIsDisplayStepOctaveSpecified(false), myDurationValue(0.0), myStep(core::Step::c()), myAlter(0), myCents(0.0), myOctave(4), myStaffNumber(0), myIsStaffSpecified(false), - myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myDurationType(core::NoteTypeValue::maxima()), - myIsDurationTypeSpecified(false), myNumDots(0), myBeams(), myTimeModificationActualNotes(-1), - myTimeModificationNormalNotes(-1), myTimeModificationNormalType(core::NoteTypeValue::maxima()), - myTimeModificationNormalTypeDots(0), myHasAccidental(false), myAccidental(core::AccidentalValue::natural()), - myIsAccidentalParenthetical(false), myIsAccidentalCautionary{false}, myIsAccidentalEditorial{false}, - myIsAccidentalBracketed{false}, myIsStemSpecified{false}, myStem{}, myIsGraceSlashSpecified{false}, - myGraceSlash{}, myIsTieStart{false}, myIsTieStop{false}, myHasLyric{false} + myVoiceNumber(0), myNoteheadValue(core::NoteheadValue::normal()), myNoteheadFilled{}, myNoteheadSmufl{}, + myDurationType(core::NoteTypeValue::maxima()), myIsDurationTypeSpecified(false), myNumDots(0), myBeams(), + myTimeModificationActualNotes(-1), myTimeModificationNormalNotes(-1), + myTimeModificationNormalType(core::NoteTypeValue::maxima()), myTimeModificationNormalTypeDots(0), + myHasAccidental(false), myAccidental(core::AccidentalValue::natural()), myIsAccidentalParenthetical(false), + myIsAccidentalCautionary{false}, myIsAccidentalEditorial{false}, myIsAccidentalBracketed{false}, + myIsStemSpecified{false}, myStem{}, myIsGraceSlashSpecified{false}, myGraceSlash{}, myIsTieStart{false}, + myIsTieStop{false}, myHasLyric{false} { setNormalGraceCueItems(); setRestPitchUnpitchedItems(); setChord(); setStaffNumber(); setVoiceNumber(); - setNoteheadValue(); + setNoteheadItems(); setDurationType(); setNumDots(); setBeams(); @@ -323,11 +324,20 @@ void NoteReader::setVoiceNumber() utility::stringToInt(myNote.editorialVoice().voice()->c_str(), myVoiceNumber); } -void NoteReader::setNoteheadValue() +void NoteReader::setNoteheadItems() { - if (myNote.notehead().has_value()) + if (!myNote.notehead().has_value()) { - myNoteheadValue = myNote.notehead()->value(); + return; + } + + const auto ¬ehead = *myNote.notehead(); + myNoteheadValue = notehead.value(); + myNoteheadFilled = notehead.filled(); + + if (notehead.smufl().has_value()) + { + myNoteheadSmufl = notehead.smufl()->toString(); } } diff --git a/src/private/mx/impl/NoteReader.h b/src/private/mx/impl/NoteReader.h index a5f51f501..413dd7abc 100644 --- a/src/private/mx/impl/NoteReader.h +++ b/src/private/mx/impl/NoteReader.h @@ -7,7 +7,9 @@ #include "mx/api/LyricData.h" #include "mx/core/generated/Note.h" +#include #include +#include #include namespace mx @@ -132,6 +134,16 @@ class NoteReader return myNoteheadValue; } + inline const std::optional &getNoteheadFilled() const + { + return myNoteheadFilled; + } + + inline const std::optional &getNoteheadSmufl() const + { + return myNoteheadSmufl; + } + inline core::NoteTypeValue getDurationType() const { return myDurationType; @@ -259,6 +271,8 @@ class NoteReader bool myIsStaffSpecified; int myVoiceNumber; core::NoteheadValue myNoteheadValue; + std::optional myNoteheadFilled; + std::optional myNoteheadSmufl; core::NoteTypeValue myDurationType; bool myIsDurationTypeSpecified; int myNumDots; @@ -289,7 +303,7 @@ class NoteReader void setChord(); void setStaffNumber(); void setVoiceNumber(); - void setNoteheadValue(); + void setNoteheadItems(); void setDurationType(); void setNumDots(); void setBeams(); diff --git a/src/private/mx/impl/NoteWriter.cpp b/src/private/mx/impl/NoteWriter.cpp index 2645462fe..fcef3c10b 100644 --- a/src/private/mx/impl/NoteWriter.cpp +++ b/src/private/mx/impl/NoteWriter.cpp @@ -20,6 +20,7 @@ #include "mx/core/generated/NormalNoteGroup.h" #include "mx/core/generated/Pitch.h" #include "mx/core/generated/Rest.h" +#include "mx/core/generated/SmuflGlyphName.h" #include "mx/core/generated/Syllabic.h" #include "mx/core/generated/TextElementData.h" #include "mx/core/generated/Tied.h" @@ -477,12 +478,31 @@ void NoteWriter::setDurationNameAndDots() const void NoteWriter::setNotehead() const { - if (myNoteData.notehead != mx::api::Notehead::normal) + const bool isFilledSpecified = myNoteData.noteheadFilled != api::Bool::unspecified; + // An empty glyph name names no glyph, so it is treated the same as no smufl at all. + const bool isSmuflSpecified = myNoteData.noteheadSmufl.has_value() && !myNoteData.noteheadSmufl->empty(); + + // stays out of the file for a plain notehead, but filled or smufl still needs + // the element even when the value itself is 'normal'. + if (myNoteData.notehead == api::Notehead::normal && !isFilledSpecified && !isSmuflSpecified) + { + return; + } + + core::Notehead notehead; + notehead.setValue(myConverter.convert(myNoteData.notehead)); + + if (isFilledSpecified) { - core::Notehead notehead; - notehead.setValue(myConverter.convert(myNoteData.notehead)); - myOutNote.setNotehead(std::move(notehead)); + notehead.setFilled(myConverter.convert(myNoteData.noteheadFilled)); } + + if (isSmuflSpecified) + { + notehead.setSmufl(core::SmuflGlyphName{*myNoteData.noteheadSmufl}); + } + + myOutNote.setNotehead(std::move(notehead)); } void NoteWriter::setStemDirection() const diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index b5f718087..6039f9651 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -1471,6 +1471,149 @@ TEST(noteheadOtherRoundtrip, NoteData) CHECK(outNote.notehead == Notehead::other); } +T_END; + +// A one-note score whose single quarter note is ready for notehead fields to be set on it. +ScoreData makeNoteheadScore() +{ + ScoreData score; + score.ticksPerQuarter = 96; + score.parts.emplace_back(); + auto &part = score.parts.back(); + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(); + auto &staff = measure.staves.back(); + auto &voice = staff.voices[0]; + + NoteData note; + note.durationData.durationName = DurationName::quarter; + note.durationData.durationTimeTicks = 96; + voice.notes.push_back(note); + + return score; +} + +const NoteData &firstNoteheadNote(const ScoreData &score) +{ + return score.parts.front().measures.front().staves.front().voices.begin()->second.notes.front(); +} + +// filled="no" on an otherwise normal notehead: the element has to be written even +// though the notehead value itself is the default. +TEST(noteheadFilledRoundtrip, NoteData) +{ + auto score = makeNoteheadScore(); + score.parts.back().measures.back().staves.back().voices.at(0).notes.back().noteheadFilled = Bool::no; + + const std::string xml = mxtest::toXml(score); + CHECK(xml.find(R"(normal)") != std::string::npos); + + const auto outScore = mxtest::fromXml(xml); + CHECK(Bool::no == firstNoteheadNote(outScore).noteheadFilled); + CHECK(Notehead::normal == firstNoteheadNote(outScore).notehead); +} + +T_END; + +// filled="yes" alongside a non-default notehead value. +TEST(noteheadFilledYesWithShapeRoundtrip, NoteData) +{ + auto score = makeNoteheadScore(); + auto ¬e = score.parts.back().measures.back().staves.back().voices.at(0).notes.back(); + note.notehead = Notehead::diamond; + note.noteheadFilled = Bool::yes; + + const std::string xml = mxtest::toXml(score); + CHECK(xml.find(R"(diamond)") != std::string::npos); + + const auto outScore = mxtest::fromXml(xml); + CHECK(Bool::yes == firstNoteheadNote(outScore).noteheadFilled); + CHECK(Notehead::diamond == firstNoteheadNote(outScore).notehead); +} + +T_END; + +// The smufl attribute names a glyph that MusicXML has no notehead value for; it pairs with the +// 'other' notehead value. +TEST(noteheadSmuflRoundtrip, NoteData) +{ + auto score = makeNoteheadScore(); + auto ¬e = score.parts.back().measures.back().staves.back().voices.at(0).notes.back(); + note.notehead = Notehead::other; + note.noteheadSmufl = "noteheadSlashHorizontalEnds"; + + const std::string xml = mxtest::toXml(score); + CHECK(xml.find(R"(smufl="noteheadSlashHorizontalEnds")") != std::string::npos); + + const auto outScore = mxtest::fromXml(xml); + const auto &outNote = firstNoteheadNote(outScore); + CHECK(Notehead::other == outNote.notehead); + REQUIRE(outNote.noteheadSmufl.has_value()); + CHECK_EQUAL("noteheadSlashHorizontalEnds", *outNote.noteheadSmufl); +} + +T_END; + +// Both attributes at once, refining a notehead value that MusicXML names only broadly. +TEST(noteheadFilledAndSmuflRoundtrip, NoteData) +{ + auto score = makeNoteheadScore(); + auto ¬e = score.parts.back().measures.back().staves.back().voices.at(0).notes.back(); + note.notehead = Notehead::cluster; + note.noteheadFilled = Bool::yes; + note.noteheadSmufl = "noteheadClusterSquareBlack"; + + const auto outScore = mxtest::fromXml(mxtest::toXml(score)); + const auto &outNote = firstNoteheadNote(outScore); + CHECK(Notehead::cluster == outNote.notehead); + CHECK(Bool::yes == outNote.noteheadFilled); + REQUIRE(outNote.noteheadSmufl.has_value()); + CHECK_EQUAL("noteheadClusterSquareBlack", *outNote.noteheadSmufl); +} + +T_END; + +// A note that sets neither field writes no element at all. +TEST(noteheadAttributesDefaultToAbsent, NoteData) +{ + const auto score = makeNoteheadScore(); + + const std::string xml = mxtest::toXml(score); + CHECK(xml.find(" api read path independently of what the writer emits. +TEST(noteheadSyntheticFileRead, NoteData) +{ + const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/notehead.3.1.xml"; + auto &docMgr = DocumentManager::getInstance(); + const auto docIdResult = docMgr.createFromFile(path); + REQUIRE(docIdResult.ok()); + const int docId = docIdResult.value(); + const auto scoreResult = docMgr.getData(docId); + docMgr.destroyDocument(docId); + REQUIRE(scoreResult.ok()); + const auto &score = scoreResult.value(); + REQUIRE(score.parts.size() == 1); + REQUIRE(score.parts.front().measures.size() == 1); + + const auto &outNote = firstNoteheadNote(score); + CHECK(Notehead::slash == outNote.notehead); + CHECK(Bool::yes == outNote.noteheadFilled); + REQUIRE(outNote.noteheadSmufl.has_value()); + CHECK_EQUAL("noteheadBlack", *outNote.noteheadSmufl); +} + +T_END; + TEST(printObjectNo, NoteData) { ScoreData score; From 509c9e0743f5c2391346a772b739c9aa67fda36e Mon Sep 17 00:00:00 2001 From: Robert Patterson Date: Wed, 29 Jul 2026 13:57:48 -0500 Subject: [PATCH 7/7] Fix dangling reference in the notehead default-absent test The test bound a reference to firstNoteheadNote(mxtest::fromXml(xml)). Lifetime extension does not apply when a reference binds to one returned by a function, so the temporary ScoreData was destroyed at the end of the statement and outNote dangled. Hold the score in a local first, as the neighbouring notehead tests already do. MSVC's debug CRT caught this by filling the freed block with 0xDD, which read back as -572662307; libc++ happened not to reuse the block, so the Linux and macOS jobs passed. --- src/private/mxtest/api/NoteDataTest.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index 6039f9651..1749a7f4e 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -1582,7 +1582,8 @@ TEST(noteheadAttributesDefaultToAbsent, NoteData) const std::string xml = mxtest::toXml(score); CHECK(xml.find("