From 29f0fdb188f5ad27005c488d6a1f07288b1f87cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 8 Jan 2020 13:47:48 +0100 Subject: [PATCH] MeshTools: explicit generateSmoothNormals() overloads for each index type. The templated version had the unfortunate "feature" of not being able to figure out the type when an array view or a C array got passed to it. That led to worse-than-ideal UX and even though it's now a bit more verbose on the implementation side, it's the preferred solution. --- doc/snippets/MagnumMeshTools-stl.cpp | 2 +- src/Magnum/MeshTools/Compile.cpp | 2 +- src/Magnum/MeshTools/GenerateNormals.cpp | 46 ++++++++++++++----- src/Magnum/MeshTools/GenerateNormals.h | 36 ++++++++++----- .../MeshTools/Test/GenerateNormalsTest.cpp | 33 +++++++------ 5 files changed, 76 insertions(+), 43 deletions(-) diff --git a/doc/snippets/MagnumMeshTools-stl.cpp b/doc/snippets/MagnumMeshTools-stl.cpp index 82bbf5a35..dc0157174 100644 --- a/doc/snippets/MagnumMeshTools-stl.cpp +++ b/doc/snippets/MagnumMeshTools-stl.cpp @@ -58,7 +58,7 @@ std::vector indices; std::vector positions; std::vector normals{positions.size()}; -MeshTools::generateSmoothNormalsInto(indices, positions, normals); +MeshTools::generateSmoothNormalsInto(indices, positions, normals); /* [generateSmoothNormalsInto] */ } diff --git a/src/Magnum/MeshTools/Compile.cpp b/src/Magnum/MeshTools/Compile.cpp index c4c5d5b80..1cb807b96 100644 --- a/src/Magnum/MeshTools/Compile.cpp +++ b/src/Magnum/MeshTools/Compile.cpp @@ -199,7 +199,7 @@ GL::Mesh compile(const Trade::MeshData3D& meshData, CompileFlags flags) { normalStorage = generateFlatNormals(positions); useIndices = false; } else { - normalStorage = generateSmoothNormals(meshData.indices(), positions); + normalStorage = generateSmoothNormals(meshData.indices(), positions); useIndices = true; } diff --git a/src/Magnum/MeshTools/GenerateNormals.cpp b/src/Magnum/MeshTools/GenerateNormals.cpp index b6f9142f1..76d9ef748 100644 --- a/src/Magnum/MeshTools/GenerateNormals.cpp +++ b/src/Magnum/MeshTools/GenerateNormals.cpp @@ -86,6 +86,8 @@ std::pair, std::vector> generateFlatNormals(co } #endif +namespace { + #if defined(CORRADE_MSVC2019_COMPATIBILITY) && !defined(CORRADE_MSVC2017_COMPATIBILITY) /* When using /permissive- with MSVC2019, using namespace inside the function below FOR SOME REASON gets lost when instantiating the template. That's @@ -94,7 +96,7 @@ std::pair, std::vector> generateFlatNormals(co using namespace Math::Literals; #endif -template void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals) { +template inline void generateSmoothNormalsIntoImplementation(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals) { CORRADE_ASSERT(indices.size() % 3 == 0, "MeshTools::generateSmoothNormalsInto(): index count not divisible by 3", ); CORRADE_ASSERT(normals.size() == positions.size(), @@ -217,22 +219,42 @@ template void generateSmoothNormalsInto(const Containers::StridedArrayV } } -#ifndef DOXYGEN_GENERATING_OUTPUT -template void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -template void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -template void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -#endif +} + +/* 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 generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals) { + generateSmoothNormalsIntoImplementation(indices, positions, normals); +} +void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals) { + generateSmoothNormalsIntoImplementation(indices, positions, normals); +} +void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals) { + generateSmoothNormalsIntoImplementation(indices, positions, normals); +} -template Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions) { +namespace { + +template inline Containers::Array generateSmoothNormalsImplementation(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions) { Containers::Array out{Containers::NoInit, positions.size()}; generateSmoothNormalsInto(indices, positions, out); return out; } -#ifndef DOXYGEN_GENERATING_OUTPUT -template Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -template Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -template Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -#endif +} + +/* 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 */ +Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions) { + return generateSmoothNormalsImplementation(indices, positions); +} +Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions) { + return generateSmoothNormalsImplementation(indices, positions); +} +Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions) { + return generateSmoothNormalsImplementation(indices, positions); +} }} diff --git a/src/Magnum/MeshTools/GenerateNormals.h b/src/Magnum/MeshTools/GenerateNormals.h index 026ddabdc..fa1ccdffb 100644 --- a/src/Magnum/MeshTools/GenerateNormals.h +++ b/src/Magnum/MeshTools/GenerateNormals.h @@ -118,13 +118,19 @@ Martijn Buijs. @see @ref generateSmoothNormalsInto(), @ref generateFlatNormals(), @ref MeshTools::CompileFlag::GenerateSmoothNormals */ -template MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions); +MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions); -#if defined(CORRADE_TARGET_WINDOWS) && !defined(__MINGW32__) -extern template MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -extern template MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -extern template MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -#endif +/** + * @overload + * @m_since{2019,10} + */ +MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions); + +/** + * @overload + * @m_since{2019,10} + */ +MAGNUM_MESHTOOLS_EXPORT Containers::Array generateSmoothNormals(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions); /** @brief Generate smooth normals into an existing array @@ -147,13 +153,19 @@ conversions: @see @ref generateFlatNormalsInto() */ -template MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals); +MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals); -#if defined(CORRADE_TARGET_WINDOWS) && !defined(__MINGW32__) -extern template MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -extern template MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -extern template MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&); -#endif +/** + * @overload + * @m_since{2019,10} + */ +MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals); + +/** + * @overload + * @m_since{2019,10} + */ +MAGNUM_MESHTOOLS_EXPORT void generateSmoothNormalsInto(const Containers::StridedArrayView1D& indices, const Containers::StridedArrayView1D& positions, const Containers::StridedArrayView1D& normals); }} diff --git a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp b/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp index 872e532e1..22e36c3bb 100644 --- a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp +++ b/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp @@ -51,7 +51,7 @@ struct GenerateNormalsTest: TestSuite::Tester { void flatWrongCount(); void flatIntoWrongSize(); - void smoothTwoTriangles(); + template void smoothTwoTriangles(); void smoothCube(); void smoothBeveledCube(); void smoothCylinder(); @@ -72,7 +72,9 @@ GenerateNormalsTest::GenerateNormalsTest() { &GenerateNormalsTest::flatWrongCount, &GenerateNormalsTest::flatIntoWrongSize, - &GenerateNormalsTest::smoothTwoTriangles, + &GenerateNormalsTest::smoothTwoTriangles, + &GenerateNormalsTest::smoothTwoTriangles, + &GenerateNormalsTest::smoothTwoTriangles, &GenerateNormalsTest::smoothCube, &GenerateNormalsTest::smoothBeveledCube, &GenerateNormalsTest::smoothCylinder, @@ -155,12 +157,14 @@ void GenerateNormalsTest::flatIntoWrongSize() { CORRADE_COMPARE(out.str(), "MeshTools::generateFlatNormalsInto(): bad output size, expected 6 but got 7\n"); } -void GenerateNormalsTest::smoothTwoTriangles() { - const UnsignedInt indices[]{0, 1, 2, 3, 4, 5}; +template void GenerateNormalsTest::smoothTwoTriangles() { + setTestCaseTemplateName(Math::TypeTraits::name()); + + const T indices[]{0, 1, 2, 3, 4, 5}; /* Should generate the same output as flat normals */ CORRADE_COMPARE_AS( - generateSmoothNormals(Containers::stridedArrayView(indices), TwoTriangles), + generateSmoothNormals(indices, TwoTriangles), (Containers::Array{Containers::InPlaceInit, { Vector3::zAxis(), Vector3::zAxis(), @@ -194,7 +198,7 @@ void GenerateNormalsTest::smoothCube() { /* Normals should be the same as positions, only normalized */ CORRADE_COMPARE_AS( - generateSmoothNormals(Containers::stridedArrayView(indices), positions), + generateSmoothNormals(indices, positions), (Containers::Array{Containers::InPlaceInit, { positions[0]/Constants::sqrt3(), positions[1]/Constants::sqrt3(), @@ -207,7 +211,6 @@ void GenerateNormalsTest::smoothCube() { }}), TestSuite::Compare::Container); } - constexpr Vector3 BeveledCubePositions[] { {-1.0f, -0.6f, 1.1f}, { 1.0f, -0.6f, 1.1f}, @@ -284,7 +287,7 @@ void GenerateNormalsTest::smoothBeveledCube() { Vector3 x{0.996072f, 0.0754969f, 0.0462723f}; Vector3 y{0.0467958f, 0.997808f, 0.0467958f}; CORRADE_COMPARE_AS(generateSmoothNormals( - Containers::stridedArrayView(BeveledCubeIndices), BeveledCubePositions), + BeveledCubeIndices, BeveledCubePositions), (Containers::Array{Containers::InPlaceInit, { z*Math::sign(BeveledCubePositions[ 0]), z*Math::sign(BeveledCubePositions[ 1]), @@ -341,8 +344,7 @@ void GenerateNormalsTest::smoothZeroAreaTriangle() { 0, 1, 2, 1, 2, 1 }; - CORRADE_COMPARE_AS(generateSmoothNormals( - Containers::stridedArrayView(indices), positions), + CORRADE_COMPARE_AS(generateSmoothNormals(indices, positions), (Containers::Array{Containers::InPlaceInit, { Vector3::zAxis(), Vector3::zAxis(), @@ -364,8 +366,7 @@ void GenerateNormalsTest::smoothNanPosition() { 0, 1, 2, 1, 2, 1 }; - Containers::Array generated = generateSmoothNormals( - Containers::stridedArrayView(indices), positions); + Containers::Array generated = generateSmoothNormals(indices, positions); CORRADE_COMPARE_AS(generated.prefix(3), (Containers::Array{Containers::InPlaceInit, { Vector3::zAxis(), @@ -381,7 +382,7 @@ void GenerateNormalsTest::smoothWrongCount() { const UnsignedByte indices[7]{}; const Vector3 positions[1]; - generateSmoothNormals(Containers::stridedArrayView(indices), positions); + generateSmoothNormals(indices, positions); CORRADE_COMPARE(out.str(), "MeshTools::generateSmoothNormalsInto(): index count not divisible by 3\n"); } @@ -392,7 +393,7 @@ void GenerateNormalsTest::smoothIntoWrongSize() { const UnsignedByte indices[6]{}; const Vector3 positions[3]; Vector3 normals[4]; - generateSmoothNormalsInto(Containers::stridedArrayView(indices), positions, normals); + generateSmoothNormalsInto(indices, positions, normals); CORRADE_COMPARE(out.str(), "MeshTools::generateSmoothNormalsInto(): bad output size, expected 3 but got 4\n"); } @@ -412,9 +413,7 @@ void GenerateNormalsTest::benchmarkFlat() { void GenerateNormalsTest::benchmarkSmooth() { Containers::Array normals{Containers::NoInit, Containers::arraySize(BeveledCubePositions)}; CORRADE_BENCHMARK(10) { - generateSmoothNormalsInto( - Containers::stridedArrayView(BeveledCubeIndices), - BeveledCubePositions, normals); + generateSmoothNormalsInto(BeveledCubeIndices, BeveledCubePositions, normals); } CORRADE_COMPARE(Math::min(normals), (Vector3{-0.996072f, -0.997808f, -0.996072f}));