-
-
Notifications
You must be signed in to change notification settings - Fork 2k
refactor: Move geo utilities #7881
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
Open
camdecoster
wants to merge
3
commits into
master
Choose a base branch
from
cam/7879/move-geo-utilities
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+159
−107
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,28 @@ function locationToFeature(locationmode, location, features) { | |
| return false; | ||
| } | ||
|
|
||
| // Offset used to lift negative longitudes (-180..0) into a continuous frame | ||
| // (180..360) so polygons and points that straddle the antimeridian can be | ||
| // compared with linear math. Shared between polygon stitching and hover | ||
| // hit-testing so both sides stay in sync. | ||
| const ANTIMERIDIAN_LON_SHIFT = 360; | ||
|
|
||
| /** | ||
| * Find the first index where a polygon ring crosses the antimeridian | ||
| * (a transition from positive to negative longitude between consecutive | ||
| * points). Returns null when no crossing is found. | ||
| * | ||
| * @param {Array<Array<number>>} pts - polygon points as [lon, lat] pairs | ||
| * @return {number|null} index of the segment that crosses, or null | ||
| */ | ||
| function doesCrossAntiMeridian(pts) { | ||
| for (let l = 0; l < pts.length - 1; l++) { | ||
| if (pts[l][0] > 0 && pts[l + 1][0] < 0) return l; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| function feature2polygons(feature) { | ||
| var geometry = feature.geometry; | ||
| var coords = geometry.coordinates; | ||
|
|
@@ -87,13 +109,6 @@ function feature2polygons(feature) { | |
| var polygons = []; | ||
| var appendPolygon, j, k, m; | ||
|
|
||
| function doesCrossAntiMerdian(pts) { | ||
| for (var l = 0; l < pts.length - 1; l++) { | ||
| if (pts[l][0] > 0 && pts[l + 1][0] < 0) return l; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| if (loc === 'RUS' || loc === 'FJI') { | ||
| // Russia and Fiji have landmasses that cross the antimeridian, | ||
| // we need to add +360 to their longitude coordinates, so that | ||
|
|
@@ -105,13 +120,13 @@ function feature2polygons(feature) { | |
| appendPolygon = function (_pts) { | ||
| var pts; | ||
|
|
||
| if (doesCrossAntiMerdian(_pts) === null) { | ||
| if (doesCrossAntiMeridian(_pts) === null) { | ||
| pts = _pts; | ||
| } else { | ||
| pts = new Array(_pts.length); | ||
| for (m = 0; m < _pts.length; m++) { | ||
| // do not mutate calcdata[i][j].geojson !! | ||
| pts[m] = [_pts[m][0] < 0 ? _pts[m][0] + 360 : _pts[m][0], _pts[m][1]]; | ||
| pts[m] = [_pts[m][0] < 0 ? _pts[m][0] + ANTIMERIDIAN_LON_SHIFT : _pts[m][0], _pts[m][1]]; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -121,7 +136,7 @@ function feature2polygons(feature) { | |
| // Antarctica has a landmass that wraps around every longitudes which | ||
| // confuses the 'contains' methods. | ||
| appendPolygon = function (pts) { | ||
| var crossAntiMeridianIndex = doesCrossAntiMerdian(pts); | ||
| var crossAntiMeridianIndex = doesCrossAntiMeridian(pts); | ||
|
|
||
| // polygon that do not cross anti-meridian need no special handling | ||
| if (crossAntiMeridianIndex === null) { | ||
|
|
@@ -139,7 +154,7 @@ function feature2polygons(feature) { | |
|
|
||
| for (m = 0; m < pts.length; m++) { | ||
| if (m > crossAntiMeridianIndex) { | ||
| stitch[si++] = [pts[m][0] + 360, pts[m][1]]; | ||
| stitch[si++] = [pts[m][0] + ANTIMERIDIAN_LON_SHIFT, pts[m][1]]; | ||
| } else if (m === crossAntiMeridianIndex) { | ||
| stitch[si++] = pts[m]; | ||
| stitch[si++] = [pts[m][0], -90]; | ||
|
|
@@ -381,11 +396,76 @@ function computeBbox(d) { | |
| return turfBbox(d); | ||
| } | ||
|
|
||
| /** | ||
| * Pick a compact longitude range for `fitbounds`-style auto-framing when the | ||
| * data straddles the antimeridian (±180°). | ||
| * | ||
| * Longitude is cyclic, so the naive [min, max] range used by the autorange | ||
| * machinery can include a large empty span when points sit on both sides of | ||
| * ±180° (e.g. lon = [131.8855, -179] spans ~311° the long way round, when the | ||
| * compact view spans ~49° across the antimeridian). This finds the largest gap | ||
| * between consecutive longitudes and, when that gap is wider than the gap across | ||
| * the antimeridian, returns the complementary range so the map shows the dense | ||
| * cluster of points rather than the empty ocean between them. | ||
| * | ||
| * The returned upper bound may exceed 180°; downstream `makeRangeBox` (and | ||
| * MapLibre's `LngLatBounds`) handle ranges that cross the antimeridian without | ||
| * ambiguity. | ||
| * | ||
| * @param {Array} lons - longitude values (may contain non-finite entries) | ||
| * @return {Array|null} [lonStart, lonEnd] when an antimeridian-crossing range is | ||
| * more compact, otherwise null (caller keeps the autorange result). | ||
| */ | ||
| function getFitboundsLonRange(lons) { | ||
| const sorted = lons.filter(isFinite).sort((a, b) => a - b); | ||
| if (sorted.length < 2) return null; | ||
|
|
||
| const n = sorted.length; | ||
| const naiveSpan = sorted[n - 1] - sorted[0]; | ||
| // Data already wraps the whole globe; there is nothing to compact. | ||
| if (naiveSpan >= 360) return null; | ||
|
|
||
| // Widest gap between consecutive longitudes. | ||
| let maxGap = -Infinity; | ||
| let gapStart = -1; | ||
| for (let i = 0; i < n - 1; i++) { | ||
| const gap = sorted[i + 1] - sorted[i]; | ||
| if (gap > maxGap) { | ||
| maxGap = gap; | ||
| gapStart = i; | ||
| } | ||
| } | ||
|
|
||
| // Only worth wrapping when an interior gap is wider than the gap that the | ||
| // naive [min, max] range already leaves open across the antimeridian. | ||
| const antimeridianGap = 360 - naiveSpan; | ||
| if (maxGap <= antimeridianGap) return null; | ||
|
|
||
| return [sorted[gapStart + 1], sorted[gapStart] + ANTIMERIDIAN_LON_SHIFT]; | ||
| } | ||
|
|
||
| /** | ||
| * Return a monotonic version of a `[lon0, lon1]` longitude range so its | ||
| * midpoint and span can be computed as if longitude were a regular linear | ||
| * coordinate. When the range crosses the antimeridian (`lon0 > 0`, `lon1 < 0`) | ||
| * `lon1` is shifted by +360°; otherwise the input pair is returned unchanged. | ||
| * | ||
| * @param {[number, number]} lonRange - `[lon0, lon1]`, each in [-180, 180] | ||
| * @return {[number, number]} the unwrapped range | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand this description to give the possible range of the returned numbers? I can sort of figure it out by reading the above docstring but it would be clearer to state it explicitly here. |
||
| */ | ||
| function unwrapLonRange([lon0, lon1]) { | ||
| return [lon0, lon0 > 0 && lon1 < 0 ? lon1 + ANTIMERIDIAN_LON_SHIFT : lon1]; | ||
| } | ||
|
|
||
| module.exports = { | ||
| locationToFeature: locationToFeature, | ||
| feature2polygons: feature2polygons, | ||
| getTraceGeojson: getTraceGeojson, | ||
| extractTraceFeature: extractTraceFeature, | ||
| fetchTraceGeoData: fetchTraceGeoData, | ||
| computeBbox: computeBbox | ||
| locationToFeature, | ||
| feature2polygons, | ||
| getTraceGeojson, | ||
| extractTraceFeature, | ||
| fetchTraceGeoData, | ||
| computeBbox, | ||
| doesCrossAntiMeridian, | ||
| getFitboundsLonRange, | ||
| unwrapLonRange, | ||
| ANTIMERIDIAN_LON_SHIFT | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| const { getFitboundsLonRange, unwrapLonRange, doesCrossAntiMeridian } = require('../../../src/lib/geo_location_utils'); | ||
|
|
||
| describe('Test geo_location_utils.getFitboundsLonRange', () => { | ||
| it('returns the compact crossing range when point data straddles the antimeridian', () => { | ||
| expect(getFitboundsLonRange([131.8855, -179])).toEqual([131.8855, 181]); | ||
| expect(getFitboundsLonRange([170, 175, -170])).toEqual([170, 190]); | ||
| }); | ||
|
|
||
| it('keeps the naive range (null) when the data does not straddle the antimeridian', () => { | ||
| expect(getFitboundsLonRange([131.8855, 179])).toBe(null); | ||
| expect(getFitboundsLonRange([-10, 0, 20])).toBe(null); | ||
| }); | ||
|
|
||
| it('keeps the naive range (null) when the data spans the whole globe', () => { | ||
| const lons = []; | ||
| for (let lon = 0; lon <= 360; lon += 2.5) lons.push(lon); | ||
| expect(getFitboundsLonRange(lons)).toBe(null); | ||
| }); | ||
|
|
||
| it('returns null when fewer than two finite longitudes are available', () => { | ||
| expect(getFitboundsLonRange([10])).toBe(null); | ||
| expect(getFitboundsLonRange([NaN, 5])).toBe(null); | ||
| expect(getFitboundsLonRange([])).toBe(null); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Test geo_location_utils.unwrapLonRange', () => { | ||
| it('shifts lon1 by +360 when the range crosses the antimeridian', () => { | ||
| expect(unwrapLonRange([170, -170])).toEqual([170, 190]); | ||
| expect(unwrapLonRange([1, -1])).toEqual([1, 359]); | ||
| }); | ||
|
|
||
| it('leaves the pair unchanged when the range does not cross the antimeridian', () => { | ||
| expect(unwrapLonRange([-170, 170])).toEqual([-170, 170]); | ||
| expect(unwrapLonRange([-10, 10])).toEqual([-10, 10]); | ||
| expect(unwrapLonRange([-170, -10])).toEqual([-170, -10]); | ||
| expect(unwrapLonRange([10, 170])).toEqual([10, 170]); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Test geo_location_utils.doesCrossAntiMeridian', () => { | ||
| it('returns the index of the first positive-to-negative longitude transition', () => { | ||
| expect(doesCrossAntiMeridian([[170, 0], [179, 0], [-179, 0], [-170, 0]])).toBe(1); | ||
| expect(doesCrossAntiMeridian([[1, 0], [-1, 0]])).toBe(0); | ||
| }); | ||
|
|
||
| it('returns null when no segment crosses the antimeridian', () => { | ||
| expect(doesCrossAntiMeridian([[-179, 0], [-170, 0], [170, 0]])).toBe(null); | ||
| expect(doesCrossAntiMeridian([[10, 0], [20, 0], [30, 0]])).toBe(null); | ||
| expect(doesCrossAntiMeridian([])).toBe(null); | ||
| expect(doesCrossAntiMeridian([[10, 0]])).toBe(null); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Wording suggestion for clarity