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

polygonContainsPoint returns false for intersections. Is this intended? #71

Open
BlueWater86 opened this issue Dec 29, 2021 · 5 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@BlueWater86
Copy link

I swapped out a pile of custom helpers in favour of your library (thankyou!). A couple of my unit tests expected points intersecting with polygon boundaries to be contained within said polygon.

I went looking through your unit tests but they do not cover this edge case. Were point intersections with polygon boundaries excluded from being within the polygon on purpose?

const coordinates = [
  [1, 1],
  [0, 2],
  [0, 0]
];

coordinatesContainPoint(coordinates, [1, 1]); //false
coordinatesContainPoint(coordinates, [0.5, 1.5]); //false
coordinatesContainPoint(coordinates, [0, 2]); //false
coordinatesContainPoint(coordinates, [0.5, 1]); //true
@jgravois
Copy link
Member

jgravois commented Dec 30, 2021

Were point intersections with polygon boundaries excluded from being within the polygon on purpose?

as far as terraformer is concerned, if the point and polygon share a vertex or the point falls exactly on the line formed between polygon verticies, the point is not considered to be contained by the polygon.

intersects is the spatial operator to capture the relationship you describe, but as noted in #33, point/polygon intersection isn't implemented in this library.

to be perfectly honest, i'm not sure why this is the case.

@BlueWater86
Copy link
Author

Thank you for the response. I'll fork and make the changes I need for it to match with the behaviour of leaflet plugins. It does seem strange to me that there are so many inconsistencies in implementations.

Do you mind if I raise a pull request to add a test that shows that point/polygon intersections are not supposed to be contained within the polygon? Also, would you consider another pull request to add options to change this behaviour?

@jgravois
Copy link
Member

would you consider another pull request to add options to change this behaviour?

there's no need to expose a user-facing option.

a pull request to make the behavior of coordinatesContainPoint() match your expectation (and the definition of 'Contains' in the Esri Geometry API) would be welcome.

@BlueWater86
Copy link
Author

Perfect. That solves everything. I'll get the work done either tonight or on the 3rd. Have a good new year.

@BlueWater86
Copy link
Author

This was more complicated than I originally thought! Much learnt. I'll commit tomorrow.

@jgravois jgravois added enhancement New feature or request question Further information is requested labels Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants