Skip to content

Commit c8accba

Browse files
acalcuttclaude
andcommitted
Document the Vulkan occlusion fix and the GL regression it likely caused
Phase 2's status note said occlusion was verified on OpenGL and structurally the same elsewhere; it had in fact never worked on Vulkan. Record the six independent defects found and fixed there (aaa145c), and flag the open issue: GL occlusion, previously working, stopped hiding labels behind terrain after the same fix landed - prime suspect is switching calculate_visibility's argument from gl_Position to projectedPoint (fix #6), not yet diagnosed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5d39f97 commit c8accba

1 file changed

Lines changed: 65 additions & 2 deletions

File tree

TERRAIN.md

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,71 @@ already compute fade opacity). The convention was confirmed per backend against
364364
the existing `heatmap_texture` shaders, which sample a render target the same
365365
way: GL flips `a_pos.y`, Metal and WebGPU do not.
366366

367-
Verified working on OpenGL on device (icons behind terrain are hidden);
368-
Metal/Vulkan/WebGPU follow the same structure but are not yet run on hardware.
367+
**2026-07-19/20 update - Vulkan occlusion debugged and fixed on device; GL now
368+
suspected broken by the same change.** Occlusion had *never* actually worked on
369+
Vulkan (the "verified... on OpenGL" note above was accurate, but Vulkan/Metal/
370+
WebGPU following "the same structure" turned out not to be enough). On-device
371+
debugging (screenshots + targeted shader probes) found six independent Vulkan
372+
defects, all now fixed:
373+
374+
1. The terrain depth layer group is never registered with the orchestrator (by
375+
design - it's rendered only from `RenderTerrain::renderDepth`), so the
376+
general upload pass skipped it: its drawables had no vertex buffers, and
377+
Vulkan's `bindAttributes` failed silently every frame. Fix: upload it
378+
explicitly in `Renderer::Impl::render`. GL builds attribute state at draw
379+
time, so it never hit this.
380+
2. `RenderTerrain::renderDepth` created the depth render target lazily, after
381+
the symbol tweaker had already bound `getDepthTexture()` for the frame -
382+
symbols got the 1x1 far-plane placeholder for that frame, permanently so in
383+
single-frame renders (the render tests). Fix: `prepareDepthTarget()` runs
384+
before the upload phase now.
385+
3. Vulkan's `unpack_depth` returned the stored `[0,1]` depth as-is, but symbol
386+
matrices carry no `[-1,1]->[0,1]` remap (only the terrain/drape matrices do
387+
- see Phase 2's original z-remap note above), so the compared z was still
388+
GL-convention and the comparison almost never fired. Fixed to `*2-1`,
389+
matching GL and Metal.
390+
4. The visibility test was a hard binary z compare; on-surface labels z-fought
391+
it. Replaced with gl-js's actual `depthOpacity()`: a small bias, a soft fade
392+
over ~0.002 NDC, and a second sample above the anchor (a label just behind a
393+
ridge still shows if its glyphs poke above it). Ported to all four backends.
394+
5. Occlusion was gated on `dem_enabled` (does *this tile* have DEM data), so
395+
labels from tiles outside the terrain cover - exactly the distant labels
396+
that pierce mountains near the horizon - skipped occlusion entirely. Added
397+
a global `depth_enabled` flag (terrain on) in `SymbolDrawableUBO`'s former
398+
padding slot; `dem_enabled` still gates only the elevation.
399+
6. The position compared against the depth texture was `gl_Position` - for
400+
screen-aligned text that's post label-plane/`coord_matrix`, a different clip
401+
space than the depth pass (and than gl-js, which compares its tile-projected
402+
`projectedPoint`). Switched all four backends to pass `projectedPoint`, with
403+
Vulkan additionally running it through a new `surface_transformed()` helper
404+
(same transform `applySurfaceTransform()` applies to `gl_Position`).
405+
406+
Verified on Android/Vulkan on-device (`TerrainVectorMapActivity`, planet-vector
407+
basemap over the Alps): distant city labels correctly hide behind the
408+
Nordkette at both steep and shallow pitch, with gl-js's soft fade at ridge
409+
crests. Committed as `aaa145cc85df`.
410+
411+
**Known issue - GL regressed by the same fix (#6 above), unconfirmed cause.**
412+
After the Vulkan fix landed, GL symbol occlusion - previously verified working
413+
on device - stopped hiding labels behind terrain (OpenGL flavour,
414+
`TerrainVectorMapActivity`, reported on device 2026-07-20). Fixes #1-3 are
415+
Vulkan/Metal-specific dead code on GL (its `unpack_depth` already had the
416+
`*2-1`, and GL builds attribute state at draw time so #1 never applied), and
417+
#4/#5 only *broaden* when occlusion applies, so they're unlikely culprits. The
418+
prime suspect is #6: GL's occlusion was implemented and tuned against
419+
`gl_Position` (the final label-plane-adjusted screen position), and switching
420+
its `calculate_visibility` argument to `projectedPoint` (the raw tile-projected
421+
anchor, matching gl-js and fixing Vulkan's shallow-pitch failure) may not be
422+
correct for GL's particular label-plane/pitch/rotation pipeline, or may expose
423+
a second, GL-specific bug that `gl_Position` happened to mask. Not yet
424+
diagnosed on device - `projectedPoint` is provably what gl-js itself compares,
425+
so the fix is more likely to find *why* it fails on GL than to revert it.
426+
Next step: run the local `terrain/occlusion-debug` render test (uncommitted
427+
style at `metrics/integration/render-tests/terrain/occlusion-debug/`) against
428+
a GL build to check whether the depth texture populates on GL the way it was
429+
confirmed to on Vulkan (see the diagnostic pattern in the aaa145c commit
430+
message / this session's history), before assuming the coordinate-space
431+
hypothesis is right.
369432

370433
### Phase 3 - Seams and quality
371434

0 commit comments

Comments
 (0)