Continue 3d terrain work - #4389
Draft
acalcutt wants to merge 304 commits into
Draft
Conversation
…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>
Contributor
Bloaty Results (iOS) 🐋Compared to main Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-4389-compared-to-main.txt |
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>
Contributor
Bloaty Results 🐋Compared to main Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-4389-compared-to-main.txtCompared to d387090 (legacy) Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-4389-compared-to-legacy.txt |
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>
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
force-pushed
the
terrain-3d-color-relief
branch
from
August 2, 2026 07:17
7103ad6 to
71c6d3b
Compare
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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.
changes. Prepare passes (e.g. hillshade) run before the drapes that sample them.
Elevated layers
displaced by the terrain, on all four backends.
Tile cover
elevation, so terrain leaning toward the camera is requested. After
expandToDeepestCoverthe mesh is frustum-culled, so a sparse DEM's largelow-zoom tiles aren't meshed across their off-screen area.
Other
fallback, terrain root-property parsing (
source,exaggeration).terrain_depthshader header included where Metal/Vulkan/WebGPU register it.Remaining work and known issues are in
TERRAIN.md(mesh skirts, drape targetsize, camera-terrain anchoring, a zoom-0 exaggeration bug).
Diff size: against
mainthis is ~255 files. Againstfeature/terrain-3ditshows ~850 because it also brings that branch up to
main— the extra files anddeletions are the upstream catch-up, not terrain changes.