From 382366bf61ab558be4a5e3eabd40b88da7ebac93 Mon Sep 17 00:00:00 2001 From: Daniel Nachbaur Date: Tue, 4 Aug 2015 15:35:33 +0200 Subject: [PATCH] Qt: Fix transfer window deadlock --- doc/Changelog.md | 2 ++ eq/channel.cpp | 3 +-- eq/pipe.cpp | 16 ++++++++++++---- eq/pipe.h | 3 ++- eq/qt/window.cpp | 27 +++++++++++++++++++++++---- eq/qt/window.h | 4 ++-- eq/qt/windowFactory.cpp | 18 +----------------- eq/qt/windowImpl.h | 2 +- eq/qt/windowSystem.cpp | 32 +++++++++++++++++++++++++------- eq/window.cpp | 13 ++----------- eq/window.h | 6 +----- 11 files changed, 72 insertions(+), 54 deletions(-) diff --git a/doc/Changelog.md b/doc/Changelog.md index a5e1f2b7b6..51d0256f31 100644 --- a/doc/Changelog.md +++ b/doc/Changelog.md @@ -3,6 +3,8 @@ Changelog {#Changelog} # git master {#master} +* [484](https://github.com/Eyescale/Equalizer/pull/484): + Fix transfer window deadlock with Qt5 * [481](https://github.com/Eyescale/Equalizer/pull/481): Fix Config::getNextEvent() with definite timeout * [467](https://github.com/Eyescale/Equalizer/issues/467): diff --git a/eq/channel.cpp b/eq/channel.cpp index 71f585a06a..0768128f19 100644 --- a/eq/channel.cpp +++ b/eq/channel.cpp @@ -1189,7 +1189,6 @@ bool Channel::_asyncFinishReadback( const std::vector< size_t >& imagePos, if( images[j]->hasAsyncReadback( )) // finish async readback { LBCHECK( getPipe()->startTransferThread( )); - LBCHECK( getWindow()->createTransferWindow( )); hasAsyncReadback = true; _refFrame( frameNumber ); @@ -1734,7 +1733,7 @@ bool Channel::_cmdFinishReadback( co::ICommand& cmd ) command.read< std::vector< uint128_t > >(); const co::NodeIDs& netNodes = command.read< co::NodeIDs >(); - getWindow()->makeCurrentTransfer(); + LBCHECK( getWindow()->createTransferWindow( )); _finishReadback( frameData, imageIndex, frameNumber, taskID, nodes, netNodes ); _unrefFrame( frameNumber ); diff --git a/eq/pipe.cpp b/eq/pipe.cpp index c760834423..3d027c5e84 100644 --- a/eq/pipe.cpp +++ b/eq/pipe.cpp @@ -121,8 +121,9 @@ class RenderThread : public eq::Worker class TransferThread : public co::Worker { public: - explicit TransferThread( const uint32_t index ) + explicit TransferThread( const eq::Pipe* pipe, const uint32_t index ) : co::Worker( co::Global::getCommandQueueLimit( )) + , _pipe( pipe ) , _index( index ) , _stop( false ) {} @@ -136,10 +137,17 @@ class TransferThread : public co::Worker return true; } + void run() override + { + LB_TS_THREAD( _pipe->_tferThread ); + co::Worker::run(); + } + bool stopRunning() override { return _stop; } void postStop() { _stop = true; } private: + const eq::Pipe* _pipe; uint32_t _index; bool _stop; // thread will exit if this is true }; @@ -147,7 +155,7 @@ class TransferThread : public co::Worker class Pipe { public: - explicit Pipe( const uint32_t index ) + Pipe( const eq::Pipe* parent, const uint32_t index ) : systemPipe( 0 ) #ifdef AGL , windowSystem( "AGL" ) @@ -162,7 +170,7 @@ class Pipe , currentFrame( 0 ) , frameTime( 0 ) , thread( 0 ) - , transferThread( index ) + , transferThread( parent, index ) , computeContext( 0 ) {} @@ -240,7 +248,7 @@ void RenderThread::run() Pipe::Pipe( Node* parent ) : Super( parent ) - , _impl( new detail::Pipe( getPath().pipeIndex )) + , _impl( new detail::Pipe( this, getPath().pipeIndex )) { } diff --git a/eq/pipe.h b/eq/pipe.h index 1fbc5fa616..41cd35f1be 100644 --- a/eq/pipe.h +++ b/eq/pipe.h @@ -411,7 +411,8 @@ class Pipe : public fabric::Pipe< Node, Pipe, eq::Window, PipeVisitor > bool _cmdDetachView( co::ICommand& command ); bool _cmdExitTransferThread( co::ICommand& command ); - LB_TS_VAR( _pipeThread ); + LB_TS_VAR( _pipeThread ) + LB_TS_VAR( _tferThread ) }; } diff --git a/eq/qt/window.cpp b/eq/qt/window.cpp index b66af9abe1..8cc8b8f496 100644 --- a/eq/qt/window.cpp +++ b/eq/qt/window.cpp @@ -21,23 +21,42 @@ #include "windowImpl.h" #include "windowEvent.h" +#include "shareContextWindow.h" + namespace eq { namespace qt { +namespace +{ +QOpenGLContext* _getShareContext( const WindowSettings& settings ) +{ + const SystemWindow* shareWindow = settings.getSharedContextWindow(); + const Window* window = dynamic_cast< const Window* >( shareWindow ); + if( window ) + // This only works if configInit has already been called in the window + return window->getContext(); + + const ShareContextWindow* dummyWindow = + dynamic_cast< const ShareContextWindow* >( shareWindow ); + return dummyWindow ? dummyWindow->getContext() : 0; +} +} detail::Window* Window::createImpl( const WindowSettings& settings, - QOpenGLContext* sharedContext, QThread* thread ) { + QOpenGLContext* shareContext = _getShareContext( settings ); + const int32_t drawable = getAttribute( IATTR_HINT_DRAWABLE ); detail::Window* window = 0; if( drawable == eq::WINDOW ) - window = new detail::QWindowWrapper( settings, sharedContext ); + window = new detail::QWindowWrapper( settings, shareContext ); else - window = new detail::QOffscreenSurfaceWrapper( settings, sharedContext); + window = new detail::QOffscreenSurfaceWrapper( settings, shareContext ); - window->getContext()->moveToThread( thread ); + if( thread ) + window->getContext()->moveToThread( thread ); return window; } diff --git a/eq/qt/window.h b/eq/qt/window.h index ca6ca67d89..6a580cccdf 100644 --- a/eq/qt/window.h +++ b/eq/qt/window.h @@ -135,9 +135,9 @@ class Window : public QObject, public WindowIF private: detail::Window* const _impl; - static detail::Window* createImpl( const WindowSettings&, QOpenGLContext*, - QThread* ); + static detail::Window* createImpl( const WindowSettings&, QThread* ); friend class WindowFactory; + friend class WindowSystem; }; } } diff --git a/eq/qt/windowFactory.cpp b/eq/qt/windowFactory.cpp index a3be1445d0..cfef7ce96e 100644 --- a/eq/qt/windowFactory.cpp +++ b/eq/qt/windowFactory.cpp @@ -17,7 +17,6 @@ #include "windowFactory.h" -#include "shareContextWindow.h" #include "window.h" #include "windowImpl.h" @@ -25,26 +24,11 @@ namespace eq { namespace qt { -namespace -{ -QOpenGLContext* _getShareContext( const WindowSettings& settings ) -{ - const SystemWindow* shareWindow = settings.getSharedContextWindow(); - const Window* window = dynamic_cast< const Window* >( shareWindow ); - if( window ) - // This only works if configInit has already been called in the window - return window->getContext(); - - const ShareContextWindow* dummyWindow = - dynamic_cast< const ShareContextWindow* >( shareWindow ); - return dummyWindow ? dummyWindow->getContext() : 0; -} -} detail::Window* WindowFactory::onCreateImpl( const WindowSettings& settings, QThread* thread_ ) { - return Window::createImpl( settings, _getShareContext( settings ), thread_); + return Window::createImpl( settings, thread_ ); } void WindowFactory::onDestroyImpl( detail::Window* window ) diff --git a/eq/qt/windowImpl.h b/eq/qt/windowImpl.h index 9c866ed811..2e2b71b0f7 100644 --- a/eq/qt/windowImpl.h +++ b/eq/qt/windowImpl.h @@ -33,7 +33,7 @@ namespace eq { namespace qt -{ +{ namespace detail { namespace diff --git a/eq/qt/windowSystem.cpp b/eq/qt/windowSystem.cpp index 69f174411c..07dd72b91b 100644 --- a/eq/qt/windowSystem.cpp +++ b/eq/qt/windowSystem.cpp @@ -27,6 +27,7 @@ #include "pipe.h" #include "shareContextWindow.h" #include "window.h" +#include "windowImpl.h" #include "windowFactory.h" #include @@ -65,14 +66,31 @@ eq::SystemWindow* WindowSystem::createWindow( eq::Window* window, const WindowSettings& settings ) { LBDEBUG << "Using qt::Window" << std::endl; - window->getClient()->interruptMainThread(); - qt::detail::Window* impl = createImpl( settings, QThread::currentThread( )); - Window* qtWindow = new Window( *window, settings, impl ); - - QCoreApplication* app = QApplication::instance(); - app->connect( qtWindow, SIGNAL( destroyImpl( detail::Window* )), - _factory, SLOT( onDestroyImpl( detail::Window* ))); + if( settings.getIAttribute( + WindowSettings::IATTR_HINT_DRAWABLE ) == eq::WINDOW ) + { + // QWindow creation/destruction must happen in the app thread; + // unblock main thread to give QApplication the change to process the + // createImpl signal. + window->getClient()->interruptMainThread(); + qt::detail::Window* impl = createImpl( settings, + QThread::currentThread( )); + Window* qtWindow = new Window( *window, settings, impl ); + qtWindow->connect( qtWindow, SIGNAL( destroyImpl( detail::Window* )), + _factory, SLOT( onDestroyImpl( detail::Window* ))); + return qtWindow; + } + + // Offscreen surface can be created in the current thread. In the case + // of the transfer window, it MUST be created w/o using the signal/ + // thread dispatch as the main thread is potentially blocking for + // frame assembly, hence QApplication is not served. + qt::detail::Window* impl = Window::createImpl( settings, + 0 /*don't move to other thread*/ ); + Window* qtWindow = new Window( *window, settings, impl ); + qtWindow->connect( qtWindow, &Window::destroyImpl, + [&]( detail::Window* impl_ ) { delete impl_; }); return qtWindow; } diff --git a/eq/window.cpp b/eq/window.cpp index 8adb9b6ca9..0fde8c1d6d 100644 --- a/eq/window.cpp +++ b/eq/window.cpp @@ -499,6 +499,7 @@ bool Window::configInitGL( const uint128_t& ) bool Window::createTransferWindow() { + LB_TS_THREAD( _tferThread ); LBASSERT( _systemWindow ); if( _transferWindow ) @@ -522,14 +523,12 @@ bool Window::createTransferWindow() _transferWindow = 0; } else - makeCurrentTransfer(); // #177 + _transferWindow->makeCurrent(); // #177 } else LBERROR << "Window system " << pipe->getWindowSystem() << " not implemented or supported" << std::endl; - makeCurrent(); - LBVERB << "Transfer window initialization finished" << std::endl; return _transferWindow != 0; } @@ -542,14 +541,6 @@ const GLEWContext* Window::getTransferGlewContext() return 0; } -void Window::makeCurrentTransfer( const bool useCache ) const -{ - LBASSERT( _transferWindow ); - if( _transferWindow ) - _transferWindow->makeCurrent( useCache ); -} - - void Window::deleteTransferSystemWindow() { if( !_transferWindow ) diff --git a/eq/window.h b/eq/window.h index ec011ba8d9..4f38ce5d6c 100644 --- a/eq/window.h +++ b/eq/window.h @@ -233,11 +233,6 @@ class Window : public fabric::Window< Pipe, Window, Channel, WindowSettings >, */ EQ_API virtual void makeCurrent( const bool cache = true ) const; - /** @internal - * Make the shared transfer window's drawable and context current. - */ - void makeCurrentTransfer( const bool cache = true ) const; - /** @internal Bind the window's FBO, if it uses one. */ EQ_API virtual void bindFrameBuffer() const; @@ -510,6 +505,7 @@ class Window : public fabric::Window< Pipe, Window, Channel, WindowSettings >, bool _cmdFrameDrawFinish( co::ICommand& command ); LB_TS_VAR( _pipeThread ) + LB_TS_VAR( _tferThread ) }; }