Implement a Convex Hull method #87
Conversation
The current implementation isn't as fast as e.g. Quickhull, but it's a reasonable starting point
src/algorithm/convexhull.rs
Outdated
| // Adapted from http://codereview.stackexchange.com/a/141752/2630 | ||
| // The algorithm is from Heineman, G.T., Pollice, G., Selkow, S., 2008. | ||
| // "Algorithms in a Nutshell". O’Reilly Media, Inc., pp261–8 | ||
| pub fn convex_hull<T>(points: &BTreeSet<Point<T>>) -> Vec<Point<T>> |
There was a problem hiding this comment.
This doesn't need to be pub, right?
| use types::{Point, Polygon, LineString}; | ||
| use std::collections::BTreeSet; | ||
|
|
||
| impl<T> Eq for Point<T> where T: Float {} |
There was a problem hiding this comment.
Ord needs Eq, and deriving it in types.rs didn't work…
There was a problem hiding this comment.
Is there some reason you need Ord? IIRC, num::Float intentionally doesn't implement Ord since it can't guarantee total order...I think?
There was a problem hiding this comment.
This is a problem related to punting on NaN values – BTreeSet needs Ord, but as you say, there's no total ordering for types which include NaN. However, I'm not sure that it should be possible to construct geometries with NaN values in the first place.
I feel like there should be a more elegant solution here, but it'll need input from someone who knows more than I do about floating point…
There was a problem hiding this comment.
BTreeSetneedsOrd
Ah right, forgot about this. What you have is fine for now. Can you just add a comment next to the Ord implementation stating that it's needed for BTreeSet?
src/algorithm/convexhull.rs
Outdated
| // we need to close the Polygon | ||
| let final_element = hull[0].clone(); | ||
| hull.push(final_element); | ||
| hull.into_iter().collect::<Vec<Point<T>>>() |
There was a problem hiding this comment.
Prior to this line, is hull not already a Vec<Point<T>>?
src/algorithm/convexhull.rs
Outdated
| impl<T> Convexhull<T> for Polygon<T> | ||
| where T: Float | ||
| { | ||
| fn convexhull(&self) -> Polygon<T> { |
There was a problem hiding this comment.
Nit: If "convex hull" is two words, I'd personally call this ConvexHull::convex_hull
The current implementation is slow (the worst case is On4) compared to e.g.
Quickhull (I struggled with the recursion + retained points implementation), but it's a reasonable starting point, and a convex hull method is the fundamental building block for Delaunay triangulation and Voronoi tessellation methods.