diff --git a/packages/turf-clean-coords/index.ts b/packages/turf-clean-coords/index.ts index 3be8ba3ef..9c8660252 100644 --- a/packages/turf-clean-coords/index.ts +++ b/packages/turf-clean-coords/index.ts @@ -1,6 +1,8 @@ import { Position } from "geojson"; import { feature } from "@turf/helpers"; import { getCoords, getType } from "@turf/invariant"; +import { booleanPointOnLine } from "@turf/boolean-point-on-line"; +import { lineString } from "@turf/helpers"; // To-Do => Improve Typescript GeoJSON handling @@ -99,62 +101,71 @@ function cleanCoords( * @returns {Array} Cleaned coordinates */ function cleanLine(line: Position[], type: string) { - var points = getCoords(line); + const points = getCoords(line); // handle "clean" segment if (points.length === 2 && !equals(points[0], points[1])) return points; - var newPoints = []; - var secondToLast = points.length - 1; - var newPointsLength = newPoints.length; - - newPoints.push(points[0]); - for (var i = 1; i < secondToLast; i++) { - var prevAddedPoint = newPoints[newPoints.length - 1]; - if ( - points[i][0] === prevAddedPoint[0] && - points[i][1] === prevAddedPoint[1] - ) - continue; - else { - newPoints.push(points[i]); - newPointsLength = newPoints.length; - if (newPointsLength > 2) { - if ( - isPointOnLineSegment( - newPoints[newPointsLength - 3], - newPoints[newPointsLength - 1], - newPoints[newPointsLength - 2] - ) - ) - newPoints.splice(newPoints.length - 2, 1); - } + const newPoints = []; + + // Segments based approach. With initial segment a-b, keep comparing to a + // longer segment a-c and as long as b is still on a-c, b is a redundant + // point. + let a = 0, + b = 1, + c = 2; + + // Guaranteed we'll use the first point. + newPoints.push(points[a]); + // While there is still room to extend the segment ... + while (c < points.length) { + if (booleanPointOnLine(points[b], lineString([points[a], points[c]]))) { + // b is on a-c, so we can discard point b, and extend a-b to be the same + // as a-c as the basis for comparison during the next iteration. + b = c; + } else { + // b is NOT on a-c, suggesting a-c is not an extension of a-b. Commit a-b + // as a necessary segment. + newPoints.push(points[b]); + + // Make our a-b for the next iteration start from the end of the segment + // that was just locked in i.e. next a-b should be the current b-(b+1). + a = b; + b++; + c = b; } + // Plan to look at the next point during the next iteration. + c++; } - newPoints.push(points[points.length - 1]); - newPointsLength = newPoints.length; - - // (Multi)Polygons must have at least 4 points, but a closed LineString with only 3 points is acceptable - if ( - (type === "Polygon" || type === "MultiPolygon") && - equals(points[0], points[points.length - 1]) && - newPointsLength < 4 - ) { - throw new Error("invalid polygon"); - } + // No remaining points, so commit the current a-b segment. + newPoints.push(points[b]); + + if (type === "Polygon" || type === "MultiPolygon") { + // For polygons need to make sure the start / end point wasn't one of the + // points that needed to be cleaned. + // https://github.com/Turfjs/turf/issues/2406 + // For points [a, b, c, ..., z, a] + // if a is on line b-z, it too can be removed. New array becomes + // [b, c, ..., z, b] + if ( + booleanPointOnLine( + newPoints[0], + lineString([newPoints[1], newPoints[newPoints.length - 2]]) + ) + ) { + newPoints.shift(); // Discard starting point. + newPoints.pop(); // Discard closing point. + newPoints.push(newPoints[0]); // Duplicate the new closing point to end of array. + } - if (type === "LineString" && newPointsLength < 3) { - return newPoints; + // (Multi)Polygons must have at least 4 points and be closed. + if (newPoints.length < 4) { + throw new Error("invalid polygon, fewer than 4 points"); + } + if (!equals(newPoints[0], newPoints[newPoints.length - 1])) { + throw new Error("invalid polygon, first and last points not equal"); + } } - if ( - isPointOnLineSegment( - newPoints[newPointsLength - 3], - newPoints[newPointsLength - 1], - newPoints[newPointsLength - 2] - ) - ) - newPoints.splice(newPoints.length - 2, 1); - return newPoints; } @@ -170,35 +181,5 @@ function equals(pt1: Position, pt2: Position) { return pt1[0] === pt2[0] && pt1[1] === pt2[1]; } -/** - * Returns if `point` is on the segment between `start` and `end`. - * Borrowed from `@turf/boolean-point-on-line` to speed up the evaluation (instead of using the module as dependency) - * - * @private - * @param {Position} start coord pair of start of line - * @param {Position} end coord pair of end of line - * @param {Position} point coord pair of point to check - * @returns {boolean} true/false - */ -function isPointOnLineSegment(start: Position, end: Position, point: Position) { - var x = point[0], - y = point[1]; - var startX = start[0], - startY = start[1]; - var endX = end[0], - endY = end[1]; - - var dxc = x - startX; - var dyc = y - startY; - var dxl = endX - startX; - var dyl = endY - startY; - var cross = dxc * dyl - dyc * dxl; - - if (cross !== 0) return false; - else if (Math.abs(dxl) >= Math.abs(dyl)) - return dxl > 0 ? startX <= x && x <= endX : endX <= x && x <= startX; - else return dyl > 0 ? startY <= y && y <= endY : endY <= y && y <= startY; -} - export { cleanCoords }; export default cleanCoords; diff --git a/packages/turf-clean-coords/package.json b/packages/turf-clean-coords/package.json index d04796fb6..c7c976e90 100644 --- a/packages/turf-clean-coords/package.json +++ b/packages/turf-clean-coords/package.json @@ -4,7 +4,8 @@ "description": "Removes redundant coordinates from a GeoJSON Geometry.", "author": "Turf Authors", "contributors": [ - "Stefano Borghi <@stebogit>" + "Stefano Borghi <@stebogit>", + "James Beard <@smallsaucepan>" ], "license": "MIT", "bugs": { @@ -58,6 +59,7 @@ "@types/benchmark": "^2.1.5", "@types/tape": "^4.13.4", "benchmark": "^2.1.4", + "geojson-equality-ts": "^1.0.2", "load-json-file": "^7.0.1", "npm-run-all": "^4.1.5", "tape": "^5.9.0", @@ -67,6 +69,7 @@ "write-json-file": "^5.0.0" }, "dependencies": { + "@turf/boolean-point-on-line": "workspace:^", "@turf/helpers": "workspace:^", "@turf/invariant": "workspace:^", "@types/geojson": "^7946.0.10", diff --git a/packages/turf-clean-coords/test.ts b/packages/turf-clean-coords/test.ts index 60f77f4cf..8bd99215f 100644 --- a/packages/turf-clean-coords/test.ts +++ b/packages/turf-clean-coords/test.ts @@ -1,6 +1,7 @@ import fs from "fs"; import test from "tape"; import path from "path"; +import { geojsonEquality } from "geojson-equality-ts"; import { fileURLToPath } from "url"; import { loadJsonFileSync } from "load-json-file"; import { truncate } from "@turf/truncate"; @@ -38,7 +39,10 @@ test("turf-clean-coords", (t) => { if (process.env.REGEN) writeJsonFileSync(directories.out + filename, results); - t.deepEqual(results, loadJsonFileSync(directories.out + filename), name); + t.true( + geojsonEquality(results, loadJsonFileSync(directories.out + filename)), + name + ); }); t.end(); }); @@ -151,3 +155,166 @@ test("turf-clean-coords -- prevent input mutation", (t) => { t.deepEqual(multiPolyBefore, multiPoly, "multiPolygon should NOT be mutated"); t.end(); }); + +test("turf-clean-coords - north south lines - issue 2305", (t) => { + // From https://github.com/Turfjs/turf/issues/2305#issue-1287442870 + t.deepEqual( + cleanCoords( + lineString([ + [0, 0], + [0, 1], + [0, 0], + ]) + ), + lineString([ + [0, 0], + [0, 1], + [0, 0], + ]), + "northern turnaround point is kept" + ); + + // From https://github.com/Turfjs/turf/issues/2305#issue-1287442870 + t.deepEqual( + cleanCoords( + lineString([ + [0, 0], + [0, 0], + [0, 2], + [0, 2], + [0, 0], + ]) + ), + lineString([ + [0, 0], + [0, 2], + [0, 0], + ]), + "northern turnaround point is kept" + ); + + t.end(); +}); + +test("turf-clean-coords - overly aggressive removal - issue 2740", (t) => { + // Issue 2740 is cleanCoords was too aggresive at removing points. + t.deepEqual( + cleanCoords( + lineString([ + [0, 0], + [0, 2], + [0, 0], + ]) + ), + lineString([ + [0, 0], + [0, 2], + [0, 0], + ]), + "north-south retraced line turnaround point kept" + ); + + t.deepEqual( + cleanCoords( + lineString([ + [0, 0], + [0, 1], + [0, 2], + [0, 3], + [0, 0], + ]) + ), + lineString([ + [0, 0], + [0, 3], + [0, 0], + ]), + "north-south retraced line properly cleaned" + ); + + t.deepEqual( + cleanCoords( + lineString([ + [0, 0], + [0, 1], + [0, 2], + [0, -2], + [0, -1], + [0, 0], + ]) + ), + lineString([ + [0, 0], + [0, 2], + [0, -2], + [0, 0], + ]), + "north-south retraced past origin and back to start line properly cleaned" + ); + + t.end(); +}); + +test("turf-clean-coords - start point protected - issue 2406", (t) => { + t.true( + geojsonEquality( + cleanCoords( + polygon([ + [ + [1, 3], // a + [3, 3], // b + [3, 1], // c + [3, -3], // d + [-3, -3], // e + [-3, 3], // f + [1, 3], // a + ], + ]) + ), + polygon([ + [ + [-3, 3], // f + [3, 3], // b + [3, -3], // d + [-3, -3], // e + [-3, 3], // f + ], + ]) + ), + "polygon start point (a) was also removed" + ); + + t.end(); +}); + +test("turf-clean-coords - multipolygon - issue #918", (t) => { + // Copied from turf-simplify as (at heart) it's cleanCoords that's being + // tested here. + // simplify hangs on this input #918 + t.throws( + () => + cleanCoords( + multiPolygon([ + [ + [ + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + [0, 90], + ], + ], + ]) + ), + /invalid polygon/, + "invalid polygon" + ); + + t.end(); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 78f46c395..cfa9527d5 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2057,6 +2057,9 @@ importers: packages/turf-clean-coords: dependencies: + '@turf/boolean-point-on-line': + specifier: workspace:^ + version: link:../turf-boolean-point-on-line '@turf/helpers': specifier: workspace:^ version: link:../turf-helpers @@ -2082,6 +2085,9 @@ importers: benchmark: specifier: ^2.1.4 version: 2.1.4 + geojson-equality-ts: + specifier: ^1.0.2 + version: 1.0.2 load-json-file: specifier: ^7.0.1 version: 7.0.1 @@ -2093,7 +2099,7 @@ importers: version: 5.9.0 tsup: specifier: ^8.3.5 - version: 8.3.5(postcss@8.4.40)(tsx@4.19.2)(typescript@5.5.4) + version: 8.3.5(tsx@4.19.2)(typescript@5.5.4) tsx: specifier: ^4.19.2 version: 4.19.2 @@ -12107,7 +12113,6 @@ packages: resolution: {integrity: sha512-h3Ryq+0mCSN/7yLs0eDgrZhvc9af23o/QuC4aTiuuzP/MRCtd6mf5rLsLRY44jX0RPUfM8c4GqERQmlUxPGPoQ==} dependencies: '@types/geojson': 7946.0.14 - dev: false /geojson-polygon-self-intersections@1.2.1: resolution: {integrity: sha512-/QM1b5u2d172qQVO//9CGRa49jEmclKEsYOQmWP9ooEjj63tBM51m2805xsbxkzlEELQ2REgTf700gUhhlegxA==} @@ -15421,6 +15426,28 @@ packages: tsx: 4.19.2 dev: true + /postcss-load-config@6.0.1(tsx@4.19.2): + resolution: {integrity: sha512-oPtTM4oerL+UXmx+93ytZVN82RrlY/wPUV8IeDxFrzIjXOLF1pN+EmKPLbubvKHT2HC20xXsCAH2Z+CKV6Oz/g==} + engines: {node: '>= 18'} + peerDependencies: + jiti: '>=1.21.0' + postcss: '>=8.0.9' + tsx: ^4.8.1 + yaml: ^2.4.2 + peerDependenciesMeta: + jiti: + optional: true + postcss: + optional: true + tsx: + optional: true + yaml: + optional: true + dependencies: + lilconfig: 3.1.3 + tsx: 4.19.2 + dev: true + /postcss-selector-parser@6.1.2: resolution: {integrity: sha512-Q8qQfPiZ+THO/3ZrOrO0cJJKfpYCagtMUkXbnEfmgUjwXg6z/WBeOyS9APBBPCTSiDV+s4SwQGu8yFsiMRIudg==} engines: {node: '>=4'} @@ -17035,6 +17062,49 @@ packages: - yaml dev: true + /tsup@8.3.5(tsx@4.19.2)(typescript@5.5.4): + resolution: {integrity: sha512-Tunf6r6m6tnZsG9GYWndg0z8dEV7fD733VBFzFJ5Vcm1FtlXB8xBD/rtrBi2a3YKEV7hHtxiZtW5EAVADoe1pA==} + engines: {node: '>=18'} + hasBin: true + peerDependencies: + '@microsoft/api-extractor': ^7.36.0 + '@swc/core': ^1 + postcss: ^8.4.12 + typescript: '>=4.5.0' + peerDependenciesMeta: + '@microsoft/api-extractor': + optional: true + '@swc/core': + optional: true + postcss: + optional: true + typescript: + optional: true + dependencies: + bundle-require: 5.0.0(esbuild@0.24.0) + cac: 6.7.14 + chokidar: 4.0.1 + consola: 3.2.3 + debug: 4.3.7 + esbuild: 0.24.0 + joycon: 3.1.1 + picocolors: 1.1.1 + postcss-load-config: 6.0.1(tsx@4.19.2) + resolve-from: 5.0.0 + rollup: 4.28.0 + source-map: 0.8.0-beta.0 + sucrase: 3.35.0 + tinyexec: 0.3.1 + tinyglobby: 0.2.10 + tree-kill: 1.2.2 + typescript: 5.5.4 + transitivePeerDependencies: + - jiti + - supports-color + - tsx + - yaml + dev: true + /tsx@4.19.2: resolution: {integrity: sha512-pOUl6Vo2LUq/bSa8S5q7b91cgNSjctn9ugq/+Mvow99qW6x/UZYwzxy/3NmqoT66eHYfCVvFvACC58UBPFf28g==} engines: {node: '>=18.0.0'}