-
Notifications
You must be signed in to change notification settings - Fork 921
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
Move buffer slice OOB check from panic in wgpu
to validation error in wgpu-core
#6493
base: trunk
Are you sure you want to change the base?
Move buffer slice OOB check from panic in wgpu
to validation error in wgpu-core
#6493
Conversation
Exercised by `webgpu:api,validation,encoding,cmds,render,setIndexBuffer:offset_and_size_oob:*`, which was disovered to be failing in Firefox in [this `webgpu-backlog1` run on a Windows debug build in `try:bed05e732fb557b6173872c508612399b6bc365e`](https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&author=egubler%40mozilla.com&selectedTaskRun=enQluY6MT36HfWG8-NnjAw.0).
This does not, in fact, remove any bounds checks in practice. It is now a validation error, implemented by the previous commit.
wgpu
to validation error in wgpu-core
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.
The validation we are removing (which was recently added in #6432) applies in more cases than RenderPass::set_index_buffer
. It also applied to:
set_index_buffer
on the render bundle encoderset_vertex_buffer
on the render pass encoder & render bundle encodermap_async
on the buffer
Looking at the spec as well, having it in these places seems correct.
To build upon that: On that note: the fields of |
@teoxoy: Ah, right, I need to ensure that there's coverage for those methods, too. Will do that.
|
@Wumpf (CC @teoxoy): This is actually an interesting tension between typical Rust programming and the experience dictated by the WebGPU spec. The spec. specifies that this sort of error is a validation error; so, still safe, but we don't get a panic at the site of usage. I think this is for |
Connections
Exercised by
webgpu:api,validation,encoding,cmds,render,setIndexBuffer:offset_and_size_oob:*
, which was disovered to be failing in Firefox in thiswebgpu-backlog1
run on a Windows debug build intry:bed05e732fb557b6173872c508612399b6bc365e
.Description
wgpu
implemented OOB checks onBufferSlice
creation as a panic. However, it should be implemented as a validation check inwgpu-core
. So fix it. 😀Testing
webgpu:api,validation,encoding,cmds,render,setIndexBuffer:offset_and_size_oob:*
in Firefox.Checklist
cargo fmt
.taplo format
.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.