refactor(viz): ring-buffered LosQueryManager (foundation for LiDAR) - #71
Merged
Merged
Conversation
Replaces the single-slot serialization in LosQueryManager with a ring of N slots. Each slot owns its own rayBuf/hitBuf/readBuf/bindGroup and serializes only its own pending queries via a .then()/.catch() chain on its inFlight tail. Round-robin slot selection ensures successive queries pick different slots so concurrent dispatches across slots don't race on shared buffers. Why now: - The mesh-link LoS path landed in PR #70 doesn't need the ring (effects.ts has its own skip-if-busy throttle so at most one query is ever in flight). But high-rate dispatches like LiDAR scans — next on the WebGPU sensor roadmap — would otherwise stall queueing behind one slot. Ring buffer is foundation work to unblock them. Why backward compatible: - Constructor adds an optional `slotCount: number = 2` parameter. Existing call site `new LosQueryManager(device, world, 256)` in sensors.ts gets a 2-slot ring with no other change. Throughput-wise, effects.ts's skip-if-busy throttle still keeps at most one query in flight, so observable behaviour is unchanged for mesh-link LoS. - query() / capacity() / LosRay public surface unchanged. Memory: each slot allocates `maxRays * (RAY_BYTES + 2 * RAY_HIT_BYTES)`. For the existing call (maxRays=256, slotCount=2) that's ~40 KB total, negligible. LiDAR with 65k rays would want slotCount=3 ≈ 16 MB. New `slotCount` getter exposes the ring depth for callers that want to tune their throttle. Validation: typecheck passes, vite build 805 KB (under 819,200 cap), dotnet Release passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the LosQueryManager to implement a ring-buffered asynchronous architecture, replacing the previous single-slot serialization model. By introducing multiple slots—each with its own set of GPU buffers and independent promise chains—the manager now supports concurrent queries via round-robin slot selection, which prevents a single slow query from blocking the entire pipeline. I have no feedback to provide.
6 tasks
WomB0ComB0
added a commit
that referenced
this pull request
Apr 28, 2026
PR #7 of the WebGPU raymarcher direction. First high-rate consumer of the ring-buffered LosQueryManager landed in PR #71. Demonstrates that the brick-map raymarcher really is a unified sensor primitive — same kernel, different ray source from mesh-link LoS. What ships: - A 16 elev × 256 azim LiDAR scan (4096 rays per scan, ±22.5° vertical FOV, 200 m range) runs every ~1 second from the first available drone in the production viz. Hits render as a cyan point cloud overlaying the Three.js scene. Skip-if-busy + interval throttle keeps GPU dispatch bounded. New file: - client/webgpu/lidar.ts (~135 LOC): LidarScan class. Pre-computes unit-vector directions in (elev, azim) order at construction; each scan reuses them and only sweeps `origin`. Returns world-space hit positions parsed from the brick-map DDA. Modified: - client/webgpu/sensors.ts: SensorContext gains a second LosQueryManager (`lidar`, capacity 4096, slotCount 3 — 3-slot ring for pipelined dispatches). Existing `los` (capacity 256, default 2 slots) is kept for the small mesh-link queries. Both share the same brick-map world. - client/effects.ts: EffectsManager gains _lidar / _lidarPoints / _lidarScanInFlight / _lidarLastScanTime fields and an _updateLidar method called from update(). Lazy-inits the LidarScan + Three.js Points object on first frame after the sensor context is ready. Falls back gracefully when WebGPU is unavailable. Bundle: main 807 KB (under 819 200 cap), sensors chunk 19.6 KB. The LiDAR class adds ~2 KB to the main bundle (lidar.ts is statically imported by effects.ts so it lands in main, not the sensors chunk — fine, it's tiny). Validation: typecheck, vite build, dotnet Release all pass. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 29, 2026
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
1 participant
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.
Summary
Replaces the single-slot serialization in
LosQueryManagerwith a ringof N slots. Foundation work for the planned LiDAR PR — current
mesh-link LoS doesn't need the ring (its effects.ts throttle keeps at
most one query in flight), but high-rate dispatches like LiDAR scans
would otherwise queue behind one slot and stall.
What changed
Each slot owns its own
rayBuf/hitBuf/readBuf/bindGroupand serializes only its own pending queries via a
.then()/.catch()chain on its
inFlighttail. Round-robin slot selection ensuressuccessive queries pick different slots so concurrent dispatches across
slots don't race on shared buffers.
Backward compatible
slotCount: number = 2parameter.Existing call site
new LosQueryManager(device, world, 256)insensors.tsgets a 2-slot ring with no other change required.query()/capacity/LosRaypublic surface unchanged.effects.ts_losQueryInFlightthrottle still gates dispatches to one at a time.Memory
Each slot allocates
maxRays * (RAY_BYTES + 2 * RAY_HIT_BYTES). Forthe existing call (
maxRays=256, slotCount=2) that's ~40 KB total —negligible. The future LiDAR caller will likely want
maxRays=65536, slotCount=3≈ 16 MB, which is acceptable.A new
slotCountgetter exposes the ring depth for callers that wantto tune their own throttle.
Test plan
npm run typecheckpassesnpm run buildpasses; main bundle 805,375 B (under 819,200 cap)dotnet build -c Releasepasses (pre-push hook)/vianpm run devwith two or more drones — mesh-linklines should still fade through terrain identically to PR feat(viz): mesh-link line-of-sight against terrain #70.
No regression: the ring has 2 slots but
effects.ts's throttlestill gates to one in-flight query, so this is purely a
structural refactor at the LoS-manager layer.
Why now
PR #70 left "ring-buffered LosQueryManager" on the four-loose-ends list
in the project memory. This is the highest-priority of those because
it's the only one that blocks new sensor primitives (LiDAR being
next). The other three (
_meshLinkOccludedeviction, larger worlds,terrain-edit invalidation) are correctness/quality gaps that real
usage may or may not actually hit — the scheduled audit in 2 weeks
will report.
Up next
LiDAR scan as a separate PR built on top of this — ~64 × 1024 rays
per scan from the drone, visualized as a thin point cloud in Three.js.
That PR will be the first consumer of
slotCount > 2.🤖 Generated with Claude Code
Summary by CodeRabbit