-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
add support for non-mercator projections #11124
Conversation
* Enable stencil clipping for line and fill layers * Use buffers from tile * Refactor tile bounds buffers * Rename things to not exclusively be RasterBounds * Create projections directory * More refactoring * Combine matrix calculations * Cleanup * Refactor projections to new folder * Begin debug work * Tile boundaries are working * Refactor indexbuffer and segmentvector to per painter * merge projectx and projecty functions * nits Co-authored-by: Ansis Brammanis <[email protected]>
* Add debug page * Add projection option * Wire up projection code with worker * Rename projections folder to projection * Refactor and update free camera * Add Projection type and fix center calculations * Fix bug with transform._center * Make Winkel projection noop for now * Update demo HTML and CSS * temp remove undistortion Co-authored-by: Ansis Brammanis <[email protected]>
#10753) * implement adaptive resampling of reprojected geometry * address feedback
* simplify projections tile transform * skip resampling for mercator
* separate tiles for background layers * additional lint & flow fixes * try fixing tests * try fixing render tests Co-authored-by: Ansis Brammanis <[email protected]>
* Pin to chrome version 91 * Pin chrome version for test-browser Co-authored-by: Arindam Bose <[email protected]>
* refactor raw projections, handle projection options * add unprojection to winkel tripel * fix flow * Pin chrome to version 91 (#10887) * Pin to chrome version 91 * Pin chrome version for test-browser * fix lint * remove to superfluous sin calls Co-authored-by: Arindam Bose <[email protected]>
…10980) * use adaptive resampling and earcut for non-Mercator tile bounds * fix unit test * use adaptive MARTINI mesh for non-Mercator raster tiles
* clamp unproject to mercator bounds in all projections * fix marker test * avoid wrapping center for non-Mercator projections * extend alt projections clamping to full lat range
* fix zoom, bearing and skew for projections * refactor adjustments * lint * add comments
* fix circle & heatmap on alternate projections (blunder) * fix unit test
and: - fix fill-extrusions - remove global projection variable to allow multiple maps on one page - avoid recalculating tileTransform
* adaptive bbox for projections, refactor resampling * better precision for adaptive bounds * remove leftover * fix zoom/shear adjustments near poles * optimize tile transform * fix lint * attempt to fix tests * simplify, clarify and consolidate constraining logic * minor renames in transform * safer clamping for zoom adjustments
Co-authored-by: Ansis Brammanis <[email protected]>
* rudimentary support for custom maxBounds in alternate projections * fix flow
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.
A first review pass! I'm planning on doing more testing, but so far:
- My biggest concern is around combination of non-compatible features (projection/terrain, projection/fog...) and finding mechanisms at disabling them.
- Also, high pitch seem to trigger a few issues in the tile cover in non-mercator projections. I tend to think that high pitch is only beneficial for 3d (terrain/fog/fill-extrusion) and has relatively low value for 2d projections under low zoom. Have we considered enforcing a max pitch per projection?
- There seem to be a leak for tile buffer memory, we should find a place for release (e.g. buffers created from
_makeTileBoundsBuffers
)
|
||
project(lng: number, lat: number) { | ||
const p1 = this.parallels[0] / 180 * Math.PI; | ||
const p2 = this.parallels[1] / 180 * Math.PI; |
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.
Not a blocker for this review. But we should capture these small tweaks to address separately: this.parallels
is always used in radians in project()
and unproject()
. We could move that conversion once when assigning this value to the 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.
53°07'25.3"N, 48°23'45.6"E
} | ||
|
||
function getInterpolationT(transform: Transform) { | ||
const range = transform.projection.range; |
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.
Should we use getInterpolationT
to fade out sky layers similar to the shear adjustments? I haven't tried projections with this layer specific to 3d layer yet but it should be a fairly small change based on the current projection here.
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.
Sky seems to work about as well with other projections as it does with mercator. It's kind of weird for both at the lowest zoom levels where the map doesn't reach the horizon (and where it's weird seeing a non-curved earth when pitched). Fading out might be a good thing to do but I think it's a separate decision.
I think we should also use a different range to fade it out since I think we want to do it consistently for all projections
… buffers when tiles are unloaded (#11128)
* enable lod tile loading for projections to significantly reduce the number of tiles at low zoom levels * use Math.hypot(...) Co-authored-by: Vladimir Agafonkin <[email protected]> * add comments Co-authored-by: Vladimir Agafonkin <[email protected]>
Co-authored-by: Karim Naaji <[email protected]>
Center projections vertically in 0 to 1 range. This shouldn't matter but there is some constraining behavior that is currently affected by this.
Looks like this branch makes loading mercator maps (with no visual change) 1% slower. Most of this seems to come from increased tile parsing time. My guess would be the extra complexity around |
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.
Code looks great and LGTM, nice work!
@ansis I kept notes notes while reviewing about issues I encountered that we should address, I'm not sure about when based on the deadline, but I suggest triaging these and making sure to order and prioritize them if the impact is high enough. Feel free to reevaluate whether merging should happen now if any of these is critical for the immediate usability of the feature at launch, or defer the launch of a few of these projections (Albers seem to have a few usability issues with gestures).
Tile bounds buffer may be created unnecessarily
A potential optimization: we should be able to create tile bound buffers only for tiles that are in the 'loaded' state, on-demand for visible tiles. Creating the tile bounds buffer can be a non-trivial amount of work. These buffers are created in the tile constructor, but tiles may never reach the 'loaded' state, resulting in these buffers being created and destroyed wastefully.
Warning after reaching max allowed extent
Tile retention issues may lead to flickering
Some tiles may flicker when zooming in or out, could be that the source cache may not retain those tiles with projections.
tile-retention-projection.mov
Custom geojson does not respect world bounds
Might be directly related to #11130?
Gesture issues with Albers projection
Gestures seem to have a few usability issue, and the map center may wrap when panning on the map at world boundaries when flinging or panning under Albers.
gesture-fling.mov
Currently under Albers projection the user isn't able to see a large part of the South America, blocked around here:
Should we leave Albers in the code, but remove it from this launch?
Low unit and render test coverage
Considering the high chance of overlap with the globe view, we should be particularly careful about not breaking existing behaviors of projection and introducing regressions. I recommend to increase the coverage before any other work for globe view happens.
Resolved offline: Albers is a projection that works well under one parallel, but a risk might be that devs may not use max bounds and let users pan and zoom around freely, eventually accessing the issues mentioned once panning too far from the original center and parallel. If we were to enforce the bounds, the nice point is that we're sure the user can't see those issues, but that adds an implicit behavior. Since conical projections can use different regions by changing the parallels and center, the max bounds isn't well defined: we would have to override the default bounds per center and parallel. It's best to suggest it by having an explicit mention of map bounds in conical projections (5af4076). |
Seem to be fixed after d0afb1b. No more repro of this particular issue. |
* main: Add touch pan blocker to gesture handling for touch devices (#11116) Address accessibility issues (#11064) add support for non-mercator projections (#11124) Image fallback expressions within paint properties (#11049) Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072) Add globe view support to heatmap shaders (#11120) Exclude flaky test (#11118) consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113) Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114) render-test-flakiness:clear worker storage (#11111) upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110) Added v1.13.2 changelog entry (#11108) One weird JSON.parse() trick (#11098) Fixed doc usage of map.getCenter (#11093) s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795) Update link to transpiling guide (#11096) Cherry pick 2.5.1 changelog (#11099) Fix an iOS15 issue where Safari tab bar interrupts panning (#11084) Fix conditional check for isFullscreen to accommodate Safari (#11086) Render tests for #11041 (#11070)
This adds support for rendering maps with non-mercator projections using existing styles and tilesets. The mercator-tiled tilesets are reprojected in the browser. Vector reprojection, instead of raster reprojection, is used for vector data to keep rendering perfectly sharp.
As you zoom in, the projection the map is gradually unskewed so that at high zoom levels the map is rendered the same as in mercator. This gives you the benefits of projections at lower zoom levels without the distortion at high zoom levels.
projections.mp4
#3184
Projections cannot be used together with 3D terrain yet.
Projections specified in the constructor take precedence over projections set in the style.
Supported projections
Albers
Supports parameters
center
andparallels
.Equal Earth
Equirectangular
Lambert conformal conic
Supports parameters
center
andparallels
.Natural Earth
Winkel tripel
Launch Checklist
mapbox-gl-js
changelog:<changelog>Add support for a variety of new map projections beyond the standard Web Mercator. Alternate projections can be used together with any existing styles and sources, and adopt a novel design that minimizes distortion across the zoom range and keeps vector rendering sharp. For the first time, this makes using alternate projections with full-featured interactive maps a seamless experience.</changelog>