Rebuild bind group when its dynamic uniform buffer is re-allocated mid-frame#8906
Merged
Conversation
…d-frame A non-persistent uniform buffer is allocated from a per-frame dynamic buffer and can move to a different GPU buffer - possibly several times within one frame when the same bind group is re-baked repeatedly (e.g. the WebGPU XR multiview pass replays the view bind groups per view). BindGroup detected this via a per-frame renderVersion, which only catches the first move per frame, so a later move left the bind group referencing a stale GPU buffer while its dynamic offset advanced, reading the wrong data (visibly: an XR view rendered with another view's matrices). Fix by comparing the uniform buffer's allocation.gpuBuffer directly and rebuilding whenever it changes. Removes the now-unused UniformBuffer.renderVersionDirty. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a WebGPU bind group correctness issue when non-persistent uniform buffers (allocated from per-frame dynamic/ring buffers) move to a different underlying GPU buffer multiple times within a single frame (notably in XR multiview), ensuring bind groups are rebuilt on every such move.
Changes:
- Remove
UniformBuffer.renderVersionDirtyand its mid-frame move tracking based on per-framerenderVersion. - Track the last
allocation.gpuBufferper uniform-buffer slot inBindGroupand mark the bind group dirty whenever a non-persistent uniform buffer’s container changes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/platform/graphics/uniform-buffer.js | Removes renderVersionDirty-based tracking for dynamic uniform buffer moves. |
| src/platform/graphics/bind-group.js | Adds per-slot dynamic-buffer container tracking to force bind group rebuilds on every mid-frame reallocation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses PR review: allocation.gpuBuffer is a DynamicBuffer (per DynamicBufferAllocation), so use the concrete type instead of object[] for better type generation and editor hints. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
A non-persistent uniform buffer is allocated from a per-frame dynamic (ring) buffer, and its underlying GPU buffer can change mid-frame. When the same bind group is re-baked multiple times in one frame — as the WebGPU XR multiview pass does (it replays the per-view bind groups once per view) — the buffer can move more than once per frame.
BindGroup.update()detected such a move via the per-framerenderVersion(renderVersionUpdated < uniformBuffer.renderVersionDirty). SincerenderVersionis constant within a frame, this only catches the first move per frame. A later move left the bind group's GPU resource pointing at the old buffer while its dynamic offset advanced into the new one, so it sampled the wrong data.Symptom: in the WebGPU
test > xr-viewsexample, the last (most-rotated) multiview view rendered with another view's matrices, shifting as the camera moved.Fix
Compare the uniform buffer's
allocation.gpuBufferdirectly per non-persistent slot and rebuild the bind group whenever it changes — catching every move within a frame, not just the first. Persistent buffers never move and are skipped. The now-unusedUniformBuffer.renderVersionDirtyis removed.The change stays in the generic
BindGroup/UniformBufferlayer (no backend-specific access); the texture change-detection path is untouched.Testing
npm test— all passingnpm run build:types— cleantest > xr-viewsexample: all 4 multiview views now render correctly and remain correct under camera/pivot motion. WebGL unaffected.🤖 Generated with Claude Code