From 7262a10e62cf181e4d36d14d95349545b19c0d4b Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Thu, 3 Dec 2015 15:39:09 -0800 Subject: [PATCH] Refactoring present thread / GL base plugin for saner context management --- .../src/display-plugins/DisplayPlugin.cpp | 22 +++--- .../display-plugins/OpenGLDisplayPlugin.cpp | 74 +++++++------------ .../src/display-plugins/OpenGLDisplayPlugin.h | 2 - .../stereo/InterleavedStereoDisplayPlugin.cpp | 6 +- .../oculus/src/OculusBaseDisplayPlugin.cpp | 2 - plugins/oculus/src/OculusDisplayPlugin.cpp | 5 -- 6 files changed, 42 insertions(+), 69 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/DisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/DisplayPlugin.cpp index 85e832abdd..6c34612e8c 100644 --- a/libraries/display-plugins/src/display-plugins/DisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/DisplayPlugin.cpp @@ -26,17 +26,17 @@ DisplayPluginList getDisplayPlugins() { DisplayPlugin* PLUGIN_POOL[] = { new Basic2DWindowOpenGLDisplayPlugin(), new NullDisplayPlugin(), -//#ifdef DEBUG -//#endif -// -// // Stereo modes -// -// // SBS left/right -// new SideBySideStereoDisplayPlugin(), -// // Interleaved left/right -// new InterleavedStereoDisplayPlugin(), -// -// // HMDs +#ifdef DEBUG +#endif + + // Stereo modes + + // SBS left/right + new SideBySideStereoDisplayPlugin(), + // Interleaved left/right + new InterleavedStereoDisplayPlugin(), + + // HMDs //#ifdef Q_OS_WIN // // SteamVR SDK // new OpenVrDisplayPlugin(), diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index 22adeb8447..45e6daef31 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -28,8 +28,6 @@ class PresentThread : public QThread, public Dependency { using Mutex = std::mutex; using Condition = std::condition_variable; using Lock = std::unique_lock; - - friend class OpenGLDisplayPlugin; public: ~PresentThread() { @@ -42,6 +40,14 @@ public: _newPlugin = plugin; } + void setContext(QGLContext * context) { + // Move the OpenGL context to the present thread + // Extra code because of the widget 'wrapper' context + _context = context; + _context->moveToThread(this); + _context->contextHandle()->moveToThread(this); + } + virtual void run() override { Q_ASSERT(_context); while (!_shutdown) { @@ -50,7 +56,7 @@ public: Lock lock(_mutex); // Move the context to the main thread _context->moveToThread(qApp->thread()); - _widgetContext->moveToThread(qApp->thread()); + _context->contextHandle()->moveToThread(qApp->thread()); _pendingMainThreadOperation = false; // Release the main thread to do it's action _condition.notify_one(); @@ -67,6 +73,7 @@ public: // Check before lock if (_newPlugin != nullptr) { Lock lock(_mutex); + _context->makeCurrent(); // Check if we have a new plugin to activate if (_newPlugin != nullptr) { // Deactivate the old plugin @@ -77,8 +84,8 @@ public: _newPlugin->customizeContext(); _activePlugin = _newPlugin; _newPlugin = nullptr; - _context->doneCurrent(); } + _context->doneCurrent(); lock.unlock(); } @@ -89,12 +96,14 @@ public: } // take the latest texture and present it + _context->makeCurrent(); _activePlugin->present(); - + _context->doneCurrent(); } + _context->doneCurrent(); - _widgetContext->moveToThread(qApp->thread()); _context->moveToThread(qApp->thread()); + _context->contextHandle()->moveToThread(qApp->thread()); } void withMainThreadContext(std::function f) { @@ -104,14 +113,16 @@ public: _finishedMainThreadOperation = false; _condition.wait(lock, [&] { return !_pendingMainThreadOperation; }); - _widgetContext->makeCurrent(); + _context->makeCurrent(); f(); - _widgetContext->doneCurrent(); + _context->doneCurrent(); + + // Move the context back to the presentation thread + _context->moveToThread(this); + _context->contextHandle()->moveToThread(this); // restore control of the context to the presentation thread and signal // the end of the operation - _widgetContext->moveToThread(this); - _context->moveToThread(this); _finishedMainThreadOperation = true; lock.unlock(); _condition.notify_one(); @@ -119,6 +130,9 @@ public: private: + void makeCurrent(); + void doneCurrent(); + bool _shutdown { false }; Mutex _mutex; // Used to allow the main thread to perform context operations @@ -128,8 +142,7 @@ private: QThread* _mainThread { nullptr }; OpenGLDisplayPlugin* _newPlugin { nullptr }; OpenGLDisplayPlugin* _activePlugin { nullptr }; - QOpenGLContext* _context { nullptr }; - QGLContext* _widgetContext { nullptr }; + QGLContext* _context { nullptr }; }; OpenGLDisplayPlugin::OpenGLDisplayPlugin() { @@ -165,16 +178,8 @@ void OpenGLDisplayPlugin::activate() { DependencyManager::set(); presentThread = DependencyManager::get(); presentThread->setObjectName("Presentation Thread"); - auto widget = _container->getPrimaryWidget(); - - // Move the OpenGL context to the present thread - // Extra code because of the widget 'wrapper' context - presentThread->_widgetContext = widget->context(); - presentThread->_widgetContext->moveToThread(presentThread.data()); - presentThread->_context = presentThread->_widgetContext->contextHandle(); - presentThread->_context->moveToThread(presentThread.data()); - + presentThread->setContext(widget->context()); // Start execution presentThread->start(); } @@ -196,9 +201,6 @@ void OpenGLDisplayPlugin::customizeContext() { auto presentThread = DependencyManager::get(); Q_ASSERT(thread() == presentThread->thread()); - bool makeCurrentResult = makeCurrent(); - Q_ASSERT(makeCurrentResult); - // TODO: write the proper code for linux #if defined(Q_OS_WIN) _vsyncSupported = wglewGetExtension("WGL_EXT_swap_control"); @@ -213,15 +215,11 @@ void OpenGLDisplayPlugin::customizeContext() { _program = loadDefaultShader(); _plane = loadPlane(_program); - - doneCurrent(); } void OpenGLDisplayPlugin::uncustomizeContext() { - makeCurrent(); _program.reset(); _plane.reset(); - doneCurrent(); } // Pressing Alt (and Meta) key alone activates the menubar because its style inherits the @@ -310,19 +308,11 @@ void OpenGLDisplayPlugin::internalPresent() { } void OpenGLDisplayPlugin::present() { - auto makeCurrentResult = makeCurrent(); - Q_ASSERT(makeCurrentResult); - if (!makeCurrentResult) { - qDebug() << "Failed to make current"; - return; - } - updateTextures(); if (_currentSceneTexture) { internalPresent(); updateFramerate(); } - doneCurrent(); } float OpenGLDisplayPlugin::presentRate() { @@ -360,18 +350,6 @@ bool OpenGLDisplayPlugin::isVsyncEnabled() { return true; #endif } -bool OpenGLDisplayPlugin::makeCurrent() { - static auto widget = _container->getPrimaryWidget(); - widget->makeCurrent(); - auto result = widget->context()->contextHandle() == QOpenGLContext::currentContext(); - Q_ASSERT(result); - return result; -} - -void OpenGLDisplayPlugin::doneCurrent() { - static auto widget = _container->getPrimaryWidget(); - widget->doneCurrent(); -} void OpenGLDisplayPlugin::swapBuffers() { static auto widget = _container->getPrimaryWidget(); diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h index edbe7db006..809a52ef7f 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h @@ -63,8 +63,6 @@ protected: void updateTextures(); void updateFramerate(); void drawUnitQuad(); - bool makeCurrent(); - void doneCurrent(); void swapBuffers(); // Plugin specific functionality to composite the scene and overlay and present the result virtual void internalPresent(); diff --git a/libraries/display-plugins/src/display-plugins/stereo/InterleavedStereoDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/stereo/InterleavedStereoDisplayPlugin.cpp index 4332c53d81..ffaf005533 100644 --- a/libraries/display-plugins/src/display-plugins/stereo/InterleavedStereoDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/stereo/InterleavedStereoDisplayPlugin.cpp @@ -67,5 +67,9 @@ glm::uvec2 InterleavedStereoDisplayPlugin::getRecommendedRenderSize() const { } void InterleavedStereoDisplayPlugin::internalPresent() { - // FIXME + using namespace oglplus; + _program->Bind(); + auto sceneSize = getRecommendedRenderSize(); + Uniform(*_program, "textureSize").SetValue(sceneSize); + WindowOpenGLDisplayPlugin::internalPresent(); } diff --git a/plugins/oculus/src/OculusBaseDisplayPlugin.cpp b/plugins/oculus/src/OculusBaseDisplayPlugin.cpp index 805ad75e95..b6690ea76e 100644 --- a/plugins/oculus/src/OculusBaseDisplayPlugin.cpp +++ b/plugins/oculus/src/OculusBaseDisplayPlugin.cpp @@ -55,11 +55,9 @@ bool OculusBaseDisplayPlugin::isSupported() const { // DLL based display plugins MUST initialize GLEW inside the DLL code. void OculusBaseDisplayPlugin::customizeContext() { - makeCurrent(); glewExperimental = true; GLenum err = glewInit(); glGetError(); - doneCurrent(); WindowOpenGLDisplayPlugin::customizeContext(); } diff --git a/plugins/oculus/src/OculusDisplayPlugin.cpp b/plugins/oculus/src/OculusDisplayPlugin.cpp index bcb39f5100..21c318677e 100644 --- a/plugins/oculus/src/OculusDisplayPlugin.cpp +++ b/plugins/oculus/src/OculusDisplayPlugin.cpp @@ -154,8 +154,6 @@ void OculusDisplayPlugin::activate() { void OculusDisplayPlugin::customizeContext() { OculusBaseDisplayPlugin::customizeContext(); - bool makeCurrentResult = makeCurrent(); - Q_ASSERT(makeCurrentResult); #if (OVR_MAJOR_VERSION >= 6) _sceneFbo = SwapFboPtr(new SwapFramebufferWrapper(_hmd)); _sceneFbo->Init(getRecommendedRenderSize()); @@ -169,14 +167,11 @@ void OculusDisplayPlugin::customizeContext() { enableVsync(false); // Only enable mirroring if we know vsync is disabled _enablePreview = !isVsyncEnabled(); - doneCurrent(); } void OculusDisplayPlugin::uncustomizeContext() { #if (OVR_MAJOR_VERSION >= 6) - makeCurrent(); _sceneFbo.reset(); - doneCurrent(); #endif OculusBaseDisplayPlugin::uncustomizeContext(); }