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.

Sign up for GitHub

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

turf-union, turf-difference, turf-intersect: Swap out martinez-polygon-clipping for polygon-clipping used in v7. #1916

Merged
merged 8 commits into from
Jul 8, 2020
Merged

turf-union, turf-difference, turf-intersect: Swap out martinez-polygon-clipping for polygon-clipping used in v7. #1916

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b29fbb
swap out martinez-polygon-clipping for polygon-clipping
mbullington Jun 11, 2020
9e3cbe7
reinstate single polygon return on turf-difference
ngottlieb Jul 8, 2020
48fffab
reinstate single polygon return for turf-intersect
ngottlieb Jul 8, 2020
54f1bf4
remove @ts-ignore from intersect and union
ngottlieb Jul 8, 2020
a64a24a
reinstate single polygon return for turf-union
ngottlieb Jul 8, 2020
eac392b
remove leftover comment on turf-intersect type error
ngottlieb Jul 8, 2020
bce1a54
Merge pull request #1 from ngottlieb/ngottlieb/requested-changes
mbullington Jul 8, 2020
25c97ca
Merge branch 'master' into master
mfedderly Jul 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 3 additions & 32 deletions packages/turf-difference/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import * as martinez from 'martinez-polygon-clipping';
import area from '@turf/area';
import { feature, multiPolygon, polygon } from '@turf/helpers';
import polygonClipping from 'polygon-clipping';
import { polygon, multiPolygon } from '@turf/helpers';
import { getGeom } from '@turf/invariant';
import { flattenEach } from '@turf/meta';

/**
* Finds the difference between two {@link Polygon|polygons} by clipping the second polygon from the first.
Expand Down Expand Up @@ -43,37 +41,10 @@ function difference(polygon1, polygon2) {
var geom2 = getGeom(polygon2);
var properties = polygon1.properties || {};

// Issue #721 - JSTS/Martinez can't handle empty polygons
geom1 = removeEmptyPolygon(geom1);
geom2 = removeEmptyPolygon(geom2);
if (!geom1) return null;
if (!geom2) return feature(geom1, properties);

var differenced = martinez.diff(geom1.coordinates, geom2.coordinates);
var differenced = polygonClipping.difference(geom1.coordinates, geom2.coordinates);
if (differenced.length === 0) return null;
if (differenced.length === 1) return polygon(differenced[0], properties);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep this Polygon return? Seems like quite a bit of the tests changed just because of that. Otherwise we should change the types and jsdoc to match the new implementation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In inclined to agree with @mfedderly - it's kind of annoying when things get returned as a multipolygon when they aren't

return multiPolygon(differenced, properties);
}

/**
* Detect Empty Polygon
*
* @private
* @param {Geometry<Polygon|MultiPolygon>} geom Geometry Object
* @returns {Geometry<Polygon|MultiPolygon>|null} removed any polygons with no areas
*/
function removeEmptyPolygon(geom) {
switch (geom.type) {
case 'Polygon':
if (area(geom) > 1) return geom;
return null;
case 'MultiPolygon':
var coordinates = [];
flattenEach(geom, function (feature) {
if (area(feature) > 1) coordinates.push(feature.geometry.coordinates);
});
if (coordinates.length) return {type: 'MultiPolygon', coordinates: coordinates};
}
}

export default difference;
4 changes: 1 addition & 3 deletions packages/turf-difference/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,8 @@
"write-json-file": "*"
},
"dependencies": {
"@turf/area": "^6.2.0-alpha.1",
"@turf/helpers": "^6.2.0-alpha.1",
"@turf/invariant": "^6.2.0-alpha.1",
"@turf/meta": "^6.2.0-alpha.1",
"martinez-polygon-clipping": "^0.6.2"
"polygon-clipping": "^0.14.3"
}
}
2 changes: 1 addition & 1 deletion packages/turf-difference/test/out/clip-polygons.geojson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,4 @@
}
}
]
}
}
10 changes: 5 additions & 5 deletions packages/turf-difference/test/out/create-hole.geojson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,16 +112,16 @@
-28
],
[
140,
-28
126,
-20
],
[
140,
-20
],
[
126,
-20
140,
-28
],
[
126,
Expand All @@ -132,4 +132,4 @@
}
}
]
}
}
134 changes: 77 additions & 57 deletions packages/turf-difference/test/out/issue-#721.geojson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,116 +252,136 @@
44.805326
],
[
-0.6471907,
44.8053946
-0.6472295,
44.8050484
],
[
-0.6465049,
44.8055554
-0.6469113,
44.8048572
],
[
-0.6464734,
44.8056645
-0.6465997,
44.8047375
],
[
-0.6465119,
44.8057959
-0.6462847,
44.8042192
],
[
-0.6465922,
44.8059124
-0.646075,
44.8042943
],
[
-0.6466376,
44.8059546
-0.645829,
44.8043891
],
[
-0.6466831,
44.8059719
-0.6460168,
44.8047948
],
[
-0.6467005,
44.8059967
-0.6460827,
44.8050017
],
[
-0.6466971,
44.8060413
-0.6461218,
44.8051741
],
[
-0.64662,
44.806065
-0.6461158838109077,
44.80524109574131
],
[
-0.64649,
44.80611
-0.646107941865921,
44.80533857879061
],
[
-0.646437,
44.806159
-0.646093673696575,
44.80541109474097
],
[
-0.6463809,
44.8061882
-0.6460848,
44.8054585
],
[
-0.6461987,
44.8060367
-0.6460471709105139,
44.80564743856641
],
[
-0.6461042,
44.8059451
-0.6460422277450562,
44.805672561504906
],
[
-0.6460542,
44.8057952
-0.6460542976856232,
44.80579530680288
],
[
-0.6460421,
44.8056729
-0.646069093721612,
44.80583985137739
],
[
-0.6460848,
44.8054585
-0.6461041344154722,
44.805944903377586
],
[
-0.6461073,
44.8053383
-0.6461991369724274,
44.80603699057963
],
[
-0.6461218,
44.8051741
-0.6462588068806041,
44.80608667910215
],
[
-0.6460827,
44.8050017
-0.6463809,
44.8061882
],
[
-0.6460168,
44.8047948
-0.646437,
44.806159
],
[
-0.645829,
44.8043891
-0.64649,
44.80611
],
[
-0.646075,
44.8042943
-0.64662,
44.806065
],
[
-0.6462847,
44.8042192
-0.6466971,
44.8060413
],
[
-0.6465997,
44.8047375
-0.6467005,
44.8059967
],
[
-0.6469113,
44.8048572
-0.6466831,
44.8059719
],
[
-0.6472295,
44.8050484
-0.6466376,
44.8059546
],
[
-0.6465922,
44.8059124
],
[
-0.6465119,
44.8057959
],
[
-0.6464734,
44.8056645
],
[
-0.6465049,
44.8055554
],
[
-0.6471907,
44.8053946
],
[
-0.6474134,
Expand All @@ -372,4 +392,4 @@
}
}
]
}
}
12 changes: 6 additions & 6 deletions packages/turf-difference/test/out/multi-polygon-target.geojson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
],
[
136.23046875,
-20.220965779522302
-20.2209657795223
],
[
130.6494140625,
Expand All @@ -185,16 +185,16 @@
-27.313213898568247
],
[
135.1318359375,
-27.313213898568247
132.38525390625,
-24.467150664738988
],
[
135.1318359375,
-24.467150664738988
],
[
132.38525390625,
-24.467150664738988
135.1318359375,
-27.313213898568247
],
[
132.38525390625,
Expand All @@ -218,7 +218,7 @@
],
[
138.6474609375,
-20.220965779522302
-20.2209657795223
],
[
138.6474609375,
Expand Down
51 changes: 6 additions & 45 deletions packages/turf-intersect/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Feature, multiPolygon, MultiPolygon, polygon, Polygon, Properties } from "@turf/helpers";
import { getGeom } from "@turf/invariant";
import * as martinez from "martinez-polygon-clipping";
import polygonClipping from 'polygon-clipping';

/**
* Takes two {@link Polygon|polygon} or {@link MultiPolygon|multi-polygon} geometries and
Expand Down Expand Up @@ -47,48 +47,9 @@ export default function intersect<P = Properties>(
): Feature<Polygon | MultiPolygon, P> | null {
const geom1 = getGeom(poly1);
const geom2 = getGeom(poly2);

if (geom1.type === "Polygon" && geom2.type === "Polygon") {
const intersection: any = martinez.intersection(geom1.coordinates, geom2.coordinates);

if (intersection === null || intersection.length === 0) { return null; }
if (intersection.length === 1) {
const start = intersection[0][0][0];
const end = intersection[0][0][intersection[0][0].length - 1];
if (start[0] === end[0] && start[1] === end[1]) { return polygon(intersection[0], options.properties); }
return null;
}
return multiPolygon(intersection, options.properties);

} else if (geom1.type === "MultiPolygon") {
let resultCoords: any[] = [];

// iterate through the polygon and run intersect with each part, adding to the resultCoords.
for (const coords of geom1.coordinates) {
const subGeom = getGeom(polygon(coords));
const subIntersection = intersect(subGeom, geom2);

if (subIntersection) {
const subIntGeom = getGeom(subIntersection);

if (subIntGeom.type === "Polygon") { resultCoords.push(subIntGeom.coordinates);
} else if (subIntGeom.type === "MultiPolygon") { resultCoords = resultCoords.concat(subIntGeom.coordinates);
} else { throw new Error("intersection is invalid"); }
}
}

// Make a polygon with the result
if (resultCoords.length === 0) { return null; }
if (resultCoords.length === 1) { return polygon(resultCoords[0], options.properties);
} else { return multiPolygon(resultCoords, options.properties); }

} else if (geom2.type === "MultiPolygon") {
// geom1 is a polygon and geom2 a multiPolygon,
// put the multiPolygon first and fallback to the previous case.
return intersect(geom2, geom1);

} else {
// handle invalid geometry types
throw new Error("poly1 and poly2 must be either polygons or multiPolygons");
}

const intersection = polygonClipping.intersection(geom1.coordinates as any, geom2.coordinates as any);
if (intersection.length === 0) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about trying to return a Polygon if the intersection is a single polygon and not a MultiPolygon.

if (intersection.length === 1) return polygon(intersection[0], options.properties);
return multiPolygon(intersection, options.properties);
}
Loading