Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 11 additions & 5 deletions wgpu-hal/src/metal/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,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,
Expand Down
1 change: 1 addition & 0 deletions wgpu/tests/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
101 changes: 101 additions & 0 deletions wgpu/tests/write_texture.rs
Original file line number Diff line number Diff line change
@@ -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<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);
}
});
}