From b6292c644f8f1c76ca6d4682858584ebf0eb965e Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Thu, 27 May 2021 15:37:05 -0700 Subject: [PATCH] Make fences mutable Change the interface for fences to accept mutable references. This will actualy help the Metal backend more than dx12 (avoiding interior mutability) but more accurately captures intent and matches gfx-hal. --- piet-gpu-hal/examples/dx12_toy.rs | 6 +++--- piet-gpu-hal/src/dx12.rs | 4 ++-- piet-gpu-hal/src/hub.rs | 12 ++++++------ piet-gpu-hal/src/lib.rs | 4 ++-- piet-gpu-hal/src/macros.rs | 20 ++++++++++++++++++++ piet-gpu-hal/src/mux.rs | 27 +++++++++++++-------------- piet-gpu-hal/src/vulkan.rs | 7 +++---- 7 files changed, 49 insertions(+), 31 deletions(-) diff --git a/piet-gpu-hal/examples/dx12_toy.rs b/piet-gpu-hal/examples/dx12_toy.rs index 63e3db4..23737bf 100644 --- a/piet-gpu-hal/examples/dx12_toy.rs +++ b/piet-gpu-hal/examples/dx12_toy.rs @@ -73,7 +73,7 @@ fn toy() -> Result<(), Error> { let pipeline = device.create_simple_compute_pipeline(SHADER_CODE, 1, 1)?; let ds = device.create_descriptor_set(&pipeline, &[&dev_buf], &[&img])?; let mut cmd_buf = device.create_cmd_buf()?; - let fence = device.create_fence(false)?; + let mut fence = device.create_fence(false)?; cmd_buf.begin(); cmd_buf.copy_buffer(&buf, &dev_buf); cmd_buf.memory_barrier(); @@ -86,8 +86,8 @@ fn toy() -> Result<(), Error> { cmd_buf.finish_timestamps(&query_pool); cmd_buf.host_barrier(); cmd_buf.finish(); - device.run_cmd_bufs(&[&cmd_buf], &[], &[], Some(&fence))?; - device.wait_and_reset(&[&fence])?; + device.run_cmd_bufs(&[&cmd_buf], &[], &[], Some(&mut fence))?; + device.wait_and_reset(&[&mut fence])?; let mut readback: Vec = vec![0u32; 256]; device.read_buffer(&buf, readback.as_mut_ptr() as *mut u8, 0, 1024)?; println!("{:?}", readback); diff --git a/piet-gpu-hal/src/dx12.rs b/piet-gpu-hal/src/dx12.rs index 0194e1e..637aa2f 100644 --- a/piet-gpu-hal/src/dx12.rs +++ b/piet-gpu-hal/src/dx12.rs @@ -348,7 +348,7 @@ impl crate::Device for Dx12Device { cmd_bufs: &[&Self::CmdBuf], _wait_semaphores: &[&Self::Semaphore], _signal_semaphores: &[&Self::Semaphore], - fence: Option<&Self::Fence>, + fence: Option<&mut Self::Fence>, ) -> Result<(), Error> { // TODO: handle semaphores let lists = cmd_bufs @@ -405,7 +405,7 @@ impl crate::Device for Dx12Device { Ok(Fence { fence, event, val }) } - unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error> { + unsafe fn wait_and_reset(&self, fences: &[&mut Self::Fence]) -> Result<(), Error> { for fence in fences { // TODO: probably handle errors here. let _status = fence.event.wait(winapi::um::winbase::INFINITE); diff --git a/piet-gpu-hal/src/hub.rs b/piet-gpu-hal/src/hub.rs index 80c03cf..a53ef2f 100644 --- a/piet-gpu-hal/src/hub.rs +++ b/piet-gpu-hal/src/hub.rs @@ -124,9 +124,9 @@ impl Session { let mut i = 0; while i < pending.len() { if let Ok(true) = self.0.device.get_fence_status(&pending[i].fence) { - let item = pending.swap_remove(i); + let mut item = pending.swap_remove(i); // TODO: wait is superfluous, can just reset - let _ = self.0.device.wait_and_reset(&[&item.fence]); + let _ = self.0.device.wait_and_reset(vec![&mut item.fence]); let mut pool = self.0.cmd_buf_pool.lock().unwrap(); pool.push((item.cmd_buf, item.fence)); std::mem::drop(item.resources); @@ -143,7 +143,7 @@ impl Session { pub unsafe fn run_cmd_buf( &self, - cmd_buf: CmdBuf, + mut cmd_buf: CmdBuf, wait_semaphores: &[&Semaphore], signal_semaphores: &[&Semaphore], ) -> Result { @@ -162,7 +162,7 @@ impl Session { &cmd_bufs, wait_semaphores, signal_semaphores, - Some(&cmd_buf.fence), + Some(&mut cmd_buf.fence), )?; Ok(SubmittedCmdBuf( Some(SubmittedCmdBufInner { @@ -313,10 +313,10 @@ impl CmdBuf { impl SubmittedCmdBuf { pub fn wait(mut self) -> Result<(), Error> { - let item = self.0.take().unwrap(); + let mut item = self.0.take().unwrap(); if let Some(session) = Weak::upgrade(&self.1) { unsafe { - session.device.wait_and_reset(&[&item.fence])?; + session.device.wait_and_reset(vec![&mut item.fence])?; } session .cmd_buf_pool diff --git a/piet-gpu-hal/src/lib.rs b/piet-gpu-hal/src/lib.rs index 26147ed..70dce4d 100644 --- a/piet-gpu-hal/src/lib.rs +++ b/piet-gpu-hal/src/lib.rs @@ -194,7 +194,7 @@ pub trait Device: Sized { cmd_buf: &[&Self::CmdBuf], wait_semaphores: &[&Self::Semaphore], signal_semaphores: &[&Self::Semaphore], - fence: Option<&Self::Fence>, + fence: Option<&mut Self::Fence>, ) -> Result<(), Error>; /// Copy data from the buffer to memory. @@ -231,7 +231,7 @@ pub trait Device: Sized { unsafe fn create_semaphore(&self) -> Result; unsafe fn create_fence(&self, signaled: bool) -> Result; - unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error>; + unsafe fn wait_and_reset(&self, fences: &[&mut Self::Fence]) -> Result<(), Error>; unsafe fn get_fence_status(&self, fence: &Self::Fence) -> Result; unsafe fn create_sampler(&self, params: SamplerParams) -> Result; diff --git a/piet-gpu-hal/src/macros.rs b/piet-gpu-hal/src/macros.rs index f1d7019..f121603 100644 --- a/piet-gpu-hal/src/macros.rs +++ b/piet-gpu-hal/src/macros.rs @@ -51,6 +51,16 @@ macro_rules! mux_enum { } } } + $crate::mux_cfg! { + #[cfg(vk)] + #[allow(unused)] + fn vk_mut(&mut self) -> &mut $vk { + match self { + $name::Vk(x) => x, + _ => panic!("downcast error") + } + } + } $crate::mux_cfg! { #[cfg(dx12)] @@ -62,6 +72,16 @@ macro_rules! mux_enum { } } } + $crate::mux_cfg! { + #[cfg(dx12)] + #[allow(unused)] + fn dx12_mut(&mut self) -> &mut $dx12 { + match self { + $name::Dx12(x) => x, + _ => panic!("downcast error") + } + } + } } }; } diff --git a/piet-gpu-hal/src/mux.rs b/piet-gpu-hal/src/mux.rs index 0971416..5019a6d 100644 --- a/piet-gpu-hal/src/mux.rs +++ b/piet-gpu-hal/src/mux.rs @@ -197,23 +197,22 @@ impl Device { } } - pub unsafe fn wait_and_reset(&self, fences: &[&Fence]) -> Result<(), Error> { + // Consider changing Vec to iterator (as is done in gfx-hal) + pub unsafe fn wait_and_reset(&self, fences: Vec<&mut Fence>) -> Result<(), Error> { mux_match! { self; Device::Vk(d) => { - let fences = fences - .iter() - .copied() - .map(Fence::vk) + let mut fences = fences + .into_iter() + .map(|f| f.vk_mut()) .collect::>(); - d.wait_and_reset(&*fences) + d.wait_and_reset(&mut fences) } Device::Dx12(d) => { - let fences = fences - .iter() - .copied() - .map(Fence::dx12) + let mut fences = fences + .into_iter() + .map(|f| f.dx12_mut()) .collect::>(); - d.wait_and_reset(&*fences) + d.wait_and_reset(&mut fences) } } } @@ -272,7 +271,7 @@ impl Device { cmd_bufs: &[&CmdBuf], wait_semaphores: &[&Semaphore], signal_semaphores: &[&Semaphore], - fence: Option<&Fence>, + fence: Option<&mut Fence>, ) -> Result<(), Error> { mux_match! { self; Device::Vk(d) => d.run_cmd_bufs( @@ -290,7 +289,7 @@ impl Device { .copied() .map(Semaphore::vk) .collect::>(), - fence.map(Fence::vk), + fence.map(Fence::vk_mut), ), Device::Dx12(d) => d.run_cmd_bufs( &cmd_bufs @@ -307,7 +306,7 @@ impl Device { .copied() .map(Semaphore::dx12) .collect::>(), - fence.map(Fence::dx12), + fence.map(Fence::dx12_mut), ), } } diff --git a/piet-gpu-hal/src/vulkan.rs b/piet-gpu-hal/src/vulkan.rs index 619c6bd..4d73c2f 100644 --- a/piet-gpu-hal/src/vulkan.rs +++ b/piet-gpu-hal/src/vulkan.rs @@ -619,12 +619,11 @@ impl crate::Device for VkDevice { Ok(device.create_semaphore(&vk::SemaphoreCreateInfo::default(), None)?) } - unsafe fn wait_and_reset(&self, fences: &[&Self::Fence]) -> Result<(), Error> { + unsafe fn wait_and_reset(&self, fences: &[&mut Self::Fence]) -> Result<(), Error> { let device = &self.device.device; let fences = fences .iter() - .copied() - .copied() + .map(|f| **f) .collect::>(); device.wait_for_fences(&fences, true, !0)?; device.reset_fences(&fences)?; @@ -718,7 +717,7 @@ impl crate::Device for VkDevice { cmd_bufs: &[&CmdBuf], wait_semaphores: &[&Self::Semaphore], signal_semaphores: &[&Self::Semaphore], - fence: Option<&Self::Fence>, + fence: Option<&mut Self::Fence>, ) -> Result<(), Error> { let device = &self.device.device;