From 7ffa2120069785ba68b04ab9519ee60428117c49 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:04:24 -0700 Subject: [PATCH 1/8] Don't use textures that are in the transfer pipeline --- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 9 +------- libraries/gpu-gl/src/gpu/gl/GLTexture.h | 27 +++++++++++++++-------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index 342b2611d5..818d95d756 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/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()) { - return Idle != syncState; - } - - if (Idle != syncState) { + if (isOutdated() || Idle != syncState) { return false; } diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.h b/libraries/gpu-gl/src/gpu/gl/GLTexture.h index 6ac83d7116..d050afbb59 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.h +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.h @@ -21,6 +21,9 @@ struct GLFilterMode { class GLTexture : public GLObject { 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 _textureTransferHelper; @@ -58,18 +61,24 @@ public: return object; } - if (object->isReady()) { - // 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()) { + 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; From 95b5a23c7abaf56464262a868a4381e06b3338fa Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:04:46 -0700 Subject: [PATCH 2/8] Properly destroy GL programs --- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index a37ac37adb..7c369f4124 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/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 { From 02c79d9714239e525e4486dee834d6d80fc136c1 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:05:29 -0700 Subject: [PATCH 3/8] Fix the OpenVR issues on AMD --- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 42 +++++++++++++--------- plugins/openvr/src/OpenVrDisplayPlugin.h | 2 ++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index 0cb95e5747..4eb371719b 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -56,7 +56,7 @@ public: using Condition = std::condition_variable; using Lock = std::unique_lock; friend class OpenVrDisplayPlugin; - OffscreenGLCanvas _canvas; + std::shared_ptr _canvas; BasicFramebufferWrapperPtr _framebuffer; ProgramPtr _program; ShapeWrapperPtr _plane; @@ -68,9 +68,7 @@ public: OpenVrSubmitThread(OpenVrDisplayPlugin& plugin) : _plugin(plugin) { - _canvas.create(plugin._container->getPrimaryContext()); - _canvas.doneCurrent(); - _canvas.moveToThreadWithContext(this); + setObjectName("OpenVR Submit Thread"); } 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(); _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(*this); - }); - _submitThread->setObjectName("OpenVR Submit Thread"); + _submitThread = std::make_shared(*this); + if (!_submitCanvas) { + withMainThreadContext([&] { + _submitCanvas = std::make_shared(); + _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 } diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.h b/plugins/openvr/src/OpenVrDisplayPlugin.h index ba51511fe8..7b8869ae93 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.h +++ b/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 _submitThread; + std::shared_ptr _submitCanvas; friend class OpenVrSubmitThread; #endif }; From 1652965ac2f4a2092bbe20c8b4c25c7f71502bb6 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:07:44 -0700 Subject: [PATCH 4/8] Destroy contexts properly --- libraries/gl/src/gl/OffscreenGLCanvas.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/gl/src/gl/OffscreenGLCanvas.cpp b/libraries/gl/src/gl/OffscreenGLCanvas.cpp index e7f821a49f..2f8b29325e 100644 --- a/libraries/gl/src/gl/OffscreenGLCanvas.cpp +++ b/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; From e4334be0cf7e7b631e7f5a0044dcc3dc23363c41 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:08:15 -0700 Subject: [PATCH 5/8] Use synchronous logging (may impact performance when debugging is enabled) --- libraries/gl/src/gl/GLHelpers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/gl/src/gl/GLHelpers.cpp b/libraries/gl/src/gl/GLHelpers.cpp index 633c4ef0eb..f464001f60 100644 --- a/libraries/gl/src/gl/GLHelpers.cpp +++ b/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); } } \ No newline at end of file From 0213dd0359d3b2767917e458ca674d46e701b26b Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:08:50 -0700 Subject: [PATCH 6/8] Don't bottleneck the render-perf tool on new frame creation --- tests/render-perf/src/main.cpp | 73 +++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 11 deletions(-) diff --git a/tests/render-perf/src/main.cpp b/tests/render-perf/src/main.cpp index eec7e994ae..f7e6b6a6d2 100644 --- a/tests/render-perf/src/main.cpp +++ b/tests/render-perf/src/main.cpp @@ -172,8 +172,8 @@ void main(void) { extern QThread* RENDER_THREAD; -class RenderThread : public GenericQueueThread { - using Parent = GenericQueueThread; +class RenderThread : public GenericThread { + using Parent = GenericThread; public: QOpenGLContextWrapper* _displayContext{ nullptr }; QSurface* _displaySurface{ nullptr }; @@ -187,8 +187,15 @@ public: std::shared_ptr _backend; std::vector _frameTimes; size_t _frameIndex; + std::mutex _frameLock; + std::queue _pendingFrames; + gpu::FramePointer _activeFrame; static const size_t FRAME_TIME_BUFFER_SIZE{ 8192 }; + void submitFrame(const gpu::FramePointer& frame) { + std::unique_lock lock(_frameLock); + _pendingFrames.push(frame); + } void initialize(QOpenGLContextWrapper* displayContext, QWindow* surface) { @@ -203,6 +210,7 @@ public: _displayContext->makeCurrent(_displaySurface); DependencyManager::get()->init(); _displayContext->doneCurrent(); + std::unique_lock 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 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); } - } - { - //_textOverlay->render(); + (void)CHECK_GL_ERROR(); } _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 pendingFrames; + { + std::unique_lock 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", _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()->releaseFramebuffer(framebuffer); }; - _renderThread.queueItem(frame); + _renderThread.submitFrame(frame); if (!_renderThread.isThreaded()) { _renderThread.process(); } From 587d015c57450d8fbbc0dbffe63449cd9f472b05 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 26 Aug 2016 19:09:26 -0700 Subject: [PATCH 7/8] Give names to offscreen contexts for easier debugging --- interface/src/Application.cpp | 2 ++ libraries/gl/src/gl/OffscreenQmlSurface.cpp | 1 + libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp | 1 + 3 files changed, 4 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index b2095dbdfe..aaf38e826b 100644 --- a/interface/src/Application.cpp +++ b/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(); diff --git a/libraries/gl/src/gl/OffscreenQmlSurface.cpp b/libraries/gl/src/gl/OffscreenQmlSurface.cpp index c47312f9f6..7973ed4b4f 100644 --- a/libraries/gl/src/gl/OffscreenQmlSurface.cpp +++ b/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"); diff --git a/libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp b/libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp index 7acb736063..766c134b16 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTextureTransfer.cpp @@ -21,6 +21,7 @@ using namespace gpu::gl; GLTextureTransferHelper::GLTextureTransferHelper() { #ifdef THREADED_TEXTURE_TRANSFER _canvas = QSharedPointer(new OffscreenGLCanvas(), &QObject::deleteLater); + _canvas->setObjectName("TextureTransferCanvas"); _canvas->create(QOpenGLContextWrapper::currentContext()); if (!_canvas->makeCurrent()) { qFatal("Unable to create texture transfer context"); From 92fa1d57769965d7f36794b587bc6ede645c348e Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Sat, 27 Aug 2016 15:08:38 -0700 Subject: [PATCH 8/8] Use stream draw for object position updates --- libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp b/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp index 314e7d79be..ace0e73cf9 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45BackendTransform.cpp +++ b/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()) {