From e0e318198cc7616b4f7eb60f7e9aedc3924fd28b Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Tue, 6 Jan 2026 14:07:37 -0800 Subject: [PATCH] fix(core): Validate storage texture formats when creating BGLs This moves existing validation from binding creation to BGL creation. Binding creation already had a check that returns `CreateBindGroupError::InvalidStorageTextureFormat` if the format does not match the layout, so it is not necessary to revalidate the format support at that point. --- cts_runner/test.lst | 3 ++ wgpu-core/src/binding_model.rs | 21 ++++----- wgpu-core/src/device/resource.rs | 80 ++++++++++++++------------------ 3 files changed, 45 insertions(+), 59 deletions(-) diff --git a/cts_runner/test.lst b/cts_runner/test.lst index 375079fd9a..f25633c640 100644 --- a/cts_runner/test.lst +++ b/cts_runner/test.lst @@ -58,9 +58,12 @@ webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:crea webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:createRenderPipeline,at_over:limitTest="atDefault";testValueName="atLimit";async=true;pointList=true;frontFacing=false;sampleIndex=false;sampleMaskIn=false;sampleMaskOut=true //FAIL: webgpu:api,validation,capability_checks,limits,maxInterStageShaderVariables:createRenderPipeline,at_over:* // https://github.com/gpuweb/cts/issues/4538 +webgpu:api,validation,createBindGroup:binding_must_contain_resource_defined_in_layout:* webgpu:api,validation,createBindGroup:buffer_offset_and_size_for_bind_groups_match:* webgpu:api,validation,createBindGroup:buffer,effective_buffer_binding_size:* webgpu:api,validation,createBindGroup:buffer,resource_binding_size:* +webgpu:api,validation,createBindGroup:storage_texture,* +webgpu:api,validation,createBindGroupLayout:storage_texture,formats:* // Fails because we coerce a size of 0 in `GPUDevice.createBindGroup(…)` to `buffer.size - offset`. // FAIL webgpu:api,validation,createBindGroup:buffer_offset_and_size_for_bind_groups_match:* webgpu:api,validation,encoding,beginComputePass:* diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index 8b24a480b8..c005ceb4e6 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -75,6 +75,12 @@ pub enum CreateBindGroupLayoutError { InvalidBindingIndex { binding: u32, maximum: u32 }, #[error("Invalid visibility {0:?}")] InvalidVisibility(wgt::ShaderStages), + #[error("Binding index {binding}: {access:?} access to storage textures with format {format:?} is not supported")] + UnsupportedStorageTextureAccess { + binding: u32, + access: wgt::StorageTextureAccess, + format: wgt::TextureFormat, + }, } impl WebGpuError for CreateBindGroupLayoutError { @@ -88,7 +94,8 @@ impl WebGpuError for CreateBindGroupLayoutError { | Self::InvalidBindingIndex { .. } | Self::InvalidVisibility(_) | Self::ContainsBothBindingArrayAndDynamicOffsetArray - | Self::ContainsBothBindingArrayAndUniformBuffer => ErrorType::Validation, + | Self::ContainsBothBindingArrayAndUniformBuffer + | Self::UnsupportedStorageTextureAccess { .. } => ErrorType::Validation, } } } @@ -242,14 +249,6 @@ pub enum CreateBindGroupError { MissingTLASVertexReturn { binding: u32 }, #[error("Bound texture views can not have both depth and stencil aspects enabled")] DepthStencilAspect, - #[error("The adapter does not support read access for storage textures of format {0:?}")] - StorageReadNotSupported(wgt::TextureFormat), - #[error("The adapter does not support atomics for storage textures of format {0:?}")] - StorageAtomicNotSupported(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)] @@ -287,10 +286,6 @@ impl WebGpuError for CreateBindGroupError { | Self::WrongSamplerComparison { .. } | Self::WrongSamplerFiltering { .. } | Self::DepthStencilAspect - | Self::StorageReadNotSupported(_) - | Self::StorageWriteNotSupported(_) - | Self::StorageReadWriteNotSupported(_) - | Self::StorageAtomicNotSupported(_) | Self::MissingTLASVertexReturn { .. } | Self::InvalidExternalTextureMipLevelCount { .. } | Self::InvalidExternalTextureFormat { .. } => return ErrorType::Validation, diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index 3a610381e8..a6a29ddbea 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -2578,8 +2578,10 @@ impl Device { Bt::StorageTexture { access, view_dimension, - format: _, + format, } => { + use wgt::{StorageTextureAccess as Access, TextureFormatFeatureFlags as Flags}; + match view_dimension { TextureViewDimension::Cube | TextureViewDimension::CubeArray => { return Err(binding_model::CreateBindGroupLayoutError::Entry { @@ -2600,6 +2602,30 @@ impl Device { } _ => (), } + + let format_features = + self.describe_format_features(format).map_err(|error| { + binding_model::CreateBindGroupLayoutError::Entry { + binding: entry.binding, + error: BindGroupLayoutEntryError::MissingFeatures(error), + } + })?; + + let required_feature_flag = match access { + Access::WriteOnly => Flags::STORAGE_WRITE_ONLY, + Access::ReadOnly => Flags::STORAGE_READ_ONLY, + Access::ReadWrite => Flags::STORAGE_READ_WRITE, + Access::Atomic => Flags::STORAGE_ATOMIC, + }; + + if !format_features.flags.contains(required_feature_flag) { + return Err(binding_model::CreateBindGroupLayoutError::UnsupportedStorageTextureAccess { + binding: entry.binding, + access, + format, + }); + } + ( Some( wgt::Features::TEXTURE_BINDING_ARRAY @@ -3484,52 +3510,14 @@ impl Device { }); } - let internal_use = match access { - wgt::StorageTextureAccess::WriteOnly => { - if !view - .format_features - .flags - .contains(wgt::TextureFormatFeatureFlags::STORAGE_WRITE_ONLY) - { - return Err(Error::StorageWriteNotSupported(view.desc.format)); - } - wgt::TextureUses::STORAGE_WRITE_ONLY - } - wgt::StorageTextureAccess::ReadOnly => { - if !view - .format_features - .flags - .contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_ONLY) - { - return Err(Error::StorageReadNotSupported(view.desc.format)); - } - wgt::TextureUses::STORAGE_READ_ONLY - } - wgt::StorageTextureAccess::ReadWrite => { - if !view - .format_features - .flags - .contains(wgt::TextureFormatFeatureFlags::STORAGE_READ_WRITE) - { - return Err(Error::StorageReadWriteNotSupported(view.desc.format)); - } - - wgt::TextureUses::STORAGE_READ_WRITE - } - wgt::StorageTextureAccess::Atomic => { - if !view - .format_features - .flags - .contains(wgt::TextureFormatFeatureFlags::STORAGE_ATOMIC) - { - return Err(Error::StorageAtomicNotSupported(view.desc.format)); - } - - wgt::TextureUses::STORAGE_ATOMIC - } - }; view.check_usage(wgt::TextureUsages::STORAGE_BINDING)?; - Ok(internal_use) + + Ok(match access { + wgt::StorageTextureAccess::ReadOnly => wgt::TextureUses::STORAGE_READ_ONLY, + wgt::StorageTextureAccess::WriteOnly => wgt::TextureUses::STORAGE_WRITE_ONLY, + wgt::StorageTextureAccess::ReadWrite => wgt::TextureUses::STORAGE_READ_WRITE, + wgt::StorageTextureAccess::Atomic => wgt::TextureUses::STORAGE_ATOMIC, + }) } wgt::BindingType::ExternalTexture => { if view.desc.dimension != TextureViewDimension::D2 {