Tweak the bytes_per_row alignment check

Previously, the check was skipped if the copy was a single row, which is
not correct. The check should be made whenever bytes_per_row is
specified. It is permissible not to specify bytes_per_row if the copy is
a single row, but if it is specified, it must be aligned.

Also removes a redundant check of the `offset` alignment.

Since the offset and bytesPerRow alignment checks are not part of
"validating linear texture data", I chose to remove that instance of
them. These checks are now in `validate_texture_buffer_copy`, which
does not correspond 1:1 with the spec, but has a comment explaining how
it does correspond.

Author: andyleiserson

CHANGELOG.md

@@ -118,6 +118,7 @@ By @cwfitzgerald in [#8162](https://github.com/gfx-rs/wgpu/pull/8162).
   - Copies within the same texture must not overlap.
   - Copies of multisampled or depth/stencil formats must span an entire subresource (layer).
   - Copies of depth/stencil formats must be 4B aligned.
+  - For texture-buffer copies, `bytes_per_row` on the buffer side must be 256B-aligned, even if the transfer is a single row.
 - The offset for `set_vertex_buffer` and `set_index_buffer` must be 4B aligned. By @andyleiserson in [#7929](https://github.com/gfx-rs/wgpu/pull/7929).
 - The offset and size of bindings are validated as fitting within the underlying buffer in more cases. By @andyleiserson in [#7911](https://github.com/gfx-rs/wgpu/pull/7911).
 - The function you pass to `Device::on_uncaptured_error()` must now implement `Sync` in addition to `Send`, and be wrapped in `Arc` instead of `Box`.

cts_runner/test.lst

@@ -82,6 +82,8 @@ fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:depth_sten
 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:*
+fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba32float";copyType="CopyB2T";dimension="1d"
+fails-if(dx12) webgpu:api,validation,image_copy,buffer_texture_copies:offset_and_bytesPerRow:format="rgba32float";copyType="CopyT2B";dimension="1d"
 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"

wgpu-core/src/command/transfer.rs

@@ -288,7 +288,6 @@ pub(crate) fn validate_linear_texture_data(
     buffer_size: BufferAddress,
     buffer_side: CopySide,
     copy_size: &Extent3d,
-    need_copy_aligned_rows: bool,
 ) -> Result<(BufferAddress, BufferAddress), TransferError> {
     let wgt::BufferTextureCopyInfo {
         copy_width,
@@ -297,7 +296,7 @@ pub(crate) fn validate_linear_texture_data(
 
         offset,
 
-        block_size_bytes,
+        block_size_bytes: _,
         block_width_texels,
         block_height_texels,
 
@@ -338,24 +337,6 @@ pub(crate) fn validate_linear_texture_data(
         return Err(TransferError::UnspecifiedRowsPerImage);
     };
 
-    if need_copy_aligned_rows {
-        let bytes_per_row_alignment = wgt::COPY_BYTES_PER_ROW_ALIGNMENT as BufferAddress;
-
-        let mut offset_alignment = block_size_bytes;
-        if format.is_depth_stencil_format() {
-            offset_alignment = 4
-        }
-        if offset % offset_alignment != 0 {
-            return Err(TransferError::UnalignedBufferOffset(offset));
-        }
-
-        // The alignment of row_stride_bytes is only required if there are
-        // multiple rows
-        if requires_multiple_rows && row_stride_bytes % bytes_per_row_alignment != 0 {
-            return Err(TransferError::UnalignedBytesPerRow);
-        }
-    }
-
     // Avoid underflow in the subtraction by checking bytes_in_copy against buffer_size first.
     if bytes_in_copy > buffer_size || offset > buffer_size - bytes_in_copy {
         return Err(TransferError::BufferOverrun {
@@ -425,7 +406,15 @@ pub(crate) fn validate_texture_copy_dst_format(
 ///  * The copy must be from/to a single aspect of the texture.
 ///  * If `aligned` is true, the buffer offset must be aligned appropriately.
 ///
-/// The following steps in the algorithm are implemented elsewhere:
+/// And implements the following check from WebGPU's [validating GPUTexelCopyBufferInfo][vtcbi]
+/// algorithm:
+///  * If `aligned` is true, `bytesPerRow` must be a multiple of 256.
+///
+/// Note that the `bytesPerRow` alignment check is enforced whenever
+/// `bytesPerRow` is specified, even if the transfer is not multiple rows and
+/// `bytesPerRow` could have been omitted.
+///
+/// The following steps in [validating texture buffer copy][vtbc] 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
@@ -435,11 +424,12 @@ pub(crate) fn validate_texture_copy_dst_format(
 ///    non-copyable format.
 ///
 /// [vtbc]: https://gpuweb.github.io/gpuweb/#abstract-opdef-validating-texture-buffer-copy
+/// [vtcbi]: https://www.w3.org/TR/webgpu/#abstract-opdef-validating-gputexelcopybufferinfo
 pub(crate) fn validate_texture_buffer_copy<T>(
     texture_copy_view: &wgt::TexelCopyTextureInfo<T>,
     aspect: hal::FormatAspects,
     desc: &wgt::TextureDescriptor<(), Vec<wgt::TextureFormat>>,
-    offset: BufferAddress,
+    layout: &wgt::TexelCopyBufferLayout,
     aligned: bool,
 ) -> Result<(), TransferError> {
     if desc.sample_count != 1 {
@@ -464,8 +454,14 @@ pub(crate) fn validate_texture_buffer_copy<T>(
             .expect("non-copyable formats should have been rejected previously")
     };
 
-    if aligned && offset % u64::from(offset_alignment) != 0 {
-        return Err(TransferError::UnalignedBufferOffset(offset));
+    if aligned && layout.offset % u64::from(offset_alignment) != 0 {
+        return Err(TransferError::UnalignedBufferOffset(layout.offset));
+    }
+
+    if let Some(bytes_per_row) = layout.bytes_per_row {
+        if aligned && bytes_per_row % wgt::COPY_BYTES_PER_ROW_ALIGNMENT != 0 {
+            return Err(TransferError::UnalignedBytesPerRow);
+        }
     }
 
     Ok(())
@@ -1004,7 +1000,7 @@ impl Global {
                 destination,
                 dst_base.aspect,
                 &dst_texture.desc,
-                source.layout.offset,
+                &source.layout,
                 true, // alignment required for buffer offset
             )?;
 
@@ -1016,7 +1012,6 @@ impl Global {
                     src_buffer.size,
                     CopySide::Source,
                     copy_size,
-                    true,
                 )?;
 
             if dst_texture.desc.format.is_depth_stencil_format() {
@@ -1125,7 +1120,7 @@ impl Global {
                 source,
                 src_base.aspect,
                 &src_texture.desc,
-                destination.layout.offset,
+                &destination.layout,
                 true, // alignment required for buffer offset
             )?;
 
@@ -1137,7 +1132,6 @@ impl Global {
                     dst_buffer.size,
                     CopySide::Destination,
                     copy_size,
-                    true,
                 )?;
 
             if src_texture.desc.format.is_depth_stencil_format() {

wgpu-core/src/device/queue.rs

@@ -763,8 +763,8 @@ impl Queue {
             &destination,
             dst_base.aspect,
             &dst.desc,
-            data_layout.offset,
-            false, // alignment not required for buffer offset
+            &data_layout,
+            false, // alignment not required for buffer offset or bytes per row
         )?;
 
         // Note: `_source_bytes_per_array_layer` is ignored since we
@@ -776,7 +776,6 @@ impl Queue {
             data.len() as wgt::BufferAddress,
             CopySide::Source,
             size,
-            false,
         )?;
 
         if dst.desc.format.is_depth_stencil_format() {