terraformer-js / terraformer

A geographic toolkit for dealing with geometry, geography, formats, and building geodatabases
MIT License
182 stars 28 forks source link

intersects has unexpected result with multipolygon geometry #84

Closed rgwozdz closed 2 years ago

rgwozdz commented 2 years ago

Hey again. I start with a multipolygon geometry (green) and a simple polygon (hash-fill). They look like this:

Screen Shot 2022-07-05 at 2 26 14 PM

When I do an intersects for these two geometries, I'm expecting a result of true. On an ArcGIS service, I do find that an intersect filter operation like this (with multipolygon as data set and simple polygon as filter) returns a result. Similarly in QGIS, a select operation on the multipolygon using an intersection of the simple polygon results in the multipolygon being selected.

But I find the terraformer intersects returns false:

const { intersects }= require("@terraformer/spatial")

const multipolygon = {"type":"MultiPolygon","coordinates":[[[[-101.0,37.0],[-99.0,37.0],[-99.0,38.0],[-101.0,38.0],[-101.0,37.0]]],[[[-97.0,35.0],[-95.0,35.0],[-95.0,36.0],[-97.0,36.0],[-97.0,35.0]]]]}

const geometryFilter = {"type":"Polygon","coordinates":[[[-98.5,34.6],[-94,34.6],[-94,37],[-98.5,37],[-98.5,34.6]]]}

intersects(multipolygon, geometryFilter) // false

So in the above example, terraformer result seems to diverge from similar operation in QGIS/ArcGIS. Not sure if this is intentional or just something that was missed. Can you advise?

Interestingly, if I adjust the filter polygon so that its boundary crosses one of the multipolygon's component polygons, the result is true.

const { intersects }= require("@terraformer/spatial")

const multipolygon = {"type":"MultiPolygon","coordinates":[[[[-101.0,37.0],[-99.0,37.0],[-99.0,38.0],[-101.0,38.0],[-101.0,37.0]]],[[[-97.0,35.0],[-95.0,35.0],[-95.0,36.0],[-97.0,36.0],[-97.0,35.0]]]]}

const geometryFilter = {"type":"Polygon","coordinates":[[[-100.5,34.6],[-94,34.6],[-94,37],[-100.5,37],[-100.5,34.6]]]}

intersects(multipolygon, geometryFilter) // true
jgravois commented 2 years ago

thanks for the thorough simplified repro-case.

Not sure if this is intentional or just something that was missed. Can you advise?

definitely sounds like the latter to me.

my guess is that the problem is within (groan!) the following clause.

// https://github.com/terraformer-js/terraformer/blob/0d8379a9ca5dc1fafd4c272b242144fcb3d53a04/packages/spatial/src/intersects.js#L14-L16
if (within(geoJSON, comparisonGeoJSON) || within(comparisonGeoJSON, geoJSON)) {
  return true;
}

if we tested whether any of the MultiPolygon's child coordinate arrays were within() the filter here, we'd return the correct result.

a PR would be welcome (as always)

rgwozdz commented 2 years ago

Thanks @jgravois! I'll try to get to this soon, though I'm about to be away from computers for two weeks.

jgravois commented 2 years ago

released in v2.1.2 🎉

thanks for tee-ing up the fix and for your patience @rgwozdz. i just sent you an invite to npm@terraformer and wrote up a few breadcrumbs for next time.

https://github.com/terraformer-js/terraformer/commit/d3ae7d8cd9f3b60f548446a39ad8f72347505207 / https://github.com/terraformer-js/terraformer/blob/main/CONTRIBUTING.md#publishing-a-release