From 54771cf1a016b95428d1a419e27db89cd7ce524e Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Mon, 6 Jul 2026 06:05:14 +0000 Subject: [PATCH] feat(api): measure-level and staff-scoped transposition changes Splits the transposition half out of the original combined time-signature and transposition PR so each can be reviewed and merged independently. Closes #269 --- src/include/mx/api/MeasureData.h | 9 ++ src/include/mx/api/TransposeData.h | 9 ++ src/private/mx/impl/MeasureReader.cpp | 26 ++++- src/private/mx/impl/MeasureWriter.cpp | 33 +++++- src/private/mx/impl/MeasureWriter.h | 2 + src/private/mx/impl/PropertiesWriter.cpp | 8 +- src/private/mx/impl/PropertiesWriter.h | 2 +- src/private/mxtest/api/TranspositionTest.cpp | 110 +++++++++++++++++++ 8 files changed, 190 insertions(+), 9 deletions(-) diff --git a/src/include/mx/api/MeasureData.h b/src/include/mx/api/MeasureData.h index 36fb7b04c..659796157 100644 --- a/src/include/mx/api/MeasureData.h +++ b/src/include/mx/api/MeasureData.h @@ -13,6 +13,7 @@ #include "mx/api/StaffData.h" #include "mx/api/TempoData.h" #include "mx/api/TimeSignatureData.h" +#include "mx/api/TransposeData.h" #include #include @@ -91,6 +92,13 @@ class MeasureData std::vector barlines; std::optional partSymbol; + // Transposition changes found in this measure's (any tick position). Unlike + // 'keys', this is not carried forward: it is only populated when this measure actually + // contains one or more elements. The very first transpose at measure 0/tick 0 is + // additionally (and independently) surfaced via PartData::transposition, unchanged from + // before this field existed. + std::vector transpositions; + MeasureData() : staves{}, timeSignature{}, number{}, measureNumbering{MeasureNumbering::unspecified}, multiMeasureRest{VALUE_UNSPECIFIED}, implicit{Bool::unspecified}, nonControlling{Bool::unspecified}, @@ -111,6 +119,7 @@ MXAPI_EQUALS_MEMBER(width) MXAPI_EQUALS_MEMBER(keys) MXAPI_EQUALS_MEMBER(barlines) MXAPI_EQUALS_MEMBER(partSymbol) +MXAPI_EQUALS_MEMBER(transpositions) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(MeasureData); } // namespace api diff --git a/src/include/mx/api/TransposeData.h b/src/include/mx/api/TransposeData.h index 8177ad613..d0f924b74 100644 --- a/src/include/mx/api/TransposeData.h +++ b/src/include/mx/api/TransposeData.h @@ -63,6 +63,13 @@ class TransposeData /// The number of diatonic steps in the interval int diatonic; + /// this value is optional. INDEX_UNSPECIFIED means unspecified. when value is + /// unspecified it means that the transposition applies to all staves within the part + int staffIndex = INDEX_UNSPECIFIED; + + /// Supports a transposition change somewhere other than at the start of a measure. + int tickTimePosition = 0; + /// If both chromatic and diatonic are zero, then TransposeData is unused (i.e. it need /// not be encoded in the MusicXML output). inline bool isUsed() const @@ -74,6 +81,8 @@ class TransposeData MXAPI_EQUALS_BEGIN(TransposeData) MXAPI_EQUALS_MEMBER(chromatic) MXAPI_EQUALS_MEMBER(diatonic) +MXAPI_EQUALS_MEMBER(staffIndex) +MXAPI_EQUALS_MEMBER(tickTimePosition) MXAPI_EQUALS_END; MXAPI_NOT_EQUALS_AND_VECTORS(TransposeData); } // namespace api diff --git a/src/private/mx/impl/MeasureReader.cpp b/src/private/mx/impl/MeasureReader.cpp index 0b3d4d6b5..ae8b2becf 100644 --- a/src/private/mx/impl/MeasureReader.cpp +++ b/src/private/mx/impl/MeasureReader.cpp @@ -532,12 +532,34 @@ std::optional MeasureReader::parseAttributes(const core::Att if (inMxAttributes.choice().isTranspose() && !inMxAttributes.choice().asTranspose().empty()) { - // TODO support transpositions at places other than the start of the score + const auto &transposeVec = inMxAttributes.choice().asTranspose(); + + // The very first transpose at measure 0/tick 0 is (still, unconditionally) treated as a + // property of the part rather than the measure. This selection logic is unchanged. if (myCurrentCursor.measureIndex == 0 && myCurrentCursor.tickTimePosition == 0) { - const auto &coreTranspose = inMxAttributes.choice().asTranspose().front(); + const auto &coreTranspose = transposeVec.front(); transpose = Converter::convertToTransposeData(coreTranspose); } + + // Additionally and independently, every element found anywhere is recorded + // onto the measure's transpositions list. + for (const auto &coreTranspose : transposeVec) + { + auto transposeData = Converter::convertToTransposeData(coreTranspose); + + if (coreTranspose.number().has_value()) + { + transposeData.staffIndex = coreTranspose.number()->value() - 1; + if (transposeData.staffIndex > myCurrentCursor.getNumStaves() - 1) + { + transposeData.staffIndex = api::INDEX_UNSPECIFIED; + } + } + + transposeData.tickTimePosition = myCurrentCursor.tickTimePosition; + myOutMeasureData.transpositions.emplace_back(std::move(transposeData)); + } } return transpose; } diff --git a/src/private/mx/impl/MeasureWriter.cpp b/src/private/mx/impl/MeasureWriter.cpp index 43a496476..9b974269e 100644 --- a/src/private/mx/impl/MeasureWriter.cpp +++ b/src/private/mx/impl/MeasureWriter.cpp @@ -47,7 +47,8 @@ MeasureWriter::MeasureWriter(const api::MeasureData &inMeasureData, const Measur : myMeasureData{inMeasureData}, myOutMeasure{}, myPreviousCursor{inCursor}, myScoreWriter{inScoreWriter}, myPropertiesWriter{nullptr}, myConverter{}, myBarlinesIter{inMeasureData.barlines.cbegin()}, myBarlinesEnd{inMeasureData.barlines.cend()}, myMeasureKeysIter{inMeasureData.keys.cbegin()}, - myMeasureKeysEnd{inMeasureData.keys.cend()}, myHistory{inCursor} + myMeasureKeysEnd{inMeasureData.keys.cend()}, myMeasureTransposeIter{inMeasureData.transpositions.cbegin()}, + myMeasureTransposeEnd{inMeasureData.transpositions.cend()}, myHistory{inCursor} { } @@ -61,6 +62,7 @@ core::PartwiseMeasure MeasureWriter::getPartwiseMeasure() myBarlinesIter = myMeasureData.barlines.cbegin(); myBarlinesEnd = myMeasureData.barlines.cend(); myMeasureKeysIter = myMeasureData.keys.cbegin(); + myMeasureTransposeIter = myMeasureData.transpositions.cbegin(); writeMeasureGlobals(); writeStaves(); @@ -161,15 +163,23 @@ void MeasureWriter::writeMeasureGlobals() ++myMeasureKeysIter; } + while (myMeasureTransposeIter != myMeasureTransposeEnd && myMeasureTransposeIter->tickTimePosition == 0) + { + myPropertiesWriter->writeTranspose(myMeasureTransposeIter->staffIndex, *myMeasureTransposeIter); + ++myMeasureTransposeIter; + } + // The transpose element goes into the measures, but for convenience we have added it to // the mx::api::PartData struct since the most typical use case is to add it as part of - // an instrument/part definition. - if (cursor().measureIndex == 0 && cursor().tickTimePosition == 0) + // an instrument/part definition. This fallback only runs when this measure has no explicit + // transpositions of its own, so old hand-authored ScoreData using only the old field keeps + // working. + if (myMeasureData.transpositions.empty() && cursor().measureIndex == 0 && cursor().tickTimePosition == 0) { const auto &part = this->myScoreWriter.getPart(cursor().partIndex); if (part.transposition && part.transposition->isUsed()) { - myPropertiesWriter->writeTranspose(part.transposition.value()); + myPropertiesWriter->writeTranspose(api::INDEX_UNSPECIFIED, part.transposition.value()); } } } @@ -437,6 +447,13 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff) } } + while (myMeasureTransposeIter != myMeasureTransposeEnd && + myMeasureTransposeIter->tickTimePosition <= myHistory.getCursor().tickTimePosition) + { + myPropertiesWriter->writeTranspose(myMeasureTransposeIter->staffIndex, *myMeasureTransposeIter); + ++myMeasureTransposeIter; + } + while (clefIter != clefEnd && clefIter->tickTimePosition <= myHistory.getCursor().tickTimePosition) { myPropertiesWriter->writeClef(clefStaffIndex(myHistory.getCursor().staffIndex), *clefIter); @@ -482,8 +499,9 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff) bool areClefsRemaining = clefIter != clefEnd; bool areMeasureKeysRemaining = myMeasureKeysIter != myMeasureKeysEnd; bool areStaffKeysRemaining = staffKeyIter != staffKeyEnd; + bool areMeasureTransposesRemaining = myMeasureTransposeIter != myMeasureTransposeEnd; - if (areClefsRemaining || areMeasureKeysRemaining || areStaffKeysRemaining) + if (areClefsRemaining || areMeasureKeysRemaining || areStaffKeysRemaining || areMeasureTransposesRemaining) { for (; clefIter != inStaff.clefs.cend(); ++clefIter) { @@ -505,6 +523,11 @@ void MeasureWriter::writeVoices(const api::StaffData &inStaff) { myPropertiesWriter->writeKey(myHistory.getCursor().staffIndex, *staffKeyIter); } + + for (; myMeasureTransposeIter != myMeasureTransposeEnd; ++myMeasureTransposeIter) + { + myPropertiesWriter->writeTranspose(myMeasureTransposeIter->staffIndex, *myMeasureTransposeIter); + } myPropertiesWriter->flushBuffer(); } diff --git a/src/private/mx/impl/MeasureWriter.h b/src/private/mx/impl/MeasureWriter.h index 3abaa5dc4..d0e9b13e1 100644 --- a/src/private/mx/impl/MeasureWriter.h +++ b/src/private/mx/impl/MeasureWriter.h @@ -199,6 +199,8 @@ class MeasureWriter std::vector::const_iterator myBarlinesEnd; std::vector::const_iterator myMeasureKeysIter; const std::vector::const_iterator myMeasureKeysEnd; + std::vector::const_iterator myMeasureTransposeIter; + const std::vector::const_iterator myMeasureTransposeEnd; History myHistory; private: diff --git a/src/private/mx/impl/PropertiesWriter.cpp b/src/private/mx/impl/PropertiesWriter.cpp index 09e44b10d..342f0a9bc 100644 --- a/src/private/mx/impl/PropertiesWriter.cpp +++ b/src/private/mx/impl/PropertiesWriter.cpp @@ -289,9 +289,15 @@ void PropertiesWriter::writePartSymbol(const api::PartSymbolData &inPartSymbolDa myHasContent = true; } -void PropertiesWriter::writeTranspose(const api::TransposeData &inTransposeData) +void PropertiesWriter::writeTranspose(int staffIndex, const api::TransposeData &inTransposeData) { auto xpose = Converter::convertToTranspose(inTransposeData); + + if (staffIndex >= 0) + { + xpose.setNumber(core::StaffNumber{staffIndex + 1}); + } + auto vec = myAttributes.choice().isTranspose() ? myAttributes.choice().asTranspose() : std::vector{}; vec.push_back(std::move(xpose)); diff --git a/src/private/mx/impl/PropertiesWriter.h b/src/private/mx/impl/PropertiesWriter.h index ffa03cb49..b3eb43021 100644 --- a/src/private/mx/impl/PropertiesWriter.h +++ b/src/private/mx/impl/PropertiesWriter.h @@ -56,7 +56,7 @@ class PropertiesWriter void writeStaffDetails(int staffIndex, int staffLines, double staffSize, double staffScaling); void writeClef(int staffIndex, const api::ClefData &inClefData); void writePartSymbol(const api::PartSymbolData &inPartSymbolData); - void writeTranspose(const api::TransposeData &inTransposeData); + void writeTranspose(int staffIndex, const api::TransposeData &inTransposeData); private: void allocate(); diff --git a/src/private/mxtest/api/TranspositionTest.cpp b/src/private/mxtest/api/TranspositionTest.cpp index fe50f1d0e..8d972721c 100644 --- a/src/private/mxtest/api/TranspositionTest.cpp +++ b/src/private/mxtest/api/TranspositionTest.cpp @@ -14,6 +14,7 @@ #include "mx/core/generated/Transpose.h" #include "mx/core/generated/TransposeGroup.h" #include "mx/impl/Converter.h" +#include "mxtest/api/TestHelpers.h" #include "mxtest/file/MxFileRepository.h" #include @@ -792,6 +793,115 @@ TEST(Tests10, Transposition) CHECK_EQUAL(diatonic, result.parts.back().transposition->diatonic); } +TEST(TransposeDataEquality_change_staffIndex, TransposeData) +{ + mx::api::TransposeData t1{-2, -1}; + t1.staffIndex = 0; + t1.tickTimePosition = 13; + auto t2 = t1; + CHECK(t1 == t2); + CHECK(!(t1 != t2)); + + t1.staffIndex += 1; + CHECK(t1 != t2); + CHECK(!(t1 == t2)); +} + +TEST(TransposeDataEquality_change_tickTimePosition, TransposeData) +{ + mx::api::TransposeData t1{-2, -1}; + t1.staffIndex = 0; + t1.tickTimePosition = 13; + auto t2 = t1; + CHECK(t1 == t2); + + t1.tickTimePosition += 1; + CHECK(t1 != t2); + CHECK(!(t1 == t2)); +} + +TEST(midPieceTranspositionAppearsOnMeasure, TransposeMeasure) +{ + auto score = mxtest::makeScore(2); + auto &part = score.parts.back(); + part.transposition = mx::api::TransposeData{-2, -1}; + part.measures.at(0).staves.emplace_back(mx::api::StaffData{}); + part.measures.at(1).staves.emplace_back(mx::api::StaffData{}); + + mx::api::TransposeData midPieceChange{2, 1}; + midPieceChange.tickTimePosition = 0; + part.measures.at(1).transpositions.push_back(midPieceChange); + + const auto result = mxtest::roundtrip(score); + REQUIRE(result.parts.size() == 1); + const auto &resultPart = result.parts.front(); + + // the part-level transposition (measure 0/tick 0) is unaffected + REQUIRE(resultPart.transposition.has_value()); + CHECK_EQUAL(-2, resultPart.transposition->chromatic); + CHECK_EQUAL(-1, resultPart.transposition->diatonic); + + // and the mid-piece change shows up on the measure that carries it. the measure 0 / tick 0 + // written from the part-level fallback is also, independently, surfaced on + // measure 0's own transpositions list (per the #269 design: any found anywhere + // is recorded onto the measure that contains it, in addition to the backward-compatible + // PartData::transposition selection above). + REQUIRE(resultPart.measures.size() == 2); + REQUIRE(resultPart.measures.at(0).transpositions.size() == 1); + CHECK_EQUAL(-2, resultPart.measures.at(0).transpositions.front().chromatic); + CHECK_EQUAL(-1, resultPart.measures.at(0).transpositions.front().diatonic); + REQUIRE(resultPart.measures.at(1).transpositions.size() == 1); + CHECK_EQUAL(2, resultPart.measures.at(1).transpositions.front().chromatic); + CHECK_EQUAL(1, resultPart.measures.at(1).transpositions.front().diatonic); +} + +TEST(staffScopedTranspositionRoundTrip, TransposeMeasure) +{ + auto score = mxtest::makeScore(1); + auto &part = score.parts.back(); + auto &measure = part.measures.back(); + measure.staves.emplace_back(mx::api::StaffData{}); + measure.staves.emplace_back(mx::api::StaffData{}); + + mx::api::TransposeData staff0Transpose{-2, -1}; + staff0Transpose.staffIndex = 0; + mx::api::TransposeData staff1Transpose{-9, -5}; + staff1Transpose.staffIndex = 1; + + measure.transpositions.push_back(staff0Transpose); + measure.transpositions.push_back(staff1Transpose); + + const auto xml = mxtest::toXml(score); + CHECK(xml.find("number=\"1\"") != std::string::npos); + CHECK(xml.find("number=\"2\"") != std::string::npos); + + const auto result = mxtest::roundtrip(score); + REQUIRE(result.parts.size() == 1); + REQUIRE(result.parts.front().measures.size() == 1); + const auto &resultTranspositions = result.parts.front().measures.front().transpositions; + REQUIRE(resultTranspositions.size() == 2); + + bool foundStaff0 = false; + bool foundStaff1 = false; + for (const auto &t : resultTranspositions) + { + if (t.staffIndex == 0) + { + foundStaff0 = true; + CHECK_EQUAL(-2, t.chromatic); + CHECK_EQUAL(-1, t.diatonic); + } + else if (t.staffIndex == 1) + { + foundStaff1 = true; + CHECK_EQUAL(-9, t.chromatic); + CHECK_EQUAL(-5, t.diatonic); + } + } + CHECK(foundStaff0); + CHECK(foundStaff1); +} + TEST(Tests11, Transposition) { // this is a funky test case. these values are actually nonsense.