search

vleue / polyanya

Pathfinding using Polyanya
Apache License 2.0
266 stars 19 forks source link

Aagu/replace rust-geo-booleanop with geoclipper #41

Closed AnAverageGitUser closed 8 months ago

AnAverageGitUser commented 9 months ago

rust-geo-booleanop seems to have an unfixed bug ( https://github.com/21re/rust-geo-booleanop/issues/17 ). One possibility might be to switch to a similar library e.g. (Clipper2, https://angusj.com/clipper2/Docs/Overview.htm).

I ran into this while using bevy_pathmesh as one task thread kept on crashing after my application ran X seconds.

Note 1

The Clipper2 library seems to work on integer grid points, therefore one has to choose a precision for the coordinates that one is comfortable with. In this PR I chose 3 digits behind the decimal point.

Note 2

One of the tests is_in_mesh_overlapping_simplified failed initially, because the amount of polygons was not reduced from 16 to 8, but was stable from 8 to 8 after the update. The function merge_overlapping_obstacles now returns 8 polygons instead of the 16 previous. I haven't looked into this further, might be an issue.

mockersf commented 9 months ago

Thanks!

This bug is actually the reason I never merged that branch, it was too unstable 🙁 I also tried geo-clipper, but it's not working in wasm. It's much faster though!

What I wanted to do at the time but got busy with other things (and also waiting for https://github.com/lelongg/geo-clipper/pull/23 to be released) was to use geo-clipper in native, and geo-booleanop in wasm. Could you add that to your PR?

mockersf commented 9 months ago

oh https://github.com/lelongg/geo-offset/pull/56 got merged but not yet released, this would be needed for the radius-baking branch to be merged to main too...

AnAverageGitUser commented 9 months ago

I've used 2 mutually exclusive feature-flags, because I couldn't find a way to use a "not(feature = "x")" in the Cargo.toml. Otherwise the two libs couldn't both be optional.

Btw, thanks for all the work that you put into the bevy ecosystem. You're doing quite a lot.

AnAverageGitUser commented 9 months ago

@mockersf Do you have outstanding requests for this pull request or is it fine as is?

mockersf commented 8 months ago

Sorry, I'm coming back to this now.

I was hoping some crates would have been published by now, I'll try to move forward. but this can be merged anyway, thanks!