Skip to content

Conversation

@mrxz
Copy link
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
Contributor

dmarcos commented Oct 29, 2025

Thank you!

@dmarcos dmarcos merged commit ad3a9bd into sparkjsdev:main Oct 29, 2025
2 checks passed
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