Skip to content

Commit 0ea7294

Browse files
fix(hal/gles): copy every depth slice in copy_texture_to_texture
The `C::CopyTextureToTexture` arm never read `copy.size.depth`. It attached one slice to the read framebuffer and issued a single `copy_tex_sub_image_*`, so a copy between 3D textures transferred only z slice 0. Nothing errored and no validation tripped; the remaining slices were silently left untouched. GL has no call that copies a volume between textures, because the read framebuffer holds one 2D slice at a time, so loop over the depth extent and copy one slice per iteration. The source slice index was also taken from `copy.src_base.array_layer` while the destination used `get_z_offset()`. For `TEXTURE_3D` the z coordinate lives in `origin.z` rather than `array_layer`, so a copy with a nonzero source z read the wrong slice; both sides now use `get_z_offset()`. Non-layered sources go through `get_2d_target()` as the destination already did, so a cube map source selects the right face rather than passing `TEXTURE_CUBE_MAP` to `glFramebufferTexture2D`. Add a test copying a four-slice 3D texture with a distinct value per slice. The existing `copy_texture_to_texture` coverage only ever copies a single slice, which is how this went unnoticed.
1 parent d5d8ed8 commit 0ea7294

3 files changed

Lines changed: 136 additions & 51 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ Bottom level categories:
107107
#### GLES
108108

109109
- Fixed signed integer `%` (and `%=`) returning the wrong result for negative operands in the GLSL (OpenGL/GLES) backend, e.g. `-1 % 768` yielding `255` instead of `-1`. GLSL's `%` is undefined when either operand is negative, so signed remainder is now lowered as `a - b * (a / b)`, matching the SPIR-V, HLSL, and Metal backends. By @mstampfli in [#9687](https://github.com/gfx-rs/wgpu/pull/9687).
110+
- Fixed `copy_texture_to_texture()` copying only the first depth slice when the copy extent had a depth greater than 1, so copies between 3D textures silently lost every slice after the first. Copies with a nonzero source z also read the wrong slice. By @AlexEgger2Build in [#10004](https://github.com/gfx-rs/wgpu/pull/10004).
110111

111112
#### WebGPU
112113

tests/tests/wgpu-gpu/transfer.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,88 @@ use wgpu_test::{apply, fail, gpu_test, GpuTestConfiguration, GpuTestInitializer,
22

33
pub fn all_tests(vec: &mut Vec<GpuTestInitializer>) {
44
vec.push(COPY_OVERFLOW_Z);
5+
vec.push(COPY_TEXTURE_TO_TEXTURE_3D);
56
}
67

8+
/// Regression test for the GLES backend copying only z slice 0 of a 3D texture.
9+
#[apply(gpu_test!)]
10+
static COPY_TEXTURE_TO_TEXTURE_3D: GpuTestConfiguration =
11+
GpuTestConfiguration::new().run_async(|ctx| async move {
12+
// Each slice is filled with a distinct value, so a dropped or misplaced slice is
13+
// caught rather than only a wrong copy length. `width` is a multiple of
14+
// `COPY_BYTES_PER_ROW_ALIGNMENT` so `bytes_per_row` stays aligned.
15+
let width = 256;
16+
let height = 2;
17+
let depth = 4;
18+
let slice_len = (width * height) as usize;
19+
20+
let descriptor = wgpu::TextureDescriptor {
21+
label: None,
22+
dimension: wgpu::TextureDimension::D3,
23+
size: wgpu::Extent3d {
24+
width,
25+
height,
26+
depth_or_array_layers: depth,
27+
},
28+
format: wgpu::TextureFormat::R8Uint,
29+
usage: wgpu::TextureUsages::COPY_DST | wgpu::TextureUsages::COPY_SRC,
30+
mip_level_count: 1,
31+
sample_count: 1,
32+
view_formats: &[],
33+
};
34+
let src = ctx.device.create_texture(&descriptor);
35+
let dst = ctx.device.create_texture(&descriptor);
36+
37+
let data: Vec<u8> = (0..depth)
38+
.flat_map(|z| vec![(z + 1) as u8; slice_len])
39+
.collect();
40+
let layout = wgpu::TexelCopyBufferLayout {
41+
offset: 0,
42+
bytes_per_row: Some(width),
43+
rows_per_image: Some(height),
44+
};
45+
ctx.queue
46+
.write_texture(src.as_image_copy(), &data, layout, descriptor.size);
47+
48+
let read_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
49+
label: None,
50+
size: data.len() as u64,
51+
usage: wgpu::BufferUsages::MAP_READ | wgpu::BufferUsages::COPY_DST,
52+
mapped_at_creation: false,
53+
});
54+
55+
let mut encoder = ctx
56+
.device
57+
.create_command_encoder(&wgpu::CommandEncoderDescriptor { label: None });
58+
encoder.copy_texture_to_texture(src.as_image_copy(), dst.as_image_copy(), descriptor.size);
59+
encoder.copy_texture_to_buffer(
60+
dst.as_image_copy(),
61+
wgpu::TexelCopyBufferInfo {
62+
buffer: &read_buffer,
63+
layout,
64+
},
65+
descriptor.size,
66+
);
67+
ctx.queue.submit(Some(encoder.finish()));
68+
69+
let slice = read_buffer.slice(..);
70+
slice.map_async(wgpu::MapMode::Read, |_| ());
71+
ctx.async_poll(wgpu::PollType::wait_indefinitely())
72+
.await
73+
.unwrap();
74+
let read: Vec<u8> = slice.get_mapped_range().unwrap().to_vec();
75+
76+
for z in 0..depth as usize {
77+
let expected = (z + 1) as u8;
78+
let got = &read[z * slice_len..(z + 1) * slice_len];
79+
assert!(
80+
got.iter().all(|&texel| texel == expected),
81+
"slice z={z} not copied: expected all {expected}, got {:?}",
82+
&got[..8],
83+
);
84+
}
85+
});
86+
787
#[apply(gpu_test!)]
888
static COPY_OVERFLOW_Z: GpuTestConfiguration = GpuTestConfiguration::new()
989
.parameters(TestParameters::default().enable_noop())

wgpu-hal/src/gles/queue.rs

Lines changed: 55 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -696,59 +696,63 @@ impl super::Queue {
696696
dst_target,
697697
ref copy,
698698
} => {
699-
//TODO: handle 3D copies
700699
unsafe { gl.bind_framebuffer(glow::READ_FRAMEBUFFER, Some(self.copy_fbo)) };
701-
if is_layered_target(src_target) {
702-
//TODO: handle GLES without framebuffer_texture_3d
703-
unsafe {
704-
gl.framebuffer_texture_layer(
705-
glow::READ_FRAMEBUFFER,
706-
glow::COLOR_ATTACHMENT0,
707-
Some(src),
708-
copy.src_base.mip_level as i32,
709-
copy.src_base.array_layer as i32,
710-
)
711-
};
712-
} else {
713-
unsafe {
714-
gl.framebuffer_texture_2d(
715-
glow::READ_FRAMEBUFFER,
716-
glow::COLOR_ATTACHMENT0,
717-
src_target,
718-
Some(src),
719-
copy.src_base.mip_level as i32,
720-
)
721-
};
722-
}
723-
724700
unsafe { gl.bind_texture(dst_target, Some(dst)) };
725-
if is_layered_target(dst_target) {
726-
unsafe {
727-
gl.copy_tex_sub_image_3d(
728-
dst_target,
729-
copy.dst_base.mip_level as i32,
730-
copy.dst_base.origin.x as i32,
731-
copy.dst_base.origin.y as i32,
732-
get_z_offset(dst_target, &copy.dst_base) as i32,
733-
copy.src_base.origin.x as i32,
734-
copy.src_base.origin.y as i32,
735-
copy.size.width as i32,
736-
copy.size.height as i32,
737-
)
738-
};
739-
} else {
740-
unsafe {
741-
gl.copy_tex_sub_image_2d(
742-
get_2d_target(dst_target, copy.dst_base.array_layer),
743-
copy.dst_base.mip_level as i32,
744-
copy.dst_base.origin.x as i32,
745-
copy.dst_base.origin.y as i32,
746-
copy.src_base.origin.x as i32,
747-
copy.src_base.origin.y as i32,
748-
copy.size.width as i32,
749-
copy.size.height as i32,
750-
)
751-
};
701+
702+
// The read framebuffer holds a single 2D slice at a time, so a copy of
703+
// depth > 1 is issued one slice per iteration.
704+
for z in 0..copy.size.depth {
705+
if is_layered_target(src_target) {
706+
//TODO: handle GLES without framebuffer_texture_3d
707+
unsafe {
708+
gl.framebuffer_texture_layer(
709+
glow::READ_FRAMEBUFFER,
710+
glow::COLOR_ATTACHMENT0,
711+
Some(src),
712+
copy.src_base.mip_level as i32,
713+
(get_z_offset(src_target, &copy.src_base) + z) as i32,
714+
)
715+
};
716+
} else {
717+
unsafe {
718+
gl.framebuffer_texture_2d(
719+
glow::READ_FRAMEBUFFER,
720+
glow::COLOR_ATTACHMENT0,
721+
get_2d_target(src_target, copy.src_base.array_layer + z),
722+
Some(src),
723+
copy.src_base.mip_level as i32,
724+
)
725+
};
726+
}
727+
728+
if is_layered_target(dst_target) {
729+
unsafe {
730+
gl.copy_tex_sub_image_3d(
731+
dst_target,
732+
copy.dst_base.mip_level as i32,
733+
copy.dst_base.origin.x as i32,
734+
copy.dst_base.origin.y as i32,
735+
(get_z_offset(dst_target, &copy.dst_base) + z) as i32,
736+
copy.src_base.origin.x as i32,
737+
copy.src_base.origin.y as i32,
738+
copy.size.width as i32,
739+
copy.size.height as i32,
740+
)
741+
};
742+
} else {
743+
unsafe {
744+
gl.copy_tex_sub_image_2d(
745+
get_2d_target(dst_target, copy.dst_base.array_layer + z),
746+
copy.dst_base.mip_level as i32,
747+
copy.dst_base.origin.x as i32,
748+
copy.dst_base.origin.y as i32,
749+
copy.src_base.origin.x as i32,
750+
copy.src_base.origin.y as i32,
751+
copy.size.width as i32,
752+
copy.size.height as i32,
753+
)
754+
};
755+
}
752756
}
753757
}
754758
C::CopyBufferToTexture {

0 commit comments

Comments
 (0)