Re: Removing __init_qt__ dependency on libgui

Thanks for your notes and thoughts John. This is probably a good point to admit that I'm not particularly a guru with C++, especially with features introduced in C++11 and beyond, e.g. lambdas, so not sure how much help or advice I can offer with some of your below questions. Hopefully some of the other maintainers can chip in with suggestions here, though.

________________________________
From: John W. Eaton <[email protected]>
Sent: 17 November 2020 21:36
To: Alex Leach <[email protected]>; [email protected] <[email protected]>
Subject: Re: Removing __init_qt__ dependency on libgui

Looking at this now, I see that the base_qobject object is just used for
access to the resource manager (see additional comments about that below).
So, no issue getting rid of the base_qobject then, by the sounds of it?
The interpreter object is primarily used to access the gh_manager mutex
object.  I don't think locking is really handled correctly now.  Fixing
that might completely avoid the need for direct access to the gh_manager
mutex in the graphics code.  Or, if we do decide that the right way is
to lock the gh_manager object in the Qt graphics thread, then maybe we
could come up with a better way to do that?

This is something that hopefully some of the other maintainers can offer advice on, as my knowledge on proper implementation of thread-safety is limited, at best.

Other than locking, the gh_manager object is also used to get
graphics_object objects.  Are there better ways of doing that job?  Can
we just emit signals that request something and then respond to a signal
that comes back from the interpreter?  Or in some instances do we need
to suspend execution and wait for the result before allowing the
graphics thread to continue processing?  For example, similar to the
interpreter_event signals that we have, maybe we could use something like

   emit gh_manager_event ([=] (gh_manager& gh_mgr) { ... });

and have the gh_manager_event processed by the qt_graphics_toolkit
object (same as the interpreter_event) and at that point we could access
the gh_manager object without needing to pass it around to all the
sub-objects that make up the Qt graphics widget.

Whatever we do, we need thread-safe access to the graphics objects that
are stored in the gh_manager in the interpreter thread and I don't think
that has ever been done 100% correctly.

The interpreter is also used to get access to the event manager object,
but that only happens in qt-graphics-toolkit.cc.  We probably don't need
to worry about the direct access to the interpreter in that file.  For
the rest.

>   * Any references to the GUI's resource manager need to be removed. I
>     think there are only a couple, used to get mouse and toolbar icons
>     (Canvas.cc and ToolBarButton.cc, respectively), and a window
>     position (annotation-dialog.cc). Are the mouse icons used by the GUI
>     elsewhere? Maybe these icons can be moved into a
>     libgui/graphics/icons/ folder, if they're only used by qthandles
>     figures? Is there any reason libgui/graphics can't have its own
>     resource file?

I assume we'd like to have one configuration dialog for the Octave GUI
that also allows setting of these resources?  Can that configuration
dialog use two separate resource files?  If the graphics widget may also
be used separately, does it need a separate configuration editor?

As far as I'm aware, there's nothing configured by the graphics toolkit in the resource manager, no? The resource manager icons are pretty static in nature; either theme icons are pulled from the system theme, or otherwise through the Qt resource file, the latter which is statically compiled into the shared library module. The annotation dialog's window position is currently saved through the resource manager, but there's no user configuration required here; it just saves its position and when next opened, opens in the same position. This should be easily achieved through graphics library having its own QSettings object.


OK.  If the Octave interpreter were also packaged as a Qt object with
public signals and slots, what set of signals and slots do we need?

Currently, the GUI has some signals that it can emit to invoke Octave
functions or, using C++ lambda expressions, run arbitrary code inside
the interpreter thread.  Those lambda expressions can then emit Qt
signals directly rather than using the octave event manager class.

As you mentioned earlier, it would be good to simplify the event manager
object.

I've just had a look through event-manager.h, as spent more time familiarising myself with octave_link in the 5.2.0 code, and noticed that it has been split into two classes: interpreter_events and event_manager, which seems logical from a multi-threading point of view. It looks like event_manager only has one virtual function now, so would be much simpler to derive from, for my purposes at least, but I'd still need to subclass interpreter_events and they both have many methods that I just wouldn't need to use at all; all I need are QFileDialog instances.

As I say, I'm no guru, but I think your FIXME comment in event-manager.h is exactly on the same line as I was thinking:-

 // FIXME: it would be nice if instead of requiring the GUI to derive
 // from this class, it could subscribe to individual events, possibly
 // multiple times.  In that way, it would be more flexible and
 // decentralized, similar to the Qt signal/slot connection mechanism
 // and would allow the GUI to connect multiple signals to a single
 // action or multiple actions to a single signal.

I don't know how easy this would be to implement, but I imagine it would be quite a major change and would affect many areas of the codebase. Further, I think the registered events would need to be well-documented somehow, or at least introspectable to get a list of created events.

I'm not sure where fltk dialogs are coming from?  It should be possible
for them to all be based on Qt widgets.
Two reasons: 1) __get_funcname__.m, called by uigetdir.m, etc. will default to calling e.g. __uigetdir_fltk__, and 2) __init_qt__.cc, before it was removed, actually had a preprocessor #if 0 statement around all the DEFUN_DLD definitions of __uigetfile_qt__ and the other related dialog functions. I think they were all essentially commented out, as the Qt widgets I guess are called by the event manager now?

Hope some of the above is of use.

KR, Alex