fix: read the valign of a credit-image

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()

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<double>(image.width()->value().value());
     }
     outImage.positionData = getPositionData(image);
-    // <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;
-    }
+    // <image> 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)});
 }

src/private/mx/impl/PageTextFunctions.cpp

@@ -41,10 +41,8 @@ api::PageImageData getImageData(const core::Image &image, int pageNumber)
     }
 
     out.positionData = getPositionData(image);
-    // The <credit-image> 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;
+    // <credit-image> reads valign-image, not valign; see PositionFunctions.h.
+    out.positionData.verticalAlignment = getImageValign(image);
     return out;
 }
 

src/private/mx/impl/PositionFunctions.h

@@ -226,5 +226,30 @@ void setImageValignFromVerticalAlignment(api::VerticalAlignment verticalAlignmen
         break;
     }
 }
+
+// <image> and <credit-image> 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 <typename ATTRIBUTES_TYPE> api::VerticalAlignment getImageValign(const ATTRIBUTES_TYPE &inAttributes)
+{
+    if (!checkHasValign<ATTRIBUTES_TYPE>(&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

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 <credit-image>'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"(<score-partwise version="3.0">
+  <credit>
+    <credit-image source="logo.png" type="image/png" valign="middle" />
+  </credit>
+  <part-list>
+    <score-part id="P1">
+      <part-name>Flute</part-name>
+    </score-part>
+  </part-list>
+  <part id="P1">
+    <measure number="1">
+      <attributes>
+        <divisions>1</divisions>
+      </attributes>
+      <note>
+        <pitch>
+          <step>C</step>
+          <octave>5</octave>
+        </pitch>
+        <duration>1</duration>
+        <voice>1</voice>
+        <type>quarter</type>
+      </note>
+    </measure>
+  </part>
+</score-partwise>
+)";
+
+    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();

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
+# <credit-image> 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