Skip to content

Fix ref counting and originToWorld in SparkViewpoint.prepare#207

Merged
dmarcos merged 1 commit intosparkjsdev:mainfrom
mrxz:fix-viewpoint-prepare
Oct 29, 2025
Merged

Fix ref counting and originToWorld in SparkViewpoint.prepare#207
dmarcos merged 1 commit intosparkjsdev:mainfrom
mrxz:fix-viewpoint-prepare

Conversation

@mrxz
Copy link
Copy Markdown
Collaborator

@mrxz mrxz commented Oct 28, 2025

See #205

The prepare method of SparkViewpoint increased the refCount of the active accumulator in case it wasn't used in the viewpoint's display state. However the display reference is already covered, resulting in double counting this reference. At the same time there's no guarantee the viewpoint holds any reference when sortUpdate is called. To resolve this a temporary reference is held.

Additionally there was a bug where the prepare method would default to not providing an originToWorld matrix. This causes the SparkRenderer to use the current toWorld of the active accumulator. This works as long as the SparkRenderer hasn't been moved, but renders incorrectly in case it did. See following scenario:

// Move camera with spark renderer
camera.position.x += 10.0;
await viewpoint.prepare({ scene, camera }); // Prepares for an outdated SparkRenderer position
renderer.render(scene, camera); // Triggers a new update, renders incorrectly

One thing that remains tricky with regards to prepare is that it directly calls sortUpdate, ignoring any ongoing sort operation or pending sort operation. This makes it possible for sortUpdate to throw an error, breaking the ref counting.

@dmarcos
Copy link
Copy Markdown
Contributor

dmarcos commented Oct 29, 2025

Thank you!

@dmarcos dmarcos merged commit ad3a9bd into sparkjsdev:main Oct 29, 2025
2 checks passed
asundqui added a commit that referenced this pull request Apr 9, 2026
- 296eaee: Copy originToWorld values instead of holding a reference in SplatAccumulator (#191)
- 1ecabe0: Interactive ripples effect (#194)
- 589b6bc: Fix build_rust_wasm.sh script (correct shebang, don't require rustup) (#197)
- 018a3ed: (already merged) Support logarithmic depth buffer (#199)
- 8f90fe0: Fix reference counting for SplatAccumulator during updates (#200)
- 18a33cb: portal effect (#203)
- ad3a9bd: Fix ref counting and originToWorld in SparkViewpoint.prepare (#207)
- 0d9f7bf: fix twister & rain effects (#211)
- c9c488c: add greyscale painting and add i/o (#217)
- 937262b: Interactive deform effect with bounce (#223)
- 196721e: Add Demo Splat Dissolve Effects (#218)
- 752cfaa: fix: resolve webpack/Next.js WASM data URL compatibility issue (#95) (#231)
- 558a9a9: refactor: use magic-string for proper source maps in WASM fix (#234)
- 237b181: Rename single character variable
- fa71609: Add three as peerdependency (fix #238)
- d6d5e7e: Ensure splatDefines ShaderChunk is ready when using Readback before SparkRenderer (#240)
- 10a7541: add undo brush to splat painter example (#241)
- a573987: Dispose of spherical harmonics arrays and packedArray in PackedSplats (#239)
- dec8c4e: Add reference to Spark 2.0 preview in README
- 1a8239b: Add Spark 2.0 example links to README
dmarcos pushed a commit that referenced this pull request Apr 14, 2026
- 296eaee: Copy originToWorld values instead of holding a reference in SplatAccumulator (#191)
- 1ecabe0: Interactive ripples effect (#194)
- 589b6bc: Fix build_rust_wasm.sh script (correct shebang, don't require rustup) (#197)
- 018a3ed: (already merged) Support logarithmic depth buffer (#199)
- 8f90fe0: Fix reference counting for SplatAccumulator during updates (#200)
- 18a33cb: portal effect (#203)
- ad3a9bd: Fix ref counting and originToWorld in SparkViewpoint.prepare (#207)
- 0d9f7bf: fix twister & rain effects (#211)
- c9c488c: add greyscale painting and add i/o (#217)
- 937262b: Interactive deform effect with bounce (#223)
- 196721e: Add Demo Splat Dissolve Effects (#218)
- 752cfaa: fix: resolve webpack/Next.js WASM data URL compatibility issue (#95) (#231)
- 558a9a9: refactor: use magic-string for proper source maps in WASM fix (#234)
- 237b181: Rename single character variable
- fa71609: Add three as peerdependency (fix #238)
- d6d5e7e: Ensure splatDefines ShaderChunk is ready when using Readback before SparkRenderer (#240)
- 10a7541: add undo brush to splat painter example (#241)
- a573987: Dispose of spherical harmonics arrays and packedArray in PackedSplats (#239)
- dec8c4e: Add reference to Spark 2.0 preview in README
- 1a8239b: Add Spark 2.0 example links to README
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.

2 participants