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

@@ -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;

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 <tuple>
 #include <unit_test_utils/unit_test_utils.h>
 
 #include <geometry/shape_line_chain.h>
@@ -41,6 +42,11 @@ struct CollisionFixture
     // Vectors containing colliding and non-colliding points
     std::vector<VECTOR2I> collidingPoints, nonCollidingPoints;
 
+    // tuple of segment under test, collision result, and intersection point
+    typedef std::tuple<SEG, bool, VECTOR2I> SEG_CASE;
+
+    std::vector<SEG_CASE> 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()