Use a fence sync to synchronize GL between threads

The NVidia driver needs more than a glFlush to ensure that GL commands
consuming Chromium resources are run only when the resource is completely
produced by the Chromium GPU thread.

This produces artifacts and an uneven frame rate in WebGL examples
on this kind of hardware.

Use the same mechanism as used by gfx::GLFence, doing a few things
manually to cope with the fact that Chromium and Qt both have their
own GL function table and contexts.

Change-Id: I33eeb1068994dc4176038a74579ce768b2bccb9d
Reviewed-by: Andras Becsi <andras.becsi@digia.com>
Reviewed-by: Zeno Albisser <zeno.albisser@digia.com>

5 files changed, 114 insertions, 3 deletions

diff --git a/src/core/chromium_gpu_helper.cpp b/src/core/chromium_gpu_helper.cpp
index 3d393c25d..8782f5d98 100644
--- a/src/core/chromium_gpu_helper.cpp
+++ b/src/core/chromium_gpu_helper.cpp
@@ -39,6 +39,10 @@
 **
 ****************************************************************************/
 
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
 #include "chromium_gpu_helper.h"
 
 #include "content/common/gpu/gpu_channel_manager.h"
@@ -52,6 +56,30 @@ static void addSyncPointCallbackDelegate(content::SyncPointManager *syncPointMan
     syncPointManager->AddSyncPointCallback(sync_point, callback);
 }
 
+FenceSync createFence()
+{
+    FenceSync ret;
+    // Logic taken from chromium/ui/gl/gl_fence.cc
+#if !defined(OS_MACOSX)
+    if (gfx::g_driver_egl.ext.b_EGL_KHR_fence_sync) {
+        ret.type = FenceSync::EglSync;
+        ret.egl.display = eglGetCurrentDisplay();
+        ret.egl.sync = eglCreateSyncKHR(ret.egl.display, EGL_SYNC_FENCE_KHR, NULL);
+    } else
+#endif
+    if (gfx::g_driver_gl.ext.b_GL_ARB_sync) {
+        ret.type = FenceSync::ArbSync;
+        ret.arb.sync = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
+    }
+
+    // glFlush is necessary to make sure that our fence creation reaches the GL server
+    // before we try waiting on it from a different context, which could deadlock.
+    // In cases where no fence extension is available, this also serves as flushing
+    // Chromium's GL context command stream before yielding to the SG thread.
+    glFlush();
+    return ret;
+}
+
 base::MessageLoop *gpu_message_loop()
 {
     return content::GpuChildThread::instance()->message_loop();
diff --git a/src/core/chromium_gpu_helper.h b/src/core/chromium_gpu_helper.h
index 09ae5a77e..285554cc9 100644
--- a/src/core/chromium_gpu_helper.h
+++ b/src/core/chromium_gpu_helper.h
@@ -60,11 +60,38 @@ class Texture;
 }
 }
 
+typedef void *EGLDisplay;
+typedef void *EGLSyncKHR;
+typedef struct __GLsync *GLsync;
+
+union FenceSync {
+    enum SyncType {
+        NoSync,
+        EglSync,
+        ArbSync
+    };
+    SyncType type;
+    struct {
+        SyncType type;
+        EGLDisplay display;
+        EGLSyncKHR sync;
+    } egl;
+    struct {
+        SyncType type;
+        GLsync sync;
+    } arb;
+
+    FenceSync() : type(NoSync) { }
+    operator bool() { return type != NoSync; }
+    void reset() { type = NoSync; }
+};
+
 // These functions wrap code that needs to include headers that are
 // incompatible with Qt GL headers.
 // From the outside, types from incompatible headers referenced in these
 // functions should only be forward-declared and considered as opaque types.
 
+FenceSync createFence();
 base::MessageLoop *gpu_message_loop();
 content::SyncPointManager *sync_point_manager();
 gpu::gles2::MailboxManager *mailbox_manager();
diff --git a/src/core/core_gyp_generator.pro b/src/core/core_gyp_generator.pro
index 4012f198a..be9bf87c0 100644
--- a/src/core/core_gyp_generator.pro
+++ b/src/core/core_gyp_generator.pro
@@ -22,6 +22,9 @@ DEFINES += QT_NO_KEYWORDS \
 # Keep Skia happy
 CONFIG(release, debug|release): DEFINES += NDEBUG
 
+contains(QT_CONFIG, egl): CONFIG += egl
+else: DEFINES += QT_NO_EGL
+
 RESOURCES += devtools.qrc
 
 # something fishy with qmake in 5.2 ?
diff --git a/src/core/delegated_frame_node.cpp b/src/core/delegated_frame_node.cpp
index 1f7084d18..608d0b63a 100644
--- a/src/core/delegated_frame_node.cpp
+++ b/src/core/delegated_frame_node.cpp
@@ -75,6 +75,11 @@
 #include <QtQuick/private/qsgrenderer_p.h>
 #include <QtQuick/private/qsgtexture_p.h>
 
+#if !defined(QT_NO_EGL)
+#include <EGL/egl.h>
+#include <EGL/eglext.h>
+#endif
+
 class RenderPassTexture : public QSGTexture
 {
 public:
@@ -205,6 +210,45 @@ static QSGNode *buildLayerChain(QSGNode *chainParent, const cc::SharedQuadState
     return layerChain;
 }
 
+static void waitAndDeleteChromiumSync(FenceSync *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:
+        break;
+    case FenceSync::EglSync:
+#ifdef EGL_KHR_reusable_sync
+        static bool resolved = false;
+        static PFNEGLCLIENTWAITSYNCKHRPROC eglClientWaitSyncKHR = 0;
+        static PFNEGLDESTROYSYNCKHRPROC eglDestroySyncKHR = 0;
+
+        if (!resolved) {
+            QOpenGLContext *context = QOpenGLContext::currentContext();
+            eglClientWaitSyncKHR = (PFNEGLCLIENTWAITSYNCKHRPROC)context->getProcAddress("eglClientWaitSyncKHR");
+            eglDestroySyncKHR = (PFNEGLDESTROYSYNCKHRPROC)context->getProcAddress("eglDestroySyncKHR");
+            resolved = true;
+        }
+
+        // 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);
+        eglDestroySyncKHR(sync->egl.display, sync->egl.sync);
+        sync->reset();
+#endif
+        break;
+    case FenceSync::ArbSync:
+#ifdef GL_ARB_sync
+        glWaitSync(sync->arb.sync, 0, GL_TIMEOUT_IGNORED);
+        glDeleteSync(sync->arb.sync);
+        sync->reset();
+#endif
+        break;
+    }
+    // If Chromium was able to create a sync, we should have been able to handle its type here too.
+    Q_ASSERT(!*sync);
+}
+
 RenderPassTexture::RenderPassTexture(const cc::RenderPass::Id &id, QSGRenderContext *context)
     : QSGTexture()
     , m_id(id)
@@ -317,6 +361,10 @@ void DelegatedFrameNode::preprocess()
         }
 
         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);
     }
 
     // Then render any intermediate RenderPass in order.
@@ -533,12 +581,14 @@ void DelegatedFrameNode::fetchTexturesAndUnlockQt(DelegatedFrameNode *frameNode,
     Q_FOREACH (MailboxTexture *mailboxTexture, *mailboxesToFetch)
         mailboxTexture->fetchTexture(mailboxManager);
 
-    // glFlush before yielding to the SG thread, whose context might already start using
-    // some shared resources provided by the unflushed context here, on the Chromium GPU thread.
-    glFlush();
+    // 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();
 
     // 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;
     frameNode->m_mailboxesFetchedWaitCond.wakeOne();
 }
 
diff --git a/src/core/delegated_frame_node.h b/src/core/delegated_frame_node.h
index 214894821..a42e293a5 100644
--- a/src/core/delegated_frame_node.h
+++ b/src/core/delegated_frame_node.h
@@ -50,6 +50,8 @@
 #include <QSharedPointer>
 #include <QWaitCondition>
 
+#include "chromium_gpu_helper.h"
+
 QT_BEGIN_NAMESPACE
 class QSGRenderContext;
 QT_END_NAMESPACE
@@ -83,6 +85,7 @@ private:
     QSGRenderContext *m_sgRenderContext;
     QList<QSharedPointer<RenderPassTexture> > m_renderPassTextures;
     int m_numPendingSyncPoints;
+    FenceSync m_mailboxesGLFence;
     QWaitCondition m_mailboxesFetchedWaitCond;
     QMutex m_mutex;