diff --git a/AGENTS.md b/AGENTS.md index fa04cefd..c97defa6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -122,3 +122,28 @@ locally. | `src/private/mx/core/generated/Document.h` | The core document model (parse, serialize, Document class) | | `src/private/mxtest/corert/CoreRoundtripImpl.cpp` | The corert test runner (discover, parse, serialize, compare) | | `src/private/mxtest/corert/Compare.cpp` | DOM normalization and comparison engine used by corert and api tests | + +## Git Authorship + +The git commit Author and Committer fields are for the user's information, not yours. No Co-Authored-By either. + +Correct: + +```text +Author: The User +Date: Sat Jun 20 10:31:06 2026 +0000 + + Blah +``` + +Incorrect: + +```text +Author: Claude +Date: Sat Jun 20 10:31:06 2026 +0000 + + Blah + + Co-Authored-By: Claude Opus 4.8 (1M context) + Claude-Session: https://claude.ai/code/session_01S42a2LXrZb5GUk9cY7Hdru +``` diff --git a/src/include/mx/api/RehearsalData.h b/src/include/mx/api/RehearsalData.h index eece7090..e45f9016 100644 --- a/src/include/mx/api/RehearsalData.h +++ b/src/include/mx/api/RehearsalData.h @@ -14,6 +14,7 @@ namespace api { enum class RehearsalEnclosure { + unspecified, rectangle, square, oval, @@ -36,7 +37,7 @@ class RehearsalData RehearsalData() : text{}, positionData{}, isColorSpecified{false}, colorData{}, fontData{}, - enclosure{RehearsalEnclosure::rectangle} + enclosure{RehearsalEnclosure::unspecified} { } }; diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index 7f4a41cd..4241b4ef 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -314,7 +314,12 @@ void DirectionReader::parseRehearsal(const core::DirectionType &directionType) api::RehearsalData outRehearsal; outRehearsal.text = rehearsal.value(); outRehearsal.positionData = getPositionData(rehearsal); - outRehearsal.colorData = getColor(rehearsal); + outRehearsal.fontData = getFontData(rehearsal); + outRehearsal.isColorSpecified = rehearsal.color().has_value(); + if (outRehearsal.isColorSpecified) + { + outRehearsal.colorData = getColor(rehearsal); + } if (rehearsal.enclosure().has_value()) { switch (rehearsal.enclosure()->tag()) diff --git a/src/private/mx/impl/DirectionWriter.cpp b/src/private/mx/impl/DirectionWriter.cpp index 12124bf9..d4b04b2f 100644 --- a/src/private/mx/impl/DirectionWriter.cpp +++ b/src/private/mx/impl/DirectionWriter.cpp @@ -449,6 +449,51 @@ std::vector DirectionWriter::getDirectionLikeThings() addDirectionType(std::move(dt), direction); } + for (const auto &item : myDirectionData.rehearsals) + { + core::FormattedTextID rehearsal{}; + rehearsal.setValue(item.text); + setAttributesFromPositionData(item.positionData, rehearsal); + setAttributesFromFontData(item.fontData, rehearsal); + if (item.isColorSpecified) + { + setAttributesFromColorData(item.colorData, rehearsal); + } + switch (item.enclosure) + { + case api::RehearsalEnclosure::unspecified: + break; + case api::RehearsalEnclosure::rectangle: + rehearsal.setEnclosure(core::EnclosureShape::rectangle()); + break; + case api::RehearsalEnclosure::square: + rehearsal.setEnclosure(core::EnclosureShape::square()); + break; + case api::RehearsalEnclosure::oval: + rehearsal.setEnclosure(core::EnclosureShape::oval()); + break; + case api::RehearsalEnclosure::circle: + rehearsal.setEnclosure(core::EnclosureShape::circle()); + break; + case api::RehearsalEnclosure::bracket: + rehearsal.setEnclosure(core::EnclosureShape::bracket()); + break; + case api::RehearsalEnclosure::triangle: + rehearsal.setEnclosure(core::EnclosureShape::triangle()); + break; + case api::RehearsalEnclosure::diamond: + rehearsal.setEnclosure(core::EnclosureShape::diamond()); + break; + case api::RehearsalEnclosure::none: + rehearsal.setEnclosure(core::EnclosureShape::none()); + break; + } + core::DirectionType dt{}; + dt.setChoice( + core::DirectionTypeChoice::rehearsal(core::OneOrMore{std::move(rehearsal)})); + addDirectionType(std::move(dt), direction); + } + if (myIsFirstDirectionTypeAdded) { // The direction has other content; attach the as a child of the . diff --git a/src/private/mxtest/api/DirectionDataTest.cpp b/src/private/mxtest/api/DirectionDataTest.cpp index 404049c6..a901c117 100644 --- a/src/private/mxtest/api/DirectionDataTest.cpp +++ b/src/private/mxtest/api/DirectionDataTest.cpp @@ -7,9 +7,11 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/DocumentManager.h" +#include "mx/api/RehearsalData.h" #include "mx/api/ScoreData.h" #include "mxtest/api/RoundTrip.h" #include "mxtest/api/TestHelpers.h" +#include "mxtest/file/Path.h" using namespace std; using namespace mx::api; @@ -194,4 +196,122 @@ TEST(OutOfOrderTorture, DirectionData) T_END; +// Parse the synthetic rehearsal file and confirm that mx::api reads the text and enclosure. +// This pins the core -> api read path and would fail if parseRehearsal regresses. +TEST(RehearsalSyntheticFileRead, DirectionData) +{ + const std::string path = mxtest::getResourcesDirectoryPath() + "synthetic/rehearsal.3.1.xml"; + auto &docMgr = DocumentManager::getInstance(); + const auto docIdResult = docMgr.createFromFile(path); + REQUIRE(docIdResult.ok()); + const int docId = docIdResult.value(); + const auto scoreResult = docMgr.getData(docId); + docMgr.destroyDocument(docId); + REQUIRE(scoreResult.ok()); + const auto &score = scoreResult.value(); + REQUIRE(score.parts.size() == 1); + REQUIRE(score.parts.front().measures.size() == 1); + REQUIRE(score.parts.front().measures.front().staves.size() == 1); + const auto &directions = score.parts.front().measures.front().staves.front().directions; + REQUIRE(directions.size() == 1); + REQUIRE(directions.front().rehearsals.size() == 1); + const auto &rehearsal = directions.front().rehearsals.front(); + CHECK_EQUAL("x", rehearsal.text); + CHECK(RehearsalEnclosure::rectangle == rehearsal.enclosure); +} + +T_END; + +// Verify that rehearsal marks survive a full MusicXML serialization/deserialization round trip. +// This catches the bug where DirectionWriter had no rehearsal write path and rehearsals were +// silently dropped on output. +TEST(RehearsalRoundTripXml, DirectionData) +{ + ScoreData oscore; + oscore.ticksPerQuarter = 10; + oscore.parts.emplace_back(); + auto &opart = oscore.parts.back(); + opart.measures.emplace_back(); + auto &omeasure = opart.measures.back(); + omeasure.staves.emplace_back(); + auto &ostaff = omeasure.staves.back(); + auto &ovoice = ostaff.voices[0]; + + NoteData onote{}; + onote.tickTimePosition = 0; + onote.durationData.durationTimeTicks = 10; + onote.durationData.durationName = DurationName::quarter; + ovoice.notes.push_back(onote); + + RehearsalData rehearsal; + rehearsal.text = "B"; + rehearsal.enclosure = RehearsalEnclosure::rectangle; + rehearsal.fontData.fontFamily = {"Times New Roman"}; + rehearsal.fontData.style = FontStyle::normal; + rehearsal.fontData.weight = FontWeight::bold; + rehearsal.fontData.sizeType = FontSizeType::point; + rehearsal.fontData.sizePoint = 12.0; + rehearsal.positionData.isDefaultXSpecified = true; + rehearsal.positionData.defaultX = 5.0; + + DirectionData directionData; + directionData.tickTimePosition = 0; + directionData.rehearsals.push_back(rehearsal); + ostaff.directions.push_back(directionData); + + const auto rscore = mxtest::roundTrip(oscore); + REQUIRE(rscore.parts.size() == 1); + REQUIRE(rscore.parts.front().measures.size() == 1); + REQUIRE(rscore.parts.front().measures.front().staves.size() == 1); + const auto &rdirections = rscore.parts.front().measures.front().staves.front().directions; + REQUIRE(rdirections.size() == 1); + REQUIRE(rdirections.front().rehearsals.size() == 1); + CHECK_EQUAL("B", rdirections.front().rehearsals.front().text); + CHECK(RehearsalEnclosure::rectangle == rdirections.front().rehearsals.front().enclosure); + CHECK(FontWeight::bold == rdirections.front().rehearsals.front().fontData.weight); +} + +T_END; + +// Verify that a rehearsal with no enclosure set (RehearsalEnclosure::unspecified) does not emit +// an enclosure attribute in the serialized XML, and that the field round-trips as unspecified. +TEST(RehearsalUnspecifiedEnclosureNoPhantomAttribute, DirectionData) +{ + ScoreData oscore; + oscore.ticksPerQuarter = 10; + oscore.parts.emplace_back(); + auto &opart = oscore.parts.back(); + opart.measures.emplace_back(); + auto &omeasure = opart.measures.back(); + omeasure.staves.emplace_back(); + auto &ostaff = omeasure.staves.back(); + auto &ovoice = ostaff.voices[0]; + + NoteData onote{}; + onote.tickTimePosition = 0; + onote.durationData.durationTimeTicks = 10; + onote.durationData.durationName = DurationName::quarter; + ovoice.notes.push_back(onote); + + RehearsalData rehearsal; + rehearsal.text = "C"; + // enclosure left at default (unspecified) — must not appear in XML or round-trip as rectangle + + DirectionData directionData; + directionData.tickTimePosition = 0; + directionData.rehearsals.push_back(rehearsal); + ostaff.directions.push_back(directionData); + + const auto xml = mxtest::toXml(oscore); + CHECK(xml.find("enclosure") == std::string::npos); + + const auto rscore = mxtest::roundTrip(oscore); + REQUIRE(rscore.parts.front().measures.front().staves.front().directions.size() == 1); + REQUIRE(rscore.parts.front().measures.front().staves.front().directions.front().rehearsals.size() == 1); + CHECK(RehearsalEnclosure::unspecified == + rscore.parts.front().measures.front().staves.front().directions.front().rehearsals.front().enclosure); +} + +T_END; + #endif diff --git a/src/private/mxtest/impl/DirectionWriterTest.cpp b/src/private/mxtest/impl/DirectionWriterTest.cpp index 0f73d91d..df9cea65 100644 --- a/src/private/mxtest/impl/DirectionWriterTest.cpp +++ b/src/private/mxtest/impl/DirectionWriterTest.cpp @@ -7,6 +7,7 @@ #include "cpul/cpulTestHarness.h" #include "mx/api/OttavaData.h" +#include "mx/api/RehearsalData.h" #include "mx/core/generated/Direction.h" #include "mx/core/generated/DirectionType.h" #include "mx/core/generated/MusicDataChoice.h" @@ -117,4 +118,53 @@ TEST(segnoAndCodaRoundTrip, DirectionWriter) T_END +// Build a RehearsalData carrying the full attribute set (text, position, color, font, enclosure), +// write it with DirectionWriter, read it back with DirectionReader, and confirm every field +// survives the api -> core -> api round trip. +TEST(rehearsalRoundTrip, DirectionWriter) +{ + api::RehearsalData rehearsal; + rehearsal.text = "A"; + rehearsal.positionData.isDefaultXSpecified = true; + rehearsal.positionData.defaultX = 10.0; + rehearsal.positionData.isDefaultYSpecified = true; + rehearsal.positionData.defaultY = 20.0; + rehearsal.positionData.isRelativeXSpecified = true; + rehearsal.positionData.relativeX = 3.0; + rehearsal.positionData.isRelativeYSpecified = true; + rehearsal.positionData.relativeY = 4.0; + rehearsal.positionData.horizontalAlignmnet = api::HorizontalAlignment::left; + rehearsal.positionData.verticalAlignment = api::VerticalAlignment::top; + rehearsal.fontData.fontFamily = {"Maestro"}; + rehearsal.fontData.style = api::FontStyle::italic; + rehearsal.fontData.weight = api::FontWeight::bold; + rehearsal.fontData.sizeType = api::FontSizeType::point; + rehearsal.fontData.sizePoint = 14.0; + rehearsal.isColorSpecified = true; + rehearsal.colorData.red = 255; + rehearsal.colorData.green = 0; + rehearsal.colorData.blue = 0; + rehearsal.colorData.isAlphaSpecified = true; + rehearsal.colorData.alpha = 255; + rehearsal.enclosure = api::RehearsalEnclosure::square; + + api::DirectionData directionData; + directionData.rehearsals.push_back(rehearsal); + + Cursor cursor{1, 100}; + DirectionWriter writer{directionData, cursor}; + const auto mdcSet = writer.getDirectionLikeThings(); + REQUIRE(mdcSet.size() >= 1); + CHECK(mdcSet.front().isDirection()); + const auto &direction = mdcSet.front().asDirection(); + + DirectionReader reader{direction, cursor}; + const auto roundTripped = reader.getDirectionData(); + + REQUIRE(roundTripped.rehearsals.size() == 1); + CHECK(rehearsal == roundTripped.rehearsals.front()); +} + +T_END + #endif