diff --git a/doc/changelog.dox b/doc/changelog.dox index 3b1776e3c..5eee0504d 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -731,6 +731,9 @@ See also: @relativeref{MeshTools,generateIndices()} now allow empty input, producing an empty index buffer as a result. Disallowing empty input was an unnecessary restriction that was inconsistent with other APIs. +- @ref MeshTools::generateIndices() now generates a trivial index buffer for + non-loop, non-strip and non-fan primitives, and if such a mesh is already + indexed it simply passes it through - Added overloads to @ref MeshTools::generateLineStripIndices(), @relativeref{MeshTools,generateLineLoopIndices()}, @relativeref{MeshTools,generateTriangleStripIndices()} and diff --git a/src/Magnum/MeshTools/GenerateIndices.cpp b/src/Magnum/MeshTools/GenerateIndices.cpp index e504af8d4..b0529f4e4 100644 --- a/src/Magnum/MeshTools/GenerateIndices.cpp +++ b/src/Magnum/MeshTools/GenerateIndices.cpp @@ -602,8 +602,9 @@ Trade::MeshData generateIndices(Trade::MeshData&& mesh) { } else if(mesh.primitive() == MeshPrimitive::TriangleStrip || mesh.primitive() == MeshPrimitive::TriangleFan) { minVertexCount = 3; - } else CORRADE_ASSERT_UNREACHABLE("MeshTools::generateIndices(): invalid primitive" << mesh.primitive(), - (Trade::MeshData{MeshPrimitive::Triangles, 0})); + } else { + minVertexCount = 1; + } CORRADE_ASSERT(vertexCount == 0 || vertexCount >= minVertexCount, "MeshTools::generateIndices(): expected either zero or at least" << minVertexCount << "vertices for" << mesh.primitive() << Debug::nospace << ", got" << vertexCount, (Trade::MeshData{MeshPrimitive::Triangles, 0})); @@ -658,7 +659,22 @@ Trade::MeshData generateIndices(Trade::MeshData&& mesh) { generateTriangleFanIndicesInto(mesh.indices(), Containers::arrayCast(indexData)); else generateTriangleFanIndicesInto(vertexCount, Containers::arrayCast(indexData)); - } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ + } else { + primitive = mesh.primitive(); + /* If the mesh is indexed, transfer the index data as-is if possible + and it's the right type already */ + if(mesh.isIndexed()) { + if(mesh.indexDataFlags() >= Trade::DataFlag::Owned && mesh.indexType() == MeshIndexType::UnsignedInt) { + indexData = mesh.releaseIndexData(); + } else { + indexData = Containers::Array{NoInit, mesh.indexCount()*sizeof(UnsignedInt)}; + mesh.indicesInto(Containers::arrayCast(indexData)); + } + } else { + indexData = Containers::Array{NoInit, vertexCount*sizeof(UnsignedInt)}; + generateTrivialIndicesInto(Containers::arrayCast(indexData)); + } + } Trade::MeshIndexData indices{MeshIndexType::UnsignedInt, indexData}; return Trade::MeshData{primitive, Utility::move(indexData), indices, diff --git a/src/Magnum/MeshTools/GenerateIndices.h b/src/Magnum/MeshTools/GenerateIndices.h index 96450f783..05c64c165 100644 --- a/src/Magnum/MeshTools/GenerateIndices.h +++ b/src/Magnum/MeshTools/GenerateIndices.h @@ -508,15 +508,21 @@ 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 one of @ref MeshPrimitive::LineStrip, +If @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 MeshPrimitive::TriangleFan primitives, calls one of @ref generateLineStripIndices(), @ref generateLineLoopIndices(), @ref generateTriangleStripIndices() or @ref generateTriangleFanIndices() -functions or their indexed overloads to generate the index buffer. +functions or their indexed overloads to generate the index buffer. In that case +expects that the mesh 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 @p mesh is a different +@relativeref{Magnum,MeshPrimitive}, it's passed through unchanged if already +indexed, and with indices generated using @ref generateTrivialIndices() +otherwise. + +If @p mesh is already indexed, the index type is expected to be +non-implementation-specific. The resulting mesh always has @ref MeshIndexType::UnsignedInt, call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result to @@ -533,8 +539,9 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData generateIndices(const Trade::MeshData& m Compared to @ref generateIndices(const Trade::MeshData&) this function can transfer ownership of @p mesh vertex buffer (in case it is owned) to the -returned instance instead of making a copy of it. Attribute data is copied -always. +returned instance instead of making a copy of it, and index buffer as well if +it's owned, doesn't need expanding and is already with +@ref MeshIndexType::UnsignedInt. Attribute data is copied always. @see @ref Trade::MeshData::vertexDataFlags() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData generateIndices(Trade::MeshData&& mesh); diff --git a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp index 90ee270f8..4f652644f 100644 --- a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -86,7 +87,10 @@ struct GenerateIndicesTest: TestSuite::Tester { void generateIndicesMeshDataEmpty(); void generateIndicesMeshDataMove(); void generateIndicesMeshDataNoAttributes(); - void generateIndicesMeshDataInvalidPrimitive(); + void generateIndicesMeshDataTrivial(); + template void generateIndicesMeshDataTrivialIndexed(); + void generateIndicesMeshDataTrivialIndexedMove(); + void generateIndicesMeshDataTrivialIndexedMoveDifferentIndexType(); void generateIndicesMeshDataInvalidVertexCount(); void generateIndicesMeshDataImplementationSpecificIndexType(); }; @@ -283,7 +287,12 @@ GenerateIndicesTest::GenerateIndicesTest() { addTests({&GenerateIndicesTest::generateIndicesMeshDataMove, &GenerateIndicesTest::generateIndicesMeshDataNoAttributes, - &GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive}); + &GenerateIndicesTest::generateIndicesMeshDataTrivial, + &GenerateIndicesTest::generateIndicesMeshDataTrivialIndexed, + &GenerateIndicesTest::generateIndicesMeshDataTrivialIndexed, + &GenerateIndicesTest::generateIndicesMeshDataTrivialIndexed, + &GenerateIndicesTest::generateIndicesMeshDataTrivialIndexedMove, + &GenerateIndicesTest::generateIndicesMeshDataTrivialIndexedMoveDifferentIndexType}); addInstancedTests({&GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount}, Containers::arraySize(MeshDataInvalidVertexCountData)); @@ -1303,16 +1312,142 @@ void GenerateIndicesTest::generateIndicesMeshDataNoAttributes() { CORRADE_COMPARE(out.attributeCount(), 0); } -void GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive() { - CORRADE_SKIP_IF_NO_ASSERT(); +void GenerateIndicesTest::generateIndicesMeshDataTrivial() { + const Vector2 positions[]{ + {1.5f, 0.3f}, + {2.5f, 1.3f}, + {3.5f, 2.3f}, + {4.5f, 3.3f}, + {5.5f, 4.3f}, + }; - Trade::MeshData mesh{MeshPrimitive::Triangles, 2}; + /* Should work with just any primitive, not just Lines etc */ + Trade::MeshData mesh{MeshPrimitive::Meshlets, {}, positions, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayView(positions)}, + }}; - std::ostringstream out; - Error redirectError{&out}; - generateIndices(mesh); - CORRADE_COMPARE(out.str(), - "MeshTools::generateIndices(): invalid primitive MeshPrimitive::Triangles\n"); + Trade::MeshData out = generateIndices(mesh); + CORRADE_COMPARE(out.primitive(), MeshPrimitive::Meshlets); + CORRADE_VERIFY(out.isIndexed()); + CORRADE_COMPARE(out.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(out.indices(), Containers::arrayView({ + 0, 1, 2, 3, 4 + }), TestSuite::Compare::Container); + + CORRADE_COMPARE(out.attributeCount(), 1); + 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); +} + +template void GenerateIndicesTest::generateIndicesMeshDataTrivialIndexed() { + setTestCaseTemplateName(Math::TypeTraits::name()); + + const Vector2 positions[]{ + {1.5f, 0.3f}, + {2.5f, 1.3f}, + {3.5f, 2.3f}, + {4.5f, 3.3f}, + {5.5f, 4.3f}, + }; + + const T indices[]{60, 21, 72, 93}; + + /* Should work with just any primitive, not just Lines etc */ + Trade::MeshData mesh{MeshPrimitive::Meshlets, + {}, indices, Trade::MeshIndexData{indices}, + {}, positions, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayView(positions)}, + }}; + + Trade::MeshData out = generateIndices(mesh); + CORRADE_COMPARE(out.primitive(), MeshPrimitive::Meshlets); + CORRADE_VERIFY(out.isIndexed()); + CORRADE_COMPARE(out.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(out.indices(), Containers::arrayView({ + 60, 21, 72, 93 + }), TestSuite::Compare::Container); + + CORRADE_COMPARE(out.attributeCount(), 1); + 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); +} + +void GenerateIndicesTest::generateIndicesMeshDataTrivialIndexedMove() { + const Vector2 positions[]{ + {1.5f, 0.3f}, + {2.5f, 1.3f}, + {3.5f, 2.3f}, + {4.5f, 3.3f}, + {5.5f, 4.3f}, + }; + + Containers::Array indexData{4*sizeof(UnsignedInt)}; + Containers::ArrayView indices = Containers::arrayCast(indexData); + Utility::copy({60, 21, 72, 93}, indices); + + Trade::MeshData out = generateIndices(Trade::MeshData{meshPrimitiveWrap(0xcaca), + Utility::move(indexData), Trade::MeshIndexData{indices}, + {}, positions, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayView(positions)}, + }}); + CORRADE_COMPARE(out.primitive(), meshPrimitiveWrap(0xcaca)); + CORRADE_VERIFY(out.isIndexed()); + CORRADE_COMPARE(out.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(out.indices(), Containers::arrayView({ + 60, 21, 72, 93 + }), TestSuite::Compare::Container); + + /* The index data should be moved, not copied */ + CORRADE_COMPARE(out.indexData().data(), static_cast(indices.data())); + + CORRADE_COMPARE(out.attributeCount(), 1); + 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); +} + +void GenerateIndicesTest::generateIndicesMeshDataTrivialIndexedMoveDifferentIndexType() { + const Vector2 positions[]{ + {1.5f, 0.3f}, + {2.5f, 1.3f}, + {3.5f, 2.3f}, + {4.5f, 3.3f}, + {5.5f, 4.3f}, + }; + + Containers::Array indexData{4*sizeof(UnsignedShort)}; + Containers::ArrayView indices = Containers::arrayCast(indexData); + Utility::copy({60, 21, 72, 93}, indices); + + Trade::MeshData out = generateIndices(Trade::MeshData{MeshPrimitive::Points, + Utility::move(indexData), Trade::MeshIndexData{indices}, + {}, positions, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayView(positions)}, + }}); + CORRADE_COMPARE(out.primitive(), MeshPrimitive::Points); + CORRADE_VERIFY(out.isIndexed()); + CORRADE_COMPARE(out.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(out.indices(), Containers::arrayView({ + 60, 21, 72, 93 + }), TestSuite::Compare::Container); + + /* In this case it has to be copied because the type is different */ + CORRADE_VERIFY(out.indexData().data() != static_cast(indices.data())); + + CORRADE_COMPARE(out.attributeCount(), 1); + 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); } void GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount() {