From 0180cb380fe70b611bec6f4f582d677304d2f6ed Mon Sep 17 00:00:00 2001
From: Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
Date: Thu, 25 May 2023 01:18:53 +0200
Subject: [PATCH] router: prevent pushout/bogus collision of via with is own
 hole (or copper) when dragging

The root cause was not removing the original (pre-dragging) via from the world, comparing the ITEM::Parent() pointers of the items was only hiding the main issue.
This also fixes the PNSViaCollisions test in the qa/ suite.
---
 pcbnew/router/pns_dragger.cpp | 17 ++++++++++++---
 pcbnew/router/pns_item.cpp    | 39 +++++++++++++++++++++++++++++------
 pcbnew/router/pns_item.h      |  5 -----
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/pcbnew/router/pns_dragger.cpp b/pcbnew/router/pns_dragger.cpp
index dfd8932010..2922cbb675 100644
--- a/pcbnew/router/pns_dragger.cpp
+++ b/pcbnew/router/pns_dragger.cpp
@@ -323,7 +323,7 @@ bool DRAGGER::dragMarkObstacles( const VECTOR2I& aP )
     if( Settings().AllowDRCViolations() )
         m_dragStatus = true;
     else
-        m_dragStatus = !m_world->CheckColliding( m_draggedItems );
+        m_dragStatus = !m_lastNode->CheckColliding( m_draggedItems );
 
     return true;
 }
@@ -398,13 +398,14 @@ bool DRAGGER::dragViaWalkaround( const VIA_HANDLE& aHandle, NODE* aNode, const V
 
             vias.insert( draggedVia.get() );
 
+            m_lastNode->Remove( via );
+
             bool ok = propagateViaForces( m_lastNode, vias );
 
             if( ok )
             {
                 viaTargetPos = draggedVia->Pos();
                 viaPropOk = true;
-                m_lastNode->Remove( via );
                 m_lastNode->Add( std::move(draggedVia) );
             }
         }
@@ -604,7 +605,7 @@ bool DRAGGER::dragWalkaround( const VECTOR2I& aP )
 
     m_dragStatus = ok;
 
-    return true;
+    return ok;
 }
 
 
@@ -745,7 +746,17 @@ bool DRAGGER::Drag( const VECTOR2I& aP )
     }
 
     if( ret )
+    {
         m_lastValidPoint = aP;
+    }
+    else
+    {
+        if( m_lastNode )
+        {
+            delete m_lastNode;
+            m_lastNode = nullptr;
+        }
+    }
 
     return ret;
 }
diff --git a/pcbnew/router/pns_item.cpp b/pcbnew/router/pns_item.cpp
index 913eb1353d..5b9dc706ea 100644
--- a/pcbnew/router/pns_item.cpp
+++ b/pcbnew/router/pns_item.cpp
@@ -44,6 +44,27 @@ static void dumpObstacles( const PNS::NODE::OBSTACLES &obstacles )
     }
 }
 
+// prune self-collisions, i.e. a via/pad annular ring with its own hole
+static bool shouldWeConsiderHoleCollisions( const ITEM* aItem, const ITEM *aHead )
+{
+    const HOLE *hi = aItem->OfKind( ITEM::HOLE_T ) ? static_cast<const HOLE*>( aItem ) : nullptr;
+    const HOLE *hh = aHead->OfKind( ITEM::HOLE_T ) ? static_cast<const HOLE*>( aHead ) : nullptr;
+
+    if( hi && hh ) // hole-to-hole case
+    {
+        if ( !hi->ParentPadVia() || !hh->ParentPadVia() )
+            return true;
+
+        return hi->ParentPadVia() != hh->ParentPadVia();
+    }
+
+    if( hi )
+        return hi->ParentPadVia() != aHead;
+    else if( hh )
+        return hh->ParentPadVia() != aItem;
+    else
+        return true;
+}
 
 bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode,
                           COLLISION_SEARCH_CONTEXT* aCtx ) const
@@ -61,13 +82,12 @@ bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode,
     int          clearanceEpsilon = aNode->GetRuleResolver()->ClearanceEpsilon();
     bool         collisionsFound = false;
 
-    //printf( "******************** CollideSimple %lu\n", aCtx->obstacles.size() );
-
-    //printf( "h %p n %p t %p ctx %p\n", aHead, aNode, this, aCtx );
-
     if( this == aHead )  // we cannot be self-colliding
         return false;
 
+    if ( !shouldWeConsiderHoleCollisions( this, aHead ) )
+        return false;
+
     // Special cases for "head" lines with vias attached at the end.  Note that this does not
     // support head-line-via to head-line-via collisions, but you can't route two independent
     // tracks at once so it shouldn't come up.
@@ -86,9 +106,16 @@ bool ITEM::collideSimple( const ITEM* aHead, const NODE* aNode,
 
     // And a special case for the "head" via's hole.
 
-    if( holeH && !HasSameParentPadVia( holeH ) )
-        collisionsFound |= collideSimple( holeH, aNode, aCtx );
 
+    if( holeH && shouldWeConsiderHoleCollisions( this, holeH ) )
+    {
+        if (collideSimple( holeH, aNode, aCtx ) )
+        {
+            collisionsFound = true;
+        }
+    }
+
+    
     // Sadly collision routines ignore SHAPE_POLY_LINE widths so we have to pass them in as part
     // of the clearance value.
     if( m_kind == LINE_T )
diff --git a/pcbnew/router/pns_item.h b/pcbnew/router/pns_item.h
index ea0318ae81..e0ecac1fa5 100644
--- a/pcbnew/router/pns_item.h
+++ b/pcbnew/router/pns_item.h
@@ -251,11 +251,6 @@ public:
     bool IsFreePad() const { return m_isFreePad; }
 
     virtual ITEM* ParentPadVia() const { return nullptr; }
-    virtual bool HasSameParentPadVia( const ITEM* aOther ) const
-    {
-        return ParentPadVia() && aOther->ParentPadVia()
-                && ParentPadVia()->Parent() == aOther->ParentPadVia()->Parent();
-    }
 
     bool IsVirtual() const
     {