Browse Source

MeshTools: rename removeDuplicates{ => Fuzzy}[Indexed]InPlace().

A breaking change, sorry, but I don't want to add yet another layer of
backwards compatibility on APIs that are in master for just a month or
so. This is a test for how many people actually use these APIs -- if
nobody complains, great!

Conflating the fuzzy operation with the discrete one wasn't a good idea,
as people could be unintentionally using the (slower) fuzzy variant on
data that could be easily deduplicated using the discrete variant. One
such case is in the icosphere primitive, and I'm going to look at that
right now.
pull/449/head
Vladimír Vondruš 6 years ago
parent
commit
9474b3e3fc
  1. 5
      doc/changelog.dox
  2. 50
      src/Magnum/MeshTools/RemoveDuplicates.h
  3. 22
      src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp
  4. 4
      src/Magnum/MeshTools/Test/SubdivideRemoveDuplicatesBenchmark.cpp
  5. 2
      src/Magnum/Primitives/Icosphere.cpp

5
doc/changelog.dox

@ -624,8 +624,9 @@ See also:
@cpp MeshTools::removeDuplicates() @ce and @cpp MeshTools::subdivide() @ce
operating on a @ref std::vector are deprecated, use the STL-free
@ref MeshTools::compressIndices(), @ref MeshTools::duplicate(),
@ref MeshTools::removeDuplicatesInPlace() and @ref MeshTools::subdivide() /
@ref MeshTools::subdivideInPlace() overloads instead
@ref MeshTools::removeDuplicatesFuzzyInPlace() and
@ref MeshTools::subdivide() / @ref MeshTools::subdivideInPlace() overloads
instead
- @cpp Primitives::CapsuleTextureCoords @ce,
@cpp Primitives::CircleTextureCoords @ce,
@cpp Primitives::PlaneTextureCoords @ce,

50
src/Magnum/MeshTools/RemoveDuplicates.h

@ -64,9 +64,9 @@ namespace Implementation {
Removes duplicate data from given array by comparing the second dimension of
each item, the second dimension is expected to be contiguous. A plain bit-exact
matching is used, if you need fuzzy comparison for floating-point data, use
@ref removeDuplicatesInPlace(const Containers::StridedArrayView1D<Vector>&, typename Vector::Type)
instead. If you want to remove duplicate data from an already indexed array,
use @ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<UnsignedInt>&, const Containers::StridedArrayView2D<char>&)
@ref removeDuplicatesFuzzyInPlace() instead. If you want to remove duplicate
data from an already indexed array, use
@ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<UnsignedInt>&, const Containers::StridedArrayView2D<char>&)
instead. Usage example:
@snippet MagnumMeshTools.cpp removeDuplicates
@ -152,16 +152,16 @@ MAGNUM_MESHTOOLS_EXPORT std::size_t removeDuplicatesIndexedInPlace(const Contain
Expects that the second dimension of @p indices is contiguous and represents
the actual 1/2/4-byte index type. Based on its size then calls one of the
@ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<UnsignedInt>&)
@ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<UnsignedInt>&, const Containers::StridedArrayView2D<char>&)
etc. overloads.
*/
MAGNUM_MESHTOOLS_EXPORT std::size_t removeDuplicatesIndexedInPlace(const Containers::StridedArrayView2D<char>& indices, const Containers::StridedArrayView2D<char>& data);
/**
@brief Remove duplicate floating-point vector data from given array in-place
@brief Remove duplicate data from given array using fuzzy comparison in-place
@param[in,out] data Data array, duplicate items will be cut away with order
preserved
@param[in] epsilon Epsilon value, vertices closer than this distance will be
@param[in] epsilon Epsilon value, data closer than this distance will be
melt together
@return Size of unique prefix in the cleaned up @p data array and the resulting
index array
@ -175,23 +175,23 @@ bit-exact matching is sufficient use @ref removeDuplicatesInPlace(const Containe
instead.
If you want to remove duplicate data from an already indexed array, use
@ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<IndexType>&, const Containers::StridedArrayView1D<Vector>&, typename Vector::Type) instead.
@ref removeDuplicatesFuzzyIndexedInPlace(const Containers::StridedArrayView1D<IndexType>&, const Containers::StridedArrayView1D<Vector>&, typename Vector::Type) instead.
If you want to remove duplicates in multiple incidental arrays, first remove
duplicates in each array separately and then combine the resulting index arrays
back into a single one using @ref combineIndexedAttributes().
*/
template<class Vector> std::pair<Containers::Array<UnsignedInt>, std::size_t> removeDuplicatesInPlace(const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon());
template<class Vector> std::pair<Containers::Array<UnsignedInt>, std::size_t> removeDuplicatesFuzzyInPlace(const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon());
#ifdef MAGNUM_BUILD_DEPRECATED
/**
@brief Remove duplicate floating-point vector data from a STL vector in-place
@brief Remove duplicate data from a STL vector using fuzzy comparison in-place
@param[in,out] data Data array, duplicate items will be cut away with order
preserved and the size shrunk to just the unique prefix
@param[in] epsilon Epsilon value, vertices closer than this distance will be
melt together
@return Resulting index array
@m_deprecated_since_latest Use @ref removeDuplicatesInPlace() instead.
@m_deprecated_since_latest Use @ref removeDuplicatesFuzzyInPlace() instead.
Similar to the above, except that it's operating on a @ref std::vector, which
gets shrunk as a result (instead of the prefix size being returned). This
@ -206,7 +206,7 @@ template<class Vector> CORRADE_DEPRECATED("use removeDuplicatesInPlace() instead
#endif
/**
@brief Remove duplicates from indexed floating-point vector data in-place
@brief Remove duplicates from indexed data using fuzzy comparison in-place
@param[in,out] indices Index array, which will get remapped to list just
unique vertices
@param[in,out] data Data array, duplicate items will be cut away with order
@ -216,14 +216,14 @@ template<class Vector> CORRADE_DEPRECATED("use removeDuplicatesInPlace() instead
@return Size of unique prefix in the cleaned up @p data array
@m_since_latest
Compared to @ref removeDuplicatesInPlace(const Containers::StridedArrayView1D<Vector>&, typename Vector::Type)
Compared to @ref removeDuplicatesFuzzyInPlace(const Containers::StridedArrayView1D<Vector>&, typename Vector::Type)
this variant is more suited for data that is already indexed as it works on
the existing index array instead of allocating a new one.
*/
template<class IndexType, class Vector> std::size_t removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<IndexType>& indices, const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon()) {
template<class IndexType, class Vector> std::size_t removeDuplicatesFuzzyIndexedInPlace(const Containers::StridedArrayView1D<IndexType>& indices, const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon()) {
/* Somehow ~IndexType{} doesn't work for < 4byte types, as the result is
int(-1) instead of the type I want */
CORRADE_ASSERT(data.size() <= IndexType(-1), "MeshTools::removeDuplicatesIndexedInPlace(): a" << sizeof(IndexType) << Debug::nospace << "-byte index type is too small for" << data.size() << "vertices", {});
CORRADE_ASSERT(data.size() <= IndexType(-1), "MeshTools::removeDuplicatesFuzzyIndexedInPlace(): a" << sizeof(IndexType) << Debug::nospace << "-byte index type is too small for" << data.size() << "vertices", {});
/* Get bounds. When NaNs appear, those will get collapsed together when
you're lucky, or cause the whole data to disappear when you're not -- it
@ -286,23 +286,23 @@ template<class IndexType, class Vector> std::size_t removeDuplicatesIndexedInPla
}
/**
@brief Remove duplicates from indexed data in-place on a type-erased index array
@brief Remove duplicates from indexed data using fuzzy comparison in-place on a type-erased index array
@m_since_latest
Expects that the second dimension of @p indices is contiguous and represents
the actual 1/2/4-byte index type. Based on its size then calls
@ref removeDuplicatesIndexedInPlace(const Containers::StridedArrayView1D<IndexType>&, const Containers::StridedArrayView1D<Vector>&, typename Vector::Type)
@ref removeDuplicatesFuzzyIndexedInPlace(const Containers::StridedArrayView1D<IndexType>&, const Containers::StridedArrayView1D<Vector>&, typename Vector::Type)
with a concrete index type.
*/
template<class Vector> std::size_t removeDuplicatesIndexedInPlace(const Containers::StridedArrayView2D<char>& indices, const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon()) {
CORRADE_ASSERT(indices.isContiguous<1>(), "MeshTools::removeDuplicatesIndexedInPlace(): second index view dimension is not contiguous", {});
template<class Vector> std::size_t removeDuplicatesFuzzyIndexedInPlace(const Containers::StridedArrayView2D<char>& indices, const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon = Math::TypeTraits<typename Vector::Type>::epsilon()) {
CORRADE_ASSERT(indices.isContiguous<1>(), "MeshTools::removeDuplicatesFuzzyIndexedInPlace(): second index view dimension is not contiguous", {});
if(indices.size()[1] == 4)
return removeDuplicatesIndexedInPlace(Containers::arrayCast<1, UnsignedInt>(indices), data, epsilon);
return removeDuplicatesFuzzyIndexedInPlace(Containers::arrayCast<1, UnsignedInt>(indices), data, epsilon);
else if(indices.size()[1] == 2)
return removeDuplicatesIndexedInPlace(Containers::arrayCast<1, UnsignedShort>(indices), data, epsilon);
return removeDuplicatesFuzzyIndexedInPlace(Containers::arrayCast<1, UnsignedShort>(indices), data, epsilon);
else {
CORRADE_ASSERT(indices.size()[1] == 1, "MeshTools::removeDuplicatesIndexedInPlace(): expected index type size 1, 2 or 4 but got" << indices.size()[1], {});
return removeDuplicatesIndexedInPlace(Containers::arrayCast<1, UnsignedByte>(indices), data, epsilon);
CORRADE_ASSERT(indices.size()[1] == 1, "MeshTools::removeDuplicatesFuzzyIndexedInPlace(): expected index type size 1, 2 or 4 but got" << indices.size()[1], {});
return removeDuplicatesFuzzyIndexedInPlace(Containers::arrayCast<1, UnsignedByte>(indices), data, epsilon);
}
}
@ -336,11 +336,11 @@ data.
*/
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData removeDuplicates(Trade::MeshData&& data);
template<class Vector> std::pair<Containers::Array<UnsignedInt>, std::size_t> removeDuplicatesInPlace(const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon) {
template<class Vector> std::pair<Containers::Array<UnsignedInt>, std::size_t> removeDuplicatesFuzzyInPlace(const Containers::StridedArrayView1D<Vector>& data, typename Vector::Type epsilon) {
/* A trivial index array that'll be remapped and returned after */
Containers::Array<UnsignedInt> indices{Containers::NoInit, data.size()};
std::iota(indices.begin(), indices.end(), 0);
const std::size_t size = removeDuplicatesIndexedInPlace(Containers::stridedArrayView(indices), data, epsilon);
const std::size_t size = removeDuplicatesFuzzyIndexedInPlace(Containers::stridedArrayView(indices), data, epsilon);
return {std::move(indices), size};
}
@ -349,7 +349,7 @@ template<class Vector> std::vector<UnsignedInt> removeDuplicates(std::vector<Vec
/* A trivial index array that'll be remapped and returned after */
std::vector<UnsignedInt> indices(data.size());
std::iota(indices.begin(), indices.end(), 0);
const std::size_t size = removeDuplicatesIndexedInPlace(Containers::stridedArrayView(indices), Containers::stridedArrayView(data), epsilon);
const std::size_t size = removeDuplicatesFuzzyIndexedInPlace(Containers::stridedArrayView(indices), Containers::stridedArrayView(data), epsilon);
data.resize(size);
return indices;
}

22
src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp

@ -278,7 +278,7 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyInPlace() {
{1, 5}
};
std::pair<Containers::Array<UnsignedInt>, std::size_t> result = MeshTools::removeDuplicatesInPlace(Containers::stridedArrayView(data), 2);
std::pair<Containers::Array<UnsignedInt>, std::size_t> result = MeshTools::removeDuplicatesFuzzyInPlace(Containers::stridedArrayView(data), 2);
CORRADE_COMPARE_AS(Containers::arrayView(result.first),
Containers::arrayView<UnsignedInt>({0, 0, 1, 1}),
TestSuite::Compare::Container);
@ -321,7 +321,7 @@ template<class T> void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlace
{1, 5}
};
std::size_t count = MeshTools::removeDuplicatesIndexedInPlace(
std::size_t count = MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::stridedArrayView(indices),
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE_AS(Containers::arrayView(indices),
@ -342,10 +342,10 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceSmallType() {
UnsignedByte indices[1];
Vector2i data[256]{};
MeshTools::removeDuplicatesIndexedInPlace(
MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::stridedArrayView(indices),
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE(out.str(), "MeshTools::removeDuplicatesIndexedInPlace(): a 1-byte index type is too small for 256 vertices\n");
CORRADE_COMPARE(out.str(), "MeshTools::removeDuplicatesFuzzyIndexedInPlace(): a 1-byte index type is too small for 256 vertices\n");
}
void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceEmptyIndices() {
@ -356,7 +356,7 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceEmptyIndices() {
{1, 5}
};
std::size_t count = MeshTools::removeDuplicatesIndexedInPlace(
std::size_t count = MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::StridedArrayView1D<UnsignedInt>{},
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE_AS(Containers::arrayView(data).prefix(count),
@ -365,7 +365,7 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceEmptyIndices() {
}
void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceEmptyIndicesVertices() {
CORRADE_COMPARE((MeshTools::removeDuplicatesIndexedInPlace<UnsignedInt, Vector2i>({}, {}, 2)), 0);
CORRADE_COMPARE((MeshTools::removeDuplicatesFuzzyIndexedInPlace<UnsignedInt, Vector2i>({}, {}, 2)), 0);
}
template<class T> void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErased() {
@ -380,7 +380,7 @@ template<class T> void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlace
{1, 5}
};
std::size_t count = MeshTools::removeDuplicatesIndexedInPlace(
std::size_t count = MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::arrayCast<2, char>(Containers::arrayView(indices)),
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE_AS(Containers::arrayView(indices),
@ -401,11 +401,11 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedNonContiguou
std::stringstream out;
Error redirectError{&out};
MeshTools::removeDuplicatesIndexedInPlace(
MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::StridedArrayView2D<char>{indices, {6, 2}, {4, 2}},
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE(out.str(),
"MeshTools::removeDuplicatesIndexedInPlace(): second index view dimension is not contiguous\n");
"MeshTools::removeDuplicatesFuzzyIndexedInPlace(): second index view dimension is not contiguous\n");
}
void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedWrongIndexSize() {
@ -418,11 +418,11 @@ void RemoveDuplicatesTest::removeDuplicatesFuzzyIndexedInPlaceErasedWrongIndexSi
std::stringstream out;
Error redirectError{&out};
MeshTools::removeDuplicatesIndexedInPlace(
MeshTools::removeDuplicatesFuzzyIndexedInPlace(
Containers::StridedArrayView2D<char>{indices, {6, 3}},
Containers::stridedArrayView(data), 2);
CORRADE_COMPARE(out.str(),
"MeshTools::removeDuplicatesIndexedInPlace(): expected index type size 1, 2 or 4 but got 3\n");
"MeshTools::removeDuplicatesFuzzyIndexedInPlace(): expected index type size 1, 2 or 4 but got 3\n");
}
void RemoveDuplicatesTest::removeDuplicatesMeshData() {

4
src/Magnum/MeshTools/Test/SubdivideRemoveDuplicatesBenchmark.cpp

@ -93,7 +93,7 @@ void SubdivideRemoveDuplicatesBenchmark::subdivideAndRemoveDuplicatesAfter() {
MeshTools::subdivide(indices, positions, interpolator);
/* Remove duplicates after */
arrayResize(positions, MeshTools::removeDuplicatesIndexedInPlace(
arrayResize(positions, MeshTools::removeDuplicatesFuzzyIndexedInPlace(
stridedArrayView(indices), stridedArrayView(positions)));
}
}
@ -120,7 +120,7 @@ void SubdivideRemoveDuplicatesBenchmark::subdivideAndRemoveDuplicatesInBetween()
/* Subdivide 5 times and remove duplicates during the operation */
for(std::size_t i = 0; i != 5; ++i) {
MeshTools::subdivide(indices, positions, interpolator);
arrayResize(positions, MeshTools::removeDuplicatesIndexedInPlace(
arrayResize(positions, MeshTools::removeDuplicatesFuzzyIndexedInPlace(
stridedArrayView(indices), stridedArrayView(positions)));
}
}

2
src/Magnum/Primitives/Icosphere.cpp

@ -116,7 +116,7 @@ Trade::MeshData icosphereSolid(const UnsignedInt subdivisions) {
/** @todo i need arrayShrinkAndGiveUpMemoryIfItDoesntCauseRealloc() */
Containers::arrayResize<Trade::ArrayAllocator>(vertexData,
MeshTools::removeDuplicatesIndexedInPlace(Containers::stridedArrayView(indices), Containers::stridedArrayView(positions))*sizeof(Vertex));
MeshTools::removeDuplicatesFuzzyIndexedInPlace(Containers::stridedArrayView(indices), Containers::stridedArrayView(positions))*sizeof(Vertex));
}
/* Build up the views again with correct size, fill the normals */

Loading…
Cancel
Save