Threaded render loop should block until the frame is swapped.

Introduced the concept of an expose cycle, as the WM_Expose
is followed by WM_RequestSync and only after we handled the
sync and then swap, should we unlock. Otherwise, we are
potentially waiting for the wrong swap.

This also fixes a bug when we get an expose while the render
thread is animating on its own. We would previously set
RequestRepaint, but this could potentially be reset by a render
thread animation if the GUI thread took a short while getting
to the point where it sent the RequestSync. If that requestsync
did not result in SG changes, we would not render anything.
With the exposeCycle, we are now guaranteed to repaint after
we handled the sync, which is how it should be.

Task-number: QTBUG-35805

Change-Id: Ib5c588f135763142e57f84b39dd3827fd05b9485
Reviewed-by: Morten Johan Sørvig <morten.sorvig@digia.com>
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>

diff --git a/src/quick/scenegraph/qsgthreadedrenderloop.cpp b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
index 5bc7be1..0db5e3f 100644 (file)
--- a/src/quick/scenegraph/qsgthreadedrenderloop.cpp
+++ b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
@@ -286,6 +286,7 @@ public:
         , pendingUpdate(0)
         , sleeping(false)
         , syncResultedInChanges(false)
+        , exposeCycle(NoExpose)
         , active(false)
         , window(0)
         , stopEventProcessing(false)
@@ -331,6 +332,12 @@ public:
         RepaintRequest      = 0x02
     };
 
+    enum ExposeCycle {
+        NoExpose,
+        ExposePendingSync,
+        ExposePendingSwap
+    };
+
     QSGThreadedRenderLoop *wm;
     QOpenGLContext *gl;
     QSGRenderContext *sgrc;
@@ -340,6 +347,7 @@ public:
     uint pendingUpdate;
     bool sleeping;
     bool syncResultedInChanges;
+    ExposeCycle exposeCycle;
 
     volatile bool active;
 
@@ -366,9 +374,10 @@ bool QSGRenderThread::event(QEvent *e)
         QSG_RT_DEBUG("WM_Expose");
         WMExposeEvent *se = static_cast<WMExposeEvent *>(e);
         Q_ASSERT(!window || window == se->window);
-        pendingUpdate |= RepaintRequest;
         windowSize = se->size;
         window = se->window;
+        Q_ASSERT(exposeCycle == NoExpose);
+        exposeCycle = ExposePendingSync;
         return true; }
 
     case WM_Obscure: {
@@ -393,6 +402,10 @@ bool QSGRenderThread::event(QEvent *e)
             stopEventProcessing = true;
         if (window)
             pendingUpdate |= SyncRequest;
+        if (exposeCycle == ExposePendingSync) {
+            pendingUpdate |= RepaintRequest;
+            exposeCycle = ExposePendingSwap;
+        }
         return true;
 
     case WM_TryRelease: {
@@ -601,6 +614,19 @@ void QSGRenderThread::syncAndRender()
 
     QSG_RT_DEBUG(" - rendering done");
 
+    // Though it would be more correct to put this block directly after
+    // fireFrameSwapped in the if (current) branch above, we don't do
+    // that to avoid blocking the GUI thread in the case where it
+    // has started rendering with a bad window, causing makeCurrent to
+    // fail or if the window has a bad size.
+    mutex.lock();
+    if (exposeCycle == ExposePendingSwap) {
+        QSG_RT_DEBUG(" - waking GUI after expose");
+        exposeCycle = NoExpose;
+        waitCondition.wakeOne();
+    }
+    mutex.unlock();
+
 #ifndef QSG_NO_RENDER_TIMING
         if (qsg_render_timing)
             qDebug("Render Thread: window=%p, framedelta=%d, sync=%d, first render=%d, after final swap=%d",
@@ -953,6 +979,15 @@ void QSGThreadedRenderLoop::handleExposure(Window *w)
     w->thread->postEvent(new WMExposeEvent(w->window));
     polishAndSync(w);
 
+    w->thread->mutex.lock();
+    if (w->thread->exposeCycle != QSGRenderThread::NoExpose) {
+        QSG_GUI_DEBUG(w->window, " - waiting for swap to complete...");
+        w->thread->waitCondition.wait(&w->thread->mutex);
+    }
+    Q_ASSERT(w->thread->exposeCycle == QSGRenderThread::NoExpose);
+    w->thread->mutex.unlock();
+    QSG_GUI_DEBUG(w->window, " - handleExposure completed...");
+
     startOrStopAnimationTimer();
 }