From e934f175efde410e44a3c4e227bdecf5194a9bfa Mon Sep 17 00:00:00 2001 From: chyyran Date: Sun, 25 Aug 2024 23:54:36 -0400 Subject: [PATCH] rt(d3d12): stop leaking transition barriers --- librashader-runtime-d3d12/src/filter_chain.rs | 63 ++++++++++++------- librashader-runtime-d3d12/src/framebuffer.rs | 12 ++-- librashader-runtime-d3d12/src/luts.rs | 29 ++------- librashader-runtime-d3d12/src/mipmap.rs | 26 ++++---- librashader-runtime-d3d12/src/util.rs | 38 ++++++----- librashader-runtime-d3d12/tests/triangle.rs | 4 +- 6 files changed, 87 insertions(+), 85 deletions(-) diff --git a/librashader-runtime-d3d12/src/filter_chain.rs b/librashader-runtime-d3d12/src/filter_chain.rs index 058ce71..5b095a9 100644 --- a/librashader-runtime-d3d12/src/filter_chain.rs +++ b/librashader-runtime-d3d12/src/filter_chain.rs @@ -43,7 +43,9 @@ use windows::Win32::Graphics::Direct3D12::{ ID3D12CommandAllocator, ID3D12CommandQueue, ID3D12DescriptorHeap, ID3D12Device, ID3D12Fence, ID3D12GraphicsCommandList, ID3D12Resource, D3D12_COMMAND_LIST_TYPE_DIRECT, D3D12_COMMAND_QUEUE_DESC, D3D12_COMMAND_QUEUE_FLAG_NONE, D3D12_FENCE_FLAG_NONE, - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_RENDER_TARGET, + D3D12_RESOURCE_BARRIER, D3D12_RESOURCE_BARRIER_TYPE_TRANSITION, + D3D12_RESOURCE_BARRIER_TYPE_UAV, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + D3D12_RESOURCE_STATE_RENDER_TARGET, }; use windows::Win32::Graphics::Dxgi::Common::DXGI_FORMAT_UNKNOWN; use windows::Win32::System::Threading::{CreateEventA, WaitForSingleObject, INFINITE}; @@ -104,6 +106,7 @@ pub(crate) struct FrameResiduals { mipmaps: Vec>, mipmap_luts: Vec, resources: Vec>>, + resource_barriers: Vec, } impl FrameResiduals { @@ -113,6 +116,7 @@ impl FrameResiduals { mipmaps: Vec::new(), mipmap_luts: Vec::new(), resources: Vec::new(), + resource_barriers: Vec::new(), } } @@ -135,12 +139,30 @@ impl FrameResiduals { self.resources.push(resource) } + /// Disposition only handles transition barriers, but it is not unsafe because + /// other things just leak and leakign is not unsafe, + pub fn dispose_barriers(&mut self, barrier: impl IntoIterator) { + self.resource_barriers.extend(barrier); + } + pub fn dispose(&mut self) { self.outputs.clear(); self.mipmaps.clear(); for resource in self.resources.drain(..) { drop(ManuallyDrop::into_inner(resource)) } + for barrier in self.resource_barriers.drain(..) { + if barrier.Type == D3D12_RESOURCE_BARRIER_TYPE_TRANSITION { + if let Some(resource) = unsafe { barrier.Anonymous.Transition }.pResource.take() { + drop(resource) + } + } else if barrier.Type == D3D12_RESOURCE_BARRIER_TYPE_UAV { + if let Some(resource) = unsafe { barrier.Anonymous.UAV }.pResource.take() { + drop(resource) + } + } + // other barrier types should be handled manually + } } } @@ -430,10 +452,7 @@ impl FilterChainD3D12 { gc.dispose_mipmap_handles(residual_mipmap); gc.dispose_mipmap_gen(mipmap_gen); - - for barrier in residual_barrier { - gc.dispose_resource(barrier.pResource) - } + gc.dispose_barriers(residual_barrier); Ok(luts) } @@ -640,7 +659,7 @@ impl FilterChainD3D12 { if let Some(options) = options { if options.clear_history { for framebuffer in &mut self.history_framebuffers { - framebuffer.clear(cmd, &mut self.rtv_heap)?; + framebuffer.clear(cmd, &mut self.rtv_heap, &mut self.residuals)?; } } } @@ -752,12 +771,13 @@ impl FilterChainD3D12 { )?; } - util::d3d12_resource_transition( - cmd, - &target.handle.resource(), - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - D3D12_RESOURCE_STATE_RENDER_TARGET, - ); + self.residuals + .dispose_barriers(util::d3d12_resource_transition( + cmd, + &target.handle.resource(), + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + D3D12_RESOURCE_STATE_RENDER_TARGET, + )); let view = target.create_render_target_view(&mut self.rtv_heap)?; let out = RenderTarget::identity(&view); @@ -775,15 +795,16 @@ impl FilterChainD3D12 { QuadType::Offscreen, )?; - util::d3d12_resource_transition( - cmd, - &target.handle.resource(), - D3D12_RESOURCE_STATE_RENDER_TARGET, - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - ); + self.residuals + .dispose_barriers(util::d3d12_resource_transition( + cmd, + &target.handle.resource(), + D3D12_RESOURCE_STATE_RENDER_TARGET, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + )); if target.max_mipmap > 1 && !self.disable_mipmaps { - let (residuals, residual_uav) = self.common.mipmap_gen.mipmapping_context( + let (residuals, residual_barriers) = self.common.mipmap_gen.mipmapping_context( cmd, &mut self.mipmap_heap, |ctx| { @@ -798,9 +819,7 @@ impl FilterChainD3D12 { )?; self.residuals.dispose_mipmap_handles(residuals); - for uav in residual_uav { - self.residuals.dispose_resource(uav.pResource) - } + self.residuals.dispose_barriers(residual_barriers); } self.residuals.dispose_output(view.descriptor); diff --git a/librashader-runtime-d3d12/src/framebuffer.rs b/librashader-runtime-d3d12/src/framebuffer.rs index 0530182..e0f6383 100644 --- a/librashader-runtime-d3d12/src/framebuffer.rs +++ b/librashader-runtime-d3d12/src/framebuffer.rs @@ -180,6 +180,7 @@ impl OwnedImage { unsafe { cmd.ResourceBarrier(&barriers); + gc.dispose_barriers(barriers); let dst = D3D12_TEXTURE_COPY_LOCATION { pResource: ManuallyDrop::new(Some(self.handle.resource().clone())), @@ -236,6 +237,8 @@ impl OwnedImage { cmd.ResourceBarrier(&barriers); } + gc.dispose_barriers(barriers); + Ok(()) } @@ -243,24 +246,25 @@ impl OwnedImage { &self, cmd: &ID3D12GraphicsCommandList, heap: &mut D3D12DescriptorHeap, + gc: &mut FrameResiduals, ) -> error::Result<()> { - util::d3d12_resource_transition( + gc.dispose_barriers(util::d3d12_resource_transition( cmd, &self.handle.resource(), D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_RENDER_TARGET, - ); + )); let rtv = self.create_render_target_view(heap)?; unsafe { cmd.ClearRenderTargetView(*rtv.descriptor.as_ref(), CLEAR, None) } - util::d3d12_resource_transition( + gc.dispose_barriers(util::d3d12_resource_transition( cmd, &self.handle.resource(), D3D12_RESOURCE_STATE_RENDER_TARGET, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - ); + )); Ok(()) } diff --git a/librashader-runtime-d3d12/src/luts.rs b/librashader-runtime-d3d12/src/luts.rs index 6044614..c69ebd0 100644 --- a/librashader-runtime-d3d12/src/luts.rs +++ b/librashader-runtime-d3d12/src/luts.rs @@ -157,39 +157,18 @@ impl LutTexture { resource_type: &ResourceType::Placed, })?; - // - // let mut upload: Option = None; - // - // unsafe { - // device.CreateCommittedResource( - // &D3D12_HEAP_PROPERTIES { - // Type: D3D12_HEAP_TYPE_UPLOAD, - // CPUPageProperty: D3D12_CPU_PAGE_PROPERTY_UNKNOWN, - // MemoryPoolPreference: D3D12_MEMORY_POOL_UNKNOWN, - // CreationNodeMask: 1, - // VisibleNodeMask: 1, - // }, - // D3D12_HEAP_FLAG_NONE, - // &buffer_desc, - // D3D12_RESOURCE_STATE_GENERIC_READ, - // None, - // &mut upload, - // )?; - // } - // assume_d3d12_init!(upload, "CreateCommittedResource"); - let subresource = [D3D12_SUBRESOURCE_DATA { pData: source.bytes.as_ptr().cast(), RowPitch: 4 * source.size.width as isize, SlicePitch: (4 * source.size.width * source.size.height) as isize, }]; - d3d12_resource_transition( + gc.dispose_barriers(d3d12_resource_transition( cmd, &resource.resource(), D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_COPY_DEST, - ); + )); d3d12_update_subresources( cmd, @@ -202,12 +181,12 @@ impl LutTexture { gc, )?; - d3d12_resource_transition( + gc.dispose_barriers(d3d12_resource_transition( cmd, &resource.resource(), D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - ); + )); let view = InputTexture::new( resource.resource().clone(), diff --git a/librashader-runtime-d3d12/src/mipmap.rs b/librashader-runtime-d3d12/src/mipmap.rs index 9667330..3a7cea7 100644 --- a/librashader-runtime-d3d12/src/mipmap.rs +++ b/librashader-runtime-d3d12/src/mipmap.rs @@ -42,7 +42,7 @@ pub struct MipmapGenContext<'a> { cmd: &'a ID3D12GraphicsCommandList, heap: &'a mut D3D12DescriptorHeap, residuals: Vec>, - residual_uav_descs: Vec, + residual_barriers: Vec, } impl<'a> MipmapGenContext<'a> { @@ -56,7 +56,7 @@ impl<'a> MipmapGenContext<'a> { cmd, heap, residuals: Vec::new(), - residual_uav_descs: Vec::new(), + residual_barriers: Vec::new(), } } @@ -74,7 +74,7 @@ impl<'a> MipmapGenContext<'a> { .gen .generate_mipmaps(self.cmd, resource, miplevels, size, format, self.heap)?; self.residuals.extend(residuals_heap); - self.residual_uav_descs.extend(residual_barriers); + self.residual_barriers.extend(residual_barriers); } Ok(()) @@ -84,9 +84,9 @@ impl<'a> MipmapGenContext<'a> { self, ) -> ( Vec>, - Vec, + Vec, ) { - (self.residuals, self.residual_uav_descs) + (self.residuals, self.residual_barriers) } } @@ -153,7 +153,7 @@ impl D3D12MipmapGen { ) -> Result< ( Vec>, - Vec, + Vec, ), E, > @@ -188,7 +188,7 @@ impl D3D12MipmapGen { work_heap: &mut D3D12DescriptorHeap, ) -> error::Result<( Vec>, - Vec, + Vec, )> { // create views for mipmap generation let srv = work_heap.alloc_slot()?; @@ -240,7 +240,7 @@ impl D3D12MipmapGen { cmd.SetComputeRootDescriptorTable(0, *heap_slots[0].deref().as_ref()); } - let mut residual_uavs = Vec::new(); + let mut residual_barriers = Vec::new(); for i in 1..miplevels as u32 { let scaled = size.scale_mipmap(i); let mipmap_params = MipConstants { @@ -267,6 +267,7 @@ impl D3D12MipmapGen { unsafe { cmd.ResourceBarrier(&barriers); + residual_barriers.extend(barriers); cmd.SetComputeRootDescriptorTable(1, *heap_slots[i as usize].deref().as_ref()); cmd.SetComputeRoot32BitConstants( @@ -311,14 +312,9 @@ impl D3D12MipmapGen { cmd.ResourceBarrier(&barriers); } - let uav = unsafe { - let [barrier, ..] = barriers; - barrier.Anonymous.UAV - }; - - residual_uavs.push(ManuallyDrop::into_inner(uav)) + residual_barriers.extend(barriers) } - Ok((heap_slots, residual_uavs)) + Ok((heap_slots, residual_barriers)) } } diff --git a/librashader-runtime-d3d12/src/util.rs b/librashader-runtime-d3d12/src/util.rs index d7b1907..19ff040 100644 --- a/librashader-runtime-d3d12/src/util.rs +++ b/librashader-runtime-d3d12/src/util.rs @@ -180,22 +180,7 @@ pub fn dxc_validate_shader( } } -#[inline(always)] -pub fn d3d12_resource_transition( - cmd: &ID3D12GraphicsCommandList, - resource: &ID3D12Resource, - before: D3D12_RESOURCE_STATES, - after: D3D12_RESOURCE_STATES, -) { - d3d12_resource_transition_subresource( - cmd, - resource, - before, - after, - D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, - ); -} - +#[must_use = "Resource Barrier must be disposed"] pub fn d3d12_get_resource_transition_subresource( resource: &ID3D12Resource, before: D3D12_RESOURCE_STATES, @@ -216,6 +201,24 @@ pub fn d3d12_get_resource_transition_subresource( } } +#[must_use = "Resource Barrier must be disposed"] +#[inline(always)] +pub fn d3d12_resource_transition( + cmd: &ID3D12GraphicsCommandList, + resource: &ID3D12Resource, + before: D3D12_RESOURCE_STATES, + after: D3D12_RESOURCE_STATES, +) -> [D3D12_RESOURCE_BARRIER; 1] { + d3d12_resource_transition_subresource( + cmd, + resource, + before, + after, + D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, + ) +} + +#[must_use = "Resource Barrier must be disposed"] #[inline(always)] pub fn d3d12_resource_transition_subresource( cmd: &ID3D12GraphicsCommandList, @@ -223,7 +226,7 @@ pub fn d3d12_resource_transition_subresource( before: D3D12_RESOURCE_STATES, after: D3D12_RESOURCE_STATES, subresource: u32, -) { +) -> [D3D12_RESOURCE_BARRIER; 1] { let barrier = [d3d12_get_resource_transition_subresource( resource, before, @@ -231,6 +234,7 @@ pub fn d3d12_resource_transition_subresource( subresource, )]; unsafe { cmd.ResourceBarrier(&barrier) } + barrier } pub(crate) fn d3d12_update_subresources( diff --git a/librashader-runtime-d3d12/tests/triangle.rs b/librashader-runtime-d3d12/tests/triangle.rs index db45df4..c511396 100644 --- a/librashader-runtime-d3d12/tests/triangle.rs +++ b/librashader-runtime-d3d12/tests/triangle.rs @@ -5,12 +5,12 @@ use crate::hello_triangle::{DXSample, SampleCommandLine}; #[test] fn triangle_d3d12() { let sample = hello_triangle::d3d12_hello_triangle::Sample::new( - // "../test/shaders_slang/bezel/Mega_Bezel/Presets/MBZ__0__SMOOTH-ADV.slangp", + "../test/shaders_slang/bezel/Mega_Bezel/Presets/MBZ__0__SMOOTH-ADV.slangp", // "../test/shaders_slang/crt/crt-lottes.slangp", // "../test/basic.slangp", // "../test/shaders_slang/handheld/console-border/gbc-lcd-grid-v2.slangp", // "../test/Mega_Bezel_Packs/Duimon-Mega-Bezel/Presets/Advanced/Nintendo_GBA_SP/GBA_SP-[ADV]-[LCD-GRID]-[Night].slangp", - "../test/shaders_slang/test/feedback.slangp", + // "../test/shaders_slang/test/feedback.slangp", // "../test/shaders_slang/test/history.slangp", // "../test/shaders_slang/crt/crt-royale.slangp", // "../test/slang-shaders/vhs/VHSPro.slangp",