Skip to content

Error-driven GSplat LOD selection with a derived fallback - #9233

Merged
mvaligursky merged 8 commits into
mainfrom
mv-gsplat-lod-error
Aug 28, 2026
Merged

Error-driven GSplat LOD selection with a derived fallback#9233
mvaligursky merged 8 commits into
mainfrom
mv-gsplat-lod-error

Conversation

@mvaligursky

Copy link
Copy Markdown
Contributor

LOD selection for streamed SOG has been purely geometric: each node picks a level from camera-distance bands, and the budget balancer redistributes by distance bucket. Distance says nothing about how much a node actually loses at a given level — a node of near-duplicate splats and one carrying fine detail at the same distance are treated alike.

This spends the budget by measured quality loss instead.

How it works. Every node starts at the cheapest level it can render, which is the coarsest the scene can be and therefore always within budget. Each single-level upgrade available anywhere in the scene is then ranked by coverage * error removed / splats added and bought best-first until one does not fit. Stopping at the first upgrade that does not fit — rather than skipping it and continuing — keeps a node's outcome from depending on whether some unrelated cheaper upgrade happened to be considered first, which is what made small camera movements flip levels on and off.

Errors always exist. They are read from the manifest when splat-transform wrote them (lodErrors), and derived from splat counts otherwise, so there is one code path rather than a fallback to distance selection. The derived measure is ln(finestCount / count): the allocator only consumes differences between adjacent levels and decimation is geometric, so a log gives equal error steps for equal count ratios.

Measured against authored errors as ground truth on three captures, over multiple cameras, budgets and weightings: the derived proxy lands within 2–17%, where the distance-based system it replaces is 20–790% worse. Accuracy tracks how finely an asset is partitioned rather than its level count.

Precomputation. Per node, the levels are reduced once to the Pareto frontier over (splat count, error) and stored as a chain of upgrades with their cost and value. Because coverage is a single non-negative per-node scalar, the running minimum that keeps a node's returns non-increasing can be taken at build time too, leaving one multiply per upgrade at selection time. Only a node's next unbought upgrade is ever queued, so at most one entry per node is live and the buckets are intrusive lists over preallocated typed arrays with no per-update allocation.

Changes:

  • New GSplatLodTable, built per octree and LOD range, holding the per-node upgrade chains
  • GSplatBudgetBalancer rewritten: value-ranked greedy over upgrades, 512 fixed-scale buckets, lazy insertion, early exit
  • GSplatOctree reads or derives per-level errors; lodErrorSource reports which
  • Underfill and prefetch step along a node's LOD chain rather than raw level indices, so a node's splat count cannot exceed what was budgeted for it — real captures do contain levels where a coarser one holds more splats
  • Distance-bucket balancing, NodeInfo.budgetBucket, computeGlobalMaxDistance and the _budgetScale feedback loop removed

API Changes:

  • GSplatComponent#lodBaseDistance and GSplatComponent#lodMultiplier are removed (Debug.removed). LOD levels are now chosen to fit app.scene.gsplat.splatBudget, so use that to control quality. The equivalents on the internal GSplatPlacement and the component schema entries are gone too.
  • GSplatParams#splatBudget defaults to 1000000 rather than 0, and budgeted selection can no longer be disabled. A non-positive value would pin every node to its coarsest level, so it warns and the default is used instead. Any content relying on splatBudget = 0 to mean "no cap" needs a real budget.

Examples:

  • All streamed-LOD examples moved onto assets exported with error metadata
  • billions sets an explicit budget, having previously relied on splatBudget = 0 plus the per-instance distance ramp

LOD selection for streamed SOG was geometric: each node picked a level from
camera-distance bands, and the budget balancer redistributed by distance bucket.
Distance says nothing about how much a node actually loses at a given level - a
node of near-duplicate splats and one carrying fine detail at the same distance
were treated alike.

Spend the budget by measured quality loss instead. Every node starts at the
cheapest level it can render, and each single-level upgrade in the scene is
ranked by coverage * error removed / splats added, then bought best-first until
one does not fit.

Errors come from the manifest when splat-transform wrote them, and are derived
from splat counts otherwise, so there is a single code path rather than a
fallback to distance. Measured against authored errors on three captures, the
derived proxy lands within 2-17% of them, where the distance-based system it
replaces was 20-790% worse.

Budget is now always enforced, which makes lodBaseDistance and lodMultiplier
inert - they are removed, and a non-positive budget warns and uses the default
rather than pinning every node to its coarsest level.
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Public API report

This PR changes the public API surface (+0 / −9), per the docs' rules (@ignore / @Private / undocumented are excluded).

Show API diff
-GSplatComponent.get lodBaseDistance(): number
-GSplatComponent.get lodMultiplier(): number
-GSplatComponent.set lodBaseDistance(value: number)
-GSplatComponent.set lodMultiplier(value: number)
-GSplatOctreeNodeLod.count: number
-GSplatOctreeNodeLod.file: string
-GSplatOctreeNodeLod.fileIndex: number
-GSplatOctreeNodeLod.offset: number
-interface GSplatOctreeNodeLod

Informational only — this never fails the build.

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Build size report

This PR changes the size of the minified bundles.

Bundle Minified Gzip Brotli
playcanvas.min.js 2389.8 KB (+2.1 KB, +0.09%) 614.5 KB (+0.9 KB, +0.14%) 476.6 KB (+0.7 KB, +0.15%)
playcanvas.min.mjs 2387.2 KB (+2.1 KB, +0.09%) 613.2 KB (+0.8 KB, +0.13%) 476.5 KB (+1.1 KB, +0.24%)

@mvaligursky mvaligursky added the area: graphics Graphics related issue label Aug 27, 2026

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated PR review by Codex (GPT-5) at exact head 51149c88040e4acdb0e9f0db38d30867e9178233.

This is a thoughtful redesign: the count/error frontier, no-allocation runtime queue, count-monotone underfill path, malformed-metadata fallback, and focused tests are all strong. I found three actionable implementation/compatibility issues and one public documentation mismatch. The change to splatBudget's default and zero semantics is also a breaking behavioral change and should be handled alongside the public accessor removals in the release/versioning decision.

Additional P3 documentation note: the public lodRangeMin and lodRangeMax descriptions still say the optimal LOD is selected by distance. Both should describe clamping the new global error/budget allocator's selected chain instead.

Verification:

  • Reviewed the full 31-file diff and the surrounding world update, streaming, placement, range, and allocator lifecycle paths.
  • Focused LOD table/balancer tests: 33 passing.
  • Full suite: 2481 passing, 2 pending.
  • ESLint passed for the changed core implementation and new tests.
  • Added a minimal allocation reproduction for a non-concave authored error curve; the current allocator spent 80/120 splats with residual error 10 while a feasible 120-splat choice had residual error 4.
  • Ran the updated authored-error lod-streaming example in the browser on WebGPU: it loaded and rendered cleanly at 3.953M splats under its 4M budget, with no warning/error logs.
  • git diff --check passed, and all GitHub build, lint, type, unit-test, docs, examples, API, size, and deployment checks are green.

const fineLod = scratch[i];
const cost = lods[fineLod].count - lods[coarseLod].count;
const benefit = lods[coarseLod].error - lods[fineLod].error;
previousRatio = Math.min(previousRatio, benefit / cost);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Build the concave frontier instead of clamping marginal returns

A Pareto frontier does not guarantee non-increasing marginal benefit, and replacing an increasing successor ratio with the running minimum loses the value of buying the required upgrades as a compound step. For example, one node with (count,error) levels (20,10) -> (90,8) -> (100,0) gets both ratios clamped to 2/70, although the direct coarse-to-fine upgrade removes 10 error for 80 splats (0.125). Against another 20 -> 60 node removing 4 error (0.1) with a total budget of 120, the current balancer buys the second node, stops at 80 splats, and leaves residual error 10; choosing the first node's full 80-splat upgrade fits exactly and leaves residual 4. Authored error curves are not guaranteed concave, so this can defeat the stated quality objective substantially. Please reduce each node to the appropriate upper concave hull (or pool adjacent increasing-slope upgrades into compound steps) before emitting the chain, and add this shape to the allocator tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5bb9fd5, though not via the hull in the end: pooling levels out of the chain removed feasible render/stream states (20-25% of Pareto levels on measured captures, every one a strict error improvement), so instead each step is priced by the best run reachable from its start — max over reachable levels of Δerror/Δcost — while the node still climbs one level at a time. Your (20,10)→(90,8)→(100,0) example prices at 0.125 and outbids the 0.1 rival at budget 120; measured 1-17% lower residual than the running-min clamp across four captures. The shape is covered in the allocator tests ('buys a compound upgrade that beats a cheaper rival outright').

getLodTable(rangeMin, rangeMax) {
let table = this._lodTable;
if (!table || table.rangeMin !== rangeMin || table.rangeMax !== rangeMax) {
table = new GSplatLodTable(this, rangeMin, rangeMax);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Avoid rebuilding tables for supported per-instance ranges

lodRangeMin/lodRangeMax are per placement, so two instances of the same octree can legitimately use different ranges. With this single cache slot, every LOD cycle alternates ranges in resolveLodRange(), then the balancer calls getLodTable() for both again: two instances produce four full table builds per update, and multiple duplicate typed-array tables remain referenced by the instances and balancer. Each build is proportional to nodes × levels and allocates both maximum-size and trimmed arrays, so large streamed captures will incur repeated allocation/GC hitches during camera movement. The debug warning acknowledges the case but does not make a supported component configuration safe. Cache active tables by range, or let each instance retain its table and have the balancer consume that resolved table, rebuilding only when that instance's configured range actually changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 6e54709 (with 603598d fixing the key): tables are per range, reference counted by the instances holding them, and the balancer consumes the table resolveLodRange() already resolved (inst.lodTable) instead of resolving again — so two instances on different ranges build each table once and share thereafter.

Comment thread src/framework/components/gsplat/component.js
The typedef is the only symbol in gsplat-unified/ that carried documentation
without an @ignore, so excludeNotDocumented let it through and the plugin pulled
it in via GSplatComponent#resource -> octree -> nodes -> lods. Its four existing
properties were already public; adding a fifth made that visible.

Also drops the {@link GSplatOctree#lodErrorSource} reference, which could not
resolve because GSplatOctree itself is undocumented.
Two review findings on the LOD selection table.

The Pareto frontier does not guarantee non-increasing marginal returns - the
slopes between consecutive levels can rise towards the finer end - and clamping
each ratio to the running minimum understated the compound step that was
actually on offer. For levels (20,10) -> (90,8) -> (100,0) both ratios clamped
to 2/70, hiding a 10-error reduction available for 80 splats at 0.125, so a
cheaper but less valuable upgrade elsewhere could win the budget and strand the
node at its coarsest level.

Reduce each node to the upper concave hull over (count, -error) instead, pooling
levels below a chord into one compound upgrade. Slopes are then non-increasing by
construction and the clamp is gone. This binds on 9-27% of upgrades on captures
with authored errors, and never on derived ones, which are concave already.
Residual error improves by 1-14% against authored errors as ground truth.

Separately, lodRangeMin/lodRangeMax are per placement, so instances of one octree
can differ. Holding a single table made each request rebuild the other's, four
full builds per update for two instances. Keep a bounded table per range again,
and have the balancer read the table resolveLodRange() already resolved rather
than resolving it a second time.

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated re-review by Codex (GPT-5) at 8690339030d2edbe99763f7cea6f94918b07528a.

The follow-up correctly removes the running-minimum distortion in the previously reported 120-splat case and makes the balancer consume each instance's already-resolved table. However, I found two remaining issues in those fixes:

  • Removing below-chord Pareto levels assumes splat upgrades are divisible. They are discrete, so an intermediate can be the best—and only—upgrade that fits a smaller budget. Removing it also makes the shared streaming navigation skip useful loaded/prefetch levels.
  • The four-entry FIFO table cache still rebuilds every live range on every update as soon as five distinct per-instance ranges coexist.

Non-blocking documentation cleanup: GSplatLodTable's class/property comments still describe single-level upgrades and running-minimum clamping, while the implementation now emits compound hull steps. The public lodRangeMin/lodRangeMax descriptions also still say selection is distance-driven. Per instruction, I did not reassess or repeat the separate deprecation-cycle concern.

Validation:

  • Focused LOD table/balancer suite: 36 passing.
  • Full suite: 2484 passing, 2 pending.
  • Full ESLint and git diff --check: pass.
  • Targeted discrete-budget reproduction: chain becomes [2, 0]; at budget 90 the allocator spends 20 with residual error 10 despite LOD 1 fitting exactly at 90 with residual error 8. The same chain cannot select loaded LOD 1 for underfill and prefetches directly from LOD 2 to LOD 0.
  • Targeted cache reproduction: five live ranges with capacity four produced five identity misses/rebuilds per subsequent pass.
  • All GitHub CI, API, build-size, docs, type, examples, test, and deployment checks are green.

}
scratch[hullCount++] = lod;
}
frontierCount = hullCount;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve feasible intermediate LODs in the discrete allocator and streaming chain

A point below the chord is redundant only for a fractional allocation; these LODs are indivisible. With this file's own (count,error) example (20,10) -> (90,8) -> (100,0), a budget of 90 can render the middle level exactly, but this compaction emits [20,100], so the balancer remains at 20 splats/error 10 and leaves 70 splats unused instead of choosing error 8. Because findCoarserAccepted and finerOnChain use this same compacted chain, an already-loaded 90-splat level is also unavailable for underfill and prefetch jumps directly from 20 to 100. Please keep a count-monotone Pareto chain for feasible/streaming states and represent pooled allocator offers separately (or otherwise handle affordable prefixes), with tests for a budget smaller than the compound endpoint and for a loaded pooled-out intermediate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5bb9fd5: the hull no longer prunes the chain. The full count-monotone Pareto chain is what the allocator, streaming, underfill and prefetch all walk (only dominated levels drop, ~2-4% on measured captures), and the compound value lives purely in the per-step pricing.

// Evict the oldest rather than growing without bound - Map iterates in insertion order.
// The cap only needs to cover the ranges live at one time, which is a handful of quality
// presets in practice.
if (this._lodTables.size >= MAX_LOD_TABLES) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not evict tables that are still used by live instances

The fixed cap reintroduces the rebuild loop as soon as five legitimate per-placement ranges are live. After warming ranges A-E, the map contains only B-E; resolving A evicts B, resolving B evicts C, and so on, causing all five node×level tables and their typed arrays to be rebuilt every LOD update. A five-range reproduction produces five cache misses on every subsequent pass. The debug warning only diagnoses the resulting allocation/GC hitch. Please retain all currently referenced ranges (for example with per-range instance refcounts and eviction of inactive entries), rather than using a FIFO cap that can evict live tables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 6e54709: the FIFO cap is gone in favour of reference counting — acquireLodTable/releaseLodTable, with an instance releasing its previous table on range change and on destroy. A table lives exactly while some instance is on its range; tests cover more live ranges than the old cap held.

A fixed cap can evict a table an instance is still using: with more live ranges
than the cap holds, every LOD update misses on all of them and rebuilds. The cap
existed to stop tables being retained for ranges no longer in use, which
reference counting answers directly.

acquireLodTable/releaseLodTable replace getLodTable. An instance takes a
reference when its range changes and releases the previous one, and releases on
destroy, so a table lives exactly as long as some instance is on its range - no
retention for dead ranges, and nothing live can be evicted however many ranges
are in play.
Reducing each node to the concave hull priced compound steps correctly but
deleted the levels in between, and those levels are not redundant: 20-25% of
Pareto levels went, affecting 70-86% of nodes, and every one of them strictly
reduces error - median 15% against the level below it. They are also the states
streaming and underfill step through, so a node could no longer show an
already-loaded intermediate, and prefetch's one-level climb became a
multi-level jump - 12 to 229 splats in one step on a parish_03 node.

Keep the full Pareto chain and move one level at a time, dropping only levels
dominated in both count and error, which is 4%. Value a step by the best deal
reachable by carrying on from where it starts rather than by its own slope, so a
step that is poor alone but opens an excellent run competes on what it is worth.
That recovers the mispricing the hull was introduced for - measured end to end
through the real balancer, residual error is 1-17% below the previous
running-minimum pricing - without removing anything.

Values are no longer monotone along a chain, so requeueing a successor is capped
at the bucket being drained. Every update re-floors from the cheapest level, so a
node pushed above the sweep would be dropped on every update rather than merely
delayed, and could never finish the run it started.
The example loads any capture through the url hash parameter and some span
kilometres, where the default far clip of 1000 cuts the distant content. Because
the far plane cuts on view-space depth, a distant node vanishes when looked at
head-on and returns when it moves off to the side, so on a 26km capture the sky
reads as patches popping around the horizon rather than as a clipped horizon.

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated in-depth re-review by Codex (GPT-5) at exact head da3f2a0ad1ff6820a0663f6e415bd050ab9c948b.

The latest commits correctly fix the two findings from the previous review: the full Pareto chain is retained for discrete allocation/streaming, and LOD tables are now reference-counted by live instances instead of using an evicting cap. I re-audited those implementations from scratch, including range changes, instance/world destruction, device loss, underfill, and prefetch; their normal ownership and navigation invariants look sound.

I found three remaining issues: the new best-reachable-run price can purchase a poor prefix even when the suffix that justifies its price cannot fit; orthographic cameras are still evaluated with a perspective distance formula; and the numeric table-cache key aliases ranges above 255. The first two affect selection quality in supported runtime scenarios; the cache issue is a lower-frequency correctness edge.

Non-blocking documentation cleanup: the PR description still explains the superseded running-minimum precomputation, and the public lodRangeMin / lodRangeMax descriptions still say selection is distance-driven. Per instruction, I did not reassess or repeat the separate deprecation-cycle concern.

Verification:

  • Reviewed the complete 31-file base-to-head diff and all six commits, not only the follow-ups.
  • Focused LOD table/balancer suite: 38 passing.
  • Full suite: 2486 passing, 2 pending.
  • Full ESLint and git diff --check: pass.
  • Reproduced the unaffordable-prefix allocation on the current classes and exhaustively enumerated its feasible assignments: current result is 110 splats / residual error 12; a feasible 80-splat assignment has residual error 10.
  • Reproduced the cache alias on the current GSplatOctree: requesting [0, 300] then [1, 44] returns the same [0, 300] table.
  • Checked the coverage derivation against the engine camera's projection-specific screen-size behavior; orthographic screen size is explicitly distance-independent.
  • All current GitHub build, lint, type, unit-test, docs, examples, API, size, and deployment checks are green.

let ratio = 0;
for (let j = i; j < frontierCount; j++) {
const reach = scratch[j];
const r = (lods[coarseLod].error - lods[reach].error) /

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not price a prefix using a suffix that may not fit

This ratio ranks only the next adjacent upgrade, but it can include the error reduction of a farther endpoint without checking that the cumulative run is affordable. For the documented chain (20,10) -> (90,8) -> (100,0) plus a rival (20,4) -> (60,0), equal coverage and budget 110, the floor costs 40. This code prices the first 70-splat step at 10/80 = 0.125, ahead of the rival's 4/40 = 0.1, so the balancer buys it and reaches (90,8); the 10-splat suffix then does not fit and the early exit leaves residual error 8 + 4 = 12. Buying the rival instead is feasible at total cost 80 and leaves residual error 10 + 0 = 10. The existing test only uses budget 120, where the whole run fits, and exactGreedy repeats this same pricing assumption. Please make the run whose value is used an atomic/affordability-aware purchase, or otherwise rank a prefix only by quality that is actually committed under the remaining budget, and add the 110-boundary regression case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified — at budget 110 this leaves residual 12 vs the optimal 10, and at 100 the hard stop leaves 14 vs 10. We built the affordability-aware drain (atomic affordable runs, demotion to the best fitting run, pass-over when nothing fits) and measured it across four captures: ~0.2% aggregate residual improvement, +13-25% drain time from the per-pop chain walk, and the pass-over semantics measured equal-or-worse temporal churn on 3 of 4 assets — it relaxes the hard early exit we keep deliberately for camera-motion stability. Decision: keep the simple drain and accept the boundary; both cases are pinned in tests with this rationale (603598d). Happy to revisit if a real capture surfaces the boundary visibly.

// well-defined, lowest-possible priority rather than a value the allocator has to
// special-case.
const radius = nodes[nodeIndex].boundingSphere.w;
const projectedRadius = radius / Math.max(radius + fovAdjustedDistance, 1e-12);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle orthographic coverage without distance attenuation

This is the perspective projected-radius formula, but it is used for every camera projection. Under an orthographic camera, two equal-radius nodes occupy the same screen area regardless of depth (Camera#getScreenSize uses radius / orthoHeight), while this expression gives the farther node lower coverage and therefore less LOD budget; moving the camera along its view axis also reshuffles quality even though neither node's rendered footprint changes. Orthographic cameras are supported by the engine and GSplat examples, so branch on camera.projection and derive the orthographic coverage without fovAdjustedDistance (with a focused test for equal-size nodes at different depths).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 603598d: coverage branches on camera.projection — orthographic uses min(radius / orthoHeight, 1)², depth-independent, mirroring Camera#getScreenSize, with the behind-camera penalty still applied so invisible content cannot win budget; FOV compensation is now perspective-only. Tests cover equal-size nodes at different depths, dolly invariance, window sizing and the behind penalty.

* @returns {GSplatLodTable} The selection table, with its reference count incremented.
*/
acquireLodTable(rangeMin, rangeMax) {
const key = rangeMin * 256 + rangeMax;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] Use a collision-free key for the LOD range

rangeMin * 256 + rangeMax is only unique while rangeMax < 256, but neither the manifest parser nor resolveLodRange() imposes that bound. For example, [0, 300] and [1, 44] both produce key 300; on the current code, acquiring them in that order returns the [0, 300] table for the [1, 44] request. That also defeats resolveLodRange() because acquiring the aliased table and releasing the previous reference leaves the instance on the wrong range. Please use a collision-free pair representation (for example, a string key or nested maps), and use the same representation in releaseLodTable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 603598d: string key ${rangeMin},${rangeMax} in both acquireLodTable and releaseLodTable, with a regression test for the [0,300] / [1,44] alias.

Coverage used the perspective projected-radius formula under every projection,
so an orthographic camera gave equal-size nodes less LOD budget the deeper they
sat, and moving along the view axis reshuffled quality while no footprint
changed. Branch on the camera's projection: orthographic coverage is the radius
against the ortho window, depth-independent and mirroring Camera#getScreenSize,
with the behind-camera penalty still applied so invisible content cannot win
budget. FOV compensation is perspective-only.

The LOD table key packed the range as rangeMin * 256 + rangeMax, which aliases
pairs once rangeMax passes 256 - nothing bounds lodLevels or the configured
range - silently handing an instance a table for the wrong range. Key by a
string in both acquire and release.

Also pin the allocator's boundary behaviour where a step's rank is justified by
a run the remaining budget cannot finish. An affordability-aware drain that
resolves those boundaries optimally was built and measured: ~0.2% aggregate
residual improvement across four captures, for a per-pop chain walk and a
relaxed early exit whose pass-over semantics measured equal-or-worse temporal
churn. Traded away for the simple drain and the hard early exit; the accepted
outcomes are pinned in tests with the rationale.

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated follow-up review by Codex (GPT-5) at exact head 603598debc811498e4a21336f8b45638974d899f, incorporating the replies on all prior reports.

The replies materially change the assessment of the allocator boundary. The 110/100-splat outcomes are now explicitly accepted heuristic behavior, the tests pin both cases, and the author reports an implemented affordability-aware comparison showing only ~0.2% aggregate residual improvement at +13–25% drain time with equal-or-worse temporal churn on most measured assets. Given that evidence and the allocator's deliberate hard-stop stability objective, I consider that report resolved by an informed design decision rather than an outstanding defect.

The collision-free string cache key correctly fixes the reported range alias. The orthographic branch fixes depth attenuation, dolly invariance, ortho-window sizing, and the behind-camera weighting at identity scale, but it introduces one remaining unit-space error: local node radius is compared directly with world-space orthoHeight, so scaled placements receive the wrong coverage. I left one inline P2 for that case.

Non-blocking documentation remains stale: the PR description still explains the superseded running-minimum precomputation, and public lodRangeMin / lodRangeMax text still says selection is distance-driven. Per instruction, I again excluded the separate deprecation-cycle concern.

Verification:

  • Read and evaluated every new inline reply, then reviewed the complete five-file follow-up and its surrounding projection/transform paths.
  • Focused allocator/table/coverage suites: 46 passing.
  • Full suite: 2494 passing, 2 pending.
  • Full ESLint and git diff --check: pass.
  • Cache-alias regression is covered and passes.
  • Targeted transform reproduction: changing an orthographic placement from scale 1 to scale 2 leaves coverage at 0.12 instead of increasing it to 0.48 (4× screen area).
  • All current GitHub build, lint, type, unit-test, docs, examples, API, size, and deployment checks are green.

// clamped to a full-window 1 as the perspective ratio is bounded by 1. The behind
// penalty divides squared, matching how a penalized distance scales the far-field
// perspective coverage.
const projectedRadius = Math.min(radius * invOrthoHeight, 1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Convert the local radius to world units for orthographic coverage

radius comes from the octree-local bounding sphere, while camera.orthoHeight is a world-space window size. The perspective path is scale-correct because both radius and the inverse-transformed camera distance are local, but this orthographic ratio mixes spaces and never uses the uniformScale already computed above. In a direct current-code reproduction, a unit leaf at scale 1 produces coverage 0.12; changing its placement to uniform scale 2 still produces 0.12, although the projected radius doubles and coverage should become 0.48 before clamping. This also misranks otherwise identical octrees placed at different scales under the shared budget. Multiply the radius by the placement's world scale for this branch (equivalent to radius * uniformScale / orthoHeight) and add a non-identity-scale regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 7ab4243: the placement's uniform scale is folded into the precomputed inverse window height, so the ortho ratio is world-space (radius * uniformScale / orthoHeight). Regression test pins the scale-2 case — coverage quadruples, and it reproduces your 0.12-stuck value on the old code.

Node radii are octree-local while orthoHeight is a world-space window, so the
orthographic coverage ratio dropped the placement's scale - the same octree at
different scales ranked identically under the shared budget. The perspective
path is unaffected: its radius and distance are both local, so the scale
cancels in the ratio.

Fold the placement's uniform scale into the precomputed inverse window height,
outside the per-node loop, and pin it with a non-identity-scale regression test.

@mvaligursky mvaligursky left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated follow-up review by Codex (GPT-5) at exact head 7ab4243d4c3754b55b118948bb402b61e72a2450.

No findings in the final P2 fix. The commit correctly converts the octree-local radius into the orthographic camera's world-space window by folding the placement's accumulated uniform world scale into invOrthoHeight. This is done once outside the per-node loop, preserves the existing no-allocation hot path, includes parent/inherited scale through getWorldTransform(), and leaves the perspective calculation unchanged because its radius and camera distance remain in the same local space.

The new regression test avoids the coverage clamp and verifies the important invariant directly: doubling placement scale quadruples projected-area coverage. It reproduces the former 0.12 -> 0.12 failure and now produces the expected 0.12 -> 0.48 behavior.

Verification:

  • Reviewed the complete two-file commit and the surrounding perspective/orthographic transform paths.
  • Focused allocator/table/coverage suites: 47 passing.
  • Full suite: 2495 passing, 2 pending.
  • Full ESLint and git diff --check: pass.
  • Confirmed accumulated parent scale is returned by the world transform used by the fix.
  • All GitHub build, lint, type, unit-test, docs, examples, API, size, and deployment checks are green.

The previously reported P2 is resolved. Per instruction, the separate deprecation-cycle concern remains outside this review.

@mvaligursky
mvaligursky merged commit 32476ee into main Aug 28, 2026
10 checks passed
@mvaligursky
mvaligursky deleted the mv-gsplat-lod-error branch August 28, 2026 10:21
@Ben-Mack

Ben-Mack commented Aug 30, 2026

Copy link
Copy Markdown

Hi @mvaligursky , Is it possible to keep using the old distance-based LOD behavior?
I've tested 2.22.0-beta.25 which included this new error-based LOD, and in my scene, compared same scene/budget, it look more blurry than old distance-based version.
I think because the old distance based can be more guaranteed that if i move close to any chunk, that nearest chunk will have higher detail LOD loaded. But with this new error-based behavior, that is not guaranteed, so for some chunk that has fine details, but not high contrast, even though it is nearest to camera, it's still more blurry than before.
My naive guess is error-driven prefer to load higher detail LOD for chunk with high contrast details even if farther away, which because i guess high contrast details usually have higher error score so it is usually preferred to select finer LOD.
Unfortunately i can't share the scene, but i think keeping both behavior for a while should be safe.
Do you think it is possible?

@mvaligursky

Copy link
Copy Markdown
Contributor Author

Thanks for testing @Ben-Mack . What you say makes sense, and I think the best solution would a single slider (0-1 range, default to perhaps 0.5 for balanced), to give more priority between foreground and background. Thoughts?

@Ben-Mack

Ben-Mack commented Aug 30, 2026

Copy link
Copy Markdown

Would set distance priority to the max would give similar result to current distance-based? I want the behavior to be mainly based on distance, with error weighting as optional/bonus.

In my scene, error-driven is clear dowgrade in visual, because the old distance-based has a very useful behavior that neighbor chunks that at the same distance to camera tends to use the same LOD level, which minimize artifacts, cleaner visual because neighbors display at the same details, avoid artifacts/difference at chunk seams when mixing different levels.
Current error driven doesn't have this behavior, neighbor chunks even though same distance to camera, LOD selection for each chunk still mixed from various different LOD level, which look visibly worse, visual look not consistent, can see a high detail chunk beside low detail chunk.

@mvaligursky

Copy link
Copy Markdown
Contributor Author

Have you had a chance to try it with the latest splat-transform that generates the error metrics? That'd be a super useful feedback if you had, to see if you find this better in that respect.

@mvaligursky

Copy link
Copy Markdown
Contributor Author

I exposed new settings to allow distance based mode. See here @Ben-Mack
#9267

That is pretty close to the old behavior, and should suite your needs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: graphics Graphics related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants