diff --git a/doc/changelog.dox b/doc/changelog.dox index ab78c2625..1f4e8054c 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -155,9 +155,10 @@ See also: - Added @ref MeshTools::interleavedLayout() for convenient creation of an interleaved mesh layout using the new @ref Trade::MeshData API - Added @ref MeshTools::interleave(const Trade::MeshData&, Containers::ArrayView), - @ref MeshTools::duplicate(const Trade::MeshData&, Containers::ArrayView) - and @ref MeshTools::compressIndices(const Trade::MeshData&, MeshIndexType) - that work directly on the new @ref Trade::MeshData API + @ref MeshTools::duplicate(const Trade::MeshData&, Containers::ArrayView), + @ref MeshTools::compressIndices(const Trade::MeshData&, MeshIndexType) + and @ref MeshTools::removeDuplicates(const Trade::MeshData&) that work + directly on the new @ref Trade::MeshData API - Added @ref MeshTools::subdivideInPlace() for allocation-less mesh subdivision - New @ref MeshTools::removeDuplicatesInPlace() variant that works on diff --git a/src/Magnum/MeshTools/RemoveDuplicates.cpp b/src/Magnum/MeshTools/RemoveDuplicates.cpp index b4e81f993..bda4f18d9 100644 --- a/src/Magnum/MeshTools/RemoveDuplicates.cpp +++ b/src/Magnum/MeshTools/RemoveDuplicates.cpp @@ -29,6 +29,10 @@ #include #include +#include "Magnum/MeshTools/Concatenate.h" +#include "Magnum/MeshTools/Interleave.h" +#include "Magnum/Trade/MeshData.h" + namespace Magnum { namespace MeshTools { struct ArrayEqual { @@ -169,4 +173,67 @@ std::size_t removeDuplicatesIndexedInPlace(const Containers::StridedArrayView2D< } } +Trade::MeshData removeDuplicates(const Trade::MeshData& data) { + return removeDuplicates(Trade::MeshData{data.primitive(), + {}, data.indexData(), Trade::MeshIndexData{data.indices()}, + {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()), + data.vertexCount()}); +} + +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})); + + /* Turn the passed data into an interleaved owned mutable instance we can + operate on -- concatenate() alone only makes the data owned, + interleave() alone only makes the data interleaved (but those can stay + non-owned). There's a chance the original data are already like this, in + which case this will be just a passthrough. */ + /** @todo concatenate() causes the resulting index type to be UnsignedInt + always, replace with owned() or some such when that's done */ + Trade::MeshData ownedInterleaved = interleave(concatenate(std::move(data))); + + const Containers::StridedArrayView2D vertexData = MeshTools::interleavedMutableData(ownedInterleaved); + + UnsignedInt uniqueVertexCount; + Containers::Array indexData; + MeshIndexType indexType; + if(ownedInterleaved.isIndexed()) { + uniqueVertexCount = removeDuplicatesIndexedInPlace(ownedInterleaved.mutableIndices(), vertexData); + indexData = ownedInterleaved.releaseIndexData(); + indexType = ownedInterleaved.indexType(); + } else { + indexData = Containers::Array{Containers::NoInit, ownedInterleaved.vertexCount()*sizeof(UnsignedInt)}; + uniqueVertexCount = removeDuplicatesInPlaceInto(vertexData, Containers::arrayCast(indexData)); + indexType = MeshIndexType::UnsignedInt; + } + + /* Allocate a new, shorter vertex data and copy the prefix */ + /** @todo better idea? even if we would use growable arrays in duplicate() + or interleave() above, arrayResize() wouldn't release the excessive + memory in any way. This is basically equivalent to STL's + shrink_to_fit(), which also copies */ + Containers::Array uniqueVertexData{Containers::NoInit, uniqueVertexCount*vertexData.size()[1]}; + Utility::copy(vertexData.prefix(uniqueVertexCount), + Containers::StridedArrayView2D{uniqueVertexData, {uniqueVertexCount, vertexData.size()[1]}}); + + /* Route all attributes to the new vertex data */ + Containers::Array attributeData{ownedInterleaved.attributeCount()}; + for(UnsignedInt i = 0; i != ownedInterleaved.attributeCount(); ++i) + attributeData[i] = Trade::MeshAttributeData{ownedInterleaved.attributeName(i), + ownedInterleaved.attributeFormat(i), + ownedInterleaved.attributeArraySize(i), + Containers::StridedArrayView1D{uniqueVertexData, + uniqueVertexData.data() + ownedInterleaved.attributeOffset(i), + uniqueVertexCount, + ownedInterleaved.attributeStride(i)}}; + + Trade::MeshIndexData indices{indexType, indexData}; + return Trade::MeshData{ownedInterleaved.primitive(), + std::move(indexData), indices, + std::move(uniqueVertexData), std::move(attributeData), + uniqueVertexCount}; +} + }} diff --git a/src/Magnum/MeshTools/RemoveDuplicates.h b/src/Magnum/MeshTools/RemoveDuplicates.h index 492b8fe7c..40d7ee3e3 100644 --- a/src/Magnum/MeshTools/RemoveDuplicates.h +++ b/src/Magnum/MeshTools/RemoveDuplicates.h @@ -40,6 +40,7 @@ #include "Magnum/Magnum.h" #include "Magnum/Math/FunctionsBatch.h" #include "Magnum/MeshTools/visibility.h" +#include "Magnum/Trade/Trade.h" namespace Magnum { namespace MeshTools { @@ -305,6 +306,36 @@ template std::size_t removeDuplicatesIndexedInPlace(const Containe } } +/** +@brief Remove mesh data duplicates +@m_since_latest + +Equivalent to calling @ref removeDuplicatesInPlace() (or +@ref removeDuplicatesIndexedInPlace(), in case the mesh is indexed) on a +mutable copy of every attribute and then putting the unique prefix and +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. + +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. +*/ +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData removeDuplicates(const Trade::MeshData& data); + +/** +@brief Remove mesh data duplicates +@m_since_latest + +Same as @ref removeDuplicates(const Trade::MeshData&), except that it operates +in-place on the passed instance, avoiding an extra copy of vertex and index +data. +*/ +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData removeDuplicates(Trade::MeshData&& data); + template std::pair, std::size_t> removeDuplicatesInPlace(const Containers::StridedArrayView1D& data, typename Vector::Type epsilon) { /* A trivial index array that'll be remapped and returned after */ Containers::Array indices{Containers::NoInit, data.size()}; diff --git a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp index d4d5aea9f..7b8cfde85 100644 --- a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp +++ b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp @@ -30,6 +30,7 @@ #include "Magnum/Math/Vector2.h" #include "Magnum/MeshTools/RemoveDuplicates.h" +#include "Magnum/Trade/MeshData.h" namespace Magnum { namespace MeshTools { namespace Test { namespace { @@ -60,6 +61,17 @@ struct RemoveDuplicatesTest: TestSuite::Tester { void removeDuplicatesFuzzyIndexedInPlaceErasedWrongIndexSize(); /* this is additionally regression-tested in PrimitivesIcosphereTest */ + + void removeDuplicatesMeshData(); + void removeDuplicatesMeshDataAttributeless(); +}; + +const struct { + const char* name; + bool indexed; +} RemoveDuplicatesMeshDataData[] { + {"", false}, + {"indexed", true} }; RemoveDuplicatesTest::RemoveDuplicatesTest() { @@ -93,6 +105,11 @@ RemoveDuplicatesTest::RemoveDuplicatesTest() { &RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErased, &RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedNonContiguous, &RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedWrongIndexSize}); + + addInstancedTests({&RemoveDuplicatesTest::removeDuplicatesMeshData}, + Containers::arraySize(RemoveDuplicatesMeshDataData)); + + addTests({&RemoveDuplicatesTest::removeDuplicatesMeshDataAttributeless}); } void RemoveDuplicatesTest::removeDuplicates() { @@ -408,6 +425,126 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedWrongIndexSi "MeshTools::removeDuplicatesIndexedInPlace(): expected index type size 1, 2 or 4 but got 3\n"); } +void RemoveDuplicatesTest::removeDuplicatesMeshData() { + auto&& data = RemoveDuplicatesMeshDataData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + + /* Deliberately not owned and not interleaved to verify that the function + will handle this */ + struct Vertex { + Vector2 positions[10]{ + {1.0f, 2.0f}, + {3.0f, 4.0f}, + {5.0f, 6.0f}, + {7.0f, 8.0f}, + {7.0f, 8.0f}, + {7.0f, 8.0f}, + {9.0f, 10.0f}, + {3.0f, 4.0f}, + {5.0f, 6.0f}, + {5.0f, 6.0f} + }; + Short data[10][2]{ + {-15, 2}, + {2, 32}, + {24, 2}, + {-15, 2}, + {15, 2}, + {2, 7541}, + {24, 2}, + {2, 32}, + {24, 2}, + {15, 2} + }; + } vertexData[1]; + + const UnsignedShort indexData[]{1, 2, 5, 9, 7, 6, 4, 7, 5, 0, 3, 8, 3}; + + Containers::ArrayView indexView; + Trade::MeshIndexData indices; + if(data.indexed) { + indexView = indexData; + indices = Trade::MeshIndexData{indexData}; + } + + Trade::MeshData mesh{MeshPrimitive::Lines, + {}, indexView, indices, + {}, vertexData, { + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayView(vertexData->positions)}, + Trade::MeshAttributeData{Trade::meshAttributeCustom(42), + VertexFormat::ShortNormalized, 2, + Containers::stridedArrayView(vertexData->data)}, + }}; + + Trade::MeshData unique = MeshTools::removeDuplicates(mesh); + CORRADE_COMPARE(unique.primitive(), MeshPrimitive::Lines); + + CORRADE_VERIFY(unique.isIndexed()); + if(data.indexed) { + CORRADE_COMPARE(unique.indexCount(), mesh.indexCount()); + { + CORRADE_EXPECT_FAIL("The function currently doesn't preserve the index type."); + CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedShort); + } + CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(unique.indices(), + Containers::arrayView({1, 2, 5, 7, 1, 6, 4, 1, 5, 0, 3, 2, 3}), + TestSuite::Compare::Container); + } else { + CORRADE_COMPARE(unique.indexCount(), mesh.vertexCount()); + CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(unique.indices(), + Containers::arrayView({0, 1, 2, 3, 4, 5, 6, 1, 2, 7}), + TestSuite::Compare::Container); + } + + CORRADE_COMPARE(unique.vertexCount(), 8); + CORRADE_COMPARE(unique.attributeCount(), 2); + CORRADE_COMPARE(unique.attributeName(0), Trade::MeshAttribute::Position); + CORRADE_COMPARE(unique.attributeFormat(0), VertexFormat::Vector2); + CORRADE_COMPARE_AS(unique.attribute(0), + Containers::arrayView({ + {1.0f, 2.0f}, + {3.0f, 4.0f}, + {5.0f, 6.0f}, + {7.0f, 8.0f}, + {7.0f, 8.0f}, + {7.0f, 8.0f}, + {9.0f, 10.0f}, + {5.0f, 6.0f} + }), + TestSuite::Compare::Container); + + CORRADE_COMPARE(unique.attributeName(1), Trade::meshAttributeCustom(42)); + CORRADE_COMPARE(unique.attributeFormat(1), VertexFormat::ShortNormalized); + CORRADE_COMPARE(unique.attributeArraySize(1), 2); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector2s>(unique.attribute(1))), + Containers::arrayView({ + {-15, 2}, + {2, 32}, + {24, 2}, + {-15, 2}, + {15, 2}, + {2, 7541}, + {24, 2}, + {15, 2} + }), + TestSuite::Compare::Container); +} + +void RemoveDuplicatesTest::removeDuplicatesMeshDataAttributeless() { + #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, 10}); + CORRADE_COMPARE(out.str(), + "MeshTools::removeDuplicates(): can't remove duplicates in an attributeless mesh\n"); +} + }}}} CORRADE_TEST_MAIN(Magnum::MeshTools::Test::RemoveDuplicatesTest)