-
-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Aagu/replace rust-geo-booleanop with geoclipper #41
Aagu/replace rust-geo-booleanop with geoclipper #41
Conversation
Thanks! This bug is actually the reason I never merged that branch, it was too unstable 🙁 What I wanted to do at the time but got busy with other things (and also waiting for lelongg/geo-clipper#23 to be released) was to use geo-clipper in native, and geo-booleanop in wasm. Could you add that to your PR? |
oh lelongg/geo-offset#56 got merged but not yet released, this would be needed for the |
…compatible" the wasm-compatible variant includes the lib geo-booleanop that is currently in some cases not reliable the wasm-incompatible variant includes the lib geo-clipper seems to be more reliable, faster but not wasm-compatible
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. |
@mockersf Do you have outstanding requests for this pull request or is it fine as is? |
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! |
* swapped out geo-booleanop through geo-clipper, because of issue 21re/rust-geo-booleanop#17 * made clippy happy * added mutually exclusive feature-flags "wasm-compatible" and "wasm-incompatible" the wasm-compatible variant includes the lib geo-booleanop that is currently in some cases not reliable the wasm-incompatible variant includes the lib geo-clipper seems to be more reliable, faster but not wasm-compatible * cargo fmt
rust-geo-booleanop seems to have an unfixed bug ( 21re/rust-geo-booleanop#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 functionmerge_overlapping_obstacles
now returns 8 polygons instead of the 16 previous. I haven't looked into this further, might be an issue.