From 53fe1a562ea8a1e3dda96d20337d8eec37753f12 Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Mon, 14 Jul 2025 13:52:52 -0700 Subject: [PATCH 1/3] Additional validation of buffer-texture copies Fixes #7936, but leaves a TODO for #7947 --- cts_runner/test.lst | 22 ++++++++ wgpu-core/src/command/transfer.rs | 84 +++++++++++++++++++++++++------ wgpu-core/src/conv.rs | 3 ++ wgpu-core/src/device/queue.rs | 18 ++++--- wgpu-types/src/lib.rs | 2 + 5 files changed, 108 insertions(+), 21 deletions(-) diff --git a/cts_runner/test.lst b/cts_runner/test.lst index 21eb6b448b2..c2660443e9d 100644 --- a/cts_runner/test.lst +++ b/cts_runner/test.lst @@ -34,6 +34,28 @@ webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:bind_grou webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:buffer_binding,render_pipeline:* webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:sampler_binding,render_pipeline:* webgpu:api,validation,encoding,queries,general:occlusion_query,query_index:* +webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_usage_and_aspect:* +webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_size:* +webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth32float";aspect="depth-only";copyType="CopyT2B" +webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyB2T" +webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyT2B" +//mix of PASS and FAIL: other subtests of copy_buffer_offset +// https://github.com/gfx-rs/wgpu/issues/7947 +webgpu:api,validation,image_copy,buffer_texture_copies:sample_count:* +webgpu:api,validation,image_copy,buffer_texture_copies:texture_buffer_usages:* +webgpu:api,validation,image_copy,buffer_texture_copies:device_mismatch:* +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="3d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="3d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyB2T";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyT2B";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="2d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="3d" +webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="3d" +//mix of PASS and FAIL: other subtests of offset_and_bytesPerRow +// https://github.com/gfx-rs/wgpu/issues/7947 webgpu:api,validation,image_copy,layout_related:copy_end_overflows_u64:* webgpu:api,validation,image_copy,texture_related:format:dimension="1d";* webgpu:api,validation,queue,submit:command_buffer,device_mismatch:* diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 4d6c8d98ca4..6c685038a08 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -363,6 +363,59 @@ pub(crate) fn validate_linear_texture_data( Ok((bytes_in_copy, image_stride_bytes)) } +/// Validation for texture/buffer copies. +/// +/// This implements the following checks from WebGPU's [validating texture buffer copy][vtbc] +/// algorithm: +/// * The texture must not be multisampled. +/// * The copy must be from/to a single aspect of the texture. +/// * If `aligned` is true, the buffer offset must be aligned appropriately. +/// +/// Other checks in this algorithm are enforced elsewhere. +/// +/// [vtbc]: https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-texture-buffer-copy +pub(crate) fn validate_texture_buffer_copy( + texture_copy_view: &wgt::TexelCopyTextureInfo, + aspect: hal::FormatAspects, + desc: &wgt::TextureDescriptor<(), Vec>, + offset: BufferAddress, + aligned: bool, +) -> Result<(), TransferError> { + if desc.sample_count != 1 { + return Err(TransferError::InvalidSampleCount { + sample_count: desc.sample_count, + }); + } + + if !aspect.is_one() { + return Err(TransferError::CopyAspectNotOne); + } + + let mut offset_alignment = if desc.format.is_depth_stencil_format() { + 4 + } else { + desc.format + .block_copy_size(Some(texture_copy_view.aspect)) + .unwrap_or(1) + }; + + // TODO(https://github.com/gfx-rs/wgpu/issues/7947): This does not match the spec. + // The spec imposes no alignment requirement if `!aligned`, and otherwise + // imposes only the `offset_alignment` as calculated above. wgpu currently + // can panic on alignments <4B, so we reject them here to avoid a panic. + if aligned { + offset_alignment = offset_alignment.max(4); + } else { + offset_alignment = 4; + } + + if offset % u64::from(offset_alignment) != 0 { + return Err(TransferError::UnalignedBufferOffset(offset)); + } + + Ok(()) +} + /// Validate the extent and alignment of a texture copy. /// /// Copied with minor modifications from WebGPU standard. This mostly follows @@ -884,10 +937,6 @@ impl Global { .map(|pending| pending.into_hal(dst_raw)) .collect::>(); - if !dst_base.aspect.is_one() { - return Err(TransferError::CopyAspectNotOne.into()); - } - if !conv::is_valid_copy_dst_texture_format(dst_texture.desc.format, destination.aspect) { return Err(TransferError::CopyToForbiddenTextureFormat { @@ -897,6 +946,14 @@ impl Global { .into()); } + validate_texture_buffer_copy( + destination, + dst_base.aspect, + &dst_texture.desc, + source.layout.offset, + true, // alignment required for buffer offset + )?; + let (required_buffer_bytes_in_copy, bytes_per_array_layer) = validate_linear_texture_data( &source.layout, @@ -999,12 +1056,7 @@ impl Global { src_texture .check_usage(TextureUsages::COPY_SRC) .map_err(TransferError::MissingTextureUsage)?; - if src_texture.desc.sample_count != 1 { - return Err(TransferError::InvalidSampleCount { - sample_count: src_texture.desc.sample_count, - } - .into()); - } + if source.mip_level >= src_texture.desc.mip_level_count { return Err(TransferError::InvalidMipLevel { requested: source.mip_level, @@ -1013,10 +1065,6 @@ impl Global { .into()); } - if !src_base.aspect.is_one() { - return Err(TransferError::CopyAspectNotOne.into()); - } - if !conv::is_valid_copy_src_texture_format(src_texture.desc.format, source.aspect) { return Err(TransferError::CopyFromForbiddenTextureFormat { format: src_texture.desc.format, @@ -1025,6 +1073,14 @@ impl Global { .into()); } + validate_texture_buffer_copy( + source, + src_base.aspect, + &src_texture.desc, + destination.layout.offset, + true, // alignment required for buffer offset + )?; + let (required_buffer_bytes_in_copy, bytes_per_array_layer) = validate_linear_texture_data( &destination.layout, diff --git a/wgpu-core/src/conv.rs b/wgpu-core/src/conv.rs index 5a5a27279bb..aa6ceb046d1 100644 --- a/wgpu-core/src/conv.rs +++ b/wgpu-core/src/conv.rs @@ -2,6 +2,9 @@ use wgt::TextureFormatFeatures; use crate::resource::{self, TextureDescriptor}; +// Some core-only texture format helpers. The helper methods on `TextureFormat` +// defined in wgpu-types may need to be modified along with the ones here. + pub fn is_valid_copy_src_texture_format( format: wgt::TextureFormat, aspect: wgt::TextureAspect, diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 6b4c1478b94..8a51e60a45b 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -19,9 +19,9 @@ use crate::device::trace::Action; use crate::{ api_log, command::{ - extract_texture_selector, validate_linear_texture_data, validate_texture_copy_range, - ClearError, CommandAllocator, CommandBuffer, CommandEncoder, CommandEncoderError, CopySide, - TexelCopyTextureInfo, TransferError, + extract_texture_selector, validate_linear_texture_data, validate_texture_buffer_copy, + validate_texture_copy_range, ClearError, CommandAllocator, CommandBuffer, CommandEncoder, + CommandEncoderError, CopySide, TexelCopyTextureInfo, TransferError, }, conv, device::{DeviceError, WaitIdleError}, @@ -737,10 +737,6 @@ impl Queue { let (selector, dst_base) = extract_texture_selector(&destination, size, &dst)?; - if !dst_base.aspect.is_one() { - return Err(TransferError::CopyAspectNotOne.into()); - } - if !conv::is_valid_copy_dst_texture_format(dst.desc.format, destination.aspect) { return Err(TransferError::CopyToForbiddenTextureFormat { format: dst.desc.format, @@ -749,6 +745,14 @@ impl Queue { .into()); } + validate_texture_buffer_copy( + &destination, + dst_base.aspect, + &dst.desc, + data_layout.offset, + false, // alignment not required for buffer offset + )?; + // Note: `_source_bytes_per_array_layer` is ignored since we // have a staging copy, and it can have a different value. let (required_bytes_in_copy, _source_bytes_per_array_layer) = validate_linear_texture_data( diff --git a/wgpu-types/src/lib.rs b/wgpu-types/src/lib.rs index e05822736ad..9bb3456d57e 100644 --- a/wgpu-types/src/lib.rs +++ b/wgpu-types/src/lib.rs @@ -2608,6 +2608,8 @@ impl TextureAspect { } } +// There are some additional texture format helpers in `wgpu-core/src/conv.rs`, +// that may need to be modified along with the ones here. impl TextureFormat { /// Returns the aspect-specific format of the original format /// From 32edf6a092288138410743351b390cb613ab2d7f Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Tue, 15 Jul 2025 10:16:28 -0700 Subject: [PATCH 2/3] Skip tests failing on dx12 --- cts_runner/test.lst | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/cts_runner/test.lst b/cts_runner/test.lst index c2660443e9d..3bac66e8481 100644 --- a/cts_runner/test.lst +++ b/cts_runner/test.lst @@ -36,26 +36,26 @@ webgpu:api,validation,encoding,programmable,pipeline_bind_group_compat:sampler_b webgpu:api,validation,encoding,queries,general:occlusion_query,query_index:* webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_usage_and_aspect:* webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_size:* -webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth32float";aspect="depth-only";copyType="CopyT2B" -webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyB2T" -webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyT2B" -//mix of PASS and FAIL: other subtests of copy_buffer_offset -// https://github.com/gfx-rs/wgpu/issues/7947 +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth32float";aspect="depth-only";copyType="CopyT2B" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyB2T" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:depth_stencil_format,copy_buffer_offset:format="depth24plus-stencil8";aspect="stencil-only";copyType="CopyT2B" +//mix of PASS and FAIL: other subtests of copy_buffer_offset. Related bugs: +// https://github.com/gfx-rs/wgpu/issues/7946, https://github.com/gfx-rs/wgpu/issues/7947 webgpu:api,validation,image_copy,buffer_texture_copies:sample_count:* webgpu:api,validation,image_copy,buffer_texture_copies:texture_buffer_usages:* webgpu:api,validation,image_copy,buffer_texture_copies:device_mismatch:* -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="3d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="3d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyB2T";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyT2B";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="2d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="3d" -webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="3d" -//mix of PASS and FAIL: other subtests of offset_and_bytesPerRow -// https://github.com/gfx-rs/wgpu/issues/7947 +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyB2T";dimension="3d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba8unorm";copyType="CopyT2B";dimension="3d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyB2T";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="bgra8unorm";copyType="CopyT2B";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="2d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyT2B";dimension="3d" +fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="astc-4x4-unorm";copyType="CopyB2T";dimension="3d" +//mix of PASS and FAIL: other subtests of offset_and_bytesPerRow. Related bugs: +// https://github.com/gfx-rs/wgpu/issues/7946, https://github.com/gfx-rs/wgpu/issues/7947 webgpu:api,validation,image_copy,layout_related:copy_end_overflows_u64:* webgpu:api,validation,image_copy,texture_related:format:dimension="1d";* webgpu:api,validation,queue,submit:command_buffer,device_mismatch:* From 22efde3bd3f7cc655c8bea99b540600a195207c8 Mon Sep 17 00:00:00 2001 From: Andy Leiserson Date: Tue, 22 Jul 2025 11:44:32 -0700 Subject: [PATCH 3/3] Update comments and change unwrap_or to expect --- wgpu-core/src/command/transfer.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 6c685038a08..56816fa87f4 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -371,7 +371,14 @@ pub(crate) fn validate_linear_texture_data( /// * The copy must be from/to a single aspect of the texture. /// * If `aligned` is true, the buffer offset must be aligned appropriately. /// -/// Other checks in this algorithm are enforced elsewhere. +/// The following steps in the algorithm are implemented elsewhere: +/// * Invocation of other validation algorithms. +/// * The texture usage (COPY_DST / COPY_SRC) check. +/// * The check for non-copyable depth/stencil formats. The caller must perform +/// this check using `is_valid_copy_src_format` / `is_valid_copy_dst_format` +/// before calling this function. This function will panic if +/// [`wgt::TextureFormat::block_copy_size`] returns `None` due to a +/// non-copyable format. /// /// [vtbc]: https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-texture-buffer-copy pub(crate) fn validate_texture_buffer_copy( @@ -394,9 +401,13 @@ pub(crate) fn validate_texture_buffer_copy( let mut offset_alignment = if desc.format.is_depth_stencil_format() { 4 } else { + // The case where `block_copy_size` returns `None` is currently + // unreachable both for the reason in the expect message, and also + // because the currently-defined non-copyable formats are depth/stencil + // formats so would take the `if` branch. desc.format .block_copy_size(Some(texture_copy_view.aspect)) - .unwrap_or(1) + .expect("non-copyable formats should have been rejected previously") }; // TODO(https://github.com/gfx-rs/wgpu/issues/7947): This does not match the spec.