Merge pull request #52110 from nekomatata/fix-segment-intersection

Fix segment intersection consistency in Geometry2D

core/math/geometry_2d.h

@@ -182,7 +182,15 @@ public:
 		C = Vector2(C.x * Bn.x + C.y * Bn.y, C.y * Bn.x - C.x * Bn.y);
 		D = Vector2(D.x * Bn.x + D.y * Bn.y, D.y * Bn.x - D.x * Bn.y);
 
-		if ((C.y < 0 && D.y < 0) || (C.y >= 0 && D.y >= 0)) {
+		// Fail if C x B and D x B have the same sign (segments don't intersect).
+		// (equivalent to condition (C.y < 0 && D.y < CMP_EPSILON) || (C.y > 0 && D.y > CMP_EPSILON))
+		if (C.y * D.y > CMP_EPSILON) {
+			return false;
+		}
+
+		// Fail if segments are parallel or colinear.
+		// (when A x B == zero, i.e (C - D) x B == zero, i.e C x B == D x B)
+		if (Math::is_equal_approx(C.y, D.y)) {
 			return false;
 		}
 
@@ -193,7 +201,7 @@ public:
 			return false;
 		}
 
-		// (4) Apply the discovered position to line A-B in the original coordinate system.
+		// Apply the discovered position to line A-B in the original coordinate system.
 		if (r_result) {
 			*r_result = p_from_a + B * ABpos;
 		}
@@ -353,8 +361,14 @@ public:
 		for (int i = 0; i < c; i++) {
 			const Vector2 &v1 = p[i];
 			const Vector2 &v2 = p[(i + 1) % c];
-			if (segment_intersects_segment(v1, v2, p_point, further_away, nullptr)) {
+
+			Vector2 res;
+			if (segment_intersects_segment(v1, v2, p_point, further_away, &res)) {
 				intersections++;
+				if (res.is_equal_approx(p_point)) {
+					// Point is in one of the polygon edges.
+					return true;
+				}
 			}
 		}
 

tests/test_geometry_2d.h

@@ -51,8 +51,6 @@ TEST_CASE("[Geometry2D] Point in circle") {
 	CHECK_FALSE(Geometry2D::is_point_in_circle(Vector2(7, -42), Vector2(4, -40), 3.5));
 
 	// This tests points on the edge of the circle. They are treated as being inside the circle.
-	// In `is_point_in_triangle` and `is_point_in_polygon` they are treated as being outside, so in order the make
-	// the behaviour consistent this may change in the future (see issue #44717 and PR #44274).
 	CHECK(Geometry2D::is_point_in_circle(Vector2(1.0, 0.0), Vector2(0, 0), 1.0));
 	CHECK(Geometry2D::is_point_in_circle(Vector2(0.0, -1.0), Vector2(0, 0), 1.0));
 }
@@ -66,7 +64,7 @@ TEST_CASE("[Geometry2D] Point in triangle") {
 	CHECK_FALSE(Geometry2D::is_point_in_triangle(Vector2(0, 0), Vector2(1, 4), Vector2(3, 2), Vector2(5, 4)));
 
 	// This tests points on the edge of the triangle. They are treated as being outside the triangle.
-	// In `is_point_in_circle` they are treated as being inside, so in order the make
+	// In `is_point_in_circle` and `is_point_in_polygon` they are treated as being inside, so in order the make
 	// the behaviour consistent this may change in the future (see issue #44717 and PR #44274).
 	CHECK_FALSE(Geometry2D::is_point_in_triangle(Vector2(1, 1), Vector2(-1, 1), Vector2(0, -1), Vector2(1, 1)));
 	CHECK_FALSE(Geometry2D::is_point_in_triangle(Vector2(0, 1), Vector2(-1, 1), Vector2(0, -1), Vector2(1, 1)));
@@ -95,11 +93,16 @@ TEST_CASE("[Geometry2D] Point in polygon") {
 	CHECK(Geometry2D::is_point_in_polygon(Vector2(370, 55), p));
 	CHECK(Geometry2D::is_point_in_polygon(Vector2(-160, 190), p));
 
-	// This tests points on the edge of the polygon. They are treated as being outside the polygon.
-	// In `is_point_in_circle` they are treated as being inside, so in order the make
-	// the behaviour consistent this may change in the future (see issue #44717 and PR #44274).
-	CHECK_FALSE(Geometry2D::is_point_in_polygon(Vector2(68, 112), p));
-	CHECK_FALSE(Geometry2D::is_point_in_polygon(Vector2(-88, 120), p));
+	// This tests points on the edge of the polygon. They are treated as being inside the polygon.
+	int c = p.size();
+	for (int i = 0; i < c; i++) {
+		const Vector2 &p1 = p[i];
+		CHECK(Geometry2D::is_point_in_polygon(p1, p));
+
+		const Vector2 &p2 = p[(i + 1) % c];
+		Vector2 midpoint((p1 + p2) * 0.5);
+		CHECK(Geometry2D::is_point_in_polygon(midpoint, p));
+	}
 }
 
 TEST_CASE("[Geometry2D] Polygon clockwise") {
@@ -140,9 +143,21 @@ TEST_CASE("[Geometry2D] Segment intersection.") {
 	CHECK(r.is_equal_approx(Vector2(0, 0)));
 
 	CHECK_FALSE(Geometry2D::segment_intersects_segment(Vector2(-1, 1), Vector2(1, -1), Vector2(1, 1), Vector2(0.1, 0.1), &r));
+	CHECK_FALSE(Geometry2D::segment_intersects_segment(Vector2(-1, 1), Vector2(1, -1), Vector2(0.1, 0.1), Vector2(1, 1), &r));
+
 	CHECK_FALSE_MESSAGE(
-			Geometry2D::segment_intersects_segment(Vector2(-1, 1), Vector2(1, -1), Vector2(0, 1), Vector2(1, -1), &r),
+			Geometry2D::segment_intersects_segment(Vector2(-1, 1), Vector2(1, -1), Vector2(0, 1), Vector2(2, -1), &r),
 			"Parallel segments should not intersect.");
+
+	CHECK_MESSAGE(
+			Geometry2D::segment_intersects_segment(Vector2(0, 0), Vector2(0, 1), Vector2(0, 0), Vector2(1, 0), &r),
+			"Touching segments should intersect.");
+	CHECK(r.is_equal_approx(Vector2(0, 0)));
+
+	CHECK_MESSAGE(
+			Geometry2D::segment_intersects_segment(Vector2(0, 1), Vector2(0, 0), Vector2(0, 0), Vector2(1, 0), &r),
+			"Touching segments should intersect.");
+	CHECK(r.is_equal_approx(Vector2(0, 0)));
 }
 
 TEST_CASE("[Geometry2D] Closest point to segment") {