Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add progressive rendering #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sxxov
Copy link

@sxxov sxxov commented Feb 4, 2024

Hi, thank you to the team at Zappar for this library!

Overview

It's really hard to get a user to wait potentially 10s+ for a full splat file to download before anything shows up. This PR adds functionality to the library to progressively display splats as they are streamed from the server.

Breaking!

This PR changes the signature of GaussianSplatMesh.load() to accept a config object (defined in GaussianSplatGeometry.load(url, config)) instead of only a loadingManager.

Alternatives

I did consider a few alternatives, but since this library just hit 0.1.0, I figured it wouldn't be too late to add to GaussianSplatMesh.load() itself! (::

If the maintainers decide otherwise, please do edit/note in the review of the PR!

  • A GaussianSplatMesh.load() & a GaussianSplatMesh.stream() method
  • GaussianSplatMesh.load(url, loadingManager?, progressive?)

Concerns/Questions

  • Why is SplatLoader written in ES6 instead of modern JS? Considering the rest of the library is written modernly 😅. I followed that style nonetheless there, but it makes the code a little more clunky. In the future it could be changed to use the AsyncIterable directly from ReadableStream instead of recursively calling the processing function (as well as async/await hopefully xdd).

Miscellaneous Notes

  • A copy was saved between JS & WASM by removing the trimBuffer function that called slice internally. Instead, subarray was used to create another view to the underlying ArrayBuffer/SharedArrayBuffer
  • Speaking of SharedArrayBuffer, it's used to write to the worker's memory from the ReadableStream in JS. If I'm not mistaken this introduces a requirement of headers to be included in the top-level document (SharedArrayBuffer is still technically constructable without it, so I'm not sure where the library gets its instances, or the difference between the 2 imports in worker.ts mean 😅). I documented this & the change in signature in the README, & added some more info regarding setting up the headers.

* Breaking!
   * Changes the signature of `GaussianSplatMesh.load()`
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