Closed RobWalt closed 8 months ago
This is great, @RobWalt, thank you! We'll wait to see how the proposed changes to spade go.
@urschrei Do you think it would be feasible to review & merge this as is and keep a tracking issue open for updating the dependencies which track the two spade PRs? I assume these two PRs are not going to be included anytime soon and this issue here is required to merge the spade boolops at some point
I'm in favour of merging this, both because it's a pre-requisite for the new boolean ops functionality, and because geo
should support Delaunay triangulation as a feature more generally. For some reason it's not currently showing up as a top-level item under the algorithms module – again, you can check for yourself by running cargo doc --open
.
Thanks for the thorough review @michaelkirk! 🏅 I like your taste 👍🏼
Ok, the proposed changes to spade were accepted and are merged as of now. There's nothing major in the way now. The only things left are:
😎
This PR aims at bringing
spade
triangulations togeo
. More specifically I'm looking into implementing constrained delauny triangulations forgeo
since it seems handy in a lot of situations and opens up possibilities for new algorithms.Todos
Commit summary
chore: add
spade
as a dependencyalthough spade is a optional dependency, it looks like a good fit for the default features. Including it has the effect of bringing in 3 new dependencies:
robust
0.2 (we already use this on version 1.1)optional
0.5spade
2.2I'll try to speak to the spade maintainers. Maybe we can bump the robust version in their crate to reduce dependencies further. Also it might make sense to ask them if the optional crate provides any real value.
Update: I created PRs for the changes
robust
version bump should make it since it comes with performance improvementsoptional
removal might make it depending on the maintainers tasteUpdate: Both PRs got merged, so this change doesn't add any additional dependencies
feat: initial implementation of
TriangulateSpade
traitincludes all the code for the integration of
spade
intogeo
and also features some testsCHANGES.md
if knowledge of this change could be valuable to users.