From 91421f9908892cb9a6efc8f211735e7c041d2ffe Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Wed, 28 Apr 2021 23:53:31 +0100
Subject: [PATCH] Separate logic for multi-select and click-select when
 filtering.

Also removes a bunch of old implementations of pad locking and
filtering which are no longer needed.  (They're now handled by the
uniform locking code.)

Also removes some of the auto-promotion logic.  Rotating a footprint
when a pad was selected is going to be surprising whether the pad
is locked or not.

Fixes https://gitlab.com/kicad/code/kicad/issues/8322
---
 pcbnew/board_commit.cpp             |   3 +-
 pcbnew/collectors.h                 |   2 +-
 pcbnew/tools/edit_tool.cpp          | 161 +++-------------------------
 pcbnew/tools/global_edit_tool.cpp   |   2 +-
 pcbnew/tools/pcb_selection_tool.cpp |  14 ++-
 pcbnew/tools/pcb_selection_tool.h   |   2 +-
 6 files changed, 30 insertions(+), 154 deletions(-)

diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp
index 2a04984b30e..94a924ac8c7 100644
--- a/pcbnew/board_commit.cpp
+++ b/pcbnew/board_commit.cpp
@@ -157,7 +157,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, bool aCreateUndoEntry, bool a
                     if( !( changeFlags & CHT_DONE ) )
                         board->Footprints().front()->Add( boardItem );
                 }
-                else if( boardItem->Type() == PCB_FP_TEXT_T ||
+                else if( boardItem->Type() == PCB_PAD_T ||
+                         boardItem->Type() == PCB_FP_TEXT_T ||
                          boardItem->Type() == PCB_FP_SHAPE_T ||
                          boardItem->Type() == PCB_FP_ZONE_T )
                 {
diff --git a/pcbnew/collectors.h b/pcbnew/collectors.h
index 125d4cb8ad3..186131877bf 100644
--- a/pcbnew/collectors.h
+++ b/pcbnew/collectors.h
@@ -437,7 +437,7 @@ public:
         m_ignoreLockedItems         = false;
 
 #if defined(USE_MATCH_LAYER)
-        m_IncludeSecondary          = false;
+        m_includeSecondary          = false;
 #else
         m_includeSecondary          = true;
 #endif
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 653e37c35e2..09a1f475462 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -286,8 +286,7 @@ int EDIT_TOOL::Drag( const TOOL_EVENT& aEvent )
                     BOARD_ITEM* item = aCollector[ i ];
 
                     // We don't operate on pads; convert them to footprint selections
-                    if( !sTool->IsFootprintEditor() && item->IsLocked() && item->Type() == PCB_PAD_T
-                            && !item->GetParent()->IsLocked() )
+                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T )
                     {
                         aCollector.Remove( item );
 
@@ -296,9 +295,9 @@ int EDIT_TOOL::Drag( const TOOL_EVENT& aEvent )
                     }
                 }
 
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             },
-            !m_isFootprintEditor /* prompt user regarding locked items */ );
+            true /* prompt user regarding locked items */ );
 
     if( selection.Empty() )
         return 0;
@@ -1413,35 +1412,9 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
     PCB_SELECTION& selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                std::set<BOARD_ITEM*> added_items;
-
-                // Iterate from the back so we don't have to worry about removals.
-                for( int i = aCollector.GetCount() - 1; i >= 0; --i )
-                {
-                    BOARD_ITEM* item = aCollector[i];
-
-                    // We don't operate on pads; convert them to footprint selections
-                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T
-                                    && !item->GetParent()->IsLocked() )
-                    {
-                        aCollector.Remove( item );
-
-                        if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                            added_items.insert( item->GetParent() );
-                    }
-
-                    // We can't rotate both a footprint and its text in the same operation, so if
-                    // both are selected, remove the text
-                    if( item->Type() == PCB_FP_TEXT_T && aCollector.HasItem( item->GetParent() ) )
-                        aCollector.Remove( item );
-                }
-
-                for( BOARD_ITEM* item : added_items )
-                    aCollector.Append( item );
-
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             },
-            !m_dragging && !m_isFootprintEditor /* prompt user regarding locked items */ );
+            !m_dragging /* prompt user regarding locked items */ );
 
     if( selection.Empty() )
         return 0;
@@ -1553,30 +1526,9 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
     PCB_SELECTION& selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                std::set<BOARD_ITEM*> added_items;
-
-                // Iterate from the back so we don't have to worry about removals.
-                for( int i = aCollector.GetCount() - 1; i >= 0; --i )
-                {
-                    BOARD_ITEM* item = aCollector[i];
-
-                    // We don't operate on pads; convert them to footprint selections
-                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T
-                                    && !item->GetParent()->IsLocked() )
-                    {
-                        aCollector.Remove( item );
-
-                        if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                            added_items.insert( item->GetParent() );
-                    }
-                }
-
-                for( BOARD_ITEM* item : added_items )
-                    aCollector.Append( item );
-
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             },
-            !m_dragging && !m_isFootprintEditor /* prompt user regarding locked items */ );
+            !m_dragging /* prompt user regarding locked items */ );
 
     if( selection.Empty() )
         return 0;
@@ -1672,35 +1624,9 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent )
     PCB_SELECTION& selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                std::set<BOARD_ITEM*> added_items;
-
-                // Iterate from the back so we don't have to worry about removals.
-                for( int i = aCollector.GetCount() - 1; i >= 0; --i )
-                {
-                    BOARD_ITEM* item = aCollector[i];
-
-                    // We don't operate on pads; convert them to footprint selections
-                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T
-                                    && !item->GetParent()->IsLocked() )
-                    {
-                        aCollector.Remove( item );
-
-                        if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                            added_items.insert( item->GetParent() );
-                    }
-
-                    // We can't flip both a footprint and its text in the same operation, so if
-                    // both are selected, remove the text
-                    if( item->Type() == PCB_FP_TEXT_T && aCollector.HasItem( item->GetParent() ) )
-                        aCollector.Remove( item );
-                }
-
-                for( BOARD_ITEM* item : added_items )
-                    aCollector.Append( item );
-
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             },
-            !m_dragging && !m_isFootprintEditor/* prompt user regarding locked items */ );
+            !m_dragging /* prompt user regarding locked items */ );
 
     if( selection.Empty() )
         return 0;
@@ -1968,8 +1894,6 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent )
     const PCB_SELECTION& selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                std::set<BOARD_ITEM*> added_items;
-
                 // Iterate from the back so we don't have to worry about removals.
                 for( int i = aCollector.GetCount() - 1; i >= 0; --i )
                 {
@@ -1977,23 +1901,11 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent )
 
                     if( item->Type() == PCB_MARKER_T )
                         aCollector.Remove( item );
-
-                    // We don't operate on pads; convert them to footprint selections
-                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T )
-                    {
-                        aCollector.Remove( item );
-
-                        if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                            added_items.insert( item->GetParent() );
-                    }
                 }
 
-                for( BOARD_ITEM* item : added_items )
-                    aCollector.Append( item );
-
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             },
-            !m_isFootprintEditor /* prompt user regarding locked items */ );
+            true /* prompt user regarding locked items */ );
 
     if( selection.Empty() )
         return 0;
@@ -2092,28 +2004,16 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
     const PCB_SELECTION& selection = m_selectionTool->RequestSelection(
                 []( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
                 {
-                    std::set<BOARD_ITEM*> added_items;
-
                     // Iterate from the back so we don't have to worry about removals.
                     for( int i = aCollector.GetCount() - 1; i >= 0; --i )
                     {
                         BOARD_ITEM* item = aCollector[i];
 
-                        // We don't operate on pads; convert them to footprint selections
-                        if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T
-                                        && !item->GetParent()->IsLocked() )
-                        {
+                        if( item->Type() == PCB_MARKER_T )
                             aCollector.Remove( item );
-
-                            if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                                added_items.insert( item->GetParent() );
-                        }
                     }
 
-                    for( BOARD_ITEM* item : added_items )
-                        aCollector.Append( item );
-
-                    sTool->FilterCollectorForHierarchy( aCollector );
+                    sTool->FilterCollectorForHierarchy( aCollector, true );
                 } );
 
     if( selection.Empty() )
@@ -2181,17 +2081,9 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
                 dupe_item = static_cast<PCB_GROUP*>( orig_item )->DeepDuplicate();
                 break;
 
-            case PCB_PAD_T:
-            case PCB_FP_TEXT_T:
-            case PCB_FP_SHAPE_T:
-            case PCB_FP_ZONE_T:
-            case PCB_MARKER_T:
-                // Silently drop these items (such as footprint texts) from duplication
-                break;
-
             default:
-                wxASSERT_MSG( false,
-                              wxString::Format( "Unknown item type %d", orig_item->Type() ) );
+                wxASSERT_MSG( false, wxString::Format( "Unhandled item type %d",
+                                                       orig_item->Type() ) );
                 break;
             }
         }
@@ -2259,28 +2151,7 @@ int EDIT_TOOL::CreateArray( const TOOL_EVENT& aEvent )
     const auto& selection = m_selectionTool->RequestSelection(
                 []( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
                 {
-                    std::set<BOARD_ITEM*> added_items;
-
-                    // Iterate from the back so we don't have to worry about removals.
-                    for( int i = aCollector.GetCount() - 1; i >= 0; --i )
-                    {
-                        BOARD_ITEM* item = aCollector[i];
-
-                        // We don't operate on pads; convert them to footprint selections
-                        if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T
-                                        && !item->GetParent()->IsLocked() )
-                        {
-                            aCollector.Remove( item );
-
-                            if( item->GetParent() && !aCollector.HasItem( item->GetParent() ) )
-                                added_items.insert( item->GetParent() );
-                        }
-                    }
-
-                    for( BOARD_ITEM* item : added_items )
-                        aCollector.Append( item );
-
-                    sTool->FilterCollectorForHierarchy( aCollector );
+                    sTool->FilterCollectorForHierarchy( aCollector, true );
                 } );
 
     if( selection.Empty() )
diff --git a/pcbnew/tools/global_edit_tool.cpp b/pcbnew/tools/global_edit_tool.cpp
index 1a560a78394..3df943b56fe 100644
--- a/pcbnew/tools/global_edit_tool.cpp
+++ b/pcbnew/tools/global_edit_tool.cpp
@@ -202,7 +202,7 @@ int GLOBAL_EDIT_TOOL::RemoveUnusedPads( const TOOL_EVENT& aEvent )
     PCB_SELECTION&  selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                sTool->FilterCollectorForHierarchy( aCollector );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
             } );
     DIALOG_UNUSED_PAD_LAYERS dlg( editFrame, selection, *m_commit );
 
diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp
index 56f298681b6..fbbca67223e 100644
--- a/pcbnew/tools/pcb_selection_tool.cpp
+++ b/pcbnew/tools/pcb_selection_tool.cpp
@@ -707,7 +707,7 @@ bool PCB_SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag,
     // Apply the stateful filter
     FilterCollectedItems( collector );
 
-    FilterCollectorForHierarchy( collector );
+    FilterCollectorForHierarchy( collector, false );
 
     // Apply some ugly heuristics to avoid disambiguation menus whenever possible
     if( collector.GetCount() > 1 && !m_skip_heuristics )
@@ -872,7 +872,7 @@ bool PCB_SELECTION_TOOL::selectMultiple()
             // Apply the stateful filter
             FilterCollectedItems( collector );
 
-            FilterCollectorForHierarchy( collector );
+            FilterCollectorForHierarchy( collector, true );
 
             for( EDA_ITEM* i : collector )
             {
@@ -985,7 +985,7 @@ int PCB_SELECTION_TOOL::SelectAll( const TOOL_EVENT& aEvent )
         collection.Append( item );
     }
 
-    FilterCollectorForHierarchy( collection );
+    FilterCollectorForHierarchy( collection, true );
 
     for( EDA_ITEM* item : collection )
         select( static_cast<BOARD_ITEM*>( item ) );
@@ -2523,7 +2523,8 @@ void PCB_SELECTION_TOOL::GuessSelectionCandidates( GENERAL_COLLECTOR& aCollector
 }
 
 
-void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector ) const
+void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector,
+                                                      bool aMultiselect ) const
 {
     std::unordered_set<BOARD_ITEM*> toAdd;
 
@@ -2536,8 +2537,11 @@ void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollec
             aCollector[j]->GetParent()->ClearFlags( TEMP_SELECTED );
     }
 
-    for( int j = 0; j < aCollector.GetCount(); j++ )
+    if( aMultiselect )
+    {
+        for( int j = 0; j < aCollector.GetCount(); j++ )
             aCollector[j]->SetFlags( TEMP_SELECTED );
+    }
 
     for( int j = 0; j < aCollector.GetCount(); )
     {
diff --git a/pcbnew/tools/pcb_selection_tool.h b/pcbnew/tools/pcb_selection_tool.h
index a05b88c6785..d327cc95c22 100644
--- a/pcbnew/tools/pcb_selection_tool.h
+++ b/pcbnew/tools/pcb_selection_tool.h
@@ -192,7 +192,7 @@ public:
      * In general we don't want to select both a parent and any of it's children.  This includes
      * both footprints and their items, and groups and their members.
      */
-    void FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector ) const;
+    void FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector, bool aMultiselect ) const;
 
     ///< Apply the SELECTION_FILTER_OPTIONS to a collection of items
     void FilterCollectedItems( GENERAL_COLLECTOR& aCollector );
-- 
GitLab