From d20a17c5c357bf7d8a307ed712110484108420b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 14 Apr 2019 14:26:20 +0200 Subject: [PATCH] MeshTools: ability to generate flat normals in compile(). --- doc/changelog.dox | 2 + src/Magnum/MeshTools/Compile.cpp | 85 +++++++++++++++--- src/Magnum/MeshTools/Compile.h | 36 +++++++- src/Magnum/MeshTools/GenerateNormals.h | 12 ++- src/Magnum/MeshTools/Test/CMakeLists.txt | 1 + src/Magnum/MeshTools/Test/CompileGLTest.cpp | 57 +++++++++--- .../Test/CompileTestFiles/phong-flat.tga | Bin 0 -> 4114 bytes .../MeshTools/Test/GenerateNormalsTest.cpp | 22 ++--- 8 files changed, 170 insertions(+), 45 deletions(-) create mode 100644 src/Magnum/MeshTools/Test/CompileTestFiles/phong-flat.tga diff --git a/doc/changelog.dox b/doc/changelog.dox index 5ce2dad2b..a2a0354e6 100644 --- a/doc/changelog.dox +++ b/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 diff --git a/src/Magnum/MeshTools/Compile.cpp b/src/Magnum/MeshTools/Compile.cpp index 136cd25a8..99d3c2ac9 100644 --- a/src/Magnum/MeshTools/Compile.cpp +++ b/src/Magnum/MeshTools/Compile.cpp @@ -25,11 +25,16 @@ #include "Compile.h" +#include +#include /** @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, std::unique_ptr> 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 positions; + Containers::StridedArrayView1D normals; + Containers::StridedArrayView1D textureCoords2D; + Containers::StridedArrayView1D 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 positionStorage; + Containers::Array normalStorage; + Containers::Array textureCoords2DStorage; + Containers::Array 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 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(meshData.isIndexed()) { + /* If indexed (and the mesh didn't have the vertex data duplicated for flat + normals), fill index buffer and configure indexed mesh */ + if(useIndices) { Containers::Array 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; } diff --git a/src/Magnum/MeshTools/Compile.h b/src/Magnum/MeshTools/Compile.h index 5221ab634..23aec67f2 100644 --- a/src/Magnum/MeshTools/Compile.h +++ b/src/Magnum/MeshTools/Compile.h @@ -32,6 +32,9 @@ #include "Magnum/configure.h" #ifdef MAGNUM_TARGET_GL +#include + +#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 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&) - * @deprecated Use @ref compile(const Trade::MeshData3D&) instead. The @p usage - * parameter is ignored and returned buffer instances are empty. +/** @brief @copybrief compile(const Trade::MeshData3D&, CompileFlags) + * @deprecated Use @ref compile(const Trade::MeshData3D&, CompileFlags) + * 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, std::unique_ptr> compile(const Trade::MeshData3D& meshData, GL::BufferUsage usage); #endif diff --git a/src/Magnum/MeshTools/GenerateNormals.h b/src/Magnum/MeshTools/GenerateNormals.h index e1ffe8787..eaaf4f8f9 100644 --- a/src/Magnum/MeshTools/GenerateNormals.h +++ b/src/Magnum/MeshTools/GenerateNormals.h @@ -62,22 +62,20 @@ MAGNUM_MESHTOOLS_EXPORT Containers::Array 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& positions, const Containers::StridedArrayView1D& normals); #ifdef MAGNUM_BUILD_DEPRECATED /** @brief Generate flat normals -@param indices Array of triangle face indices -@param positions Array of vertex positions +@param indices Triangle face indices +@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. - -@attention The function requires the mesh to have triangle faces, thus index - count must be divisible by 3. +duplicates before returning. Expects that the position count is 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 diff --git a/src/Magnum/MeshTools/Test/CMakeLists.txt b/src/Magnum/MeshTools/Test/CMakeLists.txt index 5ccfcd85b..3661084b2 100644 --- a/src/Magnum/MeshTools/Test/CMakeLists.txt +++ b/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") diff --git a/src/Magnum/MeshTools/Test/CompileGLTest.cpp b/src/Magnum/MeshTools/Test/CompileGLTest.cpp index af1e1c94c..491bb591c 100644 --- a/src/Magnum/MeshTools/Test/CompileGLTest.cpp +++ b/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, - Colors = 1 << 3 + GeneratedFlatNormals = 1 << 2, + TextureCoordinates2D = 1 << 3, + Colors = 1 << 4 }; typedef Containers::EnumSet 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 - alone ATM, don't bother with other attribs. */ + /* Duplicate everything if data are non-indexed */ 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 */ - if(data.flags & Flag::Normals) { + /* Check with the phong shader, if we have normals (but not flat generated) */ + 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); diff --git a/src/Magnum/MeshTools/Test/CompileTestFiles/phong-flat.tga b/src/Magnum/MeshTools/Test/CompileTestFiles/phong-flat.tga new file mode 100644 index 0000000000000000000000000000000000000000..3b9e47aba95a324b81751199473a27f97cbb920a GIT binary patch literal 4114 zcmeH~*KQO+6htk4#601!4*ZC7SO*MzDkA5cbIyoQXzG;vwtIKoUh{y&gF!7zW@f5x zRo$9lWaQXL{5d|t-_g<0baeh-4dyS5rG-m6m&eoM6`gD2Y3aJojfu2$b0TejoJrds zrqkAYop;k|^Nr5yskHHODs^8>{k6Aq=T4-#^J8iL;x29vrtp?;>D-=3D|dA6PNoHM zI`v5$Y94QQd2GBA4{_)|>!kH3owW9-Gn5b5p7rY9oIRNLCe!MDod=U?O$RL&b?vDy zGrc_ewa6TXWu3Ws9@>KIIcv;^!qpt>k9TK63%tOWov-5XIrGS#Y@$W@Ar8av@tT>y zUImvq!OVE;PdnDL`&_!}a19)n#h3fcq0)j}6b?i3K-Tb^d3Yw+p0mQlPqKx#%mp|6 zAPaCdhi|hzewc%D$vp>UlG)MGx@#_;?Ron!$@rFZCGU|dxHdW9Ma~1Io#FZD#=L>Kt$kzX$eQ;&bHJ+s4mp!> z4Ii5L`)H5f_>6u$!}(Ue$k~p3MwjrlD;=5~=!on?3%^Hu?O2cYXwJ-F3-|PQogcG% zzUFKVcp*K>6!(P&P5i#x_iK;l%nkOLpE|#0%S=Uw$b_rRT zdriib757l~gPn5!!htTL`{*EXU~XgQ&Lg9-XWmWq2mHxG_|U|!bY&L%z&l8pfCF=| zV>Jgf_uS&#uRXi#eNYZo$bp{4TrGUh#Gd6nZ*eGIC=c!fH0S6(?|7}fvg1xddv;Oz zYU0x~x#!$(dDr`OC|=0hf+^nk!TsJvaic%n%a!(Yd<(zyeC0#*%>V~7so82w@=iAF zdqaEPaV`AX*OEo)x-SRzLcZ3R{??Kv{@$v)I66q$^Nw%f^ZV#~z8MBML?+n_J(F%> zCw-(fx$yf~Id%6av+ke#U2xFs!KrkjP7~{4FGo9pAPs-1xn_j=qIM z-8JEdwD5i^7j!k<;5h?Rvl>$xv1@3~++g!9zm> literal 0 HcmV?d00001 diff --git a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp b/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp index 13eaa676a..e8b596a96 100644 --- a/src/Magnum/MeshTools/Test/GenerateNormalsTest.cpp +++ b/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};