Skip to content
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

Implement Simplification traits for more things #135

Merged
merged 4 commits into from Aug 14, 2017

Conversation

amandasaurus
Copy link
Member

If you can simplify a LineString, then you can simplify a MultiLineString.

I also added implementations for the simplification traits forPolygon (and MultiPolygon), which just perform the simplification on the constituant linestrings that make up polygon. PostGIS's ST_Simplfiy does this AFAIK. The link for VM algorithm shows a polygon (the USA) being simplified, so the concept of "simplifying a polygon" is well understood.

@amandasaurus
Copy link
Member Author

I was tempted to implement the simplification for Point (& MultiPoint), which would do nothing and just return a clone of the point. PostGIS's ST_Simplify does this. This allows you to implement simplication traits on Geometry, which can make it easier to mass simplify a lot of geometry, because you don't have to do any logic yourself to split out point/multipoints.

But I didn't do it this time, I'm in two minds about whether it sensible.

@urschrei
Copy link
Member

urschrei commented Aug 7, 2017

We should note that in the case of Polygons and MultiPolygons, the simplification operation is not guaranteed to return a valid geometry (and we're back to validity!). A well-performing fix is dependent upon a usable R* implementation (I have a look occasionally, but results have been mixed).

@urschrei
Copy link
Member

urschrei commented Aug 7, 2017

(I've had a look at the JTS topology-preserving implementation – I don't do much Java, but it looks to be the naïve version described above. I'm not sure about GEOS, but I assume it's the same)

@amandasaurus
Copy link
Member Author

I have expanded the comments w.r.t. validity and topology

@@ -52,7 +52,7 @@ fn rdp<T>(points: &[Point<T>], epsilon: &T) -> Vec<Point<T>>
/// The [Ramer–Douglas–Peucker
/// algorithm](https://en.wikipedia.org/wiki/Ramer–Douglas–Peucker_algorithm) simplifes a
/// linestring. Polygons are simplified by running the RDP algorithm on all their constituant
/// rings.
/// rings. This may result in invalid Polygons, and has no guarantee of perserving topology.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "constituent" and "preserving".

/// are simplified by simplifing all their constituant geometries individually.
/// Polygons are simplified by running the algorithm on all their constituant rings. This may
/// result in invalid Polygons, and has no guarantee of perserving topology. Multi* objects are
/// simplified by simplifing all their constituant geometries individually.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "simplifying" and "constituent".

Rory McCann added 4 commits August 10, 2017 21:23
If you can simplify LineStrings, it makes sense that you can simplify a
collection of linestrings (i.e. a multilinestring).
It simplifies their constituent linestrings.
Just simplify their constituent Polygons
@amandasaurus
Copy link
Member Author

I have rebased & fixed up those typo fixes now.

@urschrei
Copy link
Member

Thanks for making the changes. PR looks good to me. 👍

@frewsxcv
Copy link
Member

this is great, thanks! opened a new issue for reminding us about this potentially creating invalid geometries (#142)

bors r+

bors bot added a commit that referenced this pull request Aug 14, 2017
135: Implement Simplification traits for more things r=frewsxcv

If you can simplify a `LineString`, then you can simplify a `MultiLineString`.

I also added implementations for the simplification traits for`Polygon` (and `MultiPolygon`), which just perform the simplification on the constituant linestrings that make up polygon. PostGIS's `ST_Simplfiy` does this AFAIK. The link for VM algorithm shows a polygon (the USA) being simplified, so the concept of "simplifying a polygon" is well understood.
@bors
Copy link
Contributor

bors bot commented Aug 14, 2017

Build succeeded

@bors bors bot merged commit cd8770f into georust:master Aug 14, 2017
@amandasaurus amandasaurus deleted the simplify-more branch March 3, 2018 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants