Skip to content

Commit 984b84c

Browse files
smallsaucepansmallsaucepan
authored
Fixed bug where nearestPointOnLine would return an index to a non-existent segment (#2951)
This was having a flow on effect in lineSlice where an additional duplicate point would be added as a result.
1 parent 21c78df commit 984b84c

File tree

9 files changed

+189
-7
lines changed

9 files changed

+189
-7
lines changed

‎packages/turf-line-slice/test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,31 @@ test("turf-nearest-point-on-line -- issue 2023", (t) => {
5959
);
6060
t.end();
6161
});
62+
63+
test("issue 2946 - lineSlice didn't introduce duplicate points", (t) => {
64+
// Test case copied from https://github.com/Turfjs/turf/issues/2946
65+
// Values rounded to lower precision for realism. Same behaviour as
66+
// originally reported though.
67+
const startPt = point([2, 0]);
68+
const endPt = point([2, 2]);
69+
const line = lineString([
70+
[2.999848, 0.000152],
71+
[2, -1],
72+
[2, 2],
73+
]);
74+
75+
const r1 = lineSlice(startPt, endPt, line);
76+
t.equal(
77+
r1.geometry.coordinates.length,
78+
2,
79+
"Two points only in the linestring"
80+
);
81+
82+
// This part of the issue report should have already been fixed by the
83+
// recent PR #2940.
84+
t.doesNotThrow(() => {
85+
const r2 = lineSlice(startPt, endPt, r1);
86+
}, "Does not throw when called again on first result");
87+
88+
t.end();
89+
});

‎packages/turf-line-slice/test/out/route2.geojson‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6355,7 +6355,6 @@
63556355
[-112.622281, 45.973317],
63566356
[-112.622078, 45.970234],
63576357
[-112.619831, 45.970225],
6358-
[-112.614288, 45.970203],
63596358
[-112.614288, 45.970203]
63606359
]
63616360
}

‎packages/turf-nearest-point-on-line/index.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ function nearestPointOnLine<G extends LineString | MultiLineString>(
7575
lines,
7676
function (line: any, _featureIndex: number, multiFeatureIndex: number) {
7777
const coords: any = getCoords(line);
78+
const maxSegmentIndex = coords.length - 2;
7879

7980
for (let i = 0; i < coords.length - 1; i++) {
8081
//start - start of current line section
@@ -117,9 +118,11 @@ function nearestPointOnLine<G extends LineString | MultiLineString>(
117118
...intersectPt,
118119
properties: {
119120
...intersectPt.properties,
120-
// Legacy behaviour where index progresses to next segment # if we
121-
// went with the end point this iteration.
122-
index: wasEnd ? i + 1 : i,
121+
// Legacy behaviour where index progresses to next segment if we
122+
// went with the end point this iteration. Though make sure we
123+
// only progress to the beginning of the next segment if one
124+
// actually exists.
125+
index: wasEnd && i + 1 <= maxSegmentIndex ? i + 1 : i,
123126
},
124127
};
125128
}

‎packages/turf-nearest-point-on-line/test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ test("turf-nearest-point-on-line", (t) => {
4848

4949
if (process.env.REGEN)
5050
writeJsonFileSync(directories.out + filename, results);
51-
t.deepEqual(loadJsonFileSync(directories.out + filename), results, name);
51+
t.deepEqual(results, loadJsonFileSync(directories.out + filename), name);
5252
}
5353
t.end();
5454
});
@@ -320,7 +320,7 @@ test("turf-nearest-point-on-line - check dist and index", (t) => {
320320
const pt = point([-92.110576, 41.040649]);
321321
const snapped = truncate(nearestPointOnLine(line, pt));
322322

323-
t.equal(snapped.properties.index, 8, "properties.index");
323+
t.equal(snapped.properties.index, 7, "properties.index");
324324
t.equal(
325325
Number(snapped.properties.dist.toFixed(6)),
326326
0.823802,

‎packages/turf-nearest-point-on-line/test/in/end-point-1.geojson‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
{
2+
"type": "FeatureCollection",
3+
"features": [
4+
{
5+
"type": "Feature",
6+
"properties": {
7+
"description": "Result index should be 2 as there is a third segment and matches to an endpoint bump to the start of the next segment"
8+
},
9+
"geometry": {
10+
"coordinates": [
11+
[151.12342, -29.776038],
12+
[151.112785, -29.783069],
13+
[151.119935, -29.764135],
14+
[151.107173, -29.768495]
15+
],
16+
"type": "LineString"
17+
}
18+
},
19+
{
20+
"type": "Feature",
21+
"properties": {
22+
"marker-color": "green"
23+
},
24+
"geometry": {
25+
"coordinates": [151.12207, -29.761332],
26+
"type": "Point"
27+
}
28+
}
29+
]
30+
}

‎packages/turf-nearest-point-on-line/test/in/end-point-2.geojson‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
{
2+
"type": "FeatureCollection",
3+
"features": [
4+
{
5+
"type": "Feature",
6+
"properties": {
7+
"description": "Result index should be 1 as there is no later segment to bump the index to."
8+
},
9+
"geometry": {
10+
"coordinates": [
11+
[151.12342, -29.776038],
12+
[151.112785, -29.783069],
13+
[151.119935, -29.764135]
14+
],
15+
"type": "LineString"
16+
}
17+
},
18+
{
19+
"type": "Feature",
20+
"properties": {
21+
"marker-color": "green"
22+
},
23+
"geometry": {
24+
"coordinates": [151.12207, -29.761332],
25+
"type": "Point"
26+
}
27+
}
28+
]
29+
}

‎packages/turf-nearest-point-on-line/test/out/end-point-1.geojson‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"type": "FeatureCollection",
3+
"features": [
4+
{
5+
"type": "Feature",
6+
"properties": {
7+
"description": "Result index should be 2 as there is a third segment and matches to an endpoint bump to the start of the next segment"
8+
},
9+
"geometry": {
10+
"coordinates": [
11+
[151.12342, -29.776038],
12+
[151.112785, -29.783069],
13+
[151.119935, -29.764135],
14+
[151.107173, -29.768495]
15+
],
16+
"type": "LineString"
17+
}
18+
},
19+
{
20+
"type": "Feature",
21+
"properties": { "stroke": "#F00", "stroke-width": 6 },
22+
"geometry": {
23+
"type": "LineString",
24+
"coordinates": [
25+
[151.119935, -29.764135],
26+
[151.12207, -29.761332]
27+
]
28+
}
29+
},
30+
{
31+
"type": "Feature",
32+
"properties": { "marker-color": "green" },
33+
"geometry": { "coordinates": [151.12207, -29.761332], "type": "Point" }
34+
},
35+
{
36+
"type": "Feature",
37+
"properties": {
38+
"dist": 0.373652,
39+
"multiFeatureIndex": 0,
40+
"location": 3.505821,
41+
"index": 2,
42+
"marker-color": "#F0F"
43+
},
44+
"geometry": { "type": "Point", "coordinates": [151.119935, -29.764135] }
45+
}
46+
]
47+
}

‎packages/turf-nearest-point-on-line/test/out/end-point-2.geojson‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
{
2+
"type": "FeatureCollection",
3+
"features": [
4+
{
5+
"type": "Feature",
6+
"properties": {
7+
"description": "Result index should be 1 as there is no later segment to bump the index to."
8+
},
9+
"geometry": {
10+
"coordinates": [
11+
[151.12342, -29.776038],
12+
[151.112785, -29.783069],
13+
[151.119935, -29.764135]
14+
],
15+
"type": "LineString"
16+
}
17+
},
18+
{
19+
"type": "Feature",
20+
"properties": { "stroke": "#F00", "stroke-width": 6 },
21+
"geometry": {
22+
"type": "LineString",
23+
"coordinates": [
24+
[151.119935, -29.764135],
25+
[151.12207, -29.761332]
26+
]
27+
}
28+
},
29+
{
30+
"type": "Feature",
31+
"properties": { "marker-color": "green" },
32+
"geometry": { "coordinates": [151.12207, -29.761332], "type": "Point" }
33+
},
34+
{
35+
"type": "Feature",
36+
"properties": {
37+
"dist": 0.373652,
38+
"multiFeatureIndex": 0,
39+
"location": 3.505821,
40+
"index": 1,
41+
"marker-color": "#F0F"
42+
},
43+
"geometry": { "type": "Point", "coordinates": [151.119935, -29.764135] }
44+
}
45+
]
46+
}

‎packages/turf-nearest-point-on-line/test/out/multiLine1.geojson‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
"type": "Feature",
6969
"properties": {
7070
"dist": 114.725451,
71-
"index": 21,
71+
"index": 20,
7272
"multiFeatureIndex": 1,
7373
"location": 9479.011715,
7474
"marker-color": "#F0F"

0 commit comments

Comments
 (0)