changeset 27318:ae53e56e16f2

eliminate separate ObjectFactory class in Qt graphics toolkit * __init_qt__.cc (__init__): Move qt_graphics_toolkit object to GUI application thread. * qt-graphics-toolkit.h, qt-graphics-toolkit.cc (qt_graphics_toolkit::m_factory): Delete data member and all use. (qt_graphics_toolkit::create_object_signal): Rename from createObject. Eliminate pointer to qt_graphics_toolkit argument. (qt_graphics_toolkit::create_object): New slot, adapted from ObjectFactory::createObject slot. * ObjectFactory.h, ObjectFactory.cc: Delete. * libgui/graphics/module.mk: Update.
author John W. Eaton <jwe@octave.org>
date Fri, 02 Aug 2019 14:28:48 -0500
parents 718116e9c7d3
children 6b2d20317b26
files libgui/graphics/ObjectFactory.cc libgui/graphics/ObjectFactory.h libgui/graphics/__init_qt__.cc libgui/graphics/module.mk libgui/graphics/qt-graphics-toolkit.cc libgui/graphics/qt-graphics-toolkit.h
diffstat 6 files changed, 151 insertions(+), 226 deletions(-) [+]
line wrap: on
line diff
--- a/libgui/graphics/ObjectFactory.cc	Fri Aug 02 12:18:48 2019 -0500
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,153 +0,0 @@
-/*
-
-Copyright (C) 2011-2019 Michael Goffioul
-
-This file is part of Octave.
-
-Octave is free software: you can redistribute it and/or modify it
-under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or
-(at your option) any later version.
-
-Octave is distributed in the hope that it will be useful, but
-WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with Octave; see the file COPYING.  If not, see
-<https://www.gnu.org/licenses/>.
-
-*/
-
-#if defined (HAVE_CONFIG_H)
-#  include "config.h"
-#endif
-
-#include <QApplication>
-#include <QThread>
-
-#include "event-manager.h"
-#include "graphics.h"
-
-#include "ButtonGroup.h"
-#include "CheckBoxControl.h"
-#include "ContextMenu.h"
-#include "EditControl.h"
-#include "Figure.h"
-#include "ListBoxControl.h"
-#include "Logger.h"
-#include "Menu.h"
-#include "ObjectFactory.h"
-#include "ObjectProxy.h"
-#include "Panel.h"
-#include "PopupMenuControl.h"
-#include "PushButtonControl.h"
-#include "PushTool.h"
-#include "RadioButtonControl.h"
-#include "SliderControl.h"
-#include "Table.h"
-#include "TextControl.h"
-#include "ToggleButtonControl.h"
-#include "ToggleTool.h"
-#include "ToolBar.h"
-#include "QtHandlesUtils.h"
-#include "qt-graphics-toolkit.h"
-
-namespace QtHandles
-{
-  void
-  ObjectFactory::createObject (qt_graphics_toolkit *qt_gtk, double handle)
-  {
-    gh_manager::auto_lock lock;
-
-    graphics_object go (gh_manager::get_object (graphics_handle (handle)));
-
-    if (go.valid_object ())
-      {
-        if (go.get_properties ().is_beingdeleted ())
-          qWarning ("ObjectFactory::createObject: object is being deleted");
-        else
-          {
-            ObjectProxy *proxy = qt_graphics_toolkit::toolkitObjectProxy (go);
-
-            if (proxy)
-              {
-                Logger::debug ("ObjectFactory::createObject: "
-                               "create %s from thread %08x",
-                               go.type ().c_str (), QThread::currentThreadId ());
-
-                Object *obj = nullptr;
-
-                if (go.isa ("figure"))
-                  obj = Figure::create (go);
-                else if (go.isa ("uicontrol"))
-                  {
-                    uicontrol::properties& up =
-                      Utils::properties<uicontrol> (go);
-
-                    if (up.style_is ("pushbutton"))
-                      obj = PushButtonControl::create (go);
-                    else if (up.style_is ("edit"))
-                      obj = EditControl::create (go);
-                    else if (up.style_is ("checkbox"))
-                      obj = CheckBoxControl::create (go);
-                    else if (up.style_is ("radiobutton"))
-                      obj = RadioButtonControl::create (go);
-                    else if (up.style_is ("togglebutton"))
-                      obj = ToggleButtonControl::create (go);
-                    else if (up.style_is ("text"))
-                      obj = TextControl::create (go);
-                    else if (up.style_is ("popupmenu"))
-                      obj = PopupMenuControl::create (go);
-                    else if (up.style_is ("slider"))
-                      obj = SliderControl::create (go);
-                    else if (up.style_is ("listbox"))
-                      obj = ListBoxControl::create (go);
-                  }
-                else if (go.isa ("uibuttongroup"))
-                  obj = ButtonGroup::create (go);
-                else if (go.isa ("uipanel"))
-                  obj = Panel::create (go);
-                else if (go.isa ("uimenu"))
-                  obj = Menu::create (go);
-                else if (go.isa ("uicontextmenu"))
-                  obj = ContextMenu::create (go);
-                else if (go.isa ("uitable"))
-                  obj = Table::create (go);
-                else if (go.isa ("uitoolbar"))
-                  obj = ToolBar::create (go);
-                else if (go.isa ("uipushtool"))
-                  obj = PushTool::create (go);
-                else if (go.isa ("uitoggletool"))
-                  obj = ToggleTool::create (go);
-                else
-                  qWarning ("ObjectFactory::createObject: unsupported type '%s'",
-                            go.type ().c_str ());
-
-                if (obj)
-                  {
-                    proxy->setObject (obj);
-
-                    connect (obj,
-                             SIGNAL (interpreter_event (const octave::fcn_callback&)),
-                             qt_gtk,
-                             SLOT (interpreter_event (const octave::fcn_callback&)));
-
-                    connect (obj,
-                             SIGNAL (interpreter_event (const octave::meth_callback&)),
-                             qt_gtk,
-                             SLOT (interpreter_event (const octave::meth_callback&)));
-                  }
-              }
-            else
-              qWarning ("ObjectFactory::createObject: no proxy for handle %g",
-                        handle);
-          }
-      }
-    else
-      qWarning ("ObjectFactory::createObject: invalid object for handle %g",
-                handle);
-  }
-
-};
--- a/libgui/graphics/ObjectFactory.h	Fri Aug 02 12:18:48 2019 -0500
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,50 +0,0 @@
-/*
-
-Copyright (C) 2011-2019 Michael Goffioul
-
-This file is part of Octave.
-
-Octave is free software: you can redistribute it and/or modify it
-under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or
-(at your option) any later version.
-
-Octave is distributed in the hope that it will be useful, but
-WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with Octave; see the file COPYING.  If not, see
-<https://www.gnu.org/licenses/>.
-
-*/
-
-#if ! defined (octave_ObjectFactory_h)
-#define octave_ObjectFactory_h 1
-
-#include <QObject>
-
-class graphics_object;
-
-namespace QtHandles
-{
-  class qt_graphics_toolkit;
-  class Object;
-
-  class ObjectFactory : public QObject
-  {
-    Q_OBJECT
-
-  public:
-
-    ObjectFactory (void) : QObject () { }
-
-  public slots:
-
-    void createObject (qt_graphics_toolkit *, double handle);
-  };
-
-};
-
-#endif
--- a/libgui/graphics/__init_qt__.cc	Fri Aug 02 12:18:48 2019 -0500
+++ b/libgui/graphics/__init_qt__.cc	Fri Aug 02 14:28:48 2019 -0500
@@ -30,6 +30,7 @@
 #include <QMetaType>
 #include <QPalette>
 #include <QRegExp>
+#include <QThread>
 
 #include "defun-dld.h"
 #include "graphics.h"
@@ -65,7 +66,14 @@
 
             octave::gtk_manager& gtk_mgr = interp.get_gtk_manager ();
 
-            graphics_toolkit tk (new qt_graphics_toolkit (interp));
+            qt_graphics_toolkit *qt_gtk = new qt_graphics_toolkit (interp);
+
+            if (QThread::currentThread ()
+                != QApplication::instance ()->thread ())
+              qt_gtk->moveToThread (QApplication::instance ()->thread ());
+
+            graphics_toolkit tk (qt_gtk);
+
             gtk_mgr.load_toolkit (tk);
 
             octave::interpreter::add_atexit_function ("__shutdown_qt__");
--- a/libgui/graphics/module.mk	Fri Aug 02 12:18:48 2019 -0500
+++ b/libgui/graphics/module.mk	Fri Aug 02 14:28:48 2019 -0500
@@ -14,7 +14,6 @@
   %reldir%/moc-ListBoxControl.cc \
   %reldir%/moc-Menu.cc \
   %reldir%/moc-Object.cc \
-  %reldir%/moc-ObjectFactory.cc \
   %reldir%/moc-ObjectProxy.cc \
   %reldir%/moc-PopupMenuControl.cc \
   %reldir%/moc-PushTool.cc \
@@ -62,7 +61,6 @@
   %reldir%/Menu.h \
   %reldir%/MenuContainer.h \
   %reldir%/Object.h \
-  %reldir%/ObjectFactory.h \
   %reldir%/ObjectProxy.h \
   %reldir%/Panel.h \
   %reldir%/PopupMenuControl.h \
@@ -102,7 +100,6 @@
   %reldir%/Logger.cc \
   %reldir%/Menu.cc \
   %reldir%/Object.cc \
-  %reldir%/ObjectFactory.cc \
   %reldir%/ObjectProxy.cc \
   %reldir%/Panel.cc \
   %reldir%/PopupMenuControl.cc \
--- a/libgui/graphics/qt-graphics-toolkit.cc	Fri Aug 02 12:18:48 2019 -0500
+++ b/libgui/graphics/qt-graphics-toolkit.cc	Fri Aug 02 14:28:48 2019 -0500
@@ -30,14 +30,32 @@
 #include <QFontMetrics>
 #include <QThread>
 
+#include "ButtonGroup.h"
+#include "CheckBoxControl.h"
+#include "ContextMenu.h"
+#include "EditControl.h"
+#include "Figure.h"
+#include "ListBoxControl.h"
 #include "Logger.h"
+#include "Menu.h"
 #include "Object.h"
-#include "ObjectFactory.h"
 #include "ObjectProxy.h"
+#include "Panel.h"
+#include "PopupMenuControl.h"
+#include "PushButtonControl.h"
+#include "PushTool.h"
 #include "QtHandlesUtils.h"
+#include "RadioButtonControl.h"
+#include "SliderControl.h"
+#include "Table.h"
+#include "TextControl.h"
+#include "ToggleButtonControl.h"
+#include "ToggleTool.h"
+#include "ToolBar.h"
 #include "qt-graphics-toolkit.h"
 
 #include "event-manager.h"
+#include "graphics.h"
 #include "interpreter.h"
 
 //#if INTPTR_MAX == INT32_MAX
@@ -76,20 +94,15 @@
   }
 
   qt_graphics_toolkit::qt_graphics_toolkit (octave::interpreter& interp)
-    : QObject (), base_graphics_toolkit ("qt"), m_interpreter (interp),
-      m_factory (new ObjectFactory ())
+    : QObject (), base_graphics_toolkit ("qt"), m_interpreter (interp)
   {
-    if (QThread::currentThread () != QApplication::instance ()->thread ())
-      m_factory->moveToThread (QApplication::instance ()->thread ());
+    // Implemented with a signal/slot connection in order to properly
+    // cross from the interpreter thread (where requests to create
+    // graphics object are initiated) to the GUI application thread
+    // (where they are actually created and displayed).
 
-    connect (this, SIGNAL (createObject (qt_graphics_toolkit *, double)),
-             m_factory, SLOT (createObject (qt_graphics_toolkit *, double)),
-             Qt::BlockingQueuedConnection);
-  }
-
-  qt_graphics_toolkit::~qt_graphics_toolkit (void)
-  {
-    delete m_factory;
+    connect (this, SIGNAL (create_object_signal (double)),
+             this, SLOT (create_object (double)));
   }
 
   bool
@@ -120,7 +133,7 @@
         OCTAVE_PTR_TYPE tmp (reinterpret_cast<OCTAVE_INTPTR_TYPE> (proxy));
         gObj.get_properties ().set (toolkitObjectProperty (go), tmp);
 
-        emit createObject (this, go.get_handle ().value ());
+        emit create_object_signal (go.get_handle ().value ());
 
         return true;
       }
@@ -337,4 +350,99 @@
 
     evmgr.post_event (meth);
   }
+
+  void
+  qt_graphics_toolkit::create_object (double handle)
+  {
+    gh_manager::auto_lock lock;
+
+    graphics_object go (gh_manager::get_object (graphics_handle (handle)));
+
+    if (go.valid_object ())
+      {
+        if (go.get_properties ().is_beingdeleted ())
+          qWarning ("qt_graphics_toolkit::create_object: object is being deleted");
+        else
+          {
+            ObjectProxy *proxy = qt_graphics_toolkit::toolkitObjectProxy (go);
+
+            if (proxy)
+              {
+                Logger::debug ("qt_graphics_toolkit::create_object: "
+                               "create %s from thread %08x",
+                               go.type ().c_str (), QThread::currentThreadId ());
+
+                Object *obj = nullptr;
+
+                if (go.isa ("figure"))
+                  obj = Figure::create (go);
+                else if (go.isa ("uicontrol"))
+                  {
+                    uicontrol::properties& up =
+                      Utils::properties<uicontrol> (go);
+
+                    if (up.style_is ("pushbutton"))
+                      obj = PushButtonControl::create (go);
+                    else if (up.style_is ("edit"))
+                      obj = EditControl::create (go);
+                    else if (up.style_is ("checkbox"))
+                      obj = CheckBoxControl::create (go);
+                    else if (up.style_is ("radiobutton"))
+                      obj = RadioButtonControl::create (go);
+                    else if (up.style_is ("togglebutton"))
+                      obj = ToggleButtonControl::create (go);
+                    else if (up.style_is ("text"))
+                      obj = TextControl::create (go);
+                    else if (up.style_is ("popupmenu"))
+                      obj = PopupMenuControl::create (go);
+                    else if (up.style_is ("slider"))
+                      obj = SliderControl::create (go);
+                    else if (up.style_is ("listbox"))
+                      obj = ListBoxControl::create (go);
+                  }
+                else if (go.isa ("uibuttongroup"))
+                  obj = ButtonGroup::create (go);
+                else if (go.isa ("uipanel"))
+                  obj = Panel::create (go);
+                else if (go.isa ("uimenu"))
+                  obj = Menu::create (go);
+                else if (go.isa ("uicontextmenu"))
+                  obj = ContextMenu::create (go);
+                else if (go.isa ("uitable"))
+                  obj = Table::create (go);
+                else if (go.isa ("uitoolbar"))
+                  obj = ToolBar::create (go);
+                else if (go.isa ("uipushtool"))
+                  obj = PushTool::create (go);
+                else if (go.isa ("uitoggletool"))
+                  obj = ToggleTool::create (go);
+                else
+                  qWarning ("qt_graphics_toolkit::create_object: unsupported type '%s'",
+                            go.type ().c_str ());
+
+                if (obj)
+                  {
+                    proxy->setObject (obj);
+
+                    connect (obj,
+                             SIGNAL (interpreter_event (const octave::fcn_callback&)),
+                             this,
+                             SLOT (interpreter_event (const octave::fcn_callback&)));
+
+                    connect (obj,
+                             SIGNAL (interpreter_event (const octave::meth_callback&)),
+                             this,
+                             SLOT (interpreter_event (const octave::meth_callback&)));
+                  }
+              }
+            else
+              qWarning ("qt_graphics_toolkit::create_object: no proxy for handle %g",
+                        handle);
+          }
+      }
+    else
+      qWarning ("qt_graphics_toolkit::create_object: invalid object for handle %g",
+                handle);
+  }
+
 };
--- a/libgui/graphics/qt-graphics-toolkit.h	Fri Aug 02 12:18:48 2019 -0500
+++ b/libgui/graphics/qt-graphics-toolkit.h	Fri Aug 02 14:28:48 2019 -0500
@@ -36,7 +36,6 @@
 namespace QtHandles
 {
   class Object;
-  class ObjectFactory;
   class ObjectProxy;
 
   class qt_graphics_toolkit : public QObject, public base_graphics_toolkit
@@ -47,7 +46,22 @@
 
     qt_graphics_toolkit (octave::interpreter& interp);
 
-    ~qt_graphics_toolkit (void);
+    ~qt_graphics_toolkit (void) = default;
+
+    // The interpreter may call graphics toolkit functions that we
+    // implement here.  The Qt GUI that manages these actions runs in a
+    // separate thread.  So in order to correctly cross from the
+    // interpreter thread to the GUI thread, these functions should emit
+    // signals (in the interpreter thread) that are handled by slots
+    // that will run in the GUI thread.  This design is similar to the
+    // event_manager, interpreter_events, and qt_interpreter_events
+    // classes work to pass messages from the interpreter to the GUI.
+    //
+    // FIXME: currently most of these functions do not emit signals.
+    // They may work because they use locking and the gh_manager class,
+    // but it might be better to use Qt signals and slots.  In any case,
+    // we should ensure that they are correctly handling the connection
+    // between the interpreter and GUI threads.
 
     bool is_valid (void) const { return true; }
 
@@ -75,18 +89,19 @@
     static ObjectProxy * toolkitObjectProxy (const graphics_object& go);
 
   signals:
-    void createObject (qt_graphics_toolkit *, double handle);
+
+    void create_object_signal (double handle);
 
   public slots:
 
     void interpreter_event (const octave::fcn_callback& fcn);
     void interpreter_event (const octave::meth_callback& meth);
 
+    void create_object (double handle);
+
   private:
 
     octave::interpreter& m_interpreter;
-
-    ObjectFactory *m_factory;
   };
 
 }