Improve efficiency and precision of Geometry2D.is_point_in_polygon. #101736
Conversation
L4Vo5
force-pushed
the
improve_is_point_in_polygon
branch
2 times, most recently
from
65ed53c to
511c990
Compare
core/math/geometry_2d.h
Outdated
| float cross = (p_point.y - v1.y) * (v2.x - v1.x) - (p_point.x - v1.x) * (v2.y - v1.y); | ||
| if ((cross < 0) == (v1.y < v2.y)) { | ||
| intersections++; | ||
| } | ||
| if (cross == 0) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The cross == 0 condition should be first as it's an early return regardless of the previous condition's work.
There was a problem hiding this comment.
IMO the code is a bit clearer like this, but it's simple enough that the compiler should reorder it as it sees fit
In fact it's possible that it's better to dispatch the longer chain of comparison instructions earlier, since they also involve more data dependency, instead of the early return that won't trigger in 99% of cases. but who knows.
core/math/geometry_2d.h
Outdated
| if (v1.y < p_point.y != v2.y < p_point.y) { | ||
| float cross = (p_point.y - v1.y) * (v2.x - v1.x) - (p_point.x - v1.x) * (v2.y - v1.y); | ||
| if ((cross < 0) == (v1.y < v2.y)) { | ||
| intersections++; |
There was a problem hiding this comment.
You start with intersections = 0 and then increment it every time, and at the end check if it's even or uneven. So it's more optimal to make it a bool that gets flipped here.
|
I have improved something similar in the navigation module not so long ago (in 3D). You can see it here. We could do the same thing here. The trick behind this, is that the winding order for the edges and the point all have to be the same sign for it to be inside. It means that you can easily early out with this algorithm, with no branch (other than the return) in the calculations. |
Doesn't that only work for convex polygons? |
Yes, it does only work for convex. So if the polygons can be any shape for this function, this will not work. |
L4Vo5
force-pushed
the
improve_is_point_in_polygon
branch
from
511c990 to
dbe066b
Compare
By checking against an infinite horizontal ray, we can avoid the preliminary loop, and we can also simplify the math to improve performance, and also avoid calling segment_intersects_segment, which currently is less precise as it checks against an epsilon.
L4Vo5
force-pushed
the
improve_is_point_in_polygon
branch
from
dbe066b to
7d04073
Compare
The previos version called
segment_intersects_segment, which is less precise as it checks against an epsilon. As far as I can tell, the only inputs for which the result differs is when that imprecision comes into play.So, this version also fixes #81042 and #82305 (at least, the provided reproductions now work)
It's also about 6x faster, from my (very limited) tests.
This version turned out to be the same concept as was suggested here. It still checks for an odd number of intersections, but instead of using a preliminary loop to finding a point outside the polygon in order to construct a line, it uses an infinite ray from p_point towards (-inf, p_point.y).
In the intersection calculation, between the ray and every line in the polygon, there are some edge cases that return true for points exactly on the polygon boundary. Otherwise, an intersection can only exist if
p_point.yis betweenv1.yandv2.y. With that knowledge, we can calculatep = inverse_lerp(v1.y, v2.y, p_point.y), a value between 0 and 1. The x of the intersection point between the polygon's line, and the infinite line described byy = p_point.y, isx = lerp(v1.x, v2.x, p). Ifx <= p_point.xthen there is an intersection. Since we only care about that inequality and don't actually want the exact x of the intersection point, the math can be simplified, and it turns out to basically be a cross product betweenp_point - v1andv2 - v1. Which is easier to compute as it avoids a division caused byinverse_lerp.I also replaced a remainder
(%)with a ternary operation, which also seemed to boost performance slightly.