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

Should Diff return opposite orientation for newly created interior holes in a polygon? #129

Open
mglasgow opened this issue May 4, 2020 · 2 comments

Comments

@mglasgow
Copy link

mglasgow commented May 4, 2020

Using Diff between two polygons, the second polygon of which is contained completely within the first, will result in an interior hole in the original polygon.

Does Diff currently give any consideration to the orientation of that newly created interior hole? To be a valid polygon, the interior ring should have opposite orientation to the outer ring.

I have created a file in the "genericTestCase" format which shows what I am trying to achieve:
issue129.geojson.txt

A screenshot of the polygon is here:
screenshot

Both polygons are sent to the Diff operation with counter-clockwise orientation. Diff returns the outer and inner rings both in counter-clockwise orientation. However, to be a valid polygon, this interior ring should actually be clockwise orientation.

@rowanwins
Copy link
Collaborator

The orientation issue depends on which data spec you are following (eg for geojson your comment is accurate). But this repo doesn't particularly enforce a particular data spec.

There is a fairly simple algorithm that we could add called the shoelace formula, see this reference from TurfJS.
https://github.com/Turfjs/turf/blob/f62ba326cba17d9ebfb29ba8671107f937a89442/packages/turf-boolean-clockwise/index.ts#L19-L35
It's fairly minimal in terms of code but would add some performance overhead. Perhaps we could add an options object argument with this sort of thing?

Alternatively we could ask users to pass their results through the turf.rewind module

@w8r
Copy link
Owner

w8r commented Apr 4, 2022

Yes, maybe an option would make sense to see how much it is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants