MeshTools: ability to generate flat normals in compile().

doc/changelog.dox

@@ -140,6 +140,8 @@ See also:
 -   @ref MeshTools::generateFlatNormalsInto() alternative to
     @ref MeshTools::generateFlatNormals() that writes the output to an existing
     location
+-   @ref MeshTools::compile(const Trade::MeshData3D&, CompileFlags) now accepts
+    optional flags to control normal generation
 
 @subsubsection changelog-latest-new-platform Platform libraries
 

src/Magnum/MeshTools/Compile.cpp

@@ -25,11 +25,16 @@
 
 #include "Compile.h"
 
+#include <Corrade/Containers/StridedArrayView.h>
+#include <Corrade/Containers/ArrayViewStl.h> /** @todo remove once MeshData is sane */
+
 #include "Magnum/GL/Buffer.h"
 #include "Magnum/GL/Mesh.h"
 #include "Magnum/Math/Vector3.h"
 #include "Magnum/Math/Color.h"
 #include "Magnum/MeshTools/CompressIndices.h"
+#include "Magnum/MeshTools/GenerateNormals.h"
+#include "Magnum/MeshTools/Duplicate.h"
 #include "Magnum/MeshTools/Interleave.h"
 #include "Magnum/Trade/MeshData2D.h"
 #include "Magnum/Trade/MeshData3D.h"
@@ -121,16 +126,18 @@ std::tuple<GL::Mesh, std::unique_ptr<GL::Buffer>, std::unique_ptr<GL::Buffer>> c
 }
 #endif
 
-GL::Mesh compile(const Trade::MeshData3D& meshData) {
+GL::Mesh compile(const Trade::MeshData3D& meshData, CompileFlags flags) {
     GL::Mesh mesh;
     mesh.setPrimitive(meshData.primitive());
 
+    const bool generateNormals = flags & CompileFlag::GenerateFlatNormals && meshData.primitive() == MeshPrimitive::Triangles;
+
     /* Decide about stride and offsets */
     UnsignedInt stride = sizeof(Shaders::Generic3D::Position::Type);
     const UnsignedInt normalOffset = sizeof(Shaders::Generic3D::Position::Type);
     UnsignedInt textureCoordsOffset = sizeof(Shaders::Generic3D::Position::Type);
     UnsignedInt colorsOffset = sizeof(Shaders::Generic3D::Position::Type);
-    if(meshData.hasNormals()) {
+    if(meshData.hasNormals() || generateNormals) {
         stride += sizeof(Shaders::Generic3D::Normal::Type);
         textureCoordsOffset += sizeof(Shaders::Generic3D::Normal::Type);
         colorsOffset += sizeof(Shaders::Generic3D::Normal::Type);
@@ -146,20 +153,73 @@ GL::Mesh compile(const Trade::MeshData3D& meshData) {
     GL::Buffer vertexBuffer{GL::Buffer::TargetHint::Array};
     GL::Buffer vertexBufferRef = GL::Buffer::wrap(vertexBuffer.id(), GL::Buffer::TargetHint::Array);
 
+    /* Indirect reference to the mesh data -- either directly the original mesh
+       data or processed ones */
+    Containers::StridedArrayView1D<const Vector3> positions;
+    Containers::StridedArrayView1D<const Vector3> normals;
+    Containers::StridedArrayView1D<const Vector2> textureCoords2D;
+    Containers::StridedArrayView1D<const Color4> colors;
+    bool useIndices; /**< @todo turn into a view once compressIndices() takes views */
+
+    /* If the mesh has no normals, we want to generate them and the mesh is an
+       indexed triangle mesh, duplicate all attributes, otherwise just
+       reference the original data */
+    Containers::Array<Vector3> positionStorage;
+    Containers::Array<Vector3> normalStorage;
+    Containers::Array<Vector2> textureCoords2DStorage;
+    Containers::Array<Color4> colorStorage;
+    if(generateNormals) {
+        /* If the mesh is indexed, duplicate all attributes */
+        if(meshData.isIndexed()) {
+            positionStorage = duplicate(
+                Containers::stridedArrayView(meshData.indices()), Containers::stridedArrayView(meshData.positions(0)));
+            positions = Containers::arrayView(positionStorage);
+            if(meshData.hasTextureCoords2D()) {
+                textureCoords2DStorage = duplicate(
+                    Containers::stridedArrayView(meshData.indices()),
+                    Containers::stridedArrayView(meshData.textureCoords2D(0)));
+                textureCoords2D = Containers::arrayView(textureCoords2DStorage);
+            }
+            if(meshData.hasColors()) {
+                colorStorage = duplicate(
+                    Containers::stridedArrayView(meshData.indices()),
+                    Containers::stridedArrayView(meshData.colors(0)));
+                colors = Containers::arrayView(colorStorage);
+            }
+        } else {
+            positions = meshData.positions(0);
+            if(meshData.hasTextureCoords2D())
+                textureCoords2D = meshData.textureCoords2D(0);
+            if(meshData.hasColors())
+                colors = meshData.colors(0);
+        }
+
+        normalStorage = generateFlatNormals(positions);
+        normals = Containers::arrayView(normalStorage);
+        useIndices = false;
+
+    } else {
+        positions = meshData.positions(0);
+        if(meshData.hasNormals()) normals = meshData.normals(0);
+        if(meshData.hasTextureCoords2D()) textureCoords2D = meshData.textureCoords2D(0);
+        if(meshData.hasColors()) colors = meshData.colors(0);
+        useIndices = meshData.isIndexed();
+    }
+
     /* Interleave positions and put them in with ownership transfer, use the
        ref for the rest */
     Containers::Array<char> data = MeshTools::interleave(
-        meshData.positions(0),
+        positions,
         stride - sizeof(Shaders::Generic3D::Position::Type));
     mesh.addVertexBuffer(std::move(vertexBuffer), 0,
         Shaders::Generic3D::Position(),
         stride - sizeof(Shaders::Generic3D::Position::Type));
 
     /* Add also normals, if present */
-    if(meshData.hasNormals()) {
+    if(normals) {
         MeshTools::interleaveInto(data,
             normalOffset,
-            meshData.normals(0),
+            normals,
             stride - normalOffset - sizeof(Shaders::Generic3D::Normal::Type));
         mesh.addVertexBuffer(vertexBufferRef, 0,
             normalOffset,
@@ -168,10 +228,10 @@ GL::Mesh compile(const Trade::MeshData3D& meshData) {
     }
 
     /* Add also texture coordinates, if present */
-    if(meshData.hasTextureCoords2D()) {
+    if(textureCoords2D) {
         MeshTools::interleaveInto(data,
             textureCoordsOffset,
-            meshData.textureCoords2D(0),
+            textureCoords2D,
             stride - textureCoordsOffset - sizeof(Shaders::Generic3D::TextureCoordinates::Type));
         mesh.addVertexBuffer(vertexBufferRef, 0,
             textureCoordsOffset,
@@ -180,10 +240,10 @@ GL::Mesh compile(const Trade::MeshData3D& meshData) {
     }
 
     /* Add also colors, if present */
-    if(meshData.hasColors()) {
+    if(colors) {
         MeshTools::interleaveInto(data,
             colorsOffset,
-            meshData.colors(0),
+            colors,
             stride - colorsOffset - sizeof(Shaders::Generic3D::Color4::Type));
         mesh.addVertexBuffer(vertexBufferRef, 0,
             colorsOffset,
@@ -194,8 +254,9 @@ GL::Mesh compile(const Trade::MeshData3D& meshData) {
     /* Fill vertex buffer with interleaved data */
     vertexBufferRef.setData(data, GL::BufferUsage::StaticDraw);
 
-    /* If indexed, fill index buffer and configure indexed mesh */
+    /* If indexed (and the mesh didn't have the vertex data duplicated for flat
-    if(meshData.isIndexed()) {
+       normals), fill index buffer and configure indexed mesh */
+    if(useIndices) {
         Containers::Array<char> indexData;
         MeshIndexType indexType;
         UnsignedInt indexStart, indexEnd;
@@ -207,7 +268,7 @@ GL::Mesh compile(const Trade::MeshData3D& meshData) {
             .setIndexBuffer(std::move(indexBuffer), 0, indexType, indexStart, indexEnd);
 
     /* Else set vertex count */
-    } else mesh.setCount(meshData.positions(0).size());
+    } else mesh.setCount(positions.size());
 
     return mesh;
 }

src/Magnum/MeshTools/Compile.h

@@ -32,6 +32,9 @@
 #include "Magnum/configure.h"
 
 #ifdef MAGNUM_TARGET_GL
+#include <Corrade/Containers/EnumSet.h>
+
+#include "Magnum/Magnum.h"
 #include "Magnum/GL/GL.h"
 #include "Magnum/Trade/Trade.h"
 #include "Magnum/MeshTools/visibility.h"
@@ -44,6 +47,30 @@
 
 namespace Magnum { namespace MeshTools {
 
+/**
+@brief Mesh compilation flag
+
+@see @ref CompileFlags, @ref compile(const Trade::MeshData3D&, CompileFlags)
+*/
+enum class CompileFlag: UnsignedByte {
+    /**
+     * If the mesh is @ref MeshPrimitive::Triangles, generates normals using
+     * @ref MeshTools::generateFlatNormals(). If the mesh is not a triangle
+     * mesh or doesn't have 3D positions, this flag does nothing. If the mesh
+     * already has its own normals, these get replaced.
+     */
+    GenerateFlatNormals = 1 << 0
+};
+
+/**
+@brief Mesh compilation flags
+
+@see @ref compile(const Trade::MeshData3D&, CompileFlags)
+*/
+typedef Containers::EnumSet<CompileFlag> CompileFlags;
+
+CORRADE_ENUMSET_OPERATORS(CompileFlags)
+
 /**
 @brief Compile 2D mesh data
 
@@ -103,12 +130,13 @@ greater flexibility.
 
 @see @ref shaders-generic
 */
-MAGNUM_MESHTOOLS_EXPORT GL::Mesh compile(const Trade::MeshData3D& meshData);
+MAGNUM_MESHTOOLS_EXPORT GL::Mesh compile(const Trade::MeshData3D& meshData, CompileFlags flags = {});
 
 #ifdef MAGNUM_BUILD_DEPRECATED
-/** @brief @copybrief compile(const Trade::MeshData3D&)
+/** @brief @copybrief compile(const Trade::MeshData3D&, CompileFlags)
- * @deprecated Use @ref compile(const Trade::MeshData3D&) instead. The @p usage
+ * @deprecated Use @ref compile(const Trade::MeshData3D&, CompileFlags)
- *      parameter is ignored and returned buffer instances are empty.
+ *      instead. The @p usage parameter is ignored and returned buffer
+ *      instances are empty.
  */
 CORRADE_DEPRECATED("use compile(const Trade::MeshData3D&) instead") MAGNUM_MESHTOOLS_EXPORT std::tuple<GL::Mesh, std::unique_ptr<GL::Buffer>, std::unique_ptr<GL::Buffer>> compile(const Trade::MeshData3D& meshData, GL::BufferUsage usage);
 #endif

src/Magnum/MeshTools/GenerateNormals.h

@@ -62,22 +62,20 @@ MAGNUM_MESHTOOLS_EXPORT Containers::Array<Vector3> generateFlatNormals(const Con
 @param[out] normals     Where to put the generated normals
 
 A variant of @ref generateFlatNormals() that fills existing memory instead of
-allocating a new array.
+allocating a new array. The @p normals array is expected to have the same size
+as @p positions.
 */
 MAGNUM_MESHTOOLS_EXPORT void generateFlatNormalsInto(const Containers::StridedArrayView1D<const Vector3>& positions, const Containers::StridedArrayView1D<Vector3>& normals);
 
 #ifdef MAGNUM_BUILD_DEPRECATED
 /**
 @brief Generate flat normals
-@param indices      Array of triangle face indices
+@param indices      Triangle face indices
-@param positions    Array of vertex positions
+@param positions    Triangle vertex positions
 @return Normal indices and vectors
 
 All vertices in each triangle face get the same normal vector. Removes
-duplicates before returning.
+duplicates before returning. Expects that the position count is divisible by 3.
-
-@attention The function requires the mesh to have triangle faces, thus index
-    count must be divisible by 3.
 
 @deprecated This will generate index buffer that's different from the input
     @p indices array, so you'll need to recombine them using

src/Magnum/MeshTools/Test/CMakeLists.txt

@@ -114,6 +114,7 @@ if(BUILD_GL_TESTS)
             CompileTestFiles/flat2D.tga
             CompileTestFiles/flat3D.tga
             CompileTestFiles/phong.tga
+            CompileTestFiles/phong-flat.tga
             CompileTestFiles/textured2D.tga
             CompileTestFiles/textured3D.tga)
     set_target_properties(MeshToolsCompileGLTest PROPERTIES FOLDER "Magnum/MeshTools/Test")

src/Magnum/MeshTools/Test/CompileGLTest.cpp

@@ -55,8 +55,9 @@ namespace Magnum { namespace MeshTools { namespace Test { namespace {
 enum class Flag {
     NonIndexed = 1 << 0,
     Normals = 1 << 1,
-    TextureCoordinates2D = 1 << 2,
+    GeneratedFlatNormals = 1 << 2,
-    Colors = 1 << 3
+    TextureCoordinates2D = 1 << 3,
+    Colors = 1 << 4
 };
 
 typedef Containers::EnumSet<Flag> Flags;
@@ -108,7 +109,16 @@ constexpr struct {
     {"positions + normals", Flag::Normals},
     {"positions + normals + colors", Flag::Normals|Flag::Colors},
     {"positions + normals + texcoords", Flag::Normals|Flag::TextureCoordinates2D},
-    {"positions + normals + texcoords + colors", Flag::Normals|Flag::TextureCoordinates2D|Flag::Colors}
+    {"positions + normals + texcoords + colors", Flag::Normals|Flag::TextureCoordinates2D|Flag::Colors},
+    {"positions + gen flat normals", Flag::GeneratedFlatNormals},
+    {"positions + normals, gen flat normals", Flag::Normals|Flag::GeneratedFlatNormals},
+    {"positions + gen flat normals + colors", Flag::GeneratedFlatNormals|Flag::Colors},
+    {"positions + gen flat normals + texcoords", Flag::GeneratedFlatNormals|Flag::TextureCoordinates2D},
+    {"positions + gen flat normals + texcoords + colors", Flag::NonIndexed|Flag::GeneratedFlatNormals|Flag::TextureCoordinates2D|Flag::Colors},
+    {"positions, nonindexed + gen flat normals", Flag::NonIndexed|Flag::GeneratedFlatNormals},
+    {"positions, nonindexed + gen flat normals + colors", Flag::NonIndexed|Flag::GeneratedFlatNormals|Flag::Colors},
+    {"positions, nonindexed + gen flat normals + texcoords", Flag::NonIndexed|Flag::GeneratedFlatNormals|Flag::TextureCoordinates2D},
+    {"positions, nonindexed + gen flat normals + texcoords + colors", Flag::NonIndexed|Flag::GeneratedFlatNormals|Flag::TextureCoordinates2D|Flag::Colors}
 };
 
 using namespace Math::Literals;
@@ -366,19 +376,26 @@ void CompileGLTest::threeDimensions() {
         4, 5, 8, 4, 8, 7
     };
 
-    /* Duplicate positions if data are non-indexed. Testing only positions
+    /* Duplicate everything if data are non-indexed */
-       alone ATM, don't bother with other attribs. */
     if(data.flags & Flag::NonIndexed) {
-        CORRADE_INTERNAL_ASSERT(normals.empty());
-        CORRADE_INTERNAL_ASSERT(textureCoordinates2D.empty());
-        CORRADE_INTERNAL_ASSERT(colors.empty());
         positions = duplicate(indices, positions);
+
+        if(data.flags & Flag::Normals)
+            normals[0] = duplicate(indices, normals[0]);
+
+        if(data.flags & Flag::TextureCoordinates2D)
+            textureCoordinates2D[0] = duplicate(indices, textureCoordinates2D[0]);
+
+        if(data.flags & Flag::Colors)
+            colors[0] = duplicate(indices, colors[0]);
+
         indices.clear();
     }
 
     MAGNUM_VERIFY_NO_GL_ERROR();
 
-    GL::Mesh mesh = compile(Trade::MeshData3D{MeshPrimitive::Triangles, indices, {positions}, normals, textureCoordinates2D, colors});
+    GL::Mesh mesh = compile(Trade::MeshData3D{MeshPrimitive::Triangles, indices, {positions}, normals, textureCoordinates2D, colors},
+        data.flags & Flag::GeneratedFlatNormals ? CompileFlag::GenerateFlatNormals : CompileFlags{});
 
     MAGNUM_VERIFY_NO_GL_ERROR();
 
@@ -404,8 +421,8 @@ void CompileGLTest::threeDimensions() {
             (DebugTools::CompareImageToFile{_manager}));
     }
 
-    /* Check with the phong shader, if we have normals */
+    /* Check with the phong shader, if we have normals (but not flat generated) */
-    if(data.flags & Flag::Normals) {
+    if(data.flags & Flag::Normals && !(data.flags & Flag::GeneratedFlatNormals)) {
         _framebuffer.clear(GL::FramebufferClear::Color);
         _phong
             .setDiffuseColor(0x33ff66_rgbf)
@@ -422,6 +439,24 @@ void CompileGLTest::threeDimensions() {
             (DebugTools::CompareImageToFile{_manager, 0.5f, 0.0113f}));
     }
 
+    /* Check generated flat normals with the phong shader */
+    if(data.flags & Flag::GeneratedFlatNormals) {
+        _framebuffer.clear(GL::FramebufferClear::Color);
+        _phong
+            .setDiffuseColor(0x33ff66_rgbf)
+            .setTransformationMatrix(transformation)
+            .setNormalMatrix(transformation.rotationScaling())
+            .setProjectionMatrix(projection);
+        mesh.draw(_phong);
+
+        MAGNUM_VERIFY_NO_GL_ERROR();
+        CORRADE_COMPARE_WITH(
+            _framebuffer.read({{}, {32, 32}}, {PixelFormat::RGBA8Unorm}),
+            Utility::Directory::join(COMPILEGLTEST_TEST_DIR, "phong-flat.tga"),
+            /* SwiftShader has some minor off-by-one precision differences */
+            (DebugTools::CompareImageToFile{_manager, 0.25f, 0.0079f}));
+    }
+
     /* Check with the colored shader, if we have colors */
     if(data.flags & Flag::Colors) {
         _framebuffer.clear(GL::FramebufferClear::Color);

src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp

@@ -40,19 +40,19 @@ struct GenerateNormalsTest: TestSuite::Tester {
     explicit GenerateNormalsTest();
 
     void flat();
-    void flatWrongCount();
     #ifdef MAGNUM_BUILD_DEPRECATED
     void flatDeprecated();
     #endif
+    void flatWrongCount();
     void flatIntoWrongSize();
 };
 
 GenerateNormalsTest::GenerateNormalsTest() {
     addTests({&GenerateNormalsTest::flat,
-              &GenerateNormalsTest::flatWrongCount,
               #ifdef MAGNUM_BUILD_DEPRECATED
               &GenerateNormalsTest::flatDeprecated,
               #endif
+              &GenerateNormalsTest::flatWrongCount,
               &GenerateNormalsTest::flatIntoWrongSize});
 }
 
@@ -79,15 +79,6 @@ void GenerateNormalsTest::flat() {
         }}), TestSuite::Compare::Container);
 }
 
-void GenerateNormalsTest::flatWrongCount() {
-    std::stringstream out;
-    Error redirectError{&out};
-
-    const Vector3 positions[7];
-    generateFlatNormals(positions);
-    CORRADE_COMPARE(out.str(), "MeshTools::generateFlatNormalsInto(): position count not divisible by 3\n");
-}
-
 #ifdef MAGNUM_BUILD_DEPRECATED
 void GenerateNormalsTest::flatDeprecated() {
     /* Two vertices connected by one edge, each wound in another direction */
@@ -116,6 +107,15 @@ void GenerateNormalsTest::flatDeprecated() {
 }
 #endif
 
+void GenerateNormalsTest::flatWrongCount() {
+    std::stringstream out;
+    Error redirectError{&out};
+
+    const Vector3 positions[7];
+    generateFlatNormals(positions);
+    CORRADE_COMPARE(out.str(), "MeshTools::generateFlatNormalsInto(): position count not divisible by 3\n");
+}
+
 void GenerateNormalsTest::flatIntoWrongSize() {
     std::stringstream out;
     Error redirectError{&out};