MeshTools: unconditionally repack the input in removeDuplicates().

Since the main speed advantage of the function is that it hashes a
*whole* vertex together instead of going through the attribute arrays
one by one, it can't really operate on whatever funny interleaved layout
it was given as there may be padding bytes with random content, breaking
the deduplication.

Since checking that a layout is really padding-less is rather complex,
the repacking is now performed always. This also means the && overload
makes no sense anymore and thus it was dropped.

This makes the test added in the previous commit not assert anymore, and
behave the same as the padding-less case.

doc/changelog.dox

@@ -527,6 +527,17 @@ See also:
     for most shader-related limits
 -   Fixed assertions related to OpenGL driver workarounds when the proprietary
     AMDGPU PRO drivers are used on Linux
+-   Fixed an assertion when using @ref MeshTools::removeDuplicates() on an
+    interleaved @ref Trade::MeshData that included padding at the beginning or
+    end of each vertex
+-   Fixed @ref MeshTools::removeDuplicates() to not take into account random
+    padding bytes and filtered-out attributes in interleaved source
+    @ref Trade::MeshData. This was a particularly glaring issue when using
+    @ref magnum-sceneconverter "magnum-sceneconverter" with both
+    `--only-attributes` and `--remove-duplicates` specified together, where it
+    usually just didn't remove any duplicate whatsoever while
+    `--remove-duplicates-fuzzy` did, no matter how ridiculously low epsilon
+    was used.
 -   @ref Platform::EmscriptenApplication randomly created antialiased contexts
     due to an uninitialized variable in its
     @ref Platform::EmscriptenApplication::GLConfiguration "GLConfiguration"

src/Magnum/MeshTools/RemoveDuplicates.cpp

@@ -397,13 +397,6 @@ std::size_t removeDuplicatesFuzzyIndexedInPlace(const Containers::StridedArrayVi
 }
 
 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}));
@@ -418,12 +411,19 @@ Trade::MeshData removeDuplicates(Trade::MeshData&& data) {
 
     /* Turn the passed data into an interleaved owned mutable instance we can
        operate on -- owned() 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. */
-    Trade::MeshData ownedInterleaved = owned(interleave(std::move(data)));
-
+       alone only makes the data interleaved (but the index data can stay
+       non-owned). Additionally we need to ensure the interleaved data are
+       tightly packed by removing InterleaveFlag::PreserveInterleavedAttributes
+       which is set by default, otherwise random padding bytes or filtered-out
+       attributes may contribute to the non-uniqueness of particular
+       elements. */
+    Trade::MeshData ownedInterleaved = owned(interleave(data, {}, InterleaveFlags{}));
+
+    /* Because the interleaved mesh was forced to be repacked, the vertex data
+       should span the whole stride -- this is relied on in the attribute
+       rerouting loop below */
     const Containers::StridedArrayView2D<char> vertexData = MeshTools::interleavedMutableData(ownedInterleaved);
+    CORRADE_INTERNAL_ASSERT(vertexData.size()[1] == ownedInterleaved.attributeStride(0));
 
     UnsignedInt uniqueVertexCount;
     Containers::Array<char> indexData;

src/Magnum/MeshTools/RemoveDuplicates.h

@@ -302,24 +302,13 @@ interleaved and owned, if the input is already interleaved attribute offsets
 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.
+In order to remove random padding values from the input and make the vertices
+suitable for fast in-place duplicate removal, this function unconditionally
+copies and interleaves the input vertex and index data.
 @see @ref isVertexFormatImplementationSpecific()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData removeDuplicates(const Trade::MeshData& data);
 
-/**
-@brief Remove mesh data duplicates
-@m_since{2020,06}
-
-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);
-
 /**
 @brief Remove mesh data duplicates with fuzzy comparison for floating-point attributes
 @m_since{2020,06}