-
Notifications
You must be signed in to change notification settings - Fork 990
Fix @turf/nearest-point-on-line endpoint selection and degenerate input cases #2940
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.
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
Changes from 1 commit
1c5f047
a80a828
7511687
fcc30b8
cf33d55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,12 +75,10 @@ function nearestPointOnLine<G extends LineString | MultiLineString>( | |
| for (let i = 0; i < coords.length - 1; i++) { | ||
| //start - start of current line section | ||
| const start: Feature<Point, { dist: number }> = point(coords[i]); | ||
| start.properties.dist = distance(pt, start, options); | ||
| const startPos = getCoord(start); | ||
|
|
||
| //stop - end of current line section | ||
| const stop: Feature<Point, { dist: number }> = point(coords[i + 1]); | ||
| stop.properties.dist = distance(pt, stop, options); | ||
| const stopPos = getCoord(stop); | ||
|
|
||
| // sectionLength | ||
|
|
@@ -89,40 +87,28 @@ function nearestPointOnLine<G extends LineString | MultiLineString>( | |
| let wasEnd: boolean; | ||
|
|
||
| // Short circuit if snap point is start or end position of the line | ||
| // segment or if start is equal to stop position. | ||
| if (startPos[0] === ptPos[0] && startPos[1] === ptPos[1]) { | ||
| [intersectPos, , wasEnd] = [startPos, undefined, false]; | ||
| } else if (stopPos[0] === ptPos[0] && stopPos[1] === ptPos[1]) { | ||
| [intersectPos, , wasEnd] = [stopPos, undefined, true]; | ||
| } else if (startPos[0] === stopPos[0] && startPos[1] === stopPos[1]) { | ||
| [intersectPos, , wasEnd] = [stopPos, undefined, true]; | ||
| // Test the end position first for consistency in case they are | ||
| // coincident | ||
| if (stopPos[0] === ptPos[0] && stopPos[1] === ptPos[1]) { | ||
| [intersectPos, wasEnd] = [stopPos, true]; | ||
| } else if (startPos[0] === ptPos[0] && startPos[1] === ptPos[1]) { | ||
| [intersectPos, wasEnd] = [startPos, false]; | ||
| } else { | ||
| // Otherwise, find the nearest point the hard way. | ||
| [intersectPos, , wasEnd] = nearestPointOnSegment( | ||
| start.geometry.coordinates, | ||
| stop.geometry.coordinates, | ||
| getCoord(pt) | ||
| [intersectPos, wasEnd] = nearestPointOnSegment( | ||
| startPos, | ||
| stopPos, | ||
| ptPos | ||
| ); | ||
| } | ||
| let intersectPt: | ||
| | Feature< | ||
| Point, | ||
| { dist: number; multiFeatureIndex: number; location: number } | ||
| > | ||
| | undefined; | ||
|
|
||
| if (intersectPos) { | ||
| intersectPt = point(intersectPos, { | ||
| dist: distance(pt, intersectPos, options), | ||
| multiFeatureIndex: multiFeatureIndex, | ||
| location: length + distance(start, intersectPos, options), | ||
| }); | ||
| } | ||
|
|
||
| if ( | ||
| intersectPt && | ||
| intersectPt.properties.dist < closestPt.properties.dist | ||
| ) { | ||
| const intersectPt = point(intersectPos, { | ||
| dist: distance(pt, intersectPos, options), | ||
| multiFeatureIndex: multiFeatureIndex, | ||
| location: length + distance(start, intersectPos, options), | ||
| }); | ||
|
|
||
| if (intersectPt.properties.dist < closestPt.properties.dist) { | ||
| closestPt = { | ||
| ...intersectPt, | ||
| properties: { | ||
|
|
@@ -164,10 +150,15 @@ function cross(v1: Vector, v2: Vector): Vector { | |
| return [v1y * v2z - v1z * v2y, v1z * v2x - v1x * v2z, v1x * v2y - v1y * v2x]; | ||
| } | ||
|
|
||
| function magnitude(v: Vector) { | ||
| function magnitude(v: Vector): number { | ||
| return Math.sqrt(Math.pow(v[0], 2) + Math.pow(v[1], 2) + Math.pow(v[2], 2)); | ||
| } | ||
|
|
||
| function normalize(v: Vector): Vector { | ||
| const mag = magnitude(v); | ||
| return [v[0] / mag, v[1] / mag, v[2] / mag]; | ||
| } | ||
|
|
||
| function angle(v1: Vector, v2: Vector): number { | ||
| const theta = dot(v1, v2) / (magnitude(v1) * magnitude(v2)); | ||
| return Math.acos(Math.min(Math.max(theta, -1), 1)); | ||
|
|
@@ -185,7 +176,10 @@ function lngLatToVector(a: Position): Vector { | |
|
|
||
| function vectorToLngLat(v: Vector): Position { | ||
| const [x, y, z] = v; | ||
| const lat = radiansToDegrees(Math.asin(z)); | ||
| // Clamp the z-value to ensure that is inside the [-1, 1] domain as required | ||
| // by asin. Note that | ||
| const zClamp = Math.min(Math.max(z, -1), 1); | ||
| const lat = radiansToDegrees(Math.asin(zClamp)); | ||
| const lng = radiansToDegrees(Math.atan2(y, x)); | ||
|
|
||
| return [lng, lat]; | ||
|
|
@@ -195,7 +189,7 @@ function nearestPointOnSegment( | |
| posA: Position, // start point of segment to measure to | ||
| posB: Position, // end point of segment to measure to | ||
| posC: Position // point to measure from | ||
| ): [Position, boolean, boolean] { | ||
| ): [Position, boolean] { | ||
| // Based heavily on this article on finding cross track distance to an arc: | ||
| // https://gis.stackexchange.com/questions/209540/projecting-cross-track-distance-on-great-circle | ||
|
|
||
|
|
@@ -206,62 +200,80 @@ function nearestPointOnSegment( | |
| const B = lngLatToVector(posB); // ... to posB | ||
| const C = lngLatToVector(posC); // ... to posC | ||
|
|
||
| // Components of target point. | ||
| const [Cx, Cy, Cz] = C; | ||
| // The axis (normal vector) of the great circle plane containing the line segment | ||
| const segmentAxis = cross(A, B); | ||
|
|
||
| // Two degenerate cases exist for the segment axis cross product. The first is | ||
| // when vectors are aligned (within the bounds of floating point tolerance). | ||
| // The second is where vectors are antipodal (again within the bounds of | ||
| // tolerance. Both cases produce a [0, 0, 0] cross product which invalidates | ||
| // the rest of the algorithm, but each case must be handled separately: | ||
| // - The aligned case indicates coincidence of A and B. therefore this can be | ||
| // an early return assuming the closest point is the end (for consistency). | ||
| // - The antipodal case is truly degenerate - an infinte number of great | ||
| // circles are possible and one will always pass through C. We assume this | ||
| // is undefined behavior and therefore throw. Callers can catch this and | ||
| // return 0 if they wish. | ||
| if (segmentAxis[0] === 0 && segmentAxis[1] === 0 && segmentAxis[2] === 0) { | ||
| if (dot(A, B) > 0) { | ||
| return [[...posB], true]; | ||
| } else { | ||
| throw new Error( | ||
|
||
| `Undefined arc segment, line segment endpoints [[${posA}], [${posB}]] are antipodes` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // Calculate coefficients. | ||
| const [D, E, F] = cross(A, B); | ||
| const a = E * Cz - F * Cy; | ||
| const b = F * Cx - D * Cz; | ||
| const c = D * Cy - E * Cx; | ||
| // The axis of the great circle passing through the segment's axis and the | ||
| // target point | ||
| const targetAxis = cross(segmentAxis, C); | ||
|
|
||
| const f = c * E - b * F; | ||
| const g = a * F - c * D; | ||
| const h = b * D - a * E; | ||
| // This cross product also has a degenerate case where the segment axis is | ||
| // coincident with or antipodal to the target point. In this case the point | ||
| // is equidistant to the entire segment. For consistency, we early return the | ||
| // endpoint as the matching point. | ||
| if (targetAxis[0] === 0 && targetAxis[1] === 0 && targetAxis[2] === 0) { | ||
| return [[...posB], true]; | ||
| } | ||
|
|
||
| const t = 1 / Math.sqrt(Math.pow(f, 2) + Math.pow(g, 2) + Math.pow(h, 2)); | ||
| // The line of intersection between the two great circle planes | ||
| const intersectionAxis = cross(targetAxis, segmentAxis); | ||
|
|
||
| // Vectors to the two points these great circles intersect. | ||
| const I1: Vector = [f * t, g * t, h * t]; | ||
| const I2: Vector = [-1 * f * t, -1 * g * t, -1 * h * t]; | ||
| // Vectors to the two points these great circles intersect are the normalized | ||
| // intersection and its antipodes | ||
| const I1 = normalize(intersectionAxis); | ||
| const I2: Vector = [-I1[0], -I1[1], -I1[2]]; | ||
|
|
||
| // Figure out which is the closest intersection to this segment of the great | ||
| // circle. | ||
| const angleAB = angle(A, B); | ||
| const angleAI1 = angle(A, I1); | ||
| const angleBI1 = angle(B, I1); | ||
| const angleAI2 = angle(A, I2); | ||
| const angleBI2 = angle(B, I2); | ||
|
|
||
| let I: Vector; | ||
|
|
||
| if ( | ||
| (angleAI1 < angleAI2 && angleAI1 < angleBI2) || | ||
| (angleBI1 < angleAI2 && angleBI1 < angleBI2) | ||
| ) { | ||
| I = I1; | ||
| } else { | ||
| I = I2; | ||
| } | ||
| // Figure out which is the closest intersection to this segment of the great circle | ||
| // Note that for points on a unit sphere, the dot product represents the | ||
| // cosine of the angle between the two vectors which monotonically increases | ||
| // the closer the two points are together and therefore determines proximity | ||
| const I = dot(C, I1) > dot(C, I2) ? I1 : I2; | ||
|
|
||
| // I is the closest intersection to the segment, though might not actually be | ||
| // ON the segment. | ||
|
|
||
| // If angle AI or BI is greater than angleAB, I lies on the circle *beyond* A | ||
| // and B so use the closest of A or B as the intersection | ||
| const angleAB = angle(A, B); | ||
| const lngLatI = vectorToLngLat(I); | ||
|
|
||
| if (angle(A, I) > angleAB || angle(B, I) > angleAB) { | ||
| if ( | ||
| distance(vectorToLngLat(I), vectorToLngLat(A)) <= | ||
| distance(vectorToLngLat(I), vectorToLngLat(B)) | ||
| ) { | ||
| return [vectorToLngLat(A), true, false]; | ||
| // Similar to the usage above, we use the larger dot product to determine | ||
| // which endpoint is closer to the test coordinates | ||
| // Note that the > means we defer to the endpoint when equidistant, | ||
| // following the segment tracking logic in the caller | ||
| if (dot(A, C) > dot(B, C)) { | ||
| // Clone position when returning as it is reasonable to not expect structural | ||
| // sharing on the returned Position in all return cases | ||
| return [[...posA], false]; | ||
| } else { | ||
| return [vectorToLngLat(B), false, true]; | ||
| return [[...posB], true]; | ||
| } | ||
| } | ||
|
|
||
| // As angleAI nor angleBI don't exceed angleAB, I is on the segment | ||
| return [vectorToLngLat(I), false, false]; | ||
| return [lngLatI, false]; | ||
| } | ||
|
|
||
| export { nearestPointOnLine }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hanging
Note thatin the commentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah whoops. I'll fix that when I get a chance and also look at the point-on-polygon tests that failed. These were succeeding when I tried earlier, but are worthwhile checking into due to the floating point funniness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to confirm that the failed tests in turf-point-to-polygon-distance are due to a single bit difference in the latitude of the nearest point between the old version and the new version. See below for an example from the first failing test:
Then as the result is in meters, the single bit difference creeps up. Yay for floating point :-).
Given that a single bit floating point difference is inside even the boundaries of trig functions in different JS engines I see no issue in just changing the expected distance, although it does beg a larger questions as to whether those tests should test with a tolerance like some others do... problem for another time.
I will amend and update the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed readout from this investigation!