-
Notifications
You must be signed in to change notification settings - Fork 186
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
Using Simplification algorithm on a polygon can result in an invalid polygon #142
Comments
Personally I think that the simplification algorithm should return the geometry type, rather than an Option. Allow the caller to call the future
|
I've been looking at R-tree implementations in Rust again since this came up, and I'm making good progress with Spade's R* implementation, now that I've managed to implement the If / when that lands, I'm still in favour of returning an |
Currently, I'm in favor of returning impl<T: Float> LineString<T> {
fn from_vec(v: Vec<T>) -> Result<LineString> { .. }
/// << insert big disclaimer here >>
fn from_vec_unchecked(v: Vec<T>) -> LineString { .. }
} |
Here's a simple case (epsilon = 1.0) simplifying a Polygon exterior ring that results in a ring with only two points: Before:
After:
|
In the case of polygon rings, could we just break once the |
I think so (that's what we do in |
Here's my fix attempt: #943 |
943: `Simplify` now generates `Polygon` rings with at least four coordiantes r=frewsxcv a=frewsxcv - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/main/CODE_OF_CONDUCT.md). - [ ] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- Prior to this pull request, running `Simplify` on `Polygon` could generate rings with two coordinates, which would make it no longer a valid ring. This pull request updates `Simplify` to ensure resulting `Polygon` rings always have at least four coordinates. Fixes #142 Co-authored-by: Corey Farwell <coreyf@rwell.org>
943: `Simplify` now generates `Polygon` rings with at least four coordiantes r=michaelkirk a=frewsxcv - [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/main/CODE_OF_CONDUCT.md). - [ ] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users. --- Prior to this pull request, running `Simplify` on `Polygon` could generate rings with two coordinates, which would make it no longer a valid ring. This pull request updates `Simplify` to ensure resulting `Polygon` rings always have at least four coordinates. Fixes #142 Co-authored-by: Corey Farwell <coreyf@rwell.org>
#135 (comment)
Maybe the algorithm should sometimes return an
Option<T>
since it can sometimes fail.The text was updated successfully, but these errors were encountered: