From 1e81b8d61afb039ebd4f5c7f502df39ae9f77b03 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 4 Aug 2017 19:07:05 +0200 Subject: [PATCH] Settle proper activation of the external UI interfaces (#1098) This change was caused by investigation of UI event loop dispatch; since the GTK UI is designed to run single threaded, any invocation from other threads need to be diepatched explicitly. A possible way to achieve this is to use Glib::Dispatcher, which in turn requires that the current thread (which is in this case the UI thread) already holds a Glib::MainContext This prompted me to create a tight link between the external facade interfaces of the UI and the event loop itself. What remains to be settled is how to hand over arguments to the action in the main loop --- src/gui/ctrl/core-service.hpp | 4 - src/gui/ctrl/facade.hpp | 83 +++++ src/gui/ctrl/global-ctx.hpp | 2 +- src/gui/ctrl/ui-manager.cpp | 8 +- src/gui/ctrl/ui-manager.hpp | 2 + src/gui/display-service.cpp | 4 + src/gui/display-service.hpp | 4 + src/gui/notification-service.cpp | 29 +- src/gui/notification-service.hpp | 7 +- src/include/display-facade.h | 7 +- src/proc/control/proc-dispatcher.cpp | 6 +- wiki/renderengine.html | 12 +- wiki/thinkPad.ichthyo.mm | 537 +++++++++++++++++++++++---- 13 files changed, 607 insertions(+), 98 deletions(-) create mode 100644 src/gui/ctrl/facade.hpp diff --git a/src/gui/ctrl/core-service.hpp b/src/gui/ctrl/core-service.hpp index 62fa772af..19b39d86b 100644 --- a/src/gui/ctrl/core-service.hpp +++ b/src/gui/ctrl/core-service.hpp @@ -81,11 +81,9 @@ #include "lib/error.hpp" -#include "include/logging.h" #include "lib/idi/entry-id.hpp" #include "lib/diff/gen-node.hpp" #include "include/session-command-facade.h" -#include "gui/notification-service.hpp" #include "gui/ctrl/state-recorder.hpp" #include "gui/ctrl/bus-term.hpp" #include "gui/ctrl/nexus.hpp" @@ -114,7 +112,6 @@ namespace ctrl{ Nexus uiBusBackbone_; StateRecorder stateRecorder_; - NotificationService notificationService_; virtual void @@ -137,7 +134,6 @@ namespace ctrl{ : BusTerm(identity, uiBusBackbone_) , uiBusBackbone_{*this} , stateRecorder_{*this} - , notificationService_(uiBusBackbone_) // opens the GuiNotificationService instance { INFO (gui, "UI-Backbone operative."); } diff --git a/src/gui/ctrl/facade.hpp b/src/gui/ctrl/facade.hpp new file mode 100644 index 000000000..173cd8b62 --- /dev/null +++ b/src/gui/ctrl/facade.hpp @@ -0,0 +1,83 @@ +/* + FACADE.hpp - Manage the public facade interfaces of the UI + + Copyright (C) Lumiera.org + 2017, Hermann Vosseler + + This program 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 2 of + the License, or (at your option) any later version. + + This program 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 this program; if not, write to the Free Software + Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + +*/ + + +/** @file facade.hpp + ** Manage the lifecycle of the public UI facade interface(s). + ** Startup of the UI is a tricky process, which requires to establish a main context and to wire + ** the right components in the right order, prior to activating the GTK event loop. External interfaces + ** of the UI-Layer should be activated only after everything is wired properly and after the event loop + ** is actually operative. + ** + ** @note GTK operates single threaded by design. + ** For this reason, any call from other parts of the application need to be explicitly + ** dispatched into the UI event loop. The external facade interfaces are defined in a way + ** to ensure this constraint is met. + ** + ** @see notification-service.hpp + ** @see ui-manager.hpp + ** @see ui-bus.hpp + */ + + +#ifndef GUI_CTRL_FACADE_H +#define GUI_CTRL_FACADE_H + +#include "gui/notification-service.hpp" + +#include + + +namespace gui { +namespace ctrl { + + + /** + * A context to hold and manage the implementation of all UI facade interfaces. + * The lifecycle follows RAII-style: whenever this object is constructed, all + * facade interfaces are open and fully operative. + * + * @remark the UiManager is responsible to activate and deactivate those interfaces + */ + class Facade + : boost::noncopyable + { + NotificationService notificationService_; + + + public: + /** + * Activate all external UI facade interfaces. + */ + Facade (UiBus& bus, UiManager& manager) + : notificationService_(bus.getAccessPoint(), manager) // opens the GuiNotificationService instance + { + INFO (gui, "UI-Facade Interfaces activated."); + } + + private: + }; + + + +}}// namespace gui::ctrl +#endif /*GUI_CTRL_FACADE_H*/ diff --git a/src/gui/ctrl/global-ctx.hpp b/src/gui/ctrl/global-ctx.hpp index e5b3d79f7..ef1499c8c 100644 --- a/src/gui/ctrl/global-ctx.hpp +++ b/src/gui/ctrl/global-ctx.hpp @@ -109,5 +109,5 @@ namespace ctrl { -}}// namespace gui::workspace +}}// namespace gui::ctrl #endif /*GUI_CTRL_GLOBAL_CTX_H*/ diff --git a/src/gui/ctrl/ui-manager.cpp b/src/gui/ctrl/ui-manager.cpp index 934490a70..3a7fe218b 100644 --- a/src/gui/ctrl/ui-manager.cpp +++ b/src/gui/ctrl/ui-manager.cpp @@ -36,6 +36,7 @@ #include "gui/ctrl/ui-manager.hpp" #include "gui/ctrl/global-ctx.hpp" #include "gui/ctrl/actions.hpp" +#include "gui/ctrl/facade.hpp" #include "gui/workspace/style-manager.hpp" #include "lib/searchpath.hpp" #include "lib/util.hpp" @@ -101,6 +102,7 @@ namespace ctrl { , Gtk::UIManager() , globals_{new GlobalCtx{bus, *this}} , actions_{new Actions{*globals_}} + , facade_{} // note: not activated yet , styleManager_{new StyleManager{}} { actions_->populateMainActions (*this); @@ -120,7 +122,7 @@ namespace ctrl { /** - * Run the GTK UI. + * Run the GTK UI. Also _activate_ the UI the external interfaces. * @remarks this function is equivalent to calling Gtk::Main::run() * In GTK-3.0.3, the implementation is already based on libGIO; * after possibly handling command line arguments (which does not apply @@ -131,7 +133,11 @@ namespace ctrl { void UiManager::performMainLoop() { + facade_.reset (new Facade{globals_->uiBus_, *this}); + gtk_main(); // GTK event loop + + facade_.reset (nullptr); } diff --git a/src/gui/ctrl/ui-manager.hpp b/src/gui/ctrl/ui-manager.hpp index 3a8048756..72b653165 100644 --- a/src/gui/ctrl/ui-manager.hpp +++ b/src/gui/ctrl/ui-manager.hpp @@ -66,6 +66,7 @@ namespace ctrl { class GlobalCtx; class Actions; + class Facade; @@ -96,6 +97,7 @@ namespace ctrl { { unique_ptr globals_; unique_ptr actions_; + unique_ptr facade_; unique_ptr styleManager_; diff --git a/src/gui/display-service.cpp b/src/gui/display-service.cpp index 475d9dc3b..7a6e07211 100644 --- a/src/gui/display-service.cpp +++ b/src/gui/display-service.cpp @@ -25,6 +25,10 @@ ** Implementation of _Displayer Service,_ exposed as a public interface. ** This service allows to set up _display slots,_ which can be handed over ** to client code in the course of the play process for outputting frames. + ** + ** @deprecated This is a first draft as of 1/2009, and likely to be superseded + ** by a better design, where rather the \em provider of an output facility + ** registers with the OutputManager in the core. */ diff --git a/src/gui/display-service.hpp b/src/gui/display-service.hpp index e25446432..e7750bb3f 100644 --- a/src/gui/display-service.hpp +++ b/src/gui/display-service.hpp @@ -35,6 +35,10 @@ ** LumieraDisplaySlot passed as parameter when initiating a playback process from the GUI. Using ** this LumieraDisplaySlot handle, clients should then use lumiera::DummyPlayer#facade to access ** an implementation instance of this service in order to push actual frames up. + ** + ** @deprecated This is a first draft as of 1/2009, and likely to be superseded + ** by a better design, where rather the \em provider of an output facility + ** registers with the OutputManager in the core. ** ** @see lumiera::Display ** @see lumiera::DummyPlayer diff --git a/src/gui/notification-service.cpp b/src/gui/notification-service.cpp index a88ceb456..cab5812ba 100644 --- a/src/gui/notification-service.cpp +++ b/src/gui/notification-service.cpp @@ -29,14 +29,11 @@ ** */ -/** @file notification-service.cpp - ** @todo write documentation for the notification service - */ +#include "gui/ctrl/ui-manager.hpp" #include "gui/notification-service.hpp" #include "include/logging.h" -#include "lib/depend.hpp" #include "lib/util.hpp" extern "C" { @@ -46,21 +43,20 @@ extern "C" { #include +using lib::diff::TreeMutator; +using gui::ctrl::BusTerm; +using std::string; +using util::cStr; namespace gui { - using lib::diff::TreeMutator; - using gui::ctrl::BusTerm; - using std::string; - using util::cStr; - void NotificationService::displayInfo (string const& text) { INFO (gui, "@GUI: display '%s' as notification message.", cStr(text)); - ////////////////////////TODO actually push the information to the GUI + ////////////////////////TODO actually push the information to the GUI ///////////////////////////////////TICKET #1098 : use a suitable Dispatcher } @@ -68,7 +64,7 @@ namespace gui { NotificationService::markError (ID uiElement, string const& text) { UNIMPLEMENTED ("send a error mark message via UI-Bus"); - ////////////////////////TODO actually push the information to the GUI + ////////////////////////TODO actually push the information to the GUI ///////////////////////////////////TICKET #1098 : use a suitable Dispatcher } @@ -76,7 +72,7 @@ namespace gui { NotificationService::markNote (ID uiElement, string const& text) { UNIMPLEMENTED ("send a info/warning message via UI-Bus"); - ////////////////////////TODO actually push the information to the GUI + ////////////////////////TODO actually push the information to the GUI ///////////////////////////////////TICKET #1098 : use a suitable Dispatcher } @@ -84,7 +80,7 @@ namespace gui { NotificationService::mutate (ID uiElement, DiffMessage&) /////////////////////////////////////////////////TICKET #1066 : how to pass a diff message { UNIMPLEMENTED ("actually produce a MutationMessage on the UI-Bus"); ///////////////////////////////////TICKET #1066 : how to build and pass a MutationMessage - ////////////////////////TODO actually push the information to the GUI + ////////////////////////TODO actually push the information to the GUI ///////////////////////////////////TICKET #1098 : use a suitable Dispatcher } @@ -92,7 +88,9 @@ namespace gui { NotificationService::triggerGuiShutdown (string const& cause) { NOTICE (gui, "@GUI: shutdown triggered with explanation '%s'....", cStr(cause)); - TODO ("actually request a shutdown from the GUI"); + TODO ("actually request a shutdown from the GUI"); ///////////////////////////////////TICKET #1098 : use a suitable Dispatcher + // NOTE basically this means to issue the call... + // uiManager_.terminateUI(); } @@ -240,8 +238,9 @@ namespace gui { * Yet this simple connection is sufficient to implement this service by talking * to other facilities within the UI layer. */ - NotificationService::NotificationService (ctrl::BusTerm& upLink) + NotificationService::NotificationService (ctrl::BusTerm& upLink, ctrl::UiManager& uiManager) : BusTerm{lib::idi::EntryID{}, upLink} + , uiManager_{uiManager} , implInstance_(this,_instance) , serviceInstance_( LUMIERA_INTERFACE_REF (lumieraorg_GuiNotification, 0,lumieraorg_GuiNotificationService)) { diff --git a/src/gui/notification-service.hpp b/src/gui/notification-service.hpp index b7fac6d40..8a37bcd1d 100644 --- a/src/gui/notification-service.hpp +++ b/src/gui/notification-service.hpp @@ -48,6 +48,10 @@ namespace gui { + namespace ctrl { + class UiManager; + } + /**************************************************//** @@ -82,6 +86,7 @@ namespace gui { using ServiceInstanceHandle = lumiera::InstanceHandle< LUMIERA_INTERFACE_INAME(lumieraorg_GuiNotification, 0) , GuiNotification > ; + ctrl::UiManager& uiManager_; lib::SingletonRef implInstance_; ServiceInstanceHandle serviceInstance_; @@ -89,7 +94,7 @@ namespace gui { public: - NotificationService (ctrl::BusTerm& upLink); + NotificationService (ctrl::BusTerm& upLink, ctrl::UiManager& uiManager); }; diff --git a/src/include/display-facade.h b/src/include/display-facade.h index 1b729430a..13e14eb90 100644 --- a/src/include/display-facade.h +++ b/src/include/display-facade.h @@ -29,10 +29,11 @@ ** In the Dummy-Player design study, the lumiera::Display interface serves ** to hand over calculated frames to the GUI for displaying them in a viewer. ** - ** This is a first draft as of 1/2009, and likely to be superseded by a - ** better design, where rather the \em provider of an output facility - ** registers with the OutputManager in the core. + ** @deprecated This is a first draft as of 1/2009, and likely to be superseded + ** by a better design, where rather the \em provider of an output facility + ** registers with the OutputManager in the core. ** + ** @see [corresponding implementation](display-service.hpp) ** @see gui::GuiFacade ** @see dummy-player-facade.h ** diff --git a/src/proc/control/proc-dispatcher.cpp b/src/proc/control/proc-dispatcher.cpp index b2aa7283d..5554cce84 100644 --- a/src/proc/control/proc-dispatcher.cpp +++ b/src/proc/control/proc-dispatcher.cpp @@ -37,9 +37,9 @@ ** to manage this process is the concern of the »Session Subsystem«. ** ** Closing a session blocks further command processing, while the lifecycle of the _Session Subsystem_ is - ** actually linked to _running the \ref DispatcherLoop_ -- implementation logic defined in this translation - ** unit here. This loop implementation is performed in a dedicated thread, _the Session Loop Thread._ And - ** this also entails opening the public SessionCommandService interface. + ** actually linked to _running the \ref DispatcherLoop_ -- implementation logic defined within this + ** translation unit. This loop implementation is performed in a dedicated thread, _the Session Loop Thread._ + ** And this also entails opening the public SessionCommandService interface. ** ** ## Loop operation control ** The loop starts with a blocking wait state, bound to the condition Looper::requireAction. Here, Looper diff --git a/wiki/renderengine.html b/wiki/renderengine.html index 79cbebe36..011edf3c1 100644 --- a/wiki/renderengine.html +++ b/wiki/renderengine.html @@ -2873,9 +2873,10 @@ In order to build a sensible plan for our timeline structure, we need to investi }}} -
+
special LayerSeparationInterface which serves the main purpose to load the GuiStarterPlugin, thus bringing up the Lumiera GTK UI at application start.
-
+ +It is of no further relevance beyond management of subsystem lifecycle -- which in itself is treated in Lumiera as a mere implementation concern and is not accessible by general application logic. Thus, the UI is largely independent and will be actively accessing the other parts of the application, while these in turn need to go through the public UI façades, esp. the GuiNotificationFacade for any active access to the UI and presentation layer. 
Considering how to interface to and integrate with the GUI Layer. Running the GUI is //optional,// but it requires to be [[started up|GuiStart]], installing the necessary LayerSeparationInterfaces. Probably the most important aspect regarding the GUI integration is how to get [[access to and operate|GuiConnection]] on the [[Session|SessionInterface]].
@@ -2984,12 +2985,17 @@ This treatment ensures each nested diff is consumed within a properly typed cont
 &rarr; this is the purpose of the {{{DataCap}}} within our [[generic node element|GenNode]]
-
+
LayerSeparationInterface provided by the GUI.
 Access point for the lower layers to push information and state changes (asynchronously) to the GUI. Most operations within Lumiera are in fact initiated by the user through the GUI. In the course of such actions, the GUI uses the services of the lower layer and typically receives an immediate synchronous response to indicate the change was noted. Yet often, these operations may cause additional changes to happen asynchronously from the GUI's perspective. For example, an edit operation might trigger a re-build of the low-level model, which then detects an error. Any such consequences and notifications can be "cast" up into the UI, using this service here.
 
 Beyond that, a call to trigger shutdown of the UI layer is also exposed here.
 
+!Lifecycle and Threading concerns
+The GuiNotificationFacade is installed as part of building the UI-Bus, which happens while establishing the GuiTopLevel. Yet there is a specific twist, insofar GTK is ''not threadsafe by design''. Any event handling and presentation changes will happen from within a dedicated UI event loop, which need to be explicitly started, after all of the primary windows and widgets are created and wired. Only after this point the UI becomes //life.//
+
+A dedicated ''activation state'' is necessary for this reason -- which within the implementation translate into a queuing and dispatching facility to reschedule any calls into the UI event thread ''asynchronously''.
+
 !Addressing of UI elements
 Most calls in this interface require to specify a receiver or target, in the form of an element ID. It is assumed the caller effectively just knows these IDs, typically because the same IDs are also used as element IDs for the corresponding session entities. And, to start with, the whole UI representation of the session was at some point generated by //population diff messages,// which used the same IDs to indicate the creation of the corresponding UI representation elements.
diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index ebfe6fdeb..e5435008b 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -259,7 +259,7 @@ - + @@ -291,38 +291,64 @@ - - + + + - + - + + + - + - + - - - + + + + + + - - - + + + + + + + + + + + + + + + + + + + + + + - - + + + @@ -360,15 +386,115 @@ - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ GtkLumiera in Konstruktion +

+ + + + + + + + + + +

+ NotificationFacade noch nicht offen +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ müssen eigens aktiviert werden +

+ + + + + + + + + + - - @@ -379,12 +505,16 @@ - + + + + + - + @@ -411,6 +541,7 @@ + @@ -612,18 +743,62 @@ - - - - - - + + + + + + + + + + + +

+ weil die Abstraktion "UI-Element" eben grade +

+

+ die UI-internen Framework-Aspekte ausklammert. +

+

+ +

+

+ Die Elemente stellen eine Abbildung der Strukturen aus der Session dar, +

+

+ und ihre "Methoden" sind Commands auf der Session! +

+ + + + + + + + + + + + + +

+ muß eigens aktiviert werden +

+ + + + + + + + @@ -640,35 +815,8 @@

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + @@ -680,7 +828,161 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ ...das war genau der Kern der "Plugin-Debatte". +

+

+ Eine solche globale, flache, dynamisch gebundene Ebene +

+

+ klingt nach wahnsinnigen Möglichkeiten, aber nur solange, bis man sich +

+

+ eine einzige Funktion konkret durchdenkt: es läuft auf Spaghetti-Code hinaus +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +

+ jede Facade-Funktion brauch einen Dispatcher +

+

+ Das wird eine ganze Me +

+ + + + + + + + + + + + + + + + + + +

+ bevor die Facade geöffnet wir +

+ + + + + + + + + + + + + +

+ und arbeitet asynchron +

+ + + + + + + + + + +

+ Argument-Storage +

+

+ organisieren +

+ + + + + + + + + + + @@ -851,10 +1153,10 @@ - + - + @@ -905,8 +1207,15 @@ - + + + + + + + + @@ -1997,8 +2306,8 @@ - - + + @@ -2592,7 +2901,7 @@ - + @@ -3011,8 +3320,9 @@ - + + @@ -11196,7 +11506,7 @@ - + @@ -16911,6 +17221,99 @@ + + + + + + + + + + + + + + + +

+ der springende Punkt mit sigc::trackable ist, +

+

+ daß Desktuktoren automatisch die Signale abkoppeln. +

+

+ Dieser Vorgang ist nicht threadsafe. Folglich müssen +

+

+ auch die Destruktoen im GUI-Thread laufen. +

+

+ Das ist eine subtile Falle. +

+ + + + + + + + + + + +

+ alles was von sigc::trackable erbt +

+ + + + + + + + + + + + + + +

+ für alles aus GTKmm zu verwenden +

+ + + + + + + + +

+ ...gemeint ist: +

+

+ alles das nicht aus dem GUI-Thread heraus geschieht +

+ + + + + + + + + + + + + + + + + +