Skip to content
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

Align Storage Access enums to spec #6642

Merged
merged 12 commits into from
Dec 3, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]

#### General

- Align Storage Access enums to the webgpu spec. By @atlv24 in [#6642](https://github.com/gfx-rs/wgpu/pull/6642)
- Make `Surface::as_hal` take an immutable reference to the surface. By @jerzywilczek in [#9999](https://github.com/gfx-rs/wgpu/pull/9999)
- Add actual sample type to `CreateBindGroupError::InvalidTextureSampleType` error message. By @ErichDonGubler in [#6530](https://github.com/gfx-rs/wgpu/pull/6530).
- Improve binding error to give a clearer message when there is a mismatch between resource binding as it is in the shader and as it is in the binding layout. By @eliemichel in [#6553](https://github.com/gfx-rs/wgpu/pull/6553).
Expand Down
1 change: 1 addition & 0 deletions examples/src/ray_traced_triangle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ impl crate::framework::Example for Example {
fn required_features() -> wgpu::Features {
wgpu::Features::EXPERIMENTAL_RAY_TRACING_ACCELERATION_STRUCTURE
| wgpu::Features::EXPERIMENTAL_RAY_QUERY
| wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES
}

fn required_limits() -> wgpu::Limits {
Expand Down
2 changes: 1 addition & 1 deletion player/tests/data/zero-init-texture-binding.ron
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
(
features: [],
features: ["TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES"],
expectations: [
(
name: "Sampled Texture",
Expand Down
5 changes: 4 additions & 1 deletion tests/tests/bgra8unorm_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ static BGRA8_UNORM_STORAGE: GpuTestConfiguration = GpuTestConfiguration::new()
max_storage_textures_per_shader_stage: 1,
..Default::default()
})
.features(wgpu::Features::BGRA8UNORM_STORAGE),
.features(
wgpu::Features::BGRA8UNORM_STORAGE
| wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES,
),
)
.run_async(|ctx| async move {
let device = &ctx.device;
Expand Down
4 changes: 4 additions & 0 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ pub enum CreateBindGroupError {
DepthStencilAspect,
#[error("The adapter does not support read access for storage textures of format {0:?}")]
StorageReadNotSupported(wgt::TextureFormat),
#[error("The adapter does not support write access for storage textures of format {0:?}")]
StorageWriteNotSupported(wgt::TextureFormat),
#[error("The adapter does not support read-write access for storage textures of format {0:?}")]
StorageReadWriteNotSupported(wgt::TextureFormat),
#[error(transparent)]
ResourceUsageCompatibility(#[from] ResourceUsageCompatibilityError),
#[error(transparent)]
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ fn dispatch_indirect(
let src_transition = state
.intermediate_trackers
.buffers
.set_single(&buffer, hal::BufferUses::STORAGE_READ);
.set_single(&buffer, hal::BufferUses::STORAGE_READ_ONLY);
let src_barrier =
src_transition.map(|transition| transition.into_hal(&buffer, &state.snatch_guard));
unsafe {
Expand Down
10 changes: 7 additions & 3 deletions wgpu-core/src/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn map_buffer_usage(usage: wgt::BufferUsages) -> hal::BufferUses {
usage.contains(wgt::BufferUsages::UNIFORM),
);
u.set(
hal::BufferUses::STORAGE_READ | hal::BufferUses::STORAGE_READ_WRITE,
hal::BufferUses::STORAGE_READ_WRITE,
usage.contains(wgt::BufferUsages::STORAGE),
);
u.set(
Expand Down Expand Up @@ -122,7 +122,7 @@ pub fn map_texture_usage(
usage.contains(wgt::TextureUsages::TEXTURE_BINDING),
);
u.set(
hal::TextureUses::STORAGE_READ | hal::TextureUses::STORAGE_READ_WRITE,
hal::TextureUses::STORAGE_READ_WRITE,
usage.contains(wgt::TextureUsages::STORAGE_BINDING),
);
let is_color = aspect.contains(hal::FormatAspects::COLOR);
Expand Down Expand Up @@ -179,7 +179,11 @@ pub fn map_texture_usage_from_hal(uses: hal::TextureUses) -> wgt::TextureUsages
);
u.set(
wgt::TextureUsages::STORAGE_BINDING,
uses.contains(hal::TextureUses::STORAGE_READ | hal::TextureUses::STORAGE_READ_WRITE),
uses.intersects(
hal::TextureUses::STORAGE_READ_ONLY
| hal::TextureUses::STORAGE_WRITE_ONLY
| hal::TextureUses::STORAGE_READ_WRITE,
),
);
u.set(
wgt::TextureUsages::RENDER_ATTACHMENT,
Expand Down
41 changes: 29 additions & 12 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,16 @@ impl Device {
self.require_downlevel_flags(wgt::DownlevelFlags::INDIRECT_EXECUTION)?;
// We are going to be reading from it, internally;
// when validating the content of the buffer
usage |= hal::BufferUses::STORAGE_READ | hal::BufferUses::STORAGE_READ_WRITE;
if !usage.intersects(
hal::BufferUses::STORAGE_READ_ONLY | hal::BufferUses::STORAGE_READ_WRITE,
) {
if usage.contains(hal::BufferUses::STORAGE_WRITE_ONLY) {
usage |= hal::BufferUses::STORAGE_READ_WRITE;
usage &= !hal::BufferUses::STORAGE_WRITE_ONLY;
} else {
usage |= hal::BufferUses::STORAGE_READ_ONLY;
}
}
}

if desc.mapped_at_creation {
Expand Down Expand Up @@ -1254,7 +1263,8 @@ impl Device {
}
TextureViewDimension::D3 => {
hal::TextureUses::RESOURCE
| hal::TextureUses::STORAGE_READ
| hal::TextureUses::STORAGE_READ_ONLY
| hal::TextureUses::STORAGE_WRITE_ONLY
| hal::TextureUses::STORAGE_READ_WRITE
}
_ => hal::TextureUses::all(),
Expand Down Expand Up @@ -1916,7 +1926,7 @@ impl Device {
wgt::BufferBindingType::Storage { read_only } => (
wgt::BufferUsages::STORAGE,
if read_only {
hal::BufferUses::STORAGE_READ
hal::BufferUses::STORAGE_READ_ONLY
} else {
hal::BufferUses::STORAGE_READ_WRITE
},
Expand Down Expand Up @@ -2489,24 +2499,31 @@ impl Device {
}

let internal_use = match access {
wgt::StorageTextureAccess::WriteOnly => hal::TextureUses::STORAGE_READ_WRITE,
wgt::StorageTextureAccess::WriteOnly => {
if !view.format_features.flags.intersects(
wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY
| wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE,
atlv24 marked this conversation as resolved.
Show resolved Hide resolved
) {
return Err(Error::StorageWriteNotSupported(view.desc.format));
}
hal::TextureUses::STORAGE_WRITE_ONLY
}
wgt::StorageTextureAccess::ReadOnly => {
if !view
.format_features
.flags
.contains(wgt::TextureFormatFeatureFlags::STORAGE_WRITE)
{
if !view.format_features.flags.intersects(
wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY
| wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE,
atlv24 marked this conversation as resolved.
Show resolved Hide resolved
) {
return Err(Error::StorageReadNotSupported(view.desc.format));
}
hal::TextureUses::STORAGE_READ
hal::TextureUses::STORAGE_READ_ONLY
}
wgt::StorageTextureAccess::ReadWrite => {
if !view
.format_features
.flags
.contains(wgt::TextureFormatFeatureFlags::STORAGE_WRITE)
.contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE)
{
return Err(Error::StorageReadNotSupported(view.desc.format));
return Err(Error::StorageReadWriteNotSupported(view.desc.format));
}

hal::TextureUses::STORAGE_READ_WRITE
Expand Down
14 changes: 12 additions & 2 deletions wgpu-core/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,9 @@ impl Adapter {
);
allowed_usages.set(
wgt::TextureUsages::STORAGE_BINDING,
caps.contains(Tfc::STORAGE_WRITE),
caps.intersects(
Tfc::STORAGE_WRITE_ONLY | Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_READ_WRITE,
),
);
allowed_usages.set(
wgt::TextureUsages::RENDER_ATTACHMENT,
Expand All @@ -521,7 +523,15 @@ impl Adapter {

let mut flags = wgt::TextureFormatFeatureFlags::empty();
flags.set(
wgt::TextureFormatFeatureFlags::STORAGE_WRITE,
wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY,
caps.contains(Tfc::STORAGE_READ_ONLY),
);
flags.set(
wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY,
caps.contains(Tfc::STORAGE_WRITE_ONLY),
);
flags.set(
wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE,
caps.contains(Tfc::STORAGE_READ_WRITE),
);

Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/src/auxil/dxgi/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ pub fn map_texture_format_for_resource(
} else if format.is_depth_stencil_format()
&& usage.intersects(
crate::TextureUses::RESOURCE
| crate::TextureUses::STORAGE_READ
| crate::TextureUses::STORAGE_READ_ONLY
| crate::TextureUses::STORAGE_WRITE_ONLY
| crate::TextureUses::STORAGE_READ_WRITE,
)
{
Expand Down
4 changes: 2 additions & 2 deletions wgpu-hal/src/dx12/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,13 +670,13 @@ impl crate::Adapter for super::Adapter {
);
// UAVs use srv_uav_format
caps.set(
Tfc::STORAGE_WRITE,
Tfc::STORAGE_WRITE_ONLY,
data_srv_uav
.Support1
.contains(Direct3D12::D3D12_FORMAT_SUPPORT1_TYPED_UNORDERED_ACCESS_VIEW),
);
caps.set(
Tfc::STORAGE_READ_WRITE,
Tfc::STORAGE_READ_WRITE | Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_WRITE_ONLY,
data_srv_uav
.Support2
.contains(Direct3D12::D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD),
Expand Down
6 changes: 3 additions & 3 deletions wgpu-hal/src/dx12/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,9 @@ pub fn map_buffer_usage_to_state(usage: crate::BufferUses) -> Direct3D12::D3D12_
if usage.intersects(Bu::VERTEX | Bu::UNIFORM) {
state |= Direct3D12::D3D12_RESOURCE_STATE_VERTEX_AND_CONSTANT_BUFFER;
}
if usage.intersects(Bu::STORAGE_READ_WRITE) {
if usage.intersects(Bu::STORAGE_READ_WRITE | Bu::STORAGE_WRITE_ONLY) {
state |= Direct3D12::D3D12_RESOURCE_STATE_UNORDERED_ACCESS;
} else if usage.intersects(Bu::STORAGE_READ) {
} else if usage.intersects(Bu::STORAGE_READ_ONLY) {
state |= Direct3D12::D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE
| Direct3D12::D3D12_RESOURCE_STATE_NON_PIXEL_SHADER_RESOURCE;
}
Expand Down Expand Up @@ -168,7 +168,7 @@ pub fn map_texture_usage_to_state(usage: crate::TextureUses) -> Direct3D12::D3D1
if usage.intersects(Tu::DEPTH_STENCIL_WRITE) {
state |= Direct3D12::D3D12_RESOURCE_STATE_DEPTH_WRITE;
}
if usage.intersects(Tu::STORAGE_READ | Tu::STORAGE_READ_WRITE) {
if usage.intersects(Tu::STORAGE_READ_ONLY | Tu::STORAGE_READ_WRITE) {
state |= Direct3D12::D3D12_RESOURCE_STATE_UNORDERED_ACCESS;
}
state
Expand Down
2 changes: 1 addition & 1 deletion wgpu-hal/src/dx12/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ impl crate::Device for super::Device {
None
},
handle_uav: if desc.usage.intersects(
crate::TextureUses::STORAGE_READ | crate::TextureUses::STORAGE_READ_WRITE,
crate::TextureUses::STORAGE_READ_ONLY | crate::TextureUses::STORAGE_READ_WRITE,
) {
match unsafe { view_desc.to_uav() } {
Some(raw_desc) => {
Expand Down
3 changes: 2 additions & 1 deletion wgpu-hal/src/gles/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,8 @@ impl crate::Adapter for super::Adapter {
let renderable =
unfilterable | Tfc::COLOR_ATTACHMENT | sample_count | Tfc::MULTISAMPLE_RESOLVE;
let filterable_renderable = filterable | renderable | Tfc::COLOR_ATTACHMENT_BLEND;
let storage = base | Tfc::STORAGE_WRITE | Tfc::STORAGE_READ_WRITE;
let storage =
base | Tfc::STORAGE_READ_WRITE | Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_WRITE_ONLY;

let feature_fn = |f, caps| {
if self.shared.features.contains(f) {
Expand Down
8 changes: 6 additions & 2 deletions wgpu-hal/src/gles/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,7 +1225,9 @@ impl super::Queue {
flags |= glow::BUFFER_UPDATE_BARRIER_BIT;
}
if usage.intersects(
crate::BufferUses::STORAGE_READ | crate::BufferUses::STORAGE_READ_WRITE,
crate::BufferUses::STORAGE_READ_ONLY
| crate::BufferUses::STORAGE_WRITE_ONLY
| crate::BufferUses::STORAGE_READ_WRITE,
) {
flags |= glow::SHADER_STORAGE_BARRIER_BIT;
}
Expand All @@ -1237,7 +1239,9 @@ impl super::Queue {
flags |= glow::TEXTURE_FETCH_BARRIER_BIT;
}
if usage.intersects(
crate::TextureUses::STORAGE_READ | crate::TextureUses::STORAGE_READ_WRITE,
crate::TextureUses::STORAGE_READ_ONLY
| crate::TextureUses::STORAGE_WRITE_ONLY
| crate::TextureUses::STORAGE_READ_WRITE,
) {
flags |= glow::SHADER_IMAGE_ACCESS_BARRIER_BIT;
}
Expand Down
22 changes: 14 additions & 8 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1543,8 +1543,10 @@ bitflags!(
/// Format can be sampled with a min/max reduction sampler.
const SAMPLED_MINMAX = 1 << 2;

/// Format can be used as storage with read-only access.
const STORAGE_READ_ONLY = 1 << 16;
/// Format can be used as storage with write-only access.
const STORAGE_WRITE = 1 << 3;
const STORAGE_WRITE_ONLY = 1 << 3;
/// Format can be used as storage with read and read/write access.
atlv24 marked this conversation as resolved.
Show resolved Hide resolved
const STORAGE_READ_WRITE = 1 << 4;
/// Format can be used as storage with atomics.
Expand Down Expand Up @@ -1675,8 +1677,10 @@ bitflags::bitflags! {
/// A uniform buffer bound in a bind group.
const UNIFORM = 1 << 6;
/// A read-only storage buffer used in a bind group.
const STORAGE_READ = 1 << 7;
/// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This occupies the same bit as STORAGE_READ_WRITE.

/// A read-write buffer used in a bind group.
const STORAGE_READ_WRITE = 1 << 8;
/// The indirect or count buffer in a indirect draw or dispatch.
const INDIRECT = 1 << 9;
Expand All @@ -1688,7 +1692,7 @@ bitflags::bitflags! {
/// The combination of states that a buffer may be in _at the same time_.
const INCLUSIVE = Self::MAP_READ.bits() | Self::COPY_SRC.bits() |
Self::INDEX.bits() | Self::VERTEX.bits() | Self::UNIFORM.bits() |
Self::STORAGE_READ.bits() | Self::INDIRECT.bits() | Self::BOTTOM_LEVEL_ACCELERATION_STRUCTURE_INPUT.bits() | Self::TOP_LEVEL_ACCELERATION_STRUCTURE_INPUT.bits();
Self::STORAGE_READ_ONLY.bits() | Self::INDIRECT.bits() | Self::BOTTOM_LEVEL_ACCELERATION_STRUCTURE_INPUT.bits() | Self::TOP_LEVEL_ACCELERATION_STRUCTURE_INPUT.bits();
/// The combination of states that a buffer must exclusively be in.
const EXCLUSIVE = Self::MAP_WRITE.bits() | Self::COPY_DST.bits() | Self::STORAGE_READ_WRITE.bits() | Self::ACCELERATION_STRUCTURE_SCRATCH.bits();
/// The combination of all usages that the are guaranteed to be be ordered by the hardware.
Expand Down Expand Up @@ -1719,17 +1723,19 @@ bitflags::bitflags! {
/// Read-write depth stencil usage
const DEPTH_STENCIL_WRITE = 1 << 7;
/// Read-only storage buffer usage. Corresponds to a UAV in d3d, so is exclusive, despite being read only.
const STORAGE_READ = 1 << 8;
/// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This occupies the same bit as STORAGE_READ_WRITE.

/// Read-write storage buffer usage.
const STORAGE_READ_WRITE = 1 << 9;
/// The combination of states that a texture may be in _at the same time_.
const INCLUSIVE = Self::COPY_SRC.bits() | Self::RESOURCE.bits() | Self::DEPTH_STENCIL_READ.bits();
/// The combination of states that a texture must exclusively be in.
const EXCLUSIVE = Self::COPY_DST.bits() | Self::COLOR_TARGET.bits() | Self::DEPTH_STENCIL_WRITE.bits() | Self::STORAGE_READ.bits() | Self::STORAGE_READ_WRITE.bits() | Self::PRESENT.bits();
const EXCLUSIVE = Self::COPY_DST.bits() | Self::COLOR_TARGET.bits() | Self::DEPTH_STENCIL_WRITE.bits() | Self::STORAGE_READ_ONLY.bits() | Self::STORAGE_READ_WRITE.bits() | Self::PRESENT.bits();
/// The combination of all usages that the are guaranteed to be be ordered by the hardware.
/// If a usage is ordered, then if the texture state doesn't change between draw calls, there
/// are no barriers needed for synchronization.
const ORDERED = Self::INCLUSIVE.bits() | Self::COLOR_TARGET.bits() | Self::DEPTH_STENCIL_WRITE.bits() | Self::STORAGE_READ.bits();
const ORDERED = Self::INCLUSIVE.bits() | Self::COLOR_TARGET.bits() | Self::DEPTH_STENCIL_WRITE.bits() | Self::STORAGE_READ_ONLY.bits();

/// Flag used by the wgpu-core texture tracker to say a texture is in different states for every sub-resource
const COMPLEX = 1 << 10;
Expand Down
Loading