fix(webgpu): reset batchers when a frame is abandoned - #1609
Merged
Conversation
`abandonFrame()` finishes the command encoder without submitting, then frees every texture retired during that frame — correct, since the draws referencing them died with the buffer. But the batchers were left untouched, and a quad batcher's segment entries hold `GPUTextureView`s into exactly those textures. The next frame could then compose a bind group over destroyed resources. Reached whenever a frame is abandoned after a texture was replaced or unloaded mid-frame; a stage switch freeing the previous scene's assets is the ordinary way in. Every registered batcher is now reset before the retired textures are freed. That also drops the dead frame's queued vertices, current effect and material, and composed bind groups, none of which should replay into the next frame. Found by an adversarial review of an unrelated change, and present on master since the backend landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
None of those three entries were API changes. Two were consequences of what the release ADDED — a scene that already set one of the six modes now renders it — and the third records that 3D mesh rendering is UNCHANGED, which is the opposite of a change. The two substantive caveats, the per-draw capture and composite cost and the drawMesh fallback, move onto the Added entry they belong to. `### Changed` is for user-facing API changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
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
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.
abandonFrame()finishes the command encoder without submitting, then frees every texture retired during that frame — correct in itself, since the draws referencing them died with the buffer:But the batchers were left untouched, and a quad batcher's
segmentEntriesholdsGPUTextureViews into exactly those textures. The next frame'scomposeSegmentGroupcould then build a bind group over destroyed resources.How it's reached
Any frame abandoned after a texture was replaced or unloaded mid-frame. A stage switch freeing the previous scene's assets is the ordinary way in —
destroyTextureand theGPU_TEXTURE_CACHE_RESEThandler both retire textures, and the batcher keeps its segment entries until its next flush, which an abandoned frame never reaches.The normal submit path is safe:
flush()resets the segment beforequeue.submit()frees anything. Only the abandon path skips that.Fix
Reset every registered batcher before the retired textures are freed. That also drops the dead frame's queued vertices, current effect and material, and composed bind groups — none of which should replay into the next frame.
Tests
Three, using the established stub pattern in
webgpu_mesh_depth.spec.js:destroyRetiredTextures(), asserted on call order — getting this backwards frees the textures while entries still reference themabandonFrametest, updated for the newbatchersdependencyTwo mutations, each caught: removing the reset (2 failures) and reordering it after the destroy (1).
Provenance
Found by an adversarial review of #1608, which flagged it as pre-existing and memo-neutral. Present on master since the backend landed; not a regression from any current PR.
Full suite 6331 passing / 262 files, root lint 0 errors.
🤖 Generated with Claude Code
https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N