From 6544febaef28b7fc1931add1911a4b3ef6d80dad Mon Sep 17 00:00:00 2001 From: Matthew James Briggs Date: Sat, 19 Sep 2026 12:13:48 +0200 Subject: [PATCH] fix: read the valign of a credit-image --- src/include/mx/api/PageImageData.h | 6 +- src/private/mx/impl/DirectionReader.cpp | 26 +----- src/private/mx/impl/PageTextFunctions.cpp | 6 +- src/private/mx/impl/PositionFunctions.h | 25 ++++++ .../mxtest/api/CreditRoundTripTest.cpp | 80 ++++++++++++++++++- src/private/mxtest/api/roundtrip-baseline.txt | 7 ++ 6 files changed, 116 insertions(+), 34 deletions(-) diff --git a/src/include/mx/api/PageImageData.h b/src/include/mx/api/PageImageData.h index dcad1523e..0e28caaab 100644 --- a/src/include/mx/api/PageImageData.h +++ b/src/include/mx/api/PageImageData.h @@ -39,9 +39,9 @@ class PageImageData /// attribute). Values <= 0 mean unspecified. int pageNumber; - /// default-x/default-y/relative-x/relative-y and horizontal alignment. - /// (Vertical alignment uses a credit-image specific type and is not - /// modeled here.) + /// default-x/default-y/relative-x/relative-y and alignment. Vertical + /// alignment for an image is top, middle or bottom; there is no + /// baseline. PositionData positionData; PageImageData() diff --git a/src/private/mx/impl/DirectionReader.cpp b/src/private/mx/impl/DirectionReader.cpp index c79a07900..035fcfc2e 100644 --- a/src/private/mx/impl/DirectionReader.cpp +++ b/src/private/mx/impl/DirectionReader.cpp @@ -80,7 +80,6 @@ #include "mx/core/generated/Timpani.h" #include "mx/core/generated/TuningGroup.h" #include "mx/core/generated/UpDownStopContinue.h" -#include "mx/core/generated/ValignImage.h" #include "mx/core/generated/Wedge.h" #include "mx/core/generated/WedgeType.h" #include "mx/core/generated/YesNo.h" @@ -975,29 +974,8 @@ void DirectionReader::parseImage(const core::DirectionType &directionType) outImage.width = static_cast(image.width()->value().value()); } outImage.positionData = getPositionData(image); - // 's valign is the valign-image type (no baseline), which the generic position - // helper cannot read; take it from the element directly. - if (image.valign().has_value()) - { - switch (image.valign()->tag()) - { - case core::ValignImage::Tag::top: - outImage.positionData.verticalAlignment = api::VerticalAlignment::top; - break; - case core::ValignImage::Tag::middle: - outImage.positionData.verticalAlignment = api::VerticalAlignment::middle; - break; - case core::ValignImage::Tag::bottom: - outImage.positionData.verticalAlignment = api::VerticalAlignment::bottom; - break; - default: - break; - } - } - else - { - outImage.positionData.verticalAlignment = api::VerticalAlignment::unspecified; - } + // reads valign-image, not valign; see PositionFunctions.h. + outImage.positionData.verticalAlignment = getImageValign(image); outImage.id = getId(image); myOutDirectionData.directionTypes.emplace_back(api::DirectionChoice{std::move(outImage)}); } diff --git a/src/private/mx/impl/PageTextFunctions.cpp b/src/private/mx/impl/PageTextFunctions.cpp index 80448a01a..2d504ac8e 100644 --- a/src/private/mx/impl/PageTextFunctions.cpp +++ b/src/private/mx/impl/PageTextFunctions.cpp @@ -41,10 +41,8 @@ api::PageImageData getImageData(const core::Image &image, int pageNumber) } out.positionData = getPositionData(image); - // The valign uses the credit-image-specific - // ValignImage vocabulary, which PositionData does not model. Avoid - // recording a misleading vertical-alignment value. - out.positionData.verticalAlignment = api::VerticalAlignment::unspecified; + // reads valign-image, not valign; see PositionFunctions.h. + out.positionData.verticalAlignment = getImageValign(image); return out; } diff --git a/src/private/mx/impl/PositionFunctions.h b/src/private/mx/impl/PositionFunctions.h index 1a936c5b3..4c7dcf468 100644 --- a/src/private/mx/impl/PositionFunctions.h +++ b/src/private/mx/impl/PositionFunctions.h @@ -226,5 +226,30 @@ void setImageValignFromVerticalAlignment(api::VerticalAlignment verticalAlignmen break; } } + +// and read vertical alignment as valign-image (top, middle, bottom -- +// no baseline), not valign, which getPositionData above cannot see: the value getter for valign +// is compiled out for a ValignImage field, so a present attribute comes back as the default +// baseline. Read it from the element with this instead. An absent attribute reads as +// unspecified. +template api::VerticalAlignment getImageValign(const ATTRIBUTES_TYPE &inAttributes) +{ + if (!checkHasValign(&inAttributes)) + { + return api::VerticalAlignment::unspecified; + } + + switch (inAttributes.valign()->tag()) + { + case core::ValignImage::Tag::top: + return api::VerticalAlignment::top; + case core::ValignImage::Tag::middle: + return api::VerticalAlignment::middle; + case core::ValignImage::Tag::bottom: + return api::VerticalAlignment::bottom; + } + + return api::VerticalAlignment::unspecified; +} } // namespace impl } // namespace mx diff --git a/src/private/mxtest/api/CreditRoundTripTest.cpp b/src/private/mxtest/api/CreditRoundTripTest.cpp index 050ef37ec..26e06b936 100644 --- a/src/private/mxtest/api/CreditRoundTripTest.cpp +++ b/src/private/mxtest/api/CreditRoundTripTest.cpp @@ -114,9 +114,8 @@ TEST(creditRoundTrip, creditImage) TEST(creditRoundTrip, imageValignMiddleWritesMiddle) { - // The reader always reports credit-image valign as unspecified (it has no - // vertical-alignment vocabulary for valign-image), so a round trip would not show a - // writer bug here; check the written XML instead (#444). + // A middle alignment must be written as valign-image's middle, not left at the + // natural-zero top (#444). auto in = makeMinimalScore(); PageImageData img{}; img.source = "logo.png"; @@ -144,6 +143,81 @@ TEST(creditRoundTrip, imageValignBaselineOmitsAttribute) CHECK(xml.find("valign=") == std::string::npos); } +TEST(creditRoundTrip, imageValignSurvives) +{ + // The reader dropped 's valign attribute, so every alignment came back + // unspecified (#454). + for (const auto alignment : {VerticalAlignment::top, VerticalAlignment::middle, VerticalAlignment::bottom}) + { + auto in = makeMinimalScore(); + PageImageData img{}; + img.source = "logo.png"; + img.type = "image/png"; + img.positionData.verticalAlignment = alignment; + in.pageImageItems.push_back(img); + + const auto out = mxtest::roundTrip(in); + + REQUIRE(out.pageImageItems.size() == 1); + CHECK(alignment == out.pageImageItems.at(0).positionData.verticalAlignment); + } +} + +TEST(creditRoundTrip, imageValignAbsentStaysUnspecified) +{ + auto in = makeMinimalScore(); + PageImageData img{}; + img.source = "logo.png"; + img.type = "image/png"; + in.pageImageItems.push_back(img); + + const auto xml = mxtest::toXml(in); + CHECK(xml.find("valign=") == std::string::npos); + + const auto out = mxtest::roundTrip(in); + + REQUIRE(out.pageImageItems.size() == 1); + CHECK(VerticalAlignment::unspecified == out.pageImageItems.at(0).positionData.verticalAlignment); +} + +TEST(creditRoundTrip, imageValignIsReadFromXml) +{ + const std::string xml = R"( + + + + + + Flute + + + + + + 1 + + + + C + 5 + + 1 + 1 + quarter + + + + +)"; + + const auto score = mxtest::fromXml(xml); + + REQUIRE(score.pageImageItems.size() == 1); + const auto &img = score.pageImageItems.at(0); + CHECK_EQUAL("logo.png", img.source); + CHECK(VerticalAlignment::middle == img.positionData.verticalAlignment); +} + TEST(creditRoundTrip, justifySurvives) { auto in = makeMinimalScore(); diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index 138128359..7a25c0e60 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -742,3 +742,10 @@ musuite/testStringVoiceName.xml lysuite/ly33e_Spanners_OctaveShifts_InvalidSize.xml synthetic/octave-shift.3.0.xml synthetic/octave-shift.3.1.xml + +# Unblocked by #454 (credit-image valign round-trip). The reader read the +# valign attribute and then discarded it, so the writer had +# nothing to emit and the attribute was lost. getImageData now reads it -- top, +# middle or bottom, the valign-image vocabulary. This fixture's valign="top" was +# its only divergence. +synthetic/credit-image.3.0.xml