From b6dd52eb62c2702a8f95f37695086ac3e006334a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 18 Jan 2022 21:04:58 +0100 Subject: [PATCH] MeshTools: add a flag to toggle layout preserving in interleave() etc. This took me quite a while to realize -- not always it's desirable to have the original layout unconditionally preserved, especially if for example filtering a MeshData to just a subset of attributes. --- doc/changelog.dox | 10 +- src/Magnum/MeshTools/Concatenate.cpp | 10 +- src/Magnum/MeshTools/Concatenate.h | 21 ++- src/Magnum/MeshTools/Interleave.cpp | 44 +++--- src/Magnum/MeshTools/Interleave.h | 114 +++++++++----- src/Magnum/MeshTools/Test/ConcatenateTest.cpp | 46 ++++-- src/Magnum/MeshTools/Test/InterleaveTest.cpp | 140 +++++++++++++----- src/Magnum/Trade/MeshData.h | 2 +- 8 files changed, 274 insertions(+), 113 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index a90454223..e1b297ad7 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -345,6 +345,14 @@ See also: zero-fill while @ref Math::Matrix stays with the identity for consistency with other constructors. +@subsubsection changelog-latest-changes-meshtools MeshTools library + +- @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) + optionally take a @ref MeshTools::InterleaveFlags parameter affecting the + output, in particular whether to preserve the original interleaved layout. + @subsubsection changelog-latest-changes-platform Platform libraries - Added a @ref Platform::GlfwApplication::setWindowIcon() overload taking a @@ -1033,7 +1041,7 @@ Released 2020-06-27, tagged as getting an interleaved view - Added @ref MeshTools::interleavedLayout() for convenient creation of an interleaved mesh layout using the new @ref Trade::MeshData API -- Added @ref MeshTools::interleave(const Trade::MeshData&, Containers::ArrayView), +- Added @cpp MeshTools::interleave(const Trade::MeshData&, Containers::ArrayView) @ce, @ref MeshTools::duplicate(const Trade::MeshData&, Containers::ArrayView), @ref MeshTools::compressIndices(const Trade::MeshData&, MeshIndexType) and @ref MeshTools::removeDuplicates(const Trade::MeshData&) that work diff --git a/src/Magnum/MeshTools/Concatenate.cpp b/src/Magnum/MeshTools/Concatenate.cpp index ff56ddb57..6f3914655 100644 --- a/src/Magnum/MeshTools/Concatenate.cpp +++ b/src/Magnum/MeshTools/Concatenate.cpp @@ -185,7 +185,7 @@ Trade::MeshData concatenate(Containers::Array&& indexData, const UnsignedI } -Trade::MeshData concatenate(const Containers::ArrayView> meshes) { +Trade::MeshData concatenate(const Containers::ArrayView> meshes, const InterleaveFlags flags) { CORRADE_ASSERT(!meshes.empty(), "MeshTools::concatenate(): expected at least one mesh", (Trade::MeshData{MeshPrimitive::Points, 0})); @@ -207,10 +207,10 @@ Trade::MeshData concatenate(const Containers::ArrayViewattributeCount()) attributeData = Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(), {}, meshes.front()->vertexData(), - Trade::meshAttributeDataNonOwningArray(meshes.front()->attributeData())}, {}); + Trade::meshAttributeDataNonOwningArray(meshes.front()->attributeData())}, {}, flags); else attributeData = Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(), - meshes.front()->vertexCount()}, {}); + 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, @@ -223,8 +223,8 @@ Trade::MeshData concatenate(const Containers::ArrayView> meshes) { - return concatenate(Containers::arrayView(meshes)); +Trade::MeshData concatenate(const std::initializer_list> 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 d0807adeb..c1d40d6d3 100644 --- a/src/Magnum/MeshTools/Concatenate.h +++ b/src/Magnum/MeshTools/Concatenate.h @@ -45,6 +45,8 @@ namespace Implementation { /** @brief Concatenate meshes together +@param meshes Meshes to concatenate +@param flags Flags to pass to @ref interleavedLayout() @m_since{2020,06} The returned mesh contains vertices from all meshes concatenated together. If @@ -67,6 +69,10 @@ and index data flags always have both @ref Trade::DataFlag::Owned and @ref Trade::DataFlag::Mutable to guarante mutable access to particular parts of the concatenated mesh --- for example for applying transformations. +The data layouting is done by @ref interleavedLayout() with the @p flags +parameter propagated to it, see its documentation for detailed behavior +description. + If an index buffer is needed, @ref MeshIndexType::UnsignedInt is always used. Call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result to compress it to a smaller type, if desired. @@ -74,13 +80,13 @@ to compress it to a smaller type, if desired. @ref SceneTools::flattenMeshHierarchy2D(), @ref SceneTools::flattenMeshHierarchy3D() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::ArrayView> meshes); +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); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(std::initializer_list> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** @brief Concatenate a list of meshes into a pre-existing destination, enlarging it if necessary @@ -88,16 +94,17 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(std::initializer_list>) +Compared to @ref concatenate(Containers::ArrayView>, 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) { +template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, Containers::ArrayView> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes) { CORRADE_ASSERT(!meshes.empty(), "MeshTools::concatenateInto(): no meshes passed", ); #ifndef CORRADE_NO_ASSERT @@ -118,7 +125,7 @@ template class Allocator = Containers::ArrayAllocator> void conc Containers::arrayResize(indexData, NoInit, indexVertexCount.first*sizeof(UnsignedInt)); } - Containers::Array attributeData = Implementation::interleavedLayout(std::move(destination), {}); + Containers::Array attributeData = Implementation::interleavedLayout(std::move(destination), {}, flags); Containers::Array vertexData; if(!attributeData.empty() && indexVertexCount.second) { const UnsignedInt attributeStride = attributeData[0].stride(); @@ -137,8 +144,8 @@ template class Allocator = Containers::ArrayAllocator> void conc * @overload * @m_since{2020,06} */ -template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, std::initializer_list> meshes) { - concatenateInto(destination, Containers::arrayView(meshes)); +template class Allocator = Containers::ArrayAllocator> void concatenateInto(Trade::MeshData& destination, std::initializer_list> meshes, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes) { + concatenateInto(destination, Containers::arrayView(meshes), flags); } }} diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index 843b79d7c..eabe44119 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -109,11 +109,14 @@ Containers::StridedArrayView2D interleavedMutableData(Trade::MeshData& dat namespace Implementation { -Containers::Array interleavedLayout(Trade::MeshData&& data, const Containers::ArrayView extra) { +Containers::Array interleavedLayout(Trade::MeshData&& data, const Containers::ArrayView extra, const InterleaveFlags flags) { /* Nothing to do here, bye! */ if(!data.attributeCount() && extra.empty()) return {}; - const bool interleaved = isInterleaved(data); + /* If we're not told to preserve the layout, treat the mesh as + noninterleaved always, forcing a repack. Otherwise check if it's already + interleaved. */ + const bool interleaved = flags >= InterleaveFlag::PreserveInterleavedAttributes && isInterleaved(data); /* If the mesh is already interleaved, use the original stride to preserve all padding, but remove the initial offset. Otherwise calculate @@ -210,8 +213,8 @@ Containers::Array interleavedLayout(Trade::MeshData&& } -Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const Containers::ArrayView extra) { - Containers::Array attributeData = Implementation::interleavedLayout(std::move(data), extra); +Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const Containers::ArrayView extra, const InterleaveFlags flags) { + Containers::Array attributeData = Implementation::interleavedLayout(std::move(data), extra, flags); /* If there are no attributes, bail -- return an empty mesh with desired vertex count but nothing else */ @@ -235,23 +238,23 @@ Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vert return Trade::MeshData{data.primitive(), std::move(vertexData), std::move(attributeData)}; } -Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const std::initializer_list extra) { - return interleavedLayout(std::move(data), vertexCount, Containers::arrayView(extra)); +Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const std::initializer_list extra, const InterleaveFlags flags) { + return interleavedLayout(std::move(data), vertexCount, Containers::arrayView(extra), flags); } -Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const Containers::ArrayView extra) { +Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const Containers::ArrayView extra, const InterleaveFlags flags) { return interleavedLayout( Trade::MeshData{data.primitive(), {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()), data.vertexCount()}, - vertexCount, extra); + vertexCount, extra, flags); } -Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const std::initializer_list extra) { - return interleavedLayout(data, vertexCount, Containers::arrayView(extra)); +Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const std::initializer_list extra, const InterleaveFlags flags) { + return interleavedLayout(data, vertexCount, Containers::arrayView(extra), flags); } -Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView extra) { +Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView extra, const InterleaveFlags flags) { /* Transfer the indices unchanged, in case the mesh is indexed */ Containers::Array indexData; Trade::MeshIndexData indices; @@ -268,7 +271,10 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView= InterleaveFlag::PreserveInterleavedAttributes && isInterleaved(data); const UnsignedInt vertexCount = data.vertexCount(); /* If the mesh is already interleaved and we don't have anything extra, @@ -283,7 +289,7 @@ Trade::MeshData interleave(Trade::MeshData&& data, const Containers::ArrayView extra) { - return interleave(std::move(data), Containers::arrayView(extra)); +Trade::MeshData interleave(Trade::MeshData&& data, const std::initializer_list extra, const InterleaveFlags flags) { + return interleave(std::move(data), Containers::arrayView(extra), flags); } -Trade::MeshData interleave(const Trade::MeshData& data, const Containers::ArrayView extra) { +Trade::MeshData interleave(const Trade::MeshData& data, const Containers::ArrayView extra, const InterleaveFlags flags) { return interleave(Trade::MeshData{data.primitive(), /* If data is not indexed, the reference will be also non-indexed */ {}, data.indexData(), Trade::MeshIndexData{data.indices()}, {}, data.vertexData(), Trade::meshAttributeDataNonOwningArray(data.attributeData()), data.vertexCount() - }, extra); + }, extra, flags); } -Trade::MeshData interleave(const Trade::MeshData& data, const std::initializer_list extra) { - return interleave(std::move(data), Containers::arrayView(extra)); +Trade::MeshData interleave(const Trade::MeshData& data, const std::initializer_list extra, const InterleaveFlags flags) { + return interleave(std::move(data), Containers::arrayView(extra), flags); } }} diff --git a/src/Magnum/MeshTools/Interleave.h b/src/Magnum/MeshTools/Interleave.h index 5cadd1ef4..86a3ea09f 100644 --- a/src/Magnum/MeshTools/Interleave.h +++ b/src/Magnum/MeshTools/Interleave.h @@ -30,6 +30,7 @@ */ #include +#include #include #include #include @@ -114,9 +115,6 @@ template void writeInterleaved(std::size_t stride, char* st writeInterleaved(stride, startingOffset + writeOneInterleaved(stride, startingOffset, first), next...); } -/* Used internally by interleavedLayout() and concatenate() */ -MAGNUM_MESHTOOLS_EXPORT Containers::Array interleavedLayout(Trade::MeshData&& data, Containers::ArrayView extra); - } /** @@ -190,6 +188,42 @@ template void interleaveInto(Containers::ArrayView bu Implementation::writeInterleaved(stride, buffer.begin(), first, next...); } +/** +@brief Interleaving behavior flag +@m_since_latest + +@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) +*/ +enum class InterleaveFlag: UnsignedInt { + /** + * If the mesh is already interleaved, preserves existing layout of the + * attributes as well as any padding or aliasing among them, keeping the + * original stride and only removing the initial offset. This can also + * preserve attributes with an implementation-specific @ref VertexFormat. + * + * If not set or if the mesh is not interleaved to begin with, a tightly + * packed stride is calculated from vertex format sizes of all attributes, + * removing all padding. In that case an implementation-specific + * @ref VertexFormat can't be used for any attribute. + */ + PreserveInterleavedAttributes = 1 << 0 +}; + +/** +@brief Interleaving behavior flags +@m_since_latest + +@see @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags), + @ref interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags), + @ref concatenate(Containers::ArrayView>, InterleaveFlags) +*/ +typedef Containers::EnumSet InterleaveFlags; + +CORRADE_ENUMSET_OPERATORS(InterleaveFlags) + /** @brief If the mesh data is interleaved @m_since{2020,06} @@ -235,17 +269,19 @@ MAGNUM_MESHTOOLS_EXPORT Containers::StridedArrayView2D interleavedMutableD Returns a @ref Trade::MeshData instance with its vertex data allocated for @p vertexCount vertices containing attributes from both @p data and @p extra interleaved together. No data is actually copied, only an interleaved layout is -created. If @p data is already interleaved, keeps the attributes in the same -layout, potentially extending them with @p extra. The @p extra attributes, if -any, are interleaved together with existing attributes. Returned instance -vertex data flags have both @ref Trade::DataFlag::Mutable and -@ref Trade::DataFlag::Owned, so mutable attribute access is guaranteed. - -For greater control you can also pass an empty @ref Trade::MeshData instance -and fill @p extra with attributes cherry-picked using -@ref Trade::MeshData::attributeData(UnsignedInt) const on an existing instance. -By default the attributes are tightly packed, you can add arbitrary padding -using instances constructed via +created. If @p data is already interleaved and +@ref InterleaveFlag::PreserveInterleavedAttributes is set in @p flags, keeps +the attributes in the same layout, potentially extending them with @p extra. +The @p extra attributes, if any, are interleaved together with existing +attributes. Returned instance vertex data flags have both +@ref Trade::DataFlag::Mutable and @ref Trade::DataFlag::Owned, so mutable +attribute access is guaranteed. + +For greater control over the layout you can also pass an empty +@ref Trade::MeshData instance and fill @p extra with attributes cherry-picked +using @ref Trade::MeshData::attributeData(UnsignedInt) const on an existing +instance. By default the attributes are tightly packed, you can add arbitrary +padding using instances constructed via @ref Trade::MeshAttributeData::MeshAttributeData(Int). Example: @snippet MagnumMeshTools.cpp interleavedLayout-extra @@ -257,7 +293,7 @@ instance with attribute and vertex data transferred from the returned instance: @snippet MagnumMeshTools.cpp interleavedLayout-indices This function will unconditionally allocate a new array to store all -@ref Trade::MeshAttributeData, use @ref interleavedLayout(Trade::MeshData&&, UnsignedInt, Containers::ArrayView) +@ref Trade::MeshAttributeData, use @ref interleavedLayout(Trade::MeshData&&, UnsignedInt, Containers::ArrayView, InterleaveFlags) to avoid that allocation. All attributes in both @p data and @p extra are expected to not have an @@ -265,30 +301,31 @@ implementation-specific format, except for @p data attributes in case @p data is already interleaved, then the layout is untouched. @see @ref isVertexFormatImplementationSpecific() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** * @overload * @m_since{2020,06} */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, std::initializer_list extra); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, std::initializer_list extra, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** @brief Create an interleaved mesh layout @m_since{2020,06} -Compared to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView) +Compared to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView, InterleaveFlags) this function can reuse the @ref Trade::MeshAttributeData array from @p data -instead of allocating a new one if there are no attributes passed in @p extra -and the attribute array is owned by the mesh. +instead of allocating a new one if there are no attributes passed in @p extra, +the attribute array is owned by the mesh and +@ref InterleaveFlag::PreserveInterleavedAttributes is set in @p flags. */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** * @overload * @m_since{2020,06} */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, std::initializer_list extra); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, std::initializer_list extra, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** @brief Interleave mesh data @@ -298,15 +335,15 @@ Returns a copy of @p data with all attributes interleaved. Indices (if any) are kept as-is. The @p extra attributes, if any, are interleaved together with existing attributes (or, in case the attribute view is empty, only the corresponding space for given attribute type is reserved, with memory left -uninitialized). The data layouting is done by @ref interleavedLayout(), see its -documentation for detailed behavior description. Note that offset-only -@ref Trade::MeshAttributeData instances are not supported in the @p extra -array. +uninitialized). The data layouting is done by @ref interleavedLayout() with the +@p flags parameter propagated to it, see its documentation for detailed +behavior description. Note that offset-only @ref Trade::MeshAttributeData +instances are not supported in the @p extra array. Expects that each attribute in @p extra has either the same amount of elements as @p data vertex count or has none. This function will unconditionally make a copy of all data even if @p data is already interleaved and needs no change, -use @ref interleave(Trade::MeshData&&, Containers::ArrayView) +use @ref interleave(Trade::MeshData&&, Containers::ArrayView, InterleaveFlags) to avoid that copy. All attributes in both @p data and @p extra are expected to not have an @@ -315,34 +352,41 @@ is already interleaved, then the layout is untouched. @see @ref isInterleaved(), @ref isVertexFormatImplementationSpecific(), @ref Trade::MeshData::attributeData() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, Containers::ArrayView extra = {}); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, Containers::ArrayView extra = {}, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** * @overload * @m_since{2020,06} */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, std::initializer_list extra); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(const Trade::MeshData& data, std::initializer_list extra, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** @brief Interleave mesh data @m_since{2020,06} -Compared to @ref interleave(const Trade::MeshData&, Containers::ArrayView) +Compared to @ref interleave(const Trade::MeshData&, Containers::ArrayView, InterleaveFlags) this function can transfer ownership of @p data index buffer (in case it is -owned) and vertex buffer (in case it is owned, already interleaved and there's -no @p extra attributes) to the returned instance instead of making copies of -them. +owned) and vertex buffer (in case it is owned, already interleaved, there's no +@p extra attributes and @ref InterleaveFlag::PreserveInterleavedAttributes is +set in @p flags) to the returned instance instead of making copies of them. @see @ref isInterleaved(), @ref Trade::MeshData::indexDataFlags(), @ref Trade::MeshData::vertexDataFlags(), @ref Trade::MeshData::attributeData() */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& data, Containers::ArrayView extra = {}); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& data, Containers::ArrayView extra = {}, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); /** * @overload * @m_since{2020,06} */ -MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& data, std::initializer_list extra); +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& data, std::initializer_list extra, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes); + +namespace Implementation { + +/* Used internally by interleavedLayout() and concatenate() */ +MAGNUM_MESHTOOLS_EXPORT Containers::Array interleavedLayout(Trade::MeshData&& data, Containers::ArrayView extra, InterleaveFlags flags); + +} }} diff --git a/src/Magnum/MeshTools/Test/ConcatenateTest.cpp b/src/Magnum/MeshTools/Test/ConcatenateTest.cpp index bc99187d2..70f875723 100644 --- a/src/Magnum/MeshTools/Test/ConcatenateTest.cpp +++ b/src/Magnum/MeshTools/Test/ConcatenateTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include #include @@ -54,9 +55,20 @@ struct ConcatenateTest: TestSuite::Tester { void concatenateIntoNoMeshes(); }; +const struct { + const char* name; + Containers::Optional flags; + bool shouldPreserveLayout; +} ConcatenateData[]{ + {"", {}, true}, + {"don't preserve layout", InterleaveFlags{}, false}, +}; + ConcatenateTest::ConcatenateTest() { - addTests({&ConcatenateTest::concatenate, - &ConcatenateTest::concatenateNotIndexed, + addInstancedTests({&ConcatenateTest::concatenate}, + Containers::arraySize(ConcatenateData)); + + addTests({&ConcatenateTest::concatenateNotIndexed, &ConcatenateTest::concatenateNoAttributes, &ConcatenateTest::concatenateNoAttributesNotIndexed, &ConcatenateTest::concatenateOne, @@ -84,6 +96,9 @@ struct VertexDataA { }; void ConcatenateTest::concatenate() { + auto&& data = ConcatenateData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + using namespace Math::Literals; /* First is non-indexed, this layout (including the gap) will be @@ -167,7 +182,11 @@ void ConcatenateTest::concatenate() { &vertexDataC[0].texcoords3, 3, sizeof(VertexDataC))}, }}; - Trade::MeshData dst = MeshTools::concatenate({a, b, c}); + /* To catch when the default argument becomes different */ + Trade::MeshData dst = data.flags ? + MeshTools::concatenate({a, b, c}, *data.flags) : + MeshTools::concatenate({a, b, c}); + CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.attributeCount(), 4); CORRADE_COMPARE_AS(dst.attribute(Trade::MeshAttribute::Position), @@ -218,13 +237,22 @@ void ConcatenateTest::concatenate() { 6, 7, 8 /* implicit + offset for the third mesh */ }), TestSuite::Compare::Container); - /* The original interleaved layout should be preserved */ CORRADE_VERIFY(isInterleaved(dst)); - CORRADE_COMPARE(dst.attributeStride(0), sizeof(VertexDataA)); - CORRADE_COMPARE(dst.attributeOffset(0), 0); - CORRADE_COMPARE(dst.attributeOffset(1), sizeof(Vector2)); - CORRADE_COMPARE(dst.attributeOffset(2), 2*sizeof(Vector2) + 4); - CORRADE_COMPARE(dst.attributeOffset(3), 2*sizeof(Vector2) + 4 + sizeof(Vector3)); + if(data.shouldPreserveLayout) { + /* The original interleaved layout should be preserved */ + CORRADE_COMPARE(dst.attributeStride(0), sizeof(VertexDataA)); + CORRADE_COMPARE(dst.attributeOffset(0), 0); + CORRADE_COMPARE(dst.attributeOffset(1), sizeof(Vector2)); + CORRADE_COMPARE(dst.attributeOffset(2), 2*sizeof(Vector2) + 4); + CORRADE_COMPARE(dst.attributeOffset(3), 2*sizeof(Vector2) + 4 + sizeof(Vector3)); + } else { + /* Everything gets tightly packed */ + CORRADE_COMPARE(dst.attributeStride(0), 2*sizeof(Vector2) + sizeof(Vector3) + 2*sizeof(Short)); + CORRADE_COMPARE(dst.attributeOffset(0), 0); + CORRADE_COMPARE(dst.attributeOffset(1), sizeof(Vector2)); + CORRADE_COMPARE(dst.attributeOffset(2), 2*sizeof(Vector2)); + CORRADE_COMPARE(dst.attributeOffset(3), 2*sizeof(Vector2) + sizeof(Vector3)); + } } void ConcatenateTest::concatenateNotIndexed() { diff --git a/src/Magnum/MeshTools/Test/InterleaveTest.cpp b/src/Magnum/MeshTools/Test/InterleaveTest.cpp index da3fb851b..9fb31cc94 100644 --- a/src/Magnum/MeshTools/Test/InterleaveTest.cpp +++ b/src/Magnum/MeshTools/Test/InterleaveTest.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -101,9 +102,12 @@ struct InterleaveTest: Corrade::TestSuite::Tester { const struct { const char* name; VertexFormat vertexFormat; + Containers::Optional flags; + bool shouldPreserveLayout; } AlreadyInterleavedData[]{ - {"", VertexFormat::Vector3}, - {"implementation-specific vertex format", vertexFormatWrap(0xcaca)} + {"", VertexFormat::Vector3, {}, true}, + {"implementation-specific vertex format", vertexFormatWrap(0xcaca), {}, true}, + {"don't preserve layout", VertexFormat::Vector3, InterleaveFlags{}, false} }; InterleaveTest::InterleaveTest() { @@ -881,7 +885,11 @@ void InterleaveTest::interleavedLayoutAlreadyInterleaved() { std::move(vertexData), {positions, normals}}; CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10); + /* To catch when the default argument becomes different */ + Trade::MeshData layout = data.flags ? + MeshTools::interleavedLayout(mesh, 10, {}, *data.flags) : + MeshTools::interleavedLayout(mesh, 10); + CORRADE_VERIFY(MeshTools::isInterleaved(layout)); CORRADE_VERIFY(!layout.isIndexed()); /* Indices are not preserved */ CORRADE_COMPARE(layout.attributeCount(), 2); @@ -889,14 +897,24 @@ void InterleaveTest::interleavedLayoutAlreadyInterleaved() { CORRADE_COMPARE(layout.attributeName(1), Trade::MeshAttribute::Normal); CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); - /* Original stride should be preserved no matter what the formats are */ - CORRADE_COMPARE(layout.attributeStride(0), 24); - CORRADE_COMPARE(layout.attributeStride(1), 24); - /* Relative offsets should be preserved, but the initial one removed */ - CORRADE_COMPARE(layout.attributeOffset(0), 0); - CORRADE_COMPARE(layout.attributeOffset(1), 10); + CORRADE_COMPARE(layout.vertexCount(), 10); - CORRADE_COMPARE(layout.vertexData().size(), 10*24); + if(data.shouldPreserveLayout) { + /* Original stride should be preserved no matter what the formats are */ + CORRADE_COMPARE(layout.attributeStride(0), 24); + CORRADE_COMPARE(layout.attributeStride(1), 24); + /* Relative offsets should be preserved, but the initial one removed */ + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 10); + CORRADE_COMPARE(layout.vertexData().size(), 10*24); + } else { + /* Everything gets tightly packed */ + CORRADE_COMPARE(layout.attributeStride(0), 8 + 12); + CORRADE_COMPARE(layout.attributeStride(1), 8 + 12); + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 8); + CORRADE_COMPARE(layout.vertexData().size(), 10*20); + } } void InterleaveTest::interleavedLayoutAlreadyInterleavedAliased() { @@ -917,7 +935,11 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedAliased() { std::move(vertexData), {positions, normals}}; CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10); + /* To catch when the default argument becomes different */ + Trade::MeshData layout = data.flags ? + MeshTools::interleavedLayout(mesh, 10, {}, *data.flags) : + MeshTools::interleavedLayout(mesh, 10); + CORRADE_VERIFY(MeshTools::isInterleaved(layout)); CORRADE_VERIFY(!layout.isIndexed()); /* Indices are not preserved */ CORRADE_COMPARE(layout.attributeCount(), 2); @@ -925,12 +947,22 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedAliased() { CORRADE_COMPARE(layout.attributeName(1), Trade::MeshAttribute::Normal); CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); - CORRADE_COMPARE(layout.attributeStride(0), 12); - CORRADE_COMPARE(layout.attributeStride(1), 12); - CORRADE_COMPARE(layout.attributeOffset(0), 0); - CORRADE_COMPARE(layout.attributeOffset(1), 0); /* aliases */ + CORRADE_COMPARE(layout.vertexCount(), 10); - CORRADE_COMPARE(layout.vertexData().size(), 10*12); + if(data.shouldPreserveLayout) { + CORRADE_COMPARE(layout.attributeStride(0), 12); + CORRADE_COMPARE(layout.attributeStride(1), 12); + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 0); /* aliases */ + CORRADE_COMPARE(layout.vertexData().size(), 10*12); + } else { + /* The attribute gets duplicated */ + CORRADE_COMPARE(layout.attributeStride(0), 8 + 12); + CORRADE_COMPARE(layout.attributeStride(1), 8 + 12); + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 8); + CORRADE_COMPARE(layout.vertexData().size(), 10*20); + } } void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { @@ -948,7 +980,7 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { std::move(vertexData), {positions, normals}}; CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - Trade::MeshData layout = MeshTools::interleavedLayout(mesh, 10, { + std::initializer_list extra{ Trade::MeshAttributeData{1}, Trade::MeshAttributeData{Trade::meshAttributeCustom(15), VertexFormat::UnsignedShort, nullptr}, @@ -956,7 +988,13 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { Trade::MeshAttributeData{Trade::MeshAttribute::Color, VertexFormat::Vector3, nullptr}, Trade::MeshAttributeData{4} - }); + }; + + /* To catch when the default argument becomes different */ + Trade::MeshData layout = data.flags ? + MeshTools::interleavedLayout(mesh, 10, extra, *data.flags) : + MeshTools::interleavedLayout(mesh, 10, extra); + CORRADE_VERIFY(MeshTools::isInterleaved(layout)); CORRADE_COMPARE(layout.attributeCount(), 4); CORRADE_COMPARE(layout.attributeName(0), Trade::MeshAttribute::Position); @@ -967,19 +1005,35 @@ void InterleaveTest::interleavedLayoutAlreadyInterleavedExtra() { CORRADE_COMPARE(layout.attributeFormat(1), data.vertexFormat); CORRADE_COMPARE(layout.attributeFormat(2), VertexFormat::UnsignedShort); CORRADE_COMPARE(layout.attributeFormat(3), VertexFormat::Vector3); - /* Original stride should be preserved no matter what the formats, with - stride from extra attribs added */ - CORRADE_COMPARE(layout.attributeStride(0), 24 + 20); - CORRADE_COMPARE(layout.attributeStride(1), 24 + 20); - CORRADE_COMPARE(layout.attributeStride(2), 24 + 20); - CORRADE_COMPARE(layout.attributeStride(3), 24 + 20); - /* Relative offsets should be preserved, but the initial one removed */ - CORRADE_COMPARE(layout.attributeOffset(0), 0); - CORRADE_COMPARE(layout.attributeOffset(1), 10); - CORRADE_COMPARE(layout.attributeOffset(2), 25); - CORRADE_COMPARE(layout.attributeOffset(3), 28); + CORRADE_COMPARE(layout.vertexCount(), 10); - CORRADE_COMPARE(layout.vertexData().size(), 10*44); + if(data.shouldPreserveLayout) { + /* Original stride should be preserved no matter what the formats, with + stride from extra attribs added */ + CORRADE_COMPARE(layout.attributeStride(0), 24 + 20); + CORRADE_COMPARE(layout.attributeStride(1), 24 + 20); + CORRADE_COMPARE(layout.attributeStride(2), 24 + 20); + CORRADE_COMPARE(layout.attributeStride(3), 24 + 20); + /* Relative offsets should be preserved, but the initial one removed */ + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 10); + CORRADE_COMPARE(layout.attributeOffset(2), 25); + CORRADE_COMPARE(layout.attributeOffset(3), 28); + CORRADE_COMPARE(layout.vertexData().size(), 10*44); + } else { + /* Original data get tightly packed, but any explicit padding in extra + attributes gets preserved */ + CORRADE_COMPARE(layout.attributeStride(0), 8 + 12 + 20); + CORRADE_COMPARE(layout.attributeStride(1), 8 + 12 + 20); + CORRADE_COMPARE(layout.attributeStride(2), 8 + 12 + 20); + CORRADE_COMPARE(layout.attributeStride(3), 8 + 12 + 20); + /* Any explicit padding in extra attributes gets preserved */ + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 8); + CORRADE_COMPARE(layout.attributeOffset(2), 20 + 1); + CORRADE_COMPARE(layout.attributeOffset(3), 22 + 1 + 1); + CORRADE_COMPARE(layout.vertexData().size(), 10*40); + } } void InterleaveTest::interleavedLayoutNothing() { @@ -1257,16 +1311,30 @@ void InterleaveTest::interleaveMeshDataAlreadyInterleavedMove() { std::move(vertexData), std::move(attributeData)}; CORRADE_VERIFY(MeshTools::isInterleaved(mesh)); - /* {} just to cover the initializer_list overload :P */ - Trade::MeshData interleaved = MeshTools::interleave(std::move(mesh), {}); + /* To catch when the default argument becomes different */ + Trade::MeshData interleaved = data.flags ? + MeshTools::interleave(std::move(mesh), {}, *data.flags) : + /* {} just to cover the initializer_list overload :P */ + MeshTools::interleave(std::move(mesh), {}); + CORRADE_VERIFY(MeshTools::isInterleaved(interleaved)); CORRADE_COMPARE(interleaved.indexCount(), 2); CORRADE_COMPARE(interleaved.attributeCount(), 2); CORRADE_COMPARE(interleaved.vertexCount(), 3); - /* Things got just moved without copying */ - CORRADE_VERIFY(interleaved.indexData().data() == static_cast(indexView.data())); - CORRADE_VERIFY(interleaved.attributeData().data() == attributePointer); - CORRADE_VERIFY(interleaved.vertexData().data() == positionView.data()); + + if(data.shouldPreserveLayout) { + /* Things got just moved without copying */ + CORRADE_COMPARE(interleaved.attributeStride(0), 24); + CORRADE_VERIFY(interleaved.indexData().data() == static_cast(indexView.data())); + CORRADE_VERIFY(interleaved.attributeData().data() == attributePointer); + CORRADE_VERIFY(interleaved.vertexData().data() == positionView.data()); + } else { + /* Things got repacked, only the index array stayed the same */ + CORRADE_COMPARE(interleaved.attributeStride(0), 20); + CORRADE_VERIFY(interleaved.indexData().data() == static_cast(indexView.data())); + CORRADE_VERIFY(interleaved.attributeData().data() != attributePointer); + CORRADE_VERIFY(interleaved.vertexData().data() != positionView.data()); + } } void InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveNonOwned() { diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h index 3237471f2..1f411b578 100644 --- a/src/Magnum/Trade/MeshData.h +++ b/src/Magnum/Trade/MeshData.h @@ -290,7 +290,7 @@ class MAGNUM_TRADE_EXPORT MeshIndexData { Convenience type for populating @ref MeshData, see its documentation for an introduction. Additionally usable in various @ref MeshTools algorithms such as @ref MeshTools::duplicate(const Trade::MeshData& data, Containers::ArrayView) -or @ref MeshTools::interleave(const Trade::MeshData& data, Containers::ArrayView). +or @ref MeshTools::interleave(const Trade::MeshData& data, Containers::ArrayView, InterleaveFlags). @section Trade-MeshAttributeData-usage Usage