From 87fd89c66f576204041ef5b7aa7a26b50975a8bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 16 Aug 2022 15:52:45 +0200 Subject: [PATCH] MeshTools: use Containers::Iterable in concatenate*() and combine*(). This allows people to directly pass Containers::Array there, without having to put them to an annoying temporary Containers::Array. The Iterable header is included for backwards compatibility, apart from that there should be no breaking change. --- doc/changelog.dox | 2 +- src/Magnum/MeshTools/Combine.cpp | 40 ++++++++++------------- src/Magnum/MeshTools/Combine.h | 15 +++++---- src/Magnum/MeshTools/Concatenate.cpp | 38 ++++++++++----------- src/Magnum/MeshTools/Concatenate.h | 26 ++++----------- src/Magnum/MeshTools/InterleaveFlags.h | 6 ++-- src/Magnum/MeshTools/Test/CombineTest.cpp | 2 +- 7 files changed, 53 insertions(+), 76 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 50e188a76..347ff6d51 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -428,7 +428,7 @@ See also: - @ref MeshTools::interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags), @ref MeshTools::interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags) and - @ref MeshTools::concatenate(Containers::ArrayView>, InterleaveFlags) + @ref MeshTools::concatenate(Containers::Iterable, InterleaveFlags) optionally take a @ref MeshTools::InterleaveFlags parameter affecting the output, in particular whether to preserve the original interleaved layout. diff --git a/src/Magnum/MeshTools/Combine.cpp b/src/Magnum/MeshTools/Combine.cpp index 0ca545e2d..fcf08792f 100644 --- a/src/Magnum/MeshTools/Combine.cpp +++ b/src/Magnum/MeshTools/Combine.cpp @@ -26,8 +26,7 @@ #include "Combine.h" #include -#include -#include +#include #include #include "Magnum/Math/Functions.h" @@ -44,20 +43,20 @@ Trade::MeshData combineIndexedImplementation( #ifndef CORRADE_NO_ASSERT const char* assertPrefix, #endif - const MeshPrimitive primitive, Containers::Array& combinedIndices, const UnsignedInt indexCount, const UnsignedInt indexStride, const Containers::ArrayView> data) + const MeshPrimitive primitive, Containers::Array& combinedIndices, const UnsignedInt indexCount, const UnsignedInt indexStride, const Containers::Iterable data) { /* Calculate attribute count and vertex stride */ UnsignedInt attributeCount = 0; UnsignedInt vertexStride = 0; for(std::size_t i = 0; i != data.size(); ++i) { - attributeCount += data[i]->attributeCount(); - for(UnsignedInt j = 0; j != data[i]->attributeCount(); ++j) { - const VertexFormat format = data[i]->attributeFormat(j); + attributeCount += data[i].attributeCount(); + for(UnsignedInt j = 0; j != data[i].attributeCount(); ++j) { + const VertexFormat format = data[i].attributeFormat(j); CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), assertPrefix << "attribute" << j << "of mesh" << i << "has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), (Trade::MeshData{MeshPrimitive::Points, 0})); - vertexStride += vertexFormatSize(format)*Math::max(data[i]->attributeArraySize(j), UnsignedShort{1}); + vertexStride += vertexFormatSize(format)*Math::max(data[i].attributeArraySize(j), UnsignedShort{1}); } } @@ -111,7 +110,7 @@ Trade::MeshData combineIndexedImplementation( } -Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data) { +Trade::MeshData combineIndexedAttributes(const Containers::Iterable data) { CORRADE_ASSERT(!data.isEmpty(), "MeshTools::combineIndexedAttributes(): no meshes passed", (Trade::MeshData{MeshPrimitive{}, 0})); @@ -124,21 +123,21 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayViewisIndexed(), + CORRADE_ASSERT(data[i].isIndexed(), "MeshTools::combineIndexedAttributes(): data" << i << "is not indexed", (Trade::MeshData{MeshPrimitive{}, 0})); - const MeshIndexType indexType = data[i]->indexType(); + const MeshIndexType indexType = data[i].indexType(); CORRADE_ASSERT(!isMeshIndexTypeImplementationSpecific(indexType), "MeshTools::combineIndexedAttributes(): data" << i << "has an implementation-specific index type" << reinterpret_cast(meshIndexTypeUnwrap(indexType)), (Trade::MeshData{MeshPrimitive{}, 0})); if(i == 0) { - primitive = data[i]->primitive(); - indexCount = data[i]->indexCount(); + primitive = data[i].primitive(); + indexCount = data[i].indexCount(); } else { - CORRADE_ASSERT(data[i]->primitive() == primitive, - "MeshTools::combineIndexedAttributes(): data" << i << "is" << data[i]->primitive() << "but expected" << primitive, (Trade::MeshData{MeshPrimitive{}, 0})); - CORRADE_ASSERT(data[i]->indexCount() == indexCount, - "MeshTools::combineIndexedAttributes(): data" << i << "has" << data[i]->indexCount() << "indices but expected" << indexCount, (Trade::MeshData{MeshPrimitive{}, 0})); + CORRADE_ASSERT(data[i].primitive() == primitive, + "MeshTools::combineIndexedAttributes(): data" << i << "is" << data[i].primitive() << "but expected" << primitive, (Trade::MeshData{MeshPrimitive{}, 0})); + CORRADE_ASSERT(data[i].indexCount() == indexCount, + "MeshTools::combineIndexedAttributes(): data" << i << "has" << data[i].indexCount() << "indices but expected" << indexCount, (Trade::MeshData{MeshPrimitive{}, 0})); } indexStride += meshIndexTypeSize(indexType); } @@ -171,10 +170,6 @@ Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data) { - return combineIndexedAttributes(Containers::arrayView(data)); -} - Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade::MeshData& faceAttributes) { CORRADE_ASSERT(mesh.isIndexed(), "MeshTools::combineFaceAttributes(): vertex mesh is not indexed", @@ -236,10 +231,9 @@ Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, const Trade:: #ifndef CORRADE_NO_ASSERT "MeshTools::combineFaceAttributes():", #endif - mesh.primitive(), combinedIndices, meshIndexCount, indexStride, - Containers::arrayView>({ + mesh.primitive(), combinedIndices, meshIndexCount, indexStride, { mesh, faceAttributes - })); + }); } Trade::MeshData combineFaceAttributes(const Trade::MeshData& mesh, Containers::ArrayView faceAttributes) { diff --git a/src/Magnum/MeshTools/Combine.h b/src/Magnum/MeshTools/Combine.h index bbceef152..fb0341802 100644 --- a/src/Magnum/MeshTools/Combine.h +++ b/src/Magnum/MeshTools/Combine.h @@ -35,6 +35,13 @@ #include "Magnum/MeshTools/visibility.h" #include "Magnum/Trade/Trade.h" +#ifdef MAGNUM_BUILD_DEPRECATED +/* combineIndexedAttributes() used to take an ArrayView>, + now it's through the Iterable class. Include it explicitly until people + learn to include it themselves. */ +#include +#endif + namespace Magnum { namespace MeshTools { /** @@ -88,13 +95,7 @@ implementation-specific format. @see @ref isMeshIndexTypeImplementationSpecific(), @ref isVertexFormatImplementationSpecific() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(const Containers::ArrayView> data); - -/** - * @overload - * @m_since{2020,06} - */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(std::initializer_list> data); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData combineIndexedAttributes(const Containers::Iterable data); /** @brief Combine per-face attributes into an existing mesh diff --git a/src/Magnum/MeshTools/Concatenate.cpp b/src/Magnum/MeshTools/Concatenate.cpp index 34a0f4e73..67517cc7f 100644 --- a/src/Magnum/MeshTools/Concatenate.cpp +++ b/src/Magnum/MeshTools/Concatenate.cpp @@ -33,7 +33,7 @@ namespace Magnum { namespace MeshTools { namespace Implementation { -std::pair concatenateIndexVertexCount(Containers::ArrayView> meshes) { +std::pair concatenateIndexVertexCount(Containers::Iterable meshes) { UnsignedInt indexCount = 0; UnsignedInt vertexCount = 0; for(const Trade::MeshData& mesh: meshes) { @@ -62,7 +62,7 @@ struct MeshAttributeHash: std::hash&& indexData, const UnsignedInt vertexCount, Containers::Array&& vertexData, Containers::Array&& attributeData, const Containers::ArrayView> meshes, const char* const assertPrefix) { +Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedInt vertexCount, Containers::Array&& vertexData, Containers::Array&& attributeData, const Containers::Iterable meshes, const char* const assertPrefix) { #ifdef CORRADE_NO_ASSERT static_cast(assertPrefix); #endif @@ -82,17 +82,17 @@ Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedI /** @todo delegate to `indexTriangleStrip()` (`duplicate*()`?) etc when those are done */ CORRADE_ASSERT( - meshes.front()->primitive() != MeshPrimitive::LineStrip && - meshes.front()->primitive() != MeshPrimitive::LineLoop && - meshes.front()->primitive() != MeshPrimitive::TriangleStrip && - meshes.front()->primitive() != MeshPrimitive::TriangleFan, - assertPrefix << meshes.front()->primitive() << "is not supported, turn it into a plain indexed mesh first", + meshes.front().primitive() != MeshPrimitive::LineStrip && + meshes.front().primitive() != MeshPrimitive::LineLoop && + meshes.front().primitive() != MeshPrimitive::TriangleStrip && + meshes.front().primitive() != MeshPrimitive::TriangleFan, + assertPrefix << meshes.front().primitive() << "is not supported, turn it into a plain indexed mesh first", (Trade::MeshData{MeshPrimitive{}, 0})); /* Populate the resulting instance with what we have. It'll be used below for convenient access to vertex / index data */ auto indices = Containers::arrayCast(indexData); - Trade::MeshData out{meshes.front()->primitive(), + Trade::MeshData out{meshes.front().primitive(), /* If the index array is empty, we're creating a non-indexed mesh (not an indexed mesh with zero indices) */ std::move(indexData), indices.isEmpty() ? @@ -189,13 +189,13 @@ Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedI } -Trade::MeshData concatenate(const Containers::ArrayView> meshes, const InterleaveFlags flags) { +Trade::MeshData concatenate(const Containers::Iterable meshes, const InterleaveFlags flags) { CORRADE_ASSERT(!meshes.isEmpty(), "MeshTools::concatenate(): expected at least one mesh", (Trade::MeshData{MeshPrimitive::Points, 0})); #ifndef CORRADE_NO_ASSERT - for(std::size_t i = 0; i != meshes.front()->attributeCount(); ++i) { - const VertexFormat format = meshes.front()->attributeFormat(i); + for(std::size_t i = 0; i != meshes.front().attributeCount(); ++i) { + const VertexFormat format = meshes.front().attributeFormat(i); CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), "MeshTools::concatenate(): attribute" << i << "of the first mesh has an implementation-specific format" << reinterpret_cast(vertexFormatUnwrap(format)), (Trade::MeshData{MeshPrimitive::Points, 0})); @@ -208,13 +208,13 @@ Trade::MeshData concatenate(const Containers::ArrayView attributeData; - if(meshes.front()->attributeCount()) - attributeData = Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(), - {}, meshes.front()->vertexData(), - Trade::meshAttributeDataNonOwningArray(meshes.front()->attributeData())}, {}, flags); + if(meshes.front().attributeCount()) + attributeData = Implementation::interleavedLayout(Trade::MeshData{meshes.front().primitive(), + {}, meshes.front().vertexData(), + Trade::meshAttributeDataNonOwningArray(meshes.front().attributeData())}, {}, flags); else attributeData = - Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(), - meshes.front()->vertexCount()}, {}, flags); + Implementation::interleavedLayout(Trade::MeshData{meshes.front().primitive(), + meshes.front().vertexCount()}, {}, flags); /* Calculate total index/vertex count and allocate the target memory. Index data are allocated with NoInit as the whole array will be written, @@ -227,8 +227,4 @@ Trade::MeshData concatenate(const Containers::ArrayView> meshes, const InterleaveFlags flags) { - return concatenate(Containers::arrayView(meshes), flags); -} - }} diff --git a/src/Magnum/MeshTools/Concatenate.h b/src/Magnum/MeshTools/Concatenate.h index 5ba5c5d43..3f5323baf 100644 --- a/src/Magnum/MeshTools/Concatenate.h +++ b/src/Magnum/MeshTools/Concatenate.h @@ -31,7 +31,7 @@ */ #include -#include +#include #include "Magnum/MeshTools/Interleave.h" #include "Magnum/Trade/MeshData.h" @@ -39,8 +39,8 @@ namespace Magnum { namespace MeshTools { namespace Implementation { - MAGNUM_MESHTOOLS_EXPORT std::pair concatenateIndexVertexCount(Containers::ArrayView> meshes); - MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::Array&& indexData, UnsignedInt vertexCount, Containers::Array&& vertexData, Containers::Array&& attributeData, Containers::ArrayView> meshes, const char* assertPrefix); + MAGNUM_MESHTOOLS_EXPORT std::pair concatenateIndexVertexCount(Containers::Iterable meshes); + MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::Array&& indexData, UnsignedInt vertexCount, Containers::Array&& vertexData, Containers::Array&& attributeData, Containers::Iterable meshes, const char* assertPrefix); } /** @@ -82,13 +82,7 @@ to compress it to a smaller type, if desired. @ref SceneTools::flattenMeshHierarchy2D(), @ref SceneTools::flattenMeshHierarchy3D() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::ArrayView> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); - -/** - * @overload - * @m_since{2020,06} - */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(std::initializer_list> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::Iterable meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** @brief Concatenate a list of meshes into a pre-existing destination, enlarging it if necessary @@ -99,14 +93,14 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(std::initializer_list>, InterleaveFlags) +Compared to @ref concatenate(Containers::Iterable, InterleaveFlags) this function resizes existing index and vertex buffers in @p destination using @ref Containers::arrayResize() and given @p allocator, and reuses its atttribute data array instead of always allocating new ones. Only the attribute layout from @p destination is used, all vertex/index data are taken from @p meshes. Expects that @p meshes contains at least one item. */ -template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, Containers::ArrayView> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes) { +template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, Containers::Iterable meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes) { CORRADE_ASSERT(!meshes.isEmpty(), "MeshTools::concatenateInto(): no meshes passed", ); #ifndef CORRADE_NO_ASSERT @@ -142,14 +136,6 @@ template class Allocator = Containers::ArrayAllocator> void conc destination = Implementation::concatenate(std::move(indexData), indexVertexCount.second, std::move(vertexData), std::move(attributeData), meshes, "MeshTools::concatenateInto():"); } -/** - * @overload - * @m_since{2020,06} - */ -template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, std::initializer_list> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes) { - concatenateInto(destination, Containers::arrayView(meshes), flags); -} - }} #endif diff --git a/src/Magnum/MeshTools/InterleaveFlags.h b/src/Magnum/MeshTools/InterleaveFlags.h index 2bf4387d5..d9437980a 100644 --- a/src/Magnum/MeshTools/InterleaveFlags.h +++ b/src/Magnum/MeshTools/InterleaveFlags.h @@ -43,7 +43,7 @@ namespace Magnum { namespace MeshTools { @see @ref InterleaveFlags, @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags), @ref interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags), - @ref concatenate(Containers::ArrayView>, InterleaveFlags) + @ref concatenate(Containers::Iterable, InterleaveFlags) */ enum class InterleaveFlag: UnsignedInt { /** @@ -72,7 +72,7 @@ enum class InterleaveFlag: UnsignedInt { * * Has no effect when passed to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags) "interleavedLayout()" * as that function doesn't preserve the index buffer. Has no effect when - * passed to @ref concatenate(Containers::ArrayView>, InterleaveFlags) "concatenate()" + * passed to @ref concatenate(Containers::Iterable, InterleaveFlags) "concatenate()" * as that function allocates a new combined index buffer anyway. * @see @ref isMeshIndexTypeImplementationSpecific() */ @@ -85,7 +85,7 @@ enum class InterleaveFlag: UnsignedInt { @see @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags), @ref interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags), - @ref concatenate(Containers::ArrayView>, InterleaveFlags) + @ref concatenate(Containers::Iterable, InterleaveFlags) */ typedef Containers::EnumSet InterleaveFlags; diff --git a/src/Magnum/MeshTools/Test/CombineTest.cpp b/src/Magnum/MeshTools/Test/CombineTest.cpp index 0f40e4639..cda4d9485 100644 --- a/src/Magnum/MeshTools/Test/CombineTest.cpp +++ b/src/Magnum/MeshTools/Test/CombineTest.cpp @@ -24,7 +24,7 @@ */ #include -#include +#include #include #include #include