Align Storage Access enums to spec #6642
Conversation
atlv24
force-pushed
the
storage-access
branch
from
79b2708 to
bd7721f
Compare
There was a problem hiding this comment.
Nice, thanks!
I think that in the case of bgra8unorm-storage, we should not have to request TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES (for write-only access, which seems to be the case for at least the bgra8unorm-storage test). Instead, the write-only capability should be added automatically when the BGRA8UNORM_STORAGE feature is requested (and granted).
Co-authored-by: Nicolas Silva <[email protected]>
Co-authored-by: Nicolas Silva <[email protected]>
|
apologies, i thought i was doing a favor fixing the fact that storage write wasnt being set for other storages. pushed a better fix |
|
Looks like some of the tests where you previously added "TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES" now need "BGRA8UNORM_STORAGE". |
|
@nical I just tested locally and thats not enough to fix it -- zero-init-texture-binding.ron uses Rgba8Unorm not Bgra8Unorm so the feature doesnt apply. |
Head branch was pushed to by a user without write access
| /// A read-write or write-only buffer used in a bind group. | ||
| const STORAGE_READ_ONLY = 1 << 7; | ||
| /// A write-only storage buffer used in a bind group. | ||
| const STORAGE_WRITE_ONLY = 1 << 8; |
There was a problem hiding this comment.
This occupies the same bit as STORAGE_READ_WRITE.
| /// Read-write or write-only storage buffer usage. | ||
| const STORAGE_READ_ONLY = 1 << 8; | ||
| /// Write-only storage buffer usage. | ||
| const STORAGE_WRITE_ONLY = 1 << 9; |
There was a problem hiding this comment.
This occupies the same bit as STORAGE_READ_WRITE.
| | Tfc::STORAGE_WRITE | ||
| | Tfc::STORAGE_READ_ONLY | ||
| | Tfc::STORAGE_WRITE_ONLY | ||
| | Tfc::STORAGE_READ_WRITE |
There was a problem hiding this comment.
all_caps should not include read-write usage; it should be based on the read-write tier.
| attachment | TextureUsages::STORAGE_BINDING | ||
| let (bgra8unorm_f, bgra8unorm) = if device_features.contains(Features::BGRA8UNORM_STORAGE) { | ||
| ( | ||
| msaa_resolve | TextureFormatFeatureFlags::STORAGE_WRITE_ONLY, |
There was a problem hiding this comment.
With the changes in this PR, all the formats below that support TextureUsages::STORAGE_BINDING need at least one of the TextureFormatFeatureFlags::STORAGE_* flags to be enabled to be used as storage textures.
wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES bypasses guaranteed_format_features and that's why it gets the ray tracing example and one of the player tests passing.
With this PR it is effectively impossible to use/bind a storage texture (unless you enable TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES; which should not be required).
Running cargo run --bin wgpu-examples storage_texture results in:
wgpu error: Validation Error
Caused by:
In Device::create_bind_group
The adapter does not support write access for storage textures of format Rgba8Unorm
Connections
Fixes #6599
Description
Rename enum variants and change logic to match webgpu storage access spec as concluded in #6599.
Write-only storage access is now verified correctly, and as such two tests had to be updated to use TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES to get write-only access on the formats they were using.
Future work
BufferUses::STORAGE_WRITE_ONLYandTextureUses::STORAGE_WRITE_ONLYa different bit?wgt::BufferUsages::STORAGE->hal::BufferUses::STORAGE_READ_WRITEandwgt::TextureUsages::STORAGE_BINDING->hal::TextureUses::STORAGE_READ_WRITEinwgpu-core/src/conv.rsis correct.Figure out what the hell is going on withWhyBufferBarrier/TextureBarrierusage: Range<BufferUses>andusage: Range<TextureUses>. This smells real fishy, ranges of bitfields dont sound like they should work. They're not ordered.Range<BufferUses>#6645Testing
Existing tests pass.
Checklist
cargo fmt.taplo format.cargo clippy. If applicable, add:--target wasm32-unknown-unknown--target wasm32-unknown-emscriptencargo xtask testto run tests.CHANGELOG.md. See simple instructions inside file.