MeshTools: fail isInterleaved() for zero and negative strides.

I tried, I really did, but the downsides eventually just outweighed the
potential of this feature and I gave up. May try to tackle this again in
the future, but not now.

src/Magnum/MeshTools/Interleave.cpp

@@ -47,7 +47,13 @@ Containers::Optional<Containers::StridedArrayView2D<const char>> interleavedData
     if(!data.attributeCount())
         return Containers::StridedArrayView2D<const char>{data.vertexData(), {data.vertexCount(), 0}};
 
-    const UnsignedInt stride = data.attributeStride(0);
+    /* Technically zero and negative strides *may* also be categorized as
+       interleaved if they are all the same, but it causes way too many
+       problems especially when used within interleavedLayout() etc. May
+       tackle properly later. */
+    const Int stride = data.attributeStride(0);
+    if(stride <= 0) return Containers::NullOpt;
+
     std::size_t minOffset = ~std::size_t{};
     std::size_t maxOffset = 0;
     bool hasImplementationSpecificVertexFormat = false;
@@ -77,7 +83,7 @@ Containers::Optional<Containers::StridedArrayView2D<const char>> interleavedData
         maxOffset = Math::max(maxOffset, minOffset + stride);
 
     /* The offsets can't fit into the stride, report failure */
-    if(maxOffset - minOffset > stride) return Containers::NullOpt;
+    if(maxOffset - minOffset > UnsignedInt(stride)) return Containers::NullOpt;
 
     return Containers::StridedArrayView2D<const char>{
         data.vertexData(), data.vertexData().data() + minOffset,

src/Magnum/MeshTools/Interleave.h

@@ -228,10 +228,14 @@ CORRADE_ENUMSET_OPERATORS(InterleaveFlags)
 @brief If the mesh data is interleaved
 @m_since{2020,06}
 
-Returns @cpp true @ce if all attributes have the same stride and the difference
+Returns @cpp true @ce if all attributes have the same *positive* stride and the
-between minimal and maximal offset is not larger than the stride, @cpp false @ce
+difference between minimal and maximal offset is not larger than the stride,
-otherwise. In particular, returns @cpp true @ce also if the mesh has just one
+@cpp false @ce otherwise. In particular, returns @cpp true @ce also if the mesh
-or no attributes.
+has just one or no attributes.
+
+While interleaved layouts technically may also have zero or negative strides,
+this case is currently not implemented and such layouts are treated as
+non-interleaved.
 @see @ref Trade::MeshData::attributeStride(),
     @ref Trade::MeshData::attributeOffset(), @ref interleavedData()
 */
@@ -269,11 +273,11 @@ MAGNUM_MESHTOOLS_EXPORT Containers::StridedArrayView2D<char> interleavedMutableD
 Returns a @ref Trade::MeshData instance with its vertex data allocated for
 @p vertexCount vertices containing attributes from both @p data and @p extra
 interleaved together. No data is actually copied, only an interleaved layout is
-created. If @p data is already interleaved and
+created. If @p data is already interleaved according to @ref isInterleaved()
-@ref InterleaveFlag::PreserveInterleavedAttributes is set in @p flags, keeps
+and @ref InterleaveFlag::PreserveInterleavedAttributes is set in @p flags,
-the attributes in the same layout, potentially extending them with @p extra.
+keeps the attributes in the same layout, potentially extending them with
-The @p extra attributes, if any, are interleaved together with existing
+@p extra. The @p extra attributes, if any, are interleaved together with
-attributes. Returned instance vertex data flags have both
+existing attributes. Returned instance vertex data flags have both
 @ref Trade::DataFlag::Mutable and @ref Trade::DataFlag::Owned, so mutable
 attribute access is guaranteed.
 

src/Magnum/MeshTools/Test/InterleaveTest.cpp

@@ -64,6 +64,8 @@ struct InterleaveTest: Corrade::TestSuite::Tester {
     void interleavedDataNoVertices();
     void interleavedDataNotInterleaved();
     void interleavedDataAttributeAcrossStride();
+    void interleavedDataZeroStride();
+    void interleavedDataNegativeStride();
     void interleavedDataVertexDataWholeMemory();
     void interleavedMutableDataNotMutable();
     void interleavedDataImplementationSpecificVertexFormat();
@@ -129,6 +131,8 @@ InterleaveTest::InterleaveTest() {
               &InterleaveTest::interleavedDataNoVertices,
               &InterleaveTest::interleavedDataNotInterleaved,
               &InterleaveTest::interleavedDataAttributeAcrossStride,
+              &InterleaveTest::interleavedDataZeroStride,
+              &InterleaveTest::interleavedDataNegativeStride,
               &InterleaveTest::interleavedDataVertexDataWholeMemory,
               &InterleaveTest::interleavedMutableDataNotMutable,
               &InterleaveTest::interleavedDataImplementationSpecificVertexFormat,
@@ -422,6 +426,40 @@ void InterleaveTest::interleavedDataArrayAttributes() {
     CORRADE_COMPARE(interleaved.stride()[1], 1);
 }
 
+void InterleaveTest::interleavedDataZeroStride() {
+    Containers::Array<char> vertexData{100 + 20};
+    Containers::StridedArrayView1D<Vector2> positions{vertexData,
+        reinterpret_cast<Vector2*>(vertexData.data() + 100), 3, 0};
+    Containers::StridedArrayView1D<Vector3> normals{vertexData,
+        reinterpret_cast<Vector3*>(vertexData.data() + 100 + 8), 3, 0};
+
+    Trade::MeshData data{MeshPrimitive::Triangles, std::move(vertexData), {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, positions},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Normal, normals}
+    }};
+
+    /* Technically they *are*, but it causes way too many problems especially
+       when used within interleavedLayout() etc. May tackle properly later. */
+    CORRADE_VERIFY(!MeshTools::isInterleaved(data));
+}
+
+void InterleaveTest::interleavedDataNegativeStride() {
+    Containers::Array<char> vertexData{100 + 3*20};
+    Containers::StridedArrayView1D<Vector2> positions{vertexData,
+        reinterpret_cast<Vector2*>(vertexData.data() + 100), 3, 20};
+    Containers::StridedArrayView1D<Vector3> normals{vertexData,
+        reinterpret_cast<Vector3*>(vertexData.data() + 100 + 8), 3, 20};
+
+    Trade::MeshData data{MeshPrimitive::Triangles, std::move(vertexData), {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, positions.flipped<0>()},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Normal, normals.flipped<0>()}
+    }};
+
+    /* Technically they *are*, but it causes way too many problems especially
+       when used within interleavedLayout() etc. May tackle properly later. */
+    CORRADE_VERIFY(!MeshTools::isInterleaved(data));
+}
+
 void InterleaveTest::interleavedDataEmpty() {
     Trade::MeshData data{MeshPrimitive::Triangles, 5};
     CORRADE_VERIFY(MeshTools::isInterleaved(data));