From 37de3a35745ed89994ed1d71fc57db3289b48745 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 5 Oct 2022 23:45:12 +0200 Subject: [PATCH] Set the image stride to zero when updating a single layer on metal. (#3063) Co-authored-by: Connor Fitzgerald --- CHANGELOG.md | 1 + wgpu-hal/src/metal/command.rs | 16 ++++-- wgpu/tests/root.rs | 1 + wgpu/tests/write_texture.rs | 98 +++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 wgpu/tests/write_texture.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c73299d22..e4603fc07 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) - Fix `_buffer_sizes` encoding by @dtiselice in [#3047](https://github.com/gfx-rs/wgpu/pull/3047) #### Vulkan diff --git a/wgpu-hal/src/metal/command.rs b/wgpu-hal/src/metal/command.rs index 958b3c4d3..3bc07d21e 100644 --- a/wgpu-hal/src/metal/command.rs +++ b/wgpu-hal/src/metal/command.rs @@ -227,15 +227,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, diff --git a/wgpu/tests/root.rs b/wgpu/tests/root.rs index 788767131..06a98a9ee 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 000000000..742e97d81 --- /dev/null +++ b/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 = 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); + } + }); +}