From 9893273d33f892f4d11c9423ce486e8d32905d84 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 30 Jan 2018 21:11:48 -0800 Subject: [PATCH 1/2] Trying to fix AMD crash in QML rendering --- libraries/gl/src/gl/Context.cpp | 40 +++++++++++---------- libraries/gl/src/gl/Context.h | 1 + libraries/gl/src/gl/OffscreenGLCanvas.cpp | 24 +++++++++++++ libraries/gl/src/gl/OffscreenGLCanvas.h | 5 ++- libraries/ui/src/ui/OffscreenQmlSurface.cpp | 2 ++ 5 files changed, 52 insertions(+), 20 deletions(-) diff --git a/libraries/gl/src/gl/Context.cpp b/libraries/gl/src/gl/Context.cpp index 8acbada5aa..309839808e 100644 --- a/libraries/gl/src/gl/Context.cpp +++ b/libraries/gl/src/gl/Context.cpp @@ -23,25 +23,31 @@ #include #include #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 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); } diff --git a/libraries/gl/src/gl/Context.h b/libraries/gl/src/gl/Context.h index 4f0747a86f..b6160cbd6c 100644 --- a/libraries/gl/src/gl/Context.h +++ b/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(); diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.cpp b/libraries/gl/src/gl/OffscreenGLCanvas.cpp index 8a476c45ad..380ba085cc 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.cpp +++ b/libraries/gl/src/gl/OffscreenGLCanvas.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #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, []{ diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.h b/libraries/gl/src/gl/OffscreenGLCanvas.h index 583aa7c60f..be0e0d9678 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.h +++ b/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 }; diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.cpp b/libraries/ui/src/ui/OffscreenQmlSurface.cpp index db3df34dc5..b56de957d2 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.cpp +++ b/libraries/ui/src/ui/OffscreenQmlSurface.cpp @@ -543,6 +543,7 @@ void OffscreenQmlSurface::cleanup() { void OffscreenQmlSurface::render() { #if !defined(DISABLE_QML) + if (nsightActive()) { return; } @@ -565,6 +566,7 @@ void OffscreenQmlSurface::render() { glBindTexture(GL_TEXTURE_2D, texture); glGenerateMipmap(GL_TEXTURE_2D); glBindTexture(GL_TEXTURE_2D, 0); + glFinish(); { From ef699c2a78a5848f4c628bded0221193d03c7621 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Wed, 31 Jan 2018 10:38:20 -0800 Subject: [PATCH 2/2] Better fix for AMD crashing (hopefully --- libraries/ui/src/ui/OffscreenQmlSurface.cpp | 40 ++++++++++++++------- libraries/ui/src/ui/OffscreenQmlSurface.h | 4 +++ 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.cpp b/libraries/ui/src/ui/OffscreenQmlSurface.cpp index b56de957d2..6ff0a7d416 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.cpp +++ b/libraries/ui/src/ui/OffscreenQmlSurface.cpp @@ -566,19 +566,22 @@ void OffscreenQmlSurface::render() { glBindTexture(GL_TEXTURE_2D, texture); glGenerateMipmap(GL_TEXTURE_2D); glBindTexture(GL_TEXTURE_2D, 0); - glFinish(); { // 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(); @@ -590,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; } @@ -815,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); } diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.h b/libraries/ui/src/ui/OffscreenQmlSurface.h index 7aeac8d7c3..08c7ca6bf8 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.h +++ b/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; + 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 };