From 3bcc847596f38818ecd0a1ca8be5d1c0e7f4615b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Wed, 10 May 2023 19:21:59 +0200
Subject: [PATCH] MeshTools: preserve offset-only attributes in copy().

---
 src/Magnum/MeshTools/Copy.cpp          | 19 +++++++++++++------
 src/Magnum/MeshTools/Copy.h            |  8 ++++++--
 src/Magnum/MeshTools/Test/CopyTest.cpp | 26 ++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/src/Magnum/MeshTools/Copy.cpp b/src/Magnum/MeshTools/Copy.cpp
index bd6e69428..23ecba51d 100644
--- a/src/Magnum/MeshTools/Copy.cpp
+++ b/src/Magnum/MeshTools/Copy.cpp
@@ -142,15 +142,22 @@ Trade::MeshData copy(Trade::MeshData&& mesh) {
            problematic to use in plugins */
         attributeData = Containers::Array<Trade::MeshAttributeData>{DefaultInit, originalAttributeData.size()};
         for(std::size_t i = 0; i != originalAttributeData.size(); ++i) {
-            attributeData[i] = Trade::MeshAttributeData{
-                originalAttributeData[i].name(),
-                originalAttributeData[i].format(),
+            const Trade::MeshAttributeData& originalAttribute = originalAttributeData[i];
+
+            /* If the attribute is offset-only, copy it directly, yay! */
+            if(originalAttribute.isOffsetOnly())
+                attributeData[i] = originalAttribute;
+
+            /* Otherwise it's kinda verbose */
+            else attributeData[i] = Trade::MeshAttributeData{
+                originalAttribute.name(),
+                originalAttribute.format(),
                 Containers::StridedArrayView1D<const void>{
                     vertexData,
-                    vertexData.data() + originalAttributeData[i].offset(originalVertexData),
+                    vertexData.data() + originalAttribute.offset(originalVertexData),
                     vertexCount,
-                    originalAttributeData[i].stride()},
-                originalAttributeData[i].arraySize()};
+                    originalAttribute.stride()},
+                originalAttribute.arraySize()};
         }
     }
 
diff --git a/src/Magnum/MeshTools/Copy.h b/src/Magnum/MeshTools/Copy.h
index 0f7791635..7f7fc616c 100644
--- a/src/Magnum/MeshTools/Copy.h
+++ b/src/Magnum/MeshTools/Copy.h
@@ -46,7 +46,9 @@ All other properties such as the primitive or importer state are passed through
 unchanged, the data layout isn't changed in any way. The resulting
 @ref Trade::MeshData::indexDataFlags() and
 @relativeref{Trade::MeshData,vertexDataFlags()} are always
-@ref Trade::DataFlag::Owned and @ref Trade::DataFlag::Mutable.
+@ref Trade::DataFlag::Owned and @ref Trade::DataFlag::Mutable. Attributes that
+were offset-only before are kept offset-only, others have offsets recalculated
+against the newly-allocated vertex data.
 @see @ref copy(Trade::MeshData&&), @ref reference(), @ref mutableReference()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData copy(const Trade::MeshData& mesh);
@@ -63,7 +65,9 @@ data don't have the default deleter, allocates a copy of
 @relativeref{Trade::MeshData,vertexData()} or
 @relativeref{Trade::MeshData,attributeData()}, otherwise transfers their
 ownership. The resulting data are always owned and mutable, the data layout
-isn't changed in any way.
+isn't changed in any way. Attributes that were offset-only before are kept
+offset-only, others have offsets recalculated against the
+potentially-newly-allocated vertex data.
 @see @ref reference(), @ref mutableReference()
 */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData copy(Trade::MeshData&& mesh);
diff --git a/src/Magnum/MeshTools/Test/CopyTest.cpp b/src/Magnum/MeshTools/Test/CopyTest.cpp
index 349999e33..84266c211 100644
--- a/src/Magnum/MeshTools/Test/CopyTest.cpp
+++ b/src/Magnum/MeshTools/Test/CopyTest.cpp
@@ -42,6 +42,7 @@ struct CopyTest: TestSuite::Tester {
     explicit CopyTest();
 
     void copy();
+    void copyOffsetOnlyAttributes();
     void copyNoIndexData();
     void copyNoAttributeVertexData();
     void copyStridedIndices();
@@ -72,6 +73,7 @@ struct {
 
 CopyTest::CopyTest() {
     addTests({&CopyTest::copy,
+              &CopyTest::copyOffsetOnlyAttributes,
               &CopyTest::copyNoIndexData,
               &CopyTest::copyNoAttributeVertexData});
 
@@ -134,6 +136,30 @@ void CopyTest::copy() {
     CORRADE_VERIFY(!attributeData.deleter());
 }
 
+void CopyTest::copyOffsetOnlyAttributes() {
+    Trade::MeshData circle = Primitives::circle2DSolid(5);
+    Trade::MeshData copy = MeshTools::copy(circle);
+    CORRADE_COMPARE(copy.primitive(), circle.primitive());
+    CORRADE_COMPARE(copy.vertexCount(), circle.vertexCount());
+    CORRADE_COMPARE(copy.attributeCount(), circle.attributeCount());
+
+    for(std::size_t i = 0; i != circle.attributeCount(); ++i) {
+        CORRADE_ITERATION(i);
+
+        CORRADE_COMPARE(copy.attributeName(i), circle.attributeName(i));
+        CORRADE_COMPARE(copy.attributeFormat(i), circle.attributeFormat(i));
+        CORRADE_COMPARE(copy.attributeOffset(i), circle.attributeOffset(i));
+        CORRADE_COMPARE(copy.attributeStride(i), circle.attributeStride(i));
+        CORRADE_COMPARE(copy.attributeArraySize(i), circle.attributeArraySize(i));
+
+        /* The circle has offset-only attributes, that should be preserved
+           after the copy as well */
+        CORRADE_VERIFY(copy.attributeData()[i].isOffsetOnly());
+    }
+
+    CORRADE_COMPARE_AS(copy.vertexData(), circle.vertexData(), TestSuite::Compare::Container);
+}
+
 void CopyTest::copyNoIndexData() {
     Trade::MeshData cube = Primitives::cubeSolidStrip();
     CORRADE_VERIFY(!cube.isIndexed());