Refactor PICKER_TOOL and push up snapping disable functionality

Fixes https://gitlab.com/kicad/code/kicad/-/issues/7348

common/tool/picker_tool.cpp

@@ -28,11 +28,30 @@
 #include <eda_draw_frame.h>
 
 
+void PICKER_TOOL_BASE::reset()
+{
+    m_cursor = KICURSOR::ARROW;
+    m_snap   = true;
+
+    m_picked = NULLOPT;
+    m_clickHandler = NULLOPT;
+    m_motionHandler = NULLOPT;
+    m_cancelHandler = NULLOPT;
+    m_finalizeHandler = NULLOPT;
+}
+
+
+PICKER_TOOL::PICKER_TOOL( const std::string& aName ) :
+        TOOL_INTERACTIVE( aName ),
+        PICKER_TOOL_BASE()
+{
+}
+
+
 PICKER_TOOL::PICKER_TOOL() :
         TOOL_INTERACTIVE( "common.InteractivePicker" ),
-        m_frame( nullptr )
+        PICKER_TOOL_BASE()
 {
-    resetPicker();
 }
 
 
@@ -76,7 +95,7 @@ int PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
     while( TOOL_EVENT* evt = Wait() )
     {
         setCursor();
-        VECTOR2D cursorPos = controls->GetCursorPosition( m_frame->IsGridVisible() );
+        VECTOR2D cursorPos = controls->GetCursorPosition( m_snap && m_frame->IsGridVisible() );
 
         if( evt->IsCancelInteractive() || evt->IsActivate() )
         {
@@ -173,7 +192,7 @@ int PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
         }
     }
 
-    resetPicker();
+    reset();
     controls->ForceCursorPosition( false );
     m_frame->PopTool( tool );
     return 0;
@@ -186,18 +205,6 @@ void PICKER_TOOL::setTransitions()
 }
 
 
-void PICKER_TOOL::resetPicker()
-{
-    m_cursor = KICURSOR::ARROW;
-
-    m_picked = NULLOPT;
-    m_clickHandler = NULLOPT;
-    m_motionHandler = NULLOPT;
-    m_cancelHandler = NULLOPT;
-    m_finalizeHandler = NULLOPT;
-}
-
-
 void PICKER_TOOL::setControls()
 {
     KIGFX::VIEW_CONTROLS* controls = getViewControls();

eeschema/tools/sch_edit_tool.cpp

@@ -1088,6 +1088,7 @@ int SCH_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent )
     Activate();
 
     picker->SetCursor( KICURSOR::REMOVE );
+    picker->SetSnapping( false );
 
     picker->SetClickHandler(
             [this]( const VECTOR2D& aPosition ) -> bool

eeschema/tools/sch_editor_control.cpp

@@ -617,6 +617,7 @@ int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent )
     Activate();
 
     picker->SetCursor( KICURSOR::VOLTAGE_PROBE );
+    picker->SetSnapping( false );
 
     picker->SetClickHandler(
             [this, simFrame]( const VECTOR2D& aPosition )
@@ -737,6 +738,7 @@ int SCH_EDITOR_CONTROL::SimTune( const TOOL_EVENT& aEvent )
     Activate();
 
     picker->SetCursor( KICURSOR::TUNE );
+    picker->SetSnapping( false );
 
     picker->SetClickHandler(
             [this]( const VECTOR2D& aPosition )
@@ -1131,6 +1133,7 @@ int SCH_EDITOR_CONTROL::HighlightNetCursor( const TOOL_EVENT& aEvent )
     Activate();
 
     picker->SetCursor( KICURSOR::BULLSEYE );
+    picker->SetSnapping( false );
 
     picker->SetClickHandler(
         [this] ( const VECTOR2D& aPos )

include/tool/picker_tool.h

@@ -33,18 +33,9 @@
 class EDA_DRAW_FRAME;
 
 
-class PICKER_TOOL : public TOOL_INTERACTIVE
+class PICKER_TOOL_BASE
 {
 public:
-    PICKER_TOOL();
-    ~PICKER_TOOL() {}
-
-    /// @copydoc TOOL_INTERACTIVE::Init()
-    bool Init() override;
-
-    /// @copydoc TOOL_INTERACTIVE::Reset()
-    void Reset( RESET_REASON aReason ) override { }
-
     ///< Event handler types.
     typedef std::function<bool(const VECTOR2D&)> CLICK_HANDLER;
     typedef std::function<void(const VECTOR2D&)> MOTION_HANDLER;
@@ -60,11 +51,16 @@ public:
         EXCEPTION_CANCEL
     };
 
-    ///< Main event loop.
+    PICKER_TOOL_BASE() :
-    int Main( const TOOL_EVENT& aEvent );
+            m_frame( nullptr )
+    {
+        reset();
+    }
 
     inline void SetCursor( KICURSOR aCursor ) { m_cursor = aCursor; }
 
+    inline void SetSnapping( bool aSnap ) { m_snap = aSnap; }
+
     /**
      * Set a handler for mouse click event.
      *
@@ -107,19 +103,13 @@ public:
         m_finalizeHandler = aHandler;
     }
 
-private:
+protected:
     ///< Reinitializes tool to its initial state.
-    void resetPicker();
+    virtual void reset();
 
-    ///< Applies the requested VIEW_CONTROLS settings.
-    void setControls();
-
-    ///< @copydoc TOOL_INTERACTIVE::setTransitions();
-    void setTransitions() override;
-
-private:
     EDA_DRAW_FRAME* m_frame;
     KICURSOR        m_cursor;
+    bool            m_snap;
 
     OPT<CLICK_HANDLER>    m_clickHandler;
     OPT<MOTION_HANDLER>   m_motionHandler;
@@ -129,4 +119,31 @@ private:
     OPT<VECTOR2D>         m_picked;
 };
 
+
+class PICKER_TOOL : public TOOL_INTERACTIVE, public PICKER_TOOL_BASE
+{
+public:
+    PICKER_TOOL();
+
+    PICKER_TOOL( const std::string& aName );
+
+    virtual ~PICKER_TOOL() = default;
+
+    /// @copydoc TOOL_INTERACTIVE::Init()
+    bool Init() override;
+
+    /// @copydoc TOOL_INTERACTIVE::Reset()
+    void Reset( RESET_REASON aReason ) override { }
+
+    ///< Main event loop.
+    int Main( const TOOL_EVENT& aEvent );
+
+protected:
+    ///< Applies the requested VIEW_CONTROLS settings.
+    void setControls();
+
+    ///< @copydoc TOOL_INTERACTIVE::setTransitions();
+    void setTransitions() override;
+};
+
 #endif /* PICKER_TOOL_H */

pcbnew/dialogs/dialog_group_properties.cpp

@@ -24,6 +24,8 @@
 #include <tool/tool_manager.h>
 #include <tools/pcb_actions.h>
 #include <tools/pcb_picker_tool.h>
+#include <pcb_base_edit_frame.h>
+#include <pcb_group.h>
 #include <status_popup.h>
 #include <board_commit.h>
 #include <bitmaps.h>

pcbnew/tools/pcb_picker_tool.cpp

@@ -31,10 +31,10 @@
 #include "pcb_selection_tool.h"
 
 
-PCB_PICKER_TOOL::PCB_PICKER_TOOL()
+PCB_PICKER_TOOL::PCB_PICKER_TOOL() :
-    : PCB_TOOL_BASE( "pcbnew.InteractivePicker" )
+        PCB_TOOL_BASE( "pcbnew.InteractivePicker" ),
+        PICKER_TOOL_BASE()
 {
-    reset();
 }
 
 
@@ -185,14 +185,7 @@ void PCB_PICKER_TOOL::setTransitions()
 void PCB_PICKER_TOOL::reset()
 {
     m_layerMask = LSET::AllLayersMask();
-    m_cursor    = KICURSOR::ARROW;
+    PICKER_TOOL_BASE::reset();
-    m_snap      = true;
-
-    m_picked = NULLOPT;
-    m_clickHandler = NULLOPT;
-    m_motionHandler = NULLOPT;
-    m_cancelHandler = NULLOPT;
-    m_finalizeHandler = NULLOPT;
 }
 
 

pcbnew/tools/pcb_picker_tool.h

@@ -27,35 +27,18 @@
 #ifndef PCB_PICKER_TOOL_H
 #define PCB_PICKER_TOOL_H
 
-#include <boost/optional/optional.hpp>
+#include <layers_id_colors_and_visibility.h>
+#include <tool/picker_tool.h>
 #include <tools/pcb_tool_base.h>
 
 /**
  *Generic tool for picking an item.
  */
-class PCB_PICKER_TOOL : public PCB_TOOL_BASE
+class PCB_PICKER_TOOL : public PCB_TOOL_BASE, public PICKER_TOOL_BASE
 {
 public:
     PCB_PICKER_TOOL();
-    ~PCB_PICKER_TOOL() override { }
+    virtual ~PCB_PICKER_TOOL() = default;
-
-    ///< Event handler types.
-    typedef std::function<bool(const VECTOR2D&)> CLICK_HANDLER;
-    typedef std::function<void(const VECTOR2D&)> MOTION_HANDLER;
-    typedef std::function<void(void)> CANCEL_HANDLER;
-    typedef std::function<void(const int&)> FINALIZE_HANDLER;
-
-    enum pickerEndState
-    {
-        WAIT_CANCEL,
-        CLICK_CANCEL,
-        END_ACTIVATE,
-        EVT_CANCEL,
-        EXCEPTION_CANCEL
-    };
-
-    ///< @copydoc TOOL_INTERACTIVE::Reset()
-    void Reset( RESET_REASON aReason ) override {}
 
     ///< Main event loop.
     int Main( const TOOL_EVENT& aEvent );
@@ -65,67 +48,19 @@ public:
      */
     inline void SetLayerSet( LSET aLayerSet ) { m_layerMask = aLayerSet; }
 
-    inline void SetCursor( KICURSOR aCursor ) { m_cursor = aCursor; }
+protected:
-
-    inline void SetSnapping( bool aSnap ) { m_snap = aSnap; }
-    /**
-     * Set a handler for mouse click event. Handler may decide to receive further click by
-     * returning true.
-     */
-    inline void SetClickHandler( CLICK_HANDLER aHandler )
-    {
-        wxASSERT( !m_clickHandler );
-        m_clickHandler = aHandler;
-    }
-
-    /**
-     * Set a handler for mouse motion.  Used for roll-over highlighting.
-     */
-    inline void SetMotionHandler( MOTION_HANDLER aHandler )
-    {
-        wxASSERT( !m_motionHandler );
-        m_motionHandler = aHandler;
-    }
-
-    /**
-     * Set a handler for cancel events (ESC or context-menu Cancel).
-     */
-    inline void SetCancelHandler( CANCEL_HANDLER aHandler )
-    {
-        wxASSERT( !m_cancelHandler );
-        m_cancelHandler = aHandler;
-    }
-
-    /**
-     * Set a handler for the finalize event. Takes the state of the exit from the Main loop.
-     */
-    inline void SetFinalizeHandler( FINALIZE_HANDLER aHandler )
-    {
-        wxASSERT( !m_finalizeHandler );
-        m_finalizeHandler = aHandler;
-    }
-
     ///< @copydoc TOOL_INTERACTIVE::setTransitions();
     void setTransitions() override;
 
-    ///< Reinitialize tool to its initial state.
+    ///< Applies the requested VIEW_CONTROLS settings.
-    void reset();
-
-    ///< Apply the requested VIEW_CONTROLS settings.
     void setControls();
 
+    ///< Reinitialize tool to its initial state.
+    void reset() override;
+
 private:
     ///< The layer set to use for optional snapping.
     LSET                  m_layerMask;
-    KICURSOR              m_cursor;
-    bool                  m_snap;
-
-    OPT<CLICK_HANDLER>    m_clickHandler;
-    OPT<MOTION_HANDLER>   m_motionHandler;
-    OPT<CANCEL_HANDLER>   m_cancelHandler;
-    OPT<FINALIZE_HANDLER> m_finalizeHandler;
-
-    OPT<VECTOR2D>         m_picked;
 };
 
 #endif /* PCB_PICKER_TOOL_H */