-
Notifications
You must be signed in to change notification settings - Fork 905
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make wgpu_render_pass_set_bind_group clamp the number of dynamic offsets within limits. #4918
Conversation
…ets within limits.
b881fb5
to
810edbd
Compare
@@ -919,9 +919,17 @@ pub mod compute_ffi { | |||
return; | |||
} | |||
|
|||
// Clamp the number of dynamic offsets to the maximum value that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For validation we should include the original length, so that in the body of the pass we can check to make sure the original_length == clamped length, and error if they don't match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Briefly looking at the code around num_dynamic_offsets
it looks to me like it would make more sense to just change its data type to be usize
since every time we set it we cast it to u8
and every time we read it we cast it back to usize
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For bookkeeping, going to request changes
Addressing this differently in #5026 |
This similarly updates wgpu_compute_pass_set_bind_group. Neither function throws an error when clamping. It's not clear what mechanism there is for error reporting at this entry point.
Connections
N/A
Description
User agents calling wgpu_compute_pass_set_bind_group directly can provide an arbitrary number of dynamic offsets without passing validation.
Testing
Untested. Any test using something like
cpass.set_bind_group
is stopped by validation.Checklist
cargo fmt
.cargo clippy
. If applicable, add:--target wasm32-unknown-unknown
--target wasm32-unknown-emscripten
cargo xtask test
to run tests.CHANGELOG.md
. See simple instructions inside file.