-
Notifications
You must be signed in to change notification settings - Fork 16
fix: handle antimeridian-crossing tiles in Web Mercator reprojection #374
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -8,6 +8,11 @@ const WGS84_ELLIPSOID_A = 6378137; | |
| // Beyond this, the Mercator projection is undefined. | ||
| const MAX_WEB_MERCATOR_LAT = 85.05112877980659; | ||
|
|
||
| const WEB_MERCATOR_METER_CIRCUMFERENCE = 2 * Math.PI * WGS84_ELLIPSOID_A; | ||
| const HALF_CIRCUMFERENCE = WEB_MERCATOR_METER_CIRCUMFERENCE / 2; | ||
|
|
||
| type ProjectionFn = (x: number, y: number) => [number, number]; | ||
|
Member
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. Edit you can use the copy in |
||
|
|
||
| /** | ||
| * Convert a WGS84 longitude/latitude to EPSG:3857 meters analytically. | ||
| * Valid for latitudes in [-MAX_WEB_MERCATOR_LAT, MAX_WEB_MERCATOR_LAT]. | ||
|
|
@@ -19,6 +24,37 @@ function wgs84To3857(lon: number, lat: number): [number, number] { | |
| return [x, y]; | ||
| } | ||
|
|
||
| /** | ||
| * If a tile's EPSG:3857 corner x-values span more than half the globe, wrap | ||
| * `forwardTo3857` / `inverseFrom3857` so the coordinate space is continuous. | ||
| * | ||
| * Returns the original functions unchanged when no wrapping is needed. | ||
| */ | ||
|
Comment on lines
+27
to
+32
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. Would this heuristic trigger on global rasters? Since we have the corners of the raster in WGS84, can't we instead just check to see whether the left longitude is greater than the right longitude?
Member
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. Thanks @gadomski, this is a good point. If we have an image that's 512px wide, 256px tall that covers the entire EPSG:4326 coordinate space, then it would trigger this function, even though it doesn't wrap the antimeridian. (We should add unit test cases that such an input image doesn't trigger antimeridian handling) |
||
| export function wrapAntimeridianProjections( | ||
| cornerXs3857: number[], | ||
| forwardTo3857: ProjectionFn, | ||
| inverseFrom3857: ProjectionFn, | ||
| ): { forwardTo3857: ProjectionFn; inverseFrom3857: ProjectionFn } { | ||
| const xMin = Math.min(...cornerXs3857); | ||
| const xMax = Math.max(...cornerXs3857); | ||
|
|
||
| if (xMax - xMin <= HALF_CIRCUMFERENCE) { | ||
| return { forwardTo3857, inverseFrom3857 }; | ||
| } | ||
|
|
||
| return { | ||
| forwardTo3857: (x: number, y: number): [number, number] => { | ||
| const [px, py] = forwardTo3857(x, y); | ||
| return [px < 0 ? px + WEB_MERCATOR_METER_CIRCUMFERENCE : px, py]; | ||
| }, | ||
| inverseFrom3857: (x: number, y: number): [number, number] => { | ||
| const unwrapped = | ||
| x > HALF_CIRCUMFERENCE ? x - WEB_MERCATOR_METER_CIRCUMFERENCE : x; | ||
| return inverseFrom3857(unwrapped, y); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Wrap a proj4 forward projection to EPSG:3857 so that it never returns NaN. | ||
| * | ||
|
|
||
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.
wrapAntimeridianProjectionscreates new closure functions forforwardReproject/inverseReprojecton every_renderSubLayerscall when wrapping is needed.RasterLayer.updateStatetreats changes in these function identities as a reason to regenerate the adaptive mesh, so antimeridian tiles can end up re-meshing every render. Consider memoizing/caching the wrapped projection functions (e.g., once per COGLayer instance or via a WeakMap keyed by the original fns) so their identities remain stable across renders for the same source projection.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.
This does seem like a valid critique. I'm not sure at a glance how it could be fixed.