From a1fc2ba76d011ced1138c9e5f9c00b05a97189f7 Mon Sep 17 00:00:00 2001 From: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com> Date: Tue, 10 Dec 2024 21:34:38 +0100 Subject: [PATCH] fix being able to use storage textures without `TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES` (#6690) Co-authored-by: Erich Gubler --- examples/src/ray_traced_triangle/mod.rs | 1 - .../tests/data/zero-init-texture-binding.ron | 2 +- wgpu-core/src/conv.rs | 21 ++++++-- wgpu-core/src/device/global.rs | 8 ++- wgpu-core/src/device/resource.rs | 20 ++++---- wgpu-core/src/present.rs | 6 ++- wgpu-hal/src/dx12/adapter.rs | 14 ++++-- wgpu-hal/src/dx12/conv.rs | 8 ++- wgpu-hal/src/dx12/device.rs | 4 +- wgpu-hal/src/lib.rs | 14 +++--- wgpu-hal/src/metal/adapter.rs | 6 +-- wgpu-hal/src/vulkan/conv.rs | 4 +- wgpu-types/src/lib.rs | 50 ++++++++++--------- 13 files changed, 98 insertions(+), 60 deletions(-) diff --git a/examples/src/ray_traced_triangle/mod.rs b/examples/src/ray_traced_triangle/mod.rs index aeda991fa2..d508e6113e 100644 --- a/examples/src/ray_traced_triangle/mod.rs +++ b/examples/src/ray_traced_triangle/mod.rs @@ -30,7 +30,6 @@ 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 { diff --git a/player/tests/data/zero-init-texture-binding.ron b/player/tests/data/zero-init-texture-binding.ron index 6838479bf4..8920c41b7a 100644 --- a/player/tests/data/zero-init-texture-binding.ron +++ b/player/tests/data/zero-init-texture-binding.ron @@ -1,5 +1,5 @@ ( - features: "TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES", + features: "", expectations: [ ( name: "Sampled Texture", diff --git a/wgpu-core/src/conv.rs b/wgpu-core/src/conv.rs index b114e9826e..5459aa4de9 100644 --- a/wgpu-core/src/conv.rs +++ b/wgpu-core/src/conv.rs @@ -107,6 +107,7 @@ pub fn map_buffer_usage(usage: wgt::BufferUsages) -> hal::BufferUses { pub fn map_texture_usage( usage: wgt::TextureUsages, aspect: hal::FormatAspects, + flags: wgt::TextureFormatFeatureFlags, ) -> hal::TextureUses { let mut u = hal::TextureUses::empty(); u.set( @@ -121,10 +122,20 @@ pub fn map_texture_usage( hal::TextureUses::RESOURCE, usage.contains(wgt::TextureUsages::TEXTURE_BINDING), ); - u.set( - hal::TextureUses::STORAGE_READ_WRITE, - usage.contains(wgt::TextureUsages::STORAGE_BINDING), - ); + if usage.contains(wgt::TextureUsages::STORAGE_BINDING) { + u.set( + hal::TextureUses::STORAGE_READ_ONLY, + flags.contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY), + ); + u.set( + hal::TextureUses::STORAGE_WRITE_ONLY, + flags.contains(wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY), + ); + u.set( + hal::TextureUses::STORAGE_READ_WRITE, + flags.contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE), + ); + } let is_color = aspect.contains(hal::FormatAspects::COLOR); u.set( hal::TextureUses::COLOR_TARGET, @@ -143,7 +154,7 @@ pub fn map_texture_usage_for_texture( ) -> hal::TextureUses { // Enforce having COPY_DST/DEPTH_STENCIL_WRITE/COLOR_TARGET otherwise we // wouldn't be able to initialize the texture. - map_texture_usage(desc.usage, desc.format.into()) + map_texture_usage(desc.usage, desc.format.into(), format_features.flags) | if desc.format.is_depth_stencil_format() { hal::TextureUses::DEPTH_STENCIL_WRITE } else if desc.usage.contains(wgt::TextureUsages::COPY_DST) { diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index b2bdba04b0..3b2ab44c32 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -1859,7 +1859,13 @@ impl Global { height: config.height, depth_or_array_layers: 1, }, - usage: conv::map_texture_usage(config.usage, hal::FormatAspects::COLOR), + usage: conv::map_texture_usage( + config.usage, + hal::FormatAspects::COLOR, + wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY + | wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY + | wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE, + ), view_formats: hal_view_formats, }; diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index cf5ed7038b..15ca7988b8 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -650,7 +650,7 @@ impl Device { let texture = Texture::new( self, resource::TextureInner::Native { raw: hal_texture }, - conv::map_texture_usage(desc.usage, desc.format.into()), + conv::map_texture_usage(desc.usage, desc.format.into(), format_features.flags), desc, format_features, resource::TextureClearMode::None, @@ -2502,19 +2502,21 @@ impl Device { let internal_use = match access { wgt::StorageTextureAccess::WriteOnly => { - if !view.format_features.flags.intersects( - wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY - | wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE, - ) { + if !view + .format_features + .flags + .contains(wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY) + { return Err(Error::StorageWriteNotSupported(view.desc.format)); } hal::TextureUses::STORAGE_WRITE_ONLY } wgt::StorageTextureAccess::ReadOnly => { - if !view.format_features.flags.intersects( - wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY - | wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE, - ) { + if !view + .format_features + .flags + .contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY) + { return Err(Error::StorageReadNotSupported(view.desc.format)); } hal::TextureUses::STORAGE_READ_ONLY diff --git a/wgpu-core/src/present.rs b/wgpu-core/src/present.rs index 05124af252..f1b01a1a21 100644 --- a/wgpu-core/src/present.rs +++ b/wgpu-core/src/present.rs @@ -153,12 +153,16 @@ impl Surface { usage: config.usage, view_formats: config.view_formats, }; - let hal_usage = conv::map_texture_usage(config.usage, config.format.into()); let format_features = wgt::TextureFormatFeatures { allowed_usages: wgt::TextureUsages::RENDER_ATTACHMENT, flags: wgt::TextureFormatFeatureFlags::MULTISAMPLE_X4 | wgt::TextureFormatFeatureFlags::MULTISAMPLE_RESOLVE, }; + let hal_usage = conv::map_texture_usage( + config.usage, + config.format.into(), + format_features.flags, + ); let clear_view_desc = hal::TextureViewDescriptor { label: hal_label( Some("(wgpu internal) clear surface texture view"), diff --git a/wgpu-hal/src/dx12/adapter.rs b/wgpu-hal/src/dx12/adapter.rs index dfdf15454b..f081febfdb 100644 --- a/wgpu-hal/src/dx12/adapter.rs +++ b/wgpu-hal/src/dx12/adapter.rs @@ -672,16 +672,20 @@ impl crate::Adapter for super::Adapter { ); // UAVs use srv_uav_format caps.set( - Tfc::STORAGE_WRITE_ONLY, + Tfc::STORAGE_READ_ONLY, data_srv_uav - .Support1 - .contains(Direct3D12::D3D12_FORMAT_SUPPORT1_TYPED_UNORDERED_ACCESS_VIEW), + .Support2 + .contains(Direct3D12::D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD), ); caps.set( - Tfc::STORAGE_READ_WRITE | Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_WRITE_ONLY, + Tfc::STORAGE_WRITE_ONLY, data_srv_uav .Support2 - .contains(Direct3D12::D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD), + .contains(Direct3D12::D3D12_FORMAT_SUPPORT2_UAV_TYPED_STORE), + ); + caps.set( + Tfc::STORAGE_READ_WRITE, + caps.contains(Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_WRITE_ONLY), ); // We load via UAV/SRV so use srv_uav_format diff --git a/wgpu-hal/src/dx12/conv.rs b/wgpu-hal/src/dx12/conv.rs index 70ba83fa42..29d99abc3d 100644 --- a/wgpu-hal/src/dx12/conv.rs +++ b/wgpu-hal/src/dx12/conv.rs @@ -34,7 +34,11 @@ pub fn map_texture_usage_to_resource_flags( flags |= Direct3D12::D3D12_RESOURCE_FLAG_DENY_SHADER_RESOURCE; } } - if usage.contains(crate::TextureUses::STORAGE_READ_WRITE) { + if usage.intersects( + crate::TextureUses::STORAGE_READ_ONLY + | crate::TextureUses::STORAGE_WRITE_ONLY + | crate::TextureUses::STORAGE_READ_WRITE, + ) { flags |= Direct3D12::D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS; } @@ -168,7 +172,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_ONLY | Tu::STORAGE_READ_WRITE) { + if usage.intersects(Tu::STORAGE_READ_ONLY | Tu::STORAGE_WRITE_ONLY | Tu::STORAGE_READ_WRITE) { state |= Direct3D12::D3D12_RESOURCE_STATE_UNORDERED_ACCESS; } state diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs index 18b0b142c2..9cba60b896 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -577,7 +577,9 @@ impl crate::Device for super::Device { None }, handle_uav: if desc.usage.intersects( - crate::TextureUses::STORAGE_READ_ONLY | crate::TextureUses::STORAGE_READ_WRITE, + crate::TextureUses::STORAGE_READ_ONLY + | crate::TextureUses::STORAGE_WRITE_ONLY + | crate::TextureUses::STORAGE_READ_WRITE, ) { match unsafe { view_desc.to_uav() } { Some(raw_desc) => { diff --git a/wgpu-hal/src/lib.rs b/wgpu-hal/src/lib.rs index d989f6dc7e..03d4040ef6 100644 --- a/wgpu-hal/src/lib.rs +++ b/wgpu-hal/src/lib.rs @@ -1719,26 +1719,26 @@ bitflags::bitflags! { const DEPTH_STENCIL_READ = 1 << 6; /// 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. + /// Read-only storage texture usage. Corresponds to a UAV in d3d, so is exclusive, despite being read only. const STORAGE_READ_ONLY = 1 << 8; - /// Write-only storage buffer usage. + /// Write-only storage texture usage. const STORAGE_WRITE_ONLY = 1 << 9; - /// Read-write storage buffer usage. - const STORAGE_READ_WRITE = 1 << 9; + /// Read-write storage texture usage. + const STORAGE_READ_WRITE = 1 << 10; /// 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_ONLY.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_WRITE_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_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; + const COMPLEX = 1 << 11; /// Flag used by the wgpu-core texture tracker to say that the tracker does not know the state of the sub-resource. /// This is different from UNINITIALIZED as that says the tracker does know, but the texture has not been initialized. - const UNKNOWN = 1 << 11; + const UNKNOWN = 1 << 12; } } diff --git a/wgpu-hal/src/metal/adapter.rs b/wgpu-hal/src/metal/adapter.rs index 42ece4ac6d..d343d8881a 100644 --- a/wgpu-hal/src/metal/adapter.rs +++ b/wgpu-hal/src/metal/adapter.rs @@ -111,9 +111,7 @@ impl crate::Adapter for super::Adapter { // Metal defined pixel format capabilities let all_caps = Tfc::SAMPLED_LINEAR - | Tfc::STORAGE_READ_ONLY | Tfc::STORAGE_WRITE_ONLY - | Tfc::STORAGE_READ_WRITE | Tfc::COLOR_ATTACHMENT | Tfc::COLOR_ATTACHMENT_BLEND | msaa_count @@ -311,7 +309,7 @@ impl crate::Adapter for super::Adapter { } }; - Tfc::COPY_SRC | Tfc::COPY_DST | Tfc::SAMPLED | extra + Tfc::COPY_SRC | Tfc::COPY_DST | Tfc::SAMPLED | Tfc::STORAGE_READ_ONLY | extra } unsafe fn surface_capabilities( @@ -360,6 +358,8 @@ impl crate::Adapter for super::Adapter { usage: crate::TextureUses::COLOR_TARGET | crate::TextureUses::COPY_SRC | crate::TextureUses::COPY_DST + | crate::TextureUses::STORAGE_READ_ONLY + | crate::TextureUses::STORAGE_WRITE_ONLY | crate::TextureUses::STORAGE_READ_WRITE, }) } diff --git a/wgpu-hal/src/vulkan/conv.rs b/wgpu-hal/src/vulkan/conv.rs index ce4aa410bf..d9b6f92661 100644 --- a/wgpu-hal/src/vulkan/conv.rs +++ b/wgpu-hal/src/vulkan/conv.rs @@ -350,7 +350,9 @@ pub fn map_vk_image_usage(usage: vk::ImageUsageFlags) -> crate::TextureUses { bits |= crate::TextureUses::DEPTH_STENCIL_READ | crate::TextureUses::DEPTH_STENCIL_WRITE; } if usage.contains(vk::ImageUsageFlags::STORAGE) { - bits |= crate::TextureUses::STORAGE_READ_WRITE; + bits |= crate::TextureUses::STORAGE_READ_ONLY + | crate::TextureUses::STORAGE_WRITE_ONLY + | crate::TextureUses::STORAGE_READ_WRITE; } bits } diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index 4bb38b8a73..d5f54e48ac 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -2374,7 +2374,7 @@ bitflags::bitflags! { /// [`StorageTextureAccess::WriteOnly`]. const STORAGE_WRITE_ONLY = 1 << 6; /// When used as a STORAGE texture, then a texture with this format can be bound with - /// any [`StorageTextureAccess`]. + /// [`StorageTextureAccess::ReadWrite`]. const STORAGE_READ_WRITE = 1 << 9; /// If not present, the texture can't be blended into the render target. const BLENDABLE = 1 << 7; @@ -3400,6 +3400,10 @@ impl TextureFormat { let msaa = TextureFormatFeatureFlags::MULTISAMPLE_X4; let msaa_resolve = msaa | TextureFormatFeatureFlags::MULTISAMPLE_RESOLVE; + let s_ro_wo = TextureFormatFeatureFlags::STORAGE_READ_ONLY + | TextureFormatFeatureFlags::STORAGE_WRITE_ONLY; + let s_all = s_ro_wo | TextureFormatFeatureFlags::STORAGE_READ_WRITE; + // Flags let basic = TextureUsages::COPY_SRC | TextureUsages::COPY_DST | TextureUsages::TEXTURE_BINDING; @@ -3437,31 +3441,31 @@ impl TextureFormat { Self::Rg8Snorm => ( noaa, basic), Self::Rg8Uint => ( msaa, attachment), Self::Rg8Sint => ( msaa, attachment), - Self::R32Uint => ( noaa, all_flags), - Self::R32Sint => ( noaa, all_flags), - Self::R32Float => ( msaa, all_flags), + Self::R32Uint => ( noaa | s_all, all_flags), + Self::R32Sint => ( noaa | s_all, all_flags), + Self::R32Float => ( msaa | s_all, all_flags), Self::Rg16Uint => ( msaa, attachment), Self::Rg16Sint => ( msaa, attachment), Self::Rg16Float => (msaa_resolve, attachment), - Self::Rgba8Unorm => (msaa_resolve, all_flags), + Self::Rgba8Unorm => (msaa_resolve | s_ro_wo, all_flags), Self::Rgba8UnormSrgb => (msaa_resolve, attachment), - Self::Rgba8Snorm => ( noaa, storage), - Self::Rgba8Uint => ( msaa, all_flags), - Self::Rgba8Sint => ( msaa, all_flags), + Self::Rgba8Snorm => ( s_ro_wo, storage), + Self::Rgba8Uint => ( msaa | s_ro_wo, all_flags), + Self::Rgba8Sint => ( msaa | s_ro_wo, all_flags), Self::Bgra8Unorm => (bgra8unorm_f, bgra8unorm), Self::Bgra8UnormSrgb => (msaa_resolve, attachment), Self::Rgb10a2Uint => ( msaa, attachment), Self::Rgb10a2Unorm => (msaa_resolve, attachment), Self::Rg11b10Ufloat => ( msaa, rg11b10f), - Self::Rg32Uint => ( noaa, all_flags), - Self::Rg32Sint => ( noaa, all_flags), - Self::Rg32Float => ( noaa, all_flags), - Self::Rgba16Uint => ( msaa, all_flags), - Self::Rgba16Sint => ( msaa, all_flags), - Self::Rgba16Float => (msaa_resolve, all_flags), - Self::Rgba32Uint => ( noaa, all_flags), - Self::Rgba32Sint => ( noaa, all_flags), - Self::Rgba32Float => ( noaa, all_flags), + Self::Rg32Uint => ( noaa | s_ro_wo, all_flags), + Self::Rg32Sint => ( noaa | s_ro_wo, all_flags), + Self::Rg32Float => ( noaa | s_ro_wo, all_flags), + Self::Rgba16Uint => ( msaa | s_ro_wo, all_flags), + Self::Rgba16Sint => ( msaa | s_ro_wo, all_flags), + Self::Rgba16Float => (msaa_resolve | s_ro_wo, all_flags), + Self::Rgba32Uint => ( noaa | s_ro_wo, all_flags), + Self::Rgba32Sint => ( noaa | s_ro_wo, all_flags), + Self::Rgba32Float => ( noaa | s_ro_wo, all_flags), Self::Stencil8 => ( msaa, attachment), Self::Depth16Unorm => ( msaa, attachment), @@ -3473,12 +3477,12 @@ impl TextureFormat { // We only support sampling nv12 textures until we implement transfer plane data. Self::NV12 => ( noaa, binding), - Self::R16Unorm => ( msaa, storage), - Self::R16Snorm => ( msaa, storage), - Self::Rg16Unorm => ( msaa, storage), - Self::Rg16Snorm => ( msaa, storage), - Self::Rgba16Unorm => ( msaa, storage), - Self::Rgba16Snorm => ( msaa, storage), + Self::R16Unorm => ( msaa | s_ro_wo, storage), + Self::R16Snorm => ( msaa | s_ro_wo, storage), + Self::Rg16Unorm => ( msaa | s_ro_wo, storage), + Self::Rg16Snorm => ( msaa | s_ro_wo, storage), + Self::Rgba16Unorm => ( msaa | s_ro_wo, storage), + Self::Rgba16Snorm => ( msaa | s_ro_wo, storage), Self::Rgb9e5Ufloat => ( noaa, basic),