Try harder to merge colinear tracks

Each time we iterator through the tracks list to merge, we change the
connectivity system and can leave possible merges.  By iterating, we
keep merging until we cannot anymore.

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

pcbnew/tracks_cleaner.cpp

@@ -383,53 +383,60 @@ void TRACKS_CLEANER::cleanupSegments()
     if( !m_dryRun )
         removeItems( toRemove );
 
-    // Keep a duplicate deque to all deleting in the primary
+    bool merged = false;
-    std::deque<TRACK*> temp_segments( m_brd->Tracks() );
 
-    // merge collinear segments:
+    do
-    for( TRACK* segment : temp_segments )
     {
-        if( segment->Type() != PCB_TRACE_T )    // one can merge only track collinear segments, not vias.
+        m_brd->BuildConnectivity();
-            continue;
 
-        if( segment->HasFlag( IS_DELETED ) )  // already taken in account
+        // Keep a duplicate deque to all deleting in the primary
-            continue;
+        std::deque<TRACK*> temp_segments( m_brd->Tracks() );
 
-        auto connectivity = m_brd->GetConnectivity();
+        // merge collinear segments:
-
+        for( TRACK* segment : temp_segments )
-        auto& entry = connectivity->GetConnectivityAlgo()->ItemEntry( segment );
-
-        for( CN_ITEM* citem : entry.GetItems() )
         {
-            for( CN_ITEM* connected : citem->ConnectedItems() )
+            if( segment->Type() != PCB_TRACE_T )    // one can merge only track collinear segments, not vias.
+                continue;
+
+            if( segment->HasFlag( IS_DELETED ) )  // already taken in account
+                continue;
+
+            auto connectivity = m_brd->GetConnectivity();
+
+            auto& entry = connectivity->GetConnectivityAlgo()->ItemEntry( segment );
+
+            for( CN_ITEM* citem : entry.GetItems() )
             {
-                if( !connected->Valid() )
+                for( CN_ITEM* connected : citem->ConnectedItems() )
-                    continue;
-
-                BOARD_CONNECTED_ITEM* candidateItem = connected->Parent();
-
-                if( candidateItem->Type() == PCB_TRACE_T && !candidateItem->HasFlag( IS_DELETED ) )
                 {
-                    TRACK* candidateSegment = static_cast<TRACK*>( candidateItem );
+                    if( !connected->Valid() )
-
-                    // Do not merge segments having different widths: it is a frequent case
-                    // to draw a track between 2 pads:
-                    if( candidateSegment->GetWidth() != segment->GetWidth() )
                         continue;
 
-                    if( segment->ApproxCollinear( *candidateSegment ) )
+                    BOARD_CONNECTED_ITEM* candidateItem = connected->Parent();
-                        mergeCollinearSegments( segment, candidateSegment );
+
+                    if( candidateItem->Type() == PCB_TRACE_T && !candidateItem->HasFlag( IS_DELETED ) )
+                    {
+                        TRACK* candidateSegment = static_cast<TRACK*>( candidateItem );
+
+                        // Do not merge segments having different widths: it is a frequent case
+                        // to draw a track between 2 pads:
+                        if( candidateSegment->GetWidth() != segment->GetWidth() )
+                            continue;
+
+                        if( segment->ApproxCollinear( *candidateSegment ) )
+                            merged = mergeCollinearSegments( segment, candidateSegment );
+                    }
                 }
             }
         }
-    }
+    } while( merged );
 }
 
 
-void TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 )
+bool TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 )
 {
     if( aSeg1->IsLocked() || aSeg2->IsLocked() )
-        return;
+        return false;
 
     auto connectivity = m_brd->GetConnectivity();
 
@@ -463,13 +470,13 @@ void TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 )
     if( aSeg1->GetStart() != dummy_seg.GetStart() && aSeg1->GetStart() != dummy_seg.GetEnd() )
     {
         if( testTrackEndpointIsNode( aSeg1, true ) )
-            return;
+            return false;
     }
 
     if( aSeg1->GetEnd() != dummy_seg.GetStart() && aSeg1->GetEnd() != dummy_seg.GetEnd() )
     {
         if( testTrackEndpointIsNode( aSeg1, false ) )
-            return;
+            return false;
     }
 
     CLEANUP_ITEM* item = new CLEANUP_ITEM( CLEANUP_MERGE_TRACKS );
@@ -496,6 +503,8 @@ void TRACKS_CLEANER::mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 )
         m_brd->Remove( aSeg2 );
         m_commit.Removed( aSeg2 );
     }
+
+    return true;
 }
 
 

pcbnew/tracks_cleaner.h

@@ -86,7 +86,7 @@ private:
      * @param aSeg1 is the reference
      * @param aSeg2 is the candidate, and after merging, the removed segment
      */
-    void mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 );
+    bool mergeCollinearSegments( TRACK* aSeg1, TRACK* aSeg2 );
 
     /**
      * @return true if a track end position is a node, i.e. a end connected