Skip to content

Commit 504bfea

Browse files
smallsaucepansmallsaucepan
authored
Merge pull request Turfjs#2771 from smallsaucepan/clean-coords-fixes
Fixed cleanCoords to remove points with appropriate tenacity
2 parents 8ef8597 + 2cfe86b commit 504bfea

File tree

4 files changed

+303
-82
lines changed

4 files changed

+303
-82
lines changed

‎packages/turf-clean-coords/index.ts

+59-78
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Position } from "geojson";
22
import { feature } from "@turf/helpers";
33
import { getCoords, getType } from "@turf/invariant";
4+
import { booleanPointOnLine } from "@turf/boolean-point-on-line";
5+
import { lineString } from "@turf/helpers";
46

57
// To-Do => Improve Typescript GeoJSON handling
68

@@ -99,62 +101,71 @@ function cleanCoords(
99101
* @returns {Array<number>} Cleaned coordinates
100102
*/
101103
function cleanLine(line: Position[], type: string) {
102-
var points = getCoords(line);
104+
const points = getCoords(line);
103105
// handle "clean" segment
104106
if (points.length === 2 && !equals(points[0], points[1])) return points;
105107

106-
var newPoints = [];
107-
var secondToLast = points.length - 1;
108-
var newPointsLength = newPoints.length;
109-
110-
newPoints.push(points[0]);
111-
for (var i = 1; i < secondToLast; i++) {
112-
var prevAddedPoint = newPoints[newPoints.length - 1];
113-
if (
114-
points[i][0] === prevAddedPoint[0] &&
115-
points[i][1] === prevAddedPoint[1]
116-
)
117-
continue;
118-
else {
119-
newPoints.push(points[i]);
120-
newPointsLength = newPoints.length;
121-
if (newPointsLength > 2) {
122-
if (
123-
isPointOnLineSegment(
124-
newPoints[newPointsLength - 3],
125-
newPoints[newPointsLength - 1],
126-
newPoints[newPointsLength - 2]
127-
)
128-
)
129-
newPoints.splice(newPoints.length - 2, 1);
130-
}
108+
const newPoints = [];
109+
110+
// Segments based approach. With initial segment a-b, keep comparing to a
111+
// longer segment a-c and as long as b is still on a-c, b is a redundant
112+
// point.
113+
let a = 0,
114+
b = 1,
115+
c = 2;
116+
117+
// Guaranteed we'll use the first point.
118+
newPoints.push(points[a]);
119+
// While there is still room to extend the segment ...
120+
while (c < points.length) {
121+
if (booleanPointOnLine(points[b], lineString([points[a], points[c]]))) {
122+
// b is on a-c, so we can discard point b, and extend a-b to be the same
123+
// as a-c as the basis for comparison during the next iteration.
124+
b = c;
125+
} else {
126+
// b is NOT on a-c, suggesting a-c is not an extension of a-b. Commit a-b
127+
// as a necessary segment.
128+
newPoints.push(points[b]);
129+
130+
// Make our a-b for the next iteration start from the end of the segment
131+
// that was just locked in i.e. next a-b should be the current b-(b+1).
132+
a = b;
133+
b++;
134+
c = b;
131135
}
136+
// Plan to look at the next point during the next iteration.
137+
c++;
132138
}
133-
newPoints.push(points[points.length - 1]);
134-
newPointsLength = newPoints.length;
135-
136-
// (Multi)Polygons must have at least 4 points, but a closed LineString with only 3 points is acceptable
137-
if (
138-
(type === "Polygon" || type === "MultiPolygon") &&
139-
equals(points[0], points[points.length - 1]) &&
140-
newPointsLength < 4
141-
) {
142-
throw new Error("invalid polygon");
143-
}
139+
// No remaining points, so commit the current a-b segment.
140+
newPoints.push(points[b]);
141+
142+
if (type === "Polygon" || type === "MultiPolygon") {
143+
// For polygons need to make sure the start / end point wasn't one of the
144+
// points that needed to be cleaned.
145+
// https://github.com/Turfjs/turf/issues/2406
146+
// For points [a, b, c, ..., z, a]
147+
// if a is on line b-z, it too can be removed. New array becomes
148+
// [b, c, ..., z, b]
149+
if (
150+
booleanPointOnLine(
151+
newPoints[0],
152+
lineString([newPoints[1], newPoints[newPoints.length - 2]])
153+
)
154+
) {
155+
newPoints.shift(); // Discard starting point.
156+
newPoints.pop(); // Discard closing point.
157+
newPoints.push(newPoints[0]); // Duplicate the new closing point to end of array.
158+
}
144159

145-
if (type === "LineString" && newPointsLength < 3) {
146-
return newPoints;
160+
// (Multi)Polygons must have at least 4 points and be closed.
161+
if (newPoints.length < 4) {
162+
throw new Error("invalid polygon, fewer than 4 points");
163+
}
164+
if (!equals(newPoints[0], newPoints[newPoints.length - 1])) {
165+
throw new Error("invalid polygon, first and last points not equal");
166+
}
147167
}
148168

149-
if (
150-
isPointOnLineSegment(
151-
newPoints[newPointsLength - 3],
152-
newPoints[newPointsLength - 1],
153-
newPoints[newPointsLength - 2]
154-
)
155-
)
156-
newPoints.splice(newPoints.length - 2, 1);
157-
158169
return newPoints;
159170
}
160171

@@ -170,35 +181,5 @@ function equals(pt1: Position, pt2: Position) {
170181
return pt1[0] === pt2[0] && pt1[1] === pt2[1];
171182
}
172183

173-
/**
174-
* Returns if `point` is on the segment between `start` and `end`.
175-
* Borrowed from `@turf/boolean-point-on-line` to speed up the evaluation (instead of using the module as dependency)
176-
*
177-
* @private
178-
* @param {Position} start coord pair of start of line
179-
* @param {Position} end coord pair of end of line
180-
* @param {Position} point coord pair of point to check
181-
* @returns {boolean} true/false
182-
*/
183-
function isPointOnLineSegment(start: Position, end: Position, point: Position) {
184-
var x = point[0],
185-
y = point[1];
186-
var startX = start[0],
187-
startY = start[1];
188-
var endX = end[0],
189-
endY = end[1];
190-
191-
var dxc = x - startX;
192-
var dyc = y - startY;
193-
var dxl = endX - startX;
194-
var dyl = endY - startY;
195-
var cross = dxc * dyl - dyc * dxl;
196-
197-
if (cross !== 0) return false;
198-
else if (Math.abs(dxl) >= Math.abs(dyl))
199-
return dxl > 0 ? startX <= x && x <= endX : endX <= x && x <= startX;
200-
else return dyl > 0 ? startY <= y && y <= endY : endY <= y && y <= startY;
201-
}
202-
203184
export { cleanCoords };
204185
export default cleanCoords;

‎packages/turf-clean-coords/package.json

+4-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"description": "Removes redundant coordinates from a GeoJSON Geometry.",
55
"author": "Turf Authors",
66
"contributors": [
7-
"Stefano Borghi <@stebogit>"
7+
"Stefano Borghi <@stebogit>",
8+
"James Beard <@smallsaucepan>"
89
],
910
"license": "MIT",
1011
"bugs": {
@@ -58,6 +59,7 @@
5859
"@types/benchmark": "^2.1.5",
5960
"@types/tape": "^4.13.4",
6061
"benchmark": "^2.1.4",
62+
"geojson-equality-ts": "^1.0.2",
6163
"load-json-file": "^7.0.1",
6264
"npm-run-all": "^4.1.5",
6365
"tape": "^5.9.0",
@@ -67,6 +69,7 @@
6769
"write-json-file": "^5.0.0"
6870
},
6971
"dependencies": {
72+
"@turf/boolean-point-on-line": "workspace:^",
7073
"@turf/helpers": "workspace:^",
7174
"@turf/invariant": "workspace:^",
7275
"@types/geojson": "^7946.0.10",

‎packages/turf-clean-coords/test.ts

+168-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import fs from "fs";
22
import test from "tape";
33
import path from "path";
4+
import { geojsonEquality } from "geojson-equality-ts";
45
import { fileURLToPath } from "url";
56
import { loadJsonFileSync } from "load-json-file";
67
import { truncate } from "@turf/truncate";
@@ -38,7 +39,10 @@ test("turf-clean-coords", (t) => {
3839

3940
if (process.env.REGEN)
4041
writeJsonFileSync(directories.out + filename, results);
41-
t.deepEqual(results, loadJsonFileSync(directories.out + filename), name);
42+
t.true(
43+
geojsonEquality(results, loadJsonFileSync(directories.out + filename)),
44+
name
45+
);
4246
});
4347
t.end();
4448
});
@@ -151,3 +155,166 @@ test("turf-clean-coords -- prevent input mutation", (t) => {
151155
t.deepEqual(multiPolyBefore, multiPoly, "multiPolygon should NOT be mutated");
152156
t.end();
153157
});
158+
159+
test("turf-clean-coords - north south lines - issue 2305", (t) => {
160+
// From https://github.com/Turfjs/turf/issues/2305#issue-1287442870
161+
t.deepEqual(
162+
cleanCoords(
163+
lineString([
164+
[0, 0],
165+
[0, 1],
166+
[0, 0],
167+
])
168+
),
169+
lineString([
170+
[0, 0],
171+
[0, 1],
172+
[0, 0],
173+
]),
174+
"northern turnaround point is kept"
175+
);
176+
177+
// From https://github.com/Turfjs/turf/issues/2305#issue-1287442870
178+
t.deepEqual(
179+
cleanCoords(
180+
lineString([
181+
[0, 0],
182+
[0, 0],
183+
[0, 2],
184+
[0, 2],
185+
[0, 0],
186+
])
187+
),
188+
lineString([
189+
[0, 0],
190+
[0, 2],
191+
[0, 0],
192+
]),
193+
"northern turnaround point is kept"
194+
);
195+
196+
t.end();
197+
});
198+
199+
test("turf-clean-coords - overly aggressive removal - issue 2740", (t) => {
200+
// Issue 2740 is cleanCoords was too aggresive at removing points.
201+
t.deepEqual(
202+
cleanCoords(
203+
lineString([
204+
[0, 0],
205+
[0, 2],
206+
[0, 0],
207+
])
208+
),
209+
lineString([
210+
[0, 0],
211+
[0, 2],
212+
[0, 0],
213+
]),
214+
"north-south retraced line turnaround point kept"
215+
);
216+
217+
t.deepEqual(
218+
cleanCoords(
219+
lineString([
220+
[0, 0],
221+
[0, 1],
222+
[0, 2],
223+
[0, 3],
224+
[0, 0],
225+
])
226+
),
227+
lineString([
228+
[0, 0],
229+
[0, 3],
230+
[0, 0],
231+
]),
232+
"north-south retraced line properly cleaned"
233+
);
234+
235+
t.deepEqual(
236+
cleanCoords(
237+
lineString([
238+
[0, 0],
239+
[0, 1],
240+
[0, 2],
241+
[0, -2],
242+
[0, -1],
243+
[0, 0],
244+
])
245+
),
246+
lineString([
247+
[0, 0],
248+
[0, 2],
249+
[0, -2],
250+
[0, 0],
251+
]),
252+
"north-south retraced past origin and back to start line properly cleaned"
253+
);
254+
255+
t.end();
256+
});
257+
258+
test("turf-clean-coords - start point protected - issue 2406", (t) => {
259+
t.true(
260+
geojsonEquality(
261+
cleanCoords(
262+
polygon([
263+
[
264+
[1, 3], // a
265+
[3, 3], // b
266+
[3, 1], // c
267+
[3, -3], // d
268+
[-3, -3], // e
269+
[-3, 3], // f
270+
[1, 3], // a
271+
],
272+
])
273+
),
274+
polygon([
275+
[
276+
[-3, 3], // f
277+
[3, 3], // b
278+
[3, -3], // d
279+
[-3, -3], // e
280+
[-3, 3], // f
281+
],
282+
])
283+
),
284+
"polygon start point (a) was also removed"
285+
);
286+
287+
t.end();
288+
});
289+
290+
test("turf-clean-coords - multipolygon - issue #918", (t) => {
291+
// Copied from turf-simplify as (at heart) it's cleanCoords that's being
292+
// tested here.
293+
// simplify hangs on this input #918
294+
t.throws(
295+
() =>
296+
cleanCoords(
297+
multiPolygon([
298+
[
299+
[
300+
[0, 90],
301+
[0, 90],
302+
[0, 90],
303+
[0, 90],
304+
[0, 90],
305+
[0, 90],
306+
[0, 90],
307+
[0, 90],
308+
[0, 90],
309+
[0, 90],
310+
[0, 90],
311+
],
312+
],
313+
])
314+
),
315+
/invalid polygon/,
316+
"invalid polygon"
317+
);
318+
319+
t.end();
320+
});

0 commit comments

Comments
 (0)