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

Various bug fixes for LineString.simplify and Polygon.simplify #151

Closed
wants to merge 5 commits into from

Conversation

nighthawk
Copy link
Contributor

@nighthawk nighthawk commented Sep 5, 2021

This branch fixes these issues introduced in the port of turf-simplify from #138:

  • LineString.simplify with highestQuality = false was broken as it was using start.coordinate in one formula when it should have used coordinate
  • Polygon.simplify ignored highestQuality = false as it first assigned the self.coordinates to a local variable, then simplified the self.coordinates, but then passed the previous local variable to the Douglas-Peucker algorithm. That means it always returned results as if highestQuality = true was set.
  • Polygon.simplify forgot to square the tolerance to the Douglas-Peucker algorithm (LineString.simplify did it correctly)
  • Polygon.simplify didn't simplify if any of the exterior/interior polygons was a triangle; now it just skips simplification of the triangle but simplifies the others.

I've also refactored the code to remove the code duplication between LineString.swift and Polygon.swift, and ported the turf-simplify test suite using the in/out directory -- excluding the test cases for MultiLineString and MultiPolygon as they aren't supported yet.


TODO:

  • Port the full test suite of turf-simplify to this repo
  • Investigate remaining test failure for polygon.geojson

Fixes these bugs, too:
- Polygon's simplify method didn't square the tolerance parameter when it should have
- Polygon's 'highestQuality = false' didn't work at all(!)
@nighthawk nighthawk marked this pull request as draft September 5, 2021 03:56
Also removes hard-coded test which had wrong values
Also fix compiling using .xcodeproj file
@nighthawk nighthawk marked this pull request as ready for review September 5, 2021 04:52
@nighthawk
Copy link
Contributor Author

This is ready for review. It runs fine locally and the tests pass. I can't access the bitrise issue for tvOS, so if someone can point me at what's wrong, I'm happy to investigate.

@nighthawk nighthawk closed this Sep 5, 2021
@nighthawk nighthawk deleted the fix-simplify branch September 5, 2021 07:53
@nighthawk nighthawk restored the fix-simplify branch September 5, 2021 07:53
@nighthawk
Copy link
Contributor Author

Oops. I accidentally deleted my branch. Re-instated as #152.

@nighthawk nighthawk deleted the fix-simplify branch October 15, 2021 04:08
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.

1 participant