Remove stale teardrops before rebuilding connectivity.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15398

pcbnew/board_commit.cpp

@@ -174,6 +174,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
 
     undoList.SetDescription( aMessage );
 
+    TEARDROP_MANAGER                   teardropMgr( board, m_toolMgr );
     std::shared_ptr<CONNECTIVITY_DATA> connectivity = board->GetConnectivity();
 
     // Note: frame == nullptr happens in QA tests
@@ -194,14 +195,9 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
 
     for( COMMIT_LINE& ent : m_changes )
     {
-        int changeType = ent.m_type & CHT_TYPE;
-        int changeFlags = ent.m_type & CHT_FLAGS;
         BOARD_ITEM* boardItem = dynamic_cast<BOARD_ITEM*>( ent.m_item );
 
-        wxASSERT( ent.m_item );
+        if( m_isBoardEditor && boardItem )
-        wxCHECK2( boardItem, continue );
-
-        if( m_isBoardEditor )
         {
             if( boardItem->Type() == PCB_VIA_T || boardItem->Type() == PCB_FOOTPRINT_T
                     || boardItem->IsOnLayer( F_Mask ) || boardItem->IsOnLayer( B_Mask ) )
@@ -211,7 +207,12 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
 
             if( !( aCommitFlags & SKIP_TEARDROPS ) )
             {
-                if( boardItem->Type() == PCB_PAD_T || boardItem->Type() == PCB_VIA_T )
+                if( boardItem->Type() == PCB_FOOTPRINT_T )
+                {
+                    for( PAD* pad : static_cast<FOOTPRINT*>( boardItem )->Pads() )
+                        staleTeardropPadsAndVias.push_back( pad );
+                }
+                else if( boardItem->Type() == PCB_PAD_T || boardItem->Type() == PCB_VIA_T )
                 {
                     staleTeardropPadsAndVias.push_back( boardItem );
                 }
@@ -235,8 +236,22 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
             }
         }
 
-        if( boardItem->IsSelected() )
+        if( boardItem && boardItem->IsSelected() )
             selectedModified = true;
+    }
+
+    // Old teardrops must be removed before connectivity is rebuilt
+    if( !staleTeardropPadsAndVias.empty() || !staleTeardropTracks.empty() )
+        teardropMgr.RemoveTeardrops( *this, &staleTeardropPadsAndVias, &staleTeardropTracks );
+
+    for( COMMIT_LINE& ent : m_changes )
+    {
+        int changeType = ent.m_type & CHT_TYPE;
+        int changeFlags = ent.m_type & CHT_FLAGS;
+        BOARD_ITEM* boardItem = dynamic_cast<BOARD_ITEM*>( ent.m_item );
+
+        wxASSERT( ent.m_item );
+        wxCHECK2( boardItem, continue );
 
         switch( changeType )
         {
@@ -448,10 +463,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
         }
 
         if( !staleTeardropPadsAndVias.empty() || !staleTeardropTracks.empty() )
-        {
-            TEARDROP_MANAGER teardropMgr( board, m_toolMgr );
             teardropMgr.UpdateTeardrops( *this, &staleTeardropPadsAndVias, &staleTeardropTracks );
-        }
 
         // Log undo items for any connectivity or teardrop changes
         for( size_t i = num_changes; i < m_changes.size(); ++i )
@@ -465,7 +477,6 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
             if( !( aCommitFlags & SKIP_UNDO ) )
             {
                 ITEM_PICKER itemWrapper( nullptr, boardItem, convert( ent.m_type & CHT_TYPE ) );
-                wxASSERT( boardItemCopy );
                 itemWrapper.SetLink( boardItemCopy );
                 undoList.PushItem( itemWrapper );
             }

pcbnew/teardrop/teardrop.cpp

@@ -96,6 +96,58 @@ ZONE* TEARDROP_MANAGER::createTeardrop( TEARDROP_VARIANT aTeardropVariant,
 }
 
 
+void TEARDROP_MANAGER::RemoveTeardrops( BOARD_COMMIT& aCommit,
+                                        const std::vector<BOARD_ITEM*>* dirtyPadsAndVias,
+                                        const std::set<PCB_TRACK*>* dirtyTracks )
+{
+    std::shared_ptr<CONNECTIVITY_DATA> connectivity = m_board->GetConnectivity();
+    std::vector<ZONE*>                 stale_teardrops;
+
+    for( ZONE* zone : m_board->Zones() )
+    {
+        if( zone->IsTeardropArea() )
+        {
+            bool stale = false;
+
+            std::vector<PAD*>     connectedPads;
+            std::vector<PCB_VIA*> connectedVias;
+
+            connectivity->GetConnectedPadsAndVias( zone, &connectedPads, &connectedVias );
+
+            for( PAD* pad : connectedPads )
+            {
+                if( alg::contains( *dirtyPadsAndVias, pad ) )
+                {
+                    stale = true;
+                    break;
+                }
+            }
+
+            if( !stale )
+            {
+                for( PCB_VIA* via : connectedVias )
+                {
+                    if( alg::contains( *dirtyPadsAndVias, via ) )
+                    {
+                        stale = true;
+                        break;
+                    }
+                }
+            }
+
+            if( stale )
+                stale_teardrops.push_back( zone );
+        }
+    }
+
+    for( ZONE* td : stale_teardrops )
+    {
+        m_board->Remove( td, REMOVE_MODE::BULK );
+        aCommit.Removed( td );
+    }
+}
+
+
 void TEARDROP_MANAGER::UpdateTeardrops( BOARD_COMMIT& aCommit,
                                         const std::vector<BOARD_ITEM*>* dirtyPadsAndVias,
                                         const std::set<PCB_TRACK*>* dirtyTracks,
@@ -109,41 +161,25 @@ void TEARDROP_MANAGER::UpdateTeardrops( BOARD_COMMIT& aCommit,
 
     buildTrackCaches();
 
-    std::shared_ptr<CONNECTIVITY_DATA> connectivity = m_board->GetConnectivity();
-
     // Old teardrops must be removed, to ensure a clean teardrop rebuild
-    std::vector<ZONE*> stale_teardrops;
+    if( aForceFullUpdate )
-
-    for( ZONE* zone : m_board->Zones() )
     {
-        if( zone->IsTeardropArea() )
+        std::vector<ZONE*> teardrops;
+
+        for( ZONE* zone : m_board->Zones() )
         {
-            bool stale = aForceFullUpdate;
+            if( zone->IsTeardropArea() )
+                teardrops.push_back( zone );
+        }
 
-            if( !stale )
+        for( ZONE* td : teardrops )
-            {
+        {
-                std::vector<PAD*>     connectedPads;
+            m_board->Remove( td, REMOVE_MODE::BULK );
-                std::vector<PCB_VIA*> connectedVias;
+            aCommit.Removed( td );
-
-                connectivity->GetConnectedPadsAndVias( zone, &connectedPads, &connectedVias );
-
-                for( PAD* pad : connectedPads )
-                    stale = stale || alg::contains( *dirtyPadsAndVias, pad );
-
-                for( PCB_VIA* via : connectedVias )
-                    stale = stale || alg::contains( *dirtyPadsAndVias, via );
-            }
-
-            if( stale )
-                stale_teardrops.push_back( zone );
         }
     }
 
-    for( ZONE* td : stale_teardrops )
+    std::shared_ptr<CONNECTIVITY_DATA> connectivity = m_board->GetConnectivity();
-    {
-        m_board->Remove( td, REMOVE_MODE::BULK );
-        aCommit.Removed( td );
-    }
 
     for( PCB_TRACK* track : m_board->Tracks() )
     {

pcbnew/teardrop/teardrop.h

@@ -106,6 +106,14 @@ public:
 
     TEARDROP_MANAGER( BOARD* aBoard, TOOL_MANAGER* aToolManager );
 
+    /**
+     * Remove teardrops connected to any dirty pads, vias or tracks.  They need to be removed
+     * before being rebuilt.
+     *
+     * NB: this must be called BEFORE the connectivity is updated for the change in question.
+     */
+    void RemoveTeardrops( BOARD_COMMIT& aCommit, const std::vector<BOARD_ITEM*>* dirtyPadsAndVias,
+                          const std::set<PCB_TRACK*>* dirtyTracks );
     /**
      * Update teardrops on a list of items.
      */