Fix transfer window deadlock with Qt, transfer window related cleanups

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
 * [484](https://github.com/Eyescale/Equalizer/pull/484):
   Implement Window::doneCurrent() to make no OpenGL context current in the
   current thread

eq/channel.cpp

@@ -19,6 +19,12 @@
 
 #include "channel.h"
 
+// must be included before any header defining Bool
+#ifdef EQ_QT_USED
+#  include "qt/window.h"
+#  include <QThread>
+#endif
+
 #include "channelStatistics.h"
 #include "client.h"
 #include "compositor.h"
@@ -137,8 +143,11 @@ void Channel::attach( const uint128_t& id, const uint32_t instanceID )
                      CmdFunc( this, &Channel::_cmdFrameTiles ), queue );
     registerCommand( fabric::CMD_CHANNEL_FINISH_READBACK,
                      CmdFunc( this, &Channel::_cmdFinishReadback ), transferQ );
-    registerCommand( fabric::CMD_CHANNEL_DELETE_TRANSFER_CONTEXT,
-                     CmdFunc( this,&Channel::_cmdDeleteTransferContext ),
+    registerCommand( fabric::CMD_CHANNEL_CREATE_TRANSFER_WINDOW,
+                     CmdFunc( this,&Channel::_cmdCreateTransferWindow ),
+                     queue );
+    registerCommand( fabric::CMD_CHANNEL_DELETE_TRANSFER_WINDOW,
+                     CmdFunc( this,&Channel::_cmdDeleteTransferWindow ),
                      transferQ );
 }
 
@@ -291,9 +300,10 @@ void Channel::addStatistic( Event& event )
 //---------------------------------------------------------------------------
 // operations
 //---------------------------------------------------------------------------
-void Channel::waitFrameFinished( const uint32_t frame ) const
+bool Channel::waitFrameFinished( const uint32_t frame,
+                                 const uint32_t timeout ) const
 {
-    _impl->finishedFrame.waitGE( frame );
+    return _impl->finishedFrame.timedWaitGE( frame, timeout );
 }
 
 void Channel::frameClear( const uint128_t& )
@@ -1189,7 +1199,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 );
@@ -1507,15 +1516,38 @@ void Channel::_setReady( FrameDataPtr frame, detail::RBStat* stat,
     }
 }
 
-void Channel::_deleteTransferContext()
+void Channel::_createTransferWindow()
+{
+    const SystemWindow* transferWindow = getWindow()->getTransferWindow();
+    if( !transferWindow )
+    {
+        // transfer window creation must happen in pipe thread (#177), but the
+        // context is used in the transfer thread and Qt requires moving the
+        // object to that thread.
+        co::LocalNodePtr localNode = getLocalNode();
+
+        void* tferThread = 0;
+#ifdef EQ_QT_USED
+        tferThread = QThread::currentThread();
+#endif
+        lunchbox::Request< void > request =
+                localNode->registerRequest< void >( tferThread );
+        send( localNode, fabric::CMD_CHANNEL_CREATE_TRANSFER_WINDOW ) <<request;
+        request.wait();
+        transferWindow = getWindow()->getTransferWindow();
+    }
+    transferWindow->makeCurrent();
+}
+
+void Channel::_deleteTransferWindow()
 {
     if( !getPipe()->hasTransferThread( ))
         return;
 
     co::LocalNodePtr localNode = getLocalNode();
     const lunchbox::Request< void >& request =
         localNode->registerRequest< void >();
-    send( localNode, fabric::CMD_CHANNEL_DELETE_TRANSFER_CONTEXT ) << request;
+    send( localNode, fabric::CMD_CHANNEL_DELETE_TRANSFER_WINDOW ) << request;
 }
 
 //---------------------------------------------------------------------------
@@ -1563,7 +1595,7 @@ bool Channel::_cmdConfigExit( co::ICommand& cmd )
     LBLOG( LOG_INIT ) << "Exit channel " << co::ObjectICommand( cmd )
                       << std::endl;
 
-    _deleteTransferContext();
+    _deleteTransferWindow();
 
     if( _impl->state != STATE_STOPPED )
         _impl->state = configExit() ? STATE_STOPPED : STATE_FAILED;
@@ -1734,7 +1766,7 @@ bool Channel::_cmdFinishReadback( co::ICommand& cmd )
             command.read< std::vector< uint128_t > >();
     const co::NodeIDs& netNodes = command.read< co::NodeIDs >();
 
-    getWindow()->makeCurrentTransfer();
+    _createTransferWindow();
     _finishReadback( frameData, imageIndex, frameNumber, taskID, nodes,
                      netNodes );
     _unrefFrame( frameNumber );
@@ -1878,13 +1910,34 @@ bool Channel::_cmdFrameTiles( co::ICommand& cmd )
     return true;
 }
 
-bool Channel::_cmdDeleteTransferContext( co::ICommand& cmd )
+bool Channel::_cmdCreateTransferWindow( co::ICommand& cmd )
+{
+    co::ObjectICommand command( cmd );
+
+    LBLOG( LOG_INIT ) << "Create transfer window " << command << std::endl;
+
+    LBCHECK( getWindow()->createTransferWindow( ));
+
+    const uint32_t requestID = command.get< uint32_t >();
+#ifdef EQ_QT_USED
+    QThread* transferThread = reinterpret_cast< QThread* >
+            ( getLocalNode()->getRequestData( requestID ));
+    qt::Window* transferWindow = dynamic_cast< qt::Window* >
+            ( getWindow()->getTransferWindow( ));
+    if( transferWindow )
+        transferWindow->moveContextToThread( transferThread );
+#endif
+    getLocalNode()->serveRequest( requestID );
+    return true;
+}
+
+bool Channel::_cmdDeleteTransferWindow( co::ICommand& cmd )
 {
     co::ObjectICommand command( cmd );
 
-    LBLOG( LOG_INIT ) << "Delete transfer context " << command << std::endl;
+    LBLOG( LOG_INIT ) << "Delete transfer window " << command << std::endl;
 
-    getWindow()->deleteTransferSystemWindow();
+    getWindow()->deleteTransferWindow();
     getLocalNode()->serveRequest( command.read< uint32_t >( ));
     return true;
 }

eq/channel.h

@@ -54,7 +54,9 @@ class Channel : public fabric::Channel< Window, Channel >
     EQ_API co::CommandQueue* getPipeThreadQueue(); //!< @internal
     EQ_API co::CommandQueue* getCommandThreadQueue(); //!< @internal
     EQ_API uint32_t getCurrentFrame() const; //!< @internal render thr only
-    void waitFrameFinished( const uint32_t frame ) const; //!< @internal
+
+    /** @internal */
+    bool waitFrameFinished( uint32_t frame, uint32_t timeout ) const;
 
     /**
      * @return true if this channel is stopped, false otherwise.
@@ -654,7 +656,8 @@ class Channel : public fabric::Channel< Window, Channel >
     Frames _getFrames( const co::ObjectVersions& frameIDs,
                        const bool isOutput );
 
-    void _deleteTransferContext();
+    void _createTransferWindow();
+    void _deleteTransferWindow();
 
     /* The command handler functions. */
     bool _cmdConfigInit( co::ICommand& command );
@@ -674,7 +677,8 @@ class Channel : public fabric::Channel< Window, Channel >
     bool _cmdFrameViewFinish( co::ICommand& command );
     bool _cmdStopFrame( co::ICommand& command );
     bool _cmdFrameTiles( co::ICommand& command );
-    bool _cmdDeleteTransferContext( co::ICommand& command );
+    bool _cmdCreateTransferWindow( co::ICommand& command );
+    bool _cmdDeleteTransferWindow( co::ICommand& command );
 
     LB_TS_VAR( _pipeThread );
 };

eq/config.h

@@ -115,7 +115,7 @@ class Config : public fabric::Config< Server, Config, Observer, Layout, Canvas,
     EQ_API int64_t getTime() const;
 
     /** @return the config's message pump, or 0. @version 1.0 */
-    MessagePump* getMessagePump();
+    EQ_API MessagePump* getMessagePump();
 
     /** @internal */
     const Channel* findChannel( const std::string& name ) const

eq/fabric/commands.h

@@ -177,7 +177,8 @@ namespace fabric
         CMD_CHANNEL_STOP_FRAME,
         CMD_CHANNEL_FRAME_TILES,
         CMD_CHANNEL_FINISH_READBACK,
-        CMD_CHANNEL_DELETE_TRANSFER_CONTEXT,
+        CMD_CHANNEL_CREATE_TRANSFER_WINDOW,
+        CMD_CHANNEL_DELETE_TRANSFER_WINDOW,
         CMD_CHANNEL_CUSTOM = CMD_OBJECT_CUSTOM + 30
     };
 

eq/node.cpp

@@ -331,7 +331,7 @@ void Node::_finishFrame( const uint32_t frameNumber ) const
     const Pipes& pipes = getPipes();
     for( Pipes::const_iterator i = pipes.begin(); i != pipes.end(); ++i )
     {
-        const Pipe* pipe = *i;
+        Pipe* pipe = *i;
         LBASSERT( pipe->isThreaded() || pipe->getFinishedFrame()>=frameNumber );
 
         pipe->waitFrameLocal( frameNumber );

eq/pipe.cpp

@@ -750,23 +750,38 @@ namespace
 class WaitFinishedVisitor : public PipeVisitor
 {
 public:
-    WaitFinishedVisitor( const uint32_t frame ) : _frame( frame ) {}
+    WaitFinishedVisitor( const uint32_t frame, MessagePump* pump )
+        : _frame( frame ), _pump( pump ) {}
 
     virtual VisitorResult visit( Channel* channel )
         {
-            channel->waitFrameFinished( _frame );
+            while( !channel->waitFrameFinished( _frame, 100 ))
+            {
+                // process potential pending Qt slots
+                if( _pump )
+                    _pump->dispatchAll();
+            }
+
             return TRAVERSE_CONTINUE;
         }
 
 private:
     const uint32_t _frame;
+    MessagePump* _pump;
 };
 }
 
-void Pipe::waitFrameFinished( const uint32_t frameNumber ) const
+void Pipe::waitFrameFinished( const uint32_t frameNumber )
 {
-    _impl->finishedFrame.waitGE( frameNumber );
-    WaitFinishedVisitor waiter( frameNumber );
+    MessagePump* pump = getConfig()->getMessagePump();
+    while( !_impl->finishedFrame.timedWaitGE( frameNumber, 100 ))
+    {
+        // process potential pending Qt slots
+        if( pump )
+            pump->dispatchAll();
+    }
+
+    WaitFinishedVisitor waiter( frameNumber, pump );
     accept( waiter );
 }
 

eq/pipe.h

@@ -147,7 +147,7 @@ class Pipe : public fabric::Pipe< Node, Pipe, eq::Window, PipeVisitor >
      * @param frameNumber the frame number.
      * @sa releaseFrame()
      */
-    EQ_API void waitFrameFinished( const uint32_t frameNumber ) const;
+    EQ_API void waitFrameFinished( const uint32_t frameNumber );
 
     /**
      * Send a pipe error event to the application node.

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, use FBO instead
+    if( drawable == FBO || drawable == PBUFFER )
         return configInitFBO();
     return true;
 }
@@ -132,5 +154,10 @@ QObject* Window::getEventProcessor()
     return _impl->getEventProcessor();
 }
 
+void Window::moveContextToThread( QThread* thread_ )
+{
+    _impl->getContext()->moveToThread( thread_ );
+}
+
 }
 }

eq/qt/window.h

@@ -133,13 +133,15 @@ class Window : public QObject, public WindowIF
      */
     EQ_API QObject* getEventProcessor();
 
+    /** Move OpenGLContext object to given thread. @version 1.10 */
+    void moveContextToThread( QThread* thread );
+
 signals:
     void destroyImpl( detail::Window* );
 
 private:
     detail::Window* const _impl;
-    static detail::Window* createImpl( const WindowSettings&, QOpenGLContext*,
-                                       QThread* );
+    static detail::Window* createImpl( const WindowSettings&, QThread* );
     friend class WindowFactory;
 };
 }

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 )