From 1e68c353f17c32d30ba853e06f755c797e073600 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Mon, 22 Aug 2022 22:32:42 +0100
Subject: [PATCH] Account for groups in undo of Position Relative To.

Also fixes a bug were pads weren't correctly being found for anchoring
the selection before the Position Relative To.

Fixes https://gitlab.com/kicad/code/kicad/issues/11793
---
 pcbnew/tools/pcb_selection_tool.cpp     |  2 +-
 pcbnew/tools/position_relative_tool.cpp | 84 +++++++++++++------------
 2 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp
index 35243b814c..7d9ccb1699 100644
--- a/pcbnew/tools/pcb_selection_tool.cpp
+++ b/pcbnew/tools/pcb_selection_tool.cpp
@@ -2819,7 +2819,7 @@ void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollec
     std::unordered_set<BOARD_ITEM*> toAdd;
 
     // Set CANDIDATE on all parents which are included in the GENERAL_COLLECTOR.  This
-    // algorithm is O3n, whereas checking for the parent inclusion could potentially be On^2.
+    // algorithm is O(3n), whereas checking for the parent inclusion could potentially be O(n^2).
     for( int j = 0; j < aCollector.GetCount(); j++ )
     {
         if( aCollector[j]->GetParent() )
diff --git a/pcbnew/tools/position_relative_tool.cpp b/pcbnew/tools/position_relative_tool.cpp
index 002f5bfa6f..dbc6c6ab97 100644
--- a/pcbnew/tools/position_relative_tool.cpp
+++ b/pcbnew/tools/position_relative_tool.cpp
@@ -25,18 +25,18 @@
 #include <memory>
 using namespace std::placeholders;
 
-#include "position_relative_tool.h"
-#include "pcb_actions.h"
-#include "pcb_selection_tool.h"
-#include "edit_tool.h"
-#include "pcb_picker_tool.h"
+#include <tools/position_relative_tool.h>
+#include <tools/pcb_actions.h>
+#include <tools/pcb_selection_tool.h>
+#include <tools/pcb_picker_tool.h>
 #include <dialogs/dialog_position_relative.h>
 #include <status_popup.h>
 #include <board_commit.h>
-#include <bitmaps.h>
 #include <confirm.h>
 #include <collectors.h>
 #include <pad.h>
+#include <footprint.h>
+#include <pcb_group.h>
 
 
 POSITION_RELATIVE_TOOL::POSITION_RELATIVE_TOOL() :
@@ -64,44 +64,16 @@ bool POSITION_RELATIVE_TOOL::Init()
 }
 
 
-// TODO: Clean up this global once TOOL_EVENT supports std::functions as parameters
-BOARD_ITEM* g_PositionRelativePadAnchor;
-
-
 int POSITION_RELATIVE_TOOL::PositionRelative( const TOOL_EVENT& aEvent )
 {
     PCB_BASE_FRAME* editFrame = getEditFrame<PCB_BASE_FRAME>();
 
-    g_PositionRelativePadAnchor = nullptr;
-
     const auto& selection = m_selectionTool->RequestSelection(
             []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
             {
-                std::set<BOARD_ITEM*> to_add;
-
-                // 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];
-
-                    if( item->Type() == PCB_MARKER_T )
-                        aCollector.Remove( item );
-
-                    /// Locked pads do not get moved independently of the footprint
-                    if( !sTool->IsFootprintEditor() && item->Type() == PCB_PAD_T && item->IsLocked()
-                        && !item->GetParent()->IsLocked() )
-                    {
-                        if( !aCollector.HasItem( item->GetParent() ) )
-                            to_add.insert( item->GetParent() );
-
-                        g_PositionRelativePadAnchor = item;
-
-                        aCollector.Remove( item );
-                    }
-                }
-
-                for( BOARD_ITEM* item : to_add )
-                    aCollector.Append( item );
+                sTool->FilterCollectorForHierarchy( aCollector, true );
+                sTool->FilterCollectorForMarkers( aCollector );
+                sTool->FilterCollectorForFreePads( aCollector );
             },
             !m_isFootprintEditor /* prompt user regarding locked items */ );
 
@@ -110,8 +82,29 @@ int POSITION_RELATIVE_TOOL::PositionRelative( const TOOL_EVENT& aEvent )
 
     m_selection = selection;
 
-    if( g_PositionRelativePadAnchor )
-        m_selectionAnchor = static_cast<PAD*>( g_PositionRelativePadAnchor )->GetPosition();
+    PCB_TYPE_COLLECTOR collector;
+    collector.Collect( static_cast<BOARD_ITEM*>( m_selection.GetTopLeftItem() ), { PCB_PAD_T } );
+
+    if( collector.GetCount() == 0 )
+    {
+        for( FOOTPRINT* footprint : editFrame->GetBoard()->Footprints() )
+        {
+            for( PAD* pad : footprint->Pads() )
+            {
+                if( pad->IsSelected() )
+                    collector.Append( pad );
+
+                if( collector.GetCount() > 0 )
+                    break;
+            }
+
+            if( collector.GetCount() > 0 )
+                break;
+        }
+    }
+
+    if( collector.GetCount() > 0 )
+        m_selectionAnchor = collector[0]->GetPosition();
     else
         m_selectionAnchor = m_selection.GetTopLeftItem()->GetPosition();
 
@@ -138,13 +131,24 @@ int POSITION_RELATIVE_TOOL::RelativeItemSelectionMove( const VECTOR2I& aPosAncho
 {
     VECTOR2I aggregateTranslation = aPosAnchor + aTranslation - GetSelectionAnchorPosition();
 
-    for( auto item : m_selection )
+    for( EDA_ITEM* item : m_selection )
     {
         // Don't move a pad by itself unless editing the footprint
         if( item->Type() == PCB_PAD_T && frame()->IsType( FRAME_PCB_EDITOR ) )
             item = item->GetParent();
 
         m_commit->Modify( item );
+
+        // If moving a group, record position of all the descendants for undo
+        if( item->Type() == PCB_GROUP_T )
+        {
+            PCB_GROUP* group = static_cast<PCB_GROUP*>( item );
+            group->RunOnDescendants( [&]( BOARD_ITEM* bItem )
+                                     {
+                                         m_commit->Modify( bItem );
+                                     });
+        }
+
         static_cast<BOARD_ITEM*>( item )->Move( aggregateTranslation );
     }