From cc1142cc1761f2454b576ce71c15b45cea9242d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 30 Jan 2014 20:13:55 +0100 Subject: [PATCH] MeshTools: reworked removeDuplicates(). It now returns new index array instead of operating on already existing one and also custom vector size is removed. The internal implementation is now much simpler and cleaner. The old way (but still without custom vector size) is now alias to the new implementation, is marked as deprecated and will be removed in future release. Renamed the parameters and reworded the documentation so it doesn't talk about vertices, but rather about generic floating-point vector data. --- src/Magnum/MeshTools/GenerateFlatNormals.cpp | 3 +- src/Magnum/MeshTools/RemoveDuplicates.h | 209 ++++++++++-------- src/Magnum/MeshTools/Test/CMakeLists.txt | 2 +- .../MeshTools/Test/RemoveDuplicatesTest.cpp | 30 ++- src/Magnum/MeshTools/Test/SubdivideTest.cpp | 5 - src/Magnum/Primitives/Icosphere.cpp | 5 +- 6 files changed, 141 insertions(+), 113 deletions(-) diff --git a/src/Magnum/MeshTools/GenerateFlatNormals.cpp b/src/Magnum/MeshTools/GenerateFlatNormals.cpp index bbda7a9b8..c4b78db09 100644 --- a/src/Magnum/MeshTools/GenerateFlatNormals.cpp +++ b/src/Magnum/MeshTools/GenerateFlatNormals.cpp @@ -26,6 +26,7 @@ #include "GenerateFlatNormals.h" #include "Magnum/Math/Vector3.h" +#include "Magnum/MeshTools/Duplicate.h" #include "Magnum/MeshTools/RemoveDuplicates.h" namespace Magnum { namespace MeshTools { @@ -50,7 +51,7 @@ std::tuple, std::vector> generateFlatNormals(c } /* Remove duplicate normals and return */ - MeshTools::removeDuplicates(normalIndices, normals); + normalIndices = MeshTools::duplicate(normalIndices, MeshTools::removeDuplicates(normals)); return std::make_tuple(std::move(normalIndices), std::move(normals)); } diff --git a/src/Magnum/MeshTools/RemoveDuplicates.h b/src/Magnum/MeshTools/RemoveDuplicates.h index 7a226aa8a..b2c58a807 100644 --- a/src/Magnum/MeshTools/RemoveDuplicates.h +++ b/src/Magnum/MeshTools/RemoveDuplicates.h @@ -26,10 +26,11 @@ */ /** @file - * @brief Function Magnum::MeshTools::removeDuplicates() + * @brief Function @ref Magnum::MeshTools::removeDuplicates() */ #include +#include #include #include #include @@ -37,120 +38,142 @@ #include "Magnum/Magnum.h" #include "Magnum/Math/Functions.h" -namespace Magnum { namespace MeshTools { - -namespace Implementation { - -template class RemoveDuplicates { - public: - RemoveDuplicates(std::vector& indices, std::vector& vertices): indices(indices), vertices(vertices) {} - - void operator()(typename Vertex::Type epsilon = Math::TypeTraits::epsilon()); - - private: - class IndexHash { - public: - std::size_t operator()(const Math::Vector& data) const { - return *reinterpret_cast(Utility::MurmurHash2()(reinterpret_cast(&data), sizeof(data)).byteArray()); - } - }; +#ifdef MAGNUM_BUILD_DEPRECATED +#include - struct HashedVertex { - UnsignedInt oldIndex, newIndex; - - HashedVertex(UnsignedInt oldIndex, UnsignedInt newIndex): oldIndex(oldIndex), newIndex(newIndex) {} - }; +#include "Magnum/MeshTools/Duplicate.h" +#endif - std::vector& indices; - std::vector& vertices; -}; +namespace Magnum { namespace MeshTools { +namespace Implementation { + template class VectorHash { + public: + std::size_t operator()(const Math::Vector& data) const { + return *reinterpret_cast(Utility::MurmurHash2()(reinterpret_cast(&data), sizeof(data)).byteArray()); + } + }; } /** -@brief %Remove duplicate vertices from the mesh -@tparam Vertex Vertex data type -@tparam vertexSize How many initial vertex fields are important (for - example, when dealing with perspective in 3D space, only first three - fields of otherwise 4D vertex are important) -@param[in,out] indices Index array to operate on -@param[in,out] vertices Vertex array to operate on -@param[in] epsilon Epsilon value, vertices nearer than this distance will - be melt together. - -Removes duplicate vertices from the mesh. -@see duplicate() - -@todo Different (no cycle) implementation for integral vertices -@todo Interpolate vertices, not collapse them to first in the cell -@todo Ability to specify other attributes for interpolation +@brief %Remove duplicate floating-point vector data from given array +@param[in,out] data Input data array +@param[out] epsilon Epsilon value, vertices nearer than this distance will be + melt together +@return Index array and unique data + +Removes duplicate data from the array by collapsing them into buckets of size +@p epsilon. First vector in given bucket is used, other ones are thrown away, +no interpolation is done. Note that this function is meant to be used for +floating-point data (or generally with non-zero @p epsilon), for discrete data +the usual sorting method is much more efficient. + +If you want to remove duplicate data from already indexed array, first remove +duplicates as if the array wasn't indexed at all and then use @ref duplicate() +to combine the two index arrays: +@code +std::vector indices; +std::vector positions; + +indices = MeshTools::duplicate(indices, MeshTools::removeDuplicates(positions)); +@endcode + +Removing duplicates in multiple indcidental arrays is also possible -- first +remove duplicates in each array separately and then use @ref combineIndexedArrays() +to combine the resulting index arrays to single index array and reorder the +data accordingly: +@code +std::vector positions; +std::vector texCoords; + +std::vector positionIndices; +std::tie(positionIndices, positions) = MeshTools::removeDuplicates(positions); + +std::vector texCoordIndices; +std::tie(texCoordIndices, texCoords) = MeshTools::removeDuplicates(texCoords); + +std::vector indices = MeshTools::combineIndexedArrays( + std::make_pair(std::cref(positionIndices), std::ref(positions)), + std::make_pair(std::cref(texCoordIndices), std::ref(texCoords)) +); +@endcode */ -template inline void removeDuplicates(std::vector& indices, std::vector& vertices, typename Vertex::Type epsilon = Math::TypeTraits::epsilon()) { - Implementation::RemoveDuplicates(indices, vertices)(epsilon); -} - -namespace Implementation { - -template void RemoveDuplicates::operator()(typename Vertex::Type epsilon) { - if(indices.empty()) return; - - /* Get mesh bounds */ - Vertex min = vertices[0], max = vertices[0]; - for(const auto& v: vertices) { +template std::vector removeDuplicates(std::vector& data, typename Vector::Type epsilon = Math::TypeTraits::epsilon()) { + /* Get bounds */ + Vector min = data[0], max = data[0]; + for(const auto& v: data) { min = Math::min(v, min); max = Math::max(v, max); } - /* Make epsilon so large that std::size_t can index all vertices inside - mesh bounds. */ - epsilon = Math::max(epsilon, static_cast((max-min).max()/std::numeric_limits::max())); - - /* First go with original vertex coordinates, then move them by - epsilon/2 in each direction. */ - Vertex moved; - for(std::size_t moving = 0; moving <= vertexSize; ++moving) { - - /* Under each index is pointer to face which contains given vertex - and index of vertex in the face. */ - std::unordered_map, HashedVertex, IndexHash> table; - - /* Reserve space for all vertices */ - table.reserve(vertices.size()); - - /* Go through all faces' vertices */ - for(auto it = indices.begin(); it != indices.end(); ++it) { - /* Index of a vertex in vertexSize-dimensional table */ - std::size_t index[vertexSize]; - for(std::size_t ii = 0; ii != vertexSize; ++ii) - index[ii] = std::size_t((vertices[*it][ii]+moved[ii]-min[ii])/epsilon); - - /* Try inserting the vertex into table, if it already - exists, change vertex pointer of the face to already - existing vertex */ - HashedVertex v(*it, table.size()); + /* Make epsilon so large that std::size_t can index all vectors inside the + bounds. */ + epsilon = Math::max(epsilon, typename Vector::Type((max-min).max()/std::numeric_limits::max())); + + /* Resulting index array */ + std::vector resultIndices(data.size()); + std::iota(resultIndices.begin(), resultIndices.end(), 0); + + /* Table containing original vector index for each discretized vector. + Reserving more buckets than necessary (i.e. as if each vector was + unique). */ + std::unordered_map, UnsignedInt, Implementation::VectorHash> table(data.size()); + + /* Index array for each pass, new data array */ + std::vector indices; + indices.reserve(data.size()); + + /* First go with original coordinates, then move them by epsilon/2 in each + direction. */ + Vector moved; + for(std::size_t moving = 0; moving <= Vector::Size; ++moving) { + /* Go through all vectors */ + for(std::size_t i = 0; i != data.size(); ++i) { + /* Try to insert new vertex to the table */ + const Math::Vector v((data[i] + moved - min)/epsilon); #ifndef CORRADE_GCC46_COMPATIBILITY - auto result = table.emplace(Math::Vector::from(index), v); + const auto result = table.emplace(v, table.size()); #else - auto result = table.insert({Math::Vector::from(index), v}); + const auto result = table.insert({v, table.size()}); #endif - *it = result.first->second.newIndex; + + /* Add the (either new or already existing) index to index array */ + indices.push_back(result.first->second); + + /* If this is new combination, copy the data to new (earlier) + possition in the array */ + if(result.second && i != table.size()-1) data[table.size()-1] = data[i]; } - /* Shrink vertices array */ - std::vector newVertices(table.size()); - for(auto it = table.cbegin(); it != table.cend(); ++it) - newVertices[it->second.newIndex] = vertices[it->second.oldIndex]; - std::swap(newVertices, vertices); + /* Shrink the data array */ + CORRADE_INTERNAL_ASSERT(data.size() >= table.size()); + data.resize(table.size()); + + /* Remap the resulting index array */ + for(auto& i: resultIndices) i = indices[i]; + + /* Finished */ + if(moving == Vector::Size) continue; /* Move vertex coordinates by epsilon/2 in next direction */ - if(moving != Vertex::Size) { - moved = Vertex(); - moved[moving] = epsilon/2; - } + moved = Vector(); + moved[moving] = epsilon/2; + + /* Clear the structures for next pass */ + table.clear(); + indices.clear(); } + + return resultIndices; } +#ifdef MAGNUM_BUILD_DEPRECATED +template void removeDuplicates(std::vector& indices, std::vector& data, typename Vector::Type epsilon = Math::TypeTraits::epsilon()) { + std::vector uniqueIndices; + std::tie(uniqueIndices, data) = removeDuplicates(data); + indices = MeshTools::duplicate(indices, uniqueIndices); } +#endif }} diff --git a/src/Magnum/MeshTools/Test/CMakeLists.txt b/src/Magnum/MeshTools/Test/CMakeLists.txt index aec260cae..ec7cfe6b9 100644 --- a/src/Magnum/MeshTools/Test/CMakeLists.txt +++ b/src/Magnum/MeshTools/Test/CMakeLists.txt @@ -29,7 +29,7 @@ corrade_add_test(MeshToolsDuplicateTest DuplicateTest.cpp) corrade_add_test(MeshToolsFlipNormalsTest FlipNormalsTest.cpp LIBRARIES MagnumMeshToolsTestLib) corrade_add_test(MeshToolsGenerateFlatNormalsTest GenerateFlatNormalsTest.cpp LIBRARIES MagnumMeshToolsTestLib) corrade_add_test(MeshToolsInterleaveTest InterleaveTest.cpp) -corrade_add_test(MeshToolsRemoveDuplicatesTest RemoveDuplicatesTest.cpp) +corrade_add_test(MeshToolsRemoveDuplicatesTest RemoveDuplicatesTest.cpp LIBRARIES Magnum) corrade_add_test(MeshToolsSubdivideTest SubdivideTest.cpp) # corrade_add_test(MeshToolsSubdivideRemoveDuplicatesBenchmark SubdivideRemoveDuplicatesBenchmark.h SubdivideRemoveDuplicatesBenchmark.cpp MagnumPrimitives) corrade_add_test(MeshToolsTipsifyTest TipsifyTest.cpp LIBRARIES MagnumMeshTools) diff --git a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp index b8733fa70..c6549b48a 100644 --- a/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp +++ b/src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp @@ -25,6 +25,7 @@ #include +#include "Magnum/Math/Vector2.h" #include "Magnum/MeshTools/RemoveDuplicates.h" namespace Magnum { namespace MeshTools { namespace Test { @@ -33,23 +34,30 @@ class RemoveDuplicatesTest: public TestSuite::Tester { public: RemoveDuplicatesTest(); - void cleanMesh(); + void removeDuplicates(); }; -typedef Math::Vector<1, int> Vector1; - RemoveDuplicatesTest::RemoveDuplicatesTest() { - addTests({&RemoveDuplicatesTest::cleanMesh}); + addTests({&RemoveDuplicatesTest::removeDuplicates}); } -void RemoveDuplicatesTest::cleanMesh() { - std::vector positions{1, 2, 1, 4}; - std::vector indices{0, 1, 2, 1, 2, 3}; - MeshTools::removeDuplicates(indices, positions); +void RemoveDuplicatesTest::removeDuplicates() { + /* Numbers with distance 1 should be merged, numbers with distance 2 should + be kept. Testing both even-odd and odd-even sequence to verify that + half-epsilon translations are applied properly. */ + std::vector data{ + {1, 0}, + {2, 1}, + {0, 4}, + {1, 5} + }; - /* Verify cleanup */ - CORRADE_VERIFY(positions == (std::vector{1, 2, 4})); - CORRADE_COMPARE(indices, (std::vector{0, 1, 0, 1, 0, 2})); + const std::vector indices = MeshTools::removeDuplicates(data, 2); + CORRADE_COMPARE(indices, (std::vector{0, 0, 1, 1})); + CORRADE_COMPARE(data, (std::vector{ + {1, 0}, + {0, 4} + })); } }}} diff --git a/src/Magnum/MeshTools/Test/SubdivideTest.cpp b/src/Magnum/MeshTools/Test/SubdivideTest.cpp index da5bdb7db..c4c3b928d 100644 --- a/src/Magnum/MeshTools/Test/SubdivideTest.cpp +++ b/src/Magnum/MeshTools/Test/SubdivideTest.cpp @@ -71,11 +71,6 @@ void SubdivideTest::subdivide() { CORRADE_VERIFY(positions == (std::vector{0, 2, 6, 8, 1, 4, 3, 4, 7, 5})); CORRADE_COMPARE(indices, (std::vector{4, 5, 6, 7, 8, 9, 0, 4, 6, 4, 1, 5, 6, 5, 2, 1, 7, 9, 7, 2, 8, 9, 8, 3})); - - MeshTools::removeDuplicates(indices, positions); - - /* Positions 0, 1, 2, 3, 4, 5, 6, 7, 8 */ - CORRADE_COMPARE(positions.size(), 9); } }}} diff --git a/src/Magnum/Primitives/Icosphere.cpp b/src/Magnum/Primitives/Icosphere.cpp index 0c3911331..83a3d7496 100644 --- a/src/Magnum/Primitives/Icosphere.cpp +++ b/src/Magnum/Primitives/Icosphere.cpp @@ -27,8 +27,9 @@ #include "Magnum/Mesh.h" #include "Magnum/Math/Vector3.h" -#include "Magnum/MeshTools/Subdivide.h" +#include "Magnum/MeshTools/Duplicate.h" #include "Magnum/MeshTools/RemoveDuplicates.h" +#include "Magnum/MeshTools/Subdivide.h" #include "Magnum/Trade/MeshData3D.h" namespace Magnum { namespace Primitives { @@ -77,7 +78,7 @@ Trade::MeshData3D Icosphere::solid(const UnsignedInt subdivisions) { return (a+b).normalized(); }); - MeshTools::removeDuplicates(indices, positions); + indices = MeshTools::duplicate(indices, MeshTools::removeDuplicates(positions)); std::vector normals(positions); return Trade::MeshData3D(MeshPrimitive::Triangles, std::move(indices), {std::move(positions)}, {std::move(normals)}, std::vector>{});