From cdac93545266caf58cda85bef0804c9ef29e9d04 Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sun, 12 Jul 2026 15:29:49 +0000 Subject: [PATCH] fix: do not invent for scores with no time resolution A source that declares no anywhere reads as ticksPerQuarter = 0 (LCM of an empty set). The writer then emitted divisions for the first measure of every part, and core::PositiveDivisions clamped the 0 to a bogus 1e-06. The writer now omits when the score has no time resolution; an otherwise-empty is no longer flushed. Pins the 41 corpus files this flips to PASS (212 total). --- src/include/mx/api/ScoreData.h | 6 +++ src/private/mx/impl/MeasureWriter.cpp | 8 ++- src/private/mxtest/api/roundtrip-baseline.txt | 48 ++++++++++++++++++ src/private/mxtest/impl/MeasureWriterTest.cpp | 49 +++++++++++++++++++ 4 files changed, 110 insertions(+), 1 deletion(-) diff --git a/src/include/mx/api/ScoreData.h b/src/include/mx/api/ScoreData.h index 63fde1fd1..81852c679 100644 --- a/src/include/mx/api/ScoreData.h +++ b/src/include/mx/api/ScoreData.h @@ -79,6 +79,12 @@ class ScoreData DefaultsData defaults; std::vector parts; std::vector partGroups; + + /// The score's time resolution: how many `tickTimePosition`/duration ticks make up one + /// quarter note. Corresponds to the `` MusicXML element. Choose a value with + /// enough factors for the durations you need (the default accommodates tuplets through + /// septuplets). Zero means the score has no time resolution at all — no durations exist + /// anywhere — and no `` is written. int ticksPerQuarter; /// Specifies page breaks, system breaks, and changes to system and page layout. Global/default page and diff --git a/src/private/mx/impl/MeasureWriter.cpp b/src/private/mx/impl/MeasureWriter.cpp index 481bff7ff..d050e6133 100644 --- a/src/private/mx/impl/MeasureWriter.cpp +++ b/src/private/mx/impl/MeasureWriter.cpp @@ -117,7 +117,13 @@ void MeasureWriter::writeMeasureGlobals() if (myHistory.getCursor().isFirstMeasureInPart) { - myPropertiesWriter->writeDivisions(myHistory.getCursor().getGlobalTicksPerQuarter()); + // A score with no time resolution (ticksPerQuarter <= 0: the source declared no + // and contains no durations) must not have one invented for it; writing + // 0 here would be clamped to a bogus positive value by core::PositiveDivisions. + if (myHistory.getCursor().getGlobalTicksPerQuarter() > 0) + { + myPropertiesWriter->writeDivisions(myHistory.getCursor().getGlobalTicksPerQuarter()); + } if (myMeasureData.staves.size() > 1) { diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 8bb4082ac..64395d60e 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -350,3 +350,51 @@ rpatters1/timesigs_composite-ref.musicxml # the serialized stream once voice 1 is written ahead of voice 2, plus a wedge. # Pins explicit slur/wedge numbers surviving read -> write -> read verbatim. rpatters1/slurs_overbars.musicxml + +# Unblocked by the spurious-divisions fix: a score whose source declares no +# anywhere (and has no durations) reads as ticksPerQuarter = 0, +# and the writer used to invent 0.000001 +# for the first measure of every part (LCM of an empty set = 0, clamped to a +# bogus positive by core::PositiveDivisions). The writer now omits +# when the score has no time resolution. +lysuite/ly51b_Header_Quotes.xml +mjbsuite/HasMusicXmlVersionFalse.xml +mjbsuite/HasMusicXmlVersionTrue.xml +synthetic/beat-type.3.0.xml +synthetic/beats.3.0.xml +synthetic/bracket.3.0.xml +synthetic/dashes.3.0.xml +synthetic/encoding-description.3.0.xml +synthetic/first.4.0.xml +synthetic/group.3.0.xml +synthetic/instrument-abbreviation.3.0.xml +synthetic/instrument-name.3.0.xml +synthetic/instrument-sound-enum.3.0.xml +synthetic/interchangeable.3.0.xml +synthetic/lyric-font.3.0.xml +synthetic/miscellaneous-field.3.0.xml +synthetic/movement-number.3.0.xml +synthetic/movement-title.3.0.xml +synthetic/music-font.3.0.xml +synthetic/n.3.1.xml +synthetic/numeral-fifths.4.0.xml +synthetic/numeral-mode.4.0.xml +synthetic/numeral.4.0.xml +synthetic/other-dynamics.3.0.xml +synthetic/pf.3.1.xml +synthetic/second.4.0.xml +synthetic/senza-misura.3.0.xml +synthetic/sfzp.3.1.xml +synthetic/software.3.0.xml +synthetic/staff-size.4.0.xml +synthetic/straight.4.0.xml +synthetic/swing-style.4.0.xml +synthetic/swing-type.4.0.xml +synthetic/swing.4.0.xml +synthetic/time-relation.3.0.xml +synthetic/virtual-library.3.0.xml +synthetic/virtual-name.3.0.xml +synthetic/voice.3.0.xml +synthetic/word-font.3.0.xml +synthetic/work-number.3.0.xml +synthetic/work-title.3.0.xml diff --git a/src/private/mxtest/impl/MeasureWriterTest.cpp b/src/private/mxtest/impl/MeasureWriterTest.cpp index ee8883458..4f2bfe947 100644 --- a/src/private/mxtest/impl/MeasureWriterTest.cpp +++ b/src/private/mxtest/impl/MeasureWriterTest.cpp @@ -224,6 +224,55 @@ TEST(PropertiesButNoNotes, MeasureWriter) T_END +TEST(ZeroTicksPerQuarterWritesNoDivisions, MeasureWriter) +{ + // A score with no time resolution (ticksPerQuarter == 0, e.g. a source that declared no + // and has no durations) must not have a bogus invented for it. + mxtest::TestParameters params; + params.ticksPerQuarter = 0; + params.measureIndex = 0; + params.partIndex = 0; + params.numStaves = 1; + mxtest::TestItems t = mxtest::setupTestItems(params); + + const auto partwiseMeasure = t.measureWriter->getPartwiseMeasure(); + CHECK(partwiseMeasure.musicData().empty()); +} + +T_END + +TEST(ZeroTicksPerQuarterKeepsOtherProperties, MeasureWriter) +{ + // With ticksPerQuarter == 0 the element is still written when it has real + // content (here a clef); only the child is suppressed. + mxtest::TestParameters params; + params.ticksPerQuarter = 0; + params.measureIndex = 0; + params.partIndex = 0; + params.numStaves = 1; + mxtest::TestItems t = mxtest::setupTestItems(params); + auto &staff = t.measureData->staves.at(0); + staff.clefs.emplace_back(api::ClefData{}); + auto &clef = staff.clefs.back(); + clef.symbol = api::ClefSymbol::g; + clef.line = 2; + + const auto partwiseMeasure = t.measureWriter->getPartwiseMeasure(); + auto musicData = partwiseMeasure.musicData(); + auto mdcIter = musicData.begin(); + auto mdcEnd = musicData.end(); + + CHECK(mdcIter != mdcEnd); + CHECK(mdcIter->isAttributes()); + const auto &props = mdcIter->asAttributes(); + CHECK(!props.divisions().has_value()); + CHECK_EQUAL(1, props.clef().size()); + + CHECK(++mdcIter == mdcEnd); +} + +T_END + TEST(MultiStaffClefKeepsNumber, MeasureWriter) { // Counterpart to PropertiesButNoNotes: in a multi-staff part the clef number is required