Qt: Fix transfer window deadlock

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):

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 );

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,18 +137,25 @@ 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
 };
 
 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 ))
 {
 }
 

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 )
 };
 }
 

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;
 }
 
@@ -61,7 +80,10 @@ bool Window::configInit()
     makeCurrent();
     initGLEW();
 
-    if( getIAttribute( WindowSettings::IATTR_HINT_DRAWABLE ) == FBO )
+    const int32_t drawable =
+            getIAttribute( WindowSettings::IATTR_HINT_DRAWABLE );
+    // pbuffer is deprecated in Qt, used FBO instead
+    if( drawable == FBO || drawable == PBUFFER )
         return configInitFBO();
     return true;
 }

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;
 };
 }
 }

eq/qt/windowFactory.cpp

@@ -17,34 +17,18 @@
 
 #include "windowFactory.h"
 
-#include "shareContextWindow.h"
 #include "window.h"
 #include "windowImpl.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* 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 )

eq/qt/windowImpl.h

@@ -33,7 +33,7 @@
 namespace eq
 {
 namespace qt
-{    
+{
 namespace detail
 {
 namespace

eq/qt/windowSystem.cpp

@@ -27,6 +27,7 @@
 #include "pipe.h"
 #include "shareContextWindow.h"
 #include "window.h"
+#include "windowImpl.h"
 #include "windowFactory.h"
 
 #include <eq/client.h>
@@ -65,14 +66,32 @@ 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::OFF )
+    {
+        // QWindow creation/destruction must happen in the app thread;
+        // unblock main thread to give QApplication the change to process the
+        // createImpl signal. Note that even a QOffscreenSurface is backed by a
+        // QWindow on some platforms.
+        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;
 }
 

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 )

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 )
 };
 }