From ca7abcb538d7835f3f68b8cd017f8637a85c600f Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Tue, 19 Sep 2023 12:04:23 +0200 Subject: [PATCH 1/4] modify test to trigger validation error --- tests/tests/write_texture.rs | 52 +++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/tests/tests/write_texture.rs b/tests/tests/write_texture.rs index 8b33cae7f5..68daae6648 100644 --- a/tests/tests/write_texture.rs +++ b/tests/tests/write_texture.rs @@ -51,9 +51,16 @@ fn write_texture_subset_2d() { ctx.queue.submit(None); - let read_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor { + let read_buffer_0 = ctx.device.create_buffer(&wgpu::BufferDescriptor { label: None, - size: (size * size) as u64, + size: (size * 2) as u64, + usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST, + mapped_at_creation: false, + }); + + let read_buffer_1 = ctx.device.create_buffer(&wgpu::BufferDescriptor { + label: None, + size: (size * (size - 2)) as u64, usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST, mapped_at_creation: false, }); @@ -70,7 +77,7 @@ fn write_texture_subset_2d() { aspect: wgpu::TextureAspect::All, }, wgpu::ImageCopyBuffer { - buffer: &read_buffer, + buffer: &read_buffer_0, layout: wgpu::ImageDataLayout { offset: 0, bytes_per_row: Some(size), @@ -79,22 +86,49 @@ fn write_texture_subset_2d() { }, wgpu::Extent3d { width: size, - height: size, + height: 2, + depth_or_array_layers: 1, + }, + ); + + encoder.copy_texture_to_buffer( + wgpu::ImageCopyTexture { + texture: &tex, + mip_level: 0, + origin: wgpu::Origin3d { x: 0, y: 2, z: 0 }, + aspect: wgpu::TextureAspect::All, + }, + wgpu::ImageCopyBuffer { + buffer: &read_buffer_1, + layout: wgpu::ImageDataLayout { + offset: 0, + bytes_per_row: Some(size), + rows_per_image: Some(size), + }, + }, + wgpu::Extent3d { + width: size, + height: size - 2, depth_or_array_layers: 1, }, ); ctx.queue.submit(Some(encoder.finish())); - let slice = read_buffer.slice(..); - slice.map_async(wgpu::MapMode::Read, |_| ()); + let slice_0 = read_buffer_0.slice(..); + slice_0.map_async(wgpu::MapMode::Read, |_| ()); + let slice_1 = read_buffer_1.slice(..); + slice_1.map_async(wgpu::MapMode::Read, |_| ()); + ctx.device.poll(wgpu::Maintain::Wait); - let data: Vec = slice.get_mapped_range().to_vec(); - for byte in &data[..(size as usize * 2)] { + let data_0 = slice_0.get_mapped_range(); + let data_1 = slice_1.get_mapped_range(); + + for byte in data_0.as_ref() { assert_eq!(*byte, 1); } - for byte in &data[(size as usize * 2)..] { + for byte in data_1.as_ref() { assert_eq!(*byte, 0); } }); From ee10bb603d2dd3b9424b4cedaebf948ce45b7145 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 21 Sep 2023 18:12:38 +0200 Subject: [PATCH 2/4] set `METAL_DEVICE_WRAPPER_TYPE=1` --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 981fcd3498..2b755682ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,7 +280,7 @@ jobs: for backend in ${{ matrix.backends }}; do echo "======= NATIVE TESTS $backend ======"; - WGPU_BACKEND=$backend cargo llvm-cov --no-cfg-coverage nextest --no-fail-fast --no-report --features vulkan-portability + WGPU_BACKEND=$backend METAL_DEVICE_WRAPPER_TYPE=1 cargo llvm-cov --no-cfg-coverage nextest --no-fail-fast --no-report --features vulkan-portability done - uses: actions/upload-artifact@v3 From cc3335b3c7233e1084043b1e536f07e88949b031 Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 21 Sep 2023 18:50:00 +0200 Subject: [PATCH 3/4] metal only --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2b755682ed..4ba2da6a05 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -280,7 +280,10 @@ jobs: for backend in ${{ matrix.backends }}; do echo "======= NATIVE TESTS $backend ======"; - WGPU_BACKEND=$backend METAL_DEVICE_WRAPPER_TYPE=1 cargo llvm-cov --no-cfg-coverage nextest --no-fail-fast --no-report --features vulkan-portability + if ["$backend" == "metal"]; then + echo "METAL_DEVICE_WRAPPER_TYPE=1" >> "$GITHUB_ENV" + fi + WGPU_BACKEND=$backend cargo llvm-cov --no-cfg-coverage nextest --no-fail-fast --no-report --features vulkan-portability done - uses: actions/upload-artifact@v3 From 7f94f6325a4f8312aa190646d1c1041f936e82ae Mon Sep 17 00:00:00 2001 From: teoxoy <28601907+teoxoy@users.noreply.github.com> Date: Thu, 21 Sep 2023 19:06:29 +0200 Subject: [PATCH 4/4] [metal] set the image stride to zero when updating a single layer of a 3D texture --- wgpu-hal/src/metal/command.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/wgpu-hal/src/metal/command.rs b/wgpu-hal/src/metal/command.rs index c4b37f9932..65de187efc 100644 --- a/wgpu-hal/src/metal/command.rs +++ b/wgpu-hal/src/metal/command.rs @@ -360,10 +360,16 @@ impl crate::CommandEncoder for super::CommandEncoder { .max_copy_size(&src.copy_size) .min(©.size); let bytes_per_row = copy.buffer_layout.bytes_per_row.unwrap_or(0) as u64; - let bytes_per_image = copy - .buffer_layout - .rows_per_image - .map_or(0, |v| v as u64 * bytes_per_row); + let image_byte_stride = if extent.depth > 1 { + copy.buffer_layout + .rows_per_image + .map_or(0, |v| v 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_texture_to_buffer( &src.raw, copy.texture_base.array_layer as u64, @@ -373,7 +379,7 @@ impl crate::CommandEncoder for super::CommandEncoder { &dst.raw, copy.buffer_layout.offset, bytes_per_row, - bytes_per_image, + image_byte_stride, conv::get_blit_option(src.format, copy.texture_base.aspect), ); }