martinez
martinez copied to clipboard
intersection() fails if one polygon has no area.
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()
.
Yep I reckon a validation that checks the length of input of a coord ring would be great.
Okay, cool. I'll put a PR together.
What is returned in the case of :
-
notQuiteAPolygon = []
=>false
-
notQuiteAPolygon = [2]
-
notQuiteAPolygon = 2
-
notQuiteAPolygon = false
-
notQuiteAPolygon = null
-
notQuiteAPolygon = "string"