Skip to content

Commit 54a3fd7

Browse files
authored
Don't overalign aligned values in gpu_readback::align_byte_size (#17007)
# Objective Fix alignment calculations in our rendering code. Fixes #16992 The `gpu_readback::align_byte_size` function incorrectly rounds aligned values to the next alignment. If we assume the alignment to be 256 (because that's what wgpu says it its) the function would align 0 to 256, 256 to 512, etc... ## Solution Forward the `gpu_readback::align_byte_size` to `RenderDevice::align_copy_bytes_per_row` so we don't implement the same method twice. Simplify `RenderDevice::align_copy_bytes_per_row`. ## Testing Ran the code provided in #16992 to see if the issue has been solved + added a test to check if `align_copy_bytes_per_row` returns the correct values.
1 parent b09bbfa commit 54a3fd7

File tree

2 files changed

+32
-7
lines changed

2 files changed

+32
-7
lines changed

crates/bevy_render/src/gpu_readback.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use bevy_utils::{tracing::warn, HashMap};
2727
use encase::internal::ReadFrom;
2828
use encase::private::Reader;
2929
use encase::ShaderType;
30-
use wgpu::{CommandEncoder, COPY_BYTES_PER_ROW_ALIGNMENT};
30+
use wgpu::CommandEncoder;
3131

3232
/// A plugin that enables reading back gpu buffers and textures to the cpu.
3333
pub struct GpuReadbackPlugin {
@@ -348,14 +348,17 @@ fn map_buffers(mut readbacks: ResMut<GpuReadbacks>) {
348348

349349
// Utils
350350

351-
pub(crate) fn align_byte_size(value: u32) -> u32 {
352-
value + (COPY_BYTES_PER_ROW_ALIGNMENT - (value % COPY_BYTES_PER_ROW_ALIGNMENT))
351+
/// Round up a given value to be a multiple of [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
352+
pub(crate) const fn align_byte_size(value: u32) -> u32 {
353+
RenderDevice::align_copy_bytes_per_row(value as usize) as u32
353354
}
354355

355-
pub(crate) fn get_aligned_size(width: u32, height: u32, pixel_size: u32) -> u32 {
356+
/// Get the size of a image when the size of each row has been rounded up to [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
357+
pub(crate) const fn get_aligned_size(width: u32, height: u32, pixel_size: u32) -> u32 {
356358
height * align_byte_size(width * pixel_size)
357359
}
358360

361+
/// Get a [`ImageDataLayout`] aligned such that the image can be copied into a buffer.
359362
pub(crate) fn layout_data(width: u32, height: u32, format: TextureFormat) -> ImageDataLayout {
360363
ImageDataLayout {
361364
bytes_per_row: if height > 1 {

crates/bevy_render/src/renderer/render_device.rs

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,10 +231,16 @@ impl RenderDevice {
231231
buffer.map_async(map_mode, callback);
232232
}
233233

234-
pub fn align_copy_bytes_per_row(row_bytes: usize) -> usize {
234+
// Rounds up `row_bytes` to be a multiple of [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
235+
pub const fn align_copy_bytes_per_row(row_bytes: usize) -> usize {
235236
let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT as usize;
236-
let padded_bytes_per_row_padding = (align - row_bytes % align) % align;
237-
row_bytes + padded_bytes_per_row_padding
237+
238+
// If row_bytes is aligned calculate a value just under the next aligned value.
239+
// Otherwise calculate a value greater than the next aligned value.
240+
let over_aligned = row_bytes + align - 1;
241+
242+
// Round the number *down* to the nearest aligned value.
243+
(over_aligned / align) * align
238244
}
239245

240246
pub fn get_supported_read_only_binding_type(
@@ -248,3 +254,19 @@ impl RenderDevice {
248254
}
249255
}
250256
}
257+
258+
#[cfg(test)]
259+
mod tests {
260+
use super::*;
261+
262+
#[test]
263+
fn align_copy_bytes_per_row() {
264+
// Test for https://github.com/bevyengine/bevy/issues/16992
265+
let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT as usize;
266+
267+
assert_eq!(RenderDevice::align_copy_bytes_per_row(0), 0);
268+
assert_eq!(RenderDevice::align_copy_bytes_per_row(1), align);
269+
assert_eq!(RenderDevice::align_copy_bytes_per_row(align + 1), align * 2);
270+
assert_eq!(RenderDevice::align_copy_bytes_per_row(align), align);
271+
}
272+
}

0 commit comments

Comments
 (0)