diff --git a/piet-gpu-hal/src/dx12.rs b/piet-gpu-hal/src/dx12.rs index 67b751d..857fe3c 100644 --- a/piet-gpu-hal/src/dx12.rs +++ b/piet-gpu-hal/src/dx12.rs @@ -456,6 +456,9 @@ impl crate::backend::CmdBuf for CmdBuf { unsafe fn finish(&mut self) { let _ = self.c.close(); + // This is a bit of a mess. Returning the allocator to the free pool + // makes sense if the command list will be dropped, but not if it will + // be reused. Probably need to implement some logic on drop. if let Some(free_allocators) = self.free_allocators.upgrade() { free_allocators .lock() @@ -516,6 +519,9 @@ impl crate::backend::CmdBuf for CmdBuf { ); self.c.resource_barrier(&[bar]); } + // Always do a memory barrier in case of UAV image access. We probably + // want to make these barriers more precise. + self.memory_barrier(); } unsafe fn clear_buffer(&self, buffer: &Buffer, size: Option) { @@ -558,6 +564,10 @@ impl crate::backend::CmdBuf for CmdBuf { impl crate::backend::PipelineBuilder for PipelineBuilder { fn add_buffers(&mut self, n_buffers: u32) { + // Note: if the buffer is readonly, then it needs to be bound + // as an SRV, not a UAV. I think that requires distinguishing + // readonly and read-write cases in pipeline and descriptor set + // creation. For now we punt. if n_buffers != 0 { self.ranges.push(d3d12::D3D12_DESCRIPTOR_RANGE { RangeType: d3d12::D3D12_DESCRIPTOR_RANGE_TYPE_UAV, @@ -572,7 +582,7 @@ impl crate::backend::PipelineBuilder for PipelineBuilder { fn add_images(&mut self, n_images: u32) { // These are UAV images, so the descriptor type is the same as buffers. - self.add_buffers(n_images) + self.add_buffers(n_images); } fn add_textures(&mut self, max_textures: u32) { diff --git a/piet-gpu-hal/src/dx12/wrappers.rs b/piet-gpu-hal/src/dx12/wrappers.rs index e132441..800959f 100644 --- a/piet-gpu-hal/src/dx12/wrappers.rs +++ b/piet-gpu-hal/src/dx12/wrappers.rs @@ -1307,7 +1307,7 @@ impl GraphicsCommandList { /// Copy an entire resource (buffer or image) pub unsafe fn copy_resource(&self, src: &Resource, dst: &Resource) { - self.0.CopyResource(src.get_mut(), dst.get_mut()); + self.0.CopyResource(dst.get_mut(), src.get_mut()); } pub unsafe fn copy_buffer( diff --git a/piet-gpu-hal/src/hub.rs b/piet-gpu-hal/src/hub.rs index b280881..2791bc9 100644 --- a/piet-gpu-hal/src/hub.rs +++ b/piet-gpu-hal/src/hub.rs @@ -169,6 +169,11 @@ impl Session { let mut item = pending.swap_remove(i); // TODO: wait is superfluous, can just reset let _ = self.0.device.wait_and_reset(vec![&mut item.fence]); + + // Reuse of command buffers works on Vulkan, but not at all on + // Metal and is problematic on DX12 (the allocator is returned) + // to the pool. Punt for now. + /* let mut pool = self.0.cmd_buf_pool.lock().unwrap(); pool.push((item.cmd_buf, item.fence)); std::mem::drop(item.resources); @@ -176,6 +181,7 @@ impl Session { pool.push((staging_cmd_buf.cmd_buf, staging_cmd_buf.fence)); std::mem::drop(staging_cmd_buf.resources); } + */ } else { i += 1; } @@ -562,11 +568,14 @@ impl SubmittedCmdBuf { unsafe { session.device.wait_and_reset(vec![&mut item.fence])?; } + // See discussion in `poll_cleanup` + /* session .cmd_buf_pool .lock() .unwrap() .push((item.cmd_buf, item.fence)); + */ std::mem::drop(item.resources); } // else session dropped error?