Browse Source

MeshTools: fix interleavedData() if trailing padding is omitted.

I should probably fix it in StridedArrayView already, the same
workaround is in GltfImporter in two places, and likely elsewhere as
well.
pull/674/head
Vladimír Vondruš 1 year ago
parent
commit
f19dcac1b3
  1. 19
      src/Magnum/MeshTools/Interleave.cpp
  2. 26
      src/Magnum/MeshTools/Test/InterleaveTest.cpp

19
src/Magnum/MeshTools/Interleave.cpp

@ -91,7 +91,24 @@ Containers::Optional<Containers::StridedArrayView2D<const char>> interleavedData
return Containers::NullOpt; return Containers::NullOpt;
return Containers::StridedArrayView2D<const char>{ return Containers::StridedArrayView2D<const char>{
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}, {mesh.vertexCount(), maxOffset - minOffset},
{std::ptrdiff_t(stride), 1}}; {std::ptrdiff_t(stride), 1}};
} }

26
src/Magnum/MeshTools/Test/InterleaveTest.cpp

@ -59,6 +59,7 @@ struct InterleaveTest: Corrade::TestSuite::Tester {
void interleavedData(); void interleavedData();
void interleavedDataUnordered(); void interleavedDataUnordered();
void interleavedDataGaps(); void interleavedDataGaps();
void interleavedDataGapsTrailingOmitted();
void interleavedDataAliased(); void interleavedDataAliased();
void interleavedDataSingleAttribute(); void interleavedDataSingleAttribute();
void interleavedDataArrayAttributes(); void interleavedDataArrayAttributes();
@ -147,6 +148,7 @@ InterleaveTest::InterleaveTest() {
&InterleaveTest::interleavedData, &InterleaveTest::interleavedData,
&InterleaveTest::interleavedDataUnordered, &InterleaveTest::interleavedDataUnordered,
&InterleaveTest::interleavedDataGaps, &InterleaveTest::interleavedDataGaps,
&InterleaveTest::interleavedDataGapsTrailingOmitted,
&InterleaveTest::interleavedDataAliased, &InterleaveTest::interleavedDataAliased,
&InterleaveTest::interleavedDataSingleAttribute, &InterleaveTest::interleavedDataSingleAttribute,
&InterleaveTest::interleavedDataArrayAttributes, &InterleaveTest::interleavedDataArrayAttributes,
@ -450,6 +452,30 @@ void InterleaveTest::interleavedDataGaps() {
CORRADE_COMPARE(interleaved.stride()[1], 1); 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<char> 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<const char> 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() { void InterleaveTest::interleavedDataAliased() {
/* Compared to interleavedData() the normals share first two components /* Compared to interleavedData() the normals share first two components
with positions */ with positions */

Loading…
Cancel
Save