Skip to content

Commit 4f1bc8e

Browse files
Brezakmockersf
authored andcommitted
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 a590adc commit 4f1bc8e

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::{default, 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 {
@@ -349,14 +349,17 @@ fn map_buffers(mut readbacks: ResMut<GpuReadbacks>) {
349349

350350
// Utils
351351

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

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

362+
/// Get a [`ImageDataLayout`] aligned such that the image can be copied into a buffer.
360363
pub(crate) fn layout_data(width: u32, height: u32, format: TextureFormat) -> ImageDataLayout {
361364
ImageDataLayout {
362365
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)