From fba2b7a04a7a9ca3ea3c0d7ef1d7cf885318d3d4 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <seth@kipro-pcb.com>
Date: Fri, 16 Aug 2024 17:02:48 -0700
Subject: [PATCH] Modify previous commit to correct logic
We are calculating the same value in two different ways. If there is an
off-by-one issue in our calculation along axis, this still means that it
was a direct hit on the line as seen in the unit test cases
---
libs/kimath/src/geometry/seg.cpp | 22 +++++++++----------
.../libs/kimath/geometry/test_segment.cpp | 18 ++++++++++++---
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/libs/kimath/src/geometry/seg.cpp b/libs/kimath/src/geometry/seg.cpp
index 2f596237e2..ad98c74646 100644
--- a/libs/kimath/src/geometry/seg.cpp
+++ b/libs/kimath/src/geometry/seg.cpp
@@ -399,8 +399,8 @@ int SEG::Distance( const VECTOR2I& aP ) const
SEG::ecoord SEG::SquaredDistance( const VECTOR2I& aP ) const
{
- VECTOR2L ab = VECTOR2L( B.x - A.x, B.y - A.y );
- VECTOR2L ap = VECTOR2L( aP.x - A.x, aP.y - A.y );
+ VECTOR2L ab = VECTOR2L( B ) - A;
+ VECTOR2L ap = VECTOR2L( aP ) - A;
ecoord e = ap.Dot( ab );
@@ -410,18 +410,18 @@ SEG::ecoord SEG::SquaredDistance( const VECTOR2I& aP ) const
ecoord f = ab.SquaredEuclideanNorm();
if( e >= f )
- return VECTOR2L( aP.x - B.x, aP.y - B.y ).SquaredEuclideanNorm();
+ return ( VECTOR2L( aP ) - B ).Dot( VECTOR2L( aP ) - B );
- const double g = ( double( e ) * e ) / f;
+ const double g = ap.SquaredEuclideanNorm() - ( double( e ) * e ) / f;
- //Squared distance can't be negative
- if( g > ap.SquaredEuclideanNorm() )
- {
- return (ecoord) std::numeric_limits<std::int32_t>::max()
- * std::numeric_limits<std::int32_t>::max();
- }
+ // The only way g can be negative is if there was a rounding error since
+ // e is the projection of aP onto ab and therefore cannot be greater than
+ // the length of ap and f is guaranteed to be greater than e, meaning
+ // e * e / f cannot be greater than ap.SquaredEuclideanNorm()
+ if( g < 0 )
+ return 0;
- return KiROUND<double, ecoord>( ap.SquaredEuclideanNorm() - g );
+ return KiROUND<double, ecoord>( g );
}
diff --git a/qa/tests/libs/kimath/geometry/test_segment.cpp b/qa/tests/libs/kimath/geometry/test_segment.cpp
index 65a82f3fcd..317be0b9cf 100644
--- a/qa/tests/libs/kimath/geometry/test_segment.cpp
+++ b/qa/tests/libs/kimath/geometry/test_segment.cpp
@@ -346,11 +346,23 @@ static const std::vector<SEG_VECTOR_DISTANCE_CASE> seg_vec_dist_cases = {
282, // sqrt(200^2 + 200^2) = 282.8, rounded to nearest
},
{
- "Issue 18473 (distance negative)",
+ "Issue 18473 (inside hit with rounding error)",
{ { 187360000, 42510000 }, { 105796472, 42510000 } },
{ 106645000, 42510000 },
- std::numeric_limits<std::int32_t>::max(), // maximal distance
- }
+ 0,
+ },
+ {
+ "Straight line x distance",
+ { { 187360000, 42510000 }, { 105796472, 42510000 } },
+ { 197360000, 42510000 },
+ 10000000,
+ },
+ {
+ "Straight line -x distance",
+ { { 187360000, 42510000 }, { 105796472, 42510000 } },
+ { 104796472, 42510000 },
+ 1000000,
+ },
};
// clang-format on