MeshTools: added a duplicateInto() variant taking a 2D strided array view.

This also allowed me to move all the complexity and assertions into a cpp file, no longer polluting the header.
6 years ago · 735c59e96b
6 changed files with 164 additions and 14 deletions
--- a/doc/changelog.dox
+++ b/doc/changelog.dox
@ -210,6 +210,8 @@ See also:
 -   Added @ref MeshTools::removeDuplicatesIndexedInPlace() that operates
    in-place on an indexed array view and a STL-less
    @ref MeshTools::removeDuplicatesInPlace() variant
+-   Added @ref MeshTools::duplicateInto() variants that take type-erased
+    2D strided array views

@subsubsection changelog-latest-changes-platform Platform libraries

--- a/src/Magnum/MeshTools/CMakeLists.txt
+++ b/src/Magnum/MeshTools/CMakeLists.txt
@ -31,6 +31,7 @@ set(MagnumMeshTools_SRCS
 set(MagnumMeshTools_GracefulAssert_SRCS
    CombineIndexedArrays.cpp
    CompressIndices.cpp
+    Duplicate.cpp
    FlipNormals.cpp
    GenerateNormals.cpp)

--- a/src/Magnum/MeshTools/Duplicate.cpp
+++ b/src/Magnum/MeshTools/Duplicate.cpp
@ -0,0 +1,64 @@
+/*
+    This file is part of Magnum.
+
+    Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019
+              Vladimír Vondruš <mosra@centrum.cz>
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice shall be included
+    in all copies or substantial portions of the Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+*/
+
+#include "Duplicate.h"
+
+#include <cstring>
+
+namespace Magnum { namespace MeshTools {
+
+namespace {
+
+template<class T> inline void duplicateIntoImplementation(const Containers::StridedArrayView1D<const T>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out) {
+    CORRADE_ASSERT(out.size()[0] == indices.size(),
+        "MeshTools::duplicateInto(): index array and output size don't match, expected" << indices.size() << "but got" << out.size()[0], );
+    CORRADE_ASSERT(data.isContiguous<1>() && out.isContiguous<1>(),
+        "MeshTools::duplicateInto(): second view dimension is not contiguous", );
+    CORRADE_ASSERT(data.size()[1] == out.size()[1],
+        "MeshTools::duplicateInto(): input and output type size doesn't match, expected" << data.size()[1] << "but got" << out.size()[1], );
+    const std::size_t size = data.size()[1];
+    for(std::size_t i = 0; i != indices.size(); ++i) {
+        const std::size_t index = indices[i];
+        CORRADE_ASSERT(index < data.size()[0], "MeshTools::duplicateInto(): index" << index << "out of bounds for" << data.size()[0] << "elements", );
+        std::memcpy(out[i].data(), data[index].data(), size);
+    }
+}
+
+}
+
+/* If not done this way but with templates instead, C++ wouldn't be able to
+   figure out on its own which overload to use when indices are not already a
+   strided arrray view */
+void duplicateInto(const Containers::StridedArrayView1D<const UnsignedByte>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out) {
+    duplicateIntoImplementation(indices, data, out);
+}
+void duplicateInto(const Containers::StridedArrayView1D<const UnsignedShort>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out) {
+    duplicateIntoImplementation(indices, data, out);
+}
+void duplicateInto(const Containers::StridedArrayView1D<const UnsignedInt>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out) {
+    duplicateIntoImplementation(indices, data, out);
+}
+
+}}
--- a/src/Magnum/MeshTools/Duplicate.h
+++ b/src/Magnum/MeshTools/Duplicate.h
@ -33,9 +33,9 @@
 #include <Corrade/Containers/Array.h>
 #include <Corrade/Containers/ArrayViewStl.h>
 #include <Corrade/Containers/StridedArrayView.h>
-#include <Corrade/Utility/Assert.h>

 #include "Magnum/Magnum.h"
+#include "Magnum/MeshTools/visibility.h"

 namespace Magnum { namespace MeshTools {

@ -83,16 +83,40 @@ template<class T> std::vector<T> duplicate(const std::vector<UnsignedInt>& indic
@m_since{2019,10}

 A variant of @ref duplicate() that fills existing memory instead of allocating
-a new array.
+a new array. Expects that @p out has the same size as @p indices and all
+indices are in range for the @p data array.
+*/
+template<class IndexType, class T> void duplicateInto(const Containers::StridedArrayView1D<const IndexType>& indices, const Containers::StridedArrayView1D<const T>& data, const Containers::StridedArrayView1D<T>& out);
+
+/**
+@brief Duplicate type-erased data using an index array into given output array
+@param[in]  indices Index array to use
+@param[in]  data    Input data
+@param[out] out     Where to store the output
+@m_since_latest
+
+Compared to @ref duplicateInto(const Containers::StridedArrayView1D<const IndexType>&, const Containers::StridedArrayView1D<const T>&, const Containers::StridedArrayView1D<T>&)
+accepts a 2D view, where the second dimension spans the actual type. Expects
+that @p out has the same size as @p indices and all indices are in range for
+the @p data array, and that the second dimension of both @p data and @p out
+is contiguous and has the same size.
 */
-template<class IndexType, class T> void duplicateInto(const Containers::StridedArrayView1D<const IndexType>& indices, const Containers::StridedArrayView1D<const T>& data, const Containers::StridedArrayView1D<T>& out) {
-    CORRADE_ASSERT(out.size() == indices.size(),
-        "MeshTools::duplicateInto(): bad output size, expected" << indices.size() << "but got" << out.size(), );
-    for(std::size_t i = 0; i != indices.size(); ++i) {
-        const std::size_t index = indices[i];
-        CORRADE_ASSERT(index < data.size(), "MeshTools::duplicateInto(): index" << index << "out of bounds for" << data.size() << "elements", );
-        out[i] = data[index];
-    }
+MAGNUM_MESHTOOLS_EXPORT void duplicateInto(const Containers::StridedArrayView1D<const UnsignedInt>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out);
+
+/**
+ * @overload
+ * @m_since_latest
+ */
+MAGNUM_MESHTOOLS_EXPORT void duplicateInto(const Containers::StridedArrayView1D<const UnsignedShort>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out);
+
+/**
+ * @overload
+ * @m_since_latest
+ */
+MAGNUM_MESHTOOLS_EXPORT void duplicateInto(const Containers::StridedArrayView1D<const UnsignedByte>& indices, const Containers::StridedArrayView2D<const char>& data, const Containers::StridedArrayView2D<char>& out);
+
+template<class IndexType, class T> inline void duplicateInto(const Containers::StridedArrayView1D<const IndexType>& indices, const Containers::StridedArrayView1D<const T>& data, const Containers::StridedArrayView1D<T>& out) {
+    duplicateInto(indices, Containers::arrayCast<2, const char>(data), Containers::arrayCast<2, char>(out));
 }

 }}
--- a/src/Magnum/MeshTools/Test/CMakeLists.txt
+++ b/src/Magnum/MeshTools/Test/CMakeLists.txt
@ -25,7 +25,7 @@

 corrade_add_test(MeshToolsCombineIndexedArraysTest CombineIndexedArraysTest.cpp LIBRARIES MagnumMeshToolsTestLib)
 corrade_add_test(MeshToolsCompressIndicesTest CompressIndicesTest.cpp LIBRARIES MagnumMeshToolsTestLib)
-corrade_add_test(MeshToolsDuplicateTest DuplicateTest.cpp LIBRARIES Magnum)
+corrade_add_test(MeshToolsDuplicateTest DuplicateTest.cpp LIBRARIES MagnumMeshToolsTestLib)
 corrade_add_test(MeshToolsFlipNormalsTest FlipNormalsTest.cpp LIBRARIES MagnumMeshToolsTestLib)
 corrade_add_test(MeshToolsGenerateNormalsTest GenerateNormalsTest.cpp LIBRARIES MagnumMeshToolsTestLib MagnumPrimitives)
 corrade_add_test(MeshToolsInterleaveTest InterleaveTest.cpp LIBRARIES Magnum)
@ -33,7 +33,7 @@ corrade_add_test(MeshToolsRemoveDuplicatesTest RemoveDuplicatesTest.cpp LIBRARIE
 corrade_add_test(MeshToolsSubdivideTest SubdivideTest.cpp LIBRARIES Magnum)
 corrade_add_test(MeshToolsTipsifyTest TipsifyTest.cpp LIBRARIES MagnumMeshTools)
 corrade_add_test(MeshToolsTransformTest TransformTest.cpp LIBRARIES MagnumMeshTools)
-corrade_add_test(MeshToolsSubdivideRemov___Benchmark SubdivideRemoveDuplicatesBenchmark.cpp LIBRARIES MagnumPrimitives)
+corrade_add_test(MeshToolsSubdivideRemov___Benchmark SubdivideRemoveDuplicatesBenchmark.cpp LIBRARIES MagnumMeshTools MagnumPrimitives)

 # Graceful assert for testing
 set_property(TARGET
--- a/src/Magnum/MeshTools/Test/DuplicateTest.cpp
+++ b/src/Magnum/MeshTools/Test/DuplicateTest.cpp
@ -29,6 +29,7 @@
 #include <Corrade/Utility/DebugStl.h>

 #include "Magnum/Magnum.h"
+#include "Magnum/Math/TypeTraits.h"
 #include "Magnum/MeshTools/Duplicate.h"

 namespace Magnum { namespace MeshTools { namespace Test { namespace {
@ -42,6 +43,10 @@ struct DuplicateTest: TestSuite::Tester {

    void duplicateInto();
    void duplicateIntoWrongSize();
+
+    template<class T> void duplicateIntoErased();
+    void duplicateIntoErasedWrongTypeSize();
+    void duplicateIntoErasedNonContiguous();
 };

 DuplicateTest::DuplicateTest() {
@ -50,7 +55,13 @@ DuplicateTest::DuplicateTest() {
              &DuplicateTest::duplicateStl,

              &DuplicateTest::duplicateInto,
-              &DuplicateTest::duplicateIntoWrongSize});
+              &DuplicateTest::duplicateIntoWrongSize,
+
+              &DuplicateTest::duplicateIntoErased<UnsignedByte>,
+              &DuplicateTest::duplicateIntoErased<UnsignedShort>,
+              &DuplicateTest::duplicateIntoErased<UnsignedInt>,
+              &DuplicateTest::duplicateIntoErasedWrongTypeSize,
+              &DuplicateTest::duplicateIntoErasedNonContiguous});
 }

 void DuplicateTest::duplicate() {
@ -102,7 +113,55 @@ void DuplicateTest::duplicateIntoWrongSize() {

    MeshTools::duplicateInto<UnsignedByte, Int>(indices, data, output);
    CORRADE_COMPARE(out.str(),
-        "MeshTools::duplicateInto(): bad output size, expected 6 but got 5\n");
+        "MeshTools::duplicateInto(): index array and output size don't match, expected 6 but got 5\n");
+}
+
+template<class T> void DuplicateTest::duplicateIntoErased() {
+    setTestCaseTemplateName(Math::TypeTraits<T>::name());
+
+    constexpr T indices[]{1, 1, 0, 3, 2, 2};
+    constexpr Int data[]{-7, 35, 12, -18};
+    Int output[6];
+
+    MeshTools::duplicateInto(
+        Containers::stridedArrayView(indices),
+        Containers::arrayCast<2, const char>(Containers::stridedArrayView(data)),
+        Containers::arrayCast<2, char>(Containers::stridedArrayView(output)));
+    CORRADE_COMPARE_AS(Containers::arrayView<const Int>(output),
+        Containers::arrayView({35, 35, -7, -18, 12, 12}),
+        TestSuite::Compare::Container);
+}
+
+void DuplicateTest::duplicateIntoErasedWrongTypeSize() {
+    constexpr UnsignedByte indices[]{1, 1, 0, 3, 2, 2};
+    constexpr Int data[]{-7, 35, 12, -18};
+    Short output[6];
+
+    std::ostringstream out;
+    Error redirectError{&out};
+
+    MeshTools::duplicateInto(
+        Containers::stridedArrayView(indices),
+        Containers::arrayCast<2, const char>(Containers::stridedArrayView(data)),
+        Containers::arrayCast<2, char>(Containers::stridedArrayView(output)));
+    CORRADE_COMPARE(out.str(),
+        "MeshTools::duplicateInto(): input and output type size doesn't match, expected 4 but got 2\n");
+}
+
+void DuplicateTest::duplicateIntoErasedNonContiguous() {
+    constexpr UnsignedByte indices[]{1, 1, 0, 3, 2, 2};
+    constexpr Int data[]{-7, 35, 12, -18};
+    Short output[6];
+
+    std::ostringstream out;
+    Error redirectError{&out};
+
+    MeshTools::duplicateInto(
+        Containers::stridedArrayView(indices),
+        Containers::arrayCast<2, const char>(Containers::stridedArrayView(data)).every({1, 2}),
+        Containers::arrayCast<2, char>(Containers::stridedArrayView(output)));
+    CORRADE_COMPARE(out.str(),
+        "MeshTools::duplicateInto(): second view dimension is not contiguous\n");
 }

 }}}}