From e48a2b5ee4386ebaf90d6a95158aed39e8a664bf Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 13 Mar 2025 16:08:25 +0000
Subject: [PATCH] Don't report non-meeting line-pairs.

We're called on each line pair, so a rect would otherwise
always report to failures for the two pairs of opposite
sides.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/20326
---
 pcbnew/tools/edit_tool.cpp                 |  2 +-
 pcbnew/tools/item_modification_routine.cpp | 42 +++++++++++-----------
 pcbnew/tools/item_modification_routine.h   | 40 +++++++++++++--------
 3 files changed, 47 insertions(+), 37 deletions(-)

diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 68d4da3483..b570756702 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -1599,7 +1599,7 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
 
     commit.Push( pairwise_line_routine->GetCommitDescription() );
 
-    if( const std::optional<wxString> msg = pairwise_line_routine->GetStatusMessage() )
+    if( const std::optional<wxString> msg = pairwise_line_routine->GetStatusMessage( segmentCount ) )
         frame()->ShowInfoBarMsg( *msg );
 
     return 0;
diff --git a/pcbnew/tools/item_modification_routine.cpp b/pcbnew/tools/item_modification_routine.cpp
index ec526d108c..e727f8fda4 100644
--- a/pcbnew/tools/item_modification_routine.cpp
+++ b/pcbnew/tools/item_modification_routine.cpp
@@ -105,13 +105,13 @@ wxString LINE_FILLET_ROUTINE::GetCommitDescription() const
 }
 
 
-std::optional<wxString> LINE_FILLET_ROUTINE::GetStatusMessage() const
+std::optional<wxString> LINE_FILLET_ROUTINE::GetStatusMessage( int aSegmentCount ) const
 {
     if( GetSuccesses() == 0 )
     {
         return _( "Unable to fillet the selected lines." );
     }
-    else if( GetFailures() > 0 )
+    else if( GetFailures() > 0 || (int) GetSuccesses() < aSegmentCount - 1 )
     {
         return _( "Some of the lines could not be filleted." );
     }
@@ -132,7 +132,6 @@ void LINE_FILLET_ROUTINE::ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB
     if( !a_pt || !b_pt )
     {
         // The lines do not share an endpoint, so we can't fillet them
-        AddFailure();
         return;
     }
 
@@ -142,20 +141,21 @@ void LINE_FILLET_ROUTINE::ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB
     SHAPE_ARC sArc( seg_a, seg_b, m_filletRadiusIU );
     VECTOR2I  t1newPoint, t2newPoint;
 
-    auto setIfPointOnSeg = []( VECTOR2I& aPointToSet, SEG aSegment, VECTOR2I aVecToTest )
-    {
-        VECTOR2I segToVec = aSegment.NearestPoint( aVecToTest ) - aVecToTest;
+    auto setIfPointOnSeg =
+            []( VECTOR2I& aPointToSet, SEG aSegment, VECTOR2I aVecToTest )
+            {
+                VECTOR2I segToVec = aSegment.NearestPoint( aVecToTest ) - aVecToTest;
 
-        // Find out if we are on the segment (minimum precision)
-        if( segToVec.EuclideanNorm() < SHAPE_ARC::MIN_PRECISION_IU )
-        {
-            aPointToSet.x = aVecToTest.x;
-            aPointToSet.y = aVecToTest.y;
-            return true;
-        }
+                // Find out if we are on the segment (minimum precision)
+                if( segToVec.EuclideanNorm() < SHAPE_ARC::MIN_PRECISION_IU )
+                {
+                    aPointToSet.x = aVecToTest.x;
+                    aPointToSet.y = aVecToTest.y;
+                    return true;
+                }
 
-        return false;
-    };
+                return false;
+            };
 
     //Do not draw a fillet if the end points of the arc are not within the track segments
     if( !setIfPointOnSeg( t1newPoint, seg_a, sArc.GetP0() )
@@ -200,13 +200,13 @@ wxString LINE_CHAMFER_ROUTINE::GetCommitDescription() const
 }
 
 
-std::optional<wxString> LINE_CHAMFER_ROUTINE::GetStatusMessage() const
+std::optional<wxString> LINE_CHAMFER_ROUTINE::GetStatusMessage( int aSegmentCount ) const
 {
     if( GetSuccesses() == 0 )
     {
         return _( "Unable to chamfer the selected lines." );
     }
-    else if( GetFailures() > 0 )
+    else if( GetFailures() > 0 || (int) GetSuccesses() < aSegmentCount - 1 )
     {
         return _( "Some of the lines could not be chamfered." );
     }
@@ -267,7 +267,7 @@ wxString DOGBONE_CORNER_ROUTINE::GetCommitDescription() const
 }
 
 
-std::optional<wxString> DOGBONE_CORNER_ROUTINE::GetStatusMessage() const
+std::optional<wxString> DOGBONE_CORNER_ROUTINE::GetStatusMessage( int aSegmentCount ) const
 {
     wxString msg;
 
@@ -275,7 +275,7 @@ std::optional<wxString> DOGBONE_CORNER_ROUTINE::GetStatusMessage() const
     {
         msg += _( "Unable to add dogbone corners to the selected lines." );
     }
-    else if( GetFailures() > 0 )
+    else if( GetFailures() > 0 || (int) GetSuccesses() < aSegmentCount - 1 )
     {
         msg += _( "Some of the lines could not have dogbone corners added." );
     }
@@ -386,13 +386,13 @@ wxString LINE_EXTENSION_ROUTINE::GetCommitDescription() const
 }
 
 
-std::optional<wxString> LINE_EXTENSION_ROUTINE::GetStatusMessage() const
+std::optional<wxString> LINE_EXTENSION_ROUTINE::GetStatusMessage( int aSegmentCount ) const
 {
     if( GetSuccesses() == 0 )
     {
         return _( "Unable to extend the selected lines to meet." );
     }
-    else if( GetFailures() > 0 )
+    else if( GetFailures() > 0 || (int) GetSuccesses() < aSegmentCount - 1 )
     {
         return _( "Some of the lines could not be extended to meet." );
     }
diff --git a/pcbnew/tools/item_modification_routine.h b/pcbnew/tools/item_modification_routine.h
index c8600a40db..980d959ed6 100644
--- a/pcbnew/tools/item_modification_routine.h
+++ b/pcbnew/tools/item_modification_routine.h
@@ -168,13 +168,6 @@ public:
 
     virtual wxString GetCommitDescription() const = 0;
 
-    /**
-     * @brief Get a status message to show when the routine is complete
-     *
-     * Usually this will be an error or nothing.
-     */
-    virtual std::optional<wxString> GetStatusMessage() const = 0;
-
 protected:
     /**
      * The BOARD used when creating new shapes
@@ -240,6 +233,13 @@ public:
      * @return did the action succeed
      */
     virtual void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) = 0;
+
+    /**
+     * @brief Get a status message to show when the routine is complete
+     *
+     * Usually this will be an error or nothing.
+     */
+    virtual std::optional<wxString> GetStatusMessage( int aSegmentCount ) const = 0;
 };
 
 /**
@@ -249,12 +249,13 @@ class LINE_FILLET_ROUTINE : public PAIRWISE_LINE_ROUTINE
 {
 public:
     LINE_FILLET_ROUTINE( BOARD_ITEM* aBoard, CHANGE_HANDLER& aHandler, int filletRadiusIU ) :
-            PAIRWISE_LINE_ROUTINE( aBoard, aHandler ), m_filletRadiusIU( filletRadiusIU )
+            PAIRWISE_LINE_ROUTINE( aBoard, aHandler ),
+            m_filletRadiusIU( filletRadiusIU )
     {
     }
 
     wxString GetCommitDescription() const override;
-    std::optional<wxString> GetStatusMessage() const override;
+    std::optional<wxString> GetStatusMessage( int aSegmentCount ) const override;
 
     void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) override;
 
@@ -276,7 +277,7 @@ public:
     }
 
     wxString GetCommitDescription() const override;
-    std::optional<wxString> GetStatusMessage() const override;
+    std::optional<wxString> GetStatusMessage( int aSegmentCount ) const override;
 
     void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) override;
 
@@ -296,7 +297,7 @@ public:
     }
 
     wxString GetCommitDescription() const override;
-    std::optional<wxString> GetStatusMessage() const override;
+    std::optional<wxString> GetStatusMessage( int aSegmentCount ) const override;
 
     void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) override;
 };
@@ -314,13 +315,14 @@ public:
     };
 
     DOGBONE_CORNER_ROUTINE( BOARD_ITEM* aBoard, CHANGE_HANDLER& aHandler, PARAMETERS aParams ) :
-            PAIRWISE_LINE_ROUTINE( aBoard, aHandler ), m_params( std::move( aParams ) ),
+            PAIRWISE_LINE_ROUTINE( aBoard, aHandler ),
+            m_params( std::move( aParams ) ),
             m_haveNarrowMouths( false )
     {
     }
 
     wxString                GetCommitDescription() const override;
-    std::optional<wxString> GetStatusMessage() const override;
+    std::optional<wxString> GetStatusMessage( int aSegmentCount ) const override;
 
     void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) override;
 
@@ -348,6 +350,13 @@ public:
      */
     void Finalize();
 
+    /**
+     * @brief Get a status message to show when the routine is complete
+     *
+     * Usually this will be an error or nothing.
+     */
+    virtual std::optional<wxString> GetStatusMessage() const = 0;
+
 protected:
     SHAPE_POLY_SET& GetWorkingPolygons() { return m_workingPolygons; }
 
@@ -427,13 +436,14 @@ public:
     };
 
     OUTSET_ROUTINE( BOARD_ITEM* aBoard, CHANGE_HANDLER& aHandler, const PARAMETERS& aParams ) :
-            ITEM_MODIFICATION_ROUTINE( aBoard, aHandler ), m_params( aParams )
+            ITEM_MODIFICATION_ROUTINE( aBoard, aHandler ),
+            m_params( aParams )
     {
     }
 
     wxString GetCommitDescription() const override;
 
-    std::optional<wxString> GetStatusMessage() const override;
+    std::optional<wxString> GetStatusMessage() const;
 
     void ProcessItem( BOARD_ITEM& aItem );