From ce2ca47a81e127958c993223dbad65bd9802c526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 11 Jan 2022 23:04:38 +0100 Subject: [PATCH] MeshTools: assert on impl-specific vert format in removeDuplicates*(). It was already done in removeDuplicatesFuzzy(), so just make it consistent with the rest (shorter message, listing also the offending attribute index). --- src/Magnum/MeshTools/RemoveDuplicates.cpp | 10 ++++- src/Magnum/MeshTools/RemoveDuplicates.h | 4 +- .../MeshTools/Test/RemoveDuplicatesTest.cpp | 37 ++++++++++++++----- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/Magnum/MeshTools/RemoveDuplicates.cpp b/src/Magnum/MeshTools/RemoveDuplicates.cpp index 9e5fe1d84..e5581124f 100644 --- a/src/Magnum/MeshTools/RemoveDuplicates.cpp +++ b/src/Magnum/MeshTools/RemoveDuplicates.cpp @@ -407,6 +407,14 @@ Trade::MeshData removeDuplicates(Trade::MeshData&& data) { CORRADE_ASSERT(data.attributeCount(), "MeshTools::removeDuplicates(): can't remove duplicates in an attributeless mesh", (Trade::MeshData{MeshPrimitive::Points, 0})); + #ifndef CORRADE_NO_ASSERT + for(std::size_t i = 0; i != data.attributeCount(); ++i) { + const VertexFormat format = data.attributeFormat(i); + CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), + "MeshTools::removeDuplicates(): attribute" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), + (Trade::MeshData{MeshPrimitive::Points, 0})); + } + #endif /* Turn the passed data into an interleaved owned mutable instance we can operate on -- owned() alone only makes the data owned, interleave() @@ -487,7 +495,7 @@ Trade::MeshData removeDuplicatesFuzzy(const Trade::MeshData& data, const Float f for(UnsignedInt i = 0; i != owned.attributeCount(); ++i) { const VertexFormat format = owned.attributeFormat(i); CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), - "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), + "MeshTools::removeDuplicatesFuzzy(): attribute" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), (Trade::MeshData{MeshPrimitive::Points, 0})); const Containers::StridedArrayView1D outputIndices = perAttributeIndices[i]; diff --git a/src/Magnum/MeshTools/RemoveDuplicates.h b/src/Magnum/MeshTools/RemoveDuplicates.h index 4f85eaf9a..0c1ff9403 100644 --- a/src/Magnum/MeshTools/RemoveDuplicates.h +++ b/src/Magnum/MeshTools/RemoveDuplicates.h @@ -299,12 +299,14 @@ newly generated index buffer into a new @ref Trade::MeshData instance. If the mesh is indexed, the original index type is preserved, otherwise the mesh gets @ref MeshIndexType::UnsignedInt indices. The resulting mesh is always interleaved and owned, if the input is already interleaved attribute offsets -and paddings are preserved. +and paddings are preserved. All attributes are expected to not have an +implementation-specific format. This function unconditionally copies and interleaves passed vertex and index data in order to operate on them in-place. If your data is interleaved and owned by the instance and you don't need the original data after the process, call @ref removeDuplicates(Trade::MeshData&&) instead to avoid the extra copy. +@see @ref isVertexFormatImplementationSpecific() */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData removeDuplicates(const Trade::MeshData& data); diff --git a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp index 69d2fb905..d1ec60c64 100644 --- a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp +++ b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp @@ -72,11 +72,12 @@ struct RemoveDuplicatesTest: TestSuite::Tester { void removeDuplicatesMeshData(); void removeDuplicatesMeshDataAttributeless(); + void removeDuplicatesMeshDataImplementationSpecificVertexFormat(); void removeDuplicatesMeshDataFuzzy(); void removeDuplicatesMeshDataFuzzyDouble(); void removeDuplicatesMeshDataFuzzyAttributeless(); - void removeDuplicatesMeshDataFuzzyImplementationSpecific(); + void removeDuplicatesMeshDataFuzzyImplementationSpecificVertexFormat(); void soakTest(); void soakTestFuzzy(); @@ -208,7 +209,8 @@ RemoveDuplicatesTest::RemoveDuplicatesTest() { addInstancedTests({&RemoveDuplicatesTest::removeDuplicatesMeshData}, Containers::arraySize(RemoveDuplicatesMeshDataData)); - addTests({&RemoveDuplicatesTest::removeDuplicatesMeshDataAttributeless}); + addTests({&RemoveDuplicatesTest::removeDuplicatesMeshDataAttributeless, + &RemoveDuplicatesTest::removeDuplicatesMeshDataImplementationSpecificVertexFormat}); addInstancedTests({&RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzy}, Containers::arraySize(RemoveDuplicatesMeshDataFuzzyData)); @@ -216,7 +218,7 @@ RemoveDuplicatesTest::RemoveDuplicatesTest() { addTests({&RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyDouble, &RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyAttributeless, - &RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyImplementationSpecific}); + &RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyImplementationSpecificVertexFormat}); addRepeatedTests({&RemoveDuplicatesTest::soakTest, &RemoveDuplicatesTest::soakTestFuzzy}, 10); @@ -726,6 +728,22 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataAttributeless() { "MeshTools::removeDuplicates(): can't remove duplicates in an attributeless mesh\n"); } +void RemoveDuplicatesTest::removeDuplicatesMeshDataImplementationSpecificVertexFormat() { + #ifdef CORRADE_NO_ASSERT + CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); + #endif + + std::ostringstream out; + Error redirectError{&out}; + MeshTools::removeDuplicates(Trade::MeshData{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr} + }}); + CORRADE_COMPARE(out.str(), + "MeshTools::removeDuplicates(): attribute 1 has an implementation-specific format 0xcaca\n"); +} + void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzy() { auto&& data = RemoveDuplicatesMeshDataFuzzyData[testCaseInstanceId()]; setTestCaseDescription(data.name); @@ -1044,19 +1062,20 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyAttributeless() { "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an attributeless mesh\n"); } -void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyImplementationSpecific() { +void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyImplementationSpecificVertexFormat() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions"); #endif std::ostringstream out; Error redirectError{&out}; - - MeshTools::removeDuplicatesFuzzy(Trade::MeshData{MeshPrimitive::Points, - nullptr, {Trade::MeshAttributeData{Trade::MeshAttribute::Position, - vertexFormatWrap(0x1234), nullptr}}}); + MeshTools::removeDuplicatesFuzzy(Trade::MeshData{MeshPrimitive::Points, nullptr, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + VertexFormat::Vector3, nullptr}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, vertexFormatWrap(0xcaca), nullptr} + }}); CORRADE_COMPARE(out.str(), - "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an implementation-specific format 0x1234\n"); + "MeshTools::removeDuplicatesFuzzy(): attribute 1 has an implementation-specific format 0xcaca\n"); } void RemoveDuplicatesTest::soakTest() {