From bf347109c993600d0a94599fd462cdf41db3d60e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 24 Jan 2023 18:40:07 +0100 Subject: [PATCH] MeshTools: make generateIndices() work with indexed MeshData. This way one no longer needs to duplicate() the original first, leading to an enlarged size of vertex data for no reason. --- src/Magnum/MeshTools/GenerateIndices.cpp | 26 ++- src/Magnum/MeshTools/GenerateIndices.h | 18 +- .../MeshTools/Test/GenerateIndicesTest.cpp | 160 +++++++++++++----- 3 files changed, 149 insertions(+), 55 deletions(-) diff --git a/src/Magnum/MeshTools/GenerateIndices.cpp b/src/Magnum/MeshTools/GenerateIndices.cpp index 24423b693..5ab1371f8 100644 --- a/src/Magnum/MeshTools/GenerateIndices.cpp +++ b/src/Magnum/MeshTools/GenerateIndices.cpp @@ -577,9 +577,9 @@ void generateQuadIndicesInto(const Containers::StridedArrayView1D } Trade::MeshData generateIndices(Trade::MeshData&& data) { - CORRADE_ASSERT(!data.isIndexed(), - "MeshTools::generateIndices(): mesh data already indexed", - (Trade::MeshData{MeshPrimitive::Triangles, 0})); + CORRADE_ASSERT(!data.isIndexed() || !isMeshIndexTypeImplementationSpecific(data.indexType()), + "MeshTools::generateIndices(): mesh has an implementation-specific index type" << reinterpret_cast(meshIndexTypeUnwrap(data.indexType())), + (Trade::MeshData{MeshPrimitive{}, 0})); const UnsignedInt vertexCount = data.vertexCount(); #ifndef CORRADE_NO_ASSERT @@ -624,19 +624,31 @@ Trade::MeshData generateIndices(Trade::MeshData&& data) { if(data.primitive() == MeshPrimitive::LineStrip) { primitive = MeshPrimitive::Lines; indexData = Containers::Array{NoInit, 2*(Math::max(vertexCount, 1u) - 1)*sizeof(UnsignedInt)}; - generateLineStripIndicesInto(vertexCount, Containers::arrayCast(indexData)); + if(data.isIndexed()) + generateLineStripIndicesInto(data.indices(), Containers::arrayCast(indexData)); + else + generateLineStripIndicesInto(vertexCount, Containers::arrayCast(indexData)); } else if(data.primitive() == MeshPrimitive::LineLoop) { primitive = MeshPrimitive::Lines; indexData = Containers::Array{NoInit, 2*vertexCount*sizeof(UnsignedInt)}; - generateLineLoopIndicesInto(vertexCount, Containers::arrayCast(indexData)); + if(data.isIndexed()) + generateLineLoopIndicesInto(data.indices(), Containers::arrayCast(indexData)); + else + generateLineLoopIndicesInto(vertexCount, Containers::arrayCast(indexData)); } else if(data.primitive() == MeshPrimitive::TriangleStrip) { primitive = MeshPrimitive::Triangles; indexData = Containers::Array{NoInit, 3*(Math::max(vertexCount, 2u) - 2)*sizeof(UnsignedInt)}; - generateTriangleStripIndicesInto(vertexCount, Containers::arrayCast(indexData)); + if(data.isIndexed()) + generateTriangleStripIndicesInto(data.indices(), Containers::arrayCast(indexData)); + else + generateTriangleStripIndicesInto(vertexCount, Containers::arrayCast(indexData)); } else if(data.primitive() == MeshPrimitive::TriangleFan) { primitive = MeshPrimitive::Triangles; indexData = Containers::Array{NoInit, 3*(Math::max(vertexCount, 2u) - 2)*sizeof(UnsignedInt)}; - generateTriangleFanIndicesInto(vertexCount, Containers::arrayCast(indexData)); + if(data.isIndexed()) + generateTriangleFanIndicesInto(data.indices(), Containers::arrayCast(indexData)); + else + generateTriangleFanIndicesInto(vertexCount, Containers::arrayCast(indexData)); } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ Trade::MeshIndexData indices{MeshIndexType::UnsignedInt, indexData}; diff --git a/src/Magnum/MeshTools/GenerateIndices.h b/src/Magnum/MeshTools/GenerateIndices.h index ff23b459c..059bbcb4a 100644 --- a/src/Magnum/MeshTools/GenerateIndices.h +++ b/src/Magnum/MeshTools/GenerateIndices.h @@ -484,22 +484,22 @@ MAGNUM_MESHTOOLS_EXPORT void generateQuadIndicesInto(const Containers::StridedAr @brief Convert a mesh to plain indexed lines or triangles @m_since{2020,06} -Expects that @p mesh is not indexed, is one of -@ref MeshPrimitive::LineStrip, @ref MeshPrimitive::LineLoop, -@ref MeshPrimitive::TriangleStrip, @ref MeshPrimitive::TriangleFan primitives -and has either @cpp 0 @ce vertices or at least @cpp 2 @ce vertices for a -line-based primitive or @cpp 3 @ce vertices for a triangle-based primitive. -Calls one of @ref generateLineStripIndices(), @ref generateLineLoopIndices(), +Expects that @p mesh is one of @ref MeshPrimitive::LineStrip, +@ref MeshPrimitive::LineLoop, @ref MeshPrimitive::TriangleStrip or +@ref MeshPrimitive::TriangleFan primitives and has either @cpp 0 @ce vertices +or at least @cpp 2 @ce vertices for a line-based primitive and @cpp 3 @ce +vertices for a triangle-based primitive. If it's indexed, the index type is +expected to be non-implementation-specific. Calls one of +@ref generateLineStripIndices(), @ref generateLineLoopIndices(), @ref generateTriangleStripIndices() or @ref generateTriangleFanIndices() -functions to generate the index buffer. If your mesh is indexed, call -@ref duplicate(const Trade::MeshData& data, Containers::ArrayView) -on it first. +functions or their indexed overloads to generate the index buffer. The resulting mesh always has @ref MeshIndexType::UnsignedInt, call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result to compress it to a smaller type, if desired. This function will unconditionally make a copy of all vertex data, use @ref generateIndices(Trade::MeshData&&) to avoid that copy. +@see @ref isMeshIndexTypeImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData generateIndices(const Trade::MeshData& mesh); diff --git a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp index b8e97570e..5b6519428 100644 --- a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp @@ -80,12 +80,13 @@ struct GenerateIndicesTest: TestSuite::Tester { void generateQuadIndicesIntoWrongSize(); void generateIndicesMeshData(); + template void generateIndicesMeshDataIndexed(); void generateIndicesMeshDataEmpty(); void generateIndicesMeshDataMove(); void generateIndicesMeshDataNoAttributes(); - void generateIndicesMeshDataIndexed(); void generateIndicesMeshDataInvalidPrimitive(); void generateIndicesMeshDataInvalidVertexCount(); + void generateIndicesMeshDataImplementationSpecificIndexType(); }; using namespace Math::Literals; @@ -150,30 +151,50 @@ const struct { const struct { MeshPrimitive primitive; Containers::Array expectedIndices; + Containers::Array expectedIndexedIndices; } MeshDataData[] { {MeshPrimitive::LineStrip, {InPlaceInit, { - 0, 1, - 1, 2, - 2, 3, - 3, 4 - }}}, + 0, 1, + 1, 2, + 2, 3, + 3, 4 + }}, {InPlaceInit, { + 60, 21, + 21, 72, + 72, 93, + 93, 44 + }}}, {MeshPrimitive::LineLoop, {InPlaceInit, { - 0, 1, - 1, 2, - 2, 3, - 3, 4, - 4, 0 - }}}, + 0, 1, + 1, 2, + 2, 3, + 3, 4, + 4, 0 + }}, {InPlaceInit, { + 60, 21, + 21, 72, + 72, 93, + 93, 44, + 44, 60 + }}}, {MeshPrimitive::TriangleStrip, {InPlaceInit, { - 0, 1, 2, - 2, 1, 3, /* Reversed */ - 2, 3, 4 - }}}, + 0, 1, 2, + 2, 1, 3, /* Reversed */ + 2, 3, 4 + }}, {InPlaceInit, { + 60, 21, 72, + 72, 21, 93, /* Reversed */ + 72, 93, 44 + }}}, {MeshPrimitive::TriangleFan, {InPlaceInit, { - 0, 1, 2, - 0, 2, 3, - 0, 3, 4 - }}} + 0, 1, 2, + 0, 2, 3, + 0, 3, 4 + }}, {InPlaceInit, { + 60, 21, 72, + 60, 72, 93, + 60, 93, 44 + }}} }; const struct { @@ -249,16 +270,20 @@ GenerateIndicesTest::GenerateIndicesTest() { &GenerateIndicesTest::generateQuadIndicesIntoWrongSize}); addInstancedTests({&GenerateIndicesTest::generateIndicesMeshData, + &GenerateIndicesTest::generateIndicesMeshDataIndexed, + &GenerateIndicesTest::generateIndicesMeshDataIndexed, + &GenerateIndicesTest::generateIndicesMeshDataIndexed, &GenerateIndicesTest::generateIndicesMeshDataEmpty}, Containers::arraySize(MeshDataData)); addTests({&GenerateIndicesTest::generateIndicesMeshDataMove, &GenerateIndicesTest::generateIndicesMeshDataNoAttributes, - &GenerateIndicesTest::generateIndicesMeshDataIndexed, &GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive}); addInstancedTests({&GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount}, Containers::arraySize(MeshDataInvalidVertexCountData)); + + addTests({&GenerateIndicesTest::generateIndicesMeshDataImplementationSpecificIndexType}); } void GenerateIndicesTest::primitiveCount() { @@ -1089,6 +1114,68 @@ void GenerateIndicesTest::generateIndicesMeshData() { }), TestSuite::Compare::Container); } +template void GenerateIndicesTest::generateIndicesMeshDataIndexed() { + auto&& data = MeshDataData[testCaseInstanceId()]; + setTestCaseTemplateName(Math::TypeTraits::name()); + { + std::ostringstream out; + Debug{&out, Debug::Flag::NoNewlineAtTheEnd} << data.primitive; + setTestCaseDescription(out.str()); + } + + const struct Vertex { + Vector2 position; + Short data[2]; + Vector2 textureCoordinates; + } vertexData[] { + {{1.5f, 0.3f}, {28, -15}, {0.2f, 0.8f}}, + {{2.5f, 1.3f}, {29, -16}, {0.3f, 0.7f}}, + {{3.5f, 2.3f}, {30, -17}, {0.4f, 0.6f}}, + {{4.5f, 3.3f}, {40, -18}, {0.5f, 0.5f}}, + {{5.5f, 4.3f}, {41, -19}, {0.6f, 0.4f}} + }; + Containers::StridedArrayView1D vertices = vertexData; + + const T indexData[]{60, 21, 72, 93, 44}; + + Trade::MeshData mesh{data.primitive, + {}, indexData, Trade::MeshIndexData{indexData}, + {}, vertexData, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + vertices.slice(&Vertex::position)}, + /* Array attribute to verify it's correctly propagated */ + Trade::MeshAttributeData{Trade::meshAttributeCustom(42), + VertexFormat::Short, vertices.slice(&Vertex::data), 2}, + Trade::MeshAttributeData{Trade::MeshAttribute::TextureCoordinates, + vertices.slice(&Vertex::textureCoordinates)} + }}; + + Trade::MeshData out = generateIndices(mesh); + CORRADE_VERIFY(out.isIndexed()); + CORRADE_COMPARE(out.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(out.indices(), data.expectedIndexedIndices, + TestSuite::Compare::Container); + + CORRADE_COMPARE(out.attributeCount(), 3); + CORRADE_COMPARE_AS(out.attribute(Trade::MeshAttribute::Position), + Containers::arrayView({ + {1.5f, 0.3f}, {2.5f, 1.3f}, {3.5f, 2.3f}, {4.5f, 3.3f}, {5.5f, 4.3f} + }), TestSuite::Compare::Container); + + CORRADE_COMPARE(out.attributeName(1), Trade::meshAttributeCustom(42)); + CORRADE_COMPARE(out.attributeFormat(1), VertexFormat::Short); + CORRADE_COMPARE(out.attributeArraySize(1), 2); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector2s>(out.attribute(1))), + Containers::arrayView({ + {28, -15}, {29, -16}, {30, -17}, {40, -18}, {41, -19} + }), TestSuite::Compare::Container); + + CORRADE_COMPARE_AS(out.attribute(Trade::MeshAttribute::TextureCoordinates), + Containers::arrayView({ + {0.2f, 0.8f}, {0.3f, 0.7f}, {0.4f, 0.6f}, {0.5f, 0.5f}, {0.6f, 0.4f} + }), TestSuite::Compare::Container); +} + void GenerateIndicesTest::generateIndicesMeshDataEmpty() { auto&& data = MeshDataData[testCaseInstanceId()]; { @@ -1189,24 +1276,6 @@ void GenerateIndicesTest::generateIndicesMeshDataNoAttributes() { CORRADE_COMPARE(out.attributeCount(), 0); } -void GenerateIndicesTest::generateIndicesMeshDataIndexed() { - CORRADE_SKIP_IF_NO_ASSERT(); - - UnsignedByte indices[]{0}; - Trade::MeshData mesh{MeshPrimitive::TriangleFan, - {}, indices, Trade::MeshIndexData{indices}, 0}; - - /* Test both r-value and l-value overload */ - std::ostringstream out; - Error redirectError{&out}; - generateIndices(mesh); - generateIndices(Trade::MeshData{MeshPrimitive::TriangleFan, - {}, indices, Trade::MeshIndexData{indices}, 0}); - CORRADE_COMPARE(out.str(), - "MeshTools::generateIndices(): mesh data already indexed\n" - "MeshTools::generateIndices(): mesh data already indexed\n"); -} - void GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -1236,6 +1305,19 @@ void GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount() { "MeshTools::generateIndices(): expected either zero or at least {} vertices for {}, got {}\n", data.expectedVertexCount, primitiveName.str(), data.invalidVertexCount)); } +void GenerateIndicesTest::generateIndicesMeshDataImplementationSpecificIndexType() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Trade::MeshData a{MeshPrimitive::LineLoop, + nullptr, Trade::MeshIndexData{meshIndexTypeWrap(0xcaca), Containers::StridedArrayView1D{}}, + nullptr, {}, 3}; + + std::ostringstream out; + Error redirectError{&out}; + generateIndices(a); + CORRADE_COMPARE(out.str(), "MeshTools::generateIndices(): mesh has an implementation-specific index type 0xcaca\n"); +} + }}}} CORRADE_TEST_MAIN(Magnum::MeshTools::Test::GenerateIndicesTest)