From 2fc8ad65f9a8ab94dd78b844466c3c8a7e9c0df7 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 5 Oct 2022 14:06:24 +0200 Subject: [PATCH 1/5] Set the image stride to zero when updating a single layer on metal. --- 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 33e282ac2f..90509e2ae9 100644 --- a/wgpu-hal/src/metal/command.rs +++ b/wgpu-hal/src/metal/command.rs @@ -228,15 +228,21 @@ impl crate::CommandEncoder 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, From b72ce5269b774c4bb546a33cfdfa36a8d925d813 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 5 Oct 2022 14:55:20 +0200 Subject: [PATCH 2/5] Add a test. The test is more valuable when run with the validation layer enabled, but even without it, it's better than nothing. --- wgpu/tests/root.rs | 1 + wgpu/tests/write_texture.rs | 101 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100644 wgpu/tests/write_texture.rs diff --git a/wgpu/tests/root.rs b/wgpu/tests/root.rs index 788767131e..06a98a9ee3 100644 --- a/wgpu/tests/root.rs +++ b/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; diff --git a/wgpu/tests/write_texture.rs b/wgpu/tests/write_texture.rs new file mode 100644 index 0000000000..ee16ef121f --- /dev/null +++ b/wgpu/tests/write_texture.rs @@ -0,0 +1,101 @@ +//! Tests for texture copy + +use wgpu::{Extent3d, ImageDataLayout}; +use wgt::BufferAddress; + +use crate::common::{initialize_test, TestParameters}; + +use std::num::NonZeroU32; + +#[test] +fn write_texture_subset() { + let size = 256; + let mut parameters = TestParameters::default(); + 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 = 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); + } + }); +} From 18aa5c939f732b17a5d39a51bba19e6fd3d15aad Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 5 Oct 2022 14:56:57 +0200 Subject: [PATCH 3/5] Update the changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fd4e5348e..225996226c 100644 --- a/CHANGELOG.md +++ b/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) #### Vulkan - Fix `astc_hdr` formats support by @jinleili in [#2971]](https://github.com/gfx-rs/wgpu/pull/2971) From 93c2e991c2631d02ef743445f276e679d29c81a9 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Wed, 5 Oct 2022 16:51:25 -0400 Subject: [PATCH 4/5] Fix warnings --- wgpu/tests/write_texture.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/wgpu/tests/write_texture.rs b/wgpu/tests/write_texture.rs index ee16ef121f..cce2524475 100644 --- a/wgpu/tests/write_texture.rs +++ b/wgpu/tests/write_texture.rs @@ -1,8 +1,5 @@ //! Tests for texture copy -use wgpu::{Extent3d, ImageDataLayout}; -use wgt::BufferAddress; - use crate::common::{initialize_test, TestParameters}; use std::num::NonZeroU32; @@ -10,7 +7,7 @@ use std::num::NonZeroU32; #[test] fn write_texture_subset() { let size = 256; - let mut parameters = TestParameters::default(); + let parameters = TestParameters::default(); initialize_test(parameters, |ctx| { let tex = ctx.device.create_texture(&wgpu::TextureDescriptor { label: None, From fb6abd6e11b2c70bbce805df388ece6e672d6b16 Mon Sep 17 00:00:00 2001 From: Connor Fitzgerald Date: Wed, 5 Oct 2022 17:19:22 -0400 Subject: [PATCH 5/5] mark test as failing DX12 --- wgpu/tests/write_texture.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu/tests/write_texture.rs b/wgpu/tests/write_texture.rs index cce2524475..742e97d818 100644 --- a/wgpu/tests/write_texture.rs +++ b/wgpu/tests/write_texture.rs @@ -7,7 +7,7 @@ use std::num::NonZeroU32; #[test] fn write_texture_subset() { let size = 256; - let parameters = TestParameters::default(); + let parameters = TestParameters::default().backend_failure(wgpu::Backends::DX12); initialize_test(parameters, |ctx| { let tex = ctx.device.create_texture(&wgpu::TextureDescriptor { label: None,