From eb1ff80d57c99611ea8112f22e417b250dc3abc3 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 2 Jul 2020 21:34:36 +0100 Subject: [PATCH] SHAPE collision fixes. 1) An actual distance of 0 is still a collision, even if the allowed distance is 0. 2) Be consitent about edges and interiors. Everyone expect the edge of a RECT to be part of the RECT; same with a CIRCLE. SHAPE_POLY_SET shouldn't be any different. (And SHAPE_LINE_CHAIN was a split- personality with the edge considered part of it for Collide() but not for PointInside()). --- libs/kimath/include/geometry/shape_circle.h | 2 +- libs/kimath/include/geometry/shape_poly_set.h | 24 ++++++++++++++----- libs/kimath/include/geometry/shape_segment.h | 4 ++-- libs/kimath/src/geometry/seg.cpp | 2 +- libs/kimath/src/geometry/shape_arc.cpp | 2 +- libs/kimath/src/geometry/shape_collisions.cpp | 2 +- libs/kimath/src/geometry/shape_line_chain.cpp | 20 +++++++--------- libs/kimath/src/geometry/shape_poly_set.cpp | 4 ++-- pcbnew/router/pns_kicad_iface.cpp | 2 +- .../test_shape_poly_set_collision.cpp | 22 ++++++++++++++--- 10 files changed, 54 insertions(+), 30 deletions(-) diff --git a/libs/kimath/include/geometry/shape_circle.h b/libs/kimath/include/geometry/shape_circle.h index 0884f320e1..4e4e4153f8 100644 --- a/libs/kimath/include/geometry/shape_circle.h +++ b/libs/kimath/include/geometry/shape_circle.h @@ -68,7 +68,7 @@ public: int minDist = aClearance + m_radius; ecoord dist_sq = aSeg.SquaredDistance( m_center ); - if( dist_sq < (ecoord) minDist * minDist ) + if( dist_sq == 0 || dist_sq < (ecoord) minDist * minDist ) { if( aActual ) *aActual = std::max( 0, (int) sqrt( dist_sq ) - m_radius ); diff --git a/libs/kimath/include/geometry/shape_poly_set.h b/libs/kimath/include/geometry/shape_poly_set.h index 1dd14e3860..b126f4935f 100644 --- a/libs/kimath/include/geometry/shape_poly_set.h +++ b/libs/kimath/include/geometry/shape_poly_set.h @@ -996,9 +996,15 @@ class SHAPE_POLY_SET : public SHAPE /** * Function Collide - * Checks whether the point aP collides with the inside of the polygon set; if the point - * lies on an edge or on a corner of any of the polygons, there is no collision: the edges - * does not belong to the polygon itself. + * Checks whether the point aP is either inside or on the edge of the polygon set. + * + * Note that prior to Jul 2020 we considered the edge to *not* be part of the polygon. + * However, most other shapes (rects, circles, segments, etc.) include their edges and + * the difference was causing issues when used for DRC. + * + * (FWIW, SHAPE_LINE_CHAIN was a split personality, with Collide() including its edges + * but PointInside() not. That has also been corrected.) + * * @param aP is the VECTOR2I point whose collision with respect to the poly set * will be tested. * @param aClearance is the security distance; if the point lies closer to the polygon @@ -1011,9 +1017,15 @@ class SHAPE_POLY_SET : public SHAPE /** * Function Collide - * Checks whether the segment aSeg collides with the inside of the polygon set; if the - * segment touches an edge or a corner of any of the polygons, there is no collision: - * the edges do not belong to the polygon itself. + * Checks whether the segment aSeg collides with the polygon set (or its edge). + * + * Note that prior to Jul 2020 we considered the edge to *not* be part of the polygon. + * However, most other shapes (rects, circles, segments, etc.) include their edges and + * the difference was causing issues when used for DRC. + * + * (FWIW, SHAPE_LINE_CHAIN was a split personality, with Collide() including its edges + * but PointInside() not. That has also been corrected.) + * * @param aSeg is the SEG segment whose collision with respect to the poly set * will be tested. * @param aClearance is the security distance; if the segment passes closer to the polygon diff --git a/libs/kimath/include/geometry/shape_segment.h b/libs/kimath/include/geometry/shape_segment.h index 584f0291fc..0c9f4a73e1 100644 --- a/libs/kimath/include/geometry/shape_segment.h +++ b/libs/kimath/include/geometry/shape_segment.h @@ -59,7 +59,7 @@ public: int min_dist = ( m_width + 1 ) / 2 + aClearance; ecoord dist_sq = m_seg.SquaredDistance( aSeg ); - if( dist_sq < (ecoord) min_dist * min_dist ) + if( dist_sq == 0 || dist_sq < (ecoord) min_dist * min_dist ) { if( aActual ) *aActual = std::max( 0, (int) sqrt( dist_sq ) - ( m_width + 1 ) / 2 ); @@ -75,7 +75,7 @@ public: int min_dist = ( m_width + 1 ) / 2 + aClearance; ecoord dist_sq = m_seg.SquaredDistance( aP ); - if( dist_sq < (ecoord) min_dist * min_dist ) + if( dist_sq == 0 || dist_sq < (ecoord) min_dist * min_dist ) { if( aActual ) *aActual = std::max( 0, (int) sqrt( dist_sq ) - ( m_width + 1 ) / 2 ); diff --git a/libs/kimath/src/geometry/seg.cpp b/libs/kimath/src/geometry/seg.cpp index 9fabbc53ea..b0fd5215f6 100644 --- a/libs/kimath/src/geometry/seg.cpp +++ b/libs/kimath/src/geometry/seg.cpp @@ -145,7 +145,7 @@ bool SEG::Collide( const SEG& aSeg, int aClearance, int* aActual ) const dist_sq = std::min( dist_sq, aSeg.SquaredDistance( A ) ); dist_sq = std::min( dist_sq, aSeg.SquaredDistance( B ) ); - if( dist_sq < (ecoord) aClearance * aClearance ) + if( dist_sq == 0 || dist_sq < (ecoord) aClearance * aClearance ) { if( aActual ) *aActual = sqrt( dist_sq ); diff --git a/libs/kimath/src/geometry/shape_arc.cpp b/libs/kimath/src/geometry/shape_arc.cpp index 6a9d7e06a9..d457d3bf6f 100644 --- a/libs/kimath/src/geometry/shape_arc.cpp +++ b/libs/kimath/src/geometry/shape_arc.cpp @@ -98,7 +98,7 @@ bool SHAPE_ARC::Collide( const SEG& aSeg, int aClearance, int* aActual ) const dist_sq = std::min( dist_sq, aSeg.SquaredDistance( m_start ) ); dist_sq = std::min( dist_sq, aSeg.SquaredDistance( m_end ) ); - if( dist_sq < (ecoord) minDist * minDist ) + if( dist_sq == 0 || dist_sq < (ecoord) minDist * minDist ) { if( aActual ) *aActual = std::max( 0, (int) sqrt( dist_sq ) - m_width / 2 ); diff --git a/libs/kimath/src/geometry/shape_collisions.cpp b/libs/kimath/src/geometry/shape_collisions.cpp index e67cbc0420..103f187a4f 100644 --- a/libs/kimath/src/geometry/shape_collisions.cpp +++ b/libs/kimath/src/geometry/shape_collisions.cpp @@ -102,7 +102,7 @@ static inline bool Collide( const SHAPE_RECT& aA, const SHAPE_CIRCLE& aB, int aC VECTOR2I pn = side.NearestPoint( c ); int side_dist_sq = ( pn - c ).SquaredEuclideanNorm(); - if(( side_dist_sq < min_dist_sq ) && !aMTV && !aActual ) + if( ( side_dist_sq == 0 || side_dist_sq < min_dist_sq ) && !aMTV && !aActual ) return true; if( side_dist_sq < nearest_side_dist_sq ) diff --git a/libs/kimath/src/geometry/shape_line_chain.cpp b/libs/kimath/src/geometry/shape_line_chain.cpp index e72927827e..1e47481571 100644 --- a/libs/kimath/src/geometry/shape_line_chain.cpp +++ b/libs/kimath/src/geometry/shape_line_chain.cpp @@ -95,11 +95,11 @@ bool SHAPE_LINE_CHAIN::Collide( const VECTOR2I& aP, int aClearance, int* aActual const SEG& s = CSegment( i ); dist_sq = std::min( dist_sq, s.SquaredDistance( aP ) ); - if( !aActual && dist_sq < clearance_sq ) + if( ( dist_sq == 0 || dist_sq < clearance_sq ) && !aActual ) return true; } - if( dist_sq < clearance_sq ) + if( dist_sq == 0 || dist_sq < clearance_sq ) { if( aActual ) *aActual = sqrt( dist_sq ); @@ -135,11 +135,11 @@ bool SHAPE_LINE_CHAIN::Collide( const SEG& aSeg, int aClearance, int* aActual ) const SEG& s = CSegment( i ); dist_sq = std::min( dist_sq, s.SquaredDistance( aSeg ) ); - if( !aActual && dist_sq < clearance_sq ) + if( ( dist_sq == 0 || dist_sq < clearance_sq ) && !aActual ) return true; } - if( dist_sq < clearance_sq ) + if( dist_sq == 0 || dist_sq < clearance_sq ) { if( aActual ) *aActual = sqrt( dist_sq ); @@ -644,16 +644,12 @@ bool SHAPE_LINE_CHAIN::PointInside( const VECTOR2I& aPt, int aAccuracy, bool aUs } } - // If accuracy is 0 then we need to make sure the point isn't actually on the edge. - // If accuracy is 1 then we don't really care whether or not the point is *exactly* on the - // edge, so we skip edge processing for performance. - // If accuracy is > 1, then we use "OnEdge(accuracy-1)" as a proxy for "Inside(accuracy)". - if( aAccuracy == 0 ) - return inside && !PointOnEdge( aPt ); - else if( aAccuracy == 1 ) + // If accuracy is <= 1 (nm) then we skip the accuracy test for performance. Otherwise + // we use "OnEdge(accuracy)" as a proxy for "Inside(accuracy)". + if( aAccuracy <= 1 ) return inside; else - return inside || PointOnEdge( aPt, aAccuracy - 1 ); + return inside || PointOnEdge( aPt, aAccuracy ); } diff --git a/libs/kimath/src/geometry/shape_poly_set.cpp b/libs/kimath/src/geometry/shape_poly_set.cpp index 013b63284a..6be4a3c882 100644 --- a/libs/kimath/src/geometry/shape_poly_set.cpp +++ b/libs/kimath/src/geometry/shape_poly_set.cpp @@ -1228,7 +1228,7 @@ bool SHAPE_POLY_SET::Collide( const SEG& aSeg, int aClearance, int* aActual ) co { ecoord dist_sq = SquaredDistance( aSeg ); - if( dist_sq < (ecoord) aClearance * aClearance ) + if( dist_sq == 0 || dist_sq < (ecoord) aClearance * aClearance ) { if( aActual ) *aActual = sqrt( dist_sq ); @@ -1244,7 +1244,7 @@ bool SHAPE_POLY_SET::Collide( const VECTOR2I& aP, int aClearance, int* aActual ) { ecoord dist_sq = SquaredDistance( aP ); - if( dist_sq < (ecoord) aClearance * aClearance ) + if( dist_sq == 0 || dist_sq < (ecoord) aClearance * aClearance ) { if( aActual ) *aActual = sqrt( dist_sq ); diff --git a/pcbnew/router/pns_kicad_iface.cpp b/pcbnew/router/pns_kicad_iface.cpp index 15cdeb0024..1ebeac416c 100644 --- a/pcbnew/router/pns_kicad_iface.cpp +++ b/pcbnew/router/pns_kicad_iface.cpp @@ -218,7 +218,7 @@ bool PNS_PCBNEW_RULE_RESOLVER::CollideHoles( const PNS::ITEM* aA, const PNS::ITE ecoord dist_sq = delta.SquaredEuclideanNorm(); - if( dist_sq < min_dist_sq ) + if( dist_sq == 0 || dist_sq < min_dist_sq ) { if( aNeedMTV ) *aMTV = delta.Resize( min_dist - sqrt( dist_sq ) + 3 ); // fixme: apparent rounding error diff --git a/qa/common/geometry/test_shape_poly_set_collision.cpp b/qa/common/geometry/test_shape_poly_set_collision.cpp index 6fd23e814e..d3127ceb22 100644 --- a/qa/common/geometry/test_shape_poly_set_collision.cpp +++ b/qa/common/geometry/test_shape_poly_set_collision.cpp @@ -57,10 +57,10 @@ struct CollisionFixture // On a hole edge => inside the polygon collidingPoints.emplace_back( 40, 25 ); - // Create points not colliding with the poly set. + // On the outline edge => inside the polygon + collidingPoints.emplace_back( 0, 10 ); - // On the outline edge => outside the polygon - nonCollidingPoints.emplace_back( 0, 10 ); + // Create points not colliding with the poly set. // Completely outside of the polygon nonCollidingPoints.emplace_back( 200, 200 ); @@ -122,12 +122,20 @@ BOOST_AUTO_TEST_CASE( pointInPolygonSet ) // Check that the set contains the points that collide with it for( const VECTOR2I& point : collidingPoints ) { + std::stringstream ss; + ss << "Point {" << point.x << ", " << point.y << " }"; + BOOST_TEST_INFO( ss.str() ); + BOOST_CHECK( common.holeyPolySet.Contains( point ) ); } // Check that the set does not contain any point outside of it for( const VECTOR2I& point : nonCollidingPoints ) { + std::stringstream ss; + ss << "Point {" << point.x << ", " << point.y << " }"; + BOOST_TEST_INFO( ss.str() ); + BOOST_CHECK( !common.holeyPolySet.Contains( point ) ); } } @@ -142,12 +150,20 @@ BOOST_AUTO_TEST_CASE( Collide ) // Check that the set collides with the colliding points for( const VECTOR2I& point : collidingPoints ) { + std::stringstream ss; + ss << "Point {" << point.x << ", " << point.y << " }"; + BOOST_TEST_INFO( ss.str() ); + BOOST_CHECK( common.holeyPolySet.Collide( point, 0 ) ); } // Check that the set does not collide with the non colliding points for( const VECTOR2I& point : nonCollidingPoints ) { + std::stringstream ss; + ss << "Point {" << point.x << ", " << point.y << " }"; + BOOST_TEST_INFO( ss.str() ); + BOOST_CHECK( !common.holeyPolySet.Collide( point, 0 ) ); }