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

intersection() fails if one polygon has no area. #65

Open
erikh2000 opened this issue Feb 12, 2018 · 3 comments
Open

intersection() fails if one polygon has no area. #65

erikh2000 opened this issue Feb 12, 2018 · 3 comments

Comments

@erikh2000
Copy link

erikh2000 commented Feb 12, 2018

This is probably in the category of just handling invalid input. See my comments below that seek to clarify the contract of the API. I'm willing to add a quick PR fix for this, depending on what the solution should be.

 const notQuiteAPolygon = [[[-93.621844, 44.154535],[-93.621672, 44.154625],[-93.621844, 44.154535]]];
const aPolygon = [[[-93.621887, 44.154764],[-93.621844, 44.154535],[-93.621672, 44.154625],[-93.621887, 44.154764]]];
intersect(aPolygon,notQuiteAPolygon); // Fails with connectEdges() infinite inner loop 

The above code causes the inner loop inside of connectEdges() to loop forever and crash the JS execution environment. (My loop guard PR is coming for this.)The first polygon only contain 2 points (3 including closing), so it's really just a line. If I add one more unique point to the first polygon, the test passes.

The question I have is whether you want to interpret this as valid input or not. I know the TurfJS people have asked for clipping functions that work with points and line segments. (Although, I think if we did pass a set of LineString coordinates, it should probably be just 2 levels of array depth.)

If it is valid input, I think you'd want execution to branch to a more trivial evaluation of the intersection, or at least a change that prevents connectEdges() from freaking out. Especially, if that helps avoid adding special case logic inside of loop-iterated code of connectEdges() (performance and readability).

If it's not valid input, I think it would be good to throw an early exception instead of descending into connectEdges().

@rowanwins
Copy link
Collaborator

Yep I reckon a validation that checks the length of input of a coord ring would be great.

@erikh2000
Copy link
Author

Okay, cool. I'll put a PR together.

@Portur
Copy link

Portur commented Mar 28, 2018

What is returned in the case of :

  1. notQuiteAPolygon = [] => false
  2. notQuiteAPolygon = [2]
  3. notQuiteAPolygon = 2
  4. notQuiteAPolygon = false
  5. notQuiteAPolygon = null
  6. notQuiteAPolygon = "string"

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

4 participants