Skip to content

Fix paged root eviction issue - #451

Open
asundqui wants to merge 6 commits into
sparkjsdev:mainfrom
asundqui:fix-paged-root-eviction
Open

asundqui wants to merge 6 commits into
sparkjsdev:mainfrom
asundqui:fix-paged-root-eviction

Conversation

@asundqui

Copy link
Copy Markdown
Contributor

This PR builds on top of #449 and fixes the issue labeled P2 from #428 . Only the last commit in this PR is stacked on top of #449. Once that's merged I will rebase this on main. The delta can be seen here: asundqui/spark@fix-paged-lod-browser-test...asundqui:spark:fix-paged-root-eviction

When the splat page table is full, if a paged splat is hidden or removed and its root (page 0) is evicted, we fail to clear its internal rootPage. As a result, if another splat object loads new data into that same page and the old splat becomes visible again within the 3 sec LoD disposal timeout, it will traverse from that same page and show incorrect data. The fix is simply to clear the rootPage and the displayed splats for that object if it's evicted:

           } else if (!lodTree && chunk === 0 && record.rootPage === page) {
              // Root chunk evicted: forget the page (traversal skips this mesh
              // until the root is refetched) and stop drawing indices into
              // pages that no longer hold this mesh's data.
              record.rootPage = undefined;
              splats.clear();
            }

This PR also adds a new test case to paged-lod.test.ts that fails before this fix but now succeeds. It creates a page table with a capacity of 1, adds two SplatMeshes where only one is visible. Then it switches visibility to the other splat, and sets enableLodFetching = false which evicts the previous splat page but prevents the new one from loading. This creates a scenario where nothing should be rendered, but without this fix we see the root splat from the pre-existing data rendered.

In order for this test to run deterministically we also add an option SparkRenderer.lodCleanupTimeoutMs which previously was hardcoded to 3s. Setting it to Infinity allows the above test to detect the failure even if it takes longer than 3s for the scenario to settle.

Finally, in addition to providing an adjustable lodCleanupTimeoutMs this PR will dispose all LoD objects that exceed the timeout, rather than only one per LoD work cycle.

…ming

- Browser tests: Playwright + SwiftShader harness, exact pixel snapshots for basic, transforms, instances, LoD and paged LoD
- SplatPager/SparkRenderer: fire onDirty when a chunk lands, add pending(); portable setTimeout types
- CI: ci-browser.yml with HTML report artifact, CPU-only build-lod
- Docs/examples: on-demand rendering guide, example, onDirty option docs
- playwright.config: keep threshold 0, allow maxDiffPixelRatio 0.001 (65 px); CI showed 6-16 scattered pixel diffs vs macOS snapshots
- ci-browser.yml: bump actions to current majors (checkout/setup-node/upload-artifact v7, cache v6) to clear Node 20 deprecation warnings
- SplatPager: rename onDirty to onUpdate, clear it in dispose()
- Docs: onDirty may fire several times per frame; schedule a single render
- Tests: move unit test to test/unit (vitest --dir), rename browser specs to *.test.ts
- CI: upload report with !cancelled() instead of always()
- SparkRenderer: on root-chunk eviction, clear rootPage and indices so a re-shown mesh draws nothing until refetched
- SparkRenderer: lodCleanupTimeoutMs option (default 3000); cleanupLodTrees releases all expired trees
- SplatPager: add PagedSplats.clear()
- paged-lod test: root-eviction scenario with blank/left-alone snapshots
@asundqui asundqui self-assigned this Sep 23, 2026
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.

1 participant