diff --git a/CHANGELOG.md b/CHANGELOG.md index b227ccd955d..a2d2a3a7b60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -184,6 +184,7 @@ By @cwfitzgerald in [#8609](https://github.com/gfx-rs/wgpu/pull/8609). #### Naga - Prevent UB with invalid ray query calls on spirv. By @Vecvec in [#8390](https://github.com/gfx-rs/wgpu/pull/8390). +- Update the set of binding_array capabilities. In most cases, they are set automatically from `wgpu` features, and this change should not be user-visible. By @andyleiserson in TBD. ### Bug Fixes diff --git a/naga/src/valid/analyzer.rs b/naga/src/valid/analyzer.rs index a25377c5381..7f759ee2dc3 100644 --- a/naga/src/valid/analyzer.rs +++ b/naga/src/valid/analyzer.rs @@ -551,30 +551,34 @@ impl FunctionInfo { base: array_element_ty_handle, .. } => { - // these are nasty aliases, but these idents are too long and break rustfmt - let sto = super::Capabilities::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING; - let uni = super::Capabilities::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING; - let st_sb = super::Capabilities::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING; - let sampler = super::Capabilities::SAMPLER_NON_UNIFORM_INDEXING; - // We're a binding array, so lets use the type of _what_ we are array of to determine if we can non-uniformly index it. let array_element_ty = &resolve_context.types[array_element_ty_handle].inner; needed_caps |= match *array_element_ty { - // If we're an image, use the appropriate limit. + // If we're an image, use the appropriate capability. crate::TypeInner::Image { class, .. } => match class { - crate::ImageClass::Storage { .. } => sto, - _ => st_sb, + crate::ImageClass::Storage { .. } => { + super::Capabilities::STORAGE_TEXTURE_BINDING_ARRAY_NON_UNIFORM_INDEXING + } + _ => { + super::Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING + } }, - crate::TypeInner::Sampler { .. } => sampler, - // If we're anything but an image, assume we're a buffer and use the address space. + crate::TypeInner::Sampler { .. } => { + super::Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING + } + // If we're anything but an image or sampler, assume we're a buffer and use the address space. _ => { if let E::GlobalVariable(global_handle) = expression_arena[base] { let global = &resolve_context.global_vars[global_handle]; match global.space { - crate::AddressSpace::Uniform => uni, - crate::AddressSpace::Storage { .. } => st_sb, + crate::AddressSpace::Uniform => { + super::Capabilities::BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING + } + crate::AddressSpace::Storage { .. } => { + super::Capabilities::STORAGE_BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING + } _ => unreachable!(), } } else { diff --git a/naga/src/valid/interface.rs b/naga/src/valid/interface.rs index 1e054700c01..faa00478328 100644 --- a/naga/src/valid/interface.rs +++ b/naga/src/valid/interface.rs @@ -18,6 +18,8 @@ pub enum GlobalVariableError { InvalidUsage(crate::AddressSpace), #[error("Type isn't compatible with address space {0:?}")] InvalidType(crate::AddressSpace), + #[error("Type {0:?} isn't compatible with binding arrays")] + InvalidBindingArray(Handle), #[error("Type flags {seen:?} do not meet the required {required:?}")] MissingTypeFlags { required: super::TypeFlags, @@ -643,9 +645,80 @@ impl super::Validator { // series of individually bound resources, so we can (mostly) // validate a `binding_array` as if it were just a plain `T`. crate::TypeInner::BindingArray { base, .. } => match var.space { - crate::AddressSpace::Storage { .. } - | crate::AddressSpace::Uniform - | crate::AddressSpace::Handle => base, + crate::AddressSpace::Storage { .. } => { + if !self + .capabilities + .contains(Capabilities::STORAGE_BUFFER_BINDING_ARRAY) + { + return Err(GlobalVariableError::UnsupportedCapability( + Capabilities::STORAGE_BUFFER_BINDING_ARRAY, + )); + } + base + } + crate::AddressSpace::Uniform => { + if !self + .capabilities + .contains(Capabilities::BUFFER_BINDING_ARRAY) + { + return Err(GlobalVariableError::UnsupportedCapability( + Capabilities::BUFFER_BINDING_ARRAY, + )); + } + base + } + crate::AddressSpace::Handle => { + match gctx.types[base].inner { + crate::TypeInner::Image { class, .. } => match class { + crate::ImageClass::Storage { .. } => { + if !self + .capabilities + .contains(Capabilities::STORAGE_TEXTURE_BINDING_ARRAY) + { + return Err(GlobalVariableError::UnsupportedCapability( + Capabilities::STORAGE_TEXTURE_BINDING_ARRAY, + )); + } + } + crate::ImageClass::Sampled { .. } | crate::ImageClass::Depth { .. } => { + if !self + .capabilities + .contains(Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY) + { + return Err(GlobalVariableError::UnsupportedCapability( + Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY, + )); + } + } + crate::ImageClass::External => { + // This should have been rejected in `validate_type`. + unreachable!("binding arrays of external images are not supported"); + } + }, + crate::TypeInner::Sampler { .. } => { + if !self + .capabilities + .contains(Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY) + { + return Err(GlobalVariableError::UnsupportedCapability( + Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY, + )); + } + } + crate::TypeInner::AccelerationStructure { .. } => { + return Err(GlobalVariableError::InvalidBindingArray(base)); + } + crate::TypeInner::RayQuery { .. } => { + // This should have been rejected in `validate_type`. + unreachable!("binding arrays of ray queries are not supported"); + } + _ => { + // Fall through to the regular validation, which will reject `base` + // as invalid in `AddressSpace::Handle`. + } + } + base + } _ => return Err(GlobalVariableError::InvalidUsage(var.space)), }, _ => var.ty, diff --git a/naga/src/valid/mod.rs b/naga/src/valid/mod.rs index 787746ff56f..c081be3b507 100644 --- a/naga/src/valid/mod.rs +++ b/naga/src/valid/mod.rs @@ -83,7 +83,7 @@ bitflags::bitflags! { #[cfg_attr(feature = "serialize", derive(serde::Serialize))] #[cfg_attr(feature = "deserialize", derive(serde::Deserialize))] #[derive(Clone, Copy, Debug, Eq, PartialEq)] - pub struct Capabilities: u32 { + pub struct Capabilities: u64 { /// Support for [`AddressSpace::Immediate`][1]. /// /// [1]: crate::AddressSpace::Immediate @@ -94,14 +94,14 @@ bitflags::bitflags! { /// /// [1]: crate::BuiltIn::PrimitiveIndex const PRIMITIVE_INDEX = 1 << 2; - /// Support for non-uniform indexing of sampled textures and storage buffer arrays. - const SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING = 1 << 3; - /// Support for non-uniform indexing of storage texture arrays. - const STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING = 1 << 4; - /// Support for non-uniform indexing of uniform buffer arrays. - const UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING = 1 << 5; - /// Support for non-uniform indexing of samplers. - const SAMPLER_NON_UNIFORM_INDEXING = 1 << 6; + /// Support for binding arrays of sampled textures and samplers. + const TEXTURE_AND_SAMPLER_BINDING_ARRAY = 1 << 3; + /// Support for binding arrays of uniform buffers. + const BUFFER_BINDING_ARRAY = 1 << 4; + /// Support for binding arrays of storage textures. + const STORAGE_TEXTURE_BINDING_ARRAY = 1 << 5; + /// Support for binding arrays of storage buffers. + const STORAGE_BUFFER_BINDING_ARRAY = 1 << 6; /// Support for [`BuiltIn::ClipDistance`]. /// /// [`BuiltIn::ClipDistance`]: crate::BuiltIn::ClipDistance @@ -192,6 +192,14 @@ bitflags::bitflags! { const MESH_SHADER = 1 << 30; /// Support for mesh shaders which output points. const MESH_SHADER_POINT_TOPOLOGY = 1 << 31; + /// Support for non-uniform indexing of binding arrays of sampled textures and samplers. + const TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 32; + /// Support for non-uniform indexing of binding arrays of uniform buffers. + const BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 33; + /// Support for non-uniform indexing of binding arrays of storage textures. + const STORAGE_TEXTURE_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 34; + /// Support for non-uniform indexing of binding arrays of storage buffers. + const STORAGE_BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING = 1 << 35; } } diff --git a/naga/src/valid/type.rs b/naga/src/valid/type.rs index 62aaf97b793..54e997b0d73 100644 --- a/naga/src/valid/type.rs +++ b/naga/src/valid/type.rs @@ -802,6 +802,7 @@ impl super::Validator { if base_info.flags.contains(TypeFlags::DATA) { // Currently Naga only supports binding arrays of structs for non-handle types. + // `validate_global_var` relies on ray queries (which are `DATA`) being rejected here match gctx.types[base].inner { crate::TypeInner::Struct { .. } => {} _ => return Err(TypeError::BindingArrayBaseTypeNotStruct(base)), @@ -815,7 +816,8 @@ impl super::Validator { } ) { // Binding arrays of external textures are not yet supported. - // https://github.com/gfx-rs/wgpu/issues/8027 + // See . Note that + // `validate_global_var` relies on this error being raised here. return Err(TypeError::BindingArrayBaseExternalTextures); } diff --git a/naga/tests/in/spv/binding-arrays.dynamic.toml b/naga/tests/in/spv/binding-arrays.dynamic.toml index ca7a97c390c..fd5664c637a 100644 --- a/naga/tests/in/spv/binding-arrays.dynamic.toml +++ b/naga/tests/in/spv/binding-arrays.dynamic.toml @@ -1,2 +1,4 @@ +god_mode = true + [spv-in] adjust_coordinate_space = true diff --git a/naga/tests/in/spv/binding-arrays.static.toml b/naga/tests/in/spv/binding-arrays.static.toml index ca7a97c390c..fd5664c637a 100644 --- a/naga/tests/in/spv/binding-arrays.static.toml +++ b/naga/tests/in/spv/binding-arrays.static.toml @@ -1,2 +1,4 @@ +god_mode = true + [spv-in] adjust_coordinate_space = true diff --git a/naga/tests/in/wgsl/binding-buffer-arrays.toml b/naga/tests/in/wgsl/binding-buffer-arrays.toml index 7532299cb76..4a8aaab5de5 100644 --- a/naga/tests/in/wgsl/binding-buffer-arrays.toml +++ b/naga/tests/in/wgsl/binding-buffer-arrays.toml @@ -1,4 +1,5 @@ targets = "WGSL | SPIRV" +god_mode = true [bounds_check_policies] index = "ReadZeroSkipWrite" diff --git a/naga/tests/naga/wgsl_errors.rs b/naga/tests/naga/wgsl_errors.rs index bc4386d0156..9fbc11563f0 100644 --- a/naga/tests/naga/wgsl_errors.rs +++ b/naga/tests/naga/wgsl_errors.rs @@ -3093,6 +3093,19 @@ fn binding_array_non_struct() { .. }) } + + check_validation! { + r#" + enable wgpu_ray_query; + @group(0) @binding(0) + var ray_query_array: binding_array; + "#: + Err(naga::valid::ValidationError::Type { + source: naga::valid::TypeError::BindingArrayBaseTypeNotStruct(_), + .. + }), + Capabilities::RAY_QUERY + } } #[test] @@ -4358,7 +4371,7 @@ fn ray_query_vertex_return_enable_extension() { check_extension_validation!( Capabilities::RAY_HIT_VERTEX_POSITION, r#"enable wgpu_ray_query; - + @group(0) @binding(0) var acc_struct: acceleration_structure; "#, @@ -4379,3 +4392,100 @@ fn ray_query_vertex_return_enable_extension() { }) ); } + +#[test] +fn binding_array_requires_capability() { + check_validation! { + r#" + struct Buffer { data: u32 } + @group(0) @binding(0) + var storage_array: binding_array; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::UnsupportedCapability( + Capabilities::STORAGE_BUFFER_BINDING_ARRAY + ), + .. + }) + } + + check_validation! { + r#" + struct Buffer { data: u32 } + @group(0) @binding(0) + var uniform_array: binding_array; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::UnsupportedCapability( + Capabilities::BUFFER_BINDING_ARRAY + ), + .. + }) + } + + check_validation! { + r#" + @group(0) @binding(0) + var storage_texture_array: binding_array, 10>; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::UnsupportedCapability( + Capabilities::STORAGE_TEXTURE_BINDING_ARRAY + ), + .. + }) + } + + check_validation! { + r#" + @group(0) @binding(0) + var sampled_texture_array: binding_array, 10>; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::UnsupportedCapability( + Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY + ), + .. + }) + } + + check_validation! { + r#" + @group(0) @binding(0) + var sampler_array: binding_array; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::UnsupportedCapability( + Capabilities::TEXTURE_AND_SAMPLER_BINDING_ARRAY + ), + .. + }) + } + + // Binding arrays of external textures are not yet supported. + check_validation! { + r#" + @group(0) @binding(0) + var external_texture_array: binding_array; + "#: + Err(naga::valid::ValidationError::Type { + source: naga::valid::TypeError::BindingArrayBaseExternalTextures, + .. + }), + Capabilities::TEXTURE_EXTERNAL + } + + // Acceleration structures are not allowed in binding arrays + check_validation! { + r#" + enable wgpu_ray_query; + @group(0) @binding(0) + var acc_struct_array: binding_array; + "#: + Err(naga::valid::ValidationError::GlobalVariable { + source: naga::valid::GlobalVariableError::InvalidBindingArray(_), + .. + }), + Capabilities::all() + } +} diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index daff9eb0b6b..ee4022d7328 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -417,21 +417,38 @@ pub fn create_validator( features.contains(wgt::Features::SHADER_PRIMITIVE_INDEX), ); caps.set( - Caps::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING, + Caps::TEXTURE_AND_SAMPLER_BINDING_ARRAY, + features.contains(wgt::Features::TEXTURE_BINDING_ARRAY), + ); + caps.set( + Caps::BUFFER_BINDING_ARRAY, + features.contains(wgt::Features::BUFFER_BINDING_ARRAY), + ); + caps.set( + Caps::STORAGE_TEXTURE_BINDING_ARRAY, + features.contains(wgt::Features::TEXTURE_BINDING_ARRAY) + && features.contains(wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY), + ); + caps.set( + Caps::STORAGE_BUFFER_BINDING_ARRAY, + features.contains(wgt::Features::BUFFER_BINDING_ARRAY) + && features.contains(wgt::Features::STORAGE_RESOURCE_BINDING_ARRAY), + ); + caps.set( + Caps::TEXTURE_AND_SAMPLER_BINDING_ARRAY_NON_UNIFORM_INDEXING, features .contains(wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING), ); caps.set( - Caps::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING, - features.contains(wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING), + Caps::BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING, + features.contains(wgt::Features::UNIFORM_BUFFER_BINDING_ARRAYS), ); caps.set( - Caps::UNIFORM_BUFFER_ARRAY_NON_UNIFORM_INDEXING, - features.contains(wgt::Features::UNIFORM_BUFFER_BINDING_ARRAYS), + Caps::STORAGE_TEXTURE_BINDING_ARRAY_NON_UNIFORM_INDEXING, + features.contains(wgt::Features::STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING), ); - // TODO: This needs a proper wgpu feature caps.set( - Caps::SAMPLER_NON_UNIFORM_INDEXING, + Caps::STORAGE_BUFFER_BINDING_ARRAY_NON_UNIFORM_INDEXING, features .contains(wgt::Features::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING), );