Merge pull request #8531 from jherico/gl_fixes

(AMD) OpenGL Fixes

interface/src/Application.cpp

@@ -1483,6 +1483,7 @@ void Application::initializeGL() {
 
     _glWidget->makeCurrent();
     _chromiumShareContext = new OffscreenGLCanvas();
+    _chromiumShareContext->setObjectName("ChromiumShareContext");
     _chromiumShareContext->create(_glWidget->context()->contextHandle());
     _chromiumShareContext->makeCurrent();
     qt_gl_set_global_share_context(_chromiumShareContext->getContext());
@@ -1529,6 +1530,7 @@ void Application::initializeGL() {
     _idleLoopStdev.reset();
 
     _offscreenContext = new OffscreenGLCanvas();
+    _offscreenContext->setObjectName("MainThreadContext");
     _offscreenContext->create(_glWidget->context()->contextHandle());
     _offscreenContext->makeCurrent();
 

libraries/gl/src/gl/GLHelpers.cpp

@@ -102,5 +102,6 @@ void GLDebug::setupLogger(QObject* window) {
         QObject::connect(logger, &QOpenGLDebugLogger::messageLogged, window, [&](const QOpenGLDebugMessage & debugMessage) {
             GLDebug::log(debugMessage);
         });
+        logger->startLogging(QOpenGLDebugLogger::SynchronousLogging);
     }
 }

libraries/gl/src/gl/OffscreenGLCanvas.cpp

@@ -23,7 +23,8 @@ OffscreenGLCanvas::OffscreenGLCanvas() : _context(new QOpenGLContext), _offscree
 }
 
 OffscreenGLCanvas::~OffscreenGLCanvas() {
-    _context->doneCurrent();
+    // A context with logging enabled needs to be current when it's destroyed
+    _context->makeCurrent(_offscreenSurface);
     delete _context;
     _context = nullptr;
 

libraries/gl/src/gl/OffscreenQmlSurface.cpp

@@ -195,6 +195,7 @@ QEvent* OffscreenQmlRenderThread::Queue::take() {
 }
 
 OffscreenQmlRenderThread::OffscreenQmlRenderThread(OffscreenQmlSurface* surface, QOpenGLContext* shareContext) : _surface(surface) {
+    _canvas.setObjectName("OffscreenQmlRenderCanvas");
     qDebug() << "Building QML Renderer";
     if (!_canvas.create(shareContext)) {
         qWarning("Failed to create OffscreenGLCanvas");

libraries/gpu-gl/src/gpu/gl/GLBackend.cpp

@@ -582,7 +582,7 @@ void GLBackend::releaseShader(GLuint id) const {
 
 void GLBackend::releaseProgram(GLuint id) const {
     Lock lock(_trashMutex);
-    _shadersTrash.push_back(id);
+    _programsTrash.push_back(id);
 }
 
 void GLBackend::releaseQuery(GLuint id) const {

libraries/gpu-gl/src/gpu/gl/GLTexture.cpp

@@ -243,15 +243,8 @@ bool GLTexture::isReady() const {
         return false;
     }
 
-    // If we're out of date, but the transfer is in progress, report ready
-    // as a special case
     auto syncState = _syncState.load();
-
+    if (isOutdated() || Idle != syncState) {
-    if (isOutdated()) {
-        return Idle != syncState;
-    }
-
-    if (Idle != syncState) {
         return false;
     }
 

libraries/gpu-gl/src/gpu/gl/GLTexture.h

@@ -21,6 +21,9 @@ struct GLFilterMode {
 
 class GLTexture : public GLObject<Texture> {
 public:
+    static const uint16_t INVALID_MIP { (uint16_t)-1 };
+    static const uint8_t INVALID_FACE { (uint8_t)-1 };
+
     static void initTextureTransferHelper();
     static std::shared_ptr<GLTextureTransferHelper> _textureTransferHelper;
 
@@ -58,18 +61,24 @@ public:
             return object;
         }
 
-        if (object->isReady()) {
+        if (object->isOutdated()) {
-            // Do we need to reduce texture memory usage?
-            if (object->isOverMaxMemory() && texturePointer->incremementMinMip()) {
-                // WARNING, this code path will essentially `delete this`, 
-                // so no dereferencing of this instance should be done past this point
-                object = new GLTextureType(backend.shared_from_this(), texture, object);
-                _textureTransferHelper->transferTexture(texturePointer);
-            }
-        } else if (object->isOutdated()) {
             // Object might be outdated, if so, start the transfer
             // (outdated objects that are already in transfer will have reported 'true' for ready()
             _textureTransferHelper->transferTexture(texturePointer);
+            return nullptr;
+        }
+
+        if (!object->isReady()) {
+            return nullptr;
+        }
+
+        // Do we need to reduce texture memory usage?
+        if (object->isOverMaxMemory() && texturePointer->incremementMinMip()) {
+            // WARNING, this code path will essentially `delete this`, 
+            // so no dereferencing of this instance should be done past this point
+            object = new GLTextureType(backend.shared_from_this(), texture, object);
+            _textureTransferHelper->transferTexture(texturePointer);
+            return nullptr;
         }
 
         return object;

libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp

@@ -21,6 +21,7 @@ using namespace gpu::gl;
 GLTextureTransferHelper::GLTextureTransferHelper() {
 #ifdef THREADED_TEXTURE_TRANSFER
     _canvas = QSharedPointer<OffscreenGLCanvas>(new OffscreenGLCanvas(), &QObject::deleteLater);
+    _canvas->setObjectName("TextureTransferCanvas");
     _canvas->create(QOpenGLContextWrapper::currentContext());
     if (!_canvas->makeCurrent()) {
         qFatal("Unable to create texture transfer context");

libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp

@@ -38,7 +38,7 @@ void GL45Backend::transferTransformState(const Batch& batch) const {
     }
 
     if (!batch._objects.empty()) {
-        glNamedBufferData(_transform._objectBuffer, batch._objects.size() * sizeof(Batch::TransformObject), batch._objects.data(), GL_DYNAMIC_DRAW);
+        glNamedBufferData(_transform._objectBuffer, batch._objects.size() * sizeof(Batch::TransformObject), batch._objects.data(), GL_STREAM_DRAW);
     }
 
     if (!batch._namedData.empty()) {

plugins/openvr/src/OpenVrDisplayPlugin.cpp

@@ -56,7 +56,7 @@ public:
     using Condition = std::condition_variable;
     using Lock = std::unique_lock<Mutex>;
     friend class OpenVrDisplayPlugin;
-    OffscreenGLCanvas _canvas;
+    std::shared_ptr<OffscreenGLCanvas> _canvas;
     BasicFramebufferWrapperPtr _framebuffer;
     ProgramPtr _program;
     ShapeWrapperPtr _plane;
@@ -68,9 +68,7 @@ public:
 
 
     OpenVrSubmitThread(OpenVrDisplayPlugin& plugin) : _plugin(plugin) { 
-        _canvas.create(plugin._container->getPrimaryContext());
+        setObjectName("OpenVR Submit Thread");
-        _canvas.doneCurrent();
-        _canvas.moveToThreadWithContext(this);
     }
 
     void updateReprojectionProgram() {
@@ -131,19 +129,20 @@ public:
 
     void run() override {
         QThread::currentThread()->setPriority(QThread::Priority::TimeCriticalPriority);
-        _canvas.makeCurrent();
+        assert(_canvas->thread() == QThread::currentThread());
+        _canvas->makeCurrent();
         glDisable(GL_DEPTH_TEST);
         glViewport(0, 0, _plugin._renderTargetSize.x, _plugin._renderTargetSize.y);
         _framebuffer = std::make_shared<BasicFramebufferWrapper>();
         _framebuffer->Init(_plugin._renderTargetSize);
         updateReprojectionProgram();
         _plane = loadPlane(_program);
-        _canvas.doneCurrent();
+        _canvas->doneCurrent();
         while (!_quit) {
-            _canvas.makeCurrent();
+            _canvas->makeCurrent();
             updateSource();
             if (!_current.texture) {
-                _canvas.doneCurrent();
+                _canvas->doneCurrent();
                 QThread::usleep(1);
                 continue;
             }
@@ -199,15 +198,15 @@ public:
                     _presented.notify_one();
                 });
             }
-            _canvas.doneCurrent();
+            _canvas->doneCurrent();
         }
 
-        _canvas.makeCurrent();
+        _canvas->makeCurrent();
         _plane.reset();
         _program.reset();
         _framebuffer.reset();
-        _canvas.doneCurrent();
+        _canvas->doneCurrent();
-
+        _canvas->moveToThreadWithContext(qApp->thread());
     }
 
     void update(const CompositeInfo& newCompositeInfo) {
@@ -307,10 +306,17 @@ bool OpenVrDisplayPlugin::internalActivate() {
     }
 
 #if OPENVR_THREADED_SUBMIT
-    withMainThreadContext([&] {
+    _submitThread = std::make_shared<OpenVrSubmitThread>(*this);
-        _submitThread = std::make_shared<OpenVrSubmitThread>(*this);
+    if (!_submitCanvas) {
-    });
+        withMainThreadContext([&] {
-    _submitThread->setObjectName("OpenVR Submit Thread");
+            _submitCanvas = std::make_shared<OffscreenGLCanvas>();
+            _submitCanvas->setObjectName("OpenVRSubmitContext");
+            _submitCanvas->create(_container->getPrimaryContext());
+            _submitCanvas->doneCurrent();
+        });
+    }
+    _submitCanvas->moveToThreadWithContext(_submitThread.get());
+    assert(_submitCanvas->thread() == _submitThread.get());
 #endif
 
     return Parent::internalActivate();
@@ -348,6 +354,8 @@ void OpenVrDisplayPlugin::customizeContext() {
         }
         _compositeInfos[i].textureID = getGLBackend()->getTextureID(_compositeInfos[i].texture, false);
     }
+    assert(_submitCanvas->thread() == _submitThread.get());
+    _submitThread->_canvas = _submitCanvas;
     _submitThread->start(QThread::HighPriority);
 #endif
 }
@@ -358,6 +366,8 @@ void OpenVrDisplayPlugin::uncustomizeContext() {
 #if OPENVR_THREADED_SUBMIT
     _submitThread->_quit = true;
     _submitThread->wait();
+    _submitThread.reset();
+    assert(_submitCanvas->thread() == qApp->thread());
 #endif
 }
 

plugins/openvr/src/OpenVrDisplayPlugin.h

@@ -19,6 +19,7 @@ const float TARGET_RATE_OpenVr = 90.0f;  // FIXME: get from sdk tracked device p
 
 #if OPENVR_THREADED_SUBMIT
 class OpenVrSubmitThread;
+class OffscreenGLCanvas;
 static const size_t COMPOSITING_BUFFER_SIZE = 3;
 
 struct CompositeInfo {
@@ -78,6 +79,7 @@ private:
     CompositeInfo::Array _compositeInfos;
     size_t _renderingIndex { 0 };
     std::shared_ptr<OpenVrSubmitThread> _submitThread;
+    std::shared_ptr<OffscreenGLCanvas> _submitCanvas;
     friend class OpenVrSubmitThread;
 #endif
 };

tests/render-perf/src/main.cpp

@@ -172,8 +172,8 @@ void main(void) {
 
 extern QThread* RENDER_THREAD;
 
-class RenderThread : public GenericQueueThread<gpu::FramePointer> {
+class RenderThread : public GenericThread {
-    using Parent = GenericQueueThread<gpu::FramePointer>;
+    using Parent = GenericThread;
 public:
     QOpenGLContextWrapper* _displayContext{ nullptr };
     QSurface* _displaySurface{ nullptr };
@@ -187,8 +187,15 @@ public:
     std::shared_ptr<gpu::Backend> _backend;
     std::vector<uint64_t> _frameTimes;
     size_t _frameIndex;
+    std::mutex _frameLock;
+    std::queue<gpu::FramePointer> _pendingFrames;
+    gpu::FramePointer _activeFrame;
     static const size_t FRAME_TIME_BUFFER_SIZE{ 8192 };
 
+    void submitFrame(const gpu::FramePointer& frame) {
+        std::unique_lock<std::mutex> lock(_frameLock);
+        _pendingFrames.push(frame);
+    }
 
 
     void initialize(QOpenGLContextWrapper* displayContext, QWindow* surface) {
@@ -203,6 +210,7 @@ public:
         _displayContext->makeCurrent(_displaySurface);
         DependencyManager::get<DeferredLightingEffect>()->init();
         _displayContext->doneCurrent();
+        std::unique_lock<std::mutex> lock(_mutex);
         Parent::initialize();
         if (isThreaded()) {
             _displayContext->moveToThread(thread());
@@ -211,6 +219,11 @@ public:
 
     void setup() override {
         RENDER_THREAD = QThread::currentThread();
+        QThread::currentThread()->setPriority(QThread::HighestPriority);
+        // Wait until the context has been moved to this thread
+        {
+            std::unique_lock<std::mutex> lock(_mutex);
+        }
         _displayContext->makeCurrent(_displaySurface);
         glewExperimental = true;
         glewInit();
@@ -229,12 +242,21 @@ public:
 
         //_textOverlay = new TextOverlay(glm::uvec2(800, 600));
         glViewport(0, 0, 800, 600);
+        (void)CHECK_GL_ERROR();
         _elapsed.start();
     }
 
     void shutdown() override {
+        _activeFrame.reset();
+        while (!_pendingFrames.empty()) {
+            _gpuContext->consumeFrameUpdates(_pendingFrames.front());
+            _pendingFrames.pop();
+        }
         _presentPipeline.reset();
         _gpuContext.reset();
+        if (isThreaded()) {
+            _displayContext->moveToThread(qApp->thread());
+        }
     }
 
     void renderFrame(gpu::FramePointer& frame) {
@@ -256,9 +278,7 @@ public:
                 presentBatch.draw(gpu::TRIANGLE_STRIP, 4);
                 _gpuContext->executeBatch(presentBatch);
             }
-        }
+            (void)CHECK_GL_ERROR();
-        {
-            //_textOverlay->render();
         }
         _displayContext->swapBuffers(_displaySurface);
         _fpsCounter.increment();
@@ -269,6 +289,8 @@ public:
             _frameCount = 0;
             _elapsed.restart();
         }
+        (void)CHECK_GL_ERROR();
+        _displayContext->doneCurrent();
     }
 
     void report() {
@@ -292,10 +314,30 @@ public:
         }
     }
 
-    bool processQueueItems(const Queue& items) override {
+
-        for (auto frame : items) {
+    bool process() override {
+        std::queue<gpu::FramePointer> pendingFrames;
+        {
+            std::unique_lock<std::mutex> lock(_frameLock);
+            pendingFrames.swap(_pendingFrames);
+        }
+
+        while (!pendingFrames.empty()) {
+            _activeFrame = pendingFrames.front();
+            if (_activeFrame) {
+                _gpuContext->consumeFrameUpdates(_activeFrame);
+            }
+            pendingFrames.pop();
+        }
+
+        if (!_activeFrame) {
+            QThread::msleep(1);
+            return true;
+        }
+
+        {
             auto start = usecTimestampNow();
-            renderFrame(frame);
+            renderFrame(_activeFrame);
             auto duration = usecTimestampNow() - start;
             auto frameBufferIndex = _frameIndex % FRAME_TIME_BUFFER_SIZE;
             _frameTimes[frameBufferIndex] = duration;
@@ -416,6 +458,7 @@ public:
     }
 
     QTestWindow() {
+        installEventFilter(this);
         _camera.movementSpeed = 50.0f;
         QThreadPool::globalInstance()->setMaxThreadCount(2);
         QThread::currentThread()->setPriority(QThread::HighestPriority);
@@ -457,7 +500,7 @@ public:
         _renderThread.initialize(&_context, this);
         // FIXME use a wait condition
         QThread::msleep(1000);
-        _renderThread.queueItem(gpu::FramePointer());
+        _renderThread.submitFrame(gpu::FramePointer());
         _initContext.makeCurrent();
         // Render engine init
         _renderEngine->addJob<RenderShadowTask>("RenderShadowTask", _cullFunctor);
@@ -479,7 +522,6 @@ public:
     }
 
     virtual ~QTestWindow() {
-        _renderThread.terminate();
         getEntities()->shutdown(); // tell the entities system we're shutting down, so it will stop running scripts
         _renderEngine.reset();
         _main3DScene.reset();
@@ -497,6 +539,15 @@ public:
     }
 
 protected:
+
+    bool eventFilter(QObject *obj, QEvent *event) override {
+        if (event->type() == QEvent::Close) {
+            _renderThread.terminate();
+        }
+
+        return QWindow::eventFilter(obj, event);
+    }
+
     void keyPressEvent(QKeyEvent* event) override {
         switch (event->key()) {
         case Qt::Key_F1:
@@ -742,7 +793,7 @@ private:
         frame->framebufferRecycler = [](const gpu::FramebufferPointer& framebuffer){ 
             DependencyManager::get<FramebufferCache>()->releaseFramebuffer(framebuffer);
         };
-        _renderThread.queueItem(frame);
+        _renderThread.submitFrame(frame);
         if (!_renderThread.isThreaded()) {
             _renderThread.process();
         }