-
Notifications
You must be signed in to change notification settings - Fork 185
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
Add Voronoi diagrams with zero dependency delaunay triangulation #1144
base: main
Are you sure you want to change the base?
Conversation
b5f6b05
to
d5a6cd6
Compare
9e2b29f
to
6c9ef8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry it took me a while to review this. I started with just the Delauny triangulation part. This is just a first iteration cleaning the code up here and there. I didn't really go through how the algorithm works yet. Will do that in a follow up review.
Overall, even though I left some comments, I think the code is really clean and the comments were really helpful so far! Also thanks for taking the time to create nice Errors which implement Display! And I'm really happy to see this implemented natively in geo
. Although I also like spade for its API, I can't understand its code which isn't so nice 🙈
processed_points.push(*pt); | ||
} | ||
} | ||
let triangles = remove_super_triangle(&triangles, &super_triangle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Questions:
- Can it happen that the
super_triangle
gets removed during the algorithm? - If not, can it happen that the
super_triangle
gets moved away from the first position to somewhere else in the Vec
Depending on the answers, the removal might be easier than a full search through the whole vec as done in remove_super_triangle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No the super_triangle
isn't removed during the process. The scaffolding provided by the super_triangle
will remain as well as other triangles formed from it https://commons.wikimedia.org/wiki/File:Bowyer-Watson_6.png#/media/File:Bowyer-Watson_6.png In this diagram the super_triangle
and corresponding artefacts are shown in red while the final delaunay triangulation is shown in below.
The process of removing the super_triangle
also requires removing triangles that share vertices with the super_triangle
and these triangles can be located anywhere in the vec.
Thanks so much for your thorough review @RobWalt! I'll work through them and have an update soon. |
Signed-off-by: Ben Johnston <ben.johnston@franklin.ai>
6c9ef8c
to
49f3839
Compare
Hi @RobWalt sorry for the delay, I've updated the PR as per your comments. It definitely looks much cleaner! Thanks! |
This PR adds the
VoronoiDiagram
trait to create Voronoi diagrams fromPolygon
andMultiPoint
types. The Voronoi diagram is created using the property that voronoi diagrams are the dual of delaunay triangulation.The
VoronoiDiagram
trait uses an addedTriangulateDelaunay
trait that computes Delaunay triangles without any additional dependencies. As per the conversation here this work was started before the introduction of the spade methodology. As requested in the conversation I've added a bench performance test to compare the different methods and these are the results when running on my laptop.the
TriangulateDelaunay
trait is faster then theConstrained Outer Triangulation
method but not theUnconstrained Method
.The original intent behind implementing Delaunay triangulation in
geo
arose when we needed a method that was compliant with the Apache license usinggeo-types
and was not otherwise available.Thank you for considering this PR.
CHANGES.md
if knowledge of this change could be valuable to users.