fix: don't depend on BG{,L} layout entry order in HAL #5421
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
Description
Multiple backends for
wgpu-hal
currently make assumptions about the ordering of descriptor entries provided for bind groups and bind group layouts that are invalid. In particular, it was assumed that the element order of resource entries stored inVec
s in bind group layouts were the same for bind group resources entries stored inVec
s. This caused binding slot order to become out-of-sync when actually creating bind groups from descriptors and bind group layouts with code likedesc.entries.iter().zip(desc.layout.entries.iter())
, bypassing essential validation for, i.e., type matching in resource entries, and usage masks for buffers and textures. At best, this caused crashes because of different numbers of types of resources being used (as observed in Firefox's bug 1877461, comment 3). At worst, this caused resources of (of the same and different) types to be bound in the wrong places. We in Firefox noticed because of crashes and putting graphics drivers into invalid states. I'm sure there are other potential types of damage that are even worse. 😬Testing
The test
wgpu_test::different_bgl_order_bw_shader_and_api
has been added, which exercises the previously crashing case where more than one resource type was being used in a bind group layout. This doesn't cover cases of resources of the same type having their order mixed up, but it should be sufficient to prevent a significant regression.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.