From 6afdfc55ffdaa2136e2f9a433b9239ef83e4d54b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 24 Jan 2023 13:00:19 +0100 Subject: [PATCH] MeshTools: add assertions for element count to primitiveCount() as well. To be consistent with what the generate*Indices() APIs expect -- it doesn't make sense for this API to silently round down while the other would fail for the same input. In particular, the primitiveCount() may be used to calculate allocation size for an array to pass to generate*IndicesInto(), and thus it should use the same rules. --- doc/changelog.dox | 4 +++ src/Magnum/MeshTools/GenerateIndices.cpp | 26 +++++++++++++++ src/Magnum/MeshTools/GenerateIndices.h | 7 ++-- .../MeshTools/Test/GenerateIndicesTest.cpp | 33 +++++++++++++++---- 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 7fd25d432..c0843866f 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -1279,6 +1279,10 @@ See also: @ref Math::RectangularMatrix::data() are no longer @cpp constexpr @ce in order to make them return a reference to a fixed-size array instead of a pointer, which was deemed a more useful property. +- @ref MeshTools::primitiveCount() now requires the element count to follow + rules defined by a particular primitive to be consistent with requirements + of @ref MeshTools::generateIndices() and related APIs. Before it was just + rounding down to the nearest lower primitive count. - @cpp Platform::WindowlessWindowsEglApplication @ce is now merged into @ref Platform::WindowlessEglApplication. Since its use case was rather rare (windowless applications on ANGLE on Windows) and it wasn't even built in diff --git a/src/Magnum/MeshTools/GenerateIndices.cpp b/src/Magnum/MeshTools/GenerateIndices.cpp index 1422ca359..d6d61323a 100644 --- a/src/Magnum/MeshTools/GenerateIndices.cpp +++ b/src/Magnum/MeshTools/GenerateIndices.cpp @@ -36,6 +36,32 @@ namespace Magnum { namespace MeshTools { UnsignedInt primitiveCount(const MeshPrimitive primitive, const UnsignedInt elementCount) { + #ifndef CORRADE_NO_ASSERT + UnsignedInt minElementCount; + if(primitive == MeshPrimitive::Lines || + primitive == MeshPrimitive::LineStrip || + primitive == MeshPrimitive::LineLoop) + minElementCount = 2; + else if(primitive == MeshPrimitive::Triangles || + primitive == MeshPrimitive::TriangleStrip || + primitive == MeshPrimitive::TriangleFan) + minElementCount = 3; + else + minElementCount = 1; + CORRADE_ASSERT(elementCount == 0 || elementCount >= minElementCount, + "MeshTools::primitiveCount(): expected either zero or at least" << minElementCount << "elements for" << primitive << Debug::nospace << ", got" << elementCount, {}); + + UnsignedInt elementCountDivisor; + if(primitive == MeshPrimitive::Lines) + elementCountDivisor = 2; + else if(primitive == MeshPrimitive::Triangles) + elementCountDivisor = 3; + else + elementCountDivisor = 1; + CORRADE_ASSERT(elementCount % elementCountDivisor == 0, + "MeshTools::primitiveCount(): expected element count to be divisible by" << elementCountDivisor << "for" << primitive << Debug::nospace << ", got" << elementCount, {}); + #endif + if(primitive == MeshPrimitive::Points || primitive == MeshPrimitive::Edges || primitive == MeshPrimitive::Faces || diff --git a/src/Magnum/MeshTools/GenerateIndices.h b/src/Magnum/MeshTools/GenerateIndices.h index dbcd2532e..6e8c49436 100644 --- a/src/Magnum/MeshTools/GenerateIndices.h +++ b/src/Magnum/MeshTools/GenerateIndices.h @@ -42,9 +42,10 @@ namespace Magnum { namespace MeshTools { Returns how many primitives is generated for given @p primitive and @p elementCount, for example for @ref MeshPrimitive::Triangles returns -@cpp elementCount/3 @ce. Expects that @p primitive is valid, return value is -rounded down if there's not enough elements for given primitive type (so for -14 vertices you get just 4 triangles, for example). +@cpp elementCount/3 @ce. Expects that @p primitive is valid, @p elementCount +is either zero or at least @cpp 2 @ce for a line-based primitive and at least +@cpp 3 @ce for a triangle-based primitive, is divisible by @cpp 2 @ce for +@ref MeshPrimitive::Lines and by @cpp 3 @ce for @ref MeshPrimitive::Triangles. */ MAGNUM_MESHTOOLS_EXPORT UnsignedInt primitiveCount(MeshPrimitive primitive, UnsignedInt elementCount); diff --git a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp index c003a4da6..ca8087881 100644 --- a/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateIndicesTest.cpp @@ -42,6 +42,7 @@ struct GenerateIndicesTest: TestSuite::Tester { explicit GenerateIndicesTest(); void primitiveCount(); + void primitiveCountInvalidVertexCount(); void primitiveCountInvalidPrimitive(); void generateLineStripIndices(); @@ -175,6 +176,7 @@ const struct { GenerateIndicesTest::GenerateIndicesTest() { addTests({&GenerateIndicesTest::primitiveCount, + &GenerateIndicesTest::primitiveCountInvalidVertexCount, &GenerateIndicesTest::primitiveCountInvalidPrimitive, &GenerateIndicesTest::generateLineStripIndices, @@ -224,26 +226,43 @@ void GenerateIndicesTest::primitiveCount() { CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Instances, 13), 13); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Lines, 4), 2); - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Lines, 5), 2); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Lines, 14), 7); - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineStrip, 1), 0); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineStrip, 2), 1); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineStrip, 4), 3); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineStrip, 10), 9); - /* This is a degenerate line, which technically still is a primitive */ - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineLoop, 1), 1); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineLoop, 2), 2); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::LineLoop, 9), 9); - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Triangles, 2), 0); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Triangles, 3), 1); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Triangles, 6), 2); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::Triangles, 21), 7); - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleStrip, 2), 0); - CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleFan, 2), 0); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleStrip, 3), 1); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleFan, 3), 1); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleStrip, 7), 5); CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleFan, 7), 5); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleStrip, 22), 20); + CORRADE_COMPARE(MeshTools::primitiveCount(MeshPrimitive::TriangleFan, 22), 20); +} + +void GenerateIndicesTest::primitiveCountInvalidVertexCount() { + CORRADE_SKIP_IF_NO_ASSERT(); + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::primitiveCount(MeshPrimitive::LineLoop, 1); + MeshTools::primitiveCount(MeshPrimitive::TriangleStrip, 1); + MeshTools::primitiveCount(MeshPrimitive::TriangleFan, 2); + MeshTools::primitiveCount(MeshPrimitive::Lines, 7); + MeshTools::primitiveCount(MeshPrimitive::Triangles, 14); + CORRADE_COMPARE(out.str(), + "MeshTools::primitiveCount(): expected either zero or at least 2 elements for MeshPrimitive::LineLoop, got 1\n" + "MeshTools::primitiveCount(): expected either zero or at least 3 elements for MeshPrimitive::TriangleStrip, got 1\n" + "MeshTools::primitiveCount(): expected either zero or at least 3 elements for MeshPrimitive::TriangleFan, got 2\n" + "MeshTools::primitiveCount(): expected element count to be divisible by 2 for MeshPrimitive::Lines, got 7\n" + "MeshTools::primitiveCount(): expected element count to be divisible by 3 for MeshPrimitive::Triangles, got 14\n"); } void GenerateIndicesTest::primitiveCountInvalidPrimitive() {