From 61d6d6e1ae1699fdb7b87c49671070757c71f64b Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Fri, 6 Nov 2026 05:30:45 +0100 Subject: [PATCH 1/3] feat: read and write a hairpin's niente circle through WedgeStart draws a small circle at the point of a hairpin: a crescendo out of nothing or a diminuendo to nothing. It was dropped on read and never written. WedgeStart gains Bool niente, set on the start for either direction. MusicXML states it on the start of a crescendo but on the stop of a diminuendo, so the writer routes it: a crescendo (or a wedge of unspecified type, which is written as a crescendo) carries it on its start, and SpannerResolver hands a diminuendo's value to the stop that closes it, as it already does for an octave-shift's size. A diminuendo with niente and no stop is reported as dropped data. The reader pairs the part's wedges in one pass before reading its measures, so a diminuendo start takes the niente of its stop even when the stop is in a later measure. niente on the wide end of a hairpin (a diminuendo start or a crescendo stop) or on a continue segment means nothing in MusicXML and is reported as dropped data. The pairing rule (within a part, by number, a stop closes the most recently opened start) now lives in one place, SpannerPairing.h, shared by the reader, the octave-shift sizes and the wedge niente. Pins synthetic/wedge.3.0.xml and synthetic/wedge.3.1.xml. --- data/api.features.xml | 3 +- src/include/mx/api/WedgeData.h | 10 +- src/private/mx/impl/DirectionReader.cpp | 105 +++++- src/private/mx/impl/DirectionReader.h | 31 +- src/private/mx/impl/DirectionWriter.cpp | 14 + src/private/mx/impl/MeasureReader.cpp | 10 +- src/private/mx/impl/MeasureReader.h | 6 +- src/private/mx/impl/PartReader.cpp | 6 +- src/private/mx/impl/SpannerPairing.h | 79 +++++ src/private/mx/impl/SpannerResolver.cpp | 114 +++++-- src/private/mx/impl/SpannerResolver.h | 26 +- src/private/mxtest/api/DirectionDataTest.cpp | 317 ++++++++++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 6 + 13 files changed, 680 insertions(+), 47 deletions(-) create mode 100644 src/private/mx/impl/SpannerPairing.h diff --git a/data/api.features.xml b/data/api.features.xml index 8d98ced92..413e9c0af 100644 --- a/data/api.features.xml +++ b/data/api.features.xml @@ -1208,8 +1208,9 @@ + - api::WedgeStart/WedgeStop; DirectionReader.parseWedge (stop/continue routed structurally, not via WedgeType enum). + api::WedgeStart/WedgeStop; DirectionReader.parseWedge (stop/continue routed structurally, not via WedgeType enum). niente lives on WedgeStart for either direction; a diminuendo's niente is read from and written to its stop. Part of bend; not modeled. diff --git a/src/include/mx/api/WedgeData.h b/src/include/mx/api/WedgeData.h index dbdd5a4f0..e963c542a 100644 --- a/src/include/mx/api/WedgeData.h +++ b/src/include/mx/api/WedgeData.h @@ -36,12 +36,19 @@ struct WedgeStart bool isColorSpecified; ColorData colorData; + // Whether the point of the hairpin is drawn as a small circle (niente): a crescendo that grows + // out of nothing, or a diminuendo that fades away to nothing. Set it here on the start for + // either direction. MusicXML puts it on the start of a crescendo but on the stop of a + // diminuendo, and mx writes it in the right place for you. Leave it unspecified for a plain + // point. + Bool niente; + // The element's id attribute (see Id.h). std::optional id; WedgeStart() : number{}, wedgeType{WedgeType::unspecified}, isSpreadSpecified{false}, spread{0.0}, lineData{}, - positionData{}, isColorSpecified{false}, colorData{}, id{} + positionData{}, isColorSpecified{false}, colorData{}, niente{Bool::unspecified}, id{} { } }; @@ -70,6 +77,7 @@ MXAPI_EQUALS_MEMBER(lineData) MXAPI_EQUALS_MEMBER(positionData) MXAPI_EQUALS_MEMBER(isColorSpecified) MXAPI_EQUALS_MEMBER(colorData) +MXAPI_EQUALS_MEMBER(niente) MXAPI_EQUALS_MEMBER(id) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(WedgeStart); diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index bbe979242..296a13496 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -42,6 +42,7 @@ #include "mx/core/generated/Membrane.h" #include "mx/core/generated/Metal.h" #include "mx/core/generated/Metronome.h" +#include "mx/core/generated/MusicDataChoice.h" #include "mx/core/generated/Numeral.h" #include "mx/core/generated/NumeralKey.h" #include "mx/core/generated/NumeralMode.h" @@ -53,6 +54,8 @@ #include "mx/core/generated/OnOff.h" #include "mx/core/generated/OtherDirection.h" #include "mx/core/generated/OtherText.h" +#include "mx/core/generated/PartwiseMeasure.h" +#include "mx/core/generated/PartwisePart.h" #include "mx/core/generated/Pedal.h" #include "mx/core/generated/PedalTuning.h" #include "mx/core/generated/PedalType.h" @@ -90,6 +93,7 @@ #include "mx/impl/PrintFunctions.h" #include "mx/impl/SoundFunctions.h" #include "mx/impl/SpannerFunctions.h" +#include "mx/impl/SpannerPairing.h" #include "mx/impl/TuningFunctions.h" #include "mx/utility/Round.h" #include "mx/utility/StringToInt.h" @@ -100,18 +104,79 @@ namespace mx namespace impl { DirectionReader::DirectionReader(const core::Direction &inDirection, MeasureCursor inCursor, - DiagnosticsContext diagnostics) + DiagnosticsContext diagnostics, const WedgeNiente *inWedgeNiente) : myDirection{&inDirection}, myHarmony{nullptr}, myCursor{inCursor}, myConverter{}, myDiagnostics{diagnostics}, - myOutDirectionData{} + myWedgeNiente{inWedgeNiente}, myOutDirectionData{} { } DirectionReader::DirectionReader(const core::Harmony &inHarmony, MeasureCursor inCursor, DiagnosticsContext diagnostics) : myDirection{nullptr}, myHarmony{&inHarmony}, myCursor{inCursor}, myConverter{}, myDiagnostics{diagnostics}, - myOutDirectionData{} + myWedgeNiente{nullptr}, myOutDirectionData{} { } +WedgeNiente DirectionReader::scanWedgeNiente(const core::PartwisePart &inPart) +{ + // A continue segment neither opens nor closes a hairpin, so it takes no part in the pairing. + struct OpenWedge + { + const core::Wedge *wedge; + bool isDiminuendo; + }; + + const Converter converter{}; + WedgeNiente result; + SpannerPairing pairing; + for (const auto &measure : inPart.measure()) + { + for (const auto &musicData : measure.musicData()) + { + if (musicData.kind() != core::MusicDataChoice::Kind::direction) + { + continue; + } + for (const auto &directionType : musicData.asDirection().directionType()) + { + if (directionType.choice().kind() != core::DirectionTypeChoice::Kind::wedge) + { + continue; + } + const auto &wedge = directionType.choice().asWedge(); + const auto number = + wedge.number().has_value() ? api::SpannerNumber(wedge.number()->value()) : api::SpannerNumber{}; + switch (wedge.type().tag()) + { + case core::WedgeType::Tag::crescendo: + pairing.open(number, OpenWedge{&wedge, false}); + break; + + case core::WedgeType::Tag::diminuendo: + pairing.open(number, OpenWedge{&wedge, true}); + break; + + case core::WedgeType::Tag::stop: { + const auto start = pairing.close(number); + if (start.has_value() && start->isDiminuendo) + { + result.diminuendoStops.insert(&wedge); + if (wedge.niente().has_value()) + { + result.diminuendoNiente[start->wedge] = converter.convert(*wedge.niente()); + } + } + break; + } + + default: + break; + } + } + } + } + return result; +} + void DirectionReader::report(api::Severity severity, api::DiagnosticCode code, std::string message) const { myDiagnostics.report(severity, code, measureLocation(myCursor), std::move(message)); @@ -546,6 +611,13 @@ void DirectionReader::parseWedge(const core::DirectionType &directionType) } stop.positionData = positionData; stop.id = getId(wedge); + const bool closesDiminuendo = myWedgeNiente && myWedgeNiente->diminuendoStops.count(&wedge) > 0; + if (wedge.niente().has_value() && !closesDiminuendo) + { + report(api::Severity::warning, api::DiagnosticCode::droppedData, + "wedge niente is only read on the start of a crescendo or the stop of a diminuendo; " + "niente on a stop that does not close a diminuendo is not read"); + } myOutDirectionData.directionTypes.emplace_back(api::DirectionChoice{std::move(stop)}); return; } @@ -566,11 +638,38 @@ void DirectionReader::parseWedge(const core::DirectionType &directionType) start.lineData = lineData; start.isColorSpecified = isColorSpecified; start.colorData = colorData; + start.niente = parseWedgeStartNiente(wedge); start.id = getId(wedge); myOutDirectionData.directionTypes.emplace_back(api::DirectionChoice{std::move(start)}); } } +api::Bool DirectionReader::parseWedgeStartNiente(const core::Wedge &wedge) const +{ + const auto tag = wedge.type().tag(); + if (tag == core::WedgeType::Tag::crescendo) + { + return wedge.niente().has_value() ? myConverter.convert(*wedge.niente()) : api::Bool::unspecified; + } + + if (wedge.niente().has_value()) + { + report(api::Severity::warning, api::DiagnosticCode::droppedData, + tag == core::WedgeType::Tag::diminuendo + ? "wedge niente is only read on the start of a crescendo or the stop of a diminuendo; niente on " + "the start of a diminuendo is not read" + : "wedge niente is only read on the start of a crescendo or the stop of a diminuendo; niente on " + "a continue segment is not read"); + } + + if (tag != core::WedgeType::Tag::diminuendo || !myWedgeNiente) + { + return api::Bool::unspecified; + } + const auto iter = myWedgeNiente->diminuendoNiente.find(&wedge); + return iter == myWedgeNiente->diminuendoNiente.cend() ? api::Bool::unspecified : iter->second; +} + void DirectionReader::parseDynamics(const core::DirectionType &directionType) { for (const auto &dynamic : directionType.choice().asDynamics().items()) diff --git a/src/private/mx/impl/DirectionReader.h b/src/private/mx/impl/DirectionReader.h index b28f26f3d..5bcb22f8a 100644 --- a/src/private/mx/impl/DirectionReader.h +++ b/src/private/mx/impl/DirectionReader.h @@ -17,25 +17,53 @@ #include "mx/impl/PositionFunctions.h" #include +#include +#include #include namespace mx { +namespace core +{ +class PartwisePart; +class Wedge; +} // namespace core + namespace impl { +// MusicXML writes a diminuendo's niente on the stop that closes it, which comes later in the +// part than the start that api::WedgeStart::niente lives on. This is what a scan of the whole +// part learns about that pairing before the part's directions are read. +struct WedgeNiente +{ + // The niente stated by the stop of each diminuendo whose stop states one. + std::unordered_map diminuendoNiente; + + // Every stop that closes a diminuendo. niente on any other stop is not at the hairpin's point. + std::unordered_set diminuendoStops; +}; + class DirectionReader { public: - DirectionReader(const core::Direction &inDirection, MeasureCursor inCursor, DiagnosticsContext diagnostics = {}); + // inWedgeNiente is the scan of the part this direction belongs to (see scanWedgeNiente). When + // it is null, a diminuendo's niente is not read. + DirectionReader(const core::Direction &inDirection, MeasureCursor inCursor, DiagnosticsContext diagnostics = {}, + const WedgeNiente *inWedgeNiente = nullptr); DirectionReader(const core::Harmony &inHarmony, MeasureCursor inCursor, DiagnosticsContext diagnostics = {}); api::DirectionData getDirectionData(); + // Pairs the part's wedge stops with the starts they close, in one pass in document order and + // by the rule in SpannerPairing.h, and records what each diminuendo's stop says about niente. + static WedgeNiente scanWedgeNiente(const core::PartwisePart &inPart); + private: const core::Direction *const myDirection; const core::Harmony *const myHarmony; const MeasureCursor myCursor; const Converter myConverter; DiagnosticsContext myDiagnostics; + const WedgeNiente *const myWedgeNiente; api::DirectionData myOutDirectionData; private: @@ -55,6 +83,7 @@ class DirectionReader void parseWordsRun(const core::DirectionType &directionType); void parseCoda(const core::DirectionType &directionType); void parseWedge(const core::DirectionType &directionType); + api::Bool parseWedgeStartNiente(const core::Wedge &wedge) const; void parseDynamics(const core::DirectionType &directionType); void parseDashes(const core::DirectionType &directionType); void parseBracket(const core::DirectionType &directionType); diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index 544942469..e88be9976 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -383,6 +383,14 @@ void DirectionWriter::emitWedgeStop(const api::WedgeStop &wedgeStop, const void { wedge.setSpread(core::Tenths{core::Decimal{static_cast(wedgeStop.spread)}}); } + + // a diminuendo's niente comes from its start and is written here, at the hairpin's point + const auto niente = mySpannerResolver.wedgeStopNiente(inIdentity); + if (niente != api::Bool::unspecified) + { + wedge.setNiente(myConverter.convert(niente)); + } + setAttributesFromPositionData(wedgeStop.positionData, wedge); setId(wedgeStop.id, wedge, myDiagnostics, cursorLocation(myCursor)); core::DirectionType dt{}; @@ -416,6 +424,12 @@ void DirectionWriter::emitWedgeStart(const api::WedgeStart &wedgeStart, const vo wedge.setSpread(core::Tenths{core::Decimal{static_cast(wedgeStart.spread)}}); } + // a crescendo's point is at its start; a diminuendo's niente is written on its stop + if (wedgeStart.wedgeType != api::WedgeType::diminuendo && wedgeStart.niente != api::Bool::unspecified) + { + wedge.setNiente(myConverter.convert(wedgeStart.niente)); + } + setAttributesFromPositionData(wedgeStart.positionData, wedge); setAttributesFromLineData(wedgeStart.lineData, wedge); if (wedgeStart.isColorSpecified) diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index 88f8a4e84..1771e769d 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/src/private/mx/impl/MeasureReader.cpp @@ -163,9 +163,11 @@ int MeasureReader::figuredBassStaffIndex(const MeasureCursor &cursor, const api: } MeasureReader::MeasureReader(const core::PartwiseMeasure &inPartwiseMeasureRef, const MeasureCursor &cursor, - const MeasureCursor &previousMeasureCursor, DiagnosticsContext diagnostics) - : myMutex{}, myPartwiseMeasure{inPartwiseMeasureRef}, myConverter{}, myDiagnostics{diagnostics}, myOutMeasureData{}, - myCurrentCursor{cursor}, myPreviousMeasureCursor{previousMeasureCursor}, myHistory{}, myCrossStaffHomes{}, + const MeasureCursor &previousMeasureCursor, DiagnosticsContext diagnostics, + const WedgeNiente *inWedgeNiente) + : myMutex{}, myPartwiseMeasure{inPartwiseMeasureRef}, myConverter{}, myDiagnostics{diagnostics}, + myWedgeNiente{inWedgeNiente}, myOutMeasureData{}, myCurrentCursor{cursor}, + myPreviousMeasureCursor{previousMeasureCursor}, myHistory{}, myCrossStaffHomes{}, myPreviousNoteBucketStaffIndex{-1} { HistoryRecord initialCursorRecord; @@ -714,7 +716,7 @@ void MeasureReader::parseForward(const core::Forward &inMxForward) const void MeasureReader::parseDirection(const core::Direction &inDirection) const { - DirectionReader reader{inDirection, myCurrentCursor, myDiagnostics}; + DirectionReader reader{inDirection, myCurrentCursor, myDiagnostics, myWedgeNiente}; auto directionData = reader.getDirectionData(); // make an adjustment if the directionData refers to a non-existent staff diff --git a/src/private/mx/impl/MeasureReader.h b/src/private/mx/impl/MeasureReader.h index 85e9e67f8..fcf5810cd 100644 --- a/src/private/mx/impl/MeasureReader.h +++ b/src/private/mx/impl/MeasureReader.h @@ -39,12 +39,15 @@ class ForPart; namespace impl { class NoteReader; +struct WedgeNiente; class MeasureReader { public: + // inWedgeNiente is the part's wedge scan (see DirectionReader::scanWedgeNiente), or null. MeasureReader(const core::PartwiseMeasure &inPartwiseMeasureRef, const MeasureCursor &cursor, - const MeasureCursor &previousMeasureCursor, DiagnosticsContext diagnostics = {}); + const MeasureCursor &previousMeasureCursor, DiagnosticsContext diagnostics = {}, + const WedgeNiente *inWedgeNiente = nullptr); std::pair> getMeasureData() const; impl::MeasureCursor getCursor() const; @@ -54,6 +57,7 @@ class MeasureReader const core::PartwiseMeasure &myPartwiseMeasure; const Converter myConverter; DiagnosticsContext myDiagnostics; + const WedgeNiente *const myWedgeNiente; mutable api::MeasureData myOutMeasureData; mutable MeasureCursor myCurrentCursor; diff --git a/src/private/mx/impl/PartReader.cpp b/src/private/mx/impl/PartReader.cpp index ceadee241..683c9cddd 100644 --- a/src/private/mx/impl/PartReader.cpp +++ b/src/private/mx/impl/PartReader.cpp @@ -22,6 +22,7 @@ #include "mx/core/generated/ScorePartMIDIGroup.h" #include "mx/core/generated/ScorePartwise.h" #include "mx/impl/Converter.h" +#include "mx/impl/DirectionReader.h" #include "mx/impl/MeasureReader.h" #include "mx/impl/NameDisplayFunctions.h" #include "mx/impl/PositionFunctions.h" @@ -105,9 +106,12 @@ api::PartData PartReader::getPartData() myPreviousCursor = myCurrentCursor; + // a diminuendo's niente is stated on its stop, later in the part, so the wedges are paired first + const auto wedgeNiente = DirectionReader::scanWedgeNiente(myPartwisePart); + for (const auto &mxMeasure : myPartwisePart.measure()) { - MeasureReader reader{mxMeasure, myCurrentCursor, myPreviousCursor, myDiagnostics}; + MeasureReader reader{mxMeasure, myCurrentCursor, myPreviousCursor, myDiagnostics, &wedgeNiente}; // the reader returns the measure data and any data that needs to be written at // the part-level (e.g. transposition). currently this is done as a pair. auto measureDataPair = reader.getMeasureData(); diff --git a/src/private/mx/impl/SpannerPairing.h b/src/private/mx/impl/SpannerPairing.h new file mode 100644 index 000000000..b1ab7ce86 --- /dev/null +++ b/src/private/mx/impl/SpannerPairing.h @@ -0,0 +1,79 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +#pragma once + +#include "mx/api/SpannerNumber.h" + +#include +#include +#include +#include +#include + +namespace mx +{ +namespace impl +{ +// The bucket a spanner endpoint pairs within: identity endpoints pair by id, explicit endpoints +// by level, and unspecified endpoints with each other. The prefix keeps the three kinds from +// colliding (an identity of "3" is not the explicit level 3). +inline std::string spannerPairingKey(const api::SpannerNumber &inNumber) +{ + switch (inNumber.kind()) + { + case api::SpannerNumber::Kind::explicitLevel: + return "e" + std::to_string(inNumber.level()); + + case api::SpannerNumber::Kind::identity: + return "i" + inNumber.identity(); + + default: + return "u"; + } +} + +// Pairs the starts and stops of one kind of spanner within a part, fed in the order they appear +// in the document. Endpoints pair within their bucket (see spannerPairingKey), and within a +// bucket a stop closes the most recently opened start that is still open. So overlapping +// spanners with distinct numbers never cross, a spanner nested inside another of the same number +// does not steal the outer one's start, and a pair can span measures and staves. Each start +// carries a value that the stop closing it receives. +template class SpannerPairing +{ + public: + void open(const api::SpannerNumber &inNumber, VALUE_TYPE inValue) + { + myOpenStarts[spannerPairingKey(inNumber)].push_back(std::move(inValue)); + } + + // The value of the start this stop closes, or nullopt when no start with its number is open. + std::optional close(const api::SpannerNumber &inNumber) + { + auto &stack = myOpenStarts[spannerPairingKey(inNumber)]; + if (stack.empty()) + { + return std::nullopt; + } + auto value = std::move(stack.back()); + stack.pop_back(); + return value; + } + + // The values of the starts that no stop has closed. + std::vector unclosed() const + { + std::vector result; + for (const auto &keyAndStack : myOpenStarts) + { + result.insert(result.end(), keyAndStack.second.cbegin(), keyAndStack.second.cend()); + } + return result; + } + + private: + std::map> myOpenStarts; +}; +} // namespace impl +} // namespace mx diff --git a/src/private/mx/impl/SpannerResolver.cpp b/src/private/mx/impl/SpannerResolver.cpp index 3cdeae0ca..da15e7ee0 100644 --- a/src/private/mx/impl/SpannerResolver.cpp +++ b/src/private/mx/impl/SpannerResolver.cpp @@ -5,6 +5,7 @@ #include "mx/impl/SpannerResolver.h" #include "mx/api/GlissandoData.h" #include "mx/impl/OttavaFunctions.h" +#include "mx/impl/SpannerPairing.h" #include "mx/impl/WriteRefusal.h" #include "mx/utility/Throw.h" @@ -84,24 +85,18 @@ struct SpannerOttavaEvent api::Location location; }; -// The bucket an endpoint pairs within, used for ottava sizes and for same-note span detection: -// identity endpoints pair by id, explicit endpoints by level, and unspecified endpoints with -// each other. The prefix keeps the three kinds from colliding (an identity of "3" is not the -// explicit level 3). -static std::string spannerPairingKey(const api::SpannerNumber &inNumber) +// One wedge start or stop, in serialized order. Starts carry the hairpin's direction and niente, +// which MusicXML writes on the stop when the hairpin is a diminuendo; stops carry the address the +// writer will present when it asks for that niente. +struct SpannerWedgeEvent { - switch (inNumber.kind()) - { - case api::SpannerNumber::Kind::explicitLevel: - return "e" + std::to_string(inNumber.level()); - - case api::SpannerNumber::Kind::identity: - return "i" + inNumber.identity(); - - default: - return "u"; - } -} + const void *object; + api::SpannerNumber number; + bool isStart; + api::WedgeType wedgeType; + api::Bool niente; + api::Location location; +}; // Collects spanner events in the exact order MeasureWriter serializes them. // The position counter is global; only events within the same class are ever @@ -202,13 +197,21 @@ class SpannerEventCollector { switch (choice.kind()) { - case api::DirectionChoice::Kind::wedgeStart: - add(SpannerNumberClass::wedge, &choice, choice.wedgeStart().number, true, false); + case api::DirectionChoice::Kind::wedgeStart: { + const auto wedgeStart = choice.wedgeStart(); + add(SpannerNumberClass::wedge, &choice, wedgeStart.number, true, false); + myWedgeEvents.push_back(SpannerWedgeEvent{&choice, wedgeStart.number, true, wedgeStart.wedgeType, + wedgeStart.niente, myLocation}); break; + } - case api::DirectionChoice::Kind::wedgeStop: - add(SpannerNumberClass::wedge, &choice, choice.wedgeStop().number, false, true); + case api::DirectionChoice::Kind::wedgeStop: { + const auto wedgeStop = choice.wedgeStop(); + add(SpannerNumberClass::wedge, &choice, wedgeStop.number, false, true); + myWedgeEvents.push_back(SpannerWedgeEvent{&choice, wedgeStop.number, false, api::WedgeType::unspecified, + api::Bool::unspecified, myLocation}); break; + } case api::DirectionChoice::Kind::ottavaStart: { const auto ottavaStart = choice.ottavaStart(); @@ -272,6 +275,11 @@ class SpannerEventCollector return myOttavaEvents; } + const std::vector &wedgeEvents() const + { + return myWedgeEvents; + } + private: void add(SpannerNumberClass inClass, const void *inObject, const api::SpannerNumber &inNumber, bool inOpens, bool inCloses) @@ -307,6 +315,7 @@ class SpannerEventCollector api::Location myLocation; std::map> myEvents; std::vector myOttavaEvents; + std::vector myWedgeEvents; }; // Assigns numbers within one spanner class. Explicit levels reserve their @@ -439,32 +448,64 @@ static void spannerNumberAssignClass(const std::vector &inEv } } -// Pairs each ottava stop with the start it closes and records that start's size. Within one -// pairing bucket a stop closes the most recently opened start still open, so a stop that opens -// and closes inside another ottava of the same bucket does not steal the outer line's start. A -// stop with nothing open is left out of ioResolved and falls back to size 8 at write time. +// Pairs each ottava stop with the start it closes and records that start's size. A stop with +// nothing open is left out of ioResolved and falls back to size 8 at write time. static void spannerResolveOttavaSizes(const std::vector &inEvents, std::unordered_map &ioResolved, const DiagnosticsContext &diagnostics) { - std::map> openStarts; + SpannerPairing pairing; for (const auto &event : inEvents) { - auto &stack = openStarts[spannerPairingKey(event.number)]; if (event.isStart) { - stack.push_back(event.ottavaType); + pairing.open(event.number, event.ottavaType); continue; } - if (stack.empty()) + const auto ottavaType = pairing.close(event.number); + if (!ottavaType.has_value()) { diagnostics.report(api::Severity::warning, api::DiagnosticCode::unmatchedSpanner, event.location, "octave-shift stop has no matching start; using size 8"); continue; } - ioResolved[event.object] = ottavaTypeSize(stack.back()); - stack.pop_back(); + ioResolved[event.object] = ottavaTypeSize(*ottavaType); + } +} + +// Pairs each wedge stop with the start it closes and, when that start is a diminuendo with a +// niente value, records the value for the stop: MusicXML writes a diminuendo's niente on its +// stop. A diminuendo with niente and no stop has nowhere to write it, which is reported. +static void spannerResolveWedgeNiente(const std::vector &inEvents, + std::unordered_map &ioResolved, + const DiagnosticsContext &diagnostics) +{ + SpannerPairing pairing; + + for (const auto &event : inEvents) + { + if (event.isStart) + { + pairing.open(event.number, &event); + continue; + } + const auto start = pairing.close(event.number); + if (start.has_value() && (*start)->wedgeType == api::WedgeType::diminuendo && + (*start)->niente != api::Bool::unspecified) + { + ioResolved[event.object] = (*start)->niente; + } + } + + for (const auto *start : pairing.unclosed()) + { + if (start->wedgeType == api::WedgeType::diminuendo && start->niente != api::Bool::unspecified) + { + diagnostics.report(api::Severity::warning, api::DiagnosticCode::droppedData, start->location, + "a diminuendo's niente is written on its stop, and this diminuendo has no stop; " + "niente is not written"); + } } } @@ -679,6 +720,7 @@ void SpannerResolver::resolvePart(const api::PartData &inPart, int partIndex, Di } spannerResolveOttavaSizes(collector.ottavaEvents(), myOttavaStopSizes, diagnostics); + spannerResolveWedgeNiente(collector.wedgeEvents(), myWedgeStopNiente, diagnostics); } std::optional SpannerResolver::emittedNumber(const api::SpannerNumber &inNumber, const void *inObject) const @@ -713,6 +755,16 @@ int SpannerResolver::ottavaStopSize(const void *inObject) const return iter->second; } +api::Bool SpannerResolver::wedgeStopNiente(const void *inObject) const +{ + const auto iter = myWedgeStopNiente.find(inObject); + if (iter == myWedgeStopNiente.cend()) + { + return api::Bool::unspecified; + } + return iter->second; +} + const void *SpannerResolver::sameNoteSpanPartner(const void *inObject) const { const auto iter = mySameNoteSpanPartners.find(inObject); diff --git a/src/private/mx/impl/SpannerResolver.h b/src/private/mx/impl/SpannerResolver.h index 1ea8cf762..16c0dfd4e 100644 --- a/src/private/mx/impl/SpannerResolver.h +++ b/src/private/mx/impl/SpannerResolver.h @@ -16,8 +16,9 @@ namespace impl { // Resolves the writer-side facts that a spanner endpoint cannot know on its // own, in one pass over a part before the part is written: the MusicXML -// 'number' attribute for every spanner start/continue/stop, and the size an -// octave-shift stop inherits from the start it closes. +// 'number' attribute for every spanner start/continue/stop, and the facts a +// stop inherits from the start it closes (an octave-shift's size, a +// diminuendo's niente). // // == Numbers == // @@ -93,14 +94,24 @@ namespace impl // carry distinct numbers or identities never cross, and pairing spans // measures. A stop with no open start is left unpaired and falls back to // MusicXML's default size of 8. +// +// == Wedge stop niente == +// +// Whether a hairpin's point is drawn as a circle is stated by +// api::WedgeStart::niente alone. MusicXML puts it on the start of a crescendo +// but on the stop of a diminuendo, so the walk pairs each wedge stop with the +// start it closes, by the same rule as octave-shifts (see SpannerPairing.h), +// and records a diminuendo start's niente for its stop. class SpannerResolver { public: SpannerResolver() = default; // Walks inPart in serialization order, assigning a number to every - // identity spanner event and a size to every ottava stop. Reports an - // unmatched ottava stop through diagnostics. May be called once per part + // identity spanner event, a size to every ottava stop, and a niente to + // every wedge stop that closes a diminuendo. Reports unmatched spanner + // endpoints, and a diminuendo niente with no stop to carry it, through + // diagnostics. May be called once per part // of a score; results accumulate (object addresses are unique across parts). void resolvePart(const api::PartData &inPart, int partIndex, DiagnosticsContext diagnostics = {}); @@ -119,6 +130,12 @@ class SpannerResolver // gets MusicXML's default size of 8. int ottavaStopSize(const void *inObject) const; + // The niente the writer should emit for the given wedge stop: the value of + // the diminuendo start it closes. inObject must be the address of the same + // object resolvePart visited. A stop that closes a crescendo, closes + // nothing, or was written outside a resolved part gets unspecified. + api::Bool wedgeStopNiente(const void *inObject) const; + // The other end of a spanner that starts and stops on the same note, or // nullptr when the spanner extends beyond its note. inObject must be the // address of the same start/stop object that resolvePart visited. Tracked @@ -129,6 +146,7 @@ class SpannerResolver private: std::unordered_map myResolved; std::unordered_map myOttavaStopSizes; + std::unordered_map myWedgeStopNiente; std::unordered_map mySameNoteSpanPartners; }; } // namespace impl diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index 5ab8684f6..567b1716f 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -14,6 +14,10 @@ #include "mxtest/api/TestHelpers.h" #include "mxtest/file/Path.h" +#include +#include +#include + using namespace std; using namespace mx::api; using namespace mxtest; @@ -483,6 +487,319 @@ TEST(WedgeUnspecifiedColorNoPhantomAttribute, DirectionData) T_END; +// A score with one part, one staff and inMeasures measures of one whole note each, at +// ticksPerQuarter 10, for the wedge niente tests below. +inline ScoreData wedgeNienteTestScore(int inMeasures) +{ + ScoreData score; + score.ticksPerQuarter = 10; + score.parts.emplace_back(); + auto &part = score.parts.back(); + for (int i = 0; i < inMeasures; ++i) + { + part.measures.emplace_back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(); + NoteData note{}; + note.tickTimePosition = 0; + note.durationData.durationTimeTicks = 40; + note.durationData.durationName = DurationName::whole; + measure.staves.back().voices[0].notes.push_back(note); + } + return score; +} + +// Adds a direction holding inChoice to the first staff of measure inMeasureIndex. +inline void wedgeNienteTestAdd(ScoreData &ioScore, int inMeasureIndex, int inTick, DirectionChoice inChoice) +{ + DirectionData direction; + direction.tickTimePosition = inTick; + direction.directionTypes.emplace_back(std::move(inChoice)); + ioScore.parts.front() + .measures.at(static_cast(inMeasureIndex)) + .staves.front() + .directions.push_back(direction); +} + +inline WedgeStart wedgeNienteTestStart(WedgeType inType, Bool inNiente, SpannerNumber inNumber = SpannerNumber{}) +{ + WedgeStart start; + start.wedgeType = inType; + start.niente = inNiente; + start.number = inNumber; + return start; +} + +inline WedgeStop wedgeNienteTestStop(SpannerNumber inNumber = SpannerNumber{}) +{ + WedgeStop stop; + stop.number = inNumber; + return stop; +} + +// Every tag in the XML, in document order. +inline std::vector wedgeNienteTestTags(const std::string &inXml) +{ + std::vector tags; + auto begin = inXml.find("', begin); + tags.push_back(inXml.substr(begin, end - begin + 1)); + begin = inXml.find(" wedgeNienteTestStarts(const ScoreData &inScore) +{ + std::vector starts; + for (const auto &measure : inScore.parts.front().measures) + { + for (const auto &staff : measure.staves) + { + for (const auto &direction : staff.directions) + { + for (const auto &choice : direction.directionTypes) + { + if (choice.isWedgeStart()) + { + starts.push_back(choice.wedgeStart()); + } + } + } + } + } + return starts; +} + +inline Diagnostics wedgeNienteTestWriteDiagnostics(const ScoreData &inScore) +{ + Diagnostics diagnostics; + REQUIRE(fromScore(inScore, diagnostics).ok()); + return diagnostics; +} + +inline int wedgeNienteTestCount(const Diagnostics &inDiagnostics, DiagnosticCode inCode) +{ + int count = 0; + for (const auto &diagnostic : inDiagnostics.all()) + { + if (diagnostic.code == inCode) + { + ++count; + } + } + return count; +} + +// A crescendo out of nothing states niente on its start, the hairpin's point, even with no stop. +TEST(WedgeNienteCrescendoWithoutStop, DirectionData) +{ + auto score = wedgeNienteTestScore(1); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::crescendo, Bool::yes)}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 1); + CHECK(tags[0].find(R"(type="crescendo")") != std::string::npos); + CHECK(tags[0].find(R"(niente="yes")") != std::string::npos); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 1); + CHECK(Bool::yes == starts[0].niente); +} + +T_END; + +// A wedge with an unspecified direction is written as a crescendo, so its niente goes on the start. +TEST(WedgeNienteUnspecifiedTypeIsWrittenOnStart, DirectionData) +{ + auto score = wedgeNienteTestScore(1); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::unspecified, Bool::yes)}); + wedgeNienteTestAdd(score, 0, 20, DirectionChoice{wedgeNienteTestStop()}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 2); + CHECK(tags[0].find(R"(type="crescendo")") != std::string::npos); + CHECK(tags[0].find(R"(niente="yes")") != std::string::npos); + CHECK(tags[1].find("niente") == std::string::npos); +} + +T_END; + +// A diminuendo to nothing is set on the start but written on the stop, here in a later measure. +TEST(WedgeNienteDiminuendoStopInLaterMeasure, DirectionData) +{ + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes)}); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStop()}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 2); + CHECK(tags[0].find(R"(type="diminuendo")") != std::string::npos); + CHECK(tags[0].find("niente") == std::string::npos); + CHECK(tags[1].find(R"(type="stop")") != std::string::npos); + CHECK(tags[1].find(R"(niente="yes")") != std::string::npos); + CHECK(0 == wedgeNienteTestCount(wedgeNienteTestWriteDiagnostics(score), DiagnosticCode::droppedData)); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 1); + CHECK(Bool::yes == starts[0].niente); +} + +T_END; + +// Bool::no is written where MusicXML expects it for both directions and reads back as no. +TEST(WedgeNienteNoOnBothShapes, DirectionData) +{ + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::crescendo, Bool::no)}); + wedgeNienteTestAdd(score, 0, 20, DirectionChoice{wedgeNienteTestStop()}); + wedgeNienteTestAdd(score, 1, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::no)}); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStop()}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 4); + CHECK(tags[0].find(R"(niente="no")") != std::string::npos); + CHECK(tags[1].find("niente") == std::string::npos); + CHECK(tags[2].find("niente") == std::string::npos); + CHECK(tags[3].find(R"(niente="no")") != std::string::npos); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 2); + CHECK(Bool::no == starts[0].niente); + CHECK(Bool::no == starts[1].niente); +} + +T_END; + +// A hairpin with niente left unspecified does not write a niente attribute on either end. +TEST(WedgeNienteUnspecifiedIsNotWritten, DirectionData) +{ + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::crescendo, Bool::unspecified)}); + wedgeNienteTestAdd(score, 0, 20, DirectionChoice{wedgeNienteTestStop()}); + wedgeNienteTestAdd(score, 1, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::unspecified)}); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStop()}); + + const auto xml = mxtest::toXml(score); + CHECK(wedgeNienteTestTags(xml).size() == 4); + CHECK(xml.find("niente") == std::string::npos); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 2); + CHECK(Bool::unspecified == starts[0].niente); + CHECK(Bool::unspecified == starts[1].niente); +} + +T_END; + +// Two overlapping diminuendos told apart by number: each stop carries its own start's niente, and +// each start reads back its own value. +TEST(WedgeNienteOverlappingNumberedDiminuendos, DirectionData) +{ + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 0, 0, + DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes, SpannerNumber{1})}); + wedgeNienteTestAdd(score, 0, 20, + DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::no, SpannerNumber{2})}); + wedgeNienteTestAdd(score, 1, 0, DirectionChoice{wedgeNienteTestStop(SpannerNumber{2})}); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStop(SpannerNumber{1})}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 4); + CHECK(tags[0].find("niente") == std::string::npos); + CHECK(tags[1].find("niente") == std::string::npos); + CHECK(tags[2].find(R"(number="2")") != std::string::npos); + CHECK(tags[2].find(R"(niente="no")") != std::string::npos); + CHECK(tags[3].find(R"(number="1")") != std::string::npos); + CHECK(tags[3].find(R"(niente="yes")") != std::string::npos); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 2); + CHECK(Bool::yes == starts[0].niente); + CHECK(Bool::no == starts[1].niente); +} + +T_END; + +// A diminuendo's niente is written on its stop, so a diminuendo with no stop cannot write it. +TEST(WedgeNienteDiminuendoWithoutStopIsReported, DirectionData) +{ + auto score = wedgeNienteTestScore(1); + wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes)}); + + CHECK(mxtest::toXml(score).find("niente") == std::string::npos); + const auto diagnostics = wedgeNienteTestWriteDiagnostics(score); + REQUIRE(1 == wedgeNienteTestCount(diagnostics, DiagnosticCode::droppedData)); + for (const auto &diagnostic : diagnostics.all()) + { + if (diagnostic.code == DiagnosticCode::droppedData) + { + CHECK(Severity::warning == diagnostic.severity); + CHECK(diagnostic.message.find("niente") != std::string::npos); + } + } +} + +T_END; + +// niente read from a file: on a diminuendo's stop it belongs to the diminuendo, while on the wide +// end of a hairpin (a diminuendo's start, a crescendo's stop) or on a continue segment it means +// nothing and is reported. A continue segment does not close the hairpin it continues. +TEST(WedgeNienteReadFromFile, DirectionData) +{ + const std::string xml = R"( + + One + + + 1 + + + + C44whole + + + + + + + C44whole + + +)"; + + std::istringstream stream{xml}; + auto document = MusicXml::fromStream(stream); + REQUIRE(document.ok()); + Diagnostics diagnostics; + const auto score = getScore(document.value(), diagnostics); + REQUIRE(score.ok()); + + const auto starts = wedgeNienteTestStarts(score.value()); + REQUIRE(starts.size() == 4); + CHECK(Bool::yes == starts[0].niente); + CHECK(Bool::unspecified == starts[1].niente); + CHECK(Bool::unspecified == starts[2].niente); + CHECK(Bool::unspecified == starts[3].niente); + CHECK(3 == wedgeNienteTestCount(diagnostics, DiagnosticCode::droppedData)); +} + +T_END; + +TEST(WedgeStartEqualitySeesNiente, DirectionData) +{ + WedgeStart a; + WedgeStart b; + CHECK(a == b); + b.niente = Bool::yes; + CHECK(a != b); +} + +T_END; + // A note for the tests below, which use ticksPerQuarter 10. inline NoteData directionDataTestNote(int inTick, int inTicks, DurationName inName) { diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index f46b0615c..35db05890 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -887,3 +887,9 @@ synthetic/creator.3.0.xml # how much time a grace note takes from the note before or after it, or adds; only slash was kept. lysuite/ly24d_AfterGrace.xml synthetic/grace.3.0.xml + +# Unblocked by WedgeStart::niente. , the small circle at a hairpin's point that +# marks a crescendo out of nothing or a diminuendo to nothing, was dropped on read. A diminuendo +# states it on its stop, so the writer puts it there. +synthetic/wedge.3.0.xml +synthetic/wedge.3.1.xml From 20fc4e906b399f70bdb645fe45bfa8605a6d4856 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Fri, 6 Nov 2026 05:53:53 +0100 Subject: [PATCH 2/3] fix: pair a wedge continue as a start and report a lost niente as an error The reader reads a as a WedgeStart of unspecified type, which the writer writes and pairs as a crescendo start. The niente pre-pass skipped continues instead, so after a diminuendo, a continue and a stop it gave the stop's niente to the diminuendo, which the writer then could not write: the stop now ends the continue. The pre-pass opens on every wedge that is not a stop, so the reader and the writer pair a stop with the same start. A niente that is not read or not written is reported as dropped data with error severity, as every other dropped value is. The WedgeStart::niente comment now says how yes, no and unspecified are drawn, that a diminuendo's circle needs the WedgeStop that ends it, and that a hairpin of unspecified type carries the circle on its start. Tests cover the continue case, a reused number, both values on a diminuendo with no stop, the location of each diagnostic, and two unnumbered diminuendos open at once on different staves, built through the api and read from a file whose order differs from the order mx writes. --- src/include/mx/api/WedgeData.h | 11 +- src/private/mx/impl/DirectionReader.cpp | 41 ++-- src/private/mx/impl/SpannerResolver.cpp | 2 +- src/private/mxtest/api/DirectionDataTest.cpp | 235 +++++++++++++++++-- 4 files changed, 238 insertions(+), 51 deletions(-) diff --git a/src/include/mx/api/WedgeData.h b/src/include/mx/api/WedgeData.h index e963c542a..826035e91 100644 --- a/src/include/mx/api/WedgeData.h +++ b/src/include/mx/api/WedgeData.h @@ -36,11 +36,14 @@ struct WedgeStart bool isColorSpecified; ColorData colorData; - // Whether the point of the hairpin is drawn as a small circle (niente): a crescendo that grows + // Whether a small circle is drawn at the point of the hairpin (niente): a crescendo that grows // out of nothing, or a diminuendo that fades away to nothing. Set it here on the start for - // either direction. MusicXML puts it on the start of a crescendo but on the stop of a - // diminuendo, and mx writes it in the right place for you. Leave it unspecified for a plain - // point. + // either direction; yes draws the circle, and no or unspecified leave a plain point. MusicXML + // states it at the point, on the start of a crescendo and on the stop of a diminuendo, and mx + // writes it there for you. So a diminuendo's circle is written only when the diminuendo has a + // WedgeStop that ends it; without one the circle is left out and a diagnostic says so. A + // hairpin whose wedgeType is unspecified is written as a crescendo, with the circle on its + // start. Bool niente; // The element's id attribute (see Id.h). diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index 296a13496..12b653608 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -118,7 +118,9 @@ DirectionReader::DirectionReader(const core::Harmony &inHarmony, MeasureCursor i WedgeNiente DirectionReader::scanWedgeNiente(const core::PartwisePart &inPart) { - // A continue segment neither opens nor closes a hairpin, so it takes no part in the pairing. + // Every wedge that is not a stop opens. A continue segment is read as a WedgeStart, which is + // written as a crescendo start, so it opens here too and a stop pairs with the same start on + // read as on write. struct OpenWedge { const core::Wedge *wedge; @@ -145,31 +147,20 @@ WedgeNiente DirectionReader::scanWedgeNiente(const core::PartwisePart &inPart) const auto &wedge = directionType.choice().asWedge(); const auto number = wedge.number().has_value() ? api::SpannerNumber(wedge.number()->value()) : api::SpannerNumber{}; - switch (wedge.type().tag()) + const auto tag = wedge.type().tag(); + if (tag != core::WedgeType::Tag::stop) { - case core::WedgeType::Tag::crescendo: - pairing.open(number, OpenWedge{&wedge, false}); - break; - - case core::WedgeType::Tag::diminuendo: - pairing.open(number, OpenWedge{&wedge, true}); - break; - - case core::WedgeType::Tag::stop: { - const auto start = pairing.close(number); - if (start.has_value() && start->isDiminuendo) + pairing.open(number, OpenWedge{&wedge, tag == core::WedgeType::Tag::diminuendo}); + continue; + } + const auto start = pairing.close(number); + if (start.has_value() && start->isDiminuendo) + { + result.diminuendoStops.insert(&wedge); + if (wedge.niente().has_value()) { - result.diminuendoStops.insert(&wedge); - if (wedge.niente().has_value()) - { - result.diminuendoNiente[start->wedge] = converter.convert(*wedge.niente()); - } + result.diminuendoNiente[start->wedge] = converter.convert(*wedge.niente()); } - break; - } - - default: - break; } } } @@ -614,7 +605,7 @@ void DirectionReader::parseWedge(const core::DirectionType &directionType) const bool closesDiminuendo = myWedgeNiente && myWedgeNiente->diminuendoStops.count(&wedge) > 0; if (wedge.niente().has_value() && !closesDiminuendo) { - report(api::Severity::warning, api::DiagnosticCode::droppedData, + report(api::Severity::error, api::DiagnosticCode::droppedData, "wedge niente is only read on the start of a crescendo or the stop of a diminuendo; " "niente on a stop that does not close a diminuendo is not read"); } @@ -654,7 +645,7 @@ api::Bool DirectionReader::parseWedgeStartNiente(const core::Wedge &wedge) const if (wedge.niente().has_value()) { - report(api::Severity::warning, api::DiagnosticCode::droppedData, + report(api::Severity::error, api::DiagnosticCode::droppedData, tag == core::WedgeType::Tag::diminuendo ? "wedge niente is only read on the start of a crescendo or the stop of a diminuendo; niente on " "the start of a diminuendo is not read" diff --git a/src/private/mx/impl/SpannerResolver.cpp b/src/private/mx/impl/SpannerResolver.cpp index da15e7ee0..270fb4ceb 100644 --- a/src/private/mx/impl/SpannerResolver.cpp +++ b/src/private/mx/impl/SpannerResolver.cpp @@ -502,7 +502,7 @@ static void spannerResolveWedgeNiente(const std::vector &inEv { if (start->wedgeType == api::WedgeType::diminuendo && start->niente != api::Bool::unspecified) { - diagnostics.report(api::Severity::warning, api::DiagnosticCode::droppedData, start->location, + diagnostics.report(api::Severity::error, api::DiagnosticCode::droppedData, start->location, "a diminuendo's niente is written on its stop, and this diminuendo has no stop; " "niente is not written"); } diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index 567b1716f..390603985 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -724,30 +724,56 @@ TEST(WedgeNienteOverlappingNumberedDiminuendos, DirectionData) T_END; -// A diminuendo's niente is written on its stop, so a diminuendo with no stop cannot write it. -TEST(WedgeNienteDiminuendoWithoutStopIsReported, DirectionData) +// Reads inXml, failing the test when it does not read. +inline ScoreData wedgeNienteTestRead(const std::string &inXml, Diagnostics &outDiagnostics) { - auto score = wedgeNienteTestScore(1); - wedgeNienteTestAdd(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes)}); + std::istringstream stream{inXml}; + auto document = MusicXml::fromStream(stream); + REQUIRE(document.ok()); + auto score = getScore(document.value(), outDiagnostics); + REQUIRE(score.ok()); + return score.value(); +} - CHECK(mxtest::toXml(score).find("niente") == std::string::npos); - const auto diagnostics = wedgeNienteTestWriteDiagnostics(score); - REQUIRE(1 == wedgeNienteTestCount(diagnostics, DiagnosticCode::droppedData)); - for (const auto &diagnostic : diagnostics.all()) +// The dropped-data diagnostics in inDiagnostics, each checked to be an error about niente. +inline std::vector wedgeNienteTestDropped(const Diagnostics &inDiagnostics) +{ + std::vector dropped; + for (const auto &diagnostic : inDiagnostics.all()) { if (diagnostic.code == DiagnosticCode::droppedData) { - CHECK(Severity::warning == diagnostic.severity); + CHECK(Severity::error == diagnostic.severity); CHECK(diagnostic.message.find("niente") != std::string::npos); + dropped.push_back(diagnostic); } } + return dropped; +} + +// A diminuendo's niente is written on its stop, so a diminuendo with no stop cannot write it. A +// stated no is lost the same way as a yes, so both are reported, once, where the diminuendo starts. +TEST(WedgeNienteDiminuendoWithoutStopIsReported, DirectionData) +{ + for (const auto niente : {Bool::yes, Bool::no}) + { + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, niente)}); + + CHECK(mxtest::toXml(score).find("niente") == std::string::npos); + const auto dropped = wedgeNienteTestDropped(wedgeNienteTestWriteDiagnostics(score)); + REQUIRE(dropped.size() == 1); + CHECK(0 == dropped[0].location.partIndex); + CHECK(1 == dropped[0].location.measureIndex); + CHECK(20 == dropped[0].location.tickTimePosition); + } } T_END; // niente read from a file: on a diminuendo's stop it belongs to the diminuendo, while on the wide -// end of a hairpin (a diminuendo's start, a crescendo's stop) or on a continue segment it means -// nothing and is reported. A continue segment does not close the hairpin it continues. +// end of a hairpin (a diminuendo's start, a crescendo's stop) it means nothing and is reported, +// once, in the measure where it appears. TEST(WedgeNienteReadFromFile, DirectionData) { const std::string xml = R"( @@ -762,7 +788,6 @@ TEST(WedgeNienteReadFromFile, DirectionData) C44whole - @@ -771,20 +796,188 @@ TEST(WedgeNienteReadFromFile, DirectionData) )"; - std::istringstream stream{xml}; - auto document = MusicXml::fromStream(stream); - REQUIRE(document.ok()); Diagnostics diagnostics; - const auto score = getScore(document.value(), diagnostics); - REQUIRE(score.ok()); + const auto score = wedgeNienteTestRead(xml, diagnostics); - const auto starts = wedgeNienteTestStarts(score.value()); - REQUIRE(starts.size() == 4); + const auto starts = wedgeNienteTestStarts(score); + REQUIRE(starts.size() == 3); CHECK(Bool::yes == starts[0].niente); CHECK(Bool::unspecified == starts[1].niente); CHECK(Bool::unspecified == starts[2].niente); - CHECK(Bool::unspecified == starts[3].niente); - CHECK(3 == wedgeNienteTestCount(diagnostics, DiagnosticCode::droppedData)); + + const auto dropped = wedgeNienteTestDropped(diagnostics); + REQUIRE(dropped.size() == 2); + CHECK(0 == dropped[0].location.measureIndex); + CHECK(1 == dropped[1].location.measureIndex); +} + +T_END; + +// The reader takes a continue segment as the start of a hairpin of unspecified direction, which is +// written as a crescendo. So a stop after a continue ends the continue, on read and on write alike: +// its niente is not given to the diminuendo before the continue, which would then have no stop to +// write it on. The value is reported where it is read, and it is not reported again on write. +TEST(WedgeNienteContinueSegmentEndsAtTheNextStop, DirectionData) +{ + const std::string xml = R"( + + One + + + 1 + + C44whole + + + + C44whole + + + + C44whole + + +)"; + + Diagnostics readDiagnostics; + const auto score = wedgeNienteTestRead(xml, readDiagnostics); + const auto starts = wedgeNienteTestStarts(score); + REQUIRE(starts.size() == 2); + CHECK(WedgeType::diminuendo == starts[0].wedgeType); + CHECK(Bool::unspecified == starts[0].niente); + CHECK(WedgeType::unspecified == starts[1].wedgeType); + CHECK(Bool::unspecified == starts[1].niente); + const auto dropped = wedgeNienteTestDropped(readDiagnostics); + REQUIRE(dropped.size() == 1); + CHECK(2 == dropped[0].location.measureIndex); + + CHECK(mxtest::toXml(score).find("niente") == std::string::npos); + CHECK(wedgeNienteTestDropped(wedgeNienteTestWriteDiagnostics(score)).empty()); + const auto roundTripped = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(roundTripped.size() == 2); + CHECK(Bool::unspecified == roundTripped[0].niente); + CHECK(Bool::unspecified == roundTripped[1].niente); +} + +T_END; + +// A number used again after its hairpin ends starts a new hairpin, and each stop carries the +// niente of the start it ends. +TEST(WedgeNienteReusedNumber, DirectionData) +{ + auto score = wedgeNienteTestScore(2); + wedgeNienteTestAdd(score, 0, 0, + DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes, SpannerNumber{1})}); + wedgeNienteTestAdd(score, 0, 20, DirectionChoice{wedgeNienteTestStop(SpannerNumber{1})}); + wedgeNienteTestAdd(score, 1, 0, + DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::no, SpannerNumber{1})}); + wedgeNienteTestAdd(score, 1, 20, DirectionChoice{wedgeNienteTestStop(SpannerNumber{1})}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 4); + CHECK(tags[1].find(R"(niente="yes")") != std::string::npos); + CHECK(tags[3].find(R"(niente="no")") != std::string::npos); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 2); + CHECK(Bool::yes == starts[0].niente); + CHECK(Bool::no == starts[1].niente); +} + +T_END; + +// A two-staff part of inMeasures measures of one whole note per staff, at ticksPerQuarter 10. +inline ScoreData wedgeNienteTestTwoStaffScore(int inMeasures) +{ + auto score = wedgeNienteTestScore(inMeasures); + for (auto &measure : score.parts.front().measures) + { + measure.staves.push_back(measure.staves.front()); + } + return score; +} + +inline void wedgeNienteTestAddToStaff(ScoreData &ioScore, int inMeasureIndex, int inStaffIndex, + DirectionChoice inChoice) +{ + DirectionData direction; + direction.directionTypes.emplace_back(std::move(inChoice)); + ioScore.parts.front() + .measures.at(static_cast(inMeasureIndex)) + .staves.at(static_cast(inStaffIndex)) + .directions.push_back(direction); +} + +// Two unnumbered diminuendos open at once on different staves of a part. Wedges pair within the +// part in document order, a stop ending the most recent open start, and the writer pairs in the +// order it writes. So the stop that carries the second diminuendo's niente is the first stop +// written, and reading the document back gives the niente to the same diminuendo. +TEST(WedgeNienteUnnumberedOnTwoStaves, DirectionData) +{ + auto score = wedgeNienteTestTwoStaffScore(2); + wedgeNienteTestAddToStaff(score, 0, 0, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::no)}); + wedgeNienteTestAddToStaff(score, 0, 1, DirectionChoice{wedgeNienteTestStart(WedgeType::diminuendo, Bool::yes)}); + wedgeNienteTestAddToStaff(score, 1, 0, DirectionChoice{wedgeNienteTestStop()}); + wedgeNienteTestAddToStaff(score, 1, 1, DirectionChoice{wedgeNienteTestStop()}); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 4); + CHECK(tags[0].find("niente") == std::string::npos); + CHECK(tags[1].find("niente") == std::string::npos); + CHECK(tags[2].find(R"(niente="yes")") != std::string::npos); + CHECK(tags[3].find(R"(niente="no")") != std::string::npos); + CHECK(wedgeNienteTestDropped(wedgeNienteTestWriteDiagnostics(score)).empty()); + + const auto starts = wedgeNienteTestStarts(mxtest::roundTrip(score)); + REQUIRE(starts.size() == 2); + CHECK(Bool::no == starts[0].niente); + CHECK(Bool::yes == starts[1].niente); +} + +T_END; + +// The same two diminuendos read from a file that lists the lower staff's start first. The reader +// pairs in the file's order, so the upper staff's stop ends the upper staff's diminuendo. mx writes +// the upper staff first, so the written stops pair the other way round and the niente moves to the +// stop that now ends the lower staff's diminuendo; it still belongs to that diminuendo when read +// back. +TEST(WedgeNienteUnnumberedOnTwoStavesFromFile, DirectionData) +{ + const std::string xml = R"( + + One + + + 12 + 2 + 1 + C54whole1 + 4 + C34whole2 + + + 1 + C54whole1 + 4 + 2 + C34whole2 + + +)"; + + Diagnostics diagnostics; + const auto score = wedgeNienteTestRead(xml, diagnostics); + CHECK(wedgeNienteTestDropped(diagnostics).empty()); + const auto starts = wedgeNienteTestStarts(score); + REQUIRE(starts.size() == 2); + CHECK(Bool::unspecified == starts[0].niente); + CHECK(Bool::yes == starts[1].niente); + + const auto tags = wedgeNienteTestTags(mxtest::toXml(score)); + REQUIRE(tags.size() == 4); + CHECK(tags[2].find(R"(niente="yes")") != std::string::npos); + CHECK(tags[3].find("niente") == std::string::npos); + CHECK(starts == wedgeNienteTestStarts(mxtest::roundTrip(score))); } T_END; From 80dd597b05acc3da77af5a1a11fd061a777af326 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Fri, 6 Nov 2026 05:53:53 +0100 Subject: [PATCH 3/3] test: cover nested and number-1 ottava sizes across staves An unnumbered ottava nested in another, and an unnumbered ottava beside one numbered 1 on another staff, each take their stop size from their own start. --- src/private/mxtest/api/OttavaSizeApiTest.cpp | 66 ++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/private/mxtest/api/OttavaSizeApiTest.cpp b/src/private/mxtest/api/OttavaSizeApiTest.cpp index 8e9446d09..68f473fea 100644 --- a/src/private/mxtest/api/OttavaSizeApiTest.cpp +++ b/src/private/mxtest/api/OttavaSizeApiTest.cpp @@ -241,6 +241,72 @@ TEST(OttavaStopSizeSequentialUnnumbered, OttavaSize) T_END; +// An unnumbered ottava nested inside another unnumbered ottava: the first stop ends the inner line, +// so the outer line keeps its own size for its stop. +TEST(OttavaStopSizeNestedUnnumbered, OttavaSize) +{ + auto score = ottavaSizeMakeScore(4); + + OttavaStart outer; + outer.ottavaType = OttavaType::o15ma; + ottavaSizeAddDirection(score, 0, DirectionChoice{outer}); + + OttavaStart inner; + inner.ottavaType = OttavaType::o8vb; + ottavaSizeAddDirection(score, 1, DirectionChoice{inner}); + ottavaSizeAddDirection(score, 2, DirectionChoice{OttavaStop{}}); + ottavaSizeAddDirection(score, 3, DirectionChoice{OttavaStop{}}); + + const auto shifts = ottavaSizeWrittenShifts(mxtest::toXml(score)); + REQUIRE(shifts.size() == 4); + CHECK_EQUAL(std::string{"8"}, shifts.at(2).size); + CHECK_EQUAL(std::string{"15"}, shifts.at(3).size); +} + +T_END; + +// An ottava without a number and one numbered 1 are separate lines, here on different staves of a +// part. The stop numbered 1 ends the line numbered 1 even though the unnumbered line opened later. +TEST(OttavaStopSizeUnnumberedAndNumberOneAreSeparateAcrossStaves, OttavaSize) +{ + auto score = ottavaSizeMakeScore(2); + for (auto &measure : score.parts.front().measures) + { + measure.staves.push_back(measure.staves.front()); + } + const auto add = [&score](int inMeasureIndex, int inStaffIndex, const DirectionChoice &inChoice) { + DirectionData direction; + direction.directionTypes.emplace_back(inChoice); + score.parts.front() + .measures.at(static_cast(inMeasureIndex)) + .staves.at(static_cast(inStaffIndex)) + .directions.emplace_back(direction); + }; + + OttavaStart numbered; + numbered.ottavaType = OttavaType::o8va; + numbered.spannerStart.number = SpannerNumber(1); + add(0, 0, DirectionChoice{numbered}); + + OttavaStart unnumbered; + unnumbered.ottavaType = OttavaType::o15mb; + add(0, 1, DirectionChoice{unnumbered}); + + OttavaStop numberedStop; + numberedStop.spannerStop.number = SpannerNumber(1); + add(1, 0, DirectionChoice{numberedStop}); + add(1, 1, DirectionChoice{OttavaStop{}}); + + const auto shifts = ottavaSizeWrittenShifts(mxtest::toXml(score)); + REQUIRE(shifts.size() == 4); + CHECK_EQUAL(std::string{"stop"}, shifts.at(2).type); + CHECK_EQUAL(std::string{"8"}, shifts.at(2).size); + CHECK_EQUAL(std::string{"stop"}, shifts.at(3).type); + CHECK_EQUAL(std::string{"15"}, shifts.at(3).size); +} + +T_END; + // A stop with no start is an authoring error, not a crash: it writes MusicXML's default size of 8 // and the document still reads back. TEST(OttavaStopSizeDanglingStopFallsBackToEight, OttavaSize)