From 2c05d99d9faf3ba26f37b214dfca2288e77ac33a Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Sun, 4 Apr 2021 20:12:45 -0400 Subject: [PATCH] SHAPE_POLY_SET: Fix segment collision testing It appears as though there was an optimization to skip testing segments if one of their endpoints (and only the A point) was inside. Unclear the reason for this, but I'm going to assume that it was intended to optimize the case where both points are inside (like the point case above it). --- libs/kimath/src/geometry/shape_poly_set.cpp | 8 +-- .../test_shape_poly_set_collision.cpp | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/libs/kimath/src/geometry/shape_poly_set.cpp b/libs/kimath/src/geometry/shape_poly_set.cpp index bf4eecc949..da88c420a2 100644 --- a/libs/kimath/src/geometry/shape_poly_set.cpp +++ b/libs/kimath/src/geometry/shape_poly_set.cpp @@ -1725,11 +1725,9 @@ SEG::ecoord SHAPE_POLY_SET::SquaredDistanceToPolygon( VECTOR2I aPoint, int aPoly SEG::ecoord SHAPE_POLY_SET::SquaredDistanceToPolygon( const SEG& aSegment, int aPolygonIndex, VECTOR2I* aNearest ) const { - // We calculate the min dist between the segment and each outline segment. However, if the - // segment to test is inside the outline, and does not cross any edge, it can be seen outside - // the polygon. Therefore test if a segment end is inside (testing only one end is enough). - // Use an accuracy of "1" to say that we don't care if it's exactly on the edge or not. - if( containsSingle( aSegment.A, aPolygonIndex, 1 ) ) + // Check if the segment is fully-contained. If so, its midpoint is a good-enough nearest point. + if( containsSingle( aSegment.A, aPolygonIndex, 1 ) && + containsSingle( aSegment.B, aPolygonIndex, 1 ) ) { if( aNearest ) *aNearest = ( aSegment.A + aSegment.B ) / 2; diff --git a/qa/libs/kimath/geometry/test_shape_poly_set_collision.cpp b/qa/libs/kimath/geometry/test_shape_poly_set_collision.cpp index d3127ceb22..bf75e3692b 100644 --- a/qa/libs/kimath/geometry/test_shape_poly_set_collision.cpp +++ b/qa/libs/kimath/geometry/test_shape_poly_set_collision.cpp @@ -22,6 +22,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ +#include #include #include @@ -41,6 +42,11 @@ struct CollisionFixture // Vectors containing colliding and non-colliding points std::vector collidingPoints, nonCollidingPoints; + // tuple of segment under test, collision result, and intersection point + typedef std::tuple SEG_CASE; + + std::vector segs; + /** * Constructor */ @@ -67,6 +73,32 @@ struct CollisionFixture // Inside the outline and inside a hole => outside the polygon nonCollidingPoints.emplace_back( 15, 12 ); + + // Seg crossing the edge + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 90, 90 ), VECTOR2I( 110, 110 ) ), + true, VECTOR2I( 100, 100 ) ) ); + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 110, 110 ), VECTOR2I( 90, 90 ) ), + true, VECTOR2I( 100, 100 ) ) ); + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 50, -10 ), VECTOR2I( 50, 50 ) ), + true, VECTOR2I( 50, 0 ) ) ); + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 50, 50 ), VECTOR2I( 50, -10 ) ), + true, VECTOR2I( 50, 0 ) ) ); + + // Seg fully inside + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 80, 80 ), VECTOR2I( 90, 90 ) ), + true, VECTOR2I( 85, 85 ) ) ); + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 90, 90 ), VECTOR2I( 80, 80 ) ), + true, VECTOR2I( 85, 85 ) ) ); + + // Seg fully outside + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 110, 110 ), VECTOR2I( 120, 120 ) ), + false, VECTOR2I() ) ); + + // Seg touching + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 100, 100 ), VECTOR2I( 110, 110 ) ), + true, VECTOR2I( 100, 100 ) ) ); + segs.emplace_back( std::make_tuple( SEG( VECTOR2I( 110, 110 ), VECTOR2I( 100, 100 ) ), + true, VECTOR2I( 100, 100 ) ) ); } ~CollisionFixture() @@ -209,4 +241,27 @@ BOOST_AUTO_TEST_CASE( CollideVertexWithClearance ) } } + +/** + * Check that SHAPE_POLY_SET::Collide does the right thing for segments + */ +BOOST_AUTO_TEST_CASE( CollideSegments ) +{ + for( const SEG_CASE& testCase : segs ) + { + SEG seg; + bool expectedResult; + VECTOR2I expectedLocation; + + std::tie( seg, expectedResult, expectedLocation ) = testCase; + + VECTOR2I location; + + BOOST_CHECK( common.holeyPolySet.Collide( seg, 0, nullptr, &location ) == expectedResult ); + + if( expectedResult ) + BOOST_REQUIRE_EQUAL( location, expectedLocation ); + } +} + BOOST_AUTO_TEST_SUITE_END()