From d5cc2cc1c0fc429fc7249013dae25bd1a1925c97 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 20 Sep 2026 18:33:42 +0200 Subject: [PATCH 1/3] feat: tuplet numbering and portion fidelity TupletStart and TupletStop now carry a SpannerNumber instead of a raw number level, and tuplets join the other spanner families in SpannerResolver, so an author who labels both ends of a tuplet gets the numbers assigned for them. Nested tuplets overlap in the serialized stream and are handed 1 and 2; tuplets that follow one another both get 1. Two tuplets open at once with no number are reported. A tuplet may spell its ratio out in tuplet-actual and tuplet-normal or leave both out and let the note's time-modification speak for it. The reader records which shape the file used, and the writer states a portion only when it says something the note does not already say, which is what the inner tuplet of a nested pair needs. writeActual and writeNormal force the answer either way. Inside a portion, only the parts that were stated are written; an unspecified note value no longer turns into a maxima. Adds show-type, line-shape, and bracket on a tuplet stop. --- src/include/mx/api/ApiCommon.h | 2 +- src/include/mx/api/TupletData.h | 69 +++- src/private/mx/impl/Converter.cpp | 15 + src/private/mx/impl/Converter.h | 6 + src/private/mx/impl/NotationsWriter.cpp | 220 ++++++++--- src/private/mx/impl/SpannerResolver.cpp | 57 ++- src/private/mx/impl/SpannerResolver.h | 11 +- src/private/mx/impl/TupletReader.cpp | 41 ++- src/private/mxtest/api/NoteDataTest.cpp | 28 +- .../mxtest/api/SingleNoteSpannerApiTest.cpp | 14 +- src/private/mxtest/api/TupletApiTest.cpp | 342 ++++++++++++++++++ .../mxtest/api/TupletNumberApiTest.cpp | 219 +++++++++++ 12 files changed, 930 insertions(+), 94 deletions(-) create mode 100644 src/private/mxtest/api/TupletApiTest.cpp create mode 100644 src/private/mxtest/api/TupletNumberApiTest.cpp diff --git a/src/include/mx/api/ApiCommon.h b/src/include/mx/api/ApiCommon.h index 3ff4433aa..7c4cbc8b8 100644 --- a/src/include/mx/api/ApiCommon.h +++ b/src/include/mx/api/ApiCommon.h @@ -28,7 +28,7 @@ constexpr int TICK_TIME_INFINITY = std::numeric_limits::max(); // "unspecified" or "absent". Do not use these for new fields; new absent-able fields use // std::optional instead (see "mx::api conventions" in AGENTS.md and issue #249). constexpr int INDEX_UNSPECIFIED = -1; // staff/part indices and numbers, e.g. KeyData::staffIndex -constexpr int NUMBER_LEVEL_UNSPECIFIED = -1; // MusicXML 'number' attributes, e.g. TupletStart::numberLevel +constexpr int NUMBER_LEVEL_UNSPECIFIED = -1; // MusicXML 'number' attributes, e.g. SpannerNumber::level constexpr int VALUE_UNSPECIFIED = -1; // other absent-able ints, e.g. DirectionData::voice constexpr Double DOUBLE_UNSPECIFIED = -1.0; // absent-able doubles, e.g. StaffData::staffSize diff --git a/src/include/mx/api/TupletData.h b/src/include/mx/api/TupletData.h index a6f1a73e0..d4b248e2b 100644 --- a/src/include/mx/api/TupletData.h +++ b/src/include/mx/api/TupletData.h @@ -9,6 +9,7 @@ #include "mx/api/Id.h" #include "mx/api/LineData.h" #include "mx/api/PositionData.h" +#include "mx/api/SpannerNumber.h" #include #include @@ -22,12 +23,22 @@ namespace api // is the '2' and the 'normal' part of the // ratio is the '3' +// The shape of a tuplet's bracket. MusicXML calls the older, slur-like shape 'curved'. +enum class TupletLineShape +{ + unspecified, + straight, + curved +}; + class TupletStart { public: - // used to id when nested - // NUMBER_LEVEL_UNSPECIFIED means unspecified - int numberLevel; + // Tells this tuplet apart from another one that is open at the same time, which happens when + // one tuplet is written inside another. Leave it alone for a tuplet that stands on its own. + // Give both ends of a tuplet the same identity when tuplets are nested, and mx numbers them + // for you; an explicit level is written out as it stands. + SpannerNumber number; PositionData positionData; @@ -48,17 +59,38 @@ class TupletStart // a triplet or the 4 in 5:4 for quintuplet Bool showNormalNumber; + // show the note value of the tuplet's own notes, + // for example the eighth note in "3 eighths" + Bool showActualType; + + // show the note value of the 'normal' notes, + // for example the eighth note in "in the time of 2 eighths" + Bool showNormalType; + Bool bracket; + // Whether the bracket is drawn as a straight line or in the older curved style. + TupletLineShape lineShape; + + // Most users can ignore these; leave them unspecified. They control whether the ratio is + // spelled out in the file alongside the note's own time modification. unspecified (the + // default) applies the right rule automatically: state the ratio when it differs from what + // the note already says, which is what an inner tuplet needs, and leave it out otherwise. + // yes/no force it in or out, so a writer that finds the ratio noisy can turn it off. Reading + // a file sets yes when the source stated the ratio. + Bool writeActual; + Bool writeNormal; + // The element's id attribute (see Id.h). std::optional id; TupletStart() - : numberLevel{NUMBER_LEVEL_UNSPECIFIED}, positionData{}, actualNumber{VALUE_UNSPECIFIED}, - actualDurationName{api::DurationName::unspecified}, actualDots{VALUE_UNSPECIFIED}, - normalNumber{VALUE_UNSPECIFIED}, normalDurationName{api::DurationName::unspecified}, - normalDots{VALUE_UNSPECIFIED}, showActualNumber{Bool::unspecified}, showNormalNumber{Bool::unspecified}, - bracket{Bool::unspecified}, id{} + : number{}, positionData{}, actualNumber{VALUE_UNSPECIFIED}, actualDurationName{api::DurationName::unspecified}, + actualDots{VALUE_UNSPECIFIED}, normalNumber{VALUE_UNSPECIFIED}, + normalDurationName{api::DurationName::unspecified}, normalDots{VALUE_UNSPECIFIED}, + showActualNumber{Bool::unspecified}, showNormalNumber{Bool::unspecified}, showActualType{Bool::unspecified}, + showNormalType{Bool::unspecified}, bracket{Bool::unspecified}, lineShape{TupletLineShape::unspecified}, + writeActual{Bool::unspecified}, writeNormal{Bool::unspecified}, id{} { } }; @@ -66,22 +98,25 @@ class TupletStart class TupletStop { public: - // used to id when nested - // NUMBER_LEVEL_UNSPECIFIED means unspecified - int numberLevel; + // Matches the number of the TupletStart this ends. See TupletStart::number. + SpannerNumber number; PositionData positionData; + // Whether a bracket is drawn. MusicXML allows the attribute on both ends of a tuplet, and + // some programs state it only on the stop. + Bool bracket; + // The element's id attribute (see Id.h). std::optional id; - TupletStop() : numberLevel{NUMBER_LEVEL_UNSPECIFIED}, positionData{}, id{} + TupletStop() : number{}, positionData{}, bracket{Bool::unspecified}, id{} { } }; MXAPI_EQUALS_BEGIN(TupletStart) -MXAPI_EQUALS_MEMBER(numberLevel) +MXAPI_EQUALS_MEMBER(number) MXAPI_EQUALS_MEMBER(positionData) MXAPI_EQUALS_MEMBER(actualNumber) MXAPI_EQUALS_MEMBER(actualDurationName) @@ -91,14 +126,20 @@ MXAPI_EQUALS_MEMBER(normalDurationName) MXAPI_EQUALS_MEMBER(normalDots) MXAPI_EQUALS_MEMBER(showActualNumber) MXAPI_EQUALS_MEMBER(showNormalNumber) +MXAPI_EQUALS_MEMBER(showActualType) +MXAPI_EQUALS_MEMBER(showNormalType) MXAPI_EQUALS_MEMBER(bracket) +MXAPI_EQUALS_MEMBER(lineShape) +MXAPI_EQUALS_MEMBER(writeActual) +MXAPI_EQUALS_MEMBER(writeNormal) MXAPI_EQUALS_MEMBER(id) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(TupletStart); MXAPI_EQUALS_BEGIN(TupletStop) -MXAPI_EQUALS_MEMBER(numberLevel) +MXAPI_EQUALS_MEMBER(number) MXAPI_EQUALS_MEMBER(positionData) +MXAPI_EQUALS_MEMBER(bracket) MXAPI_EQUALS_MEMBER(id) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(TupletStop); diff --git a/src/private/mx/impl/Converter.cpp b/src/private/mx/impl/Converter.cpp index 79f541859..509f0d22c 100644 --- a/src/private/mx/impl/Converter.cpp +++ b/src/private/mx/impl/Converter.cpp @@ -362,6 +362,11 @@ const Converter::EnumMap Converter::lineType = { {core::LineType::wavy(), api::LineType::wavy}, }; +const Converter::EnumMap Converter::lineShape = { + {core::LineShape::straight(), api::TupletLineShape::straight}, + {core::LineShape::curved(), api::TupletLineShape::curved}, +}; + const Converter::EnumMap Converter::wedgeMap = { {core::WedgeType::crescendo(), api::WedgeType::crescendo}, {core::WedgeType::diminuendo(), api::WedgeType::diminuendo}, @@ -1878,6 +1883,16 @@ api::LineType Converter::convert(core::LineType value) const return findApiItem(lineType, api::LineType::unspecified, value); } +core::LineShape Converter::convert(api::TupletLineShape value) const +{ + return findCoreItem(lineShape, core::LineShape::straight(), value); +} + +api::TupletLineShape Converter::convert(core::LineShape value) const +{ + return findApiItem(lineShape, api::TupletLineShape::unspecified, value); +} + core::WedgeType Converter::convert(api::WedgeType value) const { return findCoreItem(wedgeMap, core::WedgeType::continue_(), value); diff --git a/src/private/mx/impl/Converter.h b/src/private/mx/impl/Converter.h index dcfceb447..7d22718b6 100644 --- a/src/private/mx/impl/Converter.h +++ b/src/private/mx/impl/Converter.h @@ -13,6 +13,7 @@ #include "mx/api/PositionData.h" #include "mx/api/ScoreData.h" #include "mx/api/SoundID.h" +#include "mx/api/TupletData.h" #include "mx/core/generated/AboveBelow.h" #include "mx/core/generated/AccidentalValue.h" #include "mx/core/generated/ArticulationsChoice.h" @@ -36,6 +37,7 @@ #include "mx/core/generated/LeftCenterRight.h" #include "mx/core/generated/LeftRight.h" #include "mx/core/generated/LineEnd.h" +#include "mx/core/generated/LineShape.h" #include "mx/core/generated/LineType.h" #include "mx/core/generated/MeasureNumberingValue.h" #include "mx/core/generated/MembraneValue.h" @@ -163,6 +165,9 @@ class Converter core::LineType convert(api::LineType value) const; api::LineType convert(core::LineType value) const; + core::LineShape convert(api::TupletLineShape value) const; + api::TupletLineShape convert(core::LineShape value) const; + core::WedgeType convert(api::WedgeType value) const; api::WedgeType convert(core::WedgeType value) const; @@ -285,6 +290,7 @@ class Converter const static EnumMap directionSystemRelationMap; const static EnumMap stemMap; const static EnumMap lineType; + const static EnumMap lineShape; const static EnumMap wedgeMap; const static EnumMap barlineMap; const static EnumMap barlinePlacementMap; diff --git a/src/private/mx/impl/NotationsWriter.cpp b/src/private/mx/impl/NotationsWriter.cpp index aac726dac..78ae84609 100644 --- a/src/private/mx/impl/NotationsWriter.cpp +++ b/src/private/mx/impl/NotationsWriter.cpp @@ -99,7 +99,8 @@ void notationsWriterSetMordentSpecificAttributes(const api::MarkData &mark, core } core::NotationsChoice notationsWriterMakeTupletStop(const api::TupletStop &inTupletStop, - const DiagnosticsContext &diagnostics, + const std::optional &inResolvedNumber, + const Converter &inConverter, const DiagnosticsContext &diagnostics, const api::Location &location) { core::Tuplet tuplet; @@ -107,16 +108,134 @@ core::NotationsChoice notationsWriterMakeTupletStop(const api::TupletStop &inTup setAttributesFromPositionData(inTupletStop.positionData, tuplet); setId(inTupletStop.id, tuplet, diagnostics, location); - if (inTupletStop.numberLevel > 0) + if (inResolvedNumber.has_value()) { - const core::NumberLevel numberLevel{inTupletStop.numberLevel}; - reportAdjusted(diagnostics, location, "tuplet number", inTupletStop.numberLevel, numberLevel.value()); - tuplet.setNumber(numberLevel); + tuplet.setNumber(core::NumberLevel{*inResolvedNumber}); + } + + if (inTupletStop.bracket != api::Bool::unspecified) + { + tuplet.setBracket(inConverter.convert(inTupletStop.bracket)); } return core::NotationsChoice::tuplet(tuplet); } +// What one portion of a tuplet ratio says: how many notes, of what written value, with how many +// dots. VALUE_UNSPECIFIED and DurationName::unspecified mean the portion leaves that part out. +struct NotationsWriterTupletPortion +{ + int number; + api::DurationName durationName; + int dots; +}; + +// True when nothing in the portion would reach the file. +bool notationsWriterTupletPortionIsEmpty(const NotationsWriterTupletPortion &inPortion) +{ + return inPortion.number == api::VALUE_UNSPECIFIED && inPortion.durationName == api::DurationName::unspecified && + inPortion.dots <= 0; +} + +// True when every part the portion states matches what the note's time modification already +// says, so writing the portion would only repeat the note. MusicXML reads an absent portion +// from the time modification, which is why the repetition can be left out. +bool notationsWriterTupletPortionIsRedundant(const NotationsWriterTupletPortion &inPortion, + const NotationsWriterTupletPortion &inImplied) +{ + if (inPortion.number != api::VALUE_UNSPECIFIED && inPortion.number != inImplied.number) + { + return false; + } + if (inPortion.durationName != api::DurationName::unspecified && inPortion.durationName != inImplied.durationName) + { + return false; + } + if (inPortion.dots != api::VALUE_UNSPECIFIED && inPortion.dots != inImplied.dots) + { + return false; + } + return true; +} + +// NoteWriter reads 1:1 as "this note has no time modification" and leaves the element out, so a +// portion on such a note has nothing to repeat and is always written. +bool notationsWriterNoteHasTimeModification(const api::DurationData &inDuration) +{ + return inDuration.timeModificationActualNotes > 0 && inDuration.timeModificationNormalNotes > 0 && + (inDuration.timeModificationActualNotes > 1 || inDuration.timeModificationNormalNotes > 1); +} + +// Decides whether one portion reaches the file. The fidelity fields force the answer; left +// unspecified, a portion is written when it says something the note does not already say. +bool notationsWriterWriteTupletPortion(api::Bool inFidelity, const NotationsWriterTupletPortion &inPortion, + const NotationsWriterTupletPortion &inImplied, bool inNoteHasTimeModification) +{ + if (inFidelity == api::Bool::yes) + { + return true; + } + if (inFidelity == api::Bool::no) + { + return false; + } + if (notationsWriterTupletPortionIsEmpty(inPortion)) + { + return false; + } + if (!inNoteHasTimeModification) + { + return true; + } + return !notationsWriterTupletPortionIsRedundant(inPortion, inImplied); +} + +core::TupletPortion notationsWriterMakeTupletPortion(const NotationsWriterTupletPortion &inPortion, + const Converter &inConverter) +{ + core::TupletPortion outPortion; + + if (inPortion.number != api::VALUE_UNSPECIFIED) + { + core::TupletNumber tupletNumber; + tupletNumber.setValue(inPortion.number); + outPortion.setTupletNumber(tupletNumber); + } + + if (inPortion.durationName != api::DurationName::unspecified) + { + core::TupletType tupletType; + tupletType.setValue(inConverter.convert(inPortion.durationName)); + outPortion.setTupletType(tupletType); + } + + for (int dot = 0; dot < inPortion.dots; ++dot) + { + outPortion.addTupletDot(core::TupletDot{}); + } + + return outPortion; +} + +// The two show-* attributes each cover a pair of api fields, one for the actual side of the +// ratio and one for the normal side. +std::optional notationsWriterShowTuplet(api::Bool inShowActual, api::Bool inShowNormal) +{ + if (inShowActual == api::Bool::unspecified) + { + return std::nullopt; + } + if (inShowActual == api::Bool::no) + { + return core::ShowTuplet::none(); + } + if (inShowNormal == api::Bool::yes) + { + return core::ShowTuplet::both(); + } + return core::ShowTuplet::actual(); +} + NotationsWriter::NotationsWriter(const api::NoteData &inNoteData, const MeasureCursor &inCursor, const ScoreWriter &inScoreWriter) : myNoteData{inNoteData}, myCursor{inCursor}, myScoreWriter{inScoreWriter}, myConverter{} @@ -226,14 +345,26 @@ core::Notations NotationsWriter::getNotations() const // A tuplet contained in a single note -- an inner tuplet covering exactly one note of its // outer tuplet, or Finale's export of a one-note tuplet -- has its start and its stop on // that one note, and the start must be written first (#429). So starts are written before - // stops, each start followed directly by the same-note stop that shares its numberLevel + // stops, each start followed directly by the same-note stop that carries the same number // when there is one, and the remaining stops (closing tuplets begun on earlier notes) - // follow. Matching numberLevels on one note always mean a single-note tuplet: two - // different tuplets can never share a note, because a note carries only one - // time-modification. + // follow. Matching numbers on one note always mean a single-note tuplet: two different + // tuplets can never share a note, because a note carries only one time-modification. const auto &tupletStops = myNoteData.noteAttachmentData.tupletStops; std::vector tupletStopWritten(tupletStops.size(), false); + const auto &duration = myNoteData.durationData; + const bool noteHasTimeModification = notationsWriterNoteHasTimeModification(duration); + + // What the note's time modification already says about each side of the ratio. An absent + // means the normal notes carry the note's own written value (#428). + const NotationsWriterTupletPortion impliedActual{duration.timeModificationActualNotes, duration.durationName, + duration.durationDots}; + const bool hasNormalType = duration.timeModificationNormalType != api::DurationName::unspecified; + const NotationsWriterTupletPortion impliedNormal{ + duration.timeModificationNormalNotes, + hasNormalType ? duration.timeModificationNormalType : duration.durationName, + hasNormalType ? duration.timeModificationNormalTypeDots : duration.durationDots}; + for (const auto &tupletStart : myNoteData.noteAttachmentData.tupletStarts) { core::Tuplet tuplet; @@ -241,38 +372,24 @@ core::Notations NotationsWriter::getNotations() const setAttributesFromPositionData(tupletStart.positionData, tuplet); setId(tupletStart.id, tuplet, myScoreWriter.getDiagnostics(), cursorLocation(myCursor)); - core::TupletPortion actual; - core::TupletNumber tn1; - tn1.setValue(tupletStart.actualNumber); - actual.setTupletNumber(tn1); - core::TupletType tt1; - tt1.setValue(myConverter.convert(tupletStart.actualDurationName)); - actual.setTupletType(tt1); - for (int d = 0; d < tupletStart.actualDots; ++d) + const NotationsWriterTupletPortion actual{tupletStart.actualNumber, tupletStart.actualDurationName, + tupletStart.actualDots}; + if (notationsWriterWriteTupletPortion(tupletStart.writeActual, actual, impliedActual, noteHasTimeModification)) { - actual.addTupletDot(core::TupletDot{}); + tuplet.setTupletActual(notationsWriterMakeTupletPortion(actual, myConverter)); } - tuplet.setTupletActual(actual); - - core::TupletPortion normal; - core::TupletNumber tn2; - tn2.setValue(tupletStart.normalNumber); - normal.setTupletNumber(tn2); - core::TupletType tt2; - tt2.setValue(myConverter.convert(tupletStart.normalDurationName)); - normal.setTupletType(tt2); - for (int d = 0; d < tupletStart.normalDots; ++d) + + const NotationsWriterTupletPortion normal{tupletStart.normalNumber, tupletStart.normalDurationName, + tupletStart.normalDots}; + if (notationsWriterWriteTupletPortion(tupletStart.writeNormal, normal, impliedNormal, noteHasTimeModification)) { - normal.addTupletDot(core::TupletDot{}); + tuplet.setTupletNormal(notationsWriterMakeTupletPortion(normal, myConverter)); } - tuplet.setTupletNormal(normal); - if (tupletStart.numberLevel > 0) + const auto resolvedNumber = spannerResolver.emittedNumber(tupletStart.number, &tupletStart); + if (resolvedNumber.has_value()) { - const core::NumberLevel numberLevel{tupletStart.numberLevel}; - reportAdjusted(myScoreWriter.getDiagnostics(), cursorLocation(myCursor), "tuplet number", - tupletStart.numberLevel, numberLevel.value()); - tuplet.setNumber(numberLevel); + tuplet.setNumber(core::NumberLevel{*resolvedNumber}); } if (tupletStart.bracket != api::Bool::unspecified) @@ -280,33 +397,24 @@ core::Notations NotationsWriter::getNotations() const tuplet.setBracket(myConverter.convert(tupletStart.bracket)); } - if (tupletStart.showActualNumber != api::Bool::unspecified) + if (tupletStart.lineShape != api::TupletLineShape::unspecified) { - if (tupletStart.showActualNumber == api::Bool::yes) - { - if (tupletStart.showNormalNumber == api::Bool::yes) - { - tuplet.setShowNumber(core::ShowTuplet::both()); - } - else - { - tuplet.setShowNumber(core::ShowTuplet::actual()); - } - } - else if (tupletStart.showActualNumber == api::Bool::no) - { - tuplet.setShowNumber(core::ShowTuplet::none()); - } + tuplet.setLineShape(myConverter.convert(tupletStart.lineShape)); } + tuplet.setShowNumber(notationsWriterShowTuplet(tupletStart.showActualNumber, tupletStart.showNormalNumber)); + tuplet.setShowType(notationsWriterShowTuplet(tupletStart.showActualType, tupletStart.showNormalType)); + outNotations.addChoice(core::NotationsChoice::tuplet(tuplet)); for (std::size_t stopIndex = 0; stopIndex < tupletStops.size(); ++stopIndex) { - if (!tupletStopWritten[stopIndex] && tupletStops[stopIndex].numberLevel == tupletStart.numberLevel) + if (!tupletStopWritten[stopIndex] && tupletStops[stopIndex].number == tupletStart.number) { + const auto &tupletStop = tupletStops[stopIndex]; outNotations.addChoice(notationsWriterMakeTupletStop( - tupletStops[stopIndex], myScoreWriter.getDiagnostics(), cursorLocation(myCursor))); + tupletStop, spannerResolver.emittedNumber(tupletStop.number, &tupletStop), myConverter, + myScoreWriter.getDiagnostics(), cursorLocation(myCursor))); tupletStopWritten[stopIndex] = true; break; } @@ -317,8 +425,10 @@ core::Notations NotationsWriter::getNotations() const { if (!tupletStopWritten[stopIndex]) { - outNotations.addChoice(notationsWriterMakeTupletStop(tupletStops[stopIndex], myScoreWriter.getDiagnostics(), - cursorLocation(myCursor))); + const auto &tupletStop = tupletStops[stopIndex]; + outNotations.addChoice( + notationsWriterMakeTupletStop(tupletStop, spannerResolver.emittedNumber(tupletStop.number, &tupletStop), + myConverter, myScoreWriter.getDiagnostics(), cursorLocation(myCursor))); } } diff --git a/src/private/mx/impl/SpannerResolver.cpp b/src/private/mx/impl/SpannerResolver.cpp index 621f02057..0cb8aed81 100644 --- a/src/private/mx/impl/SpannerResolver.cpp +++ b/src/private/mx/impl/SpannerResolver.cpp @@ -26,7 +26,8 @@ namespace impl // independent of each other in MusicXML, so each gets its own pool. // GlissandoType similarly distinguishes from , two distinct // elements with independent number attributes. Pedal lines number themselves -// independently of every other family too. +// independently of every other family too. Tuplets use the number attribute for +// the same purpose, to tell nested tuplets apart. enum class SpannerNumberClass { slur, @@ -38,7 +39,8 @@ enum class SpannerNumberClass pedal, glissando, slide, - wavyLine + wavyLine, + tuplet }; // One start/continue/stop occurrence at its position in the serialized stream. The number is @@ -159,6 +161,18 @@ class SpannerEventCollector addGlissando(start.glissandoType, &start, start.number, true, false); } + // NotationsWriter emits tuplet starts before tuplet stops, so that a tuplet living on + // one note opens before it closes (#429). An inner tuplet that covers a single note + // therefore overlaps its outer tuplet here, which is what earns it a second number. + for (const auto &start : attachments.tupletStarts) + { + add(SpannerNumberClass::tuplet, &start, start.number, true, false); + } + for (const auto &stop : attachments.tupletStops) + { + add(SpannerNumberClass::tuplet, &stop, stop.number, false, true); + } + // NotationsWriter emits wavy-line stops, then continues, then (after any mark-derived // ornaments, which carry no number) starts. for (const auto &stop : attachments.wavyLineStops) @@ -526,6 +540,8 @@ static const char *spannerClassName(SpannerNumberClass inClass) return "slide"; case SpannerNumberClass::wavyLine: return "wavy-line"; + case SpannerNumberClass::tuplet: + return "tuplet"; } return "spanner"; } @@ -573,6 +589,39 @@ static void spannerReportUnmatched(SpannerNumberClass inClass, const std::vector } } +// Warns about tuplets that overlap without numbers to tell them apart. MusicXML reads a tuplet +// with no number attribute as number 1, so a second unnumbered tuplet opening inside a first one +// gives a reader two tuplets it cannot separate. +static void spannerReportUnnumberedOverlap(const std::vector &inEvents, + const DiagnosticsContext &diagnostics) +{ + int openCount = 0; + for (const auto &event : inEvents) + { + if (!event.number.isUnspecified()) + { + continue; + } + if (event.closes) + { + if (openCount > 0) + { + --openCount; + } + continue; + } + if (event.opens) + { + if (openCount > 0) + { + diagnostics.report(api::Severity::warning, api::DiagnosticCode::missingValueDefaulted, event.location, + "two tuplets without a number are open at once; readers will default both to 1"); + } + ++openCount; + } + } +} + void SpannerResolver::resolvePart(const api::PartData &inPart, int partIndex, DiagnosticsContext diagnostics) { SpannerEventCollector collector{partIndex}; @@ -618,6 +667,10 @@ void SpannerResolver::resolvePart(const api::PartData &inPart, int partIndex, Di { spannerDetectSameNoteSpans(classAndEvents.second, mySameNoteSpanPartners); } + if (spannerClass == SpannerNumberClass::tuplet) + { + spannerReportUnnumberedOverlap(classAndEvents.second, diagnostics); + } } spannerResolveOttavaSizes(collector.ottavaEvents(), myOttavaStopSizes, diagnostics); diff --git a/src/private/mx/impl/SpannerResolver.h b/src/private/mx/impl/SpannerResolver.h index 3bd5e6ccf..c680fd34c 100644 --- a/src/private/mx/impl/SpannerResolver.h +++ b/src/private/mx/impl/SpannerResolver.h @@ -29,16 +29,17 @@ namespace impl // concurrently-open identity spanner is never handed the same number. // // Numbers come from a pool of 1..16 per part and per spanner class (slur, -// tied, wedge, octave-shift, bracket, dashes, pedal, glissando, slide, and -// wavy-line each have their own pool; a slur numbered 1 and a wedge numbered 1 -// do not conflict). The number-level documentation scopes concurrency to the part, +// tied, wedge, octave-shift, bracket, dashes, pedal, glissando, slide, +// wavy-line, and tuplet each have their own pool; a slur numbered 1 and a +// wedge numbered 1 do not conflict). The number-level documentation scopes concurrency to the part, // never the staff: two spanners conflict exactly when they overlap in the // order a streaming reader encounters them, even when they sit on different // staves of the part. So resolvePart walks the part in the exact order // MeasureWriter serializes it: measures in order, staves in order, voices // ascending, notes in vector order (curve stops/continues/starts, then -// glissando/slide stops then starts, then wavy-line stops/continues/starts, -// per note -- mirroring NotationsWriter), and each staff's directions in +// glissando/slide stops then starts, then tuplet starts then stops, then +// wavy-line stops/continues/starts, per note -- mirroring NotationsWriter), +// and each staff's directions in // vector order (mirroring DirectionWriter's per-direction emission order). An // identity spanner takes the lowest number that is free across its whole // serialized extent -- from its first event to its last, whichever of diff --git a/src/private/mx/impl/TupletReader.cpp b/src/private/mx/impl/TupletReader.cpp index f02af756b..24c4a8478 100644 --- a/src/private/mx/impl/TupletReader.cpp +++ b/src/private/mx/impl/TupletReader.cpp @@ -30,15 +30,21 @@ void TupletReader::parseTuplet(std::vector &outTupletStarts, if (myTuplet.number().has_value()) { - tupletStart.numberLevel = myTuplet.number()->value(); + tupletStart.number = api::SpannerNumber{myTuplet.number()->value()}; } + Converter converter; + if (myTuplet.type() == core::StartStop::stop()) { api::TupletStop tupletStop; tupletStop.positionData = tupletStart.positionData; - tupletStop.numberLevel = tupletStart.numberLevel; + tupletStop.number = tupletStart.number; tupletStop.id = getId(myTuplet); + if (myTuplet.bracket().has_value()) + { + tupletStop.bracket = converter.convert(*myTuplet.bracket()); + } outTupletStops.emplace_back(std::move(tupletStop)); return; } @@ -64,13 +70,37 @@ void TupletReader::parseTuplet(std::vector &outTupletStarts, } } - Converter converter; + if (myTuplet.showType().has_value()) + { + switch (myTuplet.showType()->tag()) + { + case core::ShowTuplet::Tag::none: + tupletStart.showActualType = api::Bool::no; + tupletStart.showNormalType = api::Bool::no; + break; + case core::ShowTuplet::Tag::both: + tupletStart.showActualType = api::Bool::yes; + tupletStart.showNormalType = api::Bool::yes; + break; + case core::ShowTuplet::Tag::actual: + tupletStart.showActualType = api::Bool::yes; + tupletStart.showNormalType = api::Bool::no; + break; + default: + break; + } + } if (myTuplet.bracket().has_value()) { tupletStart.bracket = converter.convert(*myTuplet.bracket()); } + if (myTuplet.lineShape().has_value()) + { + tupletStart.lineShape = converter.convert(*myTuplet.lineShape()); + } + if (myTuplet.tupletActual().has_value()) { const auto &actual = *myTuplet.tupletActual(); @@ -86,6 +116,10 @@ void TupletReader::parseTuplet(std::vector &outTupletStarts, } tupletStart.actualDots = static_cast(actual.tupletDot().size()); + + // The source stated the actual portion, so say it again on the way out even if the + // note's time modification already implies it. + tupletStart.writeActual = api::Bool::yes; } else { @@ -107,6 +141,7 @@ void TupletReader::parseTuplet(std::vector &outTupletStarts, } tupletStart.normalDots = static_cast(normal.tupletDot().size()); + tupletStart.writeNormal = api::Bool::yes; } else { diff --git a/src/private/mxtest/api/NoteDataTest.cpp b/src/private/mxtest/api/NoteDataTest.cpp index b49d19b6c..6b2aa9253 100644 --- a/src/private/mxtest/api/NoteDataTest.cpp +++ b/src/private/mxtest/api/NoteDataTest.cpp @@ -1808,8 +1808,8 @@ TEST(implicitStaffOnSingleStaffPartOmitsElement, NoteData) T_END; // issue #452: a on a dotted note whose does not restate the -// normal figure guesses that figure from the note, dots included, so the tuplet written back -// carries a tuplet-dot. +// normal figure guesses that figure from the note, dots included. The guess repeats what the +// note already says, so it stays out of the file unless the writer is told to state it. TEST(guessedTupletNormalKeepsTheNoteDots, NoteData) { const std::string xml = R"( @@ -1852,19 +1852,33 @@ TEST(guessedTupletNormalKeepsTheNoteDots, NoteData) CHECK(DurationName::quarter == starts.at(0).normalDurationName); CHECK_EQUAL(1, starts.at(0).normalDots); + // the source stated no portions, so neither is written back const auto written = mxtest::toXml(score); - const auto normalPos = written.find(""); - REQUIRE(normalPos != std::string::npos); - const auto normalEnd = written.find("", normalPos); - REQUIRE(normalEnd != std::string::npos); - CHECK(written.substr(normalPos, normalEnd - normalPos).find("") == std::string::npos); // reading the written document back reports the same dotted normal figure const auto reread = mxtest::fromXml(written); const auto &rereread = reread.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0).noteAttachmentData.tupletStarts; REQUIRE(1 == rereread.size()); + CHECK(DurationName::quarter == rereread.at(0).normalDurationName); CHECK_EQUAL(1, rereread.at(0).normalDots); + + // asking for the portion spells the dotted figure out + auto forced = score; + forced.parts.at(0) + .measures.at(0) + .staves.at(0) + .voices.at(0) + .notes.at(0) + .noteAttachmentData.tupletStarts.at(0) + .writeNormal = Bool::yes; + const auto forcedXml = mxtest::toXml(forced); + const auto normalPos = forcedXml.find(""); + REQUIRE(normalPos != std::string::npos); + const auto normalEnd = forcedXml.find("", normalPos); + REQUIRE(normalEnd != std::string::npos); + CHECK(forcedXml.substr(normalPos, normalEnd - normalPos).find(" were read into TupletStart and TupletStop and diff --git a/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp b/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp index 990e71d15..14d8649e9 100644 --- a/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp +++ b/src/private/mxtest/api/SingleNoteSpannerApiTest.cpp @@ -38,10 +38,10 @@ inline NoteData makeGraceNote(Step inStep, int inOctave) return note; } -inline TupletStart makeTripletStart(int inNumberLevel) +inline TupletStart makeTripletStart(int inLevel) { TupletStart start; - start.numberLevel = inNumberLevel; + start.number = SpannerNumber{inLevel}; start.actualNumber = 3; start.actualDurationName = DurationName::quarter; start.normalNumber = 2; @@ -118,11 +118,11 @@ TEST(nestedTupletOnFinalNoteWritesInnerStartFirst, SingleNoteSpanner) voice.notes.push_back(makeGraceNote(Step::e, 4)); TupletStop outerStop; - outerStop.numberLevel = 1; + outerStop.number = SpannerNumber{1}; voice.notes.back().noteAttachmentData.tupletStops.push_back(outerStop); voice.notes.back().noteAttachmentData.tupletStarts.push_back(makeTripletStart(2)); TupletStop innerStop; - innerStop.numberLevel = 2; + innerStop.number = SpannerNumber{2}; voice.notes.back().noteAttachmentData.tupletStops.push_back(innerStop); const auto sequence = spannerSequence(toXml(score), 2, "tuplet"); @@ -149,14 +149,14 @@ TEST(nestedTupletOnFirstNoteWritesOuterStartFirst, SingleNoteSpanner) voice.notes.back().noteAttachmentData.tupletStarts.push_back(makeTripletStart(1)); voice.notes.back().noteAttachmentData.tupletStarts.push_back(makeTripletStart(2)); TupletStop innerStop; - innerStop.numberLevel = 2; + innerStop.number = SpannerNumber{2}; voice.notes.back().noteAttachmentData.tupletStops.push_back(innerStop); voice.notes.push_back(makeGraceNote(Step::d, 4)); voice.notes.push_back(makeGraceNote(Step::e, 4)); TupletStop outerStop; - outerStop.numberLevel = 1; + outerStop.number = SpannerNumber{1}; voice.notes.back().noteAttachmentData.tupletStops.push_back(outerStop); const auto sequence = spannerSequence(toXml(score), 0, "tuplet"); @@ -181,7 +181,7 @@ TEST(oneNoteTupletWritesStartThenStop, SingleNoteSpanner) voice.notes.push_back(makeGraceNote(Step::b, 4)); voice.notes.back().noteAttachmentData.tupletStarts.push_back(makeTripletStart(1)); TupletStop stop; - stop.numberLevel = 1; + stop.number = SpannerNumber{1}; voice.notes.back().noteAttachmentData.tupletStops.push_back(stop); const auto sequence = spannerSequence(toXml(score), 0, "tuplet"); diff --git a/src/private/mxtest/api/TupletApiTest.cpp b/src/private/mxtest/api/TupletApiTest.cpp new file mode 100644 index 000000000..b6c99fc7b --- /dev/null +++ b/src/private/mxtest/api/TupletApiTest.cpp @@ -0,0 +1,342 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +// A may spell its ratio out in and , or leave both out +// and let the note's speak for it. mx reads either shape into the same +// fields and remembers which one the file used, so the ratio is stated again only where it says +// something the note does not already say -- which is what an inner tuplet of a nested pair +// needs. The coverage here is the reading, the writing, and the three attributes that +// had no home in mx::api before: show-type, line-shape, and bracket on a stop. + +#include "mxtest/control/CompileControl.h" +#ifdef MX_COMPILE_API_TESTS + +#include "cpul/cpulTestHarness.h" +#include "mx/api/MusicXml.h" +#include "mx/api/ScoreData.h" +#include "mxtest/api/TestHelpers.h" + +#include + +using namespace mx::api; +using namespace mxtest; + +namespace tupletApiTest +{ +// One quarter note carrying whatever time modification and notations a test needs. +inline std::string scoreXml(const std::string &inTimeModification, const std::string &inNotations) +{ + return R"( + + + + MusicXML Part + + + + + + 6 + + + + C + 4 + + 4 + 1 + quarter +)" + inTimeModification + + R"( +)" + inNotations + + R"( + + + + + +)"; +} + +inline const std::string &tripletTimeModification() +{ + static const std::string value = R"( + 3 + 2 + +)"; + return value; +} + +inline const NoteAttachmentData &attachments(const ScoreData &inScore) +{ + return inScore.parts.at(0).measures.at(0).staves.at(0).voices.at(0).notes.at(0).noteAttachmentData; +} + +// A score of one voice whose notes are appended in order. +inline ScoreData makeScore() +{ + ScoreData score; + score.parts.emplace_back(); + score.parts.back().uniqueId = "P1"; + score.parts.back().measures.emplace_back(); + score.parts.back().measures.back().staves.emplace_back(); + return score; +} + +inline NoteData &appendNote(ScoreData &ioScore, DurationName inDurationName, int inActualNotes, int inNormalNotes) +{ + auto &voice = ioScore.parts.back().measures.back().staves.back().voices[0]; + auto ¬e = voice.notes.emplace_back(); + note.pitchData.step = Step::c; + note.pitchData.octave = 4; + note.durationData.durationName = inDurationName; + note.durationData.durationTimeTicks = 1; + note.durationData.timeModificationActualNotes = inActualNotes; + note.durationData.timeModificationNormalNotes = inNormalNotes; + note.tickTimePosition = static_cast(voice.notes.size()) - 1; + return note; +} + +// A tuplet start stating a ratio of its own, for example 3 eighths in the time of 2 eighths. +inline TupletStart makeStart(int inActualNumber, int inNormalNumber, DurationName inDurationName) +{ + TupletStart start; + start.actualNumber = inActualNumber; + start.actualDurationName = inDurationName; + start.actualDots = 0; + start.normalNumber = inNormalNumber; + start.normalDurationName = inDurationName; + start.normalDots = 0; + return start; +} +} // namespace tupletApiTest + +// A that states no ratio of its own is written back the same way. The api still reports +// the ratio, read from the note's time modification. +TEST(bareStartWritesNoPortions, Tuplet) +{ + using namespace tupletApiTest; + const auto score = fromXml(scoreXml(tripletTimeModification(), R"( )")); + + const auto &starts = attachments(score).tupletStarts; + REQUIRE(1 == starts.size()); + CHECK(Bool::unspecified == starts.at(0).writeActual); + CHECK(Bool::unspecified == starts.at(0).writeNormal); + CHECK_EQUAL(3, starts.at(0).actualNumber); + CHECK_EQUAL(2, starts.at(0).normalNumber); + CHECK(DurationName::quarter == starts.at(0).actualDurationName); + + const auto written = toXml(score); + CHECK(written.find("") == std::string::npos); + CHECK(written.find("") == std::string::npos); +} + +T_END + +// A file that spells the ratio out keeps it, even though the note's time modification says the +// same thing. +TEST(statedPortionsAreWrittenBack, Tuplet) +{ + using namespace tupletApiTest; + const auto score = fromXml(scoreXml(tripletTimeModification(), R"( + + 3 + quarter + + + 2 + quarter + + )")); + + const auto &starts = attachments(score).tupletStarts; + REQUIRE(1 == starts.size()); + CHECK(Bool::yes == starts.at(0).writeActual); + CHECK(Bool::yes == starts.at(0).writeNormal); + + const auto written = toXml(score); + CHECK(written.find("") != std::string::npos); + CHECK(written.find("") != std::string::npos); + CHECK(written.find("quarter") != std::string::npos); +} + +T_END + +// A portion that states only part of its figure keeps exactly the part it stated. Nothing is +// invented to fill the gap, and the portion the file left out stays out. +TEST(partialPortionIsWrittenBackExactly, Tuplet) +{ + using namespace tupletApiTest; + const auto score = fromXml(scoreXml(tripletTimeModification(), R"( + + 3 + + )")); + + const auto &starts = attachments(score).tupletStarts; + REQUIRE(1 == starts.size()); + CHECK(Bool::yes == starts.at(0).writeActual); + CHECK(Bool::unspecified == starts.at(0).writeNormal); + CHECK(DurationName::unspecified == starts.at(0).actualDurationName); + + const auto written = toXml(score); + const auto actualPos = written.find(""); + REQUIRE(actualPos != std::string::npos); + const auto actualEnd = written.find("", actualPos); + REQUIRE(actualEnd != std::string::npos); + const auto actual = written.substr(actualPos, actualEnd - actualPos); + CHECK(actual.find("3") != std::string::npos); + CHECK(actual.find("") == std::string::npos); + CHECK(written.find("") == std::string::npos); +} + +T_END + +// show-type says whether the note values of the ratio are drawn, the way show-number says +// whether its numbers are. +TEST(showTypeRoundTrips, Tuplet) +{ + using namespace tupletApiTest; + const auto both = + fromXml(scoreXml(tripletTimeModification(), R"( )")); + REQUIRE(1 == attachments(both).tupletStarts.size()); + CHECK(Bool::yes == attachments(both).tupletStarts.at(0).showActualType); + CHECK(Bool::yes == attachments(both).tupletStarts.at(0).showNormalType); + CHECK(toXml(both).find(R"(show-type="both")") != std::string::npos); + + const auto actual = fromXml( + scoreXml(tripletTimeModification(), R"( )")); + CHECK(Bool::yes == attachments(actual).tupletStarts.at(0).showActualType); + CHECK(Bool::no == attachments(actual).tupletStarts.at(0).showNormalType); + CHECK(toXml(actual).find(R"(show-type="actual")") != std::string::npos); + + const auto none = + fromXml(scoreXml(tripletTimeModification(), R"( )")); + CHECK(Bool::no == attachments(none).tupletStarts.at(0).showActualType); + CHECK(Bool::no == attachments(none).tupletStarts.at(0).showNormalType); + CHECK(toXml(none).find(R"(show-type="none")") != std::string::npos); + + // a file that says nothing about show-type gets no attribute back + const auto silent = fromXml(scoreXml(tripletTimeModification(), R"( )")); + CHECK(Bool::unspecified == attachments(silent).tupletStarts.at(0).showActualType); + CHECK(toXml(silent).find("show-type") == std::string::npos); +} + +T_END + +// line-shape chooses between a straight bracket and the older curved one. +TEST(lineShapeRoundTrips, Tuplet) +{ + using namespace tupletApiTest; + const auto curved = fromXml( + scoreXml(tripletTimeModification(), R"( )")); + REQUIRE(1 == attachments(curved).tupletStarts.size()); + CHECK(TupletLineShape::curved == attachments(curved).tupletStarts.at(0).lineShape); + CHECK(toXml(curved).find(R"(line-shape="curved")") != std::string::npos); + + const auto straight = fromXml( + scoreXml(tripletTimeModification(), R"( )")); + CHECK(TupletLineShape::straight == attachments(straight).tupletStarts.at(0).lineShape); + CHECK(toXml(straight).find(R"(line-shape="straight")") != std::string::npos); + + const auto silent = fromXml(scoreXml(tripletTimeModification(), R"( )")); + CHECK(TupletLineShape::unspecified == attachments(silent).tupletStarts.at(0).lineShape); + CHECK(toXml(silent).find("line-shape") == std::string::npos); +} + +T_END + +// Some programs state the bracket on the closing end of a tuplet rather than the opening one. +TEST(stopBracketRoundTrips, Tuplet) +{ + using namespace tupletApiTest; + const auto score = + fromXml(scoreXml(tripletTimeModification(), R"( )")); + + const auto &stops = attachments(score).tupletStops; + REQUIRE(1 == stops.size()); + CHECK(Bool::no == stops.at(0).bracket); + CHECK(toXml(score).find(R"(bracket="no")") != std::string::npos); + + const auto silent = fromXml(scoreXml(tripletTimeModification(), R"( )")); + CHECK(Bool::unspecified == attachments(silent).tupletStops.at(0).bracket); + CHECK(toXml(silent).find("bracket") == std::string::npos); +} + +T_END + +// The nested case this whole design serves. An inner triplet of eighths inside a triplet of +// quarters gives its notes a cumulative 9:4, so the inner tuplet's own 3:2 differs from the +// note and is written, while the outer tuplet repeats its note's 3:2 and is left out. +TEST(nestedTupletWritesTheInnerRatioOnly, Tuplet) +{ + using namespace tupletApiTest; + auto score = makeScore(); + + auto &outerNote = appendNote(score, DurationName::quarter, 3, 2); + outerNote.noteAttachmentData.tupletStarts.push_back(makeStart(3, 2, DurationName::quarter)); + + auto &innerNote = appendNote(score, DurationName::eighth, 9, 4); + innerNote.noteAttachmentData.tupletStarts.push_back(makeStart(3, 2, DurationName::eighth)); + + const auto written = toXml(score); + CHECK(written.find("quarter") == std::string::npos); + CHECK(written.find("3") != std::string::npos); + CHECK(written.find("eighth") != std::string::npos); +} + +T_END + +// The fidelity fields override the rule in both directions. +TEST(fidelityFieldsForceAndSilencePortions, Tuplet) +{ + using namespace tupletApiTest; + + // a ratio that differs from the note is written, unless told not to + auto silenced = makeScore(); + auto &silencedNote = appendNote(silenced, DurationName::eighth, 9, 4); + auto silencedStart = makeStart(3, 2, DurationName::eighth); + silencedStart.writeActual = Bool::no; + silencedStart.writeNormal = Bool::no; + silencedNote.noteAttachmentData.tupletStarts.push_back(silencedStart); + const auto silencedXml = toXml(silenced); + CHECK(silencedXml.find("") == std::string::npos); + CHECK(silencedXml.find("") == std::string::npos); + + // a ratio that repeats the note is left out, unless told to state it + auto forced = makeScore(); + auto &forcedNote = appendNote(forced, DurationName::quarter, 3, 2); + auto forcedStart = makeStart(3, 2, DurationName::quarter); + forcedStart.writeActual = Bool::yes; + forcedStart.writeNormal = Bool::yes; + forcedNote.noteAttachmentData.tupletStarts.push_back(forcedStart); + const auto forcedXml = toXml(forced); + CHECK(forcedXml.find("") != std::string::npos); + CHECK(forcedXml.find("") != std::string::npos); +} + +T_END + +// A portion without a note value is written without a ; the old writer turned the +// missing value into a maxima. +TEST(unspecifiedDurationNameWritesNoTupletType, Tuplet) +{ + using namespace tupletApiTest; + auto score = makeScore(); + auto ¬e = appendNote(score, DurationName::quarter, 3, 2); + TupletStart start; + start.actualNumber = 5; + start.writeActual = Bool::yes; + note.noteAttachmentData.tupletStarts.push_back(start); + + const auto written = toXml(score); + CHECK(written.find("5") != std::string::npos); + CHECK(written.find("") == std::string::npos); + CHECK(written.find("") == std::string::npos); +} + +T_END + +#endif diff --git a/src/private/mxtest/api/TupletNumberApiTest.cpp b/src/private/mxtest/api/TupletNumberApiTest.cpp new file mode 100644 index 000000000..0889b92fa --- /dev/null +++ b/src/private/mxtest/api/TupletNumberApiTest.cpp @@ -0,0 +1,219 @@ +// MusicXML Class Library +// Copyright (c) by Matthew James Briggs +// Distributed under the MIT License + +// Tuplets carry a number attribute for the same reason slurs do: to tell two of them apart +// while both are open, which happens when one tuplet is written inside another. An author who +// gives both ends of each tuplet the same identity gets the numbers assigned here, the same way +// the other spanner families are numbered (see SpannerResolver). + +#include "mxtest/control/CompileControl.h" +#ifdef MX_COMPILE_API_TESTS + +#include "cpul/cpulTestHarness.h" +#include "mx/api/Diagnostics.h" +#include "mx/api/MusicXml.h" +#include "mx/api/ScoreData.h" +#include "mxtest/api/TestHelpers.h" +#include "pugixml.hpp" + +#include +#include +#include + +using namespace mx::api; +using namespace mxtest; + +namespace tupletNumberApiTest +{ +inline ScoreData makeScore() +{ + ScoreData score; + score.parts.emplace_back(); + score.parts.back().uniqueId = "P1"; + score.parts.back().measures.emplace_back(); + score.parts.back().measures.back().staves.emplace_back(); + return score; +} + +inline NoteData &appendNote(ScoreData &ioScore) +{ + auto &voice = ioScore.parts.back().measures.back().staves.back().voices[0]; + auto ¬e = voice.notes.emplace_back(); + note.pitchData.step = Step::c; + note.pitchData.octave = 4; + note.durationData.durationName = DurationName::quarter; + note.durationData.durationTimeTicks = 1; + note.durationData.timeModificationActualNotes = 3; + note.durationData.timeModificationNormalNotes = 2; + note.tickTimePosition = static_cast(voice.notes.size()) - 1; + return note; +} + +inline void addStart(NoteData &ioNote, SpannerNumber inNumber) +{ + TupletStart start; + start.number = std::move(inNumber); + ioNote.noteAttachmentData.tupletStarts.push_back(start); +} + +inline void addStop(NoteData &ioNote, SpannerNumber inNumber) +{ + TupletStop stop; + stop.number = std::move(inNumber); + ioNote.noteAttachmentData.tupletStops.push_back(stop); +} + +// Every in the document as a (type, number) pair, in document order. An element with +// no number attribute reports an empty string. +inline std::vector> tupletSequence(const std::string &inXml) +{ + std::vector> result; + pugi::xml_document xdoc; + if (!xdoc.load_string(inXml.c_str())) + { + return result; + } + for (const auto ¬e : xdoc.document_element().child("part").child("measure").children("note")) + { + for (const auto ¬ation : note.child("notations").children("tuplet")) + { + result.emplace_back(notation.attribute("type").value(), notation.attribute("number").value()); + } + } + return result; +} + +inline bool hasDiagnostic(const Diagnostics &inDiagnostics, DiagnosticCode inCode, const std::string &inMessage) +{ + for (const auto &diagnostic : inDiagnostics.all()) + { + if (diagnostic.code == inCode && diagnostic.message == inMessage) + { + return true; + } + } + return false; +} +} // namespace tupletNumberApiTest + +// A tuplet inside another tuplet: both are open at once, so they need different numbers. The +// outer one is met first and takes 1. +TEST(nestedIdentityTupletsAreNumberedOneAndTwo, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{std::string{"outer"}}); + addStart(appendNote(score), SpannerNumber{std::string{"inner"}}); + addStop(appendNote(score), SpannerNumber{std::string{"inner"}}); + addStop(appendNote(score), SpannerNumber{std::string{"outer"}}); + + const auto sequence = tupletSequence(toXml(score)); + REQUIRE(4 == sequence.size()); + CHECK_EQUAL(std::string{"start"}, sequence.at(0).first); + CHECK_EQUAL(std::string{"1"}, sequence.at(0).second); + CHECK_EQUAL(std::string{"start"}, sequence.at(1).first); + CHECK_EQUAL(std::string{"2"}, sequence.at(1).second); + CHECK_EQUAL(std::string{"stop"}, sequence.at(2).first); + CHECK_EQUAL(std::string{"2"}, sequence.at(2).second); + CHECK_EQUAL(std::string{"stop"}, sequence.at(3).first); + CHECK_EQUAL(std::string{"1"}, sequence.at(3).second); +} + +T_END + +// Two tuplets that follow one another are never open at the same time, so both are number 1. +TEST(sequentialIdentityTupletsBothGetOne, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{std::string{"first"}}); + addStop(appendNote(score), SpannerNumber{std::string{"first"}}); + addStart(appendNote(score), SpannerNumber{std::string{"second"}}); + addStop(appendNote(score), SpannerNumber{std::string{"second"}}); + + const auto sequence = tupletSequence(toXml(score)); + REQUIRE(4 == sequence.size()); + CHECK_EQUAL(std::string{"1"}, sequence.at(0).second); + CHECK_EQUAL(std::string{"1"}, sequence.at(1).second); + CHECK_EQUAL(std::string{"1"}, sequence.at(2).second); + CHECK_EQUAL(std::string{"1"}, sequence.at(3).second); +} + +T_END + +// An author who states the number keeps it. +TEST(explicitLevelsAreWrittenAsGiven, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{4}); + addStop(appendNote(score), SpannerNumber{4}); + + const auto sequence = tupletSequence(toXml(score)); + REQUIRE(2 == sequence.size()); + CHECK_EQUAL(std::string{"4"}, sequence.at(0).second); + CHECK_EQUAL(std::string{"4"}, sequence.at(1).second); +} + +T_END + +// A lone tuplet needs no number, and MusicXML reads a missing one as 1. +TEST(unspecifiedNumberOmitsTheAttribute, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{}); + addStop(appendNote(score), SpannerNumber{}); + + const auto sequence = tupletSequence(toXml(score)); + REQUIRE(2 == sequence.size()); + CHECK_EQUAL(std::string{}, sequence.at(0).second); + CHECK_EQUAL(std::string{}, sequence.at(1).second); +} + +T_END + +// Two tuplets open at once with no numbers cannot be told apart by whoever reads the file, so +// the write says so. +TEST(twoUnnumberedTupletsOpenAtOnceIsReported, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{}); + addStart(appendNote(score), SpannerNumber{}); + addStop(appendNote(score), SpannerNumber{}); + addStop(appendNote(score), SpannerNumber{}); + + Diagnostics diagnostics; + const auto document = fromScore(score, diagnostics); + REQUIRE(document.ok()); + CHECK(hasDiagnostic(diagnostics, DiagnosticCode::missingValueDefaulted, + "two tuplets without a number are open at once; readers will default both to 1")); + + // one tuplet at a time is the ordinary case and says nothing + auto ordinary = makeScore(); + addStart(appendNote(ordinary), SpannerNumber{}); + addStop(appendNote(ordinary), SpannerNumber{}); + Diagnostics quiet; + REQUIRE(fromScore(ordinary, quiet).ok()); + CHECK(quiet.all().empty()); +} + +T_END + +// A tuplet that never closes is reported like any other unfinished spanner. +TEST(unmatchedTupletStartIsReported, TupletNumber) +{ + using namespace tupletNumberApiTest; + auto score = makeScore(); + addStart(appendNote(score), SpannerNumber{std::string{"never-closed"}}); + + Diagnostics diagnostics; + REQUIRE(fromScore(score, diagnostics).ok()); + CHECK(hasDiagnostic(diagnostics, DiagnosticCode::unmatchedSpanner, "tuplet start has no matching stop")); +} + +T_END + +#endif From 4cb1ab16787767e66afbae7b5e26a9a5410ebbde Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 20 Sep 2026 18:47:57 +0200 Subject: [PATCH 2/3] test: pin the tuplet corpus files that now round-trip Adds a hand-built nested tuplet fixture: a triplet of eighths inside a triplet of quarters, so the inner notes carry the product 9:4 and the inner tuplet is the one that has to state its own ratio. Bumps the pinned corpus count and adds the seventeen wild files and three synthetic ones that the tuplet work unlocks to the api round-trip pass-list. --- data/corpus.xml | 174 +++++++++++------- data/synthetic/tuplet-nested.3.0.features.xml | 121 ++++++++++++ data/synthetic/tuplet-nested.3.0.xml | 110 +++++++++++ src/private/mxtest/api/roundtrip-baseline.txt | 35 +++- .../mxtest/corert/CoreRoundtripTest.cpp | 4 +- 5 files changed, 368 insertions(+), 76 deletions(-) create mode 100644 data/synthetic/tuplet-nested.3.0.features.xml create mode 100644 data/synthetic/tuplet-nested.3.0.xml diff --git a/data/corpus.xml b/data/corpus.xml index b330bc797..df14cdd7e 100644 --- a/data/corpus.xml +++ b/data/corpus.xml @@ -4,13 +4,13 @@ usage (descending). Do not edit by hand; regenerate with the tool. --> - + - + - + @@ -32,7 +32,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -401,6 +401,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -428,9 +429,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -447,7 +448,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -816,6 +817,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -843,7 +845,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -859,7 +861,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -1228,6 +1230,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -1255,7 +1258,7 @@ synthetic/work-title.3.0.xml - + @@ -1284,7 +1287,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -1653,6 +1656,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -1680,9 +1684,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -1699,7 +1703,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2068,6 +2072,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -2095,9 +2100,9 @@ synthetic/work-title.3.0.xml - + - + custom/musescore-slur-start-stop.musicxml @@ -2114,7 +2119,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2483,6 +2488,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -2510,7 +2516,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2526,7 +2532,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -2542,10 +2548,11 @@ synthetic/segno.3.1.xml synthetic/suffix.3.0.xml synthetic/tied.cue.4.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2561,7 +2568,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/beat-repeat.3.0.xml synthetic/beat-type.3.0.xml synthetic/beats.3.0.xml @@ -2604,10 +2611,11 @@ synthetic/time.3.1.xml synthetic/transpose.3.0.xml synthetic/transpose.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -2623,7 +2631,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -2633,10 +2641,11 @@ synthetic/segno.3.0.xml synthetic/segno.3.1.xml synthetic/tied.cue.4.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + @@ -2675,7 +2684,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -2883,6 +2892,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -2904,7 +2914,7 @@ synthetic/work-title.3.0.xml - + @@ -2937,16 +2947,17 @@ foundsuite/Invention 3.xml - + synthetic/cancel.location.3.0.xml synthetic/clef.3.0.xml synthetic/clef.3.1.xml synthetic/key-accidental.3.0.xml synthetic/key-accidental.smufl.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/systems-and-pages.xml @@ -2962,14 +2973,15 @@ foundsuite/Invention 3.xml - + synthetic/cancel.location.3.0.xml synthetic/key-accidental.3.0.xml synthetic/key-accidental.smufl.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/systems-and-pages.xml @@ -2985,17 +2997,18 @@ foundsuite/Invention 3.xml - + synthetic/cancel.location.3.0.xml synthetic/clef.3.0.xml synthetic/clef.3.1.xml synthetic/key-accidental.3.0.xml synthetic/key-accidental.smufl.3.1.xml synthetic/part-clef.4.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + @@ -3014,7 +3027,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/cancel.location.3.0.xml synthetic/coda.3.0.xml synthetic/coda.3.1.xml @@ -3024,10 +3037,11 @@ synthetic/segno.3.0.xml synthetic/segno.3.1.xml synthetic/tied.cue.4.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + @@ -3061,7 +3075,7 @@ foundsuite/Invention 2.xml - + synthetic/beat-type.3.0.xml synthetic/beats.3.0.xml synthetic/cancel.location.3.0.xml @@ -3072,10 +3086,11 @@ synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/systems-and-pages.xml @@ -3091,7 +3106,7 @@ foundsuite/Invention 2.xml - + synthetic/beat-type.3.0.xml synthetic/beats.3.0.xml synthetic/cancel.location.3.0.xml @@ -3101,10 +3116,11 @@ synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/systems-and-pages.xml @@ -3120,7 +3136,7 @@ foundsuite/Invention 2.xml - + synthetic/beat-type.3.0.xml synthetic/beats.3.0.xml synthetic/cancel.location.3.0.xml @@ -3130,10 +3146,11 @@ synthetic/time-relation.3.0.xml synthetic/time.3.0.xml synthetic/time.3.1.xml + synthetic/tuplet-nested.3.0.xml synthetic/words-symbol.4.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3149,7 +3166,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3435,6 +3452,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -3461,7 +3479,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3477,7 +3495,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3685,6 +3703,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -3706,7 +3725,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3722,7 +3741,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -3930,6 +3949,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -3951,7 +3971,7 @@ synthetic/work-title.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/segno_coda_roundtrip.3.0.xml @@ -3967,7 +3987,7 @@ foundsuite/Deutscher Tanz D.820.1.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -4175,6 +4195,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -4223,7 +4244,7 @@ synthetic/source.3.0.xml - + @@ -4253,16 +4274,17 @@ foundsuite/Invention 2.xml - + synthetic/cancel.location.3.0.xml synthetic/key-accidental.3.0.xml synthetic/key-accidental.smufl.3.1.xml synthetic/key-octave.3.0.xml synthetic/key.3.0.xml synthetic/key.3.1.xml + synthetic/tuplet-nested.3.0.xml - + custom/musescore-slur-start-stop.musicxml custom/systems-and-pages.xml @@ -4278,11 +4300,12 @@ foundsuite/Invention 2.xml - + synthetic/cancel.location.3.0.xml synthetic/key-octave.3.0.xml synthetic/key.3.0.xml synthetic/key.3.1.xml + synthetic/tuplet-nested.3.0.xml @@ -4623,7 +4646,7 @@ - + @@ -4643,7 +4666,7 @@ foundsuite/Invention_10.xml - + synthetic/accent.3.0.xml synthetic/accidental-mark.3.0.xml synthetic/accidental-mark.3.1.xml @@ -4754,6 +4777,7 @@ synthetic/trill-mark.3.0.xml synthetic/triple-tongue.3.0.xml synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -6990,7 +7014,7 @@ synthetic/breath-mark.3.0.xml - + foundsuite/12 Horn Duets.xml foundsuite/An Chloe.xml @@ -7006,11 +7030,12 @@ ksuite/k011a_Tuplets.xml - + synthetic/metronome-tuplet.3.0.xml + synthetic/tuplet-nested.3.0.xml - + foundsuite/12 Horn Duets.xml foundsuite/An Chloe.xml @@ -7026,11 +7051,12 @@ ksuite/k011a_Tuplets.xml - + synthetic/metronome-tuplet.3.0.xml + synthetic/tuplet-nested.3.0.xml - + foundsuite/12 Horn Duets.xml foundsuite/An Chloe.xml @@ -7046,7 +7072,8 @@ ksuite/k011a_Tuplets.xml - + + synthetic/tuplet-nested.3.0.xml @@ -7072,11 +7099,11 @@ synthetic/rights.3.0.xml - + - + - + @@ -7102,8 +7129,9 @@ ksuite/k011a_Tuplets.xml - + synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml synthetic/tuplet.3.0.xml @@ -11659,26 +11687,28 @@ synthetic/triple-tongue.3.0.xml - + lysuite/ly23c_Tuplet_Display_NonStandard.xml lysuite/ly23d_Tuplets_Nested.xml - + synthetic/tuplet-dot.3.0.xml + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml synthetic/tuplet-type.3.0.xml - + lysuite/ly23c_Tuplet_Display_NonStandard.xml lysuite/ly23d_Tuplets_Nested.xml - + + synthetic/tuplet-nested.3.0.xml - + @@ -11690,11 +11720,12 @@ lysuite/ly23c_Tuplet_Display_NonStandard.xml lysuite/ly23d_Tuplets_Nested.xml - + + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-number.3.0.xml - + @@ -11706,7 +11737,8 @@ lysuite/ly23c_Tuplet_Display_NonStandard.xml lysuite/ly23d_Tuplets_Nested.xml - + + synthetic/tuplet-nested.3.0.xml synthetic/tuplet-type.3.0.xml diff --git a/data/synthetic/tuplet-nested.3.0.features.xml b/data/synthetic/tuplet-nested.3.0.features.xml new file mode 100644 index 000000000..fbe5c7c42 --- /dev/null +++ b/data/synthetic/tuplet-nested.3.0.features.xml @@ -0,0 +1,121 @@ + + + + synthetic/tuplet-nested.3.0.xml + 3.0 + + + actual-notes + + + attributes + + + beat-type + + + beats + + + clef + + + divisions + + + duration + + + fifths + + + key + + + line + + + measure + + number + + + + normal-notes + + + notations + + + note + + + octave + + + part + + id + + + + part-list + + + part-name + + + pitch + + + score-part + + id + + + + score-partwise + + version + + + + sign + + + step + + + time + + + time-modification + + + tuplet + + number + type + + + + tuplet-actual + + + tuplet-normal + + + tuplet-number + + + tuplet-type + + + type + + + voice + + + diff --git a/data/synthetic/tuplet-nested.3.0.xml b/data/synthetic/tuplet-nested.3.0.xml new file mode 100644 index 000000000..fedc66a4e --- /dev/null +++ b/data/synthetic/tuplet-nested.3.0.xml @@ -0,0 +1,110 @@ + + + + + Nested Tuplets + + + + + + 36 + + 0 + + + + G + 2 + + + + + C + 5 + + 8 + 1 + eighth + + 9 + 4 + + + + + + 3 + eighth + + + 2 + eighth + + + + + + + D + 5 + + 8 + 1 + eighth + + 9 + 4 + + + + + E + 5 + + 8 + 1 + eighth + + 9 + 4 + + + + + + + + F + 5 + + 24 + 1 + quarter + + 3 + 2 + + + + + G + 5 + + 24 + 1 + quarter + + 3 + 2 + + + + + + + + diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 7a25c0e60..cdfda3994 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -722,9 +722,8 @@ lysuite/ly33b_Spanners_Tie.xml # export of a glissando and a tab slide drawn from a note toward a rest, so # each line starts and stops on the one note it can anchor to, written # start-then-stop. The writer used to emit every stop before every start, -# inverting the pair. (Its sibling tuplet_singletons-ref.xml is not pinned: -# the ordering is fixed, but the writer restates / -# on a start whose source omits them.) +# inverting the pair. (Its sibling tuplet_singletons-ref.xml is pinned with the +# tuplet portion work below.) rpatters1/gliss_to_rest-ref.xml # Unblocked by VoiceData::label: mx renumbered every voice from the position of @@ -749,3 +748,33 @@ synthetic/octave-shift.3.1.xml # middle or bottom, the valign-image vocabulary. This fixture's valign="top" was # its only divergence. synthetic/credit-image.3.0.xml + +# Unblocked by tuplet portion fidelity. A may state its ratio in +# / or leave it to the note's ; +# mx used to read both shapes the same way and then write both portions onto +# every start, inventing a where the source had none. The reader +# now records which shape the file used and the writer states a portion only +# where it says something the note does not already say. ly23b and ly23c also +# needed show-type and line-shape, and PreserveTimeModificationNormalType needed +# bracket on a tuplet stop. ly23d is the corpus's one genuinely nested tuplet, +# and tuplet-nested.3.0.xml is its synthetic companion. +foundsuite/Invention_5.xml +ksuite/k011a_Tuplets.xml +lysuite/ly23a_Tuplets.xml +lysuite/ly23b_Tuplets_Styles.xml +lysuite/ly23c_Tuplet_Display_NonStandard.xml +lysuite/ly23d_Tuplets_Nested.xml +mjbsuite/PreserveTimeModificationNormalType.xml +musuite/testDurationRoundingError.xml +musuite/testDurationRoundingError_ref.xml +musuite/testTuplets1.xml +musuite/testTuplets1_ref.xml +musuite/testTuplets2.xml +musuite/testTuplets2_ref.xml +musuite/testTuplets3.xml +musuite/testTuplets3_ref.xml +musuite/testTuplets4.xml +rpatters1/tuplet_singletons-ref.xml +synthetic/tuplet-nested.3.0.xml +synthetic/tuplet.3.0.xml +synthetic/tuplet.3.1.xml diff --git a/src/private/mxtest/corert/CoreRoundtripTest.cpp b/src/private/mxtest/corert/CoreRoundtripTest.cpp index 115f4c66c..0db47734c 100644 --- a/src/private/mxtest/corert/CoreRoundtripTest.cpp +++ b/src/private/mxtest/corert/CoreRoundtripTest.cpp @@ -127,12 +127,12 @@ const CoreRoundtripRegistrar g_coreRoundtripRegistrar; } // namespace -// Pinned counts: 840 eligible files, none skipped. Count drift is a failure +// Pinned counts: 841 eligible files, none skipped. Count drift is a failure // even with zero individual fails, so a corpus or version-gate change is a // conscious decision, not silent decay. Registered last (registration is // discovery order; "zz" keeps it last alphabetically for shuffled runs too). TEST_CASE("zz-corert-pinned-counts", "[core-roundtrip]") { - CHECK(mxtest::corert::discoverInputFiles().size() == 840); + CHECK(mxtest::corert::discoverInputFiles().size() == 841); CHECK(g_skippedCount == 0); } From b25904c361ac54e8c69a7c1734bdd9fae2fbb2a0 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Mon, 21 Sep 2026 09:03:33 +0200 Subject: [PATCH 3/3] docs: clarify tuplet show-type and fidelity --- src/include/mx/api/TupletData.h | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/include/mx/api/TupletData.h b/src/include/mx/api/TupletData.h index d4b248e2b..28af5a65a 100644 --- a/src/include/mx/api/TupletData.h +++ b/src/include/mx/api/TupletData.h @@ -59,12 +59,15 @@ class TupletStart // a triplet or the 4 in 5:4 for quintuplet Bool showNormalNumber; - // show the note value of the tuplet's own notes, - // for example the eighth note in "3 eighths" + // Whether the note value of the tuplet's own notes is shown beside the bracket, for example the + // eighth note in "3 eighths". This is optional. Left unspecified, the file does not say, and a + // reader leaves the note values off, which is what MusicXML asks for. Set it to yes to show + // them or to no to keep them off. Bool showActualType; - // show the note value of the 'normal' notes, - // for example the eighth note in "in the time of 2 eighths" + // Whether the note value of the 'normal' notes is shown as well, for example the eighth note in + // "in the time of 2 eighths". Also optional. MusicXML states both sides in a single attribute, + // so this field only takes effect when showActualType is yes. Bool showNormalType; Bool bracket; @@ -72,12 +75,18 @@ class TupletStart // Whether the bracket is drawn as a straight line or in the older curved style. TupletLineShape lineShape; - // Most users can ignore these; leave them unspecified. They control whether the ratio is - // spelled out in the file alongside the note's own time modification. unspecified (the - // default) applies the right rule automatically: state the ratio when it differs from what - // the note already says, which is what an inner tuplet needs, and leave it out otherwise. - // yes/no force it in or out, so a writer that finds the ratio noisy can turn it off. Reading - // a file sets yes when the source stated the ratio. + // Whether the actual and normal figures above are spelled out in the file next to the notes. + // mx works this out on its own, so leave both unspecified; they are here for the rare case + // where you want to decide instead. + // + // Unspecified spells out a figure when it says something the notes do not already say. Nested + // tuplets are where that happens. When one tuplet is written inside another, each note counts + // against both tuplets at once, so the note by itself cannot tell you the inner tuplet's own + // ratio, and the tuplet has to state it. A tuplet standing on its own agrees with its notes and + // is left plain. Setting yes spells the figure out anyway, and no leaves it out. + // + // Reading a file sets yes for each figure the file spelled out, so that a score read and + // written back keeps what it had. Bool writeActual; Bool writeNormal;