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

attempt to better document Simplify traits epsilon parameter #1151

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Feb 20, 2024

  • I agree to follow the project's code of conduct.
  • [n/a] I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

I'm actually not sure if this is what the parameter really means, but it's my best guess at this point. Is anyone more familiar willing to clarify?

@michaelkirk michaelkirk requested a review from urschrei February 20, 2024 23:00
@urschrei
Copy link
Member

The triangle explanation (which is correct!) only applies to the Visvalingam-Whyatt algorithm. The RDP algorithm (which underlies Simplify) is distance-based…

@michaelkirk
Copy link
Member Author

Thank you! Please take another look @urschrei

@michaelkirk michaelkirk marked this pull request as ready for review February 20, 2024 23:28
@@ -192,7 +198,13 @@ pub trait Simplify<T, Epsilon = T> {
/// This operation uses the [Ramer–Douglas–Peucker algorithm](https://en.wikipedia.org/wiki/Ramer–Douglas–Peucker_algorithm)
/// and does not guarantee that the returned geometry is valid.
///
/// An epsilon less than or equal to zero will return an unaltered version of the geometry.
Copy link
Member Author

Choose a reason for hiding this comment

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

A related question is can we alter our algorithm such that epsilon == 0 will remove collinear points, but nothing else?

Copy link
Member Author

@michaelkirk michaelkirk Feb 20, 2024

Choose a reason for hiding this comment

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

Maybe with distance and area calculations it's not feasible to have epsilon == 0 from a robustness perspective...

Copy link
Member

Choose a reason for hiding this comment

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

Hmm I don't know that either algorithm can be made to work precisely like that. I can have a look at "the literature".

@michaelkirk michaelkirk added this pull request to the merge queue Feb 23, 2024
@michaelkirk michaelkirk mentioned this pull request Feb 23, 2024
2 tasks
Merged via the queue into main with commit a46eb87 Feb 23, 2024
15 checks passed
@michaelkirk michaelkirk deleted the mkirk/simplify-docs branch February 23, 2024 02:11
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.

2 participants