webgpu: correct two defects in per-frame attribute buffer handling - #7
Open
shadowcodex wants to merge 1 commit into
Open
webgpu: correct two defects in per-frame attribute buffer handling#7shadowcodex wants to merge 1 commit into
shadowcodex wants to merge 1 commit into
Conversation
|
@shadowcodex is attempting to deploy a commit to the Eric Rowell's projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
Two defects in how a program's attribute buffers are handled within a frame. Both are reachable from one program that uploads more than once per frame — a sprite renderer drawing one atlas at a time is the everyday case.
They are in one PR because they are the same code path and the second is exposed by fixing the first: once a second upload appends instead of overwriting, the buffer grows mid-frame, which is exactly when destroying it becomes unsafe. Happy to split them if you would rather review them apart.
1. A second upload in a frame overwrites the first
A frame is one command encoder, submitted once at the end.
queue.writeBufferis ordered against that submit, not against the draw commands inside it. So every draw in the frame reads whatever was written last:It looks like sprites rendering with the wrong atlas, and nothing errors.
A second upload in the same frame now writes at a new offset, and the draw binds its vertex buffers at the offset holding its own data. The uniform ring in the same file already used this method for the same reason — this brings attribute buffers in line with it.
An upload that happens once per frame, which is what every existing example does, still writes at offset 0 and is unchanged.
2. A buffer that grows mid-frame is destroyed while still in use
Growing an attribute buffer replaced the old one and destroyed it immediately. A draw already recorded into the open render pass still refers to it, so the submit fails with "used in submit while destroyed" — and every draw in that frame fails, not just the one that grew.
Replaced buffers now go to a list and are destroyed at the next frame boundary, once the submit that could refer to them is complete. The release also runs before any early exit in
draw, so a frame where every draw is skipped still frees them rather than holding them untildispose().Tests
A GPU check in
scripts/gpu/entry.ts: one program, one frame, two draws — left half red, right half blue, with the second upload larger so the buffer grows and the first is retired mid-frame. It exercises both defects at once.Verified it fails without the fix and passes with it, by reverting
webgpu.tsand rebuilding:Without the fix the left half comes out blue — the second batch's colour, drawn with the first batch's geometry.
318 node tests pass,
npm run typecheckclean. WebKit did not run locally —playwright-core install webkitwas missing in my environment, unrelated to this change.Provenance
Extracted from #2, which bundles these with six other changes and four demos. That PR is being closed in favour of focused ones; this carries only the two defect corrections.
Deliberately not included from #2:
draw({ instanceCount })andresolveDrawCount, which are a convenience rather than a defect fix, and the WebGL2blendFuncSeparatecorrection, which no longer applies since WebGL2 was removed.Over to you
Genuine questions, not politeness: