From b727bfc16d2815ed7cdd4b80eb94c8cf357233c5 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Sat, 16 Jul 2022 18:41:59 +0100
Subject: [PATCH] Performance: avoid sqrt at all costs.

---
 common/eda_shape.cpp                          |  7 +---
 libs/kimath/include/geometry/shape_poly_set.h |  4 +-
 libs/kimath/src/geometry/shape_poly_set.cpp   | 39 +++++++++++--------
 pcbnew/zone.cpp                               | 33 +---------------
 pcbnew/zone.h                                 | 33 +++-------------
 .../test_shape_poly_set_collision.cpp         | 15 ++-----
 6 files changed, 37 insertions(+), 94 deletions(-)

diff --git a/common/eda_shape.cpp b/common/eda_shape.cpp
index cf062a7d97..0675085a3a 100644
--- a/common/eda_shape.cpp
+++ b/common/eda_shape.cpp
@@ -795,14 +795,9 @@ bool EDA_SHAPE::hitTest( const VECTOR2I& aPosition, int aAccuracy ) const
 
     case SHAPE_T::POLY:
         if( IsFilled() )
-        {
             return m_poly.Collide( VECTOR2I( aPosition ), maxdist );
-        }
         else
-        {
-            SHAPE_POLY_SET::VERTEX_INDEX dummy;
-            return m_poly.CollideEdge( VECTOR2I( aPosition ), dummy, maxdist );
-        }
+            return m_poly.CollideEdge( VECTOR2I( aPosition ), nullptr, maxdist );
 
     default:
         UNIMPLEMENTED_FOR( SHAPE_T_asString() );
diff --git a/libs/kimath/include/geometry/shape_poly_set.h b/libs/kimath/include/geometry/shape_poly_set.h
index 40a15d00f8..7b6e826762 100644
--- a/libs/kimath/include/geometry/shape_poly_set.h
+++ b/libs/kimath/include/geometry/shape_poly_set.h
@@ -1161,7 +1161,7 @@ public:
      * @param aClosestVertex is the index of the closes vertex to \p aPoint.
      * @return bool - true if there is a collision, false in any other case.
      */
-    bool CollideVertex( const VECTOR2I& aPoint, VERTEX_INDEX& aClosestVertex,
+    bool CollideVertex( const VECTOR2I& aPoint, VERTEX_INDEX* aClosestVertex = nullptr,
                         int aClearance = 0 ) const;
 
     /**
@@ -1174,7 +1174,7 @@ public:
      * @param aClosestVertex is the index of the closes vertex to \p aPoint.
      * @return bool - true if there is a collision, false in any other case.
      */
-    bool CollideEdge( const VECTOR2I& aPoint, VERTEX_INDEX& aClosestVertex,
+    bool CollideEdge( const VECTOR2I& aPoint, VERTEX_INDEX* aClosestVertex = nullptr,
                       int aClearance = 0 ) const;
 
     /**
diff --git a/libs/kimath/src/geometry/shape_poly_set.cpp b/libs/kimath/src/geometry/shape_poly_set.cpp
index b221264fde..1d9a756b23 100644
--- a/libs/kimath/src/geometry/shape_poly_set.cpp
+++ b/libs/kimath/src/geometry/shape_poly_set.cpp
@@ -1700,7 +1700,7 @@ void SHAPE_POLY_SET::Append( const VECTOR2I& aP, int aOutline, int aHole )
 
 
 bool SHAPE_POLY_SET::CollideVertex( const VECTOR2I& aPoint,
-                                    SHAPE_POLY_SET::VERTEX_INDEX& aClosestVertex,
+                                    SHAPE_POLY_SET::VERTEX_INDEX* aClosestVertex,
                                     int aClearance ) const
 {
     // Shows whether there was a collision
@@ -1708,10 +1708,8 @@ bool SHAPE_POLY_SET::CollideVertex( const VECTOR2I& aPoint,
 
     // Difference vector between each vertex and aPoint.
     VECTOR2D    delta;
-    double      distance, clearance;
-
-    // Convert clearance to double for precision when comparing distances
-    clearance = aClearance;
+    ecoord      distance_squared;
+    ecoord      clearance_squared = SEG::Square( aClearance );
 
     for( CONST_ITERATOR iterator = CIterateWithHoles(); iterator; iterator++ )
     {
@@ -1719,18 +1717,21 @@ bool SHAPE_POLY_SET::CollideVertex( const VECTOR2I& aPoint,
         delta = *iterator - aPoint;
 
         // Compute distance
-        distance = delta.EuclideanNorm();
+        distance_squared = delta.SquaredEuclideanNorm();
 
         // Check for collisions
-        if( distance <= clearance )
+        if( distance_squared <= clearance_squared )
         {
+            if( !aClosestVertex )
+                return true;
+
             collision = true;
 
-            // Update aClearance to look for closer vertices
-            clearance = distance;
+            // Update clearance to look for closer vertices
+            clearance_squared = distance_squared;
 
             // Store the indices that identify the vertex
-            aClosestVertex = iterator.GetIndex();
+            *aClosestVertex = iterator.GetIndex();
         }
     }
 
@@ -1739,27 +1740,31 @@ bool SHAPE_POLY_SET::CollideVertex( const VECTOR2I& aPoint,
 
 
 bool SHAPE_POLY_SET::CollideEdge( const VECTOR2I& aPoint,
-                                  SHAPE_POLY_SET::VERTEX_INDEX& aClosestVertex,
+                                  SHAPE_POLY_SET::VERTEX_INDEX* aClosestVertex,
                                   int aClearance ) const
 {
     // Shows whether there was a collision
-    bool collision = false;
+    bool   collision = false;
+    ecoord clearance_squared = SEG::Square( aClearance );
 
     for( CONST_SEGMENT_ITERATOR iterator = CIterateSegmentsWithHoles(); iterator; iterator++ )
     {
         const SEG currentSegment = *iterator;
-        int distance = currentSegment.Distance( aPoint );
+        ecoord    distance_squared = currentSegment.SquaredDistance( aPoint );
 
         // Check for collisions
-        if( distance <= aClearance )
+        if( distance_squared <= clearance_squared )
         {
+            if( !aClosestVertex )
+                return true;
+
             collision = true;
 
-            // Update aClearance to look for closer edges
-            aClearance = distance;
+            // Update clearance to look for closer edges
+            clearance_squared = distance_squared;
 
             // Store the indices that identify the vertex
-            aClosestVertex = iterator.GetIndex();
+            *aClosestVertex = iterator.GetIndex();
         }
     }
 
diff --git a/pcbnew/zone.cpp b/pcbnew/zone.cpp
index bfb4f1be2a..d4b85030db 100644
--- a/pcbnew/zone.cpp
+++ b/pcbnew/zone.cpp
@@ -375,49 +375,20 @@ bool ZONE::HitTest( const VECTOR2I& aPosition, int aAccuracy ) const
 }
 
 
-void ZONE::SetSelectedCorner( const VECTOR2I& aPosition, int aAccuracy )
-{
-    SHAPE_POLY_SET::VERTEX_INDEX corner;
-
-    // If there is some corner to be selected, assign it to m_CornerSelection
-    if( HitTestForCorner( aPosition, aAccuracy * 2, corner )
-        || HitTestForEdge( aPosition, aAccuracy, corner ) )
-    {
-        if( m_CornerSelection == nullptr )
-            m_CornerSelection = new SHAPE_POLY_SET::VERTEX_INDEX;
-
-        *m_CornerSelection = corner;
-    }
-}
-
 bool ZONE::HitTestForCorner( const VECTOR2I& refPos, int aAccuracy,
-                             SHAPE_POLY_SET::VERTEX_INDEX& aCornerHit ) const
+                             SHAPE_POLY_SET::VERTEX_INDEX* aCornerHit ) const
 {
     return m_Poly->CollideVertex( VECTOR2I( refPos ), aCornerHit, aAccuracy );
 }
 
 
-bool ZONE::HitTestForCorner( const VECTOR2I& refPos, int aAccuracy ) const
-{
-    SHAPE_POLY_SET::VERTEX_INDEX dummy;
-    return HitTestForCorner( refPos, aAccuracy, dummy );
-}
-
-
 bool ZONE::HitTestForEdge( const VECTOR2I& refPos, int aAccuracy,
-                           SHAPE_POLY_SET::VERTEX_INDEX& aCornerHit ) const
+                           SHAPE_POLY_SET::VERTEX_INDEX* aCornerHit ) const
 {
     return m_Poly->CollideEdge( VECTOR2I( refPos ), aCornerHit, aAccuracy );
 }
 
 
-bool ZONE::HitTestForEdge( const VECTOR2I& refPos, int aAccuracy ) const
-{
-    SHAPE_POLY_SET::VERTEX_INDEX dummy;
-    return HitTestForEdge( refPos, aAccuracy, dummy );
-}
-
-
 bool ZONE::HitTest( const EDA_RECT& aRect, bool aContained, int aAccuracy ) const
 {
     // Calculate bounding box for zone
diff --git a/pcbnew/zone.h b/pcbnew/zone.h
index 649f501cc0..c29c30c21c 100644
--- a/pcbnew/zone.h
+++ b/pcbnew/zone.h
@@ -321,9 +321,6 @@ public:
     }
 
     ///
-    // Like HitTest but selects the current corner to be operated on
-    void SetSelectedCorner( const VECTOR2I& aPosition, int aAccuracy );
-
     int GetLocalFlags() const { return m_localFlgs; }
     void SetLocalFlags( int aFlags ) { m_localFlgs = aFlags; }
 
@@ -423,43 +420,25 @@ public:
      *
      * @param  refPos     is the VECTOR2I to test.
      * @param  aAccuracy  increase the item bounding box by this amount.
-     * @param  aCornerHit [out] is the index of the closest vertex found, useless when return
-     *                    value is false.
+     * @param  aCornerHit [out, optional] is the index of the closest vertex found when return
+     *                    value is true
      * @return true if some corner was found to be closer to refPos than aClearance; false
      *         otherwise.
      */
     bool HitTestForCorner( const VECTOR2I& refPos, int aAccuracy,
-                           SHAPE_POLY_SET::VERTEX_INDEX& aCornerHit ) const;
-
-    /**
-     * Test if the given VECTOR2I is near a corner.
-     * @param  refPos     is the VECTOR2I to test.
-     * @param  aAccuracy  increase the item bounding box by this amount.
-     * @return true if some corner was found to be closer to refPos than aClearance; false
-     *         otherwise.
-     */
-    bool HitTestForCorner( const VECTOR2I& refPos, int aAccuracy ) const;
+                           SHAPE_POLY_SET::VERTEX_INDEX* aCornerHit = nullptr ) const;
 
     /**
      * Test if the given VECTOR2I is near a segment defined by 2 corners.
      *
      * @param  refPos     is the VECTOR2I to test.
      * @param  aAccuracy  increase the item bounding box by this amount.
-     * @param  aCornerHit [out] is the index of the closest vertex found, useless when return
-     *                    value is false.
+     * @param  aCornerHit [out, optional] is the index of the closest vertex found when return
+     *                    value is true.
      * @return true if some edge was found to be closer to refPos than aClearance.
      */
     bool HitTestForEdge( const VECTOR2I& refPos, int aAccuracy,
-                         SHAPE_POLY_SET::VERTEX_INDEX& aCornerHit ) const;
-
-    /**
-     * Test if the given VECTOR2I is near a segment defined by 2 corners.
-     *
-     * @param  refPos     is the VECTOR2I to test.
-     * @param  aAccuracy  increase the item bounding box by this amount.
-     * @return true if some edge was found to be closer to refPos than aClearance.
-     */
-    bool HitTestForEdge( const VECTOR2I& refPos, int aAccuracy ) const;
+                         SHAPE_POLY_SET::VERTEX_INDEX* aCornerHit = nullptr ) const;
 
     /**
      * @copydoc BOARD_ITEM::HitTest(const EDA_RECT& aRect,
diff --git a/qa/unittests/libs/kimath/geometry/test_shape_poly_set_collision.cpp b/qa/unittests/libs/kimath/geometry/test_shape_poly_set_collision.cpp
index e80fd313b6..22ed16a11e 100644
--- a/qa/unittests/libs/kimath/geometry/test_shape_poly_set_collision.cpp
+++ b/qa/unittests/libs/kimath/geometry/test_shape_poly_set_collision.cpp
@@ -214,14 +214,12 @@ BOOST_AUTO_TEST_CASE( Collide )
  */
 BOOST_AUTO_TEST_CASE( CollideVertex )
 {
-    // Variable to store the index of the corner hit
-    SHAPE_POLY_SET::VERTEX_INDEX cornerHit;
-
     // Check that the set collides with the colliding points
     for( const VECTOR2I& point : common.holeyPoints )
     {
-        BOOST_CHECK_MESSAGE( common.holeyPolySet.CollideVertex( point, cornerHit, 0 ), " Point "
-                << point.x << ", " << point.y << " does not collide with holeyPolySet polygon" );
+        BOOST_CHECK_MESSAGE( common.holeyPolySet.CollideVertex( point, nullptr, 0 ),
+                             " Point " << point.x << ", " << point.y <<
+                             " does not collide with holeyPolySet polygon" );
     }
 }
 
@@ -231,14 +229,9 @@ BOOST_AUTO_TEST_CASE( CollideVertex )
  */
 BOOST_AUTO_TEST_CASE( CollideVertexWithClearance )
 {
-    // Variable to store the index of the corner hit
-    SHAPE_POLY_SET::VERTEX_INDEX cornerHit;
-
     // Check that the set collides with the colliding points
     for( const VECTOR2I& point : common.holeyPoints )
-    {
-        BOOST_CHECK( common.holeyPolySet.CollideVertex( point + VECTOR2I( 1, 1 ), cornerHit, 2 ) );
-    }
+        BOOST_CHECK( common.holeyPolySet.CollideVertex( point + VECTOR2I( 1, 1 ), nullptr, 2 ) );
 }