From 2e4cd56f4fdaaba16a107a5dce1b5730405a75e4 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 30 Jan 2021 13:29:50 +0100 Subject: [PATCH] Relative-Hook: rectify the design Partially as a leftover from the way more ambitious initial design, we ended up with CanvasHook as an elaboration/specialisation of the ViewHook abstraction. However, as it stands, this design is tilted, since CanvasHook is not just an elaboration, but rather a variation of the same basic idea. And this is now more like a building pattern and less of a generic framework, it seems adequate to separate these two variations completely, even if this incurs a small amount of code duplication. Actually this refactoring is necessary to resolve a bug, where we ended up with the same Clip widgets attached two times to the same Canvas control, one time through the ViewHook baseclass, and a second time by the ctor of the "derived" CanvasHook --- src/stage/model/canvas-hook.hpp | 67 ++++++++++++----------- src/stage/model/view-hook.hpp | 6 +- src/stage/timeline/body-canvas-widget.cpp | 6 -- src/stage/timeline/body-canvas-widget.hpp | 1 - src/stage/timeline/display-manager.hpp | 7 +-- src/stage/timeline/timeline-layout.hpp | 1 + tests/stage/model/canvas-hook-test.cpp | 11 ---- wiki/thinkPad.ichthyo.mm | 62 ++++++++++++++++++++- 8 files changed, 97 insertions(+), 64 deletions(-) diff --git a/src/stage/model/canvas-hook.hpp b/src/stage/model/canvas-hook.hpp index f4db3a1b9..242d4b206 100644 --- a/src/stage/model/canvas-hook.hpp +++ b/src/stage/model/canvas-hook.hpp @@ -23,12 +23,14 @@ /** @file canvas-hook.hpp ** Specialised (abstracted) presentation context with positioning by coordinates. - ** This extends the generic abstraction of a ViewHook, and works in a similar way, + ** This expands the idea behind the ViewHook abstraction, and works in a similar way, ** in close collaboration with the corresponding CanvasHooked entity (abstraction). ** Elements relying on those abstractions maintain an attachment to "their view", - ** while remaining agnostic about the view's implementation details. But the key - ** point with this extended abstraction is that elements can be placed onto a - ** coordinate system or canvas, and they can be moved to a different position. + ** while remaining agnostic about the view's implementation details. However, + ** the key point with this extended variant of the abstraction is that elements + ** can be placed onto a coordinate system or canvas, and they can be moved to a + ** different position. + ** ** A CanvasHooked element is basically a decorator directly attached to the ** element, adding automatic detachment on destruction, similar to a smart-ptr. ** So the "hooked" widget will live within the common allocation, together with @@ -50,7 +52,6 @@ #ifndef STAGE_MODEL_CANVAS_HOOK_H #define STAGE_MODEL_CANVAS_HOOK_H -#include "stage/model/view-hook.hpp" #include "lib/time/timevalue.hpp" #include "lib/nocopy.hpp" #include "lib/util.hpp" @@ -67,24 +68,25 @@ namespace model { /** * Interface to represent _"some presentation layout entity",_ - * with the ability to _place_ widgets (managed elsewhere), to - * relocate those widgets to another position, and to re-establish - * a different sequence of the widgets (whatever this means). + * with the ability to _place_ widgets (managed elsewhere) onto it, + * to relocate those widgets to another position. * @remark the canonical example is a _canvas widget,_ (e.g. `Gtk::Layout`), * allowing to attach child widgets at specific positions, together with * custom drawing. * @warning please ensure the CanvasHook outlives any attached CanvasHooked. + * @see ViewHook embodies the same scheme for widgets just "added" into + * the presentation without the notion of explicit coordinates. */ template class CanvasHook - : public ViewHook { public: virtual ~CanvasHook() { } ///< this is an interface - virtual void hook (WID& widget, int xPos, int yPos) =0; - virtual void move (WID& widget, int xPos, int yPos) =0; - + virtual void hook (WID& widget, int xPos, int yPos) =0; + virtual void move (WID& widget, int xPos, int yPos) =0; + virtual void remove (WID& widget) =0; + /** Anchor point to build chains of related View Hooks */ virtual CanvasHook& getAnchorHook() noexcept @@ -94,14 +96,14 @@ namespace model { struct Pos { - CanvasHook& view; + CanvasHook* view; int x,y; }; Pos hookedAt (int x, int y) { - return Pos{*this, x,y}; + return Pos{this, x,y}; } /** build the "construction hook" for a \ref ViewHooked element, @@ -118,20 +120,16 @@ namespace model { protected: /** extension point for time axis zoom management. */ virtual int translateTimeToPixels (Time) const =0; - - private: - /** adapted but also turned unaccessible; use #hook(WID&,int,int) */ - void hook (WID& w) override { hook (w,0,0); } }; /** * A widget attached onto a display canvas or similar central presentation context. - * This decorator is an extension to the ViewHooked decorator, and thus inherits from - * the widget to be attached, i.e. the widget itself becomes embedded; moreover, the attachment - * is immediately performed at construction time and managed automatically thereafter. When the - * `CanvasHooked` element goes out of scope, it is automatically detached from presentation. + * This decorator is a variation of the ViewHooked decorator, and likewise embodies + * the widget to be attached; moreover, the attachment is immediately performed at + * construction time and managed automatically thereafter. When the `CanvasHooked` + * element goes out of scope, it is automatically detached from presentation. * With the help of the CanvasHooked API, a widget (or similar entity) may control the coordinates * of its placement onto some kind of _canvas_ (-> `Gtk::Layout`), while remaining agnostic * regarding any further implementation details of the canvas and its placement thereon. @@ -150,28 +148,33 @@ namespace model { */ template class CanvasHooked - : public ViewHooked + : public WID + , util::NonCopyable { - using Hooked = ViewHooked; using Canvas = CanvasHook; + Canvas* view_; protected: - Canvas& - getCanvas() const - { - REQUIRE (INSTANCEOF(ViewHook, &Hooked::getView() )); - return static_cast (Hooked::getView()); - } + Canvas& getCanvas() const { return *view_; } public: template CanvasHooked (typename Canvas::Pos attachmentPos, ARGS&& ...args) - : ViewHooked{attachmentPos.view - ,std::forward(args)...} + : WID{std::forward(args)...} + , view_{attachmentPos.view} { getCanvas().hook (*this, attachmentPos.x, attachmentPos.y); } + ~CanvasHooked() noexcept + { + try { + if (view_) + view_->remove (*this); + } + ERROR_LOG_AND_IGNORE (progress, "Detaching of CanvasHooked widgets from the presentation") + } + void moveTo (int xPos, int yPos) { diff --git a/src/stage/model/view-hook.hpp b/src/stage/model/view-hook.hpp index ba0055270..49baf2c80 100644 --- a/src/stage/model/view-hook.hpp +++ b/src/stage/model/view-hook.hpp @@ -78,7 +78,7 @@ namespace model { * - a _canvas widget,_ (e.g. `Gtk::Layout`), allowing to attach * child widgets at specific positions, together with custom drawing. * @warning please ensure the ViewHook outlives any attached ViewHooked. - * @see CanvasHook extended interface to support positioning by coordinates + * @see CanvasHook similar interface to support positioning by coordinates */ template class ViewHook @@ -127,10 +127,6 @@ namespace model { using View = ViewHook; View* view_; - protected: - View& - getView() const { return *view_; } - public: template ViewHooked (View& view, ARGS&& ...args) diff --git a/src/stage/timeline/body-canvas-widget.cpp b/src/stage/timeline/body-canvas-widget.cpp index 3f7192255..6308e2b1a 100644 --- a/src/stage/timeline/body-canvas-widget.cpp +++ b/src/stage/timeline/body-canvas-widget.cpp @@ -561,12 +561,6 @@ namespace timeline { getCanvas(0).remove (widget); } - void - BodyCanvasWidget::rehook (Gtk::Widget&) noexcept - { - /* NOOP */ - } - void BodyCanvasWidget::move (Gtk::Widget& widget, int xPos, int yPos) { diff --git a/src/stage/timeline/body-canvas-widget.hpp b/src/stage/timeline/body-canvas-widget.hpp index 73c55d918..1e665cd04 100644 --- a/src/stage/timeline/body-canvas-widget.hpp +++ b/src/stage/timeline/body-canvas-widget.hpp @@ -175,7 +175,6 @@ namespace timeline { void hook (Gtk::Widget&, int xPos=0, int yPos=0) override; void move (Gtk::Widget&, int xPos, int yPos) override; void remove (Gtk::Widget&) override; - void rehook (Gtk::Widget&) noexcept override; int translateTimeToPixels (Time) const override; diff --git a/src/stage/timeline/display-manager.hpp b/src/stage/timeline/display-manager.hpp index 37bb313cd..7eaff13aa 100644 --- a/src/stage/timeline/display-manager.hpp +++ b/src/stage/timeline/display-manager.hpp @@ -74,6 +74,7 @@ #include "lib/error.hpp" #include "lib/nocopy.hpp" +#include "stage/model/view-hook.hpp" #include "stage/model/canvas-hook.hpp" #include "lib/util.hpp" @@ -122,12 +123,6 @@ namespace timeline { refHook_.remove (widget); } - void - rehook (WID& hookedWidget) noexcept override - { - refHook_.rehook (hookedWidget); - } - /** allow to build a derived ViewRefHook with different offset */ model::CanvasHook& getAnchorHook() noexcept override diff --git a/src/stage/timeline/timeline-layout.hpp b/src/stage/timeline/timeline-layout.hpp index 03d608e96..b8387be18 100644 --- a/src/stage/timeline/timeline-layout.hpp +++ b/src/stage/timeline/timeline-layout.hpp @@ -88,6 +88,7 @@ #include "stage/timeline/header-pane-widget.hpp" #include "stage/timeline/body-canvas-widget.hpp" #include "stage/model/canvas-hook.hpp" +#include "stage/model/view-hook.hpp" //#include "lib/util.hpp" diff --git a/tests/stage/model/canvas-hook-test.cpp b/tests/stage/model/canvas-hook-test.cpp index 862d07741..47326f6d8 100644 --- a/tests/stage/model/canvas-hook-test.cpp +++ b/tests/stage/model/canvas-hook-test.cpp @@ -150,17 +150,6 @@ namespace test { widgets_.remove_if ([&](Attachment const& a) { return a.widget == elm; }); } - - void - rehook (DummyWidget& existingHook) noexcept override - { - auto pos = findEntry (existingHook); - REQUIRE (pos != widgets_.end(), "the given iterator must yield previously hooked-up elements"); - Attachment existing{*pos}; - this->remove (existing.widget); - this->hook (existing.widget, existing.posX,existing.posY); - } - protected: int translateTimeToPixels (Time) const override diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index f280ed32f..6f048f2d6 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -19213,7 +19213,7 @@ - + @@ -19280,6 +19280,32 @@ + + + + + + +

+ aber dieses Design ist schief +

+ + + + + + + + + + + + + + + + + @@ -19708,6 +19734,37 @@ + + + + + + + + +

+ Ursache ist ein schiefes Design +

+ + + + + + + + +

+ ...und das ist wohl entstanden, weil ich ursprünglich einen generischen Visitor im Blick hatte; es hat sich aber dann gezeigt, daß eine solche universelle "Quer-Beweglichkeit" weder notwendig noch wünschenswert ist +

+ + + + + + + + + @@ -28945,8 +29002,7 @@ diese Alternative würde dann attraktiv, wenn es häufig vorkommt, daß zwischen einem Clip-Widget und einer anderen Repräsentation des ClipDelegate dynamisch hin- und hergeschaltet werden muß. Weil man dann den relativ schwergewichtigen Datencontainer einfach umhängen könnte

- - +