From d3e1e54b24c637ad5181dd765660a0f46828f797 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Fri, 5 Jan 2024 11:38:16 -0800
Subject: [PATCH] Avoid removing segments that do not maintain conns

If the two segments being merged have connection points originally, then
the merged segment must have the same connection points.  We had been
moving the merged segment to match the original connections but this is
not as robust as just not merging these lines (and therefore leaving the
original)

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15495
---
 pcbnew/tracks_cleaner.cpp | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/pcbnew/tracks_cleaner.cpp b/pcbnew/tracks_cleaner.cpp
index d527649a72..9d134045d3 100644
--- a/pcbnew/tracks_cleaner.cpp
+++ b/pcbnew/tracks_cleaner.cpp
@@ -85,7 +85,9 @@ void TRACKS_CLEANER::CleanupBoard( bool aDryRun,
         wxSafeYield();      // Timeslice to update UI
     }
 
-    cleanup( false, false, true, aMergeSegments );
+    // If we didn't remove duplicates above, do it now
+    if( !aMergeSegments )
+        cleanup( false, false, true, false );
 
     if( aRemoveMisConnected )
     {
@@ -481,6 +483,7 @@ void TRACKS_CLEANER::cleanup( bool aDeleteDuplicateVias, bool aDeleteNullSegment
     auto mergeSegments =
             [&]( std::shared_ptr<CN_CONNECTIVITY_ALGO> connectivity ) -> bool
             {
+
                 for( PCB_TRACK* segment : m_brd->Tracks() )
                 {
                     // one can merge only collinear segments, not vias or arcs.
@@ -626,6 +629,8 @@ bool TRACKS_CLEANER::mergeCollinearSegments( PCB_TRACK* aSeg1, PCB_TRACK* aSeg2
             collectPts( item );
     }
 
+    int64_t combined_length = aSeg1->GetLength() + aSeg2->GetLength();
+
     // This means there is a node in the center
     if( pts.size() > 2 )
         return false;
@@ -656,19 +661,12 @@ bool TRACKS_CLEANER::mergeCollinearSegments( PCB_TRACK* aSeg1, PCB_TRACK* aSeg2
         dummy_seg.SetEnd( VECTOR2I( max_x, min_y ) );
     }
 
-    // If the existing connected points are not the same as the points generated by our
-    // min/max alg, then assign the missing points to the end closest.  This ensures that
-    // our replacement track is still connected
+    // The new ends of the segment must be connected to all of the same points as the original
+    // segments.  If not, the segments cannot be merged.
     for( auto& pt : pts )
     {
-        if( !dummy_seg.IsPointOnEnds( VECTOR2I( pt.x, pt.y ) ) )
-        {
-            if( ( VECTOR2I( dummy_seg.GetStart() ) - pt ).SquaredEuclideanNorm() <
-                    ( VECTOR2I( dummy_seg.GetEnd() ) - pt ).SquaredEuclideanNorm() )
-                dummy_seg.SetStart( VECTOR2I( pt.x, pt.y ) );
-            else
-                dummy_seg.SetEnd( VECTOR2I( pt.x, pt.y ) );
-        }
+        if( !dummy_seg.IsPointOnEnds( pt ) )
+            return false;
     }
 
     // Now find the removed end(s) and stop merging if it is a node: