Set the image stride to zero when updating a single layer on metal. (#…

…3063)

Co-authored-by: Connor Fitzgerald <[email protected]>

CHANGELOG.md

@@ -93,6 +93,7 @@ SurfaceConfiguration {
 - Add the missing `msg_send![view, retain]` call within `from_view` by @jinleili in [#2976](https://github.com/gfx-rs/wgpu/pull/2976)
 - Fix `max_buffer` `max_texture` and `max_vertex_buffers` limits by @jinleili in [#2978](https://github.com/gfx-rs/wgpu/pull/2978)
 - Remove PrivateCapabilities's `format_rgb10a2_unorm_surface` field by @jinleili in [#2981](https://github.com/gfx-rs/wgpu/pull/2981)
+- Fix validation error when copying into a subset of a single-layer texture by @nical in [#3063](https://github.com/gfx-rs/wgpu/pull/3063)
 - Fix `_buffer_sizes` encoding by @dtiselice in [#3047](https://github.com/gfx-rs/wgpu/pull/3047)
 
 #### Vulkan

wgpu-hal/src/metal/command.rs

@@ -227,15 +227,21 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
                 .buffer_layout
                 .bytes_per_row
                 .map_or(0, |v| v.get() as u64);
-            let bytes_per_image = copy
-                .buffer_layout
-                .rows_per_image
-                .map_or(0, |v| v.get() as u64 * bytes_per_row);
+            let image_byte_stride = if extent.depth > 1 {
+                copy.buffer_layout
+                    .rows_per_image
+                    .map_or(0, |v| v.get() as u64 * bytes_per_row)
+            } else {
+                // Don't pass a stride when updating a single layer, otherwise metal validation
+                // fails when updating a subset of the image due to the stride being larger than
+                // the amount of data to copy.
+                0
+            };
             encoder.copy_from_buffer_to_texture(
                 &src.raw,
                 copy.buffer_layout.offset,
                 bytes_per_row,
-                bytes_per_image,
+                image_byte_stride,
                 conv::map_copy_extent(&extent),
                 &dst.raw,
                 copy.texture_base.array_layer as u64,

wgpu/tests/root.rs

@@ -12,4 +12,5 @@ mod resource_descriptor_accessor;
 mod shader_primitive_index;
 mod texture_bounds;
 mod vertex_indices;
+mod write_texture;
 mod zero_init_texture_after_discard;

wgpu/tests/write_texture.rs

@@ -0,0 +1,98 @@
+//! Tests for texture copy
+
+use crate::common::{initialize_test, TestParameters};
+
+use std::num::NonZeroU32;
+
+#[test]
+fn write_texture_subset() {
+    let size = 256;
+    let parameters = TestParameters::default().backend_failure(wgpu::Backends::DX12);
+    initialize_test(parameters, |ctx| {
+        let tex = ctx.device.create_texture(&wgpu::TextureDescriptor {
+            label: None,
+            dimension: wgpu::TextureDimension::D2,
+            size: wgpu::Extent3d {
+                width: size,
+                height: size,
+                depth_or_array_layers: 1,
+            },
+            format: wgpu::TextureFormat::R8Uint,
+            usage: wgpu::TextureUsages::COPY_DST
+                | wgpu::TextureUsages::COPY_SRC
+                | wgpu::TextureUsages::TEXTURE_BINDING,
+            mip_level_count: 1,
+            sample_count: 1,
+        });
+        let data = vec![1u8; size as usize * 2];
+        // Write the first two rows
+        ctx.queue.write_texture(
+            wgpu::ImageCopyTexture {
+                texture: &tex,
+                mip_level: 0,
+                origin: wgpu::Origin3d::ZERO,
+                aspect: wgpu::TextureAspect::All,
+            },
+            bytemuck::cast_slice(&data),
+            wgpu::ImageDataLayout {
+                offset: 0,
+                bytes_per_row: std::num::NonZeroU32::new(size),
+                rows_per_image: std::num::NonZeroU32::new(size),
+            },
+            wgpu::Extent3d {
+                width: size,
+                height: 2,
+                depth_or_array_layers: 1,
+            },
+        );
+
+        ctx.queue.submit(None);
+
+        let read_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
+            label: None,
+            size: (size * size) as u64,
+            usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST,
+            mapped_at_creation: false,
+        });
+
+        let mut encoder = ctx
+            .device
+            .create_command_encoder(&wgpu::CommandEncoderDescriptor { label: None });
+
+        encoder.copy_texture_to_buffer(
+            wgpu::ImageCopyTexture {
+                texture: &tex,
+                mip_level: 0,
+                origin: wgpu::Origin3d::ZERO,
+                aspect: wgpu::TextureAspect::All,
+            },
+            wgpu::ImageCopyBuffer {
+                buffer: &read_buffer,
+                layout: wgpu::ImageDataLayout {
+                    offset: 0,
+                    bytes_per_row: NonZeroU32::new(size),
+                    rows_per_image: NonZeroU32::new(size),
+                },
+            },
+            wgpu::Extent3d {
+                width: size,
+                height: size,
+                depth_or_array_layers: 1,
+            },
+        );
+
+        ctx.queue.submit(Some(encoder.finish()));
+
+        let slice = read_buffer.slice(..);
+        slice.map_async(wgpu::MapMode::Read, |_| ());
+        ctx.device.poll(wgpu::Maintain::Wait);
+        let data: Vec<u8> = slice.get_mapped_range().to_vec();
+
+        for byte in &data[..(size as usize * 2)] {
+            assert_eq!(*byte, 1);
+        }
+        for byte in &data[(size as usize * 2)..] {
+            assert_eq!(*byte, 0);
+        }
+    });
+}