changeset 26276:5535267e88ba stable

Make creation and destruction of Qt widgets synchronous (bug #55526) * Backend.cc (Backend::Backend): Use Qt::BlockingQueuedConnection for widget creation. (Backend::initialize): Unlock mutex before creating widget. (Backend::finalize): Unlock mutex before calling ObjectProxy::finalize * ObjectProxy.h (ObjectProxy::sendFinalize): Remove signal. * ObjectProxy.cc (ObjectProxy::init): Remove connection to sendFinalize. (ObjectProxy::finalize): Make use of single QMetaObject::invokeMethod to trigger the execution of the Object's slotFinalize. Run synchronously when the interpreter thread, in which the ObjectProxy lives, is not the GUI thread. (ObjectProxy::get_pixels): Remove successive calls to QMetaObject::invokeMethod since this should not fail anymore. * graphics.cc: Remove unnecessary pause in uicontexmenu BIST. (Fdrawnow): Remove comments about unnecessary pause.
author Pantxo Diribarne <pantxo.diribarne@gmail.com>
date Sat, 15 Dec 2018 11:31:31 +0100
parents aa47b1287d7e
children e92a44730a6e
files libgui/graphics/Backend.cc libgui/graphics/ObjectProxy.cc libgui/graphics/ObjectProxy.h libinterp/corefcn/graphics.cc
diffstat 4 files changed, 23 insertions(+), 41 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/graphics/Backend.cc	Thu Dec 20 20:52:11 2018 -0800
+++ b/libgui/graphics/Backend.cc	Sat Dec 15 11:31:31 2018 +0100
@@ -76,7 +76,8 @@
     ObjectFactory *factory = ObjectFactory::instance ();
 
     connect (this, SIGNAL (createObject (double)),
-             factory, SLOT (createObject (double)));
+             factory, SLOT (createObject (double)),
+             Qt::BlockingQueuedConnection);
   }
 
   Backend::~Backend (void)
@@ -96,6 +97,11 @@
         || go.isa ("uipushtool")
         || go.isa ("uitoggletool"))
       {
+        // FIXME: We need to unlock the mutex here but we have no way to know if
+        // if it was previously locked by this thread, and thus if we should
+        // re-lock it.
+        gh_manager::unlock ();
+
         Logger::debug ("Backend::initialize %s from thread %08x",
                        go.type ().c_str (), QThread::currentThreadId ());
 
@@ -154,6 +160,11 @@
   void
   Backend::finalize (const graphics_object& go)
   {
+    // FIXME: We need to unlock the mutex here but we have no way to know if
+    // if it was previously locked by this thread, and thus if we should
+    // re-lock it.
+    gh_manager::unlock ();
+
     Logger::debug ("Backend::finalize %s from thread %08x",
                    go.type ().c_str (), QThread::currentThreadId ());
 
--- a/libgui/graphics/ObjectProxy.cc	Thu Dec 20 20:52:11 2018 -0800
+++ b/libgui/graphics/ObjectProxy.cc	Sat Dec 15 11:31:31 2018 +0100
@@ -52,8 +52,6 @@
           {
             disconnect (this, SIGNAL (sendUpdate (int)),
                         m_object, SLOT (slotUpdate (int)));
-            disconnect (this, SIGNAL (sendFinalize (void)),
-                        m_object, SLOT (slotFinalize (void)));
             disconnect (this, SIGNAL (sendRedraw (void)),
                         m_object, SLOT (slotRedraw (void)));
             disconnect (this, SIGNAL (sendShow (void)),
@@ -68,8 +66,6 @@
           {
             connect (this, SIGNAL (sendUpdate (int)),
                      m_object, SLOT (slotUpdate (int)));
-            connect (this, SIGNAL (sendFinalize (void)),
-                     m_object, SLOT (slotFinalize (void)));
             connect (this, SIGNAL (sendRedraw (void)),
                      m_object, SLOT (slotRedraw (void)));
             connect (this, SIGNAL (sendShow (void)),
@@ -84,7 +80,7 @@
   void
   ObjectProxy::setObject (Object *obj)
   {
-    emit sendFinalize ();
+    finalize ();
     init (obj);
   }
 
@@ -100,8 +96,15 @@
   void
   ObjectProxy::finalize (void)
   {
-    emit sendFinalize ();
-    init (nullptr);
+    if (! m_object)
+      return;
+
+    Qt::ConnectionType t = Qt::BlockingQueuedConnection;
+
+    if (QThread::currentThread () == QCoreApplication::instance ()->thread ())
+      t = Qt::DirectConnection;
+
+    QMetaObject::invokeMethod (m_object, "slotFinalize", t);
   }
 
   void
@@ -140,22 +143,6 @@
     QMetaObject::invokeMethod (m_object, "slotGetPixels", t,
                                Q_RETURN_ARG (uint8NDArray, retval));
 
-    // FIXME: The following may fail for obscure reasons, see bug #53328.
-    //        In absence of a solution, we retry twice before calling error().
-    if (! QMetaObject::invokeMethod (m_object, "slotGetPixels", t,
-                                     Q_RETURN_ARG (uint8NDArray, retval)))
-      {
-        octave::sleep (0.1);
-        if (! QMetaObject::invokeMethod (m_object, "slotGetPixels", t,
-                                         Q_RETURN_ARG (uint8NDArray, retval)))
-          {
-            octave::sleep (0.2);
-            if (! QMetaObject::invokeMethod (m_object, "slotGetPixels", t,
-                                             Q_RETURN_ARG (uint8NDArray, retval)))
-              error ("getframe: unable to retrieve figure pixels");
-          }
-      }
-
     return retval;
   }
 
--- a/libgui/graphics/ObjectProxy.h	Thu Dec 20 20:52:11 2018 -0800
+++ b/libgui/graphics/ObjectProxy.h	Sat Dec 15 11:31:31 2018 +0100
@@ -53,7 +53,6 @@
 
   signals:
     void sendUpdate (int pId);
-    void sendFinalize (void);
     void sendRedraw (void);
     void sendShow (void);
     void sendPrint (const QString& file_cmd, const QString& term);
--- a/libinterp/corefcn/graphics.cc	Thu Dec 20 20:52:11 2018 -0800
+++ b/libinterp/corefcn/graphics.cc	Sat Dec 15 11:31:31 2018 +0100
@@ -6112,7 +6112,7 @@
   zticklen = ticksign * (mode2D ? ticklen(0) : ticklen(1));
 
   double offset = get___fontsize_points__ () / 2;
-  
+
   xtickoffset = (mode2D ? std::max (0., xticklen) : std::abs (xticklen)) +
                 (xstate == AXE_HORZ_DIR ? offset*1.5 : offset);
   ytickoffset = (mode2D ? std::max (0., yticklen) : std::abs (yticklen)) +
@@ -10409,7 +10409,6 @@
 %!   assert (get (hax, "uicontextmenu"), hctx2);
 %!   assert (get (hf, "children"), [hctx2; hctx1; hax]);
 %!   delete (hctx2);
-%!   pause (.005);
 %!   assert (get (hf, "uicontextmenu"), []);
 %!   assert (get (hax, "uicontextmenu"), []);
 %!   assert (get (hf, "children"), [hctx1; hax]);
@@ -13160,24 +13159,10 @@
 
           graphics_object go = gh_manager::get_object (h);
 
-          // FIXME: when using qt toolkit the print_figure method
-          // returns immediately and Canvas::print doesn't have
-          // enough time to lock the mutex before we lock it here
-          // again.  We thus wait 50 ms (which may not be enough) to
-          // give it a chance: see http://octave.1599824.n4.nabble.com/Printing-issues-with-Qt-toolkit-tp4673270.html
-
           gh_manager::unlock ();
 
           go.get_toolkit ().print_figure (go, term, file, debug_file);
 
-          // FIXME: In ObjectProxy.cc ObjectProxy::init
-          // we now use connect (..., Qt::BlockingQueuedConnection)
-          // which should make the sleep unnecessary.
-          // See bug #44463 and #48519
-          // Remove it and the FIXME block above after testing.
-
-          // octave_sleep (0.05);
-
           gh_manager::lock ();
         }
     }