DrawSketchHandler: activation and deactivation via NVI pattern

==============================================================

With the introduction of overridable default behaviour for commands, it makes sense to keep control of the execution path of the activation and deactivation.

Non-virtual interface pattern enforces execution control of the virtual functions, while allowing the behaviour to be overriden in a case by case basis.

src/Mod/Sketcher/Gui/CommandConstraints.cpp

@@ -687,51 +687,14 @@ public:
         Gui::Selection().rmvSelectionGate();
     }
 
-    virtual void activated(ViewProviderSketch *)
-    {
-        selFilterGate = new GenericConstraintSelection(sketchgui->getObject());
+    virtual void mouseMove(Base::Vector2d /*onSketchPos*/) override {}
 
-        resetOngoingSequences();
-
-        selSeq.clear();
-
-        Gui::Selection().rmvSelectionGate();
-        Gui::Selection().addSelectionGate(selFilterGate);
-
-        // Constrain icon size in px
-        qreal pixelRatio = devicePixelRatio();
-        const unsigned long defaultCrosshairColor = 0xFFFFFF;
-        unsigned long color = getCrosshairColor();
-        auto colorMapping = std::map<unsigned long, unsigned long>();
-        colorMapping[defaultCrosshairColor] = color;
-
-        qreal fullIconWidth = 32 * pixelRatio;
-        qreal iconWidth = 16 * pixelRatio;
-        QPixmap cursorPixmap = Gui::BitmapFactory().pixmapFromSvg("Sketcher_Crosshair", QSizeF(fullIconWidth, fullIconWidth), colorMapping),
-                icon = Gui::BitmapFactory().pixmapFromSvg(cmd->getPixmap(), QSizeF(iconWidth, iconWidth));
-        QPainter cursorPainter;
-        cursorPainter.begin(&cursorPixmap);
-        cursorPainter.drawPixmap(16 * pixelRatio, 16 * pixelRatio, icon);
-        cursorPainter.end();
-        int hotX = 8;
-        int hotY = 8;
-        cursorPixmap.setDevicePixelRatio(pixelRatio);
-        // only X11 needs hot point coordinates to be scaled
-        if (qGuiApp->platformName() == QLatin1String("xcb")) {
-            hotX *= pixelRatio;
-            hotY *= pixelRatio;
-        }
-        setCursor(cursorPixmap, hotX, hotY, false);
-    }
-
-    virtual void mouseMove(Base::Vector2d /*onSketchPos*/) {}
-
-    virtual bool pressButton(Base::Vector2d /*onSketchPos*/)
+    virtual bool pressButton(Base::Vector2d /*onSketchPos*/) override
     {
         return true;
     }
 
-    virtual bool releaseButton(Base::Vector2d onSketchPos)
+    virtual bool releaseButton(Base::Vector2d onSketchPos) override
     {
         SelIdPair selIdPair;
         selIdPair.GeoId = GeoEnum::GeoUndef;
@@ -821,6 +784,44 @@ public:
         return true;
     }
 
+private:
+    virtual void activated() override
+    {
+        selFilterGate = new GenericConstraintSelection(sketchgui->getObject());
+
+        resetOngoingSequences();
+
+        selSeq.clear();
+
+        Gui::Selection().rmvSelectionGate();
+        Gui::Selection().addSelectionGate(selFilterGate);
+
+        // Constrain icon size in px
+        qreal pixelRatio = devicePixelRatio();
+        const unsigned long defaultCrosshairColor = 0xFFFFFF;
+        unsigned long color = getCrosshairColor();
+        auto colorMapping = std::map<unsigned long, unsigned long>();
+        colorMapping[defaultCrosshairColor] = color;
+
+        qreal fullIconWidth = 32 * pixelRatio;
+        qreal iconWidth = 16 * pixelRatio;
+        QPixmap cursorPixmap = Gui::BitmapFactory().pixmapFromSvg("Sketcher_Crosshair", QSizeF(fullIconWidth, fullIconWidth), colorMapping),
+                icon = Gui::BitmapFactory().pixmapFromSvg(cmd->getPixmap(), QSizeF(iconWidth, iconWidth));
+        QPainter cursorPainter;
+        cursorPainter.begin(&cursorPixmap);
+        cursorPainter.drawPixmap(16 * pixelRatio, 16 * pixelRatio, icon);
+        cursorPainter.end();
+        int hotX = 8;
+        int hotY = 8;
+        cursorPixmap.setDevicePixelRatio(pixelRatio);
+        // only X11 needs hot point coordinates to be scaled
+        if (qGuiApp->platformName() == QLatin1String("xcb")) {
+            hotX *= pixelRatio;
+            hotY *= pixelRatio;
+        }
+        setCursor(cursorPixmap, hotX, hotY, false);
+    }
+
 protected:
     CmdSketcherConstraint* cmd;
 
@@ -1788,26 +1789,15 @@ public:
         Gui::Selection().rmvSelectionGate();
     }
 
-    virtual void activated(ViewProviderSketch *)
-    {
-        Gui::Selection().rmvSelectionGate();
-        GenericConstraintSelection* selFilterGate = new GenericConstraintSelection(sketchgui->getObject());
-        selFilterGate->setAllowedSelTypes(SelVertex|SelRoot);
-        Gui::Selection().addSelectionGate(selFilterGate);
-        int hotX = 8;
-        int hotY = 8;
-        setCursor(QPixmap(cursor_createcoincident), hotX, hotY);
-    }
+    virtual void mouseMove(Base::Vector2d onSketchPos) override {Q_UNUSED(onSketchPos);}
 
-    virtual void mouseMove(Base::Vector2d onSketchPos) {Q_UNUSED(onSketchPos);}
-
-    virtual bool pressButton(Base::Vector2d onSketchPos)
+    virtual bool pressButton(Base::Vector2d onSketchPos) override
     {
         Q_UNUSED(onSketchPos);
         return true;
     }
 
-    virtual bool releaseButton(Base::Vector2d onSketchPos)
+    virtual bool releaseButton(Base::Vector2d onSketchPos) override
     {
         int VtId = getPreselectPoint();
         int CrsId = getPreselectCross();
@@ -1867,6 +1857,18 @@ public:
 
         return true;
     }
+private:
+    virtual void activated() override
+    {
+        Gui::Selection().rmvSelectionGate();
+        GenericConstraintSelection* selFilterGate = new GenericConstraintSelection(sketchgui->getObject());
+        selFilterGate->setAllowedSelTypes(SelVertex|SelRoot);
+        Gui::Selection().addSelectionGate(selFilterGate);
+        int hotX = 8;
+        int hotY = 8;
+        setCursor(QPixmap(cursor_createcoincident), hotX, hotY);
+    }
+
 protected:
     int GeoId1, GeoId2;
     Sketcher::PointPos PosId1, PosId2;

src/Mod/Sketcher/Gui/CommandSketcherTools.cpp

@@ -1319,14 +1319,7 @@ public:
         Snap5Degree
     };
 
-    virtual void activated(ViewProviderSketch *sketchgui)
-    {
-        setCursor(QPixmap(cursor_createcopy), 7, 7);
-        Origin = static_cast<Sketcher::SketchObject *>(sketchgui->getObject())->getPoint(OriginGeoId, OriginPos);
-        EditCurve[0] = Base::Vector2d(Origin.x, Origin.y);
-    }
-
-    virtual void mouseMove(Base::Vector2d onSketchPos)
+    virtual void mouseMove(Base::Vector2d onSketchPos) override
     {
         if (Mode == STATUS_SEEK_First) {
 
@@ -1359,7 +1352,7 @@ public:
         applyCursor();
     }
 
-    virtual bool pressButton(Base::Vector2d)
+    virtual bool pressButton(Base::Vector2d) override
     {
         if (Mode == STATUS_SEEK_First) {
             drawEdit(EditCurve);
@@ -1368,7 +1361,7 @@ public:
         return true;
     }
 
-    virtual bool releaseButton(Base::Vector2d onSketchPos)
+    virtual bool releaseButton(Base::Vector2d onSketchPos) override
     {
         Q_UNUSED(onSketchPos);
         if (Mode == STATUS_End)
@@ -1422,6 +1415,13 @@ public:
         }
         return true;
     }
+private:
+    virtual void activated() override
+    {
+        setCursor(QPixmap(cursor_createcopy), 7, 7);
+        Origin = static_cast<Sketcher::SketchObject *>(sketchgui->getObject())->getPoint(OriginGeoId, OriginPos);
+        EditCurve[0] = Base::Vector2d(Origin.x, Origin.y);
+    }
 protected:
     SelectMode Mode;
     SnapMode snapMode;
@@ -1868,14 +1868,7 @@ public:
         Snap5Degree
     };
 
-    virtual void activated(ViewProviderSketch *sketchgui)
-    {
-        setCursor(QPixmap(cursor_createrectangulararray), 7, 7);
-        Origin = static_cast<Sketcher::SketchObject *>(sketchgui->getObject())->getPoint(OriginGeoId, OriginPos);
-        EditCurve[0] = Base::Vector2d(Origin.x, Origin.y);
-    }
-
-    virtual void mouseMove(Base::Vector2d onSketchPos)
+    virtual void mouseMove(Base::Vector2d onSketchPos) override
     {
         if (Mode==STATUS_SEEK_First) {
 
@@ -1910,7 +1903,7 @@ public:
         applyCursor();
     }
 
-    virtual bool pressButton(Base::Vector2d)
+    virtual bool pressButton(Base::Vector2d) override
     {
         if (Mode == STATUS_SEEK_First) {
             drawEdit(EditCurve);
@@ -1919,7 +1912,7 @@ public:
         return true;
     }
 
-    virtual bool releaseButton(Base::Vector2d onSketchPos)
+    virtual bool releaseButton(Base::Vector2d onSketchPos) override
     {
         Q_UNUSED(onSketchPos);
         if (Mode == STATUS_End) {
@@ -1959,6 +1952,13 @@ public:
         }
         return true;
     }
+private:
+    virtual void activated() override
+    {
+        setCursor(QPixmap(cursor_createrectangulararray), 7, 7);
+        Origin = static_cast<Sketcher::SketchObject *>(sketchgui->getObject())->getPoint(OriginGeoId, OriginPos);
+        EditCurve[0] = Base::Vector2d(Origin.x, Origin.y);
+    }
 protected:
     SelectMode Mode;
     SnapMode snapMode;

src/Mod/Sketcher/Gui/DrawSketchHandler.cpp

@@ -121,14 +121,25 @@ DrawSketchHandler::DrawSketchHandler() : sketchgui(0) {}
 
 DrawSketchHandler::~DrawSketchHandler() {}
 
-void DrawSketchHandler::preActivated(ViewProviderSketch* vp)
+void DrawSketchHandler::activate(ViewProviderSketch * vp)
 {
-    ViewProviderSketchDrawSketchHandlerAttorney::setConstraintSelectability(*vp, false);
+    sketchgui = vp;
+
+    this->preActivated();
+    this->activated();
 }
 
-void DrawSketchHandler::postDeactivatedAlwaysRun(ViewProviderSketch* vp)
+void DrawSketchHandler::deactivate()
 {
-    ViewProviderSketchDrawSketchHandlerAttorney::setConstraintSelectability(*vp, true);
+    this->deactivated();
+    this->postDeactivated();
+    ViewProviderSketchDrawSketchHandlerAttorney::setConstraintSelectability(*sketchgui, true);
+    unsetCursor();
+}
+
+void DrawSketchHandler::preActivated()
+{
+    ViewProviderSketchDrawSketchHandlerAttorney::setConstraintSelectability(*sketchgui, false);
 }
 
 void DrawSketchHandler::quit(void)

src/Mod/Sketcher/Gui/DrawSketchHandler.h

@@ -97,11 +97,9 @@ public:
     DrawSketchHandler();
     virtual ~DrawSketchHandler();
 
-    virtual void preActivated(ViewProviderSketch *);
-    virtual void activated(ViewProviderSketch *){}
-    virtual void deactivated(ViewProviderSketch *){}
-    virtual void postDeactivated(ViewProviderSketch *){}
-    static void postDeactivatedAlwaysRun(ViewProviderSketch *);
+    void activate(ViewProviderSketch *);
+    void deactivate();
+
     virtual void mouseMove(Base::Vector2d onSketchPos)=0;
     virtual bool pressButton(Base::Vector2d onSketchPos)=0;
     virtual bool releaseButton(Base::Vector2d onSketchPos)=0;
@@ -130,6 +128,12 @@ public:
     void resetPositionText(void);
     void renderSuggestConstraintsCursor(std::vector<AutoConstraint> &suggestedConstraints);
 
+private:
+    virtual void preActivated();
+    virtual void activated(){}
+    virtual void deactivated(){}
+    virtual void postDeactivated(){}
+
 protected:
     // helpers
     /**

src/Mod/Sketcher/Gui/ViewProviderSketch.cpp

@@ -363,9 +363,7 @@ void ViewProviderSketch::activateHandler(DrawSketchHandler *newHandler)
 
     sketchHandler = std::unique_ptr<DrawSketchHandler>(newHandler);
     Mode = STATUS_SKETCH_UseHandler;
-    sketchHandler->sketchgui = this;
-    sketchHandler->preActivated(this);
-    sketchHandler->activated(this);
+    sketchHandler->activate(this);
 
     // make sure receiver has focus so immediately pressing Escape will be handled by
     // ViewProviderSketch::keyPressed() and dismiss the active handler, and not the entire
@@ -382,12 +380,9 @@ void ViewProviderSketch::deactivateHandler()
         editCurve.clear();
         drawEdit(editCurve); // erase any line
         resetPositionText();
-        sketchHandler->deactivated(this);
-        sketchHandler->postDeactivated(this);
-        sketchHandler->unsetCursor();
+        sketchHandler->deactivate();
         sketchHandler = nullptr;
     }
-    DrawSketchHandler::postDeactivatedAlwaysRun(this);
     Mode = STATUS_NONE;
 }
 
@@ -467,11 +462,13 @@ bool ViewProviderSketch::keyPressed(bool pressed, int key)
             }
             return false;
         }
+        break;
     case SoKeyboardEvent::LEFT_SHIFT:
         if (Mode < STATUS_SKETCH_UseHandler) {
             editCoinManager->setConstraintSelectability(!pressed);
             return true;
         }
+        [[fallthrough]];
     default:
         {
             if (isInEditMode() && sketchHandler)