From 511c80b2ec3c8d8eb8a71ddfab56b2c071b956d0 Mon Sep 17 00:00:00 2001 From: PouleyKetchoupp Date: Wed, 25 Aug 2021 13:34:32 -0700 Subject: [PATCH] Fix segment intersection consistency in Geometry2D Segment collision results could be different depending on the direction when they exactly touch (order of the points in segments). This was due to the way parallelism was checked, using different logic based on positive or negative sign of cross products. Now the results are the same whatever the direction, without changing the current design, which is that parallel or colinear segments are not considered colinear. Fixes inconsistencies with raycasts exactly on edges of convex shapes depending on the direction. --- core/math/geometry_2d.h | 20 +++++++++++++++++--- tests/test_geometry_2d.h | 33 ++++++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/core/math/geometry_2d.h b/core/math/geometry_2d.h index e1a5bfe6f2..8e5830f9b3 100644 --- a/core/math/geometry_2d.h +++ b/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; + } } } diff --git a/tests/test_geometry_2d.h b/tests/test_geometry_2d.h index 32d4114a1c..25af8c355e 100644 --- a/tests/test_geometry_2d.h +++ b/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") {