fix(particles): upload CPU vertex buffer on rebuild/reset so paused emitters don't render garbage#8945
Merged
Merged
Conversation
Build size reportThis PR changes the size of the minified bundles.
|
CPU particle emitters (sort != NONE) fill their vertex buffer in addTime() but defer the GPU upload to finishFrame(), which the component system only calls while not paused. When a non-preloaded colorMap loads after the system is paused, the resulting rebuild()+reset() leave the freshly-allocated buffer unflushed, so the GPU keeps rendering the _allocate template data reinterpreted as the CPU vertex format - producing fullscreen-stretched billboards. Flush the buffer via finishFrame() after the initial fill in rebuild()/reset(). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
a403ee2 to
b8d0c11
Compare
mvaligursky
approved these changes
Jun 23, 2026
mvaligursky
reviewed
Jun 23, 2026
mvaligursky
left a comment
Contributor
There was a problem hiding this comment.
🤖 Automated PR review — posted on my behalf by Claude Code (Opus 4.8). Not a human review; flagging for awareness.
Verdict: correct and minimal. No blocking issues.
I traced the lock/upload lifecycle to confirm the fix is safe:
- In CPU mode,
addTime()(particle-emitter.js:1082) callsthis.vertexBuffer.lock()and deliberately leaves it unlocked (line 1084 is commented out) — the GPU upload is deferred tofinishFrame(). finishFrame()(line 1013) only unlocks whenuseCpu, so it's a genuine no-op for GPU particles. ✓VertexBuffer.unlock()(vertex-buffer.js:147) is stateless —lock()just returnsstorage, there's no lock flag — so calling it after theaddTime(0)fill simply uploads the current contents. Safe with no matching explicitlock(), exactly as the PR notes. ✓
So adding finishFrame() after the initial fill in both rebuild() and reset() correctly flushes the freshly-allocated CPU buffer instead of leaving the stale stride-4 quad template on the GPU for a paused emitter.
Other angles:
- Breaking changes: none.
- Performance: in the non-paused path this causes one redundant upload per
rebuild()/reset()(the frame's regularfinishFrame()uploads again). These are infrequent (asset load / property change), so it's negligible — worth being aware of but not worth guarding. - Tests: understandable that the
NullGraphicsDeviceheadless harness can't exercise this; in-browser verification against the #5993 repro is the right call. A line in CHANGELOG/PR noting the manual-verification-only status is already present.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
CPU particle emitters (
sort != PARTICLESORT_NONE) fill their vertex buffer inaddTime()but defer the GPU upload toParticleEmitter.finishFrame(), whichParticleSystemComponentSystem.onUpdateonly calls while!component._paused.When a colorMap texture with
preload: falsefinishes loading after the system has been paused, the resultingcolorMapsetter →_setComplexProperty→rebuild()+reset()leaves the freshly-allocated vertex buffer unflushed. The GPU keeps rendering the stride-4 quad template uploaded by_allocate(), reinterpreted as the 15-float CPU vertex format → garbage scale/positions → billboards stretched fullscreen on the first frame.Fix: flush the buffer with
finishFrame()after the initialaddTime(0)fill inrebuild()andreset(). It's a no-op for GPU particles and safe to call without a matchinglock()(the non-paused update loop already does so every frame).Verified in-browser against the issue's repro (autoPlay +
sort:2+ non-preloaded colorMap +pause()onsetTimeout(0)): the fullscreen stretch is gone and the playing path is unaffected. Existing particle-system tests pass. (The headless test harness usesNullGraphicsDevice, which disables particle systems, so the emitter vertex-buffer path can't be unit-tested there.)Fixes #5993
Checklist