From 732f588d8395184ee1785de13e0f39fd714639c8 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Fri, 2 Oct 2020 12:32:48 +0100
Subject: [PATCH] Don't hit-test groups; allow only selection promotions from
 members.

Also fixes some bugs were the selection filters weren't getting
applied correctly on drag-selects.

Fixes https://gitlab.com/kicad/code/kicad/issues/5668

Fixes https://gitlab.com/kicad/code/kicad/issues/5804
---
 pcbnew/class_pcb_group.cpp      | 28 ++---------------
 pcbnew/tools/selection_tool.cpp | 53 +++++++++++++++++----------------
 2 files changed, 30 insertions(+), 51 deletions(-)

diff --git a/pcbnew/class_pcb_group.cpp b/pcbnew/class_pcb_group.cpp
index 02c56a0c23..4b46662f11 100644
--- a/pcbnew/class_pcb_group.cpp
+++ b/pcbnew/class_pcb_group.cpp
@@ -165,36 +165,14 @@ void PCB_GROUP::SwapData( BOARD_ITEM* aImage )
 
 bool PCB_GROUP::HitTest( const wxPoint& aPosition, int aAccuracy ) const
 {
-    EDA_RECT rect = GetBoundingBox();
-    return rect.Inflate( aAccuracy ).Contains( aPosition );
+    // Groups are selected by promoting a selection of one of their children
+    return false;
 }
 
 
 bool PCB_GROUP::HitTest( const EDA_RECT& aRect, bool aContained, int aAccuracy ) const
 {
-    EDA_RECT arect = aRect;
-    arect.Inflate( aAccuracy );
-
-    EDA_RECT bbox = GetBoundingBox();
-
-    if( aContained )
-    {
-        return arect.Contains( bbox );
-    }
-    else
-    {
-        // If the rect does not intersect the bounding box, skip any tests
-        if( !aRect.Intersects( bbox ) )
-            return false;
-
-        for( BOARD_ITEM* member : m_items )
-        {
-            if( member->HitTest( arect, false, 0 ) )
-                return true;
-        }
-    }
-
-    // No items were hit
+    // Groups are selected by promoting a selection of one of their children
     return false;
 }
 
diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp
index c6dba7b83a..9dad55bda7 100644
--- a/pcbnew/tools/selection_tool.cpp
+++ b/pcbnew/tools/selection_tool.cpp
@@ -533,6 +533,8 @@ bool SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag,
     // Apply the stateful filter
     filterCollectedItems( collector );
 
+    FilterCollectorForGroups( collector );
+
     // Apply some ugly heuristics to avoid disambiguation menus whenever possible
     if( collector.GetCount() > 1 && !m_skip_heuristics )
     {
@@ -660,14 +662,9 @@ bool SELECTION_TOOL::selectMultiple()
             // End drawing the selection box
             view->SetVisible( &area, false );
 
-            // Mark items within the selection box as selected
-            std::vector<KIGFX::VIEW::LAYER_ITEM_PAIR> selectedItems;
-
-            // Filter the view items based on the selection box
+            std::vector<KIGFX::VIEW::LAYER_ITEM_PAIR> candidates;
             BOX2I selectionBox = area.ViewBBox();
-            view->Query( selectionBox, selectedItems );         // Get the list of selected items
-
-            std::vector<KIGFX::VIEW::LAYER_ITEM_PAIR>::iterator it, it_end;
+            view->Query( selectionBox, candidates );    // Get the list of nearby items
 
             int width = area.GetEnd().x - area.GetOrigin().x;
             int height = area.GetEnd().y - area.GetOrigin().y;
@@ -686,25 +683,34 @@ bool SELECTION_TOOL::selectMultiple()
 
             selectionRect.Normalize();
 
-            for( it = selectedItems.begin(), it_end = selectedItems.end(); it != it_end; ++it )
+            GENERAL_COLLECTOR collector;
+
+            for( auto it = candidates.begin(), it_end = candidates.end(); it != it_end; ++it )
             {
                 BOARD_ITEM* item = static_cast<BOARD_ITEM*>( it->first );
 
-                if( !item || !Selectable( item ) || !itemPassesFilter( item ) )
-                    continue;
+                if( item && Selectable( item ) && item->HitTest( selectionRect, windowSelection ) )
+                    collector.Append( item );
+            }
 
-                if( item->HitTest( selectionRect, windowSelection ) )
+            // Apply the stateful filter
+            filterCollectedItems( collector );
+
+            FilterCollectorForGroups( collector );
+
+            for( EDA_ITEM* i : collector )
+            {
+                BOARD_ITEM* item = static_cast<BOARD_ITEM*>( i );
+
+                if( m_subtractive || ( m_exclusive_or && item->IsSelected() ) )
                 {
-                    if( m_subtractive || ( m_exclusive_or && item->IsSelected() ) )
-                    {
-                        unselect( item );
-                        anySubtracted = true;
-                    }
-                    else
-                    {
-                        select( item );
-                        anyAdded = true;
-                    }
+                    unselect( item );
+                    anySubtracted = true;
+                }
+                else
+                {
+                    select( item );
+                    anyAdded = true;
                 }
             }
 
@@ -1546,9 +1552,6 @@ bool SELECTION_TOOL::itemPassesFilter( BOARD_ITEM* aItem )
             return false;
     }
 
-    if( m_enteredGroup && !PCB_GROUP::WithinScope( aItem, m_enteredGroup ) )
-        return false;
-
     return true;
 }
 
@@ -2548,8 +2551,6 @@ void SELECTION_TOOL::GuessSelectionCandidates( GENERAL_COLLECTOR& aCollector,
             aCollector.Transfer( item );
         }
     }
-
-    FilterCollectorForGroups( aCollector );
 }