MeshTools: add interleavedLayout() that can reuse the attribute array.

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<const Trade::MeshAttributeData> extra) {
+Trade::MeshData interleavedLayout(Trade::MeshData&& data, const UnsignedInt vertexCount, const Containers::ArrayView<const Trade::MeshAttributeData> 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<Trade::MeshAttributeData> originalAttributeData =
+        data.releaseAttributeData();
+    Containers::Array<Trade::MeshAttributeData> attributeData;
+    if(!extraAttributeCount && !originalAttributeData.deleter())
+        attributeData = std::move(originalAttributeData);
+    else {
+        attributeData = Containers::Array<Trade::MeshAttributeData>{originalAttributeCount + extraAttributeCount};
+        Utility::copy(originalAttributeData, attributeData.prefix(originalAttributeCount));
+    }
+
+    /* Allocate new data array */
     Containers::Array<char> vertexData{Containers::NoInit, stride*vertexCount};
-    Containers::Array<Trade::MeshAttributeData> 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) {
+    for(UnsignedInt i = 0; i != originalAttributeCount; ++i) {
-        if(interleaved) offset = data.attributeOffset(i) - minOffset;
+        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<void>{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())
+    if(interleaved && originalAttributeCount)
-        offset = data.attributeStride(0);
+        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<Trade::MeshAttributeData> extra) {
+    return interleavedLayout(std::move(data), vertexCount, Containers::arrayView(extra));
+}
+
+Trade::MeshData interleavedLayout(const Trade::MeshData& data, const UnsignedInt vertexCount, const Containers::ArrayView<const Trade::MeshAttributeData> 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<Trade::MeshAttributeData> extra) {
     return interleavedLayout(data, vertexCount, Containers::arrayView(extra));
 }

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<const Trade::MeshAttributeData>)
+to avoid that allocation.
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(const Trade::MeshData& data, UnsignedInt vertexCount, Containers::ArrayView<const Trade::MeshAttributeData> 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<Trade::MeshAttributeData> extra);
 
+/**
+@brief Create an interleaved mesh layout
+@m_since_latest
+
+Compared to @ref interleavedLayout(const Trade::MeshData&, UnsignedInt, Containers::ArrayView<const Trade::MeshAttributeData>)
+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<const Trade::MeshAttributeData> extra = {});
+
+/**
+ * @overload
+ * @m_since_latest
+ */
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& data, UnsignedInt vertexCount, std::initializer_list<Trade::MeshAttributeData> extra);
+
 /**
 @brief Interleave mesh data
 @m_since_latest

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<char> indexData{6};
     Containers::Array<char> vertexData{3*20};
-    Trade::MeshAttributeData positions{Trade::MeshAttribute::Position,
+
-        Containers::arrayCast<Vector2>(vertexData.prefix(3*8))};
+    const Trade::MeshAttributeData attributeData[]{
-    Trade::MeshAttributeData normals{Trade::MeshAttribute::Normal,
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position,
-        Containers::arrayCast<Vector3>(vertexData.suffix(3*8))};
+            Containers::arrayCast<Vector2>(vertexData.prefix(3*8))},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Normal,
+            Containers::arrayCast<Vector3>(vertexData.suffix(3*8))}
+    };
 
     Trade::MeshIndexData indices{Containers::arrayCast<UnsignedShort>(indexData)};
     Trade::MeshData data{MeshPrimitive::TriangleFan,
-        std::move(indexData), indices,
+        std::move(indexData), indices, std::move(vertexData),
-        std::move(vertexData), {positions, normals}};
+        /* 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<char> indexData{6};
+    Containers::Array<char> vertexData{3*20};
+    Containers::Array<Trade::MeshAttributeData> attributeData{2};
+    attributeData[0] = Trade::MeshAttributeData{Trade::MeshAttribute::Position,
+        Containers::arrayCast<Vector2>(vertexData.prefix(3*8))};
+    attributeData[1] = Trade::MeshAttributeData{Trade::MeshAttribute::Normal,
+        Containers::arrayCast<Vector3>(vertexData.suffix(3*8))};
+    const void* originalAttributeData = attributeData.data();
+
+    Trade::MeshIndexData indices{Containers::arrayCast<UnsignedShort>(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<Trade::MeshAttributeData>{});
+    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];