Skip to content

webgpu: correct two defects in per-frame attribute buffer handling - #7

Open
shadowcodex wants to merge 1 commit into
ericdrowell:mainfrom
shadowcodex-forks:webgpu-buffer-defects
Open

webgpu: correct two defects in per-frame attribute buffer handling#7
shadowcodex wants to merge 1 commit into
ericdrowell:mainfrom
shadowcodex-forks:webgpu-buffer-defects

Conversation

@shadowcodex

@shadowcodex shadowcodex commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

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.writeBuffer is ordered against that submit, not against the draw commands inside it. So every draw in the frame reads whatever was written last:

program.attributes.aTint.set(batchOne);
program.draw();               // wants batchOne
program.attributes.aTint.set(batchTwo);
program.draw();               // wants batchTwo
// both draws read batchTwo

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 until dispose().

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.ts and rebuilding:

WITHOUT   ✗ two batches in one frame keep their own attribute data
WITH      ✓ two batches in one frame keep their own attribute data

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 typecheck clean. WebKit did not run locally — playwright-core install webkit was 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 }) and resolveDrawCount, which are a convenience rather than a defect fix, and the WebGL2 blendFuncSeparate correction, which no longer applies since WebGL2 was removed.


Over to you

Genuine questions, not politeness:

  • Is this applicable at all? If it does not fit where BroMetal is going, say so and close it — no offence taken. We carry it as a local patch over the published package today, so nothing of ours is blocked on it landing.
  • Is there a better approach? We arrived at this from the outside, without the context you have on the design. If you would solve it differently — a different API shape, a different layer, or a reason the current behaviour is deliberate — we would rather write your version than have you merge ours.
  • Anything you want changed — naming, comment style, test placement, scope — tell us and we will do it.

@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

@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.

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.

1 participant