Skip to content

Fix gsplat loading progress getting permanently stuck after a cancelled mid-flight load#8998

Merged
mvaligursky merged 3 commits into
playcanvas:mainfrom
slimbuck:load-dev
Jul 1, 2026
Merged

Fix gsplat loading progress getting permanently stuck after a cancelled mid-flight load#8998
mvaligursky merged 3 commits into
playcanvas:mainfrom
slimbuck:load-dev

Conversation

@slimbuck

@slimbuck slimbuck commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

Gsplat scene loading could freeze permanently (progress bar stuck short of 100%, autoRender never turning off) when a chunk's load was cancelled mid-flight — e.g. routine LOD range changes during scene traversal, which trigger frequent unload/reload cycles for prefetched and in-flight files. When this happens, SogParser resolves via the success callback with no resource (an intentional, silent cancellation signal, not an error), leaving the underlying asset marked loaded: true with resource: null. GSplatAssetLoader.load() treated loaded: true as permanently done regardless of whether a resource was ever produced, so it silently no-op'd on every future poll for that URL, and the file never loaded, even though nothing was actually broken about it — it just needed to be retried.
This was confirmed via live instrumentation on a repro scene: a prefetch entry sat indefinitely with loaded: true, resource: null, untracked by _currentlyLoading/_loadQueue/_retryCount, forever blocking pendingLoadCount from reaching zero.

Fix

GSplatAssetLoader.load() now detects a URL that resolved successfully but produced no resource, and retries it unconditionally whenever polled again, since this only happens while something still actively wants the file — cancellations are not failures and must not be silently permanent.
Genuine load errors (real network/parse failures) are tracked separately via a new _failed set, populated only once _onAssetLoadError's existing retry budget is exhausted, so a truly dead URL doesn't retry forever.
Also fixed a related bug in _onAssetLoadError's retry path: it called registry.load() directly without re-attaching an 'error' listener, so a second consecutive genuine failure had no listener left to catch it, meaning _failed could never actually be reached for a repeated real error. The retry path now re-attaches only the 'error' listener that was consumed by the previous attempt — it deliberately does not call _startLoading() (which would also re-attach 'load'), since the original 'load' listener from the first attempt is still pending and duplicating it would cause _onAssetLoadSuccess/_processQueue to run more than once on eventual success.

Test plan

  • Added test/framework/components/gsplat/gsplat-asset-loader.test.mjs covering: unbounded retry after cancellation, no retry after genuine failure exhausts retries, re-request after unload(), no duplicate 'load' listener across error retries, and no console warnings/errors on routine cancellation (mid-flight unload() before resolution).
  • Full engine test suite passes (1898 passing, 2 pending unrelated).
  • Manually reproduced the original stuck-loading bug on the affected scene, confirmed it no longer occurs across repeated reloads with this fix applied.

@slimbuck slimbuck requested review from Copilot and mvaligursky July 1, 2026 17:50
@slimbuck slimbuck self-assigned this Jul 1, 2026
@slimbuck slimbuck added the area: graphics Graphics related issue label Jul 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a gsplat chunk-loading edge case where mid-flight cancellations (success callbacks with no resource) could leave an Asset in a terminal loaded: true, resource: null state, permanently preventing future reloads and stalling overall loading/progress completion.

Changes:

  • Retry gsplat URL loads when a prior attempt “succeeded” but produced no resource (cancellation signal), while tracking genuinely permanent failures in a new _failed set.
  • Fix retry behavior in _onAssetLoadError to re-initiate loading in a way that re-establishes listeners for subsequent failures.
  • Add unit tests covering cancellation retries, permanent-failure behavior, unload/re-request behavior, and “late settle after unload” cancellation noise.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/framework/components/gsplat/gsplat-asset-loader.js Adds cancellation-aware retry behavior and permanent-failure tracking; adjusts error retry flow.
test/framework/components/gsplat/gsplat-asset-loader.test.mjs New tests validating cancellation retry, permanent failures, unload/reload, and no logging on routine cancellation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/framework/components/gsplat/gsplat-asset-loader.js Outdated
_onAssetLoadError's retry path called _startLoading(url), which
unconditionally re-attaches both 'load' and 'error' listeners. Since
the original 'load' listener from the first _startLoading() call is
still pending (it only fires on success), this duplicated it on every
retry, causing _onAssetLoadSuccess/_processQueue to run multiple times
on eventual success. Re-attach only the 'error' listener that was
actually consumed.

Addresses review feedback from PR playcanvas#8998.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread test/framework/components/gsplat/gsplat-asset-loader.test.mjs Outdated
Comment thread src/framework/components/gsplat/gsplat-asset-loader.js

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@mvaligursky mvaligursky merged commit 8f41866 into playcanvas:main Jul 1, 2026
8 of 10 checks passed
mvaligursky pushed a commit that referenced this pull request Jul 1, 2026
…ed mid-flight load (#8998)

* latest

* Avoid duplicate 'load' listeners on gsplat asset retry

_onAssetLoadError's retry path called _startLoading(url), which
unconditionally re-attaches both 'load' and 'error' listeners. Since
the original 'load' listener from the first _startLoading() call is
still pending (it only fires on success), this duplicated it on every
retry, causing _onAssetLoadSuccess/_processQueue to run multiple times
on eventual success. Re-attach only the 'error' listener that was
actually consumed.

Addresses review feedback from PR #8998.

* latest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: graphics Graphics related issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants