diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp
index 38a4781dd..113a60994 100644
--- a/src/private/mx/impl/DirectionReader.cpp
+++ b/src/private/mx/impl/DirectionReader.cpp
@@ -755,8 +755,10 @@ void DirectionReader::parseOctaveShift(const core::DirectionType &directionType)
bool isUp = octaveShift.type().tag() == core::UpDownStopContinue::Tag::up;
// MusicXML does not properly constrain the ottava size to valid music notation values. We do
- // so here by interpolating the value into an enum that maps to valid music notation.
- if (!isUp && amount == 22)
+ // so here by interpolating the value into an enum that maps to valid music notation. A size
+ // larger than 22 takes the widest line we have rather than falling back to the 15th, so the
+ // line we draw is the closest one to what the source asked for.
+ if (!isUp && amount >= 22)
{
ottavaType = api::OttavaType::o22ma;
}
@@ -768,7 +770,7 @@ void DirectionReader::parseOctaveShift(const core::DirectionType &directionType)
{
ottavaType = api::OttavaType::o8va;
}
- else if (isUp && amount == 22)
+ else if (isUp && amount >= 22)
{
ottavaType = api::OttavaType::o22mb;
}
diff --git a/src/private/mxtest/api/CorpusRoundtripMain.cpp b/src/private/mxtest/api/CorpusRoundtripMain.cpp
index 2f6e37fa4..21b40ba46 100644
--- a/src/private/mxtest/api/CorpusRoundtripMain.cpp
+++ b/src/private/mxtest/api/CorpusRoundtripMain.cpp
@@ -266,6 +266,37 @@ void canonicalizeDirectionSpellings(pugi::xml_document &doc)
mxtest::sortAttributes(doc);
}
+// MusicXML lets octave-shift/@size be any positive integer, so a corpus file can carry a number
+// that does not name a line a performer could read: 27, 11, 1. mx::api narrows each of those to
+// the closest ottava it can draw -- 22 and up become 22, anything else above 8 becomes 15, and the
+// rest become 8 -- because the api models the six ottava lines music notation has rather than the
+// whole integer range. That narrowing is what the api is for, so the expected document is brought to the
+// same value instead of the api growing a field to carry the original number back out.
+//
+// Only the expected side is narrowed. mx writes 8, 15, or 22, so a write that produced some other
+// size still fails. The size of a stop is taken here from the stop's own attribute, while mx takes
+// it from the start the stop closes, so a source whose stop contradicts its start still fails too.
+void narrowOctaveShiftSizes(pugi::xml_node el)
+{
+ if (std::string_view{el.name()} == "octave-shift")
+ {
+ pugi::xml_attribute size = el.attribute("size");
+ const int stated = size ? size.as_int(0) : 0;
+ if (stated > 0)
+ {
+ size.set_value(stated >= 22 ? 22 : (stated > 8 ? 15 : 8));
+ }
+ }
+
+ for (pugi::xml_node c = el.first_child(); c; c = c.next_sibling())
+ {
+ if (c.type() == pugi::node_element)
+ {
+ narrowOctaveShiftSizes(c);
+ }
+ }
+}
+
bool hasSuffix(const std::string &name, std::string_view suffix)
{
return name.size() >= suffix.size() && name.compare(name.size() - suffix.size(), suffix.size(), suffix) == 0;
@@ -445,6 +476,10 @@ RoundtripResult runRoundtrip(const std::string &absolutePath)
canonicalizeDirectionSpellings(expectedDoc);
canonicalizeDirectionSpellings(actualDoc);
+ // mx::api narrows an octave-shift size to a size music notation has, so the expected document
+ // states the narrowed size rather than the one the source wrote.
+ narrowOctaveShiftSizes(expectedDoc.document_element());
+
// Compare
const auto fail = mxtest::corert::compareElements(expectedDoc.document_element(), actualDoc.document_element());
if (fail.isFailure)
@@ -530,6 +565,7 @@ void dumpDocuments(const std::string &absolutePath, const std::string &relPath,
canonicalizeEncodingChildOrder(expectedDoc.document_element());
collapseEqualPageMargins(expectedDoc.document_element());
canonicalizeDirectionSpellings(expectedDoc);
+ narrowOctaveShiftSizes(expectedDoc.document_element());
if (!expectedDoc.save_file(expectedPath.c_str()))
std::cerr << "dump: failed to write " << expectedPath << "\n";
diff --git a/src/private/mxtest/api/OttavaSizeApiTest.cpp b/src/private/mxtest/api/OttavaSizeApiTest.cpp
index 0d57cd6f8..8bbc364bc 100644
--- a/src/private/mxtest/api/OttavaSizeApiTest.cpp
+++ b/src/private/mxtest/api/OttavaSizeApiTest.cpp
@@ -343,4 +343,101 @@ TEST(OttavaStopSizeContradictionIsNormalizedToTheStart, OttavaSize)
T_END;
+// MusicXML's schema allows any positive integer in octave-shift/@size, so a file can ask for a
+// line music notation has no name for. The api narrows it to the closest line it can draw, which
+// is what lets everything downstream assume an ottava is one of the six real ones. A size between
+// the 8th and the 15th takes the 15th.
+TEST(OttavaSizeBetweenTheLinesIsNarrowedToTheFifteenth, OttavaSize)
+{
+ const std::string sourceXml =
+ R"()"
+ R"()"
+ R"(P)"
+ R"(1)"
+ R"()"
+ R"(C41quarter)"
+ R"()"
+ R"()";
+
+ const auto score = mxtest::fromXml(sourceXml);
+ REQUIRE(score.parts.size() == 1);
+ const auto &directions = score.parts.front().measures.front().staves.front().directions;
+ REQUIRE(directions.size() == 2);
+ const auto &startTypes = directions.front().directionTypes;
+ REQUIRE(startTypes.size() == 1);
+ REQUIRE(startTypes.front().isOttavaStart());
+ CHECK(startTypes.front().ottavaStart().ottavaType == OttavaType::o15ma);
+
+ const auto xml = mxtest::toXml(score);
+ REQUIRE(!xml.empty());
+ const auto shifts = ottavaSizeWrittenShifts(xml);
+ REQUIRE(shifts.size() == 2);
+ CHECK_EQUAL(std::string{"15"}, shifts.front().size);
+ CHECK_EQUAL(std::string{"15"}, shifts.back().size);
+}
+
+T_END;
+
+// A size past the 22nd asks for more than the widest line we can draw, so it takes that widest
+// line. Falling back to the 15th would move the reader further from what the source asked for.
+TEST(OttavaSizePastTheTwentySecondIsNarrowedToTheTwentySecond, OttavaSize)
+{
+ const std::string sourceXml =
+ R"()"
+ R"()"
+ R"(P)"
+ R"(1)"
+ R"()"
+ R"(C41quarter)"
+ R"()"
+ R"()";
+
+ const auto score = mxtest::fromXml(sourceXml);
+ REQUIRE(score.parts.size() == 1);
+ const auto &directions = score.parts.front().measures.front().staves.front().directions;
+ REQUIRE(directions.size() == 2);
+ const auto &startTypes = directions.front().directionTypes;
+ REQUIRE(startTypes.size() == 1);
+ REQUIRE(startTypes.front().isOttavaStart());
+ CHECK(startTypes.front().ottavaStart().ottavaType == OttavaType::o22ma);
+
+ const auto xml = mxtest::toXml(score);
+ REQUIRE(!xml.empty());
+ const auto shifts = ottavaSizeWrittenShifts(xml);
+ REQUIRE(shifts.size() == 2);
+ CHECK_EQUAL(std::string{"22"}, shifts.front().size);
+ CHECK_EQUAL(std::string{"22"}, shifts.back().size);
+}
+
+T_END;
+
+// A size below 8 asks for less than an octave, which an ottava cannot draw either. It becomes a
+// plain octave line, and the size the source spelled out stays spelled out.
+TEST(OttavaSizeBelowEightIsNarrowedToAnOctave, OttavaSize)
+{
+ const std::string sourceXml =
+ R"()"
+ R"()"
+ R"(P)"
+ R"(1)"
+ R"()"
+ R"(C41quarter)"
+ R"()";
+
+ const auto score = mxtest::fromXml(sourceXml);
+ REQUIRE(score.parts.size() == 1);
+ const auto &startTypes = score.parts.front().measures.front().staves.front().directions.front().directionTypes;
+ REQUIRE(startTypes.size() == 1);
+ REQUIRE(startTypes.front().isOttavaStart());
+ CHECK(startTypes.front().ottavaStart().ottavaType == OttavaType::o8vb);
+
+ const auto xml = mxtest::toXml(score);
+ REQUIRE(!xml.empty());
+ const auto shifts = ottavaSizeWrittenShifts(xml);
+ REQUIRE(shifts.size() == 1);
+ CHECK_EQUAL(std::string{"8"}, shifts.front().size);
+}
+
+T_END;
+
#endif
diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt
index eb3347da1..dd8cc51eb 100644
--- a/src/private/mxtest/api/roundtrip-baseline.txt
+++ b/src/private/mxtest/api/roundtrip-baseline.txt
@@ -716,3 +716,12 @@ lysuite/ly33b_Spanners_Tie.xml
# was written under the wrong name.
musuite/testChordNoVoice.xml
musuite/testStringVoiceName.xml
+
+# Unblocked in the harness rather than in mx::api. Each of these sources states an
+# octave-shift size that no ottava line can mean -- 27, 11, 1 -- which MusicXML's
+# schema allows and music notation does not. mx::api narrows the size to a line it
+# can draw, which is the behavior we want, so the harness narrows the expected
+# document the same way instead of the api learning to echo the original number.
+lysuite/ly33e_Spanners_OctaveShifts_InvalidSize.xml
+synthetic/octave-shift.3.0.xml
+synthetic/octave-shift.3.1.xml