Merge pull request #12300 from jherico/fix/amd_crash

Trying to fix AMD crash in QML rendering

libraries/gl/src/gl/Context.cpp

@@ -23,25 +23,31 @@
 #include <shared/GlobalAppProperties.h>
 #include <GLMHelpers.h>
 #include "GLLogging.h"
-#include "Config.h"
-
-#ifdef Q_OS_WIN
-
-#if defined(DEBUG) || defined(USE_GLES)
-static bool enableDebugLogger = true;
-#else
-static const QString DEBUG_FLAG("HIFI_DEBUG_OPENGL");
-static bool enableDebugLogger = QProcessEnvironment::systemEnvironment().contains(DEBUG_FLAG);
-#endif
-
-#endif
-
 #include "Config.h"
 #include "GLHelpers.h"
 
 using namespace gl;
 
 
+bool Context::enableDebugLogger() {
+#if defined(DEBUG) || defined(USE_GLES)
+    static bool enableDebugLogger = true;
+#else
+    static const QString DEBUG_FLAG("HIFI_DEBUG_OPENGL");
+    static bool enableDebugLogger = QProcessEnvironment::systemEnvironment().contains(DEBUG_FLAG);
+#endif
+    static std::once_flag once;
+    std::call_once(once, [&] {
+        // If the previous run crashed, force GL debug logging on
+        if (qApp->property(hifi::properties::CRASHED).toBool()) {
+            enableDebugLogger = true;
+        }
+    });
+    return enableDebugLogger;
+}
+
+
+
 std::atomic<size_t> Context::_totalSwapchainMemoryUsage { 0 };
 
 size_t Context::getSwapchainMemoryUsage() { return _totalSwapchainMemoryUsage.load(); }
@@ -245,10 +251,6 @@ void Context::create() {
     // Create a temporary context to initialize glew
     static std::once_flag once;
     std::call_once(once, [&] {
-        // If the previous run crashed, force GL debug logging on
-        if (qApp->property(hifi::properties::CRASHED).toBool()) {
-            enableDebugLogger = true;
-        }
         auto hdc = GetDC(hwnd);
         setupPixelFormatSimple(hdc);
         auto glrc = wglCreateContext(hdc);
@@ -328,7 +330,7 @@ void Context::create() {
         contextAttribs.push_back(WGL_CONTEXT_CORE_PROFILE_BIT_ARB);
 #endif
         contextAttribs.push_back(WGL_CONTEXT_FLAGS_ARB);
-        if (enableDebugLogger) {
+        if (enableDebugLogger()) {
             contextAttribs.push_back(WGL_CONTEXT_DEBUG_BIT_ARB);
         } else {
             contextAttribs.push_back(0);
@@ -350,7 +352,7 @@ void Context::create() {
     if (!makeCurrent()) {
         throw std::runtime_error("Could not make context current");
     }
-    if (enableDebugLogger) {
+    if (enableDebugLogger()) {
         glDebugMessageCallback(debugMessageCallback, NULL);
         glEnable(GL_DEBUG_OUTPUT_SYNCHRONOUS_ARB);
     }

libraries/gl/src/gl/Context.h

@@ -47,6 +47,7 @@ namespace gl {
         Context(const Context& other);
 
     public:
+        static bool enableDebugLogger();
         Context();
         Context(QWindow* window);
         void release();

libraries/gl/src/gl/OffscreenGLCanvas.cpp

@@ -18,6 +18,7 @@
 #include <QtCore/QDebug>
 #include <QtGui/QOffscreenSurface>
 #include <QtGui/QOpenGLContext>
+#include <QtGui/QOpenGLDebugLogger>
 
 #include "Context.h"
 #include "GLHelpers.h"
@@ -68,9 +69,32 @@ bool OffscreenGLCanvas::create(QOpenGLContext* sharedContext) {
     }
 #endif
 
+    if (gl::Context::enableDebugLogger()) {
+        _context->makeCurrent(_offscreenSurface);
+        QOpenGLDebugLogger *logger = new QOpenGLDebugLogger(this);
+        connect(logger, &QOpenGLDebugLogger::messageLogged, this, &OffscreenGLCanvas::onMessageLogged);
+        logger->initialize();
+        logger->enableMessages();
+        logger->startLogging(QOpenGLDebugLogger::SynchronousLogging);
+        _context->doneCurrent();
+    }
+
     return true;
 }
 
+void OffscreenGLCanvas::onMessageLogged(const QOpenGLDebugMessage& debugMessage) {
+    auto severity = debugMessage.severity(); 
+    switch (severity) {
+    case QOpenGLDebugMessage::NotificationSeverity:
+    case QOpenGLDebugMessage::LowSeverity:
+        return;
+    default:
+        break;
+    }
+    qDebug(glLogging) << debugMessage;
+    return;
+}
+
 bool OffscreenGLCanvas::makeCurrent() {
     bool result = _context->makeCurrent(_offscreenSurface);
     std::call_once(_reportOnce, []{

libraries/gl/src/gl/OffscreenGLCanvas.h

@@ -17,7 +17,7 @@
 
 class QOpenGLContext;
 class QOffscreenSurface;
-class QOpenGLDebugLogger;
+class QOpenGLDebugMessage;
 
 class OffscreenGLCanvas : public QObject {
 public:
@@ -32,6 +32,9 @@ public:
     }
     QObject* getContextObject();
     
+private slots:
+    void onMessageLogged(const QOpenGLDebugMessage &debugMessage);
+
 protected:
     std::once_flag _reportOnce;
     QOpenGLContext* _context{ nullptr };

libraries/ui/src/ui/OffscreenQmlSurface.cpp

@@ -543,6 +543,7 @@ void OffscreenQmlSurface::cleanup() {
 
 void OffscreenQmlSurface::render() {
 #if !defined(DISABLE_QML)
+
     if (nsightActive()) {
         return;
     }
@@ -569,14 +570,18 @@ void OffscreenQmlSurface::render() {
 
     {
         // If the most recent texture was unused, we can directly recycle it
-        if (_latestTextureAndFence.first) {
-            offscreenTextures.releaseTexture(_latestTextureAndFence);
-            _latestTextureAndFence = { 0, 0 };
-        }
-
-        _latestTextureAndFence = { texture, glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0) };
+        auto fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
         // Fence will be used in another thread / context, so a flush is required
         glFlush();
+
+        {
+            Lock lock(_latestTextureAndFenceMutex);
+            if (_latestTextureAndFence.first) {
+                offscreenTextures.releaseTexture(_latestTextureAndFence);
+                _latestTextureAndFence = { 0, 0 };
+            }
+            _latestTextureAndFence = { texture, fence};
+        }
     }
 
     _quickWindow->resetOpenGLState();
@@ -588,13 +593,21 @@ void OffscreenQmlSurface::render() {
 bool OffscreenQmlSurface::fetchTexture(TextureAndFence& textureAndFence) {
     textureAndFence = { 0, 0 };
 
+    // Lock free early check
     if (0 == _latestTextureAndFence.first) {
         return false;
     }
 
     // Ensure writes to the latest texture are complete before before returning it for reading
-    textureAndFence = _latestTextureAndFence;
-    _latestTextureAndFence = { 0, 0 };
+    {
+        Lock lock(_latestTextureAndFenceMutex);
+        // Double check inside the lock
+        if (0 == _latestTextureAndFence.first) {
+            return false;
+        }
+        textureAndFence = _latestTextureAndFence;
+        _latestTextureAndFence = { 0, 0 };
+    }
     return true;
 }
 
@@ -813,10 +826,13 @@ void OffscreenQmlSurface::resize(const QSize& newSize_, bool forceResize) {
 
         // Release hold on the textures of the old size
         if (uvec2() != _size) {
-            // If the most recent texture was unused, we can directly recycle it
-            if (_latestTextureAndFence.first) {
-                offscreenTextures.releaseTexture(_latestTextureAndFence);
-                _latestTextureAndFence = { 0, 0 };
+            {
+                Lock lock(_latestTextureAndFenceMutex);
+                // If the most recent texture was unused, we can directly recycle it
+                if (_latestTextureAndFence.first) {
+                    offscreenTextures.releaseTexture(_latestTextureAndFence);
+                    _latestTextureAndFence = { 0, 0 };
+                }
             }
             offscreenTextures.releaseSize(_size);
         }

libraries/ui/src/ui/OffscreenQmlSurface.h

@@ -167,6 +167,9 @@ public slots:
     bool handlePointerEvent(const PointerEvent& event, class QTouchDevice& device, bool release = false);
 
 private:
+    using Mutex = std::mutex;
+    using Lock = std::unique_lock<std::mutex>;
+
     QQuickWindow* _quickWindow { nullptr };
     QQmlContext* _qmlContext { nullptr };
     QQuickItem* _rootItem { nullptr };
@@ -188,6 +191,7 @@ private:
 #endif
     
     // Texture management
+    Mutex _latestTextureAndFenceMutex;
     TextureAndFence _latestTextureAndFence { 0, 0 };
 
     bool _render { false };