Repository navigation
fix(core): scale cartesian z at the viewport latitude below zoom 12 - #10790
Conversation
For visgl#7064. Below zoom 12, getOffsetOrigin sets the geospatial origin to [0,0,0] for both 'lnglat' and 'cartesian' (since visgl#6117). Lnglat is corrected per vertex in the shader, but cartesian z (meters) is not, so it is scaled cos(latitude) too small, while from zoom 12 it uses the viewport latitude. Tiled TerrainLayer meshes are cartesian, so the terrain is too flat below zoom 12 (0.69 of its height at Mount St. Helens) and jumps at 12; TerrainExtension layers jump with it. In WEB_MERCATOR mode, keep the viewport center as geospatial origin for 'cartesian', so z is scaled as in WEB_MERCATOR_AUTO_OFFSET mode. Update the terrain-layer and terrain-extension-drape golden images: their terrain is at zoom 11.5 and is now drawn at its true height.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
coverage: 83.633% (+0.002%) from 83.631% — mfrnd:fix/7064-cartesian-z-scale into visgl:master |
…ow zoom 12 With a north-south position offset the viewport center lies at a different latitude than viewport.latitude. Unproject the center as in WEB_MERCATOR_AUTO_OFFSET mode, so the z scale of cartesian positions stays continuous at zoom 12 with an offset too (1.66% jump at 100 km before).
charlieforward9
left a comment
charlieforward9
left a comment
There was a problem hiding this comment.
Seems straightforward and smoothed.
Another contributor has been working on terrain and elevation compatabilities. Pinging to ensure the work is not duplicated across another PR.
|
A live before/after for this PR, measured from what each build draws: 
In the side view, the orange line is the terrain as each build draws it, over the elevation data in brown. It is picked with It is measured in Edge on a GPU. The demo's source is in my fork at scripts/globe-repro. I guided and reviewed Claude Code 5.5 xhigh in creating this comment and the demo. |
chrisgervang
left a comment
chrisgervang
left a comment
There was a problem hiding this comment.
Thanks for the explainer and the fix. The unit tests cover the regression well. I noticed there wasn't a terrain extension offset mode render test re-rendered.
Would it be possible to add one that illustrates the issue and fix clearly?
… fix The case was skipped because it no longer matched its 2023 golden image, which already showed the terrain flattened below zoom 12, with older fill colors. Regenerated with this fix: the terrain and the icons offset onto it rise to their height. With master's getOffsetOrigin the case matches 94.9%, below the 99% threshold. Skipped on WebGPU like the other TerrainExtension cases.
There was an offset mode case, 
On master the terrain is drawn at about 79% of its height at this latitude (cos 37.75°), so with the fix the hills, and the icons offset onto them, rise. The case is skipped on WebGPU like the other I guided and reviewed Claude Code 5.5 xhigh in creating this comment. |
ibgreen
left a comment
ibgreen
left a comment
There was a problem hiding this comment.
The main correction is sound: scaling preprojected elevation at the unprojected viewport center fixes the large low-zoom flattening and also handles a north-south camera position offset consistently across zoom 12. The latest CI checks, including WebGPU and coverage, are green.
I verified the projection path with an isolated WebGL GPU probe. There is a much smaller residual discontinuity from the second-order Y correction, described inline. I consider that a non-blocking follow-up to this substantial fix. Approving; I did not rerun the full suite locally.
| // Positions are already in common space and only z is in meters. Use the viewport center | ||
| // as geospatial origin so that z is scaled the same as in WEB_MERCATOR_AUTO_OFFSET mode | ||
| offsetMode = false; | ||
| geospatialOrigin = viewport.unprojectPosition([ |
There was a problem hiding this comment.
Non-blocking follow-up: the second-order correction still introduces a small zoom-12 height step. Moving the geospatial origin away from the equator makes commonUnitsPerWorldUnit2.z nonzero. Below zoom 12, project_offset_ multiplies it by absolute common-space Y; above zoom 12, Y is relative to the viewport center.
An isolated WebGL GPU probe against this commit, using project_position at the viewport center with 1,000 m elevation and latitude 80 degrees, returned:
- zoom 11.99: 1000.4046 m
- zoom 12: 1000.0000 m
At latitude 46 degrees the residual step is about 0.054 m per 1,000 m elevation. This is much smaller than the original flattening, but the new continuity tests inspect only the base uniform scale, so they miss the extra shader term.
Could we ensure the correction uses center-relative Y in the non-offset path (or omit it there), and add a GPU continuity test that checks the final projected Z on both sides of zoom 12, including northern and southern latitudes? This can be handled as a focused follow-up rather than blocking the main fix.
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
@chrisgervang @ibgreen Follow-up in #10853 |
5 participants
For #7064
Background
Below zoom 12,
getOffsetOriginuses[0, 0, 0]as geospatial origin for'cartesian'positions, so their z (meters) is scaled at the equator, cos(latitude) too small. TiledTerrainLayermeshes are cartesian: the terrain is too flat below zoom 12 (0.69 of its height at Mount St. Helens) and jumps at 12, andTerrainExtensionlayers jump with it.Change List
getOffsetOrigin: keep the viewport center as origin for'cartesian'terrain-layer,terrain-extension-drape,terrain-extension-offsetgolden images: terrain at true heightterrain-extension-offset: re-enable, skipped on WebGPU onlyLeft old, right new: markers at their DEM elevation sit on the terrain only with the fix.
Verified after merging master f509179, in headless Chromium with SwiftShader: lint, build, node, headless and render tests, no new failures against master. On d1b0ae4, picked elevation over DEM elevation went from 0.696 to 1.002 at zoom 11.5 to 11.99.
Note
Medium Risk
Touches core shader projection uniforms used by all geospatial layers; behavior change is scoped to cartesian + WEB_MERCATOR but affects terrain and draped extensions at sub-zoom-12 views.
Overview
Fixes incorrect vertical scaling for
COORDINATE_SYSTEM.CARTESIANlayers (e.g.TerrainLayermeshes) when the map uses Web Mercator below zoom 12. Previously, cartesian positions shared the lnglat path and used a[0,0,0]geospatial origin, so z in meters was scaled like the equator—terrain looked too flat and jumped when crossing zoom 12 intoWEB_MERCATOR_AUTO_OFFSET.In
getOffsetOriginforWEB_MERCATOR, cartesian is split from lnglat: the viewport center is unprojected to WGS84 and used asgeospatialOrigin, matching how z is scaled at the viewport latitude in auto-offset mode.Adds GLSL, uniform, and zoom-12 continuity unit tests, updates terrain golden images, and re-enables the
terrain-extension-offsetrender case (still skipped on WebGPU).Reviewed by Cursor Bugbot for commit 99faf9a. Bugbot is set up for automated code reviews on this repo. Configure here.