Skip to content

Conversation

@mrxz
Copy link
Collaborator

@mrxz mrxz commented Oct 24, 2025

While looking into #192 and #193 I noticed the reference counting of SplatAccumulators wasn't always correct. There were several ways it could go wrong, resulting in a range of issues. Luckily the resulting changeset is small, but tracking them down wasn't so easy. With these changes the ref counting should work correctly. There's still some possibilities for viewpoints to keep hold of accumulators starving the SparkRenderer.

The following changes were made:

  • Initialize the first accumulator with a refCount of 1, as the SparkRenderer immediately uses it as its active accumulator
  • Determine originChanged between current viewToWorld and the viewToWorld of the newly retrieved accumulator. Previously this compared to the active accumulator, but the one retrieved from the pool might be older
  • Let SparkViewpoint keep track of its two references (pending.accumulator and display.accumulator). Before it kept a maximum of 1 reference, but since sorting happens asynchronous there could be in-flight sorting operations using an accumulator that was already released.
  • When the new accumulator has correspondence with the previous one, also update the viewToWorld and trigger a prepareViewpoint. Otherwise there'd be a window for a SparkViewpoint to have mismatched accumulator and viewToWorld

@mrxz
Copy link
Collaborator Author

mrxz commented Oct 24, 2025

@manuel-kroeter FYI, this should fix both #192 and #193. Thanks for the reproduction and details, made tracking it down easier. Let me know if it resolves the issues for you.

@dmarcos dmarcos merged commit 8f90fe0 into sparkjsdev:main Oct 24, 2025
2 checks passed
@dmarcos
Copy link
Contributor

dmarcos commented Oct 24, 2025

Thank you!

@manuel-kroeter
Copy link

manuel-kroeter commented Oct 24, 2025

Awesome, thank you! I also spent some time the last days to find the issue but was a bit hard without knowing all the internals. I did some quick testing and it looks good so far 👍

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.

3 participants