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.
This commit is contained in:
		
							parent
							
								
									b09bbfa905
								
							
						
					
					
						commit
						54a3fd7754
					
				| @ -27,7 +27,7 @@ use bevy_utils::{tracing::warn, HashMap}; | |||||||
| use encase::internal::ReadFrom; | use encase::internal::ReadFrom; | ||||||
| use encase::private::Reader; | use encase::private::Reader; | ||||||
| use encase::ShaderType; | use encase::ShaderType; | ||||||
| use wgpu::{CommandEncoder, COPY_BYTES_PER_ROW_ALIGNMENT}; | use wgpu::CommandEncoder; | ||||||
| 
 | 
 | ||||||
| /// A plugin that enables reading back gpu buffers and textures to the cpu.
 | /// A plugin that enables reading back gpu buffers and textures to the cpu.
 | ||||||
| pub struct GpuReadbackPlugin { | pub struct GpuReadbackPlugin { | ||||||
| @ -348,14 +348,17 @@ fn map_buffers(mut readbacks: ResMut<GpuReadbacks>) { | |||||||
| 
 | 
 | ||||||
| // Utils
 | // Utils
 | ||||||
| 
 | 
 | ||||||
| pub(crate) fn align_byte_size(value: u32) -> u32 { | /// Round up a given value to be a multiple of [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
 | ||||||
|     value + (COPY_BYTES_PER_ROW_ALIGNMENT - (value % COPY_BYTES_PER_ROW_ALIGNMENT)) | pub(crate) const fn align_byte_size(value: u32) -> u32 { | ||||||
|  |     RenderDevice::align_copy_bytes_per_row(value as usize) as u32 | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| pub(crate) fn get_aligned_size(width: u32, height: u32, pixel_size: u32) -> u32 { | /// Get the size of a image when the size of each row has been rounded up to [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
 | ||||||
|  | pub(crate) const fn get_aligned_size(width: u32, height: u32, pixel_size: u32) -> u32 { | ||||||
|     height * align_byte_size(width * pixel_size) |     height * align_byte_size(width * pixel_size) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | /// Get a [`ImageDataLayout`] aligned such that the image can be copied into a buffer.
 | ||||||
| pub(crate) fn layout_data(width: u32, height: u32, format: TextureFormat) -> ImageDataLayout { | pub(crate) fn layout_data(width: u32, height: u32, format: TextureFormat) -> ImageDataLayout { | ||||||
|     ImageDataLayout { |     ImageDataLayout { | ||||||
|         bytes_per_row: if height > 1 { |         bytes_per_row: if height > 1 { | ||||||
|  | |||||||
| @ -231,10 +231,16 @@ impl RenderDevice { | |||||||
|         buffer.map_async(map_mode, callback); |         buffer.map_async(map_mode, callback); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     pub fn align_copy_bytes_per_row(row_bytes: usize) -> usize { |     // Rounds up `row_bytes` to be a multiple of [`wgpu::COPY_BYTES_PER_ROW_ALIGNMENT`].
 | ||||||
|  |     pub const fn align_copy_bytes_per_row(row_bytes: usize) -> usize { | ||||||
|         let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT as usize; |         let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT as usize; | ||||||
|         let padded_bytes_per_row_padding = (align - row_bytes % align) % align; | 
 | ||||||
|         row_bytes + padded_bytes_per_row_padding |         // If row_bytes is aligned calculate a value just under the next aligned value.
 | ||||||
|  |         // Otherwise calculate a value greater than the next aligned value.
 | ||||||
|  |         let over_aligned = row_bytes + align - 1; | ||||||
|  | 
 | ||||||
|  |         // Round the number *down* to the nearest aligned value.
 | ||||||
|  |         (over_aligned / align) * align | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     pub fn get_supported_read_only_binding_type( |     pub fn get_supported_read_only_binding_type( | ||||||
| @ -248,3 +254,19 @@ impl RenderDevice { | |||||||
|         } |         } | ||||||
|     } |     } | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | #[cfg(test)] | ||||||
|  | mod tests { | ||||||
|  |     use super::*; | ||||||
|  | 
 | ||||||
|  |     #[test] | ||||||
|  |     fn align_copy_bytes_per_row() { | ||||||
|  |         // Test for https://github.com/bevyengine/bevy/issues/16992
 | ||||||
|  |         let align = wgpu::COPY_BYTES_PER_ROW_ALIGNMENT as usize; | ||||||
|  | 
 | ||||||
|  |         assert_eq!(RenderDevice::align_copy_bytes_per_row(0), 0); | ||||||
|  |         assert_eq!(RenderDevice::align_copy_bytes_per_row(1), align); | ||||||
|  |         assert_eq!(RenderDevice::align_copy_bytes_per_row(align + 1), align * 2); | ||||||
|  |         assert_eq!(RenderDevice::align_copy_bytes_per_row(align), align); | ||||||
|  |     } | ||||||
|  | } | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Brezak
						Brezak