From 49e093674eeacbffc2fcfe4204c137e724a55788 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 24 Jan 2023 12:26:05 +0100 Subject: [PATCH] MeshTools: check minimal vertex count in generateIndices() too. Before it was checked only inside generate*IndicesInto() the function delegates to, which was too late as the arrays would be allocated with an insanely high size at that point. --- src/Magnum/MeshTools/GenerateIndices.cpp | 20 ++++++++++-- src/Magnum/MeshTools/GenerateIndices.h | 8 +++-- .../MeshTools/Test/GenerateIndicesTest.cpp | 32 +++++++++++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/Magnum/MeshTools/GenerateIndices.cpp b/src/Magnum/MeshTools/GenerateIndices.cpp index f68ff66c9..359640e88 100644 --- a/src/Magnum/MeshTools/GenerateIndices.cpp +++ b/src/Magnum/MeshTools/GenerateIndices.cpp @@ -275,10 +275,25 @@ Trade::MeshData generateIndices(Trade::MeshData&& data) { "MeshTools::generateIndices(): mesh data already indexed", (Trade::MeshData{MeshPrimitive::Triangles, 0})); + const UnsignedInt vertexCount = data.vertexCount(); + #ifndef CORRADE_NO_ASSERT + UnsignedInt minVertexCount; + if(data.primitive() == MeshPrimitive::LineStrip || + data.primitive() == MeshPrimitive::LineLoop) { + minVertexCount = 2; + } else if(data.primitive() == MeshPrimitive::TriangleStrip || + data.primitive() == MeshPrimitive::TriangleFan) { + minVertexCount = 3; + } else CORRADE_ASSERT_UNREACHABLE("MeshTools::generateIndices(): invalid primitive" << data.primitive(), + (Trade::MeshData{MeshPrimitive::Triangles, 0})); + CORRADE_ASSERT(vertexCount >= minVertexCount, + "MeshTools::generateIndices(): expected at least" << minVertexCount << "vertices for" << data.primitive() << Debug::nospace << ", got" << vertexCount, + (Trade::MeshData{MeshPrimitive::Triangles, 0})); + #endif + /* Transfer vertex / attribute data as-is, as those don't need any changes. Release if possible. */ Containers::Array vertexData; - const UnsignedInt vertexCount = data.vertexCount(); if(data.vertexDataFlags() & Trade::DataFlag::Owned) vertexData = data.releaseVertexData(); else { @@ -316,8 +331,7 @@ Trade::MeshData generateIndices(Trade::MeshData&& data) { primitive = MeshPrimitive::Triangles; indexData = Containers::Array{NoInit, 3*(vertexCount - 2)*sizeof(UnsignedInt)}; generateTriangleFanIndicesInto(vertexCount, Containers::arrayCast(indexData)); - } else CORRADE_ASSERT_UNREACHABLE("MeshTools::generateIndices(): invalid primitive" << data.primitive(), - (Trade::MeshData{MeshPrimitive::Triangles, 0})); + } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ Trade::MeshIndexData indices{MeshIndexType::UnsignedInt, indexData}; return Trade::MeshData{primitive, std::move(indexData), indices, diff --git a/src/Magnum/MeshTools/GenerateIndices.h b/src/Magnum/MeshTools/GenerateIndices.h index dcc215c32..148f3de71 100644 --- a/src/Magnum/MeshTools/GenerateIndices.h +++ b/src/Magnum/MeshTools/GenerateIndices.h @@ -211,10 +211,12 @@ 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 and is one of +Expects that @p mesh is not indexed, is one of @ref MeshPrimitive::LineStrip, @ref MeshPrimitive::LineLoop, -@ref MeshPrimitive::TriangleStrip, @ref MeshPrimitive::TriangleFan primitives, -calling one of @ref generateLineStripIndices(), @ref generateLineLoopIndices(), +@ref MeshPrimitive::TriangleStrip, @ref MeshPrimitive::TriangleFan primitives +and has 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(), @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) diff --git a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp index cb554d88b..5bbb1f3d5 100644 --- a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include "Magnum/Math/Matrix4.h" #include "Magnum/MeshTools/GenerateIndices.h" @@ -70,6 +71,7 @@ struct GenerateIndicesTest: TestSuite::Tester { void generateIndicesMeshDataNoAttributes(); void generateIndicesMeshDataIndexed(); void generateIndicesMeshDataInvalidPrimitive(); + void generateIndicesMeshDataInvalidVertexCount(); }; using namespace Math::Literals; @@ -160,6 +162,16 @@ const struct { }}} }; +const struct { + MeshPrimitive primitive; + UnsignedInt invalidVertexCount, expectedVertexCount; +} MeshDataInvalidVertexCountData[] { + {MeshPrimitive::LineStrip, 1, 2}, + {MeshPrimitive::LineLoop, 1, 2}, + {MeshPrimitive::TriangleStrip, 2, 3}, + {MeshPrimitive::TriangleFan, 2, 3} +}; + GenerateIndicesTest::GenerateIndicesTest() { addTests({&GenerateIndicesTest::primitiveCount, &GenerateIndicesTest::primitiveCountInvalidPrimitive, @@ -200,6 +212,9 @@ GenerateIndicesTest::GenerateIndicesTest() { &GenerateIndicesTest::generateIndicesMeshDataNoAttributes, &GenerateIndicesTest::generateIndicesMeshDataIndexed, &GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive}); + + addInstancedTests({&GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount}, + Containers::arraySize(MeshDataInvalidVertexCountData)); } void GenerateIndicesTest::primitiveCount() { @@ -733,6 +748,23 @@ void GenerateIndicesTest::generateIndicesMeshDataInvalidPrimitive() { "MeshTools::generateIndices(): invalid primitive MeshPrimitive::Triangles\n"); } +void GenerateIndicesTest::generateIndicesMeshDataInvalidVertexCount() { + auto&& data = MeshDataInvalidVertexCountData[testCaseInstanceId()]; + std::ostringstream primitiveName; + Debug{&primitiveName, Debug::Flag::NoNewlineAtTheEnd} << data.primitive; + setTestCaseDescription(primitiveName.str()); + + CORRADE_SKIP_IF_NO_ASSERT(); + + Trade::MeshData mesh{data.primitive, data.invalidVertexCount}; + + std::ostringstream out; + Error redirectError{&out}; + generateIndices(mesh); + CORRADE_COMPARE(out.str(), Utility::formatString( + "MeshTools::generateIndices(): expected at least {} vertices for {}, got {}\n", data.expectedVertexCount, primitiveName.str(), data.invalidVertexCount)); +} + }}}} CORRADE_TEST_MAIN(Magnum::MeshTools::Test::GenerateIndicesTest)