From 05b74816e4e9ce92fec9c416caf696fe09215d68 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Mon, 17 Feb 2025 12:28:04 +0100
Subject: [PATCH] Trade: avoid ambiguity when creating MeshAttributeData from a
 char view.

The ambiguity with StridedArrayView1D<const char> was there always, I
just didn't hit that anywhere so far. With the recent changes in
Corrade, where StridedArrayView2D<const T> is constructible from
StridedArrayView1D<T> as well in addition to const T, the ambiguity gets
hit by a test. So test both variants and add an overload that resolves
those.

There's no other such case in either MeshIndexData or SceneFieldData, as
the StridedArrayView1D<const void> and StridedArrayView2D<const char>
constructor variants always have differing arguments. Neither it happens
in case of (ARM) platforms where char is unsigned.
---
 src/Magnum/Trade/MeshData.h            |  9 ++++++++
 src/Magnum/Trade/Test/MeshDataTest.cpp | 31 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h
index 3e8b94d17..72a2f8f3d 100644
--- a/src/Magnum/Trade/MeshData.h
+++ b/src/Magnum/Trade/MeshData.h
@@ -483,6 +483,15 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData {
          * by GPU APIs.
          */
         explicit MeshAttributeData(MeshAttribute name, VertexFormat format, const Containers::StridedArrayView1D<const void>& data, UnsignedShort arraySize = 0, Int morphTargetId = -1) noexcept;
+        #ifndef DOXYGEN_GENERATING_OUTPUT
+        /* These two are here to direct StridedArrayView1D<[const ]char> to the
+           1D overload, because that's likely what one wants. The 2D conversion
+           adds a dimension at the front, which would make the attribute have
+           just a single vertex with likely a sparse second dimension, causing
+           an immediate assert. */
+        explicit MeshAttributeData(MeshAttribute name, VertexFormat format, const Containers::StridedArrayView1D<char>& data, UnsignedShort arraySize = 0, Int morphTargetId = -1) noexcept: MeshAttributeData{name, format, Containers::StridedArrayView1D<const void>{data}, arraySize, morphTargetId} {}
+        explicit MeshAttributeData(MeshAttribute name, VertexFormat format, const Containers::StridedArrayView1D<const char>& data, UnsignedShort arraySize = 0, Int morphTargetId = -1) noexcept: MeshAttributeData{name, format, Containers::StridedArrayView1D<const void>{data}, arraySize, morphTargetId} {}
+        #endif
 
         /**
          * @brief Constructor
diff --git a/src/Magnum/Trade/Test/MeshDataTest.cpp b/src/Magnum/Trade/Test/MeshDataTest.cpp
index 63d02a19d..9a1100ab7 100644
--- a/src/Magnum/Trade/Test/MeshDataTest.cpp
+++ b/src/Magnum/Trade/Test/MeshDataTest.cpp
@@ -73,6 +73,7 @@ struct MeshDataTest: TestSuite::Tester {
     void constructAttribute2DNonContiguous();
     void constructAttributeTypeErased();
     void constructAttributeTypeErasedMorphTarget();
+    template<class T> void constructAttributeTypeErasedCharAmbiguity();
     void constructAttributeNullptr();
     void constructAttributeNullptrMorphTarget();
     void constructAttributePadding();
@@ -284,6 +285,8 @@ MeshDataTest::MeshDataTest() {
               &MeshDataTest::constructAttribute2DNonContiguous,
               &MeshDataTest::constructAttributeTypeErased,
               &MeshDataTest::constructAttributeTypeErasedMorphTarget,
+              &MeshDataTest::constructAttributeTypeErasedCharAmbiguity<char>,
+              &MeshDataTest::constructAttributeTypeErasedCharAmbiguity<const char>,
               &MeshDataTest::constructAttributeNullptr,
               &MeshDataTest::constructAttributeNullptrMorphTarget,
               &MeshDataTest::constructAttributePadding,
@@ -1043,6 +1046,34 @@ void MeshDataTest::constructAttributeTypeErasedMorphTarget() {
     CORRADE_VERIFY(positions.data().data() == positionData);
 }
 
+template<class> struct NameTraits;
+template<> struct NameTraits<char> {
+    static const char* name() { return "char"; }
+};
+template<> struct NameTraits<const char> {
+    static const char* name() { return "const char"; }
+};
+
+template<class T> void MeshDataTest::constructAttributeTypeErasedCharAmbiguity() {
+    setTestCaseTemplateName(NameTraits<T>::name());
+
+    /* StridedArrayView1D<[const ]char> is convertible to both
+       StridedArrayView1D<const void> and StridedArrayView2D<const char>,
+       verify the void is preferred. 2D conversion would result in the size
+       being {1, 3} which doesn't make sense. */
+    char data[3]{};
+    Containers::StridedArrayView1D<T> view = data;
+    MeshAttributeData attribute{meshAttributeCustom(15), VertexFormat::Byte, view, 0, 33};
+    CORRADE_VERIFY(!attribute.isOffsetOnly());
+    CORRADE_COMPARE(attribute.arraySize(), 0);
+    CORRADE_COMPARE(attribute.morphTargetId(), 33);
+    CORRADE_COMPARE(attribute.name(), meshAttributeCustom(15));
+    CORRADE_COMPARE(attribute.format(), VertexFormat::Byte);
+    /* If the delegation would be wrong, size would be 1 */
+    CORRADE_COMPARE(attribute.data().size(), 3);
+    CORRADE_VERIFY(attribute.data().data() == data);
+}
+
 void MeshDataTest::constructAttributeNullptr() {
     MeshAttributeData positions{MeshAttribute::Position, VertexFormat::Vector2, nullptr};
     CORRADE_VERIFY(!positions.isOffsetOnly());