Trade: allow array access to all SceneData/MeshData fields/attributes.

In retrospect, the limitation made no sense and only prevented user code
from having a common handling for all possible variants.

src/Magnum/Trade/MeshData.h

@@ -1579,11 +1579,12 @@ class MAGNUM_TRADE_EXPORT MeshData {
         /**
          * @brief Data for given array attribute in a concrete type
          *
-         * Same as above, except that it works with array attributes instead
+         * Same as above, except that it works with array attributes as well
          * --- you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref attributeArraySize() for given attribute.
+         * @ref attributeArraySize() for given attribute. For non-array
+         * attributes the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<const typename std::remove_extent<T>::type> attribute(UnsignedInt id) const;
 
@@ -1599,11 +1600,12 @@ class MAGNUM_TRADE_EXPORT MeshData {
         /**
          * @brief Mutable data for given array attribute in a concrete type
          *
-         * Same as above, except that it works with array attributes instead
+         * Same as above, except that it works with array attributes as well
          * --- you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref attributeArraySize() for given attribute.
+         * @ref attributeArraySize() for given attribute. For non-array
+         * attributes the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<typename std::remove_extent<T>::type> mutableAttribute(UnsignedInt id);
 
@@ -1661,11 +1663,12 @@ class MAGNUM_TRADE_EXPORT MeshData {
         /**
          * @brief Data for given named array attribute in a concrete type
          *
-         * Same as above, except that it works with array attributes instead
+         * Same as above, except that it works with array attributes as well
          * --- you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref attributeArraySize() for given attribute.
+         * @ref attributeArraySize() for given attribute. For non-array
+         * attributes the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<const typename std::remove_extent<T>::type> attribute(MeshAttribute name, UnsignedInt id = 0) const;
 
@@ -1681,11 +1684,12 @@ class MAGNUM_TRADE_EXPORT MeshData {
         /**
          * @brief Mutable data for given named array attribute in a concrete type
          *
-         * Same as above, except that it works with array attributes instead
+         * Same as above, except that it works with array attributes as well
          * --- you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref attributeArraySize() for given attribute.
+         * @ref attributeArraySize() for given attribute. For non-array
+         * attributes the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<typename std::remove_extent<T>::type> mutableAttribute(MeshAttribute name, UnsignedInt id = 0);
 
@@ -2492,10 +2496,8 @@ template<class T> bool MeshData::checkVertexFormatCompatibility(const MeshAttrib
         prefix << "can't cast data from an implementation-specific vertex format" << reinterpret_cast<void*>(vertexFormatUnwrap(attribute._format)), false);
     CORRADE_ASSERT(Implementation::isVertexFormatCompatible<typename std::remove_extent<T>::type>(attribute._format),
         prefix << attribute._name << "is" << attribute._format << "but requested a type equivalent to" << Implementation::vertexFormatFor<typename std::remove_extent<T>::type>(), false);
-    if(attribute._arraySize) CORRADE_ASSERT(std::is_array<T>::value,
+    CORRADE_ASSERT(!attribute._arraySize || std::is_array<T>::value,
         prefix << attribute._name << "is an array attribute, use T[] to access it", false);
-    else CORRADE_ASSERT(!std::is_array<T>::value,
-        prefix << attribute._name << "is not an array attribute, can't use T[] to access it", false);
     return true;
 }
 #endif

src/Magnum/Trade/SceneData.h

@@ -2243,11 +2243,12 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * @brief Data for given array field in a concrete type
          * @m_since_latest
          *
-         * Same as above, except that it works with array fields instead ---
+         * Same as above, except that it works with array fields as well ---
          * you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref fieldArraySize(UnsignedInt) const for given field.
+         * @ref fieldArraySize(UnsignedInt) const for given field. For
+         * non-array fields the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<const typename std::remove_extent<T>::type> field(UnsignedInt id) const;
 
@@ -2265,11 +2266,12 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * @brief Mutable data for given array field in a concrete type
          * @m_since_latest
          *
-         * Same as above, except that it works with array fields instead ---
+         * Same as above, except that it works with array fields as well ---
          * you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref fieldArraySize(UnsignedInt) const for given field.
+         * @ref fieldArraySize(UnsignedInt) const for given field. For
+         * non-array fields the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<typename std::remove_extent<T>::type> mutableField(UnsignedInt id);
 
@@ -2321,11 +2323,12 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * @brief Data for given named array field in a concrete type
          * @m_since_latest
          *
-         * Same as above, except that it works with array fields instead ---
+         * Same as above, except that it works with array fields as well ---
          * you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref fieldArraySize(SceneField) const for given field.
+         * @ref fieldArraySize(SceneField) const for given field. For non-array
+         * fields the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<const typename std::remove_extent<T>::type> field(SceneField name) const;
 
@@ -2343,11 +2346,12 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * @brief Mutable data for given named array field in a concrete type
          * @m_since_latest
          *
-         * Same as above, except that it works with array fields instead ---
+         * Same as above, except that it works with array fields as well ---
          * you're expected to select this overload by passing @cpp T[] @ce
          * instead of @cpp T @ce. The second dimension is guaranteed to be
          * contiguous and have the same size as reported by
-         * @ref fieldArraySize(SceneField) const for given field.
+         * @ref fieldArraySize(SceneField) const for given field. For non-array
+         * fields the second dimension has a size of @cpp 1 @ce.
          */
         template<class T, class = typename std::enable_if<std::is_array<T>::value>::type> Containers::StridedArrayView2D<typename std::remove_extent<T>::type> mutableField(SceneField name);
 
@@ -3773,10 +3777,8 @@ template<class T> Containers::StridedArrayView1D<T> SceneData::mutableMapping(co
 template<class T> bool SceneData::checkFieldTypeCompatibility(const SceneFieldData& field, const char* const prefix) const {
     CORRADE_ASSERT(Implementation::SceneFieldTypeTraits<typename std::remove_extent<T>::type>::isCompatible(field.fieldType()),
         prefix << field._name << "is" << field.fieldType() << "but requested a type equivalent to" << Implementation::SceneFieldTypeFor<typename std::remove_extent<T>::type>::type(), false);
-    if(field.fieldArraySize()) CORRADE_ASSERT(std::is_array<T>::value,
+    CORRADE_ASSERT(!field.fieldArraySize() || std::is_array<T>::value,
         prefix << field._name << "is an array field, use T[] to access it", false);
-    else CORRADE_ASSERT(!std::is_array<T>::value,
-        prefix << field._name << "is not an array field, can't use T[] to access it", false);
     return true;
 }
 #endif

src/Magnum/Trade/Test/MeshDataTest.cpp

@@ -1411,6 +1411,15 @@ void MeshDataTest::construct() {
         CORRADE_COMPARE(data.mutableAttribute<Short[]>(4)[1][1], 2);
     }
 
+    /* Accessing a non-array attribute as an array should be possible as well
+       -- the second dimension is then just 1 */
+    CORRADE_COMPARE(data.attribute<Vector3[]>(0).size(), (Containers::Size2D{instanceData.expectedVertexCount, 1}));
+    CORRADE_COMPARE(data.mutableAttribute<Vector3[]>(0).size(), (Containers::Size2D{instanceData.expectedVertexCount, 1}));
+    if(instanceData.vertexCount) {
+        CORRADE_COMPARE(data.attribute<Vector3[]>(0)[1][0], (Vector3{0.4f, 0.5f, 0.6f}));
+        CORRADE_COMPARE(data.mutableAttribute<Vector3[]>(0)[1][0], (Vector3{0.4f, 0.5f, 0.6f}));
+    }
+
     /* Attribute access by name */
     CORRADE_VERIFY(data.hasAttribute(MeshAttribute::Position));
     CORRADE_VERIFY(data.hasAttribute(MeshAttribute::Normal));
@@ -1512,6 +1521,15 @@ void MeshDataTest::construct() {
         CORRADE_COMPARE(data.mutableAttribute<Short[]>(meshAttributeCustom(13))[2][0], 22);
         CORRADE_COMPARE(data.mutableAttribute<Short[]>(meshAttributeCustom(13))[2][1], -1);
     }
+
+    /* Accessing a non-array attribute as an array should be possible as well
+       -- the second dimension is then just 1 */
+    CORRADE_COMPARE(data.attribute<Vector3[]>(MeshAttribute::Position).size(), (Containers::Size2D{instanceData.expectedVertexCount, 1}));
+    CORRADE_COMPARE(data.mutableAttribute<Vector3[]>(MeshAttribute::Position).size(), (Containers::Size2D{instanceData.expectedVertexCount, 1}));
+    if(instanceData.vertexCount) {
+        CORRADE_COMPARE(data.attribute<Vector3[]>(MeshAttribute::Position)[1][0], (Vector3{0.4f, 0.5f, 0.6f}));
+        CORRADE_COMPARE(data.mutableAttribute<Vector3[]>(MeshAttribute::Position)[1][0], (Vector3{0.4f, 0.5f, 0.6f}));
+    }
 }
 
 void MeshDataTest::constructZeroIndices() {
@@ -3964,32 +3982,25 @@ void MeshDataTest::attributeWrongArrayAccess() {
         {1.1f, 2.2f}, {3.3f, 4.4f}, {5.5f, 6.6f}, {7.7f, 8.8f},
         {0.1f, 0.2f}, {0.3f, 0.4f}, {0.5f, 0.6f}, {0.7f, 0.8f},
     };
-    Containers::StridedArrayView1D<Vector2> positions{vertexData, 3, 4*sizeof(Vector2)};
     Containers::StridedArrayView2D<Vector2> positions2D{vertexData, {3, 4}};
 
     MeshData data{MeshPrimitive::TriangleFan, DataFlag::Mutable, vertexData, {
-        MeshAttributeData{MeshAttribute::Position, positions},
         MeshAttributeData{meshAttributeCustom(35), positions2D}
     }};
 
+    /* Array access is allowed for non-array attributes (the second dimension
+       is then always 1), tested directly in construct() */
+
     std::ostringstream out;
     Error redirectError{&out};
-    data.attribute<Vector2[]>(0);
-    data.attribute<Vector2>(1);
-    data.mutableAttribute<Vector2[]>(0);
-    data.mutableAttribute<Vector2>(1);
-    data.attribute<Vector2[]>(MeshAttribute::Position);
+    data.attribute<Vector2>(0);
+    data.mutableAttribute<Vector2>(0);
     data.attribute<Vector2>(meshAttributeCustom(35));
-    data.mutableAttribute<Vector2[]>(MeshAttribute::Position);
     data.mutableAttribute<Vector2>(meshAttributeCustom(35));
     CORRADE_COMPARE(out.str(),
-        "Trade::MeshData::attribute(): Trade::MeshAttribute::Position is not an array attribute, can't use T[] to access it\n"
         "Trade::MeshData::attribute(): Trade::MeshAttribute::Custom(35) is an array attribute, use T[] to access it\n"
-        "Trade::MeshData::mutableAttribute(): Trade::MeshAttribute::Position is not an array attribute, can't use T[] to access it\n"
         "Trade::MeshData::mutableAttribute(): Trade::MeshAttribute::Custom(35) is an array attribute, use T[] to access it\n"
-        "Trade::MeshData::attribute(): Trade::MeshAttribute::Position is not an array attribute, can't use T[] to access it\n"
         "Trade::MeshData::attribute(): Trade::MeshAttribute::Custom(35) is an array attribute, use T[] to access it\n"
-        "Trade::MeshData::mutableAttribute(): Trade::MeshAttribute::Position is not an array attribute, can't use T[] to access it\n"
         "Trade::MeshData::mutableAttribute(): Trade::MeshAttribute::Custom(35) is an array attribute, use T[] to access it\n");
 }
 

src/Magnum/Trade/Test/SceneDataTest.cpp

@@ -2019,6 +2019,13 @@ void SceneDataTest::construct() {
         Containers::stridedArrayView({37.5f, 1.5f}),
         TestSuite::Compare::Container);
 
+    /* Accessing a non-array field as an array should be possible as well --
+       the second dimension is then just 1 */
+    CORRADE_COMPARE(scene.field<UnsignedByte[]>(2).size(), (Containers::Size2D{2, 1}));
+    CORRADE_COMPARE(scene.mutableField<UnsignedByte[]>(2).size(), (Containers::Size2D{2, 1}));
+    CORRADE_COMPARE(scene.field<UnsignedByte[]>(2)[1][0], 7);
+    CORRADE_COMPARE(scene.mutableField<UnsignedByte[]>(2)[1][0], 7);
+
     /* Field property access by name */
     CORRADE_COMPARE(scene.fieldFlags(SceneField::Transformation), SceneFieldFlags{});
     CORRADE_COMPARE(scene.fieldFlags(SceneField::Parent), SceneFieldFlag::OffsetOnly);
@@ -2112,6 +2119,13 @@ void SceneDataTest::construct() {
     CORRADE_COMPARE_AS(scene.mutableField<Float[]>(sceneFieldCustom(37))[0],
         Containers::stridedArrayView({37.5f, 1.5f}),
         TestSuite::Compare::Container);
+
+    /* Accessing a non-array field as an array should be possible as well --
+       the second dimension is then just 1 */
+    CORRADE_COMPARE(scene.field<UnsignedByte[]>(SceneField::Mesh).size(), (Containers::Size2D{2, 1}));
+    CORRADE_COMPARE(scene.mutableField<UnsignedByte[]>(SceneField::Mesh).size(), (Containers::Size2D{2, 1}));
+    CORRADE_COMPARE(scene.field<UnsignedByte[]>(SceneField::Mesh)[1][0], 7);
+    CORRADE_COMPARE(scene.mutableField<UnsignedByte[]>(SceneField::Mesh)[1][0], 7);
 }
 
 void SceneDataTest::constructZeroFields() {
@@ -5769,35 +5783,28 @@ void SceneDataTest::fieldWrongArrayAccess() {
 
     struct Field {
         UnsignedInt object;
-        UnsignedInt mesh;
         UnsignedInt foobar;
     } fields[2]{};
 
     Containers::StridedArrayView1D<Field> view = fields;
 
     SceneData scene{SceneMappingType::UnsignedInt, 5, DataFlag::Mutable, fields, {
-        SceneFieldData{SceneField::Mesh, view.slice(&Field::object), view.slice(&Field::mesh)},
         SceneFieldData{sceneFieldCustom(35), view.slice(&Field::object), Containers::arrayCast<2, UnsignedInt>(view.slice(&Field::foobar))},
     }};
 
+    /* Array access is allowed for non-array fields (the second dimension is
+       then always 1), tested directly in construct() */
+
     std::ostringstream out;
     Error redirectError{&out};
-    scene.field<UnsignedInt[]>(0);
-    scene.field<UnsignedInt>(1);
-    scene.mutableField<UnsignedInt[]>(0);
-    scene.mutableField<UnsignedInt>(1);
-    scene.field<UnsignedInt[]>(SceneField::Mesh);
+    scene.field<UnsignedInt>(0);
+    scene.mutableField<UnsignedInt>(0);
     scene.field<UnsignedInt>(sceneFieldCustom(35));
-    scene.mutableField<UnsignedInt[]>(SceneField::Mesh);
     scene.mutableField<UnsignedInt>(sceneFieldCustom(35));
     CORRADE_COMPARE(out.str(),
-        "Trade::SceneData::field(): Trade::SceneField::Mesh is not an array field, can't use T[] to access it\n"
         "Trade::SceneData::field(): Trade::SceneField::Custom(35) is an array field, use T[] to access it\n"
-        "Trade::SceneData::mutableField(): Trade::SceneField::Mesh is not an array field, can't use T[] to access it\n"
         "Trade::SceneData::mutableField(): Trade::SceneField::Custom(35) is an array field, use T[] to access it\n"
-        "Trade::SceneData::field(): Trade::SceneField::Mesh is not an array field, can't use T[] to access it\n"
         "Trade::SceneData::field(): Trade::SceneField::Custom(35) is an array field, use T[] to access it\n"
-        "Trade::SceneData::mutableField(): Trade::SceneField::Mesh is not an array field, can't use T[] to access it\n"
         "Trade::SceneData::mutableField(): Trade::SceneField::Custom(35) is an array field, use T[] to access it\n");
 }