Point in triangle and rect performance improvements

[b4l]

• [x] I agree to follow the project's code of conduct.
• [x] I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

[urschrei]

Thanks for the PR @b4l! Could you say a little about how this perf improvement works? Is it purely down to switching away from using Robust? Are there any drawbacks (e.g. precision issues?)

[b4l]

From my rather superficial understanding, the previous versions involved a conversion to a polygon, which allocates a vector that is considered expensive. Then subsequent, in the case of rect robust predicates are suboptimal as there is no interpolation in play since it is axis-aligned and a simple comparison is adequate. For the triangle cases, this implementation leverages ideas around barycentric coordinates and half-plane calculations, which are somewhat robust in terms of not involving any division, though presumably not as robust as robust predicates, as the name suggests. For the latter implementations based on robust predicates without allocation are commented out offering lesser performance improvements given the robustness trade-off.

Regarding precision issues, I have only a very basic understanding of the robustness and no idea of the implications this has in practice. The test /bench with the point on the triangle line works as expected though actually looking for reviewers' input regarding that.

[urschrei]

Given the explanation (thank you!) and that it's passing tests currently, I'm inclined to merge this. Anyone else got input for or against?

[urschrei]

Let's make a decision on this as it's a perf improvement. As per the author's comments on Discord:

I can not vouch for the robustness, though there is a version using robust predicates for the triangle case commented out which still offers major performance improvements compared to the existing implementation. so maybe we can use this one for confidence?

So the conservative solution is to go with the robust predicate if there's some doubt about robustness. Alternatively, someone dreams up some more tests.

[rmanoka]

Thanks for the PR @b4l ; looks good except the non-robust impl for Triangle; let's use the slightly slower robust impl. on that too.

+1 for removing the Vec allocations though.

[b4l]

Thanks for reviewing and sorry for the late response! I changed to the versions using robust predicates.

Should I remove the commented-out versions?

Edit: CI failure seems to be unrelated, see https://github.com/georust/geo/issues/1149

[frewsxcv]

The ahash MSRV issue was resolved upstream https://github.com/tkaitchuck/aHash/pull/208

[urschrei]

Are we good to merge this before releasing? I know we have version control, but I'm personally OK with hanging on to the commented-out versions for now.