Skip to content

perf: replace the terrain coords framebuffer picking with a CPU raycast - #8158

Open
johncarmack1984 wants to merge 8 commits into
maplibre:mainfrom
johncarmack1984:gm/delete-coords-texture
Open

perf: replace the terrain coords framebuffer picking with a CPU raycast#8158
johncarmack1984 wants to merge 8 commits into
maplibre:mainfrom
johncarmack1984:gm/delete-coords-texture

Conversation

@johncarmack1984

Copy link
Copy Markdown
Contributor

Follows up on the note at the end of #8145, and fits the "more things in Phase 1" invitation in

With terrain enabled, pointCoordinate (screen point → map coordinate) currently works by rendering every terrain tile a second time into a screen-sized framebuffer, using a static 4 MB texture that encodes tile-local coordinates in RGB and a tile index in alpha, then reading one pixel back with readPixels. Every mouse and touch event pays for that render pass plus a pipeline stall, the answer is quantized to 1024 steps per tile, and the one-byte tile index silently breaks past 255 renderable tiles.

This PR computes the same answer on the CPU instead: intersect the ray through the screen pixel with the rendered terrain surface, sampling the same DEM tiles the mesh renders from. Mercator marches the frustum segment and bisects the first surface crossing; globe brackets between spheres at the min/max terrain elevation using the ray-sphere intersection that already existed for the non-terrain path. The null semantics callers depend on are unchanged: sky misses, points outside the renderable tile set, and the flat-at-zero behavior of tiles whose DEM hasn't loaded all behave as before. With the readback gone, the coords texture, its framebuffer attachment, the terrain_coords shaders, the extra draw pass, and the 255-tile limit all go away. The depth framebuffer stays — marker occlusion still uses it.

Accuracy improves: the old path sampled a 128×128 mesh interpolation quantized to 1024 steps per tile, the new one bisects against full-resolution bilinear DEM data with a final secant refinement. I ran both builds side by side in headless Chrome over the repo's Alpine DEM fixtures — 16 scenarios (mercator and globe, pitches 0–85, overzoom, high zoom, exaggeration, bearing, terrain off/re-enabled, a DEM-less low-zoom globe), a 15×15 pick grid each plus a marker-occlusion and queryRenderedFeatures comparison. Rendered output is pixel-identical in every scenario, hit/miss classification agrees at every probe, marker occlusion states match everywhere, and on mercator the picked positions differ by ~0.5 screen px median (p95 ≤ 1.6 px) — inside the old path's own quantization error, with flat terrain matching the analytic plane intersection to double precision. One pre-existing inconsistency goes away on globe: the readback answered in the rendered-mesh frame, so with terrain enabled unproject sat ~12 screen px off the same map's no-terrain unprojection (measured at z10, pitch 0); the raycast matches the no-terrain globe math exactly (0.00 px on a flattened surface across the full grid).

The old path could never run under npm run bench (it needs a GPU); the new one can, so this adds a bench file. A pick costs 0.6–66 µs depending on scenario:

bench mean
flat hit z2 4.3 µs
sloped hit z10 8.2 µs
sloped hit z16 66 µs
sky miss z10 3.4 µs

The browser integration tests aren't in CI, so for the record: they pass locally (18/18, including the terrain marker-opacity and terrain-gesture tests), alongside the unit, render, and build suites. Reviewing commit-by-commit may be easiest. The first commit adds the raycast and its tests without wiring it, the second switches pointCoordinate over and deletes the GPU machinery.

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.
  • Confirm you have read our AI policy here.

Adds src/render/terrain_raycast.ts, which intersects the ray through a
screen pixel with the terrain surface on the CPU for both mercator and
globe, plus the transform ray primitives it needs. Not wired up yet.
pointCoordinate now raycasts the DEM on the CPU. Deletes the coords
texture, its shaders, uniforms and draw pass, the coords framebuffer
attachment and coordsDirty bookkeeping, and the exact-matrix depth
refresh that only existed to serve them. Signature and null semantics
are unchanged; hit positions are no longer quantized to 1024 steps per
tile and are no longer capped at 255 renderable tiles.
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.84393% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.11%. Comparing base (f18eec0) to head (523799b).

Files with missing lines Patch % Lines
...c/geo/projection/vertical_perspective_transform.ts 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8158      +/-   ##
==========================================
+ Coverage   93.92%   94.11%   +0.18%     
==========================================
  Files         290      291       +1     
  Lines       24905    24962      +57     
  Branches     6569     6592      +23     
==========================================
+ Hits        23393    23492      +99     
+ Misses       1512     1470      -42     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/render/terrain.ts Outdated
Comment thread src/geo/projection/globe_transform.test.ts Outdated
Comment thread src/geo/projection/globe_transform.ts Outdated
Comment thread src/geo/projection/globe_utils.ts Outdated
Comment thread src/geo/projection/vertical_perspective_transform.test.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants