Gsplat LOD: empty level for nodes without coarse data; distance default - #9326
Conversation
A leaf whose data stops before rangeMax - the generator decimated the region to nothing at the coarser levels - had no chain entry for those levels, so it was pinned to its finest available data at any distance, loading a whole file for a handful of splats. GSplatLodTable now gives such a node an empty level (zero splats, no file) just above its coarsest data as the start of its chain. Distance mode prices the step to the real data as one more distance band, so the node appears exactly when the camera enters the band of its coarsest data; error mode extrapolates the node's last error step so it can still choose to lift the node. The parser gives any zero-count level no file index, and underfill treats the empty level as loaded.
Distance mode holds a memory footprint comparable to the pre-2.22 distance-based selection, while error mode lifts sparse background regions at a noticeably higher memory cost, so error mode becomes the opt-in. The docs, the lod-streaming and downtown examples and their controls follow.
Build size reportThis PR changes the size of the minified bundles.
|
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated PR review — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed.
The empty-level mechanism holds together. Two things I specifically went after and found clean:
- The inverted
fileIndex === -1predicate is safe. It looks alarming out of context, butfindCoarserAcceptedonly ever walks a node's chain (startLodplus itsupgradeToLodentries), chain membership requirescount > 0 || lod === emptyLod, andcount > 0always implies a real file index from the parse — so among the levels that predicate can see,fi === -1is exactly the empty level. Omitted and mid-chain gap levels are never candidates, so they can't sneak through as "loaded". The neighbouring sites are consistent too: the second fallback still requiresfileIndex !== -1, and bothprefetchNextLodbranches plusincrementFileRef/decrementFileRefalready guard-1. - The distance-mode pricing is right. The
continueon empty levels meanseandfinerCountstill hold thecoarsestDatastate when the loop exits, soerr[emptyLod] = err[coarsestData] + max(count, 1) * bandWeight[emptyLod]is the same "splats dropped × band weight" the normal steps use. I worked the test case by hand and it lands on the asserted ratio of 9.
65 gsplat tests pass, lint clean. Two minor points inline, plus one process note.
Process note on the default flip. The body lists the developer-site docs and the editor's lodMode default as out-of-repo follow-ups but doesn't link them. Worth filing/linking both before this merges — a default change is exactly the kind that silently fails to reach editor users (if the editor persists lodMode explicitly they keep error mode and never see the memory win; if it omits it at its default they flip with no changelog on their side), and the asymmetry is invisible from this repo once the PR is closed.
| @@ -164,7 +164,7 @@ app.scene.gsplat.dataFormat = GSPLATDATA_COMPACT; | |||
| // How the splat budget picks LOD levels: 'error' spends it where the bundle's per-node error | |||
| // metadata says detail is worth most; 'distance' orders detail by camera distance alone and | |||
| // ignores that metadata. Error is the default and the reason the bundle carries the metrics. | |||
There was a problem hiding this comment.
This comment is now stale in a way that contradicts the line directly below it — it still says "Error is the default and the reason the bundle carries the metrics", while the data.set beneath now selects distance and GSplatParams defaults to it.
The second half is still worth keeping in some form, since it explains why the bundle ships error tables at all even though the example no longer starts in that mode. Something like: "...ignores that metadata. Distance is the default; error is what the bundle's metrics are there for, and lifts sparse regions distance leaves coarse."
(lod-streaming.example.mjs has no equivalent prose — its data.set('lodMode', ...) sits in a bare block of data.set calls — so this is the only example comment affected.)
| break; | ||
| } | ||
| } | ||
| err[emptyLod] = ec + (step > 0 ? step : (ec > 0 ? ec : 1)); |
There was a problem hiding this comment.
The two fallbacks here are untested and the innermost one puts an uncalibrated magnitude into a cross-node ranking.
Reachability is not exotic: step stays 0 whenever the node has data at only one level in range, and in that case err[coarsestData] is exactly 0 under derived errors — _deriveLodErrors gives the finest renderable level Math.log(refCount / refCount). So a single-data-level node takes the ec > 0 ? ec : 1 branch and lands on the literal 1.
Whether that number means anything depends on where the errors came from, which is the part worth a thought:
- Derived errors are
log(refCount / count), so they sit in roughly 0–5 and1reads as about one e-fold of decimation. Defensible. - File-supplied errors are whatever the generator emits — the parse only requires finite and
>= 0. Against world-space errors around 1e-3 a1dominates every other candidate; against pixel errors in the hundreds it disappears. Either way that node's lift priority is set by a constant rather than by the asset.
Since lodErrorSource already distinguishes the two, scaling the fallback to something the node owns would remove the guesswork — the node's finest non-zero error, or a fraction of the table's typical step. Not a blocker, and the step > 0 path (the common one) is sound and tested.
Worth a test either way: neither fallback branch is currently covered, and a single-level node reaching 1 is a plausible shape for a small leaf. A gap case (data at 0 and 2, hole at 1, so emptyLod becomes 3 and level 1 stays excluded) would be a cheap third case — the code handles it correctly, it just isn't pinned.
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated PR review — posted on my behalf by Codex (GPT-5). This is an automated review, not a human review.
Reviewed the complete PR at 3087fbff2, including the allocator chain, budget selection, underfill/prefetch transitions, manifest parsing, public default change, examples, tests, performance characteristics, and existing review discussion.
Finding
[P2] Complete the public documentation migration to distance mode. AppBase still documents settings.render.gsplatLodMode as "error" (default) in src/framework/app-base.js:1466, even though an omitted setting now resolves to the new GSplatParams distance default. The adjacent public GSplatParams#splatBudget documentation in src/scene/gsplat-unified/gsplat-params.js:475-479 also says the budget is always spent by approximation error per splat, which is no longer true in the default mode. These feed user-facing API documentation, so they should be changed to advertise distance as the default and describe budget allocation in mode-neutral terms. The stale Downtown example comment is the same migration gap and has already been raised inline by the other automated review.
The empty-level behavior itself looks coherent: only the synthetic trailing level enters the chain with fileIndex === -1; budget costs remain positive; distance pricing preserves the intended band ratio; and the invisible → prefetch → visible transitions avoid loading or placing a nonexistent file. The extra construction work is bounded by nodes × LOD levels and adds no per-frame allocation. The implementation is backend-independent.
Validation performed:
- Targeted GSplat LOD/balancer/instance tests: 60 passing
- Full unit suite: 2682 passing, 2 pending
npm run lint: passed- Type declaration build and type tests: passed
- All engine bundle builds: passed
- GitHub CI: all checks green
The local bundle-test runner reached 30 passing before 16 existing Node-environment failures caused by unguarded navigator access in untouched platform code; this PR does not modify that path.
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated re-review (commit dad89b2) — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed.
Both findings resolved, and the error-mode fix goes further than I suggested in a way worth noting:
- Stale docs — the example comment is corrected and keeps the "why the bundle carries the metrics" half. You also caught
app-base.js'sgsplatLodModesettings doc and thesplatBudgetprose, which I'd missed. I grepped the branch for any surviving "error is the default" wording acrosssrc/andexamples/and it's clean. - The
: 1fallback — widening the step search tolod >= 0is the better fix, since it means the magnitude now comes from the node's own decimation step in nearly every case rather than a constant. Switching tolods[lod].erroris required for that and not just tidier:err[]is only written forrangeMin..rangeMax, so readingerr[lod]belowrangeMinwould have silently used 0 and made the step equal toec. AndMath.LN2is a genuinely calibrated constant — for a single-data-level node it prices the empty level exactly as one 2x decimation step in derived-error units (log(refCount/count)), so such a node ranks identically to a two-level node with 2x decimation. Self-consistent.
The three new tests cover exactly the gaps (step from below rangeMin, the constant fallback, and the interior gap). I worked the gap case by hand — coarsestData 2, emptyLod 3, step 2 from level 0, so reaching level 2 is 2/20 = 0.1 against 4/100 = 0.04 for level 0 — and it lands on the asserted 0.1 with level 1 correctly excluded. 34 passing, lint clean.
Two leftovers, neither needing action here:
Math.LN2 is still unit-foreign when errors come from the manifest. The parse only requires finite and >= 0, so file-supplied errors can be in any unit; against errors around 1e-3 a +0.693 makes such a node the most attractive lift in the scene, and against errors in the hundreds it vanishes. Much narrower than before — it now needs a node with no data at any level finer than its coarsest in-range one — and the comment is explicit that the constant is in derived-error units, so this is fine as a documented limitation. lodErrorSource is already there if it ever wants to be scale-aware. (Tiny wording note: the test name and comment say "a single level overall", but the condition is really "no data finer than its coarsest in-range level", which also admits a node holding data above rangeMax.)
The out-of-repo follow-ups are still unlinked. The body's closing line names the developer-site lodMode docs and the editor's lodMode default but carries no issue or PR references. Worth filing and linking them before this merges — of everything here, a changed default is the item most likely to be forgotten once the PR closes, and the divergence isn't visible from this repo.
mvaligursky
left a comment
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated PR re-review — posted on my behalf by Codex (GPT-5). This is an automated review, not a human review.
Re-reviewed the changes from 3087fbff2 through current head dad89b29c, then rechecked the complete PR and existing review discussion.
No actionable findings. The previous P2 is resolved.
The default migration is now consistent across the public AppBase setting documentation, GSplatParams#splatBudget, GSplatParams#lodMode, constants, and the Downtown example. The revised empty-level error pricing also correctly searches the node's finer levels below rangeMin; using lods[lod].error there is necessary because the table-local err scratch array is only populated inside the selected range. The remaining Math.LN2 branch is now limited to nodes with no finer data from which to derive a step, is explicitly documented as a one-halving heuristic in derived-error units, and is covered by a focused test. The interior-gap test also confirms that only the trailing empty level joins the chain.
The update does not alter the distance-mode pricing, positive upgrade-cost invariant, underfill/prefetch lifecycle, backend behavior, or hot per-frame path.
Validation performed on dad89b29c:
- Targeted GSplat LOD/balancer/instance tests: 63 passing
- Full unit suite: 2685 passing, 2 pending
npm run lint: passed- Type declaration build and type tests: passed
- All engine bundle builds: passed
git diff --check: passed- GitHub CI: all checks green
The developer-site and Editor default updates remain appropriately called out as out-of-repository follow-ups in the PR description.
…lt (#9326) * Give octree nodes with no coarse-level data an empty start level A leaf whose data stops before rangeMax - the generator decimated the region to nothing at the coarser levels - had no chain entry for those levels, so it was pinned to its finest available data at any distance, loading a whole file for a handful of splats. GSplatLodTable now gives such a node an empty level (zero splats, no file) just above its coarsest data as the start of its chain. Distance mode prices the step to the real data as one more distance band, so the node appears exactly when the camera enters the band of its coarsest data; error mode extrapolates the node's last error step so it can still choose to lift the node. The parser gives any zero-count level no file index, and underfill treats the empty level as loaded. * Default GSplatParams#lodMode to GSPLAT_LODMODE_DISTANCE Distance mode holds a memory footprint comparable to the pre-2.22 distance-based selection, while error mode lifts sparse background regions at a noticeably higher memory cost, so error mode becomes the opt-in. The docs, the lod-streaming and downtown examples and their controls follow. * updates --------- Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
Streamed gaussian-splat octrees can contain leaves whose data stops before the coarsest LOD level - the generator decimated the region to nothing there. The LOD chain had no entry for those levels, so such a node was pinned to its finest available data at any distance, loading a whole ~10 MB file for as few as 3 splats. This gives those nodes an empty level instead, and flips the default LOD mode to distance, whose memory footprint matches the pre-2.22 distance-based selection.
Changes:
rangeMaxnow gets a zero-splat, file-less level just above its coarsest data as the start of its LOD chain. It draws nothing until the allocator buys its real coarsest level. Distance mode prices that step as one more distance band, so the node appears exactly when the camera enters the band of its coarsest data; error mode extrapolates the node's last error step, so it can still choose to lift the node - which is that mode's purpose.GSplatParams#lodModedefaults toGSPLAT_LODMODE_DISTANCE.GSPLAT_LODMODE_ERRORremains available; its docs now describe the trade-off (lifts sparse, low-quality regions that distance leaves coarse, at a noticeably higher memory cost).API Changes:
GSplatParams#lodModedefault changed fromGSPLAT_LODMODE_ERRORtoGSPLAT_LODMODE_DISTANCE. Scenes that relied on the error-driven default should setapp.scene.gsplat.lodMode = GSPLAT_LODMODE_ERRORexplicitly.Examples:
lod-streaminganddowntownstart in distance mode, and their LOD Mode controls default to it.Performance:
example_roman_parish_02, distance mode, 4M splat budget, same camera, settled: resident files 28 -> 24, texture VRAM 383.6 -> 343.0 MB, while drawing more splats (3.83M -> 3.97M) as the freed budget is re-spent.example_roman_parish_03(few such nodes): 457 -> 441 MB on WebGL, 655 -> 639 MB on WebGPU - one 16 MB file.Follow-ups outside this repo: the developer-site
lodModedocs and the editor'slodModedefault still describe error mode as the default.