From d4f3932ae5b37d3d968616bf9ee71e502a9ff706 Mon Sep 17 00:00:00 2001 From: Josh Simmons Date: Fri, 29 Nov 2024 23:13:01 +0100 Subject: [PATCH] narcissus-gpu: Tidy up the push constants API --- .../narcissus-gpu/src/backend/vulkan/mod.rs | 12 ++++---- engine/narcissus-gpu/src/lib.rs | 29 ++++++++----------- title/shark/src/main.rs | 21 +++++++------- title/shark/tests/radix_sort.rs | 4 +-- 4 files changed, 32 insertions(+), 34 deletions(-) diff --git a/engine/narcissus-gpu/src/backend/vulkan/mod.rs b/engine/narcissus-gpu/src/backend/vulkan/mod.rs index b9618ed..1a1a014 100644 --- a/engine/narcissus-gpu/src/backend/vulkan/mod.rs +++ b/engine/narcissus-gpu/src/backend/vulkan/mod.rs @@ -2394,14 +2394,16 @@ impl Device for VulkanDevice { } } - unsafe fn cmd_push_constants_unchecked( + unsafe fn cmd_push_constants( &self, cmd_encoder: &mut CmdEncoder, stage_flags: ShaderStageFlags, offset: u32, - size: u32, - src: *const u8, + ptr: *const u8, + len: usize, ) { + let len = u32::try_from(len).unwrap(); + let cmd_encoder = self.cmd_encoder_mut(cmd_encoder); let command_buffer = cmd_encoder.command_buffer; @@ -2420,8 +2422,8 @@ impl Device for VulkanDevice { pipeline_layout, stage_flags, offset, - size, - src as *const std::ffi::c_void, + len, + ptr as *const std::ffi::c_void, ) } diff --git a/engine/narcissus-gpu/src/lib.rs b/engine/narcissus-gpu/src/lib.rs index 635beb0..24a30a5 100644 --- a/engine/narcissus-gpu/src/lib.rs +++ b/engine/narcissus-gpu/src/lib.rs @@ -1030,13 +1030,18 @@ pub trait Device { image_barriers: &[ImageBarrier], ); - unsafe fn cmd_push_constants_unchecked( + /// Incrementally update the push constants for the given shader stage flags and offset. + /// + /// # Safety + /// + /// The memory region from `ptr` through `ptr` + `len` must be valid. + unsafe fn cmd_push_constants( &self, cmd_encoder: &mut CmdEncoder, stage_flags: ShaderStageFlags, offset: u32, - size: u32, - src: *const u8, + ptr: *const u8, + len: usize, ); fn cmd_copy_buffer_to_image( @@ -1106,7 +1111,7 @@ fn overflow() -> ! { } pub trait DeviceExt: Device { - fn cmd_push_constants( + fn cmd_push_constants_with_data( &self, cmd_encoder: &mut CmdEncoder, stage_flags: ShaderStageFlags, @@ -1114,27 +1119,17 @@ pub trait DeviceExt: Device { data: &T, ) { let size = std::mem::size_of_val(data); - let src = data as *const _ as *const u8; + let ptr = data as *const _ as *const u8; // # Safety // - // The memory region from `src` through `src` + `size` must be valid as it's + // The memory region from `ptr` through `ptr` + `size` is ensured to be valid as it's // directly derived from `data`. - // - // This function will propagate undefined values from T, for example, padding - // bytes, however we promise not to materialize a rust reference to any such - // data. unsafe { if size >= u32::MAX as usize || offset >= u32::MAX as usize { overflow(); } - self.cmd_push_constants_unchecked( - cmd_encoder, - stage_flags, - offset as u32, - size as u32, - src, - ) + self.cmd_push_constants(cmd_encoder, stage_flags, offset as u32, ptr, size) } } diff --git a/title/shark/src/main.rs b/title/shark/src/main.rs index db7a294..a5e84c9 100644 --- a/title/shark/src/main.rs +++ b/title/shark/src/main.rs @@ -1273,7 +1273,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.basic_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &graphics_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::VERTEX, 0, @@ -1429,7 +1429,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.draw_2d_bin_0_clear_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1451,7 +1451,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.draw_2d_bin_1_scatter_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1469,7 +1469,8 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_dispatch( cmd_encoder, - draw_buffer_len.div_ceil(self.pipelines.draw_2d_bin_1_scatter_pipeline_workgroup_size), + draw_buffer_len + .div_ceil(self.pipelines.draw_2d_bin_1_scatter_pipeline_workgroup_size), 1, 1, ); @@ -1485,7 +1486,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.draw_2d_bin_2_sort_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1526,7 +1527,7 @@ impl<'gpu> DrawState<'gpu> { // Upsweep gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.radix_sort_0_upsweep_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1556,7 +1557,7 @@ impl<'gpu> DrawState<'gpu> { self.pipelines.radix_sort_1_downsweep_pipeline, ); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1587,7 +1588,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.draw_2d_bin_3_resolve_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1620,7 +1621,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.draw_2d_rasterize_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -1671,7 +1672,7 @@ impl<'gpu> DrawState<'gpu> { gpu.cmd_set_pipeline(cmd_encoder, self.pipelines.composite_pipeline); gpu.cmd_set_bind_group(cmd_encoder, 0, &compute_bind_group); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, diff --git a/title/shark/tests/radix_sort.rs b/title/shark/tests/radix_sort.rs index 9f3acc2..0a5809e 100644 --- a/title/shark/tests/radix_sort.rs +++ b/title/shark/tests/radix_sort.rs @@ -68,7 +68,7 @@ fn gpu_sort(values: &mut [u32]) { // Upsweep gpu.cmd_set_pipeline(cmd_encoder, pipelines.radix_sort_0_upsweep_pipeline); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, @@ -99,7 +99,7 @@ fn gpu_sort(values: &mut [u32]) { // Downsweep gpu.cmd_set_pipeline(cmd_encoder, pipelines.radix_sort_1_downsweep_pipeline); - gpu.cmd_push_constants( + gpu.cmd_push_constants_with_data( cmd_encoder, ShaderStageFlags::COMPUTE, 0, -- 2.49.0