Eeschema, selection tool: uniformize the cursor shape and the actual modifiers.

SHIFT, CTRL, ALT modifier keys were handled in 2 different codes, one for
the selection tool and another to modify the mouse cursor shape, with 2
different configs.
Now the modifiers are managed from only one function.
Pcbnew: group also SHIFT, CTRL, ALT modifier keys management to a specific function.
Fixes #8021
https://gitlab.com/kicad/code/kicad/issues/8021

eeschema/tools/ee_selection_tool.cpp

@@ -311,22 +311,13 @@ const KICAD_T movableSymbolItems[] =
 };
 
 
-int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
+void EE_SELECTION_TOOL::setModifiersState( bool aShiftState, bool aCtrlState, bool aAltState )
 {
-    m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+    // Set the configuration of m_additive, m_subtractive, m_exclusive_or
-
+    // from the state of modifier keys SHIFT, CTRL, ALT and the OS
-    KIID lastRolloverItem = niluuid;
-
-    // Main loop: keep receiving events
-    while( TOOL_EVENT* evt = Wait() )
-    {
-        bool displayWireCursor = false;
-        bool displayBusCursor = false;
-        bool displayLineCursor = false;
-        KIID rolloverItem = lastRolloverItem;
 
     // on left click, a selection is made, depending on modifiers ALT, SHIFT, CTRL:
-        // Due to the fact ALT key modifier cannot be useed freely on Winows and Linux,
+    // Due to the fact ALT key modifier cannot be useed freely on Windows and Linux,
     // actions are different on OSX and others OS
     // Especially, ALT key cannot be used to force showing the full selection choice
     // context menu (the menu is immediately closed on Windows )
@@ -350,43 +341,42 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
     // SHIFT+ALT (on others) = remove selected items to the current selection
 
 #ifdef __WXOSX_MAC__
-        m_subtractive = evt->Modifier( MD_CTRL ) &&
+    m_subtractive     = aCtrlState && aShiftState && !aAltState;
-                        evt->Modifier( MD_SHIFT ) &&
+    m_additive        = aShiftState && !aCtrlState && !aAltState;
-                        !evt->Modifier( MD_ALT );
+    m_exclusive_or    = aCtrlState && !aShiftState && !aAltState;
-
+    m_skip_heuristics = aAltState && !aShiftState && !aCtrlState;
-        m_additive = evt->Modifier( MD_SHIFT ) &&
-                     !evt->Modifier( MD_CTRL ) &&
-                     !evt->Modifier( MD_ALT );
-
-        m_exclusive_or = evt->Modifier( MD_CTRL ) &&
-                         !evt->Modifier( MD_SHIFT ) &&
-                         !evt->Modifier( MD_ALT );
-
-        m_skip_heuristics = evt->Modifier( MD_ALT ) &&
-                            !evt->Modifier( MD_SHIFT ) &&
-                            !evt->Modifier( MD_CTRL );
 
 #else
-        m_subtractive = evt->Modifier( MD_SHIFT )
+    m_subtractive  = aShiftState && !aCtrlState && aAltState;
-                        && !evt->Modifier( MD_CTRL )
+    m_additive     = aShiftState && !aCtrlState && !aAltState;
-                        && evt->Modifier( MD_ALT );
+    m_exclusive_or = !aShiftState && !aCtrlState && aAltState;
-
-        m_additive = evt->Modifier( MD_SHIFT )
-                     && !evt->Modifier( MD_CTRL )
-                     && !evt->Modifier( MD_ALT );
-
-        m_exclusive_or = !evt->Modifier( MD_SHIFT )
-                         && !evt->Modifier( MD_CTRL )
-                         && evt->Modifier( MD_ALT );
 
     // Is the user requesting that the selection list include all possible
     // items without removing less likely selection candidates
     // Cannot use the Alt key on windows or the disambiguation context menu is immediately
     // dismissed rendering it useless.
-        m_skip_heuristics = evt->Modifier( MD_CTRL )
+    m_skip_heuristics = aCtrlState && !aShiftState && !aAltState;
-                            && !evt->Modifier( MD_SHIFT )
-                            && !evt->Modifier( MD_ALT );
 #endif
+}
+
+
+int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
+{
+    m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW );
+
+    KIID lastRolloverItem = niluuid;
+
+    // Main loop: keep receiving events
+    while( TOOL_EVENT* evt = Wait() )
+    {
+        bool displayWireCursor = false;
+        bool displayBusCursor = false;
+        bool displayLineCursor = false;
+        KIID rolloverItem = lastRolloverItem;
+
+        // on left click, a selection is made, depending on modifiers ALT, SHIFT, CTRL:
+        setModifiersState( evt->Modifier( MD_SHIFT ), evt->Modifier( MD_CTRL ),
+                           evt->Modifier( MD_ALT ) );
 
         bool modifier_enabled = m_subtractive || m_additive || m_exclusive_or;
 
@@ -753,14 +743,8 @@ void EE_SELECTION_TOOL::OnIdle( wxIdleEvent& aEvent )
     {
         wxMouseState keyboardState = wxGetMouseState();
 
-        m_subtractive = m_additive = m_exclusive_or = false;
+        setModifiersState( keyboardState.ShiftDown(), keyboardState.ControlDown(),
-
+                           keyboardState.AltDown() );
-        if( keyboardState.ShiftDown() && keyboardState.ControlDown() )
-            m_subtractive = true;
-        else if( keyboardState.ShiftDown() )
-            m_additive = true;
-        else if( keyboardState.ControlDown() )
-            m_exclusive_or = true;
 
         if( m_additive )
             m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ADD );

eeschema/tools/ee_selection_tool.h

@@ -272,6 +272,12 @@ private:
     void setTransitions() override;
 
 private:
+    /**
+     * Set the configuration of m_additive, m_subtractive, m_exclusive_or, m_skip_heuristics
+     * from the state of modifier keys SHIFT, CTRL, ALT and depending on the OS
+     */
+    void setModifiersState( bool aShiftState, bool aCtrlState, bool aAltState );
+
     SCH_BASE_FRAME* m_frame;             // Pointer to the parent frame
     EE_SELECTION    m_selection;         // Current state of selection
 

pcbnew/tools/pcb_selection_tool.cpp

@@ -103,6 +103,7 @@ PCB_SELECTION_TOOL::PCB_SELECTION_TOOL() :
         m_exclusive_or( false ),
         m_multiple( false ),
         m_skip_heuristics( false ),
+        m_highlight_modifier( false ),
         m_enteredGroup( nullptr ),
         m_priv( std::make_unique<PRIV>() )
 {
@@ -206,13 +207,10 @@ void PCB_SELECTION_TOOL::Reset( RESET_REASON aReason )
 }
 
 
-int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
+void PCB_SELECTION_TOOL::setModifiersState( bool aShiftState, bool aCtrlState, bool aAltState )
 {
-    // Main loop: keep receiving events
+    // Set the configuration of m_additive, m_subtractive, m_exclusive_or
-    while( TOOL_EVENT* evt = Wait() )
+    // from the state of modifier keys SHIFT, CTRL, ALT and the OS
-    {
-        MOUSE_DRAG_ACTION dragAction = m_frame->GetDragAction();
-        TRACK_DRAG_ACTION trackDragAction = m_frame->Settings().m_TrackDragAction;
 
     // on left click, a selection is made, depending on modifiers ALT, SHIFT, CTRL:
     // Due to the fact ALT key modifier cannot be useed freely on Winows and Linux,
@@ -239,48 +237,39 @@ int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
     // SHIFT+ALT (on others) = remove selected items to the current selection
 
 #ifdef __WXOSX_MAC__
-        m_subtractive = evt->Modifier( MD_CTRL ) &&
+    m_subtractive        = aCtrlState && aShiftState && !aAltState;
-                        evt->Modifier( MD_SHIFT ) &&
+    m_additive           = aShiftState && !aCtrlState && !aAltState;
-                        !evt->Modifier( MD_ALT );
+    m_exclusive_or       = aCtrlState && !aShiftState && !aAltState;
+    m_skip_heuristics    = aAltState && !aShiftState && !aCtrlState;
+    m_highlight_modifier = aCtrlState && aAltState && !aShiftState;
 
-        m_additive = evt->Modifier( MD_SHIFT ) &&
-                     !evt->Modifier( MD_CTRL ) &&
-                     !evt->Modifier( MD_ALT );
-
-        m_exclusive_or = evt->Modifier( MD_CTRL ) &&
-                         !evt->Modifier( MD_SHIFT ) &&
-                         !evt->Modifier( MD_ALT );
-
-        m_skip_heuristics = evt->Modifier( MD_ALT ) &&
-                            !evt->Modifier( MD_SHIFT ) &&
-                            !evt->Modifier( MD_CTRL );
-
-        bool highlight_modifier = evt->Modifier( MD_CTRL )
-                                  && evt->Modifier( MD_ALT )
-                                  && !evt->Modifier( MD_SHIFT );
 #else
-        m_subtractive = evt->Modifier( MD_ALT ) &&
+    m_subtractive  = aShiftState && !aCtrlState && aAltState;
-                        evt->Modifier( MD_SHIFT ) &&
+    m_additive     = aShiftState && !aCtrlState && !aAltState;
-                        !evt->Modifier( MD_CTRL );
+    m_exclusive_or = !aShiftState && !aCtrlState && aAltState;
-
-        m_additive = evt->Modifier( MD_SHIFT ) &&
-                     !evt->Modifier( MD_ALT ) &&
-                     !evt->Modifier( MD_CTRL );
-
-        m_exclusive_or = evt->Modifier( MD_ALT ) &&
-                         !evt->Modifier( MD_SHIFT ) &&
-                         !evt->Modifier( MD_CTRL );
 
+    // Is the user requesting that the selection list include all possible
+    // items without removing less likely selection candidates
     // Cannot use the Alt key on windows or the disambiguation context menu is immediately
     // dismissed rendering it useless.
-        m_skip_heuristics = evt->Modifier( MD_CTRL ) &&
+    m_skip_heuristics = aCtrlState && !aShiftState && !aAltState;
-                            !evt->Modifier( MD_SHIFT ) &&
-                            !evt->Modifier( MD_ALT );
 
-        bool highlight_modifier = evt->Modifier( MD_CTRL )
+    m_highlight_modifier = aCtrlState && aShiftState && !aAltState;
-                                  && evt->Modifier( MD_SHIFT )
-                                  && !evt->Modifier( MD_ALT );
 #endif
+}
+
+
+int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
+{
+    // Main loop: keep receiving events
+    while( TOOL_EVENT* evt = Wait() )
+    {
+        MOUSE_DRAG_ACTION dragAction = m_frame->GetDragAction();
+        TRACK_DRAG_ACTION trackDragAction = m_frame->Settings().m_TrackDragAction;
+
+        // on left click, a selection is made, depending on modifiers ALT, SHIFT, CTRL:
+        setModifiersState( evt->Modifier( MD_SHIFT ), evt->Modifier( MD_CTRL ),
+                           evt->Modifier( MD_ALT ) );
 
         bool modifier_enabled = m_subtractive || m_additive || m_exclusive_or;
         PCB_BASE_FRAME* frame = getEditFrame<PCB_BASE_FRAME>();
@@ -295,7 +284,7 @@ int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
         // Single click? Select single object
         else if( evt->IsClick( BUT_LEFT ) )
         {
-            if( highlight_modifier && brd_editor )
+            if( m_highlight_modifier && brd_editor )
                 m_toolMgr->RunAction( PCB_ACTIONS::highlightNet, true );
             else
             {

pcbnew/tools/pcb_selection_tool.h

@@ -192,6 +192,12 @@ public:
     void FilterCollectedItems( GENERAL_COLLECTOR& aCollector );
 
 private:
+    /**
+     * Set the configuration of m_additive, m_subtractive, m_exclusive_or,
+     * m_skip_heuristics and m_highlight_modifier
+     * from the state of modifier keys SHIFT, CTRL, ALT and depending on the OS
+     */
+    void setModifiersState( bool aShiftState, bool aCtrlState, bool aAltState );
 
     /**
      * Select an item pointed by the parameter \a aWhere.
@@ -364,6 +370,7 @@ private:
     bool m_exclusive_or;          // Items' selection state should be toggled
     bool m_multiple;              // Multiple selection mode is active
     bool m_skip_heuristics;       // Heuristics are not allowed when choosing item under cursor
+    bool m_highlight_modifier;    // select highlight net on left click
 
     PCB_GROUP*        m_enteredGroup;          // If non-null, selections are limited to
                                                // members of this group