Wait on sync point fences instead of creating a new fence

Posting a runnable on the GPU thread's message loop and creating a GL
fence sync there assumes that there is a current GL context, and that
synchronizing with this context will make sure that we wait for all
GL context producing any of our consumed texture mailboxes.

This is however not always the case like when:
- The current GL context on the GPU thread is destroyed right before
  our runnable is handled, displaying errors on the console that
  glFlush needs a current context.
- The GL driver will do extra scheduling and let the scene graph
  thread synchronize its GL command stream only with the GL context
  in which the fence sync was created.

To remedy the situation, make sure that Chromium creates a fence sync
for every sync points associated with a mailbox that we consume and
do so directly in the GL context associated with the originating
glInsertSyncPointCHROMIUM call. Wait for all those syncs on the Qt
side afterward.

This might also help with a few erratic behaviors noticed on some
embedded GL drivers.

Change-Id: I5fc60fcf51497477b2e1b3a535d0a141954fc6e5
Reviewed-by: Andras Becsi <andras.becsi@digia.com>

1 files changed, 68 insertions, 24 deletions

diff --git a/src/core/delegated_frame_node.cpp b/src/core/delegated_frame_node.cpp
index 57ce581f1..3d3e0dd8f 100644
--- a/src/core/delegated_frame_node.cpp
+++ b/src/core/delegated_frame_node.cpp
@@ -202,42 +202,74 @@ static QSGNode *buildLayerChain(QSGNode *chainParent, const cc::SharedQuadState
     return layerChain;
 }
 
-static void waitAndDeleteChromiumSync(FenceSync *sync)
+static void waitChromiumSync(gfx::TransferableFence *sync)
 {
     // Chromium uses its own GL bindings and stores in in thread local storage.
     // For that reason, let chromium_gpu_helper.cpp contain the producing code that will run in the Chromium
     // GPU thread, and put the sync consuming code here that will run in the QtQuick SG or GUI thread.
     switch (sync->type) {
-    case FenceSync::NoSync:
+    case gfx::TransferableFence::NoSync:
         break;
-    case FenceSync::EglSync:
+    case gfx::TransferableFence::EglSync:
 #ifdef EGL_KHR_reusable_sync
     {
         static bool resolved = false;
         static PFNEGLCLIENTWAITSYNCKHRPROC eglClientWaitSyncKHR = 0;
-        static PFNEGLDESTROYSYNCKHRPROC eglDestroySyncKHR = 0;
 
         if (!resolved) {
             if (gfx::GLSurfaceQt::HasEGLExtension("EGL_KHR_reusable_sync")) {
                 QOpenGLContext *context = QOpenGLContext::currentContext();
                 eglClientWaitSyncKHR = (PFNEGLCLIENTWAITSYNCKHRPROC)context->getProcAddress("eglClientWaitSyncKHR");
-                eglDestroySyncKHR = (PFNEGLDESTROYSYNCKHRPROC)context->getProcAddress("eglDestroySyncKHR");
             }
             resolved = true;
         }
 
-        if (eglClientWaitSyncKHR && eglDestroySyncKHR) {
+        if (eglClientWaitSyncKHR)
             // FIXME: Use the less wasteful eglWaitSyncKHR once we have a device that supports EGL_KHR_wait_sync.
             eglClientWaitSyncKHR(sync->egl.display, sync->egl.sync, 0, EGL_FOREVER_KHR);
+    }
+#endif
+        break;
+    case gfx::TransferableFence::ArbSync:
+#ifdef GL_ARB_sync
+        glWaitSync(sync->arb.sync, 0, GL_TIMEOUT_IGNORED);
+#endif
+        break;
+    }
+}
+
+static void deleteChromiumSync(gfx::TransferableFence *sync)
+{
+    // Chromium uses its own GL bindings and stores in in thread local storage.
+    // For that reason, let chromium_gpu_helper.cpp contain the producing code that will run in the Chromium
+    // GPU thread, and put the sync consuming code here that will run in the QtQuick SG or GUI thread.
+    switch (sync->type) {
+    case gfx::TransferableFence::NoSync:
+        break;
+    case gfx::TransferableFence::EglSync:
+#ifdef EGL_KHR_reusable_sync
+    {
+        static bool resolved = false;
+        static PFNEGLDESTROYSYNCKHRPROC eglDestroySyncKHR = 0;
+
+        if (!resolved) {
+            if (gfx::GLSurfaceQt::HasEGLExtension("EGL_KHR_reusable_sync")) {
+                QOpenGLContext *context = QOpenGLContext::currentContext();
+                eglDestroySyncKHR = (PFNEGLDESTROYSYNCKHRPROC)context->getProcAddress("eglDestroySyncKHR");
+            }
+            resolved = true;
+        }
+
+        if (eglDestroySyncKHR) {
+            // FIXME: Use the less wasteful eglWaitSyncKHR once we have a device that supports EGL_KHR_wait_sync.
             eglDestroySyncKHR(sync->egl.display, sync->egl.sync);
             sync->reset();
         }
     }
 #endif
         break;
-    case FenceSync::ArbSync:
+    case gfx::TransferableFence::ArbSync:
 #ifdef GL_ARB_sync
-        glWaitSync(sync->arb.sync, 0, GL_TIMEOUT_IGNORED);
         glDeleteSync(sync->arb.sync);
         sync->reset();
 #endif
@@ -385,20 +417,33 @@ void DelegatedFrameNode::preprocess()
             mailboxesToFetch.append(mailboxTexture.data());
 
     if (!mailboxesToFetch.isEmpty()) {
-        QMutexLocker lock(&m_mutex);
-        base::MessageLoop *gpuMessageLoop = gpu_message_loop();
-        content::SyncPointManager *syncPointManager = sync_point_manager();
+        QMap<uint32, gfx::TransferableFence> transferredFences;
+        {
+            QMutexLocker lock(&m_mutex);
+            base::MessageLoop *gpuMessageLoop = gpu_message_loop();
+            content::SyncPointManager *syncPointManager = sync_point_manager();
+
+            Q_FOREACH (MailboxTexture *mailboxTexture, mailboxesToFetch) {
+                m_numPendingSyncPoints++;
+                AddSyncPointCallbackOnGpuThread(gpuMessageLoop, syncPointManager, mailboxTexture->resource().mailbox_holder.sync_point, base::Bind(&DelegatedFrameNode::syncPointRetired, this, &mailboxesToFetch));
+            }
 
-        Q_FOREACH (MailboxTexture *mailboxTexture, mailboxesToFetch) {
-            m_numPendingSyncPoints++;
-            AddSyncPointCallbackOnGpuThread(gpuMessageLoop, syncPointManager, mailboxTexture->resource().mailbox_holder.sync_point, base::Bind(&DelegatedFrameNode::syncPointRetired, this, &mailboxesToFetch));
+            m_mailboxesFetchedWaitCond.wait(&m_mutex);
+            m_mailboxGLFences.swap(transferredFences);
         }
 
-        m_mailboxesFetchedWaitCond.wait(&m_mutex);
-
-        // Tell GL to wait until Chromium is done generating resource textures on the GPU thread.
-        // We can safely start referencing those textures onto geometries afterward.
-        waitAndDeleteChromiumSync(&m_mailboxesGLFence);
+        // Tell GL to wait until Chromium is done generating resource textures on the GPU thread
+        // for each mailbox. We can safely start referencing those textures onto geometries afterward.
+        Q_FOREACH (MailboxTexture *mailboxTexture, mailboxesToFetch) {
+            gfx::TransferableFence fence = transferredFences.take(mailboxTexture->resource().mailbox_holder.sync_point);
+            waitChromiumSync(&fence);
+            deleteChromiumSync(&fence);
+        }
+        // We transferred all sync point fences from Chromium,
+        // destroy all the ones not referenced by one of our mailboxes without waiting.
+        QMap<uint32, gfx::TransferableFence>::iterator end = transferredFences.end();
+        for (QMap<uint32, gfx::TransferableFence>::iterator it = transferredFences.begin(); it != end; ++it)
+            deleteChromiumSync(&*it);
     }
 
     // Then render any intermediate RenderPass in order.
@@ -602,14 +647,13 @@ void DelegatedFrameNode::fetchTexturesAndUnlockQt(DelegatedFrameNode *frameNode,
     Q_FOREACH (MailboxTexture *mailboxTexture, *mailboxesToFetch)
         mailboxTexture->fetchTexture(mailboxManager);
 
-    // Set a fence at this point in Chromium's GL command stream
-    // and transfer the handle to the Qt scene graph thread.
-    FenceSync fence = createFence();
+    // Pick fences that we can ask GL to wait for before trying to sample the mailbox texture IDs.
+    QMap<uint32, gfx::TransferableFence> transferredFences = transferFences();
 
     // Chromium provided everything we were waiting for, let Qt start rendering.
     QMutexLocker lock(&frameNode->m_mutex);
-    Q_ASSERT(!frameNode->m_mailboxesGLFence);
-    frameNode->m_mailboxesGLFence = fence;
+    Q_ASSERT(frameNode->m_mailboxGLFences.isEmpty());
+    frameNode->m_mailboxGLFences.swap(transferredFences);
     frameNode->m_mailboxesFetchedWaitCond.wakeOne();
 }