Trade: allow array attributes to have implementation-specific formats.

I'm not sure why this restriction was there as nothing was preventing
them from being used. The attribute is only accessible through the
typeless attribute(), which gives back
`Containers::StridedArrayView2D<const char>` with second dimension size
being set to the full stride. And there it doesn't matter if the format
is an array or not.

This will be useful for joint IDs and weights, for example doing crazy
things like packing the IDs into an array of 8 4-bit numbers, saving
half the memory compared to the smallest builtin representation using
UnsignedByte[8].

doc/changelog.dox

@@ -627,6 +627,9 @@ See also:
     attribute strides for better data layout flexibility. This is however not
     commonly supported by GPU APIs and tools like @ref MeshTools::compile()
     assert in that case.
+-   @ref Trade::MeshData now allows array attributes to have
+    implementation-specific vertex formats as well. The restriction didn't make
+    sense, as there was nothing in the design preventing them from being used.
 -   Added @ref Trade::MeshData::findAttributeId() for an ability to check that
     an attribute exists and retrieve its ID in a single step, avoiding a double
     lookup compared to @relativeref{Trade::MeshData,hasAttribute()} +

src/Magnum/Trade/MeshData.h

@@ -2287,8 +2287,7 @@ namespace Implementation {
 }
 
 constexpr MeshAttributeData::MeshAttributeData(std::nullptr_t, const MeshAttribute name, const VertexFormat format, const Containers::StridedArrayView1D<const void>& data, const UnsignedShort arraySize) noexcept:
-    _format{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format),
-        "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), format)},
+    _format{format},
     _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format),
         "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name)},
     _isOffsetOnly{false}, _vertexCount{UnsignedInt(data.size())},
@@ -2300,8 +2299,7 @@ constexpr MeshAttributeData::MeshAttributeData(std::nullptr_t, const MeshAttribu
     _data{data.data()} {}
 
 constexpr MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexFormat format, const std::size_t offset, const UnsignedInt vertexCount, const std::ptrdiff_t stride, UnsignedShort arraySize) noexcept:
-    _format{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format),
-        "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), format)},
+    _format{format},
     _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format),
         "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name)},
     _isOffsetOnly{true}, _vertexCount{vertexCount},

src/Magnum/Trade/Test/MeshDataTest.cpp

@@ -85,6 +85,7 @@ struct MeshDataTest: TestSuite::Tester {
     void constructArrayAttributeTypeErased();
     void constructArrayAttributeNullptr();
     void constructArrayAttributeOffsetOnly();
+    void constructArrayAttributeImplementationSpecificFormat();
     void constructArrayAttributeNotAllowed();
 
     void construct();
@@ -258,6 +259,7 @@ MeshDataTest::MeshDataTest() {
               &MeshDataTest::constructArrayAttributeTypeErased,
               &MeshDataTest::constructArrayAttributeNullptr,
               &MeshDataTest::constructArrayAttributeOffsetOnly,
+              &MeshDataTest::constructArrayAttributeImplementationSpecificFormat,
               &MeshDataTest::constructArrayAttributeNotAllowed});
 
     addInstancedTests({&MeshDataTest::construct},
@@ -1129,6 +1131,19 @@ void MeshDataTest::constructArrayAttributeOffsetOnly() {
     CORRADE_COMPARE(cdata.arraySize(), 4);
 }
 
+void MeshDataTest::constructArrayAttributeImplementationSpecificFormat() {
+    Vector2 positions[]{{1.0f, 0.3f}, {0.5f, 0.7f}};
+
+    /* This should not fire any asserts */
+    MeshAttributeData a{meshAttributeCustom(35), vertexFormatWrap(0x3a), positions, 2};
+    CORRADE_COMPARE(a.name(), meshAttributeCustom(35));
+    CORRADE_COMPARE(a.format(), vertexFormatWrap(0x3a));
+    CORRADE_COMPARE(a.arraySize(), 2);
+    CORRADE_COMPARE_AS(Containers::arrayCast<const Vector2>(a.data()),
+        Containers::arrayView<Vector2>({{1.0f, 0.3f}, {0.5f, 0.7f}}),
+        TestSuite::Compare::Container);
+}
+
 void MeshDataTest::constructArrayAttributeNotAllowed() {
     CORRADE_SKIP_IF_NO_ASSERT();
 
@@ -1144,23 +1159,21 @@ void MeshDataTest::constructArrayAttributeNotAllowed() {
     MeshAttributeData{meshAttributeCustom(35), positions2D};
     MeshAttributeData{meshAttributeCustom(35), VertexFormat::Vector2, positions2Dchar, 3};
     MeshAttributeData{meshAttributeCustom(35), VertexFormat::Vector2, 0, 3, 6*sizeof(Vector2), 3};
+    MeshAttributeData{meshAttributeCustom(35), vertexFormatWrap(0xdead), positions, 3};
+    MeshAttributeData{meshAttributeCustom(35), vertexFormatWrap(0xdead), 0, 3, 6*sizeof(Vector2), 3};
 
     /* This is not */
     std::ostringstream out;
     Error redirectError{&out};
     MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2b, positions, 3};
-    MeshAttributeData{meshAttributeCustom(35), vertexFormatWrap(0xdead), positions, 3};
     MeshAttributeData{MeshAttribute::Position, positions2D};
     MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2, positions2Dchar, 3};
     MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2, 0, 3, 6*sizeof(Vector2), 3};
-    MeshAttributeData{meshAttributeCustom(35), vertexFormatWrap(0xdead), 0, 3, 6*sizeof(Vector2), 3};
     CORRADE_COMPARE(out.str(),
-        "Trade::MeshAttributeData: Trade::MeshAttribute::Position can't be an array attribute\n"
-        "Trade::MeshAttributeData: array attributes can't have an implementation-specific format\n"
         "Trade::MeshAttributeData: Trade::MeshAttribute::Position can't be an array attribute\n"
         "Trade::MeshAttributeData: Trade::MeshAttribute::Position can't be an array attribute\n"
         "Trade::MeshAttributeData: Trade::MeshAttribute::Position can't be an array attribute\n"
-        "Trade::MeshAttributeData: array attributes can't have an implementation-specific format\n");
+        "Trade::MeshAttributeData: Trade::MeshAttribute::Position can't be an array attribute\n");
 }
 
 void MeshDataTest::construct() {
@@ -1895,14 +1908,17 @@ void MeshDataTest::constructImplementationSpecificVertexFormat() {
         MeshAttributeData{MeshAttribute::TextureCoordinates,
             vertexFormatWrap(0xdead3), attribute},
         MeshAttributeData{MeshAttribute::Color,
-            vertexFormatWrap(0xdead4), attribute}}};
+            vertexFormatWrap(0xdead4), attribute},
+        MeshAttributeData{meshAttributeCustom(35),
+            vertexFormatWrap(0xdead5), attribute, 27}}};
 
     /* Getting typeless attribute should work also */
     UnsignedInt format = 0xdead1;
     for(MeshAttribute name: {MeshAttribute::Position,
                              MeshAttribute::Normal,
                              MeshAttribute::TextureCoordinates,
-                             MeshAttribute::Color}) {
+                             MeshAttribute::Color,
+                             meshAttributeCustom(35)}) {
         CORRADE_ITERATION(name);
         CORRADE_COMPARE(data.attributeFormat(name), vertexFormatWrap(format++));