Fix segment_intersects_segment to give exact results.

[0xafbf]

EDIT: I am renaming this issue and adding some brief explanation because I think that the previous title doesn't tell what the PR does, and instead what it was aimed to fix when made.
I feel this PR should be merged because it fixes an essential math function. The current segment_intersects_segment does two things that drive to calculation inaccuracies. The first one is a division that follows the math formula, but introduces floating point errors. The second one adds a threshold to call an intersection when it wasn't there, and presumably it was made to aid in the case the division mentioned earlier ended up causing a false negative.

The first thing I did was to remove the comparisons that compare "almost equal", which did break some tests. After moving the division to happen later (when it is actually needed), the tests started passing again.

I guess the downside of this PR is that it is possible that some values of lerp(a, b, t) won't pass the collision test even when mathematically they should, simply because float values are discrete, and in some circumstances they'll be behind and other times they'll be beyond the mathematically defined segment. Even when that is the case, I feel the correct implementation is to not let those cases pass, because numerically the segments don't intersect. If the user wants something better he could change his game logic to test for distance to segment and apply the threshold himself.

---

Original title: Fix segment intersection for extreme cases
Original PR explanation follows:

The code to detect intersection between two segments evaluates a condition that leaves a margin of error that breaks some operations that need to be exact to be accurate.

This PR fixes #70823 for the majority of cases. With this, I mean I know there is an extremely obscure corner case that will not be covered, more on that in a moment.

I submit a testing project that demonstrates the problem. To try it you need to open the node_2d.tscn scene, in there there is a NavigationRegion2D node with a NavigationPolygon.

This test has a transcription of the logic to detect if a polygon is an outer or inner polygon, this logic originally lives in navigation_polygon.cpp.

This code works by using a segment from the first vertex of a polygon, to a point that is known to be out of every polygon. This "outer point" is manually crafted by taking the maximum x and y coordinates and adding some offset to them.

This segment is traced against every segment of the other polygons. If an even number of segments were crossed, the polygon is determined to be an outer polygon.

here you can see a line that comes from very far away, and that ends at the outer point, which is offset about (0.7, 0.8) from the bottom-right most coordinates of the polygon. the red line is from the polygon's first vertex

The problem is this algorithm fails when the segment passes very closely (tens of thousandths of a unit) away from a vertex, as it starts detecting the two segments of that vertex as intersecting. The way the "outer point" is created is made to prevent this from happening, as it doesn't align to the grid, and makes it hard to have any placed vertex to be close to any segment that uses the "outer point" (hopefully my point is understood)

the problem is more noticeable when you start placing other vertices close to the vertex 0 of the polygon. you can try moving some of the inner polygon vertices to cross this line. as these vertices are closer to a grid-aligned point, the chances of getting in the threshold of detection for both sides of the segments is higher (the d from the previous screenshot is too low in this corner, so both segments are detected as intersecting)

Recording.2023-03-09.231846.mp4

alternatively you can trigger the issue by placing a vertex that, when placed correctly, makes its segment to the "outer point" cross another vertex (reduces the d distance in the first screenshot). you can also try that in the provided demo file. In this case, the hole gets detected as an outer polygon.

Recording.2023-03-09.233030.mp4

The way to reliably fix this for my use case, and the use case for the issue #70823 is to remove the threshold used when detecting a segment intersection. instead of checking if both values of the cross product operation have the same sign and are below a threshold, we just multiply them to find if they have the same sign, and compare to zero directly.

Recording.2023-03-09.234223.mp4

this makes the segment detection more accurate, but doesn't eliminate the issue completely.

In a theoretical sense, there may be some even more obscure combinations that could trigger the bug mentioned at the beggining. An appropiate fix for that bug in specific could be to use two outer points to validate the result. also, the offset added to the "outer point" and relying on its non-integer-ness to reduce the cases of collision doesn't stop feeling like a hack. Also there's the case of large values, where it will end up getting rounded to something else due to floating point limitations. Even if this seems like a corner case, it may happen, and it actually relates to how I found this bug in the first place (I was using the Godot's navmesh system to do some calculations with globe coordinates)

this is the project files where you can test the issue. It also includes the previous issue test scene and demonstrates that it fixes that as well.

test_nav_polygon.zip

[0xafbf]

some tests started failing, as they were relying on this rounding behaviour. I did move a division that introduced inaccuracy to a later part in the call.

[smix8]

For the linked issue with the NavigationPolygon specifically this fix will be no longer needed as the entire NavigationPolygon outline and polygon creation code is getting removed / replaced in pr #70724.

[0xafbf]

Hello. Today I saw a comment on the linked issue and thought that maybe it would be good to bump the discussion a bit. I updated this PR name and description to represent better what it does, and hopefully other contributors can have a look again and discuss if it is feasible to get this merged.