Basic support for minBufferBindingSize#726
Merged
bors[bot] merged 1 commit intogfx-rs:masterfrom Jun 17, 2020
Merged
Conversation
cwfitzgerald
approved these changes
Jun 17, 2020
Member
cwfitzgerald
left a comment
There was a problem hiding this comment.
Code looks good. So to clarify it's use, this is the expected size of the (uniform) buffer in the shader so that if a user provides less than the required amount, they get an error?
Member
Author
|
Thanks for reviewing!
It's the size of a uniform/storage buffer that has to be at least as large as the structured part of the buffer plus one stride of an unbound array at the end. We validate against it either at pipeline and bind group creation (if BGL has it specified), or at draw time where they meet. So, in a sense, we validate that regardless. What this allows us to do is omitting any out-of-bounds checks on accessing the fields of such struct in the shader. bors r=cwfitzgerald |
Contributor
bors bot
added a commit
to gfx-rs/wgpu-rs
that referenced
this pull request
Jun 17, 2020
377: Update with minBufferBindingSize r=cwfitzgerald a=kvark Depends on gfx-rs/wgpu#726 Also reverts #373 : buffer bindings now have to include at least one element of an unsized struct portion, so they can't be zero-sized. Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
bors bot
added a commit
to gfx-rs/wgpu-rs
that referenced
this pull request
Jun 17, 2020
377: Update with minBufferBindingSize r=cwfitzgerald a=kvark Depends on gfx-rs/wgpu#726 Also reverts #373 : buffer bindings now have to include at least one element of an unsized struct portion, so they can't be zero-sized. Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
bors bot
added a commit
to gfx-rs/wgpu-rs
that referenced
this pull request
Jun 17, 2020
377: Update with minBufferBindingSize r=cwfitzgerald a=kvark Depends on gfx-rs/wgpu#726 Also reverts #373 : buffer bindings now have to include at least one element of an unsized struct portion, so they can't be zero-sized. Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Merged
bors bot
added a commit
that referenced
this pull request
Jun 20, 2020
739: Remove Peek-Poke r=cwfitzgerald a=kvark **Connections** Related to #738 Related to djg/peek-poke#10 **Description** As of #726 , the buffers have a minimum binding size that has to include the shader struct size. It, therefore, can't be zero. We can remove the hacks we had previously and switch fully to the idiomatic `Option<NonZeroU64>`. Peek-poke doesn't `NonZeroU64` and friends, so this made me reconsider the user of it entirely. Today, render bundles as well as the Player already represent command streams using a much rustier method. I tried to move everything to this method now, and I think this is going to work much better, and safer. **Testing** wgpu-rs works - gfx-rs/wgpu-rs#396 Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
kvark
added a commit
to kvark/wgpu
that referenced
this pull request
Jun 3, 2021
377: Update with minBufferBindingSize r=cwfitzgerald a=kvark Depends on gfx-rs#726 Also reverts gfx-rs#373 : buffer bindings now have to include at least one element of an unsized struct portion, so they can't be zero-sized. Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
kvark
pushed a commit
to kvark/wgpu
that referenced
this pull request
Jun 3, 2021
726: Bump wgpu-core and update texture_view_drop call r=kvark a=mkeeter This PR updates to the latest `wgpu-core` commit ([wgpu gfx-rs#1163](gfx-rs#1163)), and is the counterpart to [wgpu-native gfx-rs#66](gfx-rs/wgpu-native#66). I'm using `wait = false` in the `texture_view_drop` call to match `buffer_drop` and `texture_drop` elsewhere `backend/direct.rs`, though I don't quite understand the implications 😅 Co-authored-by: Matt Keeter <matt.j.keeter@gmail.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
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.
Connections
Has basic (partial) implementation of gpuweb/gpuweb#678
wgpu-rs update - gfx-rs/wgpu-rs#377
Description
This change allows users to optionally specify the expected minimum binding size for buffers. We are then validating this against both the pipelines and bind groups.
If it's not provided, we'll need to validate at draw time - this PR doesn't do this (focus on API changes first).
It also moves out the
read_spirv, since wgpu-types wasn't the right home for it ever.Testing
Tested on wgpu-rs examples