From 7a9c630599484fd61ea18e60cbebbb71723cda4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 4 Mar 2020 17:04:30 +0100 Subject: [PATCH] MeshTools: add interleavedLayout() that can reuse the attribute array. --- src/Magnum/MeshTools/Interleave.cpp | 53 +++++++++++++---- src/Magnum/MeshTools/Interleave.h | 21 +++++++ src/Magnum/MeshTools/Test/InterleaveTest.cpp | 62 ++++++++++++++++++-- 3 files changed, 120 insertions(+), 16 deletions(-) diff --git a/src/Magnum/MeshTools/Interleave.cpp b/src/Magnum/MeshTools/Interleave.cpp index 0518008d4..d73e827e8 100644 --- a/src/Magnum/MeshTools/Interleave.cpp +++ b/src/Magnum/MeshTools/Interleave.cpp @@ -51,7 +51,7 @@ bool isInterleaved(const Trade::MeshData& data) { return maxOffset - minOffset <= stride; } -Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const Containers::ArrayView extra) { +Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const Containers::ArrayView extra) { /* If there are no attributes, bail -- return an empty mesh with desired vertex count but nothing else */ if(!data.attributeCount() && extra.empty()) @@ -89,32 +89,49 @@ Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt } } - /* Allocate new data and attribute array */ + /* Transfer the attribute data array. If there are no extra attributes and + the attribute data array is owned (the array has a default deleter), we + can take over the ownership and avoid an allocation. Otherwise we + allocate a new array and copy the prefix over so we can just patch the + data array later. */ + const UnsignedInt originalAttributeCount = data.attributeCount(); + const UnsignedInt originalAttributeStride = originalAttributeCount ? + data.attributeStride(0) : 0; + Containers::Array originalAttributeData = + data.releaseAttributeData(); + Containers::Array attributeData; + if(!extraAttributeCount && !originalAttributeData.deleter()) + attributeData = std::move(originalAttributeData); + else { + attributeData = Containers::Array{originalAttributeCount + extraAttributeCount}; + Utility::copy(originalAttributeData, attributeData.prefix(originalAttributeCount)); + } + + /* Allocate new data array */ Containers::Array vertexData{Containers::NoInit, stride*vertexCount}; - Containers::Array attributeData{data.attributeCount() + extraAttributeCount}; /* Copy existing attribute layout. If the original is already interleaved, preserve relative attribute offsets, otherwise pack tightly. */ std::size_t offset = 0; - for(UnsignedInt i = 0; i != data.attributeCount(); ++i) { - if(interleaved) offset = data.attributeOffset(i) - minOffset; + for(UnsignedInt i = 0; i != originalAttributeCount; ++i) { + if(interleaved) offset = attributeData[i].offset(data.vertexData()) - minOffset; attributeData[i] = Trade::MeshAttributeData{ - data.attributeName(i), data.attributeFormat(i), + attributeData[i].name(), attributeData[i].format(), Containers::StridedArrayView1D{vertexData, vertexData + offset, vertexCount, std::ptrdiff_t(stride)}}; - if(!interleaved) offset += vertexFormatSize(data.attributeFormat(i)); + if(!interleaved) offset += vertexFormatSize(attributeData[i].format()); } /* In case the original is already interleaved, set the offset for extra attribs to the original stride to preserve also potential padding at the end. */ - if(interleaved && data.attributeCount()) - offset = data.attributeStride(0); + if(interleaved && originalAttributeCount) + offset = originalAttributeStride; /* Mix in the extra attributes */ - UnsignedInt attributeIndex = data.attributeCount(); + UnsignedInt attributeIndex = originalAttributeCount; for(UnsignedInt i = 0; i != extra.size(); ++i) { /* Padding, only adjust the offset for next attribute */ if(extra[i].format() == VertexFormat{}) { @@ -132,6 +149,22 @@ Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt 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(const Trade::MeshData& data, const UnsignedInt vertexCount, const Containers::ArrayView extra) { + /* If there's no attributes in the original mesh, we need to pass vertex + count explicitly (MeshData asserts on that to avoid it getting lost.) */ + if(!data.attributeCount()) + return interleavedLayout(Trade::MeshData{data.primitive(), data.vertexCount()}, vertexCount, extra); + + return interleavedLayout( + Trade::MeshData{data.primitive(), {}, data.vertexData(), + Trade::meshAttributeDataNonOwningArray(data.attributeData())}, + vertexCount, extra); +} + Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const std::initializer_list extra) { return interleavedLayout(data, vertexCount, Containers::arrayView(extra)); } diff --git a/src/Magnum/MeshTools/Interleave.h b/src/Magnum/MeshTools/Interleave.h index d1851de47..3d5f3cfb5 100644 --- a/src/Magnum/MeshTools/Interleave.h +++ b/src/Magnum/MeshTools/Interleave.h @@ -226,6 +226,10 @@ output non-indexed. If you want to preserve index data, create a new indexed 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) +to avoid that allocation. */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}); @@ -235,6 +239,23 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& */ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, std::initializer_list extra); +/** +@brief Create an interleaved mesh layout +@m_since_latest + +Compared to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView) +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. +*/ +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, Containers::ArrayView extra = {}); + +/** + * @overload + * @m_since_latest + */ +MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, std::initializer_list extra); + /** @brief Interleave mesh data @m_since_latest diff --git a/src/Magnum/MeshTools/Test/InterleaveTest.cpp b/src/Magnum/MeshTools/Test/InterleaveTest.cpp index adebe44b2..d3542b14b 100644 --- a/src/Magnum/MeshTools/Test/InterleaveTest.cpp +++ b/src/Magnum/MeshTools/Test/InterleaveTest.cpp @@ -67,6 +67,7 @@ struct InterleaveTest: Corrade::TestSuite::Tester { void interleavedLayoutAlreadyInterleavedAliased(); void interleavedLayoutAlreadyInterleavedExtra(); void interleavedLayoutNothing(); + void interleavedLayoutRvalue(); void interleaveMeshData(); void interleaveMeshDataIndexed(); @@ -108,6 +109,7 @@ InterleaveTest::InterleaveTest() { &InterleaveTest::interleavedLayoutAlreadyInterleavedAliased, &InterleaveTest::interleavedLayoutAlreadyInterleavedExtra, &InterleaveTest::interleavedLayoutNothing, + &InterleaveTest::interleavedLayoutRvalue, &InterleaveTest::interleaveMeshData, &InterleaveTest::interleaveMeshDataIndexed, @@ -338,15 +340,20 @@ void InterleaveTest::isInterleavedAttributeAcrossStride() { void InterleaveTest::interleavedLayout() { Containers::Array indexData{6}; Containers::Array vertexData{3*20}; - Trade::MeshAttributeData positions{Trade::MeshAttribute::Position, - Containers::arrayCast(vertexData.prefix(3*8))}; - Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal, - Containers::arrayCast(vertexData.suffix(3*8))}; + + const Trade::MeshAttributeData attributeData[]{ + Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayCast(vertexData.prefix(3*8))}, + Trade::MeshAttributeData{Trade::MeshAttribute::Normal, + Containers::arrayCast(vertexData.suffix(3*8))} + }; Trade::MeshIndexData indices{Containers::arrayCast(indexData)}; Trade::MeshData data{MeshPrimitive::TriangleFan, - std::move(indexData), indices, - std::move(vertexData), {positions, normals}}; + std::move(indexData), indices, std::move(vertexData), + /* Verify that interleavedLayout() won't attempt to modify the const + array (see interleavedLayoutRvalue()) */ + Trade::meshAttributeDataNonOwningArray(attributeData)}; CORRADE_VERIFY(!MeshTools::isInterleaved(data)); Trade::MeshData layout = MeshTools::interleavedLayout(data, 10); @@ -596,6 +603,49 @@ void InterleaveTest::interleavedLayoutNothing() { CORRADE_COMPARE(layout.vertexData().size(), 0); } +void InterleaveTest::interleavedLayoutRvalue() { + Containers::Array indexData{6}; + Containers::Array vertexData{3*20}; + Containers::Array attributeData{2}; + attributeData[0] = Trade::MeshAttributeData{Trade::MeshAttribute::Position, + Containers::arrayCast(vertexData.prefix(3*8))}; + attributeData[1] = Trade::MeshAttributeData{Trade::MeshAttribute::Normal, + Containers::arrayCast(vertexData.suffix(3*8))}; + const void* originalAttributeData = attributeData.data(); + + Trade::MeshIndexData indices{Containers::arrayCast(indexData)}; + Trade::MeshData data{MeshPrimitive::TriangleFan, + std::move(indexData), indices, + std::move(vertexData), std::move(attributeData)}; + CORRADE_VERIFY(!MeshTools::isInterleaved(data)); + + /* Check that the attribute data array gets reused when moving a rvalue. + Explicitly passing an empty init list to verify the rvalue gets + propagated correctly through all functions. */ + Trade::MeshData layout = MeshTools::interleavedLayout(std::move(data), 10, + std::initializer_list{}); + CORRADE_VERIFY(layout.attributeData().data() == originalAttributeData); + + /* The rest is same as in interleavedLayout() */ + CORRADE_VERIFY(MeshTools::isInterleaved(layout)); + CORRADE_COMPARE(layout.primitive(), MeshPrimitive::TriangleFan); + CORRADE_VERIFY(!layout.isIndexed()); /* Indices are not preserved */ + CORRADE_COMPARE(layout.attributeCount(), 2); + CORRADE_COMPARE(layout.attributeName(0), Trade::MeshAttribute::Position); + CORRADE_COMPARE(layout.attributeName(1), Trade::MeshAttribute::Normal); + CORRADE_COMPARE(layout.attributeFormat(0), VertexFormat::Vector2); + CORRADE_COMPARE(layout.attributeFormat(1), VertexFormat::Vector3); + CORRADE_COMPARE(layout.attributeStride(0), 20); + CORRADE_COMPARE(layout.attributeStride(1), 20); + CORRADE_COMPARE(layout.attributeOffset(0), 0); + CORRADE_COMPARE(layout.attributeOffset(1), 8); + CORRADE_COMPARE(layout.vertexCount(), 10); + /* Needs to be like this so we can modify the data */ + CORRADE_COMPARE(layout.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned); + CORRADE_VERIFY(layout.vertexData()); + CORRADE_COMPARE(layout.vertexData().size(), 10*20); +} + void InterleaveTest::interleaveMeshData() { struct { Vector2 positions[3];