Skip to content

Request a render when a SplatMesh finishes loading - #457

Merged
dmarcos merged 5 commits into
sparkjsdev:mainfrom
asundqui:fix-on-demand-mesh-init
Sep 25, 2026
Merged

dmarcos merged 5 commits into
sparkjsdev:mainfrom
asundqui:fix-on-demand-mesh-init

Conversation

@asundqui

Copy link
Copy Markdown
Contributor

This PR builds on top of #456 , #452 , #451, and #449 and fixes the issue labeled D2 from the earlier draft PR #428. The last commit in this PR is stacked on top. Once the previous PRs are merged I'll rebase this onto main before merging. The delta can be seen here: asundqui/spark@fix-lod-dirty-stall...fix-on-demand-mesh-init

In on-demand rendering, a SplatMesh that finishes loading after the app's last render is never shown. Uninitialized meshes are already part of visibleGenerators (contributing 0 splats), but since we don't observe mesh.initialized, when the file lands the scene stays blank until something else triggers a render. This behavior matches the on-demand docs that lists "SplatMesh finished loading" as an onDirty trigger.

One new browser test was added, which fails before the fix and succeeds with it:

  • basic.test.ts: "shows a mesh that finished loading after the last render": holds back the .spz request, renders once while it is held, releases it, then settles without requesting a render. With the fix it drives the render loop and renders the splats, otherwise renders blank.

Normally Harness.settle() will await all SplatMeshes loading, so we add a new option:

  • waitForLoads (default true): Wait for all meshes' initialized promises before settling (disabled for this test)

The fix is in SparkRenderer.updateInternal(): the first time a visible SplatMesh is seen uninitialized, request a render when it finishes:

for (const generator of visibleGenerators) {
  if (
    generator instanceof SplatMesh &&
    !generator.isInitialized &&
    !this.initWatched.has(generator)
  ) {
    this.initWatched.add(generator);
    generator.initialized.then(
      () => this.setDirty(),
      () => {}, // load errors are reported by the mesh itself
    );
  }
}

initWatched is a WeakSet so disposed meshes are not retained, and setDirty() is a no-op when a render is already pending, so apps with a render loop see no extra frames. The fix is currently behind a temporary FIX_MESH_INIT_RENDER flag so reviewers can flip it to false to reproduce; it will be removed before merge, as with FIX_LATE_CHUNKS in #452 .

CI browser test result: https://github.com/asundqui/spark/actions/runs/35953568067

Note: @mrxz pointed out in #428 that perhaps it should be the user's responsibility to request a re-render when their splats finish loading using the splatMesh.initialized.then(() => requestRender()); pattern. However, I feel that not doing this automatically is an easy footgun to fix, with a very low cost of potentially rendering extra frames, one per loaded SplatMesh.

@asundqui asundqui self-assigned this Sep 24, 2026
Comment thread src/SparkRenderer.ts Outdated
this.initWatched.add(generator);
generator.initialized.then(
() => this.setDirty(),
() => {}, // load errors are reported by the mesh itself

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SplatMesh does not report load errors itself. Could the comment say what the handler prevents instead?

Suggested change
() => {}, // load errors are reported by the mesh itself
() => {}, // avoid an unhandled rejection when loading fails

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.

That's a good point, and the fact that it will suppress the "unhandled rejection" message is perhaps not good. They'd see the fetch error in the console, but they won't see the fact that they didn't handle it... Maybe not a deal breaker but let me explore other ways we could do this maybe with an event...

@asundqui
asundqui force-pushed the fix-on-demand-mesh-init branch from 4acd45a to 98fa2f2 Compare September 24, 2026 17:43
@asundqui

Copy link
Copy Markdown
Contributor Author

@oscarlorentzon I updated the PR to use an "initialized" event instead, so it no longer uses the promise chain and potentially alter what the user sees. Looks clean and tests successfully.

- SparkRenderer: move callback body to driveLodExclusive; finally requests a render if lodDirty, lodInitQueue or queued pager data remain
- Harness: settle() requestRender/ignorePendingLod options, waitUntil options object
- Tests: budget change during callback (lod), chunk landing during callback (paged-lod)
- ci-browser: also run on push to fix-lod-dirty-stall
- SparkRenderer: watch uninitialized visible meshes; initialized.then(setDirty) so on-demand apps show them (behind FIX_MESH_INIT_RENDER)
- Harness: settle() waitForLoads option
- basic test: mesh whose file lands after the last render
- ci-browser: also run on push to fix-on-demand-mesh-init
- SplatGenerator: "initialized" event map; SplatMesh dispatches it once splats are ready
- SparkRenderer: one-shot listener replaces initialized.then, so load failures stay visible
@asundqui
asundqui force-pushed the fix-on-demand-mesh-init branch from 321bbf3 to 81cffa0 Compare September 25, 2026 17:44
@asundqui

Copy link
Copy Markdown
Contributor Author

Rebased after #452 merge. All tests including browser tests still successful.

Comment thread src/SparkRenderer.ts Outdated
readback32 = new Uint32Array(0);

// Meshes seen while still loading; a render is requested when they finish.
private initWatched = new WeakSet<SplatMesh>();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nitpick: can be made readonly

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.

Okay will add.

Comment thread src/SparkRenderer.ts Outdated
Comment on lines +991 to +992
generator.removeEventListener("initialized", onInitialized);
this.setDirty();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The generator can be removed from the initWatched at this point:

this.initWatched.delete(generator);

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.

Okay!

@asundqui

Copy link
Copy Markdown
Contributor Author

@dmarcos addressed @mrxz 's comments and pushed here.

@dmarcos
dmarcos merged commit f2f9895 into sparkjsdev:main Sep 25, 2026
2 checks passed
@asundqui asundqui mentioned this pull request Sep 25, 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.

4 participants