Partition Gaussian chunks by a uniform grid before Morton-slicing them - #1254
Merged
Merged
Conversation
…cing them Chunk formation used to sort every splat into one scene-wide Morton order and slice it into fixed-count groups. On non-uniform captures that lets a chunk span the whole scene whenever a run of Morton- adjacent splats happens to cross a sparse region, producing sprawling, irregular chunk AABBs that inflate their own screen-space area and starve the per-chunk LOD rule's coarsening (density = splats / area). gridChunkPlan buckets the same Morton order into a uniform grid before slicing: a 63-bit Morton key already interleaves the coarsest bits on top, so a grid cell at 2^bitsPerAxis cells per axis is just the key's top bits — no separate cell sort needed. A chunk now ends at whichever comes first, the grid cell boundary or splatsPerChunk, so it never spans more than one cell. Validated against a real capture (Mip-NeRF 360 "garden", 3.33M splats): worst-case chunk diagonal dropped from 560m to 48m, max/median ratio from 719x to 35x, with average chunk occupancy staying close to the splatsPerChunk budget (no excess fragmentation). Known gap: no new tests added for the grid partitioning itself, and the existing golden-hash/exact-chunk-count tests (UntoldGSFormatTests, UntoldGSCookerEquivalenceTests) now fail because chunk layout legitimately changed. Needs a follow-up to add partition-specific test coverage and regenerate the golden fixtures before this can merge.
UntoldGSFormatTests and UntoldGSCookerEquivalenceTests pinned exact chunk counts, file sizes and SHA-256 hashes from the old fixed-count Morton chunking; those legitimately changed with gridChunkPlan, so the golden values are updated to match. LegacyGaussianCookPath (the frozen, test-only reimplementation of the pre-streamed-store writer, kept to validate windowed-vs-whole-array reading equivalence) had its own independent chunk-slicing loop that doesn't share code with the real writer. It needed the same uniform- grid cut mirrored into it, or it would silently diverge from the real writer's chunk layout and break the legacy/streamed equivalence checks for reasons unrelated to what those tests actually validate. UntoldGSFormatTests, UntoldGSCookerEquivalenceTests: 37/37 and 20/20 passing. GaussianChunkCullTest (UntoldEngineRenderTests) still has 31 failures against a real baked fixture with per-chunk index/order assertions tied to the old chunk layout — tracked separately, not covered by this commit.
The 200-splat fixture (16 splats/chunk) chunked to 13 by count alone before gridChunkPlan; the grid partition now also cuts a chunk at each of its 8 cells' boundary, chunking it to 16. Every assertion in this file derives from the two shared constants (expectedChunkCount, expectedVisibleChunkCounts), so updating those two resolves all 31 failures, including the per-chunk splat-ordering and HZB-occlusion assertions that looked independent but were downstream of the same stale counts. Full Gaussian test sweep (UntoldGSWriter/Format/Cooker, GaussianChunk*, GaussianRendering, GaussianProgressiveLOD): 103/103 passing, 2 skipped as before.
poolSlotCount estimated slot demand as ceil(assetBytes / slotBytes), which is only correct when at most the last chunk is partially filled. Grid-partitioned chunk formation (gridChunkPlan) breaks that assumption on purpose: a chunk ends at a grid-cell boundary or splatsPerChunk, whichever comes first, so several chunks can be short. Paging still allocates one physical slot per chunk regardless of fill, so summing bytes under-counts how many slots are actually needed once more than one chunk is short — a 200-splat/16-chunk fixture only got 13 slots, leaving 3 chunks with nowhere to land and only 173 of 200 splats ever resident. poolSlotCount now takes an optional assetSlotCount, computed once from the chunk index already in memory as the sum of each chunk's own ceil(splatCount / ranksPerPage) — correct for multi-page chunks too, and identical to the old estimate for a normal asset where only the last chunk is partial, so this generalizes rather than changes the common case. Threaded through the one production call site (GaussianChunkLoader's paged load). GaussianPagingTest's expectations (13 chunks -> 16) were staged from diagnosing this; they're kept as the regression coverage, not reverted, since they're what exposed the bug. GaussianPagingPolicyTests gains a direct unit test of the new parameter. GaussianPagingPolicyTests, GaussianPagingTest: 27/27, 39/39 passing.
untoldengine
marked this pull request as ready for review
September 26, 2026 16:50
…itioning GaussianChunkLevelTest hardcoded splatCount: 1024 (and 1024 / area for density) throughout its density/level mirrors, assuming every chunk of the levelledSlabURL fixture is a full, uniform-size chunk. gridChunkPlan breaks that: the slab's splats are placed uniformly at random, so its grid partition leaves many chunks short, and mirroring density off a fixed splat count no longer matches what the real per-chunk splatCount drives. testOutgoingWindowsAreGrantedFromThePreCommitState and testSwitchIsCoverageFading now go through new mirrorDensities/ levelsByChunk helpers that read each chunk's own splatCount from the index, same as the kernel does. GaussianScreenWeightedQuotaTest.testTheChunkYouStandInIsKeptWhole asserted every chunk reaching behind the eye keeps the same *fraction* of its own count. That was only true because every chunk was the same size; with variable-size chunks the shared invariant is the same *quota* (already asserted separately, and still true), not the same fraction of chunks that no longer share a size. Also: GaussianSyntheticAsset's cached fixtures (GaussianSyntheticAsset-300000-v2*.untoldgs in the temp directory) are keyed by splat count and coarse-level options only, not by writer algorithm — a fixture cached before gridChunkPlan landed stays cached and silently masks these tests locally (which is what happened here: local runs were green against a stale cache while CI, starting fresh each run, correctly failed). No code fix for that — deleting the two cached files reproduces CI's real result locally. Full make testrenderer (1152 tests, matching CI's scope): 0 failures.
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
density = splats / area)gridChunkPlanbuckets the same Morton order into a uniform grid before slicing: a 63-bit Morton key already interleaves the coarsest bits on top, so a grid cell at2^bitsPerAxiscells per axis is just the key's top bits — no separate cell sort needed. A chunk now ends at whichever comes first, the grid cell boundary orsplatsPerChunkwriteStorereworked to carry variable per-chunk sizes through chunk-count derivation, payload offsets, and the encode loopsplatsPerChunkbudgetTest fixes
UntoldGSFormatTests,UntoldGSCookerEquivalenceTests: golden chunk counts/file sizes/SHA-256 hashes regenerated to match the new (legitimately different) chunk layoutLegacyGaussianCookPath(frozen, test-only reimplementation of the pre-streamed-store writer, used to validate windowed-vs-whole-array reading equivalence) had its own independent chunk-slicing loop; mirrored the same uniform-grid cut into it so it stays comparable to the real writerGaussianChunkCullTest(real baked fixture, 200 splats/16 per chunk: 13 chunks → 16 once the grid partition also cuts at cell boundaries): updated the two shared constants (expectedChunkCount,expectedVisibleChunkCounts) that every assertion in the file derives fromRemaining gap
No new, dedicated test asserting the grid-partition invariant itself (a chunk never spans more than one grid cell) — the above fixes make existing tests pass against the new layout, but don't add fresh coverage for the partitioning logic in isolation.
Test plan
swift buildsucceedsUntoldGSFormatTests,UntoldGSCookerEquivalenceTests,GaussianChunkCullTest: all green