From 424eec48186befd37b7b1a5cbcbea83433c653ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 3 Jun 2018 23:42:24 +0200 Subject: [PATCH] MeshTools: simplify compile(). I wanted to do this since *ever*. --- doc/changelog.dox | 9 ++++ doc/snippets/MagnumSceneGraph-gl.cpp | 3 +- src/Magnum/MeshTools/Compile.cpp | 64 ++++++++++++++++++---------- src/Magnum/MeshTools/Compile.h | 56 ++++++++++++++++-------- 4 files changed, 89 insertions(+), 43 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 122a7795a..439670b7b 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -67,6 +67,11 @@ See also: @ref GL::Buffer to simplify resource management on user side. See @ref GL-Mesh-buffer-ownership for more information. +@subsubsection changelog-latest-changes-meshtools MeshTools library + +- @ref MeshTools::compile() API got simplified to make use of the new buffer + ownership feature of @ref GL::Mesh + @subsubsection changelog-latest-changes-platform Platform libraries - @ref Platform::GlfwApplication now behaves the same as @@ -97,6 +102,10 @@ See also: deeply nested, use @ref Math::Distance and @ref Math::Intersection instead - `Math::Geometry::Intersection::boxFrustum()` is deprecated, use @ref Math::Intersection::rangeFrustum() instead +- @ref MeshTools::compile() taking a @ref GL::BufferUsage and returning a + tuple was deprecated, use the simpler version taking just + @ref Trade::MeshData2D / @ref Trade::MeshData3D and directly returning a + @ref GL::Mesh instead @section changelog-2018-04 2018.04 diff --git a/doc/snippets/MagnumSceneGraph-gl.cpp b/doc/snippets/MagnumSceneGraph-gl.cpp index d1600b679..f1f9b63a5 100644 --- a/doc/snippets/MagnumSceneGraph-gl.cpp +++ b/doc/snippets/MagnumSceneGraph-gl.cpp @@ -93,7 +93,7 @@ typedef SceneGraph::Scene Scene3D; class RedCube: public Object3D, public SceneGraph::Drawable3D { public: explicit RedCube(Object3D* parent, SceneGraph::DrawableGroup3D* group): Object3D{parent}, SceneGraph::Drawable3D{*this, group} { - std::tie(_mesh, _vertices, _indices) = MeshTools::compile(Primitives::cubeSolid(), GL::BufferUsage::StaticDraw); + _mesh = MeshTools::compile(Primitives::cubeSolid()); } private: @@ -107,7 +107,6 @@ class RedCube: public Object3D, public SceneGraph::Drawable3D { } GL::Mesh _mesh; - std::unique_ptr _vertices, _indices; Shaders::Phong _shader; }; /* [Drawable-usage] */ diff --git a/src/Magnum/MeshTools/Compile.cpp b/src/Magnum/MeshTools/Compile.cpp index 024090632..001b56a8c 100644 --- a/src/Magnum/MeshTools/Compile.cpp +++ b/src/Magnum/MeshTools/Compile.cpp @@ -39,7 +39,7 @@ namespace Magnum { namespace MeshTools { -std::tuple, std::unique_ptr> compile(const Trade::MeshData2D& meshData, const GL::BufferUsage usage) { +GL::Mesh compile(const Trade::MeshData2D& meshData) { GL::Mesh mesh; mesh.setPrimitive(meshData.primitive()); @@ -50,13 +50,15 @@ std::tuple, std::unique_ptr> c stride += sizeof(Shaders::Generic2D::TextureCoordinates::Type); /* Create vertex buffer */ - std::unique_ptr vertexBuffer{new GL::Buffer{GL::Buffer::TargetHint::Array}}; + GL::Buffer vertexBuffer{GL::Buffer::TargetHint::Array}; + GL::Buffer vertexBufferRef = GL::Buffer::wrap(vertexBuffer.id()); - /* Interleave positions */ + /* Interleave positions and put them in with ownership transfer, use the + ref for the rest */ Containers::Array data = MeshTools::interleave( meshData.positions(0), stride - sizeof(Shaders::Generic2D::Position::Type)); - mesh.addVertexBuffer(*vertexBuffer, 0, + mesh.addVertexBuffer(std::move(vertexBuffer), 0, Shaders::Generic2D::Position(), stride - sizeof(Shaders::Generic2D::Position::Type)); @@ -66,35 +68,42 @@ std::tuple, std::unique_ptr> c normalOffset, meshData.textureCoords2D(0), stride - normalOffset - sizeof(Shaders::Generic2D::TextureCoordinates::Type)); - mesh.addVertexBuffer(*vertexBuffer, 0, + mesh.addVertexBuffer(vertexBufferRef, 0, normalOffset, Shaders::Generic2D::TextureCoordinates(), stride - normalOffset - sizeof(Shaders::Generic2D::TextureCoordinates::Type)); } /* Fill vertex buffer with interleaved data */ - vertexBuffer->setData(data, usage); + vertexBufferRef.setData(data, GL::BufferUsage::StaticDraw); /* If indexed, fill index buffer and configure indexed mesh */ - std::unique_ptr indexBuffer; if(meshData.isIndexed()) { Containers::Array indexData; MeshIndexType indexType; UnsignedInt indexStart, indexEnd; std::tie(indexData, indexType, indexStart, indexEnd) = MeshTools::compressIndices(meshData.indices()); - indexBuffer.reset(new GL::Buffer{GL::Buffer::TargetHint::ElementArray}); - indexBuffer->setData(indexData, usage); + GL::Buffer indexBuffer{GL::Buffer::TargetHint::ElementArray}; + indexBuffer.setData(indexData, GL::BufferUsage::StaticDraw); mesh.setCount(meshData.indices().size()) - .setIndexBuffer(*indexBuffer, 0, indexType, indexStart, indexEnd); + .setIndexBuffer(std::move(indexBuffer), 0, indexType, indexStart, indexEnd); /* Else set vertex count */ } else mesh.setCount(meshData.positions(0).size()); - return std::make_tuple(std::move(mesh), std::move(vertexBuffer), std::move(indexBuffer)); + return mesh; } -std::tuple, std::unique_ptr> compile(const Trade::MeshData3D& meshData, const GL::BufferUsage usage) { +#ifdef MAGNUM_BUILD_DEPRECATED +std::tuple, std::unique_ptr> compile(const Trade::MeshData2D& meshData, GL::BufferUsage) { + return std::make_tuple(compile(meshData), + std::unique_ptr{new GL::Buffer{NoCreate}}, + std::unique_ptr{meshData.isIndexed() ? new GL::Buffer{NoCreate} : nullptr}); +} +#endif + +GL::Mesh compile(const Trade::MeshData3D& meshData) { GL::Mesh mesh; mesh.setPrimitive(meshData.primitive()); @@ -110,13 +119,15 @@ std::tuple, std::unique_ptr> c stride += sizeof(Shaders::Generic3D::TextureCoordinates::Type); /* Create vertex buffer */ - std::unique_ptr vertexBuffer{new GL::Buffer{GL::Buffer::TargetHint::Array}}; + GL::Buffer vertexBuffer{GL::Buffer::TargetHint::Array}; + GL::Buffer vertexBufferRef = GL::Buffer::wrap(vertexBuffer.id()); - /* Interleave positions */ + /* Interleave positions and put them in with ownership transfer, use the + ref for the rest */ Containers::Array data = MeshTools::interleave( meshData.positions(0), stride - sizeof(Shaders::Generic3D::Position::Type)); - mesh.addVertexBuffer(*vertexBuffer, 0, + mesh.addVertexBuffer(std::move(vertexBuffer), 0, Shaders::Generic3D::Position(), stride - sizeof(Shaders::Generic3D::Position::Type)); @@ -126,7 +137,7 @@ std::tuple, std::unique_ptr> c normalOffset, meshData.normals(0), stride - normalOffset - sizeof(Shaders::Generic3D::Normal::Type)); - mesh.addVertexBuffer(*vertexBuffer, 0, + mesh.addVertexBuffer(vertexBufferRef, 0, normalOffset, Shaders::Generic3D::Normal(), stride - normalOffset - sizeof(Shaders::Generic3D::Normal::Type)); @@ -138,32 +149,39 @@ std::tuple, std::unique_ptr> c textureCoordsOffset, meshData.textureCoords2D(0), stride - textureCoordsOffset - sizeof(Shaders::Generic3D::TextureCoordinates::Type)); - mesh.addVertexBuffer(*vertexBuffer, 0, + mesh.addVertexBuffer(vertexBufferRef, 0, textureCoordsOffset, Shaders::Generic3D::TextureCoordinates(), stride - textureCoordsOffset - sizeof(Shaders::Generic3D::TextureCoordinates::Type)); } /* Fill vertex buffer with interleaved data */ - vertexBuffer->setData(data, usage); + vertexBufferRef.setData(data, GL::BufferUsage::StaticDraw); /* If indexed, fill index buffer and configure indexed mesh */ - std::unique_ptr indexBuffer; if(meshData.isIndexed()) { Containers::Array indexData; MeshIndexType indexType; UnsignedInt indexStart, indexEnd; std::tie(indexData, indexType, indexStart, indexEnd) = MeshTools::compressIndices(meshData.indices()); - indexBuffer.reset(new GL::Buffer{GL::Buffer::TargetHint::ElementArray}); - indexBuffer->setData(indexData, usage); + GL::Buffer indexBuffer{GL::Buffer::TargetHint::ElementArray}; + indexBuffer.setData(indexData, GL::BufferUsage::StaticDraw); mesh.setCount(meshData.indices().size()) - .setIndexBuffer(*indexBuffer, 0, indexType, indexStart, indexEnd); + .setIndexBuffer(std::move(indexBuffer), 0, indexType, indexStart, indexEnd); /* Else set vertex count */ } else mesh.setCount(meshData.positions(0).size()); - return std::make_tuple(std::move(mesh), std::move(vertexBuffer), std::move(indexBuffer)); + return mesh; +} + +#ifdef MAGNUM_BUILD_DEPRECATED +std::tuple, std::unique_ptr> compile(const Trade::MeshData3D& meshData, GL::BufferUsage) { + return std::make_tuple(compile(meshData), + std::unique_ptr{new GL::Buffer{NoCreate}}, + std::unique_ptr{meshData.isIndexed() ? new GL::Buffer{NoCreate} : nullptr}); } +#endif }} diff --git a/src/Magnum/MeshTools/Compile.h b/src/Magnum/MeshTools/Compile.h index 13145cc95..4e338a1e6 100644 --- a/src/Magnum/MeshTools/Compile.h +++ b/src/Magnum/MeshTools/Compile.h @@ -32,30 +32,33 @@ #include "Magnum/configure.h" #ifdef MAGNUM_TARGET_GL -#include -#include - #include "Magnum/GL/GL.h" #include "Magnum/Trade/Trade.h" #include "Magnum/MeshTools/visibility.h" +#ifdef MAGNUM_BUILD_DEPRECATED +#include +#include +#include +#endif + namespace Magnum { namespace MeshTools { /** @brief Compile 2D mesh data -Configures mesh for @ref Shaders::Generic2D shader with vertex buffer and -possibly also index buffer, if the mesh is indexed. Positions are bound to +Configures a mesh for @ref Shaders::Generic2D shader with vertex buffer and +possibly also an index buffer, if the mesh is indexed. Positions are bound to @ref Shaders::Generic2D::Position attribute. If the mesh contains texture -coordinates, they are bound to @ref Shaders::Generic2D::TextureCoordinates +coordinates, these are bound to @ref Shaders::Generic2D::TextureCoordinates attribute. No data compression or index optimization (except for index buffer -packing) is done. The @p usage parameter is used for both vertex and index -buffer. - -The second returned buffer may be @cpp nullptr @ce if the mesh is not indexed. +packing) is done, both the vertex buffer and the index buffer (if any) is owned +by the mesh, both created with @ref GL::BufferUsage::StaticDraw. This is just a convenience function for creating generic meshes, you might want -to use @ref interleave() and @ref compressIndices() functions instead for +to use @ref interleave() and @ref compressIndices() functions together with +@ref GL::Mesh::setPrimitive(), @ref GL::Mesh::setCount(), +@ref GL::Mesh::addVertexBuffer(), @ref GL::Mesh::setIndexBuffer() instead for greater flexibility. @note This function is available only if Magnum is compiled with @@ -64,7 +67,15 @@ greater flexibility. @see @ref shaders-generic */ -MAGNUM_MESHTOOLS_EXPORT std::tuple, std::unique_ptr> compile(const Trade::MeshData2D& meshData, GL::BufferUsage usage); +MAGNUM_MESHTOOLS_EXPORT GL::Mesh compile(const Trade::MeshData2D& meshData); + +#ifdef MAGNUM_BUILD_DEPRECATED +/** @brief @copybrief compile(const Trade::MeshData2D&) + * @deprecated Use @ref compile(const Trade::MeshData2D&) instead. The @p usage + * parameter is ignored and returned buffer instances are empty. + */ +CORRADE_DEPRECATED("use compile(const Trade::MeshData2D&) instead") MAGNUM_MESHTOOLS_EXPORT std::tuple, std::unique_ptr> compile(const Trade::MeshData2D& meshData, GL::BufferUsage usage); +#endif /** @brief Compile 3D mesh data @@ -74,13 +85,14 @@ possibly also index buffer, if the mesh is indexed. Positions are bound to @ref Shaders::Generic3D::Position attribute. If the mesh contains normals, they are bound to @ref Shaders::Generic3D::Normal attribute, texture coordinates are bound to @ref Shaders::Generic2D::TextureCoordinates attribute. No data -compression or index optimization (except for index buffer packing) is done. -The @p usage parameter is used for both vertex and index buffer. - -The second returned buffer may be @cpp nullptr @ce if the mesh is not indexed. +compression or index optimization (except for index buffer packing) is done, +both the vertex buffer and the index buffer (if any) is owned by the mesh, both +created with @ref GL::BufferUsage::StaticDraw. This is just a convenience function for creating generic meshes, you might want -to use @ref interleave() and @ref compressIndices() functions instead for +to use @ref interleave() and @ref compressIndices() functions together with +@ref GL::Mesh::setPrimitive(), @ref GL::Mesh::setCount(), +@ref GL::Mesh::addVertexBuffer(), @ref GL::Mesh::setIndexBuffer() instead for greater flexibility. @note This function is available only if Magnum is compiled with @@ -89,7 +101,15 @@ greater flexibility. @see @ref shaders-generic */ -MAGNUM_MESHTOOLS_EXPORT std::tuple, std::unique_ptr> compile(const Trade::MeshData3D& meshData, GL::BufferUsage usage); +MAGNUM_MESHTOOLS_EXPORT GL::Mesh compile(const Trade::MeshData3D& meshData); + +#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. + */ +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 }} #else