Skip to content

Continue 3d terrain work - #4389

Draft
acalcutt wants to merge 304 commits into
maplibre:mainfrom
WifiDB:terrain-3d-color-relief
Draft

Continue 3d terrain work#4389
acalcutt wants to merge 304 commits into
maplibre:mainfrom
WifiDB:terrain-3d-color-relief

Conversation

@acalcutt

@acalcutt acalcutt commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Builds on top of the current feature/terrain-3d , merges the main branch, and continues adding additional updates to try and make it complete. The PR uses internal DEM decoding we already use for hillshade/terrain and makes features terrain aware. So far most of the testing has been in android/opengl, but i have tried to add the other renderers completely (though they are still very untested). Most of the changes are done by Claude Fable/Opus/Sonnet through comparison with gl-js and nitpicking the results.

This is the AI description of the change made.

Draping

  • Draped layers (background, fill, line, raster, hillshade, color-relief) render
    into per-tile targets draped over the terrain mesh, on all four backends
    (OpenGL, Metal, Vulkan, WebGPU). Targets carry depth and stencil; every
    overlapping source tile is drawn and the tile clipping masks resolve
    parent/child overlap, so a parent tile stands in for unloaded children instead
    of the target rendering black.
  • Drape target textures are cached and re-rendered only when their content
    changes. Prepare passes (e.g. hillshade) run before the drapes that sample them.

Elevated layers

  • Symbol, circle and fill-extrusion sample the DEM in their vertex shaders and are
    displaced by the terrain, on all four backends.
  • Symbols behind terrain are occluded through a depth pass, all backends.

Tile cover

  • Covering tiles are tested against the frustum using their DEM min/max
    elevation, so terrain leaning toward the camera is requested. After
    expandToDeepestCover the mesh is frustum-culled, so a sparse DEM's large
    low-zoom tiles aren't meshed across their off-screen area.

Other

  • DEM decoding (Terrain-RGB and Terrarium), CPU elevation queries with ancestor
    fallback, terrain root-property parsing (source, exaggeration).
  • terrain_depth shader header included where Metal/Vulkan/WebGPU register it.

Remaining work and known issues are in TERRAIN.md (mesh skirts, drape target
size, camera-terrain anchoring, a zoom-0 exaggeration bug).

Diff size: against main this is ~255 files. Against feature/terrain-3d it
shows ~850 because it also brings that branch up to main — the extra files and
deletions are the upstream catch-up, not terrain changes.

@github-actions github-actions Bot added metal build Related to build, configuration or CI/CD iOS core Changes that affect the C++ core of MapLibre Native OpenGL Issues related to the OpenGL renderer backend Vulkan WebGPU labels Jul 8, 2026
@acalcutt
acalcutt changed the base branch from main to feature/terrain-3d July 8, 2026 13:44
@acalcutt
acalcutt changed the base branch from feature/terrain-3d to main July 8, 2026 13:49
acalcutt and others added 8 commits July 8, 2026 15:15
…ines

getTileMatrix was called with aligned=true (copied from hillshade), which
uses the pixel-snapped alignedProjMatrix. matrixForTile(), which this code
used before the terrain-aware matrix change, uses the unaligned projMatrix.
The sub-pixel sampling difference made the color-relief low-zoom and
combination render tests fail with small per-pixel drift on Metal.
Color-relief drawables have depth disabled, so with aligned=false the
non-terrain path of getTileMatrix is identical to matrixForTile().

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
decode_elevation()/get_elevation() in the shared Metal shader prelude
hardcoded the Mapbox Terrain-RGB formula and had no callers; the terrain
vertex shader decodes via the encoding-aware unpack vector from
TerrainEvaluatedPropsUBO instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ltiplier

The tweaker multiplied the styled exaggeration by 3.0 as leftover debug
tuning. The style spec defines exaggeration with default 1.0 rendering
true-scale elevation, so use the styled value directly. Also drops the
one-shot log that existed only to report the multiplied value.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 3D terrain feature only had GL and Metal shaders, so terrain rendered
nothing on the Vulkan backend (the Android default) and WebGPU. Port the
terrain vertex/fragment shaders to Vulkan GLSL and WGSL following the
hillshade/color-relief patterns, including the encoding-aware DEM unpack
vector decode, and register TerrainShader with both backends.

The Vulkan drawable image descriptor set layout is widened to be visible
from the vertex stage as well, since the terrain vertex shader samples
the DEM texture for mesh displacement.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The terrain branch disabled the perspective gamma-scale computation in the
GL line vertex shader with '#if 1', which also broke line antialiasing on
pitched maps when terrain is not active, and left the other backends
computing the perspective ratio even for draped lines.

Repurpose the LineDrawableUBO padding as a to_terrain_rtt flag, set by the
line layer tweaker when getTileMatrix targets a terrain render-to-texture
tile (new optional out-parameter). The GL, Metal, Vulkan, and WebGPU line
shaders now use gamma scale 1.0 only when draped - the perspective scaling
happens when the textured terrain mesh is projected - and the classic
perspective ratio otherwise. The UBO layout is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +1.2%  +193Ki  +1.2%  +192Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-4389-compared-to-main.txt

acalcutt and others added 7 commits July 8, 2026 19:49
vk::ShaderStageFlagBits | vk::ShaderStageFlagBits is ambiguous between
vulkan.hpp's operator and the generic enum operator in
mbgl/util/bitmask_operations.hpp. Start the chain with
vk::ShaderStageFlags() like the rest of this file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The terrain branch added a second createRenderTarget pure virtual to
gfx::Context and every backend just to pass a clear color to the shared
RenderTarget class. Replace it with a non-virtual
RenderTarget::setClearColor, restoring the single-overload interface.
The behavior is unchanged: terrain render-to-texture tiles are still
cleared to the map background color, which native needs because the
terrain mesh renders unblended (maplibre-gl-js clears to transparent
and blends instead).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RenderTerrain::getElevation was a TODO returning 0. Look up the DEM tile
matching the requested tile or its closest available ancestor, map the
tile-local coordinate into it, and bilinearly interpolate the decoded DEM
texels, mirroring maplibre-gl-js Terrain.getDEMElevation. This provides
the basis for camera-terrain collision and coordinate queries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Port render-tests/terrain/default and terrain/pitched-world from
maplibre-gl-js. All required fixtures are already present in
cache-style.db (the zoom 12 Innsbruck terrain quad, the 0-0-0 vector
tile, and the demotiles number rasters), so the styles only needed their
tile URLs pointed at the cached templates. The expected images are the
maplibre-gl-js renders, so both tests are in the platform-all ignore
list until native-rendered baselines are captured.

Also replace TERRAIN.md's stale limitations section with the current
status and a phased production plan (non-draped layer elevation, symbol
occlusion, seams/skirts) referencing the gl-js implementation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When 3D terrain is enabled, circles rendered at z=0 float over valleys and
sink into peaks. Following the maplibre-gl-js get_elevation() approach,
displace circle centers by the terrain elevation in the vertex shader:

- RenderTerrain caches per-tile DEM textures and exposes getTerrainData(),
  returning the covering DEM tile's texture and a scale/offset mapping from
  tile-local coordinates (with ancestor tile fallback), plus a 1x1
  placeholder texture so the DEM sampler always has a valid binding
- PaintParameters carries the RenderTerrain so layer tweakers can bind
  elevation data
- CircleDrawableUBO gains dem_coords/dem_unpack/dem_dim/dem_exaggeration
  and a runtime dem_enabled flag; the circle tweaker fills them and binds
  the DEM texture per drawable
- The GL, Metal, Vulkan, and WebGPU circle vertex shaders decode the DEM
  with manual bilinear interpolation on pixel centers (1px backfilled
  border) and add the elevation to every projection, matching the gl-js
  circle shader

Symbol and fill-extrusion layers are next, following the same pattern.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace traditional reverse iterator loops using rbegin()/rend() with
C++20 std::ranges::reverse_view to comply with clang-tidy's
modernize-loop-convert warning.

Changes:
- include/mbgl/renderer/render_target.hpp: Convert visitLayerGroupsReversed()
- src/mbgl/renderer/render_orchestrator.hpp: Convert visitLayerGroupsReversed()
- Add #include <ranges> to both files

This fixes the modernize-loop-convert error in the Linux CI build.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +1.1% +2.03Mi  +1.4%  +495Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-4389-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +56% +65.0Mi  +479% +28.6Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-4389-compared-to-legacy.txt

acalcutt and others added 4 commits July 9, 2026 05:32
The Terrain Phase 1 circle change added an always-declared DEM texture
binding (@group(1)) to the circle shader. When a style has no terrain,
CircleLayerTweaker binds no DEM texture, so on WebGPU the backend skipped
creating bind group 1 entirely and then issued the draw anyway. wgpu's
strict validation rejects this ("CircleShader expects a BindGroup to be
set at index 1") and panics (exit 134); Dawn renders wrong pixels
(TileLOD render-test diffs). GL/Metal/Vulkan tolerate the unbound slot --
Vulkan specifically binds Context::getDummyTexture() for it.

Mirror that precedent: add Context::getDummyTexture() (a lazily created
1x1 texture) to the WebGPU backend and, in Drawable::draw's bind-group
assembly, fall back to it for any Sampler/Texture binding the drawable
has not set, instead of dropping the whole bind group. The dummy is never
sampled (the shader guards on dem_enabled), so its contents don't matter.
This also covers the upcoming symbol/fill-extrusion terrain work.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follows maplibre-gl-js: raise the whole extrusion by the terrain elevation
sampled once at the polygon centroid (not per-vertex, which would shear a
building across a slope), plus gl-js's "basement" offset that drops ground-
level floors 10m so buildings don't hang off a slope. Elevation is sampled
(bilinear, 1px border) from the covering DEM tile in the vertex shader via
dem_* fields added to FillExtrusionDrawableUBO; the tweaker binds the DEM
texture and fills dem_* from RenderTerrain::getTerrainData.

Centroid plumbing (mirrors gl-js a_centroid):
- new attributes::centroid; added to the non-instanced FillExtrusionLayoutVertex
- the bucket computes each polygon's outer-ring centroid and writes it to every
  vertex of that polygon
- idFillExtrusionCentroidVertexAttribute bound by the render layer; a_centroid
  in the GL shader + shader_info

Scope:
- OpenGL non-pattern fill-extrusion elevates end-to-end.
- dem_* added to FillExtrusionDrawableUBO on ALL backends so the consolidated
  per-drawable SSBO stride matches; Metal/Vulkan/WebGPU compile and render
  unchanged (no elevation yet). The centroid attribute is on the non-instanced
  layout vertex (GL/WebGPU); only the GL shader reads it so far.
- Follow-ups: Metal/Vulkan/WebGPU non-pattern elevation (incl. Metal/Vulkan
  instanced side geometry) and pattern variants; the UBO struct is already grown.

Circle intentionally untouched. TERRAIN.md updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
maplibre-gl-js keeps a single get_elevation() in _prelude.vertex.glsl that
every shader calls; do the same here. Native carries the DEM data per-drawable
(not as global uniforms like gl-js), so the shared function takes the DEM
sampler and dem_* as parameters instead of reading globals — no per-shader
uniform bloat, and GL strips the function from shaders that never call it.

The GL prelude (gl/prelude.hpp) is a separate source prepended at shader-compile
time, so the function lives in exactly one place; circle and fill-extrusion drop
their identical inline copies and call the shared get_elevation(pos, u_dem,
u_dem_coords, u_dem_unpack, u_dem_dim, u_dem_exaggeration, u_dem_enabled).

OpenGL only for now; the Metal/Vulkan/WebGPU per-shader-family copies can move to
their common preludes the same way when those backends' elevation lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The experimental 3D-terrain draping path renders layers into per-tile render
targets, and some draped/terrain tiles reach stencilModeForClipping() without a
registered clip mask for the current pass. Previously that hit assert(it != end)
in debug (Dawn/wgpu render tests aborted) and, in release, silently used ref=0
(an always-pass mask) which crashed the render-test runner on Vulkan/WebGPU and
took down every subsequent render test in the suite.

Return StencilMode::disabled() (no clipping) for the missing-mask case instead of
asserting/crashing. The terrain/default render test is experimental and its output
is ignored, so degrading to "no clip" is acceptable and stops the crash from
failing unrelated backends' CI.

Note: this stops the crash; the underlying gap (draping does not register clip
masks for every tile it draws) still warrants a proper fix in the terrain draping
code once it can be debugged with a local build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
acalcutt and others added 10 commits August 2, 2026 01:31
Both styles omitted `encoding`, so their raster-dem source decoded with the
default Terrain-RGB unpack. Decoding the fixtures proves the tiles these tests
actually cover are terrarium: the styles center on [-113.23, 35.94] (Grand
Canyon) and the tiles/terrain/ entries for that region (10-189-401/402,
10-190-401/402, 11-379-*, 11-380-*) all classify as terrarium, while the five
Terrain-RGB tiles in the same directory are at [35.2, 31.0] and belong to a
different test. Read as Terrain-RGB they decode to ~870 km and put the mesh
outside the frustum.

This does NOT make the tests pass yet: they still render blank because the DEM
tile pyramid they request is incomplete. Missing from both disk and
cache-style.db: the ancestor chain 6-11-25 -> 7-23-50 -> 8-47-100 ->
9-94-200/201, 9-95-200/201, and the z10 edges 10-189-400, 10-189-403,
10-190-403, 10-191-401. The only cached low-zoom tiles (5-5-12, 7-76-52) are
for unrelated regions. Supplying those ancestors - or converging the tile
cover with gl-js so they are not requested - is the remaining step.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
computeMeshCover() is not stable within a frame. It derives its cover tile size
from `demDim`, which is 0 until the first DEM tile decodes and only then takes
the source's real value:

    terrainCoverTileSize = demDim > 0 ? demDim : 512

Renderer::Impl::render calls it once to build the frame's drape-target pool,
then RenderTerrain::update calls it again to build the mesh. For a 256px DEM
the first call falls back to 512 and the second uses 256, so the two produce
different tile sets: every getRenderTarget() lookup in update() missed and no
terrain drawable was created at all. Sources whose DEM really is 512 were
unaffected, which is why only some styles were blank.

Measured on terrain/skirts-auto before the fix:
    impl:   drapeTargets=4
    update: meshTiles=8 noRenderTarget=8 created=0 totalDrawables=0
and after:
    update: meshTiles=4 noRenderTarget=0 created=4 totalDrawables=4

Renderer::Impl now hands the cover it computed to RenderTerrain
(setFrameMeshCover) and update() meshes exactly that set, so the mesh and the
drape-target pool can never disagree. terrain/skirts-auto and skirts-none now
render their terrain (blue skirt band + red line) instead of an empty frame;
terrain/fill-extrusion still passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
skirts-auto/skirts-none pointed at tiles/terrain/, a directory that holds two
unrelated regions in two different encodings (11 terrarium tiles over the Grand
Canyon, 5 terrain-RGB over the Dead Sea). One raster-dem source applies a
single unpack vector, so the terrarium tiles decoded as terrain-RGB to ~870 km.
Their DEM pyramid was also incomplete - the z6-z10 ancestors were absent, so
the terrain rendered with gaps.

Both tests now use a new tiles/jaxa/ set (AW3D30/JAXA, terrain-RGB, the same
source as the previously bare tiles/*.terrain.png - verified pixel-identical
for 2-1-0), at tileSize 512 with no explicit `encoding` since terrain-RGB is
the raster-dem default. This supersedes 30d7e3b, which set
`encoding: terrarium` for the old tiles.

The required tile set was derived empirically rather than guessed: repointing
the style at an empty path made every tile miss, enumerating the exact 13 tiles
the cover wants. Note tileSize 512 moves the ideal zoom from z10 to z9 - the
one-level difference from the 256px cover.

Tiles were fetched from a global JAXA terrain-RGB tileset (lossless VP8L WebP)
and re-encoded to lossless PNG, so elevations are exact. The 11 pre-existing
bare tiles/*.terrain.png are moved into tiles/jaxa/ alongside them (same
source); tests read from cache-style.db, so the on-disk location is provenance
only.

Both tests now render a complete, gap-free terrain surface with zero cache
misses (previously an empty frame). They still fail their device-captured
baselines, which need regenerating against this data.

README documents the encoding hazard, the cache-DB indirection, and the
remaining 256/512 tile-size mix in jaxa/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
873da6a consolidated all bare tiles/*.terrain.png into tiles/jaxa/ on the
assumption they shared the JAXA source. Verifying each against that source
shows that holds for seven of them - 0-0-0, 1-0-0, 1-1-0, 2-1-0, 2-1-1, 2-2-0
and 2-2-1 are pixel-identical - but not for the four z12 Grand Canyon tiles:
they are 256px where JAXA serves 512px, and do not match even as a
nearest-neighbour subsample. Placing them under a JAXA-credited path
attributed them to a source they did not come from.

They move back to tiles/ and the README marks their provenance unverified,
with a note not to re-fetch them from JAXA: the resolution differs, and tile
size drives the cover zoom (see 73f162b).

tiles/jaxa/ now contains only tiles verified pixel-identical to the JAXA
terrain-RGB source, and is uniform at 512px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The attribution table had grown verbose with verification notes. Trim it to
what the table is for - what each file is and where it came from - crediting
the tiles of unknown origin as such rather than explaining at length what they
are not. The technical guidance (encoding hazard, cache-DB indirection, tile
size) stays in the notes section below the table.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The folder had 18 files but only 13 cache rows: the five tiles moved into
jaxa/ in 873da6a (1-1-0, 2-1-0, 2-1-1, 2-2-1) kept their original bare-template
keys, and 2-2-0 was not cached at all. A style pointing at
local://tiles/jaxa/{z}-{x}-{y}.terrain.png could therefore only load part of
the folder.

Insert all five under the jaxa/ template so the folder is self-consistent.
The bare-template rows stay: 55 styles still reference that path. Four of the
new rows duplicate a blob also stored under the bare key, which is the cost of
two live templates sharing tiles.

No behaviour change - the color-relief tests and the terrain suite give
identical results before and after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The note claimed jaxa/ mixes 256px and 512px tiles, citing the 12-758-*/
12-759-* tiles - but those moved back out to tiles/ in 33fc032 when their
provenance turned out not to be JAXA. jaxa/ is uniformly 512px.

Reword it as the general guidance it was meant to be: keep a folder uniform in
tile size as well as encoding, since style tileSize shifts the cover zoom.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Earlier commits in this branch grew a "Notes for adding DEM fixtures" section
onto this file (encoding hazard, cache-DB indirection, tile size). That is
working guidance, not attribution, and this README is only meant to record what
each fixture is and where it came from. The guidance already lives in
TERRAIN.md's render-test section, so drop it here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s/jaxa

Adds eight 512px terrain-RGB tiles from the same AW3D30/JAXA source as the
rest of tiles/jaxa/ (lossless VP8L WebP re-encoded to lossless PNG, so
elevations are exact): 1-0-1, 1-1-1, 10-189-402, 11-379-804 and the four z12
12-758-*/12-759-* tiles.

Every file in tiles/jaxa/ is inserted into cache-style.db under the jaxa/ URL
template, so a style pointing at that folder can load all of it - the render
tests read tiles from the cache DB, not from these files.

No test behaviour changes here; this only makes the tiles available.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Of the color-relief tests, only low-zoom uses 512px tiles: it renders at zoom 1
over [0, 45] with tileSize 512, covering the z0-z2 tiles that are verified JAXA
data. The other four are close-ups at zoom 11 over the Grand Canyon with
tileSize 256 and depend on the 256px tiles their maplibre-gl-js baselines were
rendered from, so they stay on the original path.

low-zoom passes unchanged against its existing baseline - the tiles it resolves
are the same data under a new name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@acalcutt
acalcutt force-pushed the terrain-3d-color-relief branch from 7103ad6 to 71c6d3b Compare August 2, 2026 07:17
acalcutt and others added 19 commits August 2, 2026 03:43
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Metal defaults a render-pass colour attachment to MTL::StoreActionDontCare.
mtl::OffscreenTextureResource::bind() set the attachment's texture but never
its store action, so on a tile-based GPU the rendered colour was never written
back and the texture's contents were undefined after the pass. Offscreen
textures exist precisely to be sampled afterwards, so they must store.

This is why the terrain drape looked broken on macOS/Metal CI while OpenGL
rendered the same scenes correctly: large areas of the terrain surface sampled
black. Diagnostics added to the shared render path showed every stage
behaving identically on both backends - for terrain/occlusion-debug Metal
created all 6 terrain drawables with noRenderTarget=0, and every drape target
rendered (no cache skips, no deferrals) with the same per-target drawable
counts as GL (1, 6, 2, 1, 10, 4) - and no "No drape texture" warnings, so the
textures were bound. Content correct, texture bound, no errors, still black:
the contents were being discarded at the end of the pass.

The depth attachment keeps StoreActionDontCare deliberately - it is not read
back. RenderPass copies the descriptor to override loadAction/clearColor,
which preserves the store action set here.

Not verifiable locally (Metal needs macOS); CI should show the black bands
gone from terrain/occlusion-debug and terrain/fill-extrusion.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reflow only, from running the clang-format hook; no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Metal terrain surface samples black over large areas while every stage of
the shared path reports success: terrain drawables all created
(noRenderTarget=0), every drape target renders (no cache skips, no deferrals)
with the same per-target drawable counts as OpenGL, no "No drape texture"
warnings, and no Metal errors. Setting an explicit colour store action on the
offscreen attachment (5295825) changed nothing.

That combination is what you would see if a terrain drawable sampled a
different texture from the one its drape target rendered into - the target
renders correctly, the drawable holds a handle to something else. Nothing
currently logged would reveal that, since both halves report success
independently.

Log the texture pointer at both ends, keyed by tile:
  TERRAIN tile <id> bindsTexture=<ptr>     (what the drawable will sample)
  DRAPE   <id> ... renderedInto=<ptr>      (what the target rendered into)

On OpenGL all six tiles of terrain/occlusion-debug pair up exactly, so any
mismatch in the Metal log localises the bug to the texture pool handing out a
stale or recycled target.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Narrowing the Metal terrain draping bug. Everything checked so far is correct
on Metal: drawables all created, every drape target renders with GL-identical
content, the drape texture pointers match at both the bind and render sites,
present() commits and waits, the sampler is ClampToEdge and the UVs are in
range. Yet the terrain samples black over large areas.

Metal is the only backend that ignores the render-pass descriptor's clearDepth
and clearStencil: gl/render_pass.cpp references them 3 times, vulkan 4 times,
mtl 0. RenderTarget requests clearStencil 0 for every drape target, and
mtl::OffscreenTextureResource::bind() attaches the stencil texture without a
load action - so Metal defaults it to LoadActionDontCare and the pass runs
against undefined stencil. The drape rebuilds tile clipping masks in that
buffer, so garbage stencil would discard fragments and leave the target at its
clear colour, which matches every observation.

Two logs to confirm at runtime rather than guess again:
  MTLPASS <name> clearDepthReq= clearStencilReq= stencilAttachment= stencilLoadAction=
  DRAPE <tile> drawCalls=<n>

drawCalls counts draws ISSUED - fragments killed by a stencil test still count
- so a black target with non-zero drawCalls means submitted-and-discarded
rather than never-submitted. On OpenGL the counts match enabledDrawables
exactly (1, 6, 2, 1, 10, 4 for terrain/occlusion-debug).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The stencil theory is disproved: Metal reports stencilLoadAction=2 (Clear) for
the drape passes, so the stencil buffer is cleared, and drawCalls equals
enabledDrawables on every target, so every draw is issued. Combined with the
earlier findings - matching texture pointers, GL-identical content, committed
and awaited command buffers - nothing in the submission path differs from
OpenGL, yet the terrain still samples black.

What has not been checked is what each target is cleared TO. Areas of a drape
target not covered by a draped drawable show the clear colour, so if Metal
clears to black where the style background is white, the black regions would
follow with every draw still succeeding. On OpenGL these targets clear to
1,1,1,1 for terrain/occlusion-debug.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on one switch

Logging has now eliminated every stage of the drape path on Metal: drawables
all created, targets all render, texture pointers match at both ends, command
buffers commit and wait, stencil loads as Clear, drawCalls equals
enabledDrawables, sampler is ClampToEdge with in-range UVs. The submission path
is indistinguishable from OpenGL, yet the terrain samples black.

No further log line can separate the two remaining explanations, so dump the
artifact itself: after present() (which has already committed and waited), read
each drape target back and write drape_<z>-<x>-<y>_w<wrap>.png into the working
directory for CI to upload. If the PNG shows the draped map, draping works and
terrain is sampling it wrongly; if it is blank or black, the draping is at
fault. Verified on OpenGL, where the six terrain/occlusion-debug targets dump
fully populated tiles.

All of this is temporary, so it is now behind a single switch in the new
src/mbgl/renderer/terrain_diagnostics.hpp: MLN_TERRAIN_DIAG silences every log
site, MLN_TERRAIN_DUMP_DRAPES disables the readback. Reverting the whole
investigation means deleting that header and the guarded blocks that reference
it, all marked MLN_TERRAIN_DIAG. drape_*.png is gitignored so dumps cannot be
committed by accident.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… unique

a4a7c93 wrote the drape targets to PNGs but nothing collected them, so they
never left the runner. The macOS render-test artifact only uploaded the HTML
report; add drape_*.png to that step. It already runs only when the render
tests fail, which is the case we care about.

Also prefix each dump with a sequence number. Different tests cover the same
tile ids - terrain/occlusion-debug and terrain/fill-extrusion both drape
12/2178/1433 and five others - so the previous naming had the second test
silently overwrite the first. Verified locally: running both tests now yields
12 files rather than 6. Map a file to its test by the order of its
"DRAPE dumped" line in the log.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The drape dumps did their job and are now removed. Read back from CI, every
Metal drape target contains the correct draped map - drape_0017_12-2179-1434
is pixel-identical to the OpenGL dump of the same tile, and the other targets
are populated too. Draping is NOT the bug, so MLN_TERRAIN_DUMP_DRAPES goes to
0 and the drape_*.png upload is removed from node-ci.

That leaves consumption. The terrain surface samples a texture that is known
good and still renders black, so the question is whether the terrain drawable
binds its map texture at draw time. mtl::Drawable::bindTextures only binds a
texture when shader->getSamplerLocation(id) resolves; a texture present with
no location is silently skipped, which would leave the fragment shader's
mapTexture unbound and sampling zero - black - while the vertex shader's DEM
(a different slot) still works, which matches the symptom exactly: elevation
is applied, colour is not.

Log every terrain texture bind with its slot, resolved location, size and
pointer, plus a warning for the silently-skipped case. Scoped to drawables
named "terrain*" so it cannot spam the 1300-test suite. Compare the map
texture's size against the 1024x1024 drape targets and its pointer against
the "TERRAIN tile ... bindsTexture=" line already logged at creation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every stage outside the draw now measures correct on Metal: the drape targets
are pixel-identical to OpenGL and fully opaque, their texture is bound at
slot 1 / location 1 at 1024x1024 to both the vertex and fragment stages, no
bind is silently skipped, draws equal enabled drawables, and depth/stencil load
as Clear. The surface still samples black, so the next thing to isolate is the
fragment invocation itself - which no CPU-side log can observe.

Make the terrain fragment body selectable via MLN_TERRAIN_FRAG_PROBE:
  0  sample the drape texture (the real shader)
  1  flat red        <- enabled here
  2  visualise UV as (u, v, 0, 1)

Red everywhere means the fragment runs and the sample is at fault; still black
means the fragment is not executing, or its output is discarded or overwritten
- which would point at pipeline colour state or the terrain draw being hidden
by something else, not at texture handling at all. Mode 2 then localises the
UVs if mode 1 comes back red.

Selected by the C++ preprocessor and spliced into the MSL by string literal
concatenation, so it costs nothing when the probe is 0. Not verifiable locally
(Metal needs macOS).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Probe 1 came back red across the entire terrain surface, including every
region that previously rendered black. So the fragment shader executes for
those fragments and its output does reach the framebuffer - the draw is not
being discarded, hidden or overwritten, and pipeline colour state is fine.

That isolates the failure to the sample itself: mapTexture.sample(mapSampler,
uv) returns black even though the bound texture is the correct 1024x1024 drape
target, fully opaque and pixel-identical to OpenGL's. With the texture ruled
out, the remaining input is the coordinate.

Probe 2 renders uv as (u, v, 0, 1). A red/green gradient across each tile means
the UVs are sane and the fault is in the sampler or texture state; black,
uniform, or wildly discontinuous regions localise it to the UVs - which are
computed as pos/8192 from the short4 vertex attribute, so a truncated or
mis-typed attribute would show up here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Probe 2 (UV visualisation) came back with u or v pinned to exactly 0.000 over
the regions that render black, while the working areas show proper gradients.
A constant edge UV is precisely what a skirt vertex carries: skirts hang below
the tile edge but keep that edge's coordinate, so they smear the tile's border
texel across whatever they cover. In these fixtures the drape tile borders are
black grid lines - so the sampler was faithfully returning black from a correct
texture all along, and "the drape is broken" was never the right frame.

The remaining question is why the skirts are visible at all. They are meant to
be hidden behind the terrain surface: both are drawn with DepthMaskType
ReadWrite and depth enabled, and TerrainEvaluatedPropsUBO matches the C++
layout byte for byte, so elevation_offset is not corrupt and the skirts hang
the same distance as on OpenGL, where nothing shows.

Probe 3 paints skirt fragments blue and samples normally elsewhere, carrying
the existing pos.z skirt flag through to the fragment stage. If blue covers
what used to be black, the skirts are drawing over the surface rather than
being occluded by it, which points at Metal depth state for the terrain draw
rather than anything to do with texturing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n fix)

Root cause of the Metal terrain artifacts, established by the probe sequence:

- Probe 1 (flat red): the fragment shader runs everywhere and its output
  reaches the framebuffer.
- Probe 2 (UV visualisation): u or v pinned to exactly 0 over the black
  regions - the constant edge coordinate a skirt curtain carries.
- Probe 3 (skirt flag blue): blue bands exactly where black was, with black
  above them - the curtain quads mix surface-edge verts (z=0) with skirt verts
  (z=1), so the flag interpolates down each curtain; blue bottoms and
  border-smear-black tops are one phenomenon, not two.

So the black was always the terrain SKIRTS, visible when they should be
occluded, faithfully smearing the drape tiles' black border texel (u or v = 0)
down every curtain. The drape textures themselves were verified pixel-identical
to OpenGL - draping was never the bug.

Why only Metal: mtl::Drawable::draw deliberately sets no depth-stencil state
for 3D drawables ("handled by the layer group", so all 3D drawables in a group
share one stencilModeFor3D value). mtl::TileLayerGroup does that job, which is
why fill-extrusion mostly renders. The plain mtl::LayerGroup - which is what
RenderTerrain's mesh and depth-pass groups are - never did, so the terrain drew
with the Metal encoder's default depth-stencil: depth test Always, write off.
Triangle order then decided visibility, and the skirts come after the surface
in the index buffer. GL and Vulkan select depthModeFor3D inside the drawable,
so they were unaffected.

Fix: mirror TileLayerGroup's handling in LayerGroup::render - detect 3D
drawables, set stencilModeFor3D's reference once per group, and give each 3D
drawable a depth-stencil state built from depthModeFor3D()/its stencil flag
(depth-only states cached across frames as members, stencil-based ones rebuilt
per frame because stencilModeFor3D hands out a new reference value each call).
2D drawables keep setting their own state inside draw().

This should also un-break the Metal terrain DEPTH pass (same plain-group path),
i.e. symbol occlusion on Metal. The fragment probe is reset to 0 so this run
renders the real shader against the baselines; the diagnostics header records
the probe conclusions.

Not verifiable locally (Metal needs macOS); the OpenGL build and the
terrain/fill-extrusion render test still pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The MSL fill-extrusion shaders never sampled the DEM, so on Metal the
buildings sat at sea level while the terrain around them was displaced.
Metal was the last backend with this gap - GL and WebGPU always had it,
Vulkan was fixed in 62e1fdc - and it is what remains of the
terrain/fill-extrusion diff now that the depth-state bug (96f0a95) is out
of the way.

Metal resolves drawable textures through TextureInfo/getSamplerLocation
and binds them to both the vertex and fragment stages, so none of the
Vulkan binding workarounds apply here: declare the DEM at texture/sampler
slot 0 on the vertex function and register
TextureInfo{0, idFillExtrusionDEMTexture}.

The roof shader samples at a new per-vertex centroid attribute, which
shares the buffer index of pos/decimals_ed because all three are
interleaved in the bucket's 12-byte shared vertex. The wall shader reads
centroid_pos from the aliased OutlineInstance record, already synced in
62e1fdc. Both apply the same lift as fill_extrusion.vertex.glsl,
including the 10 m basement drop gated on dem_enabled.

The pattern variants still do not elevate, on any backend.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The renderer side is healthy - mesh tiles, DEM textures and drape targets
all report correct in the diagnostics - the frame is white because the
mesh clips away. Lowering only the exaggeration (2 -> 0.45) turns the
blank output into rendered terrain, and TransformState's centerAltitude
is only ever written from explicit CameraOptions, never derived from the
terrain the way gl-js's updateElevation() does. So the camera sits at its
sea-level altitude while the ground rises to meet it.

Phase 4 work rather than a patch; the test stays enabled as the guard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drops everything added while diagnosing the Metal terrain bugs, including
the parts behind the MLN_TERRAIN_DIAG compile-time switch: the
terrain_diagnostics.hpp header itself, the TERRAIN/DRAPE counters in
render_terrain.cpp and render_target.cpp (the render_terrain ones were
never gated and logged on every frame in every build), the drape-target
PNG dumps and their .gitignore entry, the MTLPASS clear-request log, the
MTLBIND texture-bind log, and the fragment-probe plumbing in the Metal
terrain shader - including the is_skirt varying that only existed to feed
probe 3.

Also removes the older unconditional Metal texture logging in
texture2d.cpp (texture created / binding to location / uploading N bytes
/ upload complete), which is not upstream and fired on every texture
bind and upload.

The two fixes these diagnostics found stay: the 3D depth-stencil state in
mtl::LayerGroup and the fill-extrusion DEM sampling. The offscreen colour
store action stays too, with its comment corrected - it was committed on
a theory that proved wrong, but the Metal default really is undefined
behaviour for a texture that gets sampled afterwards.

Verified on the Windows GL runner: the terrain suite reports exactly the
same results as before the removal, with no diagnostic output.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
skirts-auto/skirts-none came over from gl-js with gl-js's own baselines
(13b3632), but 873da6a repointed their DEM source at the verified JAXA
terrain-RGB tiles - different elevation data for the same tiles - so the
old images can no longer be matched by any correct renderer. They were
failing at 0.2296 and 0.1951 on both OpenGL and Metal.

Regenerated with the local Windows OpenGL runner (-u default). The new
images show the same features as the gl-js ones; the terrain displacement
differs visibly, which is the fixture swap rather than a rendering
change.

Kept as its own commit so it can be reverted independently if we would
rather keep comparing against gl-js.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The baseline added with the test was made with the Windows OpenGL runner.
It passes there and on Metal, but fails on Linux CI, which is the check
that gates the PR.

Regenerated with a linux-opengl build (cmake --preset linux-opengl) under
WSL Ubuntu, run the way CI runs it: xvfb-run with EGL_PLATFORM=surfaceless
and LIBGL_ALWAYS_SOFTWARE=1, so Mesa's software rasterizer is used instead
of the d3d12 passthrough driver. That build reports the same GPU
identifier as CI, llvmpipe (LLVM 20.1.2, 256 bits), and reproduced the
failure against the old image before regenerating it - so it is the same
renderer, not merely a different one.

The renders are the same picture: the difference was 0.00034, anti-alias
speckle along the tile edges plus a one-pixel row at the top and bottom of
two of the extrusion boxes. No rendering change is involved.

The rest of the terrain suite is unaffected on Linux - skirts-auto and
skirts-none pass, and default, occlusion-debug and pitched-world still
fail as they do on CI.

Kept as its own commit so it can be reverted independently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

android build Related to build, configuration or CI/CD core Changes that affect the C++ core of MapLibre Native documentation Improvements or additions to documentation github_actions Pull requests that update GitHub Actions code iOS metal node OpenGL Issues related to the OpenGL renderer backend Qt Vulkan WebGPU windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants