Fix QBuffer::updateData being called several times in a row

QBuffer::updateData would send a single update at the time and would
overwrite the latest update if it was called again before synching with
the backend took place.

This patches fixes by storing a list of pending updates instead.

Change-Id: I1fc501ad921c953ec88117fcc49c0cfcde6ca8aa
Task-number: QTBUG-81921
Reviewed-by: Mike Krus <mike.krus@kdab.com>

5 files changed, 104 insertions, 8 deletions

diff --git a/src/render/geometry/buffer.cpp b/src/render/geometry/buffer.cpp
index 6db3bab44..a1b552fb2 100644
--- a/src/render/geometry/buffer.cpp
+++ b/src/render/geometry/buffer.cpp
@@ -137,7 +137,7 @@ void Buffer::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
             m_manager->addDirtyBuffer(peerId());
     }
     {
-        const QVariant v = node->property("QT3D_updateData");
+        const QVariant v = node->property(QBufferPrivate::UpdateDataPropertyName);
 
         // Make sure we record data if it's the first time we are called
         // or if we have no partial updates
@@ -151,17 +151,21 @@ void Buffer::syncFromFrontEnd(const QNode *frontEnd, bool firstTime)
             // if we enter this code block, there's no problem in actually
             // ignoring the partial updates
             if (v.isValid())
-                const_cast<QBuffer *>(node)->setProperty("QT3D_updateData", {});
+                const_cast<QBuffer *>(node)->setProperty(QBufferPrivate::UpdateDataPropertyName, {});
 
             if (dirty && !m_data.isEmpty())
                 forceDataUpload();
         } else if (v.isValid()) {
             // Apply partial updates and record them to allow partial upload to the GPU
-            Qt3DRender::QBufferUpdate updateData = v.value<Qt3DRender::QBufferUpdate>();
-            m_data.replace(updateData.offset, updateData.data.size(), updateData.data);
-            m_bufferUpdates.push_back(updateData);
-            m_bufferDirty = true;
-            const_cast<QBuffer *>(node)->setProperty("QT3D_updateData", {});
+            const QVariantList updateList = v.toList();
+            for (const QVariant &update : updateList) {
+                Qt3DRender::QBufferUpdate updateData = update.value<Qt3DRender::QBufferUpdate>();
+                m_data.replace(updateData.offset, updateData.data.size(), updateData.data);
+                m_bufferUpdates.push_back(updateData);
+                m_bufferDirty = true;
+            }
+
+            const_cast<QBuffer *>(node)->setProperty(QBufferPrivate::UpdateDataPropertyName, {});
         }
     }
     markDirty(AbstractRenderer::BuffersDirty);
diff --git a/src/render/geometry/qbuffer.cpp b/src/render/geometry/qbuffer.cpp
index 8399bdd7c..daf6f91c8 100644
--- a/src/render/geometry/qbuffer.cpp
+++ b/src/render/geometry/qbuffer.cpp
@@ -49,6 +49,8 @@ using namespace Qt3DCore;
 
 namespace Qt3DRender {
 
+const char *QBufferPrivate::UpdateDataPropertyName = "QT3D_updateData";
+
 QBufferPrivate::QBufferPrivate()
     : QNodePrivate()
     , m_type(QBuffer::VertexBuffer)
@@ -338,7 +340,14 @@ void QBuffer::updateData(int offset, const QByteArray &bytes)
     QBufferUpdate updateData;
     updateData.offset = offset;
     updateData.data = bytes;
-    setProperty("QT3D_updateData", QVariant::fromValue(updateData));
+
+    QVariantList updateDataList;
+    const QVariant propertyData = property(QBufferPrivate::UpdateDataPropertyName);
+    if (propertyData.isValid())
+        updateDataList = propertyData.toList();
+    updateDataList.push_back(QVariant::fromValue(updateData));
+
+    setProperty(QBufferPrivate::UpdateDataPropertyName, updateDataList);
     d->update();
 }
 
diff --git a/src/render/geometry/qbuffer_p.h b/src/render/geometry/qbuffer_p.h
index 6da28e88b..a31c49e35 100644
--- a/src/render/geometry/qbuffer_p.h
+++ b/src/render/geometry/qbuffer_p.h
@@ -77,6 +77,8 @@ public:
     QBuffer::AccessType m_access;
 
     void setData(const QByteArray &data);
+
+    static const char *UpdateDataPropertyName;
 };
 
 struct QBufferData
diff --git a/tests/auto/render/buffer/tst_buffer.cpp b/tests/auto/render/buffer/tst_buffer.cpp
index 5c2ae296d..2401c2789 100644
--- a/tests/auto/render/buffer/tst_buffer.cpp
+++ b/tests/auto/render/buffer/tst_buffer.cpp
@@ -365,6 +365,40 @@ private Q_SLOTS:
         // THEN
         QCOMPARE(renderer.dirtyBits(), Qt3DRender::Render::AbstractRenderer::BuffersDirty);
     }
+
+    void checkHandlesMultipleUpdates()
+    {
+        // GIVEN
+        Qt3DRender::Render::Buffer renderBuffer;
+        Qt3DRender::QBuffer buffer;
+        Qt3DRender::Render::BufferManager bufferManager;
+        TestRenderer renderer;
+
+        const QByteArray initData("000000");
+        buffer.setData(initData);
+
+        renderBuffer.setRenderer(&renderer);
+        renderBuffer.setManager(&bufferManager);
+
+        simulateInitializationSync(&buffer, &renderBuffer);
+
+        // THEN
+        QCOMPARE(renderBuffer.data(), initData);
+        renderBuffer.pendingBufferUpdates().clear();
+
+        // WHEN
+        buffer.updateData(0, QByteArray("012"));
+        buffer.updateData(3, QByteArray("345"));
+        renderBuffer.syncFromFrontEnd(&buffer, false);
+
+        // THEN
+        QCOMPARE(renderBuffer.pendingBufferUpdates().size(), 2);
+        QCOMPARE(renderBuffer.pendingBufferUpdates().first().offset, 0);
+        QCOMPARE(renderBuffer.pendingBufferUpdates().first().data, QByteArray("012"));
+        QCOMPARE(renderBuffer.pendingBufferUpdates().last().offset, 3);
+        QCOMPARE(renderBuffer.pendingBufferUpdates().last().data, QByteArray("345"));
+        QCOMPARE(renderBuffer.data(), QByteArray("012345"));
+    }
 };
 
 
diff --git a/tests/auto/render/qbuffer/tst_qbuffer.cpp b/tests/auto/render/qbuffer/tst_qbuffer.cpp
index fd96bf1c2..084ad0347 100644
--- a/tests/auto/render/qbuffer/tst_qbuffer.cpp
+++ b/tests/auto/render/qbuffer/tst_qbuffer.cpp
@@ -181,6 +181,53 @@ private Q_SLOTS:
         QCOMPARE(arbiter.dirtyNodes.size(), 1);
         QCOMPARE(arbiter.dirtyNodes.front(), buffer.data());
     }
+
+    void checkAccumulatesBufferUpdates()
+    {
+        // GIVEN
+        TestArbiter arbiter;
+        QScopedPointer<Qt3DRender::QBuffer> buffer(new Qt3DRender::QBuffer);
+        arbiter.setArbiterOnNode(buffer.data());
+
+        QByteArray initData(6, '\0');
+        buffer->setData(initData);
+
+        // THEN
+        QCOMPARE(buffer->data(), initData);
+        QCOMPARE(buffer->property(Qt3DRender::QBufferPrivate::UpdateDataPropertyName), QVariant());
+
+        // WHEN
+        const QByteArray uData("012");
+        buffer->updateData(0, uData);
+
+        // THEN
+        {
+            const QVariant v = buffer->property(Qt3DRender::QBufferPrivate::UpdateDataPropertyName);
+            QVERIFY(v.isValid());
+            const QVariantList l = v.toList();
+            QCOMPARE(l.size(), 1);
+            const Qt3DRender::QBufferUpdate update = l.first().value<Qt3DRender::QBufferUpdate>();
+            QCOMPARE(update.offset, 0);
+            QCOMPARE(update.data, uData);
+            QCOMPARE(buffer->data().mid(0, 3), uData);
+        }
+
+        // WHEN
+        const QByteArray uData2("345");
+        buffer->updateData(3, uData2);
+
+        // THEN
+        {
+            const QVariant v = buffer->property(Qt3DRender::QBufferPrivate::UpdateDataPropertyName);
+            QVERIFY(v.isValid());
+            const QVariantList l = v.toList();
+            QCOMPARE(l.size(), 2);
+            const Qt3DRender::QBufferUpdate update = l.last().value<Qt3DRender::QBufferUpdate>();
+            QCOMPARE(update.offset, 3);
+            QCOMPARE(update.data, uData2);
+            QCOMPARE(buffer->data(), QByteArray("012345"));
+        }
+    }
 };
 
 QTEST_MAIN(tst_QBuffer)