Repository navigation
Conversation
|
|
please set up a native github stack for these stacked PRs |
|
Nice, I did not know about the new stacked PR GitHub feature. I tried to apply it but it seems to not work across fork repos. Options that came to my mind:
You see any other ways? Please advise. |
ibgreen
left a comment
There was a problem hiding this comment.
Approving the adapter-level synchronization change on top of #10791, with the transition coverage concern recorded inline. The zoom 11–12 elevation ramp models MapLibre's camera convention and is appropriately confined to the MapLibre integration paths; it should not become a general GlobeView behavior or a requirement for other basemaps such as Tangram.
Review assisted by Codex. Local headless tests could not start because of sandbox server restrictions and incompatible installed dependency exports. The current GitHub test workflow reports action_required, so this approval does not establish a passing CI gate.
| map.setTerrain({source: 'dem'}); | ||
| await new Promise<void>(resolve => map.once('idle', () => resolve())); | ||
|
|
||
| for (const zoom of [10, 11, 12]) { |
There was a problem hiding this comment.
These real-map checks cover the endpoints but skip the fractional zooms where the new elevation blend actually runs. The zoom-11.5 unit case verifies the selected formula, rather than alignment with MapLibre's rendered camera. Since map.project() is not a valid reference inside this transition, please add rendered-pixel comparisons at representative zooms such as 11.25, 11.5 and 11.75, using an explicit alignment-error tolerance and a test marker whose pixels remain observable. The existing measurements in #10791 already show residual drift inside the band. This would protect the approximation against changes in MapLibre and make its practical accuracy clear.
There was a problem hiding this comment.
Added in 4508c90: a rendered-pixel test at zoom 11.25, 11.5 and 11.75 with MapLibre 5 and 6. It uses three 400 m squares. MapLibre drapes them over the 1,000 m terrain, so they stay visible, and deck.gl draws them at 1,000 m without depth testing. The test compares the centroids of their colors with BLEND_ALIGNMENT_TOLERANCE, 1 px. They are at most 0.6 px apart with the blend, and 10 to 21 px apart without it. The method alone accounts for up to 0.8 px at zoom 11 and 12, where both cameras match exactly, so a tighter tolerance would test rasterization rather than alignment.
The drift in my #10791 table came mostly from SwiftShader, which runs the browser tests. Its sin and cos (#10804) shift pitched globe rendering, MapLibre's and deck.gl's, by several pixels away from 0°/0°. In that table, MapLibre's drawn circle was already 2.8 px off at zoom 10, where both cameras match exactly. On a GPU (GTX 970), deck.gl and MapLibre draw the squares within 0.5 px of each other throughout the blend, at 47.3° N as well, the same as at zoom 11 and 12. That is why the test sits near 0°/0°, where SwiftShader is accurate.
Why the result is this close: MapLibre mixes the clip-space positions of its globe and Mercator cameras. For a point at the screen center, at terrain height h, the mix gives the same position as a single camera targeting h × (zoom − 11), for any h. With 3,000 m terrain, the squares are within 0.8 px in SwiftShader and 0.5 px on a GPU.
Like the rest of this file, the test runs MapLibre 5.0.0 and 6.0.0. So it catches a change in a later minor release only once the pinned versions move.
I guided and reviewed Claude Code 5.5 xhigh in creating this comment.
…om zoom 11 to 12 For visgl#7064, after the GlobeViewport position fix. MapLibre's globe camera targets sea level and its Web Mercator camera the terrain, and its globe projection blends to Web Mercator between zoom 11 and 12. Both overlays lifted the target to the center elevation at every zoom, so layers were drawn off the base map below zoom 12, by 17 to 36 px at zoom 10 and 11 in the MapLibre browser test. On the globe, scale the target elevation from 0 at zoom 11 to the full center elevation at 12: getMapLibreCameraElevation, and centerCameraOnTerrain for maplibre-gl in MapboxOverlay. syncMapLibreElevation compares against the same value, so it no longer re-syncs on every frame on the globe.
Compares MapLibreOverlay's projection with map.project() on the globe with 1,000 m terrain at zoom 10, 11 and 12, below and above MapLibre's globe to Mercator blend. The overlay is interleaved and the test waits for MapLibre to be idle: an overlaid Deck keeps its WebGL context after removal, and the tests share a page with a limit on live contexts.
|
I merged the first in this series in. Please resolve conflicts and then we'll merge |
map.project() does not follow MapLibre's rendering while its globe blends into Web Mercator from zoom 11 to 12. At zoom 11.25, 11.5 and 11.75, compare three 400 m squares that MapLibre drapes over 1,000 m terrain with the same squares drawn by MapLibreOverlay at 1,000 m, by the centroids of their colors, within 1 px. With MapLibre 5 and 6 they are at most 0.6 px apart, and 10 to 21 px without the blend. The squares are near longitude and latitude 0: SwiftShader's sin and cos are accurate there, while elsewhere they move globe shader output, of MapLibre and of deck.gl, by several pixels.
e656bba to
4508c90
Compare
For #7064. Part 2 of 2, follows #10791.
MapLibre's globe camera targets sea level and its Mercator camera the terrain, blending the two from zoom 11 to 12. Both overlays pass the full terrain elevation as
positionat every zoom, so deck.gl layers drift off the base map below zoom 12 (measured in #10791).Change List
getMapLibreCameraElevation: blend the terrain elevation from zoom 11 to 12syncMapLibreElevation: compare against the blended elevationcenterCameraOnTerrain: same blend for maplibre-gl inMapboxOverlayLeft master, right parts 1 and 2: the pink deck.gl points belong in MapLibre's cyan circles (live).
Ran the core viewport and view, MapLibre and Mapbox headless tests, lint and build.
Note
Medium Risk
Changes core overlay view-state and camera elevation sync for globe/terrain; wrong blending would misalign layers and picking, but scope is limited to MapLibre globe transition and is covered by new tests.
Overview
Fixes deck.gl overlay misalignment on MapLibre globe + terrain by matching MapLibre’s camera target elevation instead of always using full terrain height.
Adds
getMapLibreCameraElevation, which scales center elevation from 0 below zoom 11 to full terrain at zoom 12+ when projection isglobe. MapLibreOverlay uses this forviewState.positionandsyncMapLibreElevation; MapboxOverlay applies the same zoom 11–12 blend for maplibre-gl incenterCameraOnTerrain.Adds unit tests for blended
positionand WebGL tests that comparemap.project/ pixel centroids so deck layers stay aligned with the base map across the globe→Mercator transition.Reviewed by Cursor Bugbot for commit 4508c90. Bugbot is set up for automated code reviews on this repo. Configure here.