diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index e7bbf7544..baa94dd2b 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -91,7 +91,24 @@ Containers::Optional> interleavedData return Containers::NullOpt; return Containers::StridedArrayView2D{ - mesh.vertexData(), mesh.vertexData().data() + minOffset, + /* MeshData only requires the vertex data to be large enough to fit the + actual data, not to have the size large enough to fit `count*stride` + elements. The StridedArrayView expects the latter, so this extends + the size to satisfy the assert. For simplicity it overextends by the + whole stride instead of just max end offset, relying on the input + MeshData having checked the bounds already. To be clear, the output + is *never* out of bounds of the vertex data, the second dimension is + always sized to fit within. This is verified in the + interleavedDataGapsTrailingOmitted() test. + + Additionally, the max() is here because some algorithms such as + combineFaceAttributes() pass `{nullptr, ~std::size_t{}}` as + vertexData and without the max() it would overflow. That case is + tested in interleavedDataVertexDataWholeMemory(). */ + /** @todo maybe just fix StridedArrayView to require only what's really + needed? */ + {mesh.vertexData().data(), Math::max(mesh.vertexData().size(), mesh.vertexData().size() + stride)}, + mesh.vertexData().data() + minOffset, {mesh.vertexCount(), maxOffset - minOffset}, {std::ptrdiff_t(stride), 1}}; } diff --git a/src/Magnum/MeshTools/Test/InterleaveTest.cpp b/src/Magnum/MeshTools/Test/InterleaveTest.cpp index b9cb5e855..1c51d61fc 100644 --- a/src/Magnum/MeshTools/Test/InterleaveTest.cpp +++ b/src/Magnum/MeshTools/Test/InterleaveTest.cpp @@ -59,6 +59,7 @@ struct InterleaveTest: Corrade::TestSuite::Tester { void interleavedData(); void interleavedDataUnordered(); void interleavedDataGaps(); + void interleavedDataGapsTrailingOmitted(); void interleavedDataAliased(); void interleavedDataSingleAttribute(); void interleavedDataArrayAttributes(); @@ -147,6 +148,7 @@ InterleaveTest::InterleaveTest() { &InterleaveTest::interleavedData, &InterleaveTest::interleavedDataUnordered, &InterleaveTest::interleavedDataGaps, + &InterleaveTest::interleavedDataGapsTrailingOmitted, &InterleaveTest::interleavedDataAliased, &InterleaveTest::interleavedDataSingleAttribute, &InterleaveTest::interleavedDataArrayAttributes, @@ -450,6 +452,30 @@ void InterleaveTest::interleavedDataGaps() { CORRADE_COMPARE(interleaved.stride()[1], 1); } +void InterleaveTest::interleavedDataGapsTrailingOmitted() { + /* Similar to interleavedDataGaps(), but with padding at the end omitted. + MeshData allows that, but StridedArrayView constructors don't (which is + why this is using offset-only attributes), so verify we don't trip up on + that. */ + Containers::Array vertexData{2*48 + 36}; + const char* vertexDataPointer = vertexData.data(); + + Trade::MeshData data{MeshPrimitive::Triangles, Utility::move(vertexData), { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + VertexFormat::Vector2, 5, 3, 48}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, + VertexFormat::Vector3, 24, 3, 48} + }}; + CORRADE_VERIFY(MeshTools::isInterleaved(data)); + + Containers::StridedArrayView2D interleaved = MeshTools::interleavedData(data); + CORRADE_COMPARE(interleaved.data(), vertexDataPointer + 5); + CORRADE_COMPARE(interleaved.size()[0], 3); + CORRADE_COMPARE(interleaved.size()[1], 31); + CORRADE_COMPARE(interleaved.stride()[0], 48); + CORRADE_COMPARE(interleaved.stride()[1], 1); +} + void InterleaveTest::interleavedDataAliased() { /* Compared to interleavedData() the normals share first two components with positions */