Don't overwrite execute command queue when transferring queued commandsv5.10.0-beta3v5.10.0-beta2v5.10.0-beta1

There's no guarantee that we'll only transfer commands after
successfully executing the existing command in the queue, so we
must take care to append to the execute queue, not overwrite it.

This can for example happen if an update request comes in which
results in transferring commands via Canvas::sync(), followed by
a synchronous command such as readPixels, which will trigger
another transfer plus an execution of the command queue.

Task-number: QTBUG-62963
Change-Id: Id0f326e98477ca14e70ad05706c62a8bf8a8d5f2
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Tomi Korpipää <tomi.korpipaa@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>

4 files changed, 27 insertions, 18 deletions

diff --git a/src/imports/qtcanvas3d/canvasrenderer.cpp b/src/imports/qtcanvas3d/canvasrenderer.cpp
index db16051..95f77ad 100644
--- a/src/imports/qtcanvas3d/canvasrenderer.cpp
+++ b/src/imports/qtcanvas3d/canvasrenderer.cpp
@@ -809,21 +809,33 @@ void CanvasRenderer::createFBOs()
 void CanvasRenderer::transferCommands()
 {
     if (m_glContext) {
-        const int count = m_commandQueue.queuedCount();
-        if (count > m_executeQueue.size())
-            m_executeQueue.resize(count);
+        const int commandQueueCount = m_commandQueue.queuedCount();
+        const int finalExecuteQueueCount = m_executeQueueCount + commandQueueCount;
+        if (finalExecuteQueueCount > m_executeQueue.size())
+            m_executeQueue.resize(finalExecuteQueueCount);
         if (m_renderTarget == Canvas::RenderTargetOffscreenBuffer) {
-            m_executeQueueCount = count;
-            m_commandQueue.transferCommands(m_executeQueue);
+            m_commandQueue.transferCommands(m_executeQueue.data() + m_executeQueueCount);
+            m_executeQueueCount = finalExecuteQueueCount;
         } else {
             m_clearMask = m_commandQueue.resetClearMask();
-            // Use previous frame count and indices if no new commands, otherwise reset values
-            if (count) {
-                deleteCommandData();
-                m_executeQueueCount = count;
-                m_executeStartIndex = 0;
-                m_executeEndIndex = 0;
-                m_commandQueue.transferCommands(m_executeQueue);
+            if (commandQueueCount) {
+                if (m_executeStartIndex) {
+                    // The commands in the execute queue have been executed at least once,
+                    // so we can assume the new commands represent a new frame. Delete old commands.
+                    deleteCommandData();
+                    m_executeStartIndex = 0;
+                    m_executeEndIndex = 0;
+                    m_commandQueue.transferCommands(m_executeQueue.data());
+                    m_executeQueueCount = commandQueueCount;
+                } else {
+                    // Append new commands to existing non-executed commands
+                    m_commandQueue.transferCommands(m_executeQueue.data() + m_executeQueueCount);
+                    m_executeQueueCount = finalExecuteQueueCount;
+                }
+            } else {
+                // No new commands. Leave m_executeQueue alone, and let the renderer
+                // re-render the commands between m_executeStartIndex and m_executeEndIndex
+                // for the next frame.
             }
         }
     }
diff --git a/src/imports/qtcanvas3d/glcommandqueue.cpp b/src/imports/qtcanvas3d/glcommandqueue.cpp
index 0a23aae..929d7f8 100644
--- a/src/imports/qtcanvas3d/glcommandqueue.cpp
+++ b/src/imports/qtcanvas3d/glcommandqueue.cpp
@@ -176,9 +176,9 @@ GlCommand &CanvasGlCommandQueue::queueCommand(CanvasGlCommandQueue::GlCommandId
  * Copies command data to execute queue. GUI thread must be locked when this
  * method is called.
  */
-void CanvasGlCommandQueue::transferCommands(QVector<GlCommand> &executeQueue)
+void CanvasGlCommandQueue::transferCommands(GlCommand executeQueue[])
 {
-    memcpy(executeQueue.data(), m_queue.data(), m_queuedCount * sizeof(GlCommand));
+    memcpy(executeQueue, m_queue.data(), m_queuedCount * sizeof(GlCommand));
 
     m_queuedCount = 0;
 
diff --git a/src/imports/qtcanvas3d/glcommandqueue_p.h b/src/imports/qtcanvas3d/glcommandqueue_p.h
index 1c29009..b40cb66 100644
--- a/src/imports/qtcanvas3d/glcommandqueue_p.h
+++ b/src/imports/qtcanvas3d/glcommandqueue_p.h
@@ -248,7 +248,7 @@ public:
                             GLint i1, GLint i2, GLfloat p1, GLfloat p2 = 0.0f, GLfloat p3 = 0.0f,
                             GLfloat p4 = 0.0f);
 
-    void transferCommands(QVector<GlCommand> &executeQueue);
+    void transferCommands(GlCommand executeQueue[]);
     void resetQueue(int size);
     void deleteUntransferedCommandData();
 
diff --git a/tests/auto/qmltest/BLACKLIST b/tests/auto/qmltest/BLACKLIST
deleted file mode 100644
index c1fb74d..0000000
--- a/tests/auto/qmltest/BLACKLIST
+++ /dev/null
@@ -1,3 +0,0 @@
-# Race condition when doing requestUpdate via setNeedsDisplay
-[Canvas3D_render_ondemand::test_render_3_ondemand]
-osx