ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

MeshTools: rewrite copy() tests without using Primitives. 

Building a mesh from scratch makes the test much easier to grasp and
allows me to test more corner cases together instead of having to add
special cases for offset-only, array and implementation-specific
attributes.

The test now also uncovers a potential optimization opportunity where a
copy of the attribute metadata could be avoided in some cases. Not
implementing it right now, just adding a TODO and an XFAIL.
pull/620/head
Vladimír Vondruš 3 years ago
parent
015c8bba99
commit
b27e7ef865
2 changed files with 188 additions and 114 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 3
      src/Magnum/MeshTools/Copy.cpp
  2. 299
      src/Magnum/MeshTools/Test/CopyTest.cpp

3
src/Magnum/MeshTools/Copy.cpp
Unescape View File

@ -135,6 +135,9 @@ Trade::MeshData copy(Trade::MeshData&& mesh) {
/* Otherwise we have to allocate a new one and re-route the attributes to /* Otherwise we have to allocate a new one and re-route the attributes to
a potentially different vertex array */ a potentially different vertex array */
/** @todo could theoretically also just modify the array in-place if it has
a default deleter, but would need to pay attention to not copy items
to themselves and such */
} else { } else {
/* Using DefaultInit so the array has a default deleter and isn't /* Using DefaultInit so the array has a default deleter and isn't
problematic to use in plugins */ problematic to use in plugins */

299
src/Magnum/MeshTools/Test/CopyTest.cpp
Unescape View File

@ -26,6 +26,7 @@
#include <sstream> #include <sstream>
#include <Corrade/TestSuite/Tester.h> #include <Corrade/TestSuite/Tester.h>
#include <Corrade/TestSuite/Compare/Container.h> #include <Corrade/TestSuite/Compare/Container.h>
#include <Corrade/Utility/Algorithms.h>
#include <Corrade/Utility/DebugStl.h> #include <Corrade/Utility/DebugStl.h>
#include "Magnum/Math/Color.h" #include "Magnum/Math/Color.h"
@ -42,14 +43,13 @@ struct CopyTest: TestSuite::Tester {
explicit CopyTest(); explicit CopyTest();
void copy(); void copy();
void copyOffsetOnlyAttributes();
void copyNoIndexData(); void copyNoIndexData();
void copyNoAttributeVertexData(); void copyNoAttributeVertexData();
void copyStridedIndices(); void copyStridedIndices();
void copyArrayAttribute();
void copyImplementationSpecificVertexFormat(); void copyRvalueNotOwned();
void copyRvaluePassthrough(); void copyRvalueIndicesVerticesAttributesOwned();
void copyRvaluePartialPassthrough(); void copyRvalueAttributesOwned();
void reference(); void reference();
void referenceNoIndexData(); void referenceNoIndexData();
@ -73,17 +73,15 @@ struct {
CopyTest::CopyTest() { CopyTest::CopyTest() {
addTests({&CopyTest::copy, addTests({&CopyTest::copy,
&CopyTest::copyOffsetOnlyAttributes,
&CopyTest::copyNoIndexData, &CopyTest::copyNoIndexData,
&CopyTest::copyNoAttributeVertexData}); &CopyTest::copyNoAttributeVertexData});
addInstancedTests({&CopyTest::copyStridedIndices}, addInstancedTests({&CopyTest::copyStridedIndices},
Containers::arraySize(StridedIndicesData)); Containers::arraySize(StridedIndicesData));
addTests({&CopyTest::copyArrayAttribute, addTests({&CopyTest::copyRvalueNotOwned,
&CopyTest::copyImplementationSpecificVertexFormat, &CopyTest::copyRvalueIndicesVerticesAttributesOwned,
&CopyTest::copyRvaluePassthrough, &CopyTest::copyRvalueAttributesOwned,
&CopyTest::copyRvaluePartialPassthrough,
&CopyTest::reference, &CopyTest::reference,
&CopyTest::referenceNoIndexData, &CopyTest::referenceNoIndexData,
@ -98,34 +96,89 @@ CopyTest::CopyTest() {
} }
void CopyTest::copy() { void CopyTest::copy() {
Trade::MeshData cube = Primitives::cubeSolid(); const struct Vertex {
CORRADE_COMPARE(cube.indexDataFlags(), Trade::DataFlag::Global); Vector3 position;
CORRADE_COMPARE(cube.vertexDataFlags(), Trade::DataFlag::Global); Short array[3];
Vector2ub textureCoordinates;
Trade::MeshData copy = MeshTools::copy(cube); bool bit;
} vertices[]{
{{1.0f, 2.0f, 3.0f}, {15, -36, 0}, {4, 5}, false},
{{6.0f, 7.0f, 8.0f}, {0, -36, 12}, {9, 0}, true}
};
const UnsignedShort indices[]{
/* First is not used */
2, 1, 0, 1
};
Trade::MeshData mesh{MeshPrimitive::Triangles,
{}, indices, Trade::MeshIndexData{Containers::arrayView(indices).exceptPrefix(1)},
{}, vertices, {
Trade::MeshAttributeData{Trade::MeshAttribute::Position,
Containers::stridedArrayView(vertices).slice(&Vertex::position)},
/* Array attribute */
Trade::MeshAttributeData{Trade::meshAttributeCustom(37),
VertexFormat::Short,
Containers::stridedArrayView(vertices).slice(&Vertex::array), 3},
/* Offset-only attribute */
Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates,
VertexFormat::Vector2ub,
offsetof(Vertex, textureCoordinates), 2, sizeof(Vertex)},
/* Implementation-specific vertex format */
Trade::MeshAttributeData{Trade::meshAttributeCustom(56),
vertexFormatWrap(0xb001),
Containers::stridedArrayView(vertices).slice(&Vertex::bit)},
}};
Trade::MeshData copy = MeshTools::copy(mesh);
CORRADE_VERIFY(copy.isIndexed()); CORRADE_VERIFY(copy.isIndexed());
CORRADE_COMPARE(copy.primitive(), cube.primitive()); CORRADE_COMPARE(copy.primitive(), mesh.primitive());
CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.indexCount(), cube.indexCount());
CORRADE_COMPARE(copy.indexType(), cube.indexType());
CORRADE_COMPARE(copy.indexOffset(), cube.indexOffset());
CORRADE_COMPARE(copy.indexStride(), cube.indexStride());
CORRADE_COMPARE(copy.vertexCount(), cube.vertexCount());
CORRADE_COMPARE(copy.attributeCount(), cube.attributeCount());
for(std::size_t i = 0; i != cube.attributeCount(); ++i) { CORRADE_COMPARE(copy.indexType(), MeshIndexType::UnsignedShort);
CORRADE_COMPARE_AS(copy.indices<UnsignedShort>(),
Containers::arrayView(indices).exceptPrefix(1),
TestSuite::Compare::Container);
CORRADE_COMPARE(copy.vertexCount(), 2);
CORRADE_COMPARE(copy.attributeCount(), 4);
for(std::size_t i = 0; i != mesh.attributeCount(); ++i) {
CORRADE_ITERATION(i); CORRADE_ITERATION(i);
CORRADE_COMPARE(copy.attributeName(i), cube.attributeName(i)); CORRADE_COMPARE(copy.attributeName(i), mesh.attributeName(i));
CORRADE_COMPARE(copy.attributeFormat(i), cube.attributeFormat(i)); CORRADE_COMPARE(copy.attributeFormat(i), mesh.attributeFormat(i));
CORRADE_COMPARE(copy.attributeOffset(i), cube.attributeOffset(i)); CORRADE_COMPARE(copy.attributeArraySize(i), mesh.attributeArraySize(i));
CORRADE_COMPARE(copy.attributeStride(i), cube.attributeStride(i));
CORRADE_COMPARE(copy.attributeArraySize(i), cube.attributeArraySize(i));
} }
CORRADE_COMPARE_AS(copy.indexData(), cube.indexData(), TestSuite::Compare::Container); /* Offset-only attributes should be just passed through during the copy,
CORRADE_COMPARE_AS(copy.vertexData(), cube.vertexData(), TestSuite::Compare::Container); not made absolute */
CORRADE_VERIFY(copy.attributeData()[2].isOffsetOnly());
CORRADE_COMPARE_AS(copy.attribute<Vector3>(Trade::MeshAttribute::Position),
Containers::stridedArrayView(vertices).slice(&Vertex::position),
TestSuite::Compare::Container);
CORRADE_COMPARE_AS(copy.attribute<Short[]>(Trade::meshAttributeCustom(37))[0],
Containers::arrayView<Short>({15, -36, 0}),
TestSuite::Compare::Container);
CORRADE_COMPARE_AS(copy.attribute<Short[]>(Trade::meshAttributeCustom(37))[1],
Containers::arrayView<Short>({0, -36, 12}),
TestSuite::Compare::Container);
CORRADE_COMPARE_AS(copy.attribute<Vector2ub>(Trade::MeshAttribute::TextureCoordinates),
Containers::stridedArrayView(vertices).slice(&Vertex::textureCoordinates),
TestSuite::Compare::Container);
CORRADE_COMPARE_AS((Containers::arrayCast<const bool>(copy.attribute(Trade::meshAttributeCustom(56))).transposed<0, 1>()[0]),
Containers::stridedArrayView(vertices).slice(&Vertex::bit),
TestSuite::Compare::Container);
/* The data layout should be the same and thus the raw data should match
as well */
CORRADE_COMPARE_AS(copy.indexData(),
mesh.indexData(),
TestSuite::Compare::Container);
CORRADE_COMPARE_AS(copy.vertexData(),
mesh.vertexData(),
TestSuite::Compare::Container);
/* The data should have a default deleter to make this usable in plugins */ /* The data should have a default deleter to make this usable in plugins */
Containers::Array<char> indexData = copy.releaseIndexData(); Containers::Array<char> indexData = copy.releaseIndexData();
@ -136,30 +189,6 @@ void CopyTest::copy() {
CORRADE_VERIFY(!attributeData.deleter()); CORRADE_VERIFY(!attributeData.deleter());
} }
void CopyTest::copyOffsetOnlyAttributes() {
Trade::MeshData circle = Primitives::circle2DSolid(5);
Trade::MeshData copy = MeshTools::copy(circle);
CORRADE_COMPARE(copy.primitive(), circle.primitive());
CORRADE_COMPARE(copy.vertexCount(), circle.vertexCount());
CORRADE_COMPARE(copy.attributeCount(), circle.attributeCount());
for(std::size_t i = 0; i != circle.attributeCount(); ++i) {
CORRADE_ITERATION(i);
CORRADE_COMPARE(copy.attributeName(i), circle.attributeName(i));
CORRADE_COMPARE(copy.attributeFormat(i), circle.attributeFormat(i));
CORRADE_COMPARE(copy.attributeOffset(i), circle.attributeOffset(i));
CORRADE_COMPARE(copy.attributeStride(i), circle.attributeStride(i));
CORRADE_COMPARE(copy.attributeArraySize(i), circle.attributeArraySize(i));
/* The circle has offset-only attributes, that should be preserved
after the copy as well */
CORRADE_VERIFY(copy.attributeData()[i].isOffsetOnly());
}
CORRADE_COMPARE_AS(copy.vertexData(), circle.vertexData(), TestSuite::Compare::Container);
}
void CopyTest::copyNoIndexData() { void CopyTest::copyNoIndexData() {
Trade::MeshData cube = Primitives::cubeSolidStrip(); Trade::MeshData cube = Primitives::cubeSolidStrip();
CORRADE_VERIFY(!cube.isIndexed()); CORRADE_VERIFY(!cube.isIndexed());
@ -228,78 +257,120 @@ void CopyTest::copyStridedIndices() {
TestSuite::Compare::Container); TestSuite::Compare::Container);
} }
void CopyTest::copyArrayAttribute() { void CopyTest::copyRvalueNotOwned() {
const Vector3us vertexData[13]{}; Vector3 positions[]{
/* Verify that array attributes are propagated correctly */ {1.0f, 2.0f, 3.0f},
Trade::MeshData weirdThing{MeshPrimitive::Faces, {6.0f, 7.0f, 8.0f},
{}, vertexData, };
{Trade::MeshAttributeData{Trade::meshAttributeCustom(42), VertexFormat::Half, Containers::arrayView(vertexData), 3}}}; const UnsignedShort indices[]{
1, 0, 1
Trade::MeshData copy = MeshTools::copy(weirdThing); };
CORRADE_COMPARE(copy.vertexCount(), 13); const Trade::MeshAttributeData attributes[]{
CORRADE_COMPARE(copy.attributeCount(), 1); Trade::MeshAttributeData{Trade::MeshAttribute::Position,
CORRADE_COMPARE(copy.attributeArraySize(0), 3); Containers::arrayView(positions)},
} };
Trade::MeshData copy = MeshTools::copy(Trade::MeshData{MeshPrimitive::Triangles,
Trade::DataFlag::ExternallyOwned, indices, Trade::MeshIndexData{indices},
Trade::DataFlag::Mutable, positions, Trade::meshAttributeDataNonOwningArray(attributes)});
CORRADE_VERIFY(copy.isIndexed());
CORRADE_COMPARE(copy.primitive(), MeshPrimitive::Triangles);
CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
void CopyTest::copyImplementationSpecificVertexFormat() { CORRADE_COMPARE(copy.indexType(), MeshIndexType::UnsignedShort);
const Int vertexData[13]{}; CORRADE_COMPARE_AS(copy.indices<UnsignedShort>(),
/* Verify that custom vertex formats are propagated without a problem */ Containers::arrayView(indices),
Trade::MeshData weirdThing{MeshPrimitive::Faces, TestSuite::Compare::Container);
{}, vertexData,
{Trade::MeshAttributeData{Trade::meshAttributeCustom(42), vertexFormatWrap(0xcaca), Containers::arrayView(vertexData)}}};
Trade::MeshData copy = MeshTools::copy(weirdThing); CORRADE_COMPARE(copy.vertexCount(), 2);
CORRADE_COMPARE(copy.vertexCount(), 13);
CORRADE_COMPARE(copy.attributeCount(), 1); CORRADE_COMPARE(copy.attributeCount(), 1);
CORRADE_COMPARE(copy.attributeArraySize(0), 0); CORRADE_COMPARE_AS(copy.attribute<Vector3>(Trade::MeshAttribute::Position),
CORRADE_COMPARE(copy.attributeFormat(0), vertexFormatWrap(0xcaca)); Containers::arrayView(positions),
CORRADE_COMPARE_AS((Containers::arrayCast<1, const Int>(copy.attribute(0))),
Containers::arrayView(vertexData),
TestSuite::Compare::Container); TestSuite::Compare::Container);
/* Nothing should be copied in this case */
CORRADE_VERIFY(copy.indexData().data() != static_cast<const void*>(indices));
CORRADE_VERIFY(copy.vertexData().data() != static_cast<void*>(positions));
CORRADE_VERIFY(copy.attributeData().data() != attributes);
} }
void CopyTest::copyRvaluePassthrough() { void CopyTest::copyRvalueIndicesVerticesAttributesOwned() {
Trade::MeshData grid = Primitives::grid3DSolid({15, 3}, Primitives::GridFlag::Tangents); Containers::Array<char> vertexData{NoInit, sizeof(Vector3)*2};
CORRADE_COMPARE(grid.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); auto positions = Containers::arrayCast<Vector3>(vertexData);
CORRADE_COMPARE(grid.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); Utility::copy({
UnsignedInt indexCount = grid.indexCount(); {1.0f, 2.0f, 3.0f},
MeshIndexType indexType = grid.indexType(); {6.0f, 7.0f, 8.0f},
std::size_t indexOffset = grid.indexOffset(); }, positions);
Int indexStride = grid.indexStride();
UnsignedInt vertexCount = grid.vertexCount(); Containers::Array<char> indexData{NoInit, sizeof(UnsignedShort)*3};
const void* indexData = grid.indexData(); auto indices = Containers::arrayCast<UnsignedShort>(indexData);
const void* vertexData = grid.vertexData(); Utility::copy({
const void* attributeData = grid.attributeData(); 1, 0, 1
}, indices);
Trade::MeshData copy = MeshTools::copy(std::move(grid));
/* InPlaceInit causes a non-default deleter to be used, which would cause
a copy to be made internally */
Containers::Array<Trade::MeshAttributeData> attributes{ValueInit, 1};
attributes[0] = Trade::MeshAttributeData{Trade::MeshAttribute::Position,
Containers::arrayView(positions)};
const Trade::MeshAttributeData* originalAttributes = attributes;
Trade::MeshData copy = MeshTools::copy(Trade::MeshData{MeshPrimitive::Triangles,
std::move(indexData), Trade::MeshIndexData{indices},
std::move(vertexData), std::move(attributes)});
CORRADE_VERIFY(copy.isIndexed()); CORRADE_VERIFY(copy.isIndexed());
CORRADE_COMPARE(copy.primitive(), MeshPrimitive::Triangles);
CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.indexCount(), indexCount);
CORRADE_COMPARE(copy.indexType(), indexType);
CORRADE_COMPARE(copy.indexOffset(), indexOffset);
CORRADE_COMPARE(copy.indexStride(), indexStride);
CORRADE_COMPARE(copy.vertexCount(), vertexCount);
CORRADE_COMPARE(copy.indexData(), indexData);
CORRADE_COMPARE(copy.vertexData(), vertexData);
CORRADE_COMPARE(copy.attributeData(), attributeData);
}
void CopyTest::copyRvaluePartialPassthrough() { CORRADE_COMPARE(copy.indexType(), MeshIndexType::UnsignedShort);
Trade::MeshData gradient = Primitives::gradient3DHorizontal({}, {}); CORRADE_COMPARE_AS(copy.indices<UnsignedShort>(),
CORRADE_COMPARE(gradient.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); Containers::arrayView(indices),
UnsignedInt vertexCount = gradient.vertexCount(); TestSuite::Compare::Container);
const void* vertexData = gradient.vertexData();
const void* attributeData = gradient.attributeData(); CORRADE_COMPARE(copy.vertexCount(), 2);
CORRADE_COMPARE(copy.attributeCount(), 1);
CORRADE_COMPARE_AS(copy.attribute<Vector3>(Trade::MeshAttribute::Position),
Containers::arrayView(positions),
TestSuite::Compare::Container);
Trade::MeshData copy = MeshTools::copy(std::move(gradient)); /* All data should be transferred without any copy */
CORRADE_VERIFY(!copy.isIndexed()); CORRADE_COMPARE(copy.indexData().data(), static_cast<const void*>(indices));
CORRADE_COMPARE(copy.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.vertexData().data(), static_cast<void*>(positions));
CORRADE_COMPARE(copy.attributeData().data(), originalAttributes);
}
void CopyTest::copyRvalueAttributesOwned() {
Vector3 positions[]{
{1.0f, 2.0f, 3.0f},
{6.0f, 7.0f, 8.0f},
};
/* InPlaceInit causes a non-default deleter to be used, which would cause
a copy to be made internally */
Containers::Array<Trade::MeshAttributeData> attributes{ValueInit, 1};
attributes[0] = Trade::MeshAttributeData{Trade::MeshAttribute::Position,
Containers::arrayView(positions)};
const Trade::MeshAttributeData* originalAttributes = attributes;
Trade::MeshData copy = MeshTools::copy(Trade::MeshData{MeshPrimitive::Triangles, {}, positions, std::move(attributes)});
CORRADE_COMPARE(copy.primitive(), MeshPrimitive::Triangles);
CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); CORRADE_COMPARE(copy.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
CORRADE_COMPARE(copy.vertexCount(), vertexCount);
CORRADE_COMPARE(copy.vertexData(), vertexData); CORRADE_COMPARE(copy.vertexCount(), 2);
/* Attribute data is constant in the original, so this gets copied */ CORRADE_COMPARE(copy.attributeCount(), 1);
CORRADE_VERIFY(copy.attributeData() != attributeData); CORRADE_COMPARE_AS(copy.attribute<Vector3>(Trade::MeshAttribute::Position),
Containers::arrayView(positions),
TestSuite::Compare::Container);
/* Data should be copied */
CORRADE_VERIFY(copy.vertexData().data() != static_cast<void*>(positions));
{
CORRADE_EXPECT_FAIL("Attribute data currently get copied always when they need to be modified.");
CORRADE_COMPARE(copy.attributeData().data(), originalAttributes);
}
} }
void CopyTest::reference() { void CopyTest::reference() {

Write Preview
Loading…
Cancel
Save