From c57e502b782b2f18f89e9b22c9df6658235ee581 Mon Sep 17 00:00:00 2001 From: chyyran Date: Sat, 28 Sep 2024 22:10:32 -0400 Subject: [PATCH] rt(d3d12): make images with OwnedFramebuffer provenance use ManuallyDrop --- librashader-runtime-d3d12/src/filter_chain.rs | 90 ++++++++++--------- librashader-runtime-d3d12/src/framebuffer.rs | 84 ++++++++--------- librashader-runtime-d3d12/src/luts.rs | 22 ++--- librashader-runtime-d3d12/src/mipmap.rs | 39 ++++---- librashader-runtime-d3d12/src/resource.rs | 43 +++++++-- librashader-runtime-d3d12/src/texture.rs | 64 +++++++++++-- librashader-runtime-d3d12/src/util.rs | 23 +++-- .../tests/hello_triangle/mod.rs | 2 +- librashader-runtime-d3d12/tests/triangle.rs | 4 +- 9 files changed, 233 insertions(+), 138 deletions(-) diff --git a/librashader-runtime-d3d12/src/filter_chain.rs b/librashader-runtime-d3d12/src/filter_chain.rs index d2801f1..a2a208f 100644 --- a/librashader-runtime-d3d12/src/filter_chain.rs +++ b/librashader-runtime-d3d12/src/filter_chain.rs @@ -131,13 +131,18 @@ impl FrameResiduals { self.mipmaps.extend(handles) } - pub fn dispose_resource(&mut self, resource: ManuallyDrop>) { + pub unsafe fn dispose_resource(&mut self, resource: ManuallyDrop>) { 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) { + /// Disposition only handles transition barriers. + /// + /// **Safety:** It is only safe to dispose a barrier created with resource strategy IncrementRef. + /// + pub unsafe fn dispose_barriers( + &mut self, + barrier: impl IntoIterator, + ) { self.resource_barriers.extend(barrier); } @@ -230,6 +235,7 @@ mod compile { } } +use crate::resource::OutlivesFrame; use compile::{compile_passes_dxil, compile_passes_hlsl, DxilShaderPassMeta, HlslShaderPassMeta}; use librashader_runtime::parameters::RuntimeParameters; @@ -321,7 +327,8 @@ impl FilterChainD3D12 { let mut staging_heap = unsafe { D3D12DescriptorHeap::new( device, - (MAX_BINDINGS_COUNT as usize) * shader_count + // add one, because technically the input image doesn't need to count + (1 + MAX_BINDINGS_COUNT as usize) * shader_count + MIPMAP_RESERVED_WORKHEAP_DESCRIPTORS + lut_count, ) @@ -391,7 +398,7 @@ impl FilterChainD3D12 { common: FilterCommon { d3d12: device.clone(), samplers, - allocator: allocator, + allocator, output_textures, feedback_textures, luts, @@ -460,7 +467,10 @@ impl FilterChainD3D12 { gc.dispose_mipmap_handles(residual_mipmap); gc.dispose_mipmap_gen(mipmap_gen); - gc.dispose_barriers(residual_barrier); + + unsafe { + gc.dispose_barriers(residual_barrier); + } Ok(luts) } @@ -653,7 +663,7 @@ impl FilterChainD3D12 { ); } unsafe { - back.copy_from(cmd, input, &mut self.residuals)?; + back.copy_from(cmd, input)?; } self.history_framebuffers.push_front(back); } @@ -669,6 +679,8 @@ impl FilterChainD3D12 { /// librashader **will not** create a resource barrier for the final pass. The output image will /// remain in `D3D12_RESOURCE_STATE_RENDER_TARGET` after all shader passes. The caller must transition /// the output image to the final resource state. + /// + /// The input and output images must stay alive until the command list is submitted and work is complete. pub unsafe fn frame( &mut self, cmd: &ID3D12GraphicsCommandList, @@ -689,7 +701,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, &mut self.residuals)?; + framebuffer.clear(cmd, &mut self.rtv_heap)?; } } } @@ -787,13 +799,12 @@ impl FilterChainD3D12 { )?; } - self.residuals - .dispose_barriers(util::d3d12_resource_transition( - cmd, - &target.handle.resource(), - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - D3D12_RESOURCE_STATE_RENDER_TARGET, - )); + util::d3d12_resource_transition::( + cmd, + &target.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)?; @@ -811,21 +822,21 @@ impl FilterChainD3D12 { QuadType::Offscreen, )?; - self.residuals - .dispose_barriers(util::d3d12_resource_transition( - cmd, - &target.handle.resource(), - D3D12_RESOURCE_STATE_RENDER_TARGET, - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - )); + util::d3d12_resource_transition::( + cmd, + &target.resource, + D3D12_RESOURCE_STATE_RENDER_TARGET, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + ); if target.max_mipmap > 1 && !self.disable_mipmaps { - let (residuals, residual_barriers) = self.common.mipmap_gen.mipmapping_context( + // barriers don't get disposed because the context is OutlivesFrame + let (residuals, _residual_barriers) = self.common.mipmap_gen.mipmapping_context( cmd, &mut self.mipmap_heap, |ctx| { - ctx.generate_mipmaps( - &target.handle.resource(), + ctx.generate_mipmaps::( + &target.resource, target.max_mipmap, target.size, target.format.into(), @@ -835,7 +846,6 @@ impl FilterChainD3D12 { )?; self.residuals.dispose_mipmap_handles(residuals); - self.residuals.dispose_barriers(residual_barriers); } self.residuals.dispose_output(view.descriptor); @@ -862,13 +872,12 @@ impl FilterChainD3D12 { )?; } - self.residuals - .dispose_barriers(util::d3d12_resource_transition( - cmd, - &feedback_target.handle.resource(), - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - D3D12_RESOURCE_STATE_RENDER_TARGET, - )); + util::d3d12_resource_transition::( + cmd, + &feedback_target.resource, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + D3D12_RESOURCE_STATE_RENDER_TARGET, + ); let view = feedback_target.create_render_target_view(&mut self.rtv_heap)?; let out = RenderTarget::viewport_with_output(&view, viewport); @@ -885,13 +894,12 @@ impl FilterChainD3D12 { QuadType::Final, )?; - self.residuals - .dispose_barriers(util::d3d12_resource_transition( - cmd, - &feedback_target.handle.resource(), - D3D12_RESOURCE_STATE_RENDER_TARGET, - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - )); + util::d3d12_resource_transition::( + cmd, + &feedback_target.resource, + D3D12_RESOURCE_STATE_RENDER_TARGET, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + ); } if pass.pipeline.format != viewport.output.format { diff --git a/librashader-runtime-d3d12/src/framebuffer.rs b/librashader-runtime-d3d12/src/framebuffer.rs index 2a809a5..26e0a33 100644 --- a/librashader-runtime-d3d12/src/framebuffer.rs +++ b/librashader-runtime-d3d12/src/framebuffer.rs @@ -1,6 +1,6 @@ use crate::descriptor_heap::{CpuStagingHeap, RenderTargetHeap}; use crate::error::FilterChainError; -use crate::filter_chain::FrameResiduals; +use crate::resource::{OutlivesFrame, ResourceHandleStrategy}; use crate::texture::{D3D12OutputView, InputTexture}; use crate::util::d3d12_get_closest_format; use crate::{error, util}; @@ -17,12 +17,13 @@ use parking_lot::Mutex; use std::mem::ManuallyDrop; use std::sync::Arc; use windows::Win32::Graphics::Direct3D12::{ - ID3D12Device, ID3D12GraphicsCommandList, D3D12_BOX, D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING, - D3D12_FEATURE_DATA_FORMAT_SUPPORT, D3D12_FORMAT_SUPPORT1_MIP, - D3D12_FORMAT_SUPPORT1_RENDER_TARGET, D3D12_FORMAT_SUPPORT1_SHADER_SAMPLE, - D3D12_FORMAT_SUPPORT1_TEXTURE2D, D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD, - D3D12_FORMAT_SUPPORT2_UAV_TYPED_STORE, D3D12_RENDER_TARGET_VIEW_DESC, - D3D12_RENDER_TARGET_VIEW_DESC_0, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, D3D12_RESOURCE_DESC, + ID3D12Device, ID3D12GraphicsCommandList, ID3D12Resource, D3D12_BOX, + D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING, D3D12_FEATURE_DATA_FORMAT_SUPPORT, + D3D12_FORMAT_SUPPORT1_MIP, D3D12_FORMAT_SUPPORT1_RENDER_TARGET, + D3D12_FORMAT_SUPPORT1_SHADER_SAMPLE, D3D12_FORMAT_SUPPORT1_TEXTURE2D, + D3D12_FORMAT_SUPPORT2_UAV_TYPED_LOAD, D3D12_FORMAT_SUPPORT2_UAV_TYPED_STORE, + D3D12_RENDER_TARGET_VIEW_DESC, D3D12_RENDER_TARGET_VIEW_DESC_0, + D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, D3D12_RESOURCE_DESC, D3D12_RESOURCE_DIMENSION_TEXTURE2D, D3D12_RESOURCE_FLAG_ALLOW_RENDER_TARGET, D3D12_RESOURCE_FLAG_ALLOW_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_COPY_SOURCE, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, @@ -36,6 +37,7 @@ use windows::Win32::Graphics::Dxgi::Common::{DXGI_FORMAT, DXGI_SAMPLE_DESC}; #[derive(Debug)] pub(crate) struct OwnedImage { pub(crate) handle: ManuallyDrop, + pub(crate) resource: ManuallyDrop, pub(crate) size: Size, pub(crate) format: DXGI_FORMAT, pub(crate) max_mipmap: u16, @@ -113,7 +115,7 @@ impl OwnedImage { desc.Format = d3d12_get_closest_format(device, format_support); - let resource = allocator.lock().create_resource(&ResourceCreateDesc { + let allocator_resource = allocator.lock().create_resource(&ResourceCreateDesc { name: "ownedimage", memory_location: MemoryLocation::GpuOnly, resource_category: ResourceCategory::RtvDsvTexture, @@ -145,8 +147,10 @@ impl OwnedImage { // } // assume_d3d12_init!(resource, "CreateCommittedResource"); + let resource = ManuallyDrop::new(allocator_resource.resource().clone()); Ok(OwnedImage { - handle: ManuallyDrop::new(resource), + handle: ManuallyDrop::new(allocator_resource), + resource, size, format: desc.Format, device: device.clone(), @@ -161,17 +165,16 @@ impl OwnedImage { &self, cmd: &ID3D12GraphicsCommandList, input: &InputTexture, - gc: &mut FrameResiduals, ) -> error::Result<()> { let barriers = [ - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( &input.resource, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_COPY_SOURCE, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, ), - util::d3d12_get_resource_transition_subresource( - &self.handle.resource(), + util::d3d12_get_resource_transition_subresource::( + &self.resource, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, @@ -180,10 +183,9 @@ impl OwnedImage { unsafe { cmd.ResourceBarrier(&barriers); - gc.dispose_barriers(barriers); let dst = D3D12_TEXTURE_COPY_LOCATION { - pResource: ManuallyDrop::new(Some(self.handle.resource().clone())), + pResource: OutlivesFrame::obtain(&self.resource), Type: D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX, Anonymous: D3D12_TEXTURE_COPY_LOCATION_0 { SubresourceIndex: 0, @@ -191,7 +193,7 @@ impl OwnedImage { }; let src = D3D12_TEXTURE_COPY_LOCATION { - pResource: ManuallyDrop::new(Some(input.resource.clone())), + pResource: OutlivesFrame::obtain(&input.resource), Type: D3D12_TEXTURE_COPY_TYPE_SUBRESOURCE_INDEX, Anonymous: D3D12_TEXTURE_COPY_LOCATION_0 { SubresourceIndex: 0, @@ -213,20 +215,17 @@ impl OwnedImage { back: 1, }), ); - - gc.dispose_resource(dst.pResource); - gc.dispose_resource(src.pResource); } let barriers = [ - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( &input.resource, D3D12_RESOURCE_STATE_COPY_SOURCE, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, ), - util::d3d12_get_resource_transition_subresource( - &self.handle.resource(), + util::d3d12_get_resource_transition_subresource::( + &self.resource, D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, @@ -237,8 +236,6 @@ impl OwnedImage { cmd.ResourceBarrier(&barriers); } - gc.dispose_barriers(barriers); - Ok(()) } @@ -246,25 +243,26 @@ impl OwnedImage { &self, cmd: &ID3D12GraphicsCommandList, heap: &mut D3D12DescriptorHeap, - gc: &mut FrameResiduals, ) -> error::Result<()> { - gc.dispose_barriers(util::d3d12_resource_transition( - cmd, - &self.handle.resource(), - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - D3D12_RESOURCE_STATE_RENDER_TARGET, - )); + unsafe { + util::d3d12_resource_transition::( + cmd, + &self.resource, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + D3D12_RESOURCE_STATE_RENDER_TARGET, + ); - let rtv = self.create_render_target_view(heap)?; + let rtv = self.create_render_target_view(heap)?; - unsafe { cmd.ClearRenderTargetView(*rtv.descriptor.as_ref(), CLEAR, None) } + cmd.ClearRenderTargetView(*rtv.descriptor.as_ref(), CLEAR, None); - gc.dispose_barriers(util::d3d12_resource_transition( - cmd, - &self.handle.resource(), - D3D12_RESOURCE_STATE_RENDER_TARGET, - D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - )); + util::d3d12_resource_transition::( + cmd, + &self.resource, + D3D12_RESOURCE_STATE_RENDER_TARGET, + D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, + ); + } Ok(()) } @@ -297,8 +295,8 @@ impl OwnedImage { ); } - Ok(InputTexture::new( - self.handle.resource().clone(), + Ok(InputTexture::new::( + &self.resource, descriptor, self.size, self.format, @@ -390,6 +388,10 @@ impl ScaleFramebuffer for OwnedImage { impl Drop for OwnedImage { fn drop(&mut self) { + // let go of the handle + unsafe { + ManuallyDrop::drop(&mut self.resource); + } let resource = unsafe { ManuallyDrop::take(&mut self.handle) }; if let Err(e) = self.allocator.lock().free_resource(resource) { println!("librashader-runtime-d3d12: [warn] failed to deallocate owned image buffer memory {e}") diff --git a/librashader-runtime-d3d12/src/luts.rs b/librashader-runtime-d3d12/src/luts.rs index aebabcb..fdcfdc8 100644 --- a/librashader-runtime-d3d12/src/luts.rs +++ b/librashader-runtime-d3d12/src/luts.rs @@ -176,12 +176,12 @@ impl LutTexture { let resource = ManuallyDrop::new(allocator_resource.resource().clone()); let upload = ManuallyDrop::new(allocator_upload.resource().clone()); - gc.dispose_barriers(d3d12_resource_transition( + d3d12_resource_transition::( cmd, - &allocator_resource.resource(), + &resource, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_COPY_DEST, - )); + ); d3d12_update_subresources::( cmd, @@ -194,15 +194,15 @@ impl LutTexture { gc, )?; - gc.dispose_barriers(d3d12_resource_transition( + d3d12_resource_transition::( cmd, &resource, D3D12_RESOURCE_STATE_COPY_DEST, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, - )); + ); - let view = InputTexture::new( - allocator_resource.resource().clone(), + let view = InputTexture::new::( + &resource, descriptor, source.size, ImageFormat::R8G8B8A8Unorm.into(), @@ -223,8 +223,8 @@ impl LutTexture { pub fn generate_mipmaps(&self, gen_mips: &mut MipmapGenContext) -> error::Result<()> { if let Some(miplevels) = self.miplevels { - gen_mips.generate_mipmaps( - &self.allocator_resource.resource(), + gen_mips.generate_mipmaps::( + &self.resource, miplevels, self.view.size, ImageFormat::R8G8B8A8Unorm.into(), @@ -384,8 +384,8 @@ fn update_subresources>>( cmd.CopyTextureRegion(&dest_location, 0, 0, 0, &source_location, None); - S::cleanup(gc, dest_location.pResource); - S::cleanup(gc, source_location.pResource); + S::cleanup_handler(|| gc.dispose_resource(dest_location.pResource)); + S::cleanup_handler(|| gc.dispose_resource(source_location.pResource)); } } Ok(required_size) diff --git a/librashader-runtime-d3d12/src/mipmap.rs b/librashader-runtime-d3d12/src/mipmap.rs index 1f463f5..02a8c2f 100644 --- a/librashader-runtime-d3d12/src/mipmap.rs +++ b/librashader-runtime-d3d12/src/mipmap.rs @@ -1,4 +1,5 @@ use crate::descriptor_heap::ResourceWorkHeap; +use crate::resource::{ObtainResourceHandle, ResourceHandleStrategy}; use crate::util::dxc_validate_shader; use crate::{error, util}; use bytemuck::{Pod, Zeroable}; @@ -6,12 +7,13 @@ use d3d12_descriptor_heap::{D3D12DescriptorHeap, D3D12DescriptorHeapSlot}; use librashader_common::Size; use librashader_runtime::scaling::MipmapSize; use std::mem::ManuallyDrop; + use windows::Win32::Graphics::Direct3D::Dxc::{ CLSID_DxcLibrary, CLSID_DxcValidator, DxcCreateInstance, }; use windows::Win32::Graphics::Direct3D12::{ ID3D12DescriptorHeap, ID3D12Device, ID3D12GraphicsCommandList, ID3D12PipelineState, - ID3D12Resource, ID3D12RootSignature, D3D12_COMPUTE_PIPELINE_STATE_DESC, + ID3D12RootSignature, D3D12_COMPUTE_PIPELINE_STATE_DESC, D3D12_DEFAULT_SHADER_4_COMPONENT_MAPPING, D3D12_RESOURCE_BARRIER, D3D12_RESOURCE_BARRIER_0, D3D12_RESOURCE_BARRIER_TYPE_UAV, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_UAV_BARRIER, D3D12_SHADER_BYTECODE, @@ -62,9 +64,9 @@ impl<'a> MipmapGenContext<'a> { /// Generate a set of mipmaps for the resource. /// This is a "cheap" action and only dispatches a compute shader. - pub fn generate_mipmaps( + pub fn generate_mipmaps, T: ObtainResourceHandle>( &mut self, - resource: &ID3D12Resource, + resource: &T, miplevels: u16, size: Size, format: DXGI_FORMAT, @@ -72,9 +74,13 @@ impl<'a> MipmapGenContext<'a> { unsafe { let (residuals_heap, residual_barriers) = self .gen - .generate_mipmaps(self.cmd, resource, miplevels, size, format, self.heap)?; + .generate_mipmaps::(self.cmd, resource, miplevels, size, format, self.heap)?; + + // heap slots always need to be disposed self.residuals.extend(residuals_heap); - self.residual_barriers.extend(residual_barriers); + + // barriers need to be disposed if the handle strategy is incrementref + S::cleanup_handler(|| self.residual_barriers.extend(residual_barriers)); } Ok(()) @@ -178,10 +184,10 @@ impl D3D12MipmapGen { /// SAFETY: /// - handle must be a CPU handle to an SRV /// - work_heap must have enough descriptors to fit all miplevels. - unsafe fn generate_mipmaps( + unsafe fn generate_mipmaps, T: ObtainResourceHandle>( &self, cmd: &ID3D12GraphicsCommandList, - resource: &ID3D12Resource, + resource: &T, miplevels: u16, size: Size, format: DXGI_FORMAT, @@ -206,7 +212,7 @@ impl D3D12MipmapGen { }; self.device - .CreateShaderResourceView(resource, Some(&srv_desc), *srv.as_ref()); + .CreateShaderResourceView(resource.handle(), Some(&srv_desc), *srv.as_ref()); } let mut heap_slots = Vec::with_capacity(miplevels as usize); @@ -227,7 +233,7 @@ impl D3D12MipmapGen { unsafe { self.device.CreateUnorderedAccessView( - resource, + resource.handle(), None, Some(&desc), *descriptor.as_ref(), @@ -251,13 +257,13 @@ impl D3D12MipmapGen { let mipmap_params = bytemuck::bytes_of(&mipmap_params); let barriers = [ - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( resource, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, i - 1, ), - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( resource, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, @@ -267,7 +273,8 @@ impl D3D12MipmapGen { unsafe { cmd.ResourceBarrier(&barriers); - residual_barriers.extend(barriers); + + S::cleanup_handler(|| residual_barriers.extend(barriers)); cmd.SetComputeRootDescriptorTable(1, *heap_slots[i as usize].as_ref()); cmd.SetComputeRoot32BitConstants( @@ -285,7 +292,7 @@ impl D3D12MipmapGen { } let uav_barrier = ManuallyDrop::new(D3D12_RESOURCE_UAV_BARRIER { - pResource: ManuallyDrop::new(Some(resource.clone())), + pResource: unsafe { S::obtain(resource) }, }); let barriers = [ @@ -294,13 +301,13 @@ impl D3D12MipmapGen { Anonymous: D3D12_RESOURCE_BARRIER_0 { UAV: uav_barrier }, ..Default::default() }, - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( resource, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, i, ), - util::d3d12_get_resource_transition_subresource( + util::d3d12_get_resource_transition_subresource::( resource, D3D12_RESOURCE_STATE_UNORDERED_ACCESS, D3D12_RESOURCE_STATE_PIXEL_SHADER_RESOURCE, @@ -312,7 +319,7 @@ impl D3D12MipmapGen { cmd.ResourceBarrier(&barriers); } - residual_barriers.extend(barriers) + S::cleanup_handler(|| residual_barriers.extend(barriers)); } Ok((heap_slots, residual_barriers)) diff --git a/librashader-runtime-d3d12/src/resource.rs b/librashader-runtime-d3d12/src/resource.rs index 7bb347e..6ed6900 100644 --- a/librashader-runtime-d3d12/src/resource.rs +++ b/librashader-runtime-d3d12/src/resource.rs @@ -1,37 +1,64 @@ -use crate::filter_chain::FrameResiduals; use std::mem::ManuallyDrop; use std::ops::Deref; use windows::Win32::Graphics::Direct3D12::ID3D12Resource; pub trait ResourceHandleStrategy { + const NEEDS_CLEANUP: bool; unsafe fn obtain(handle: &H) -> ManuallyDrop>; - unsafe fn cleanup(gc: &mut FrameResiduals, handle: ManuallyDrop>); + + /// Run a clean-up handler if the resource type needs clean up. + fn cleanup_handler(cleanup_fn: F) { + if Self::NEEDS_CLEANUP { + cleanup_fn(); + } + } } +pub trait ObtainResourceHandle { + fn handle(&self) -> &ID3D12Resource; +} + +impl ObtainResourceHandle for ID3D12Resource { + fn handle(&self) -> &ID3D12Resource { + &self + } +} + +impl ObtainResourceHandle for ManuallyDrop { + fn handle(&self) -> &ID3D12Resource { + self.deref() + } +} + +// this isn't actually used anymore but keep it around just in case. pub struct IncrementRefcount; impl ResourceHandleStrategy> for IncrementRefcount { + const NEEDS_CLEANUP: bool = true; + unsafe fn obtain( handle: &ManuallyDrop, ) -> ManuallyDrop> { ManuallyDrop::new(Some(handle.deref().clone())) } +} - unsafe fn cleanup(gc: &mut FrameResiduals, handle: ManuallyDrop>) { - gc.dispose_resource(handle); +impl ResourceHandleStrategy for IncrementRefcount { + const NEEDS_CLEANUP: bool = true; + + unsafe fn obtain(handle: &ID3D12Resource) -> ManuallyDrop> { + ManuallyDrop::new(Some(handle.clone())) } } pub struct OutlivesFrame; impl ResourceHandleStrategy> for OutlivesFrame { + const NEEDS_CLEANUP: bool = false; + unsafe fn obtain( handle: &ManuallyDrop, ) -> ManuallyDrop> { unsafe { std::mem::transmute_copy(handle) } } - - unsafe fn cleanup(_gc: &mut FrameResiduals, _handle: ManuallyDrop>) { - // Since the lifetime is ensured for the lifetime of the filter chain strategy, do nothing - } } diff --git a/librashader-runtime-d3d12/src/texture.rs b/librashader-runtime-d3d12/src/texture.rs index 247d659..5b38e30 100644 --- a/librashader-runtime-d3d12/src/texture.rs +++ b/librashader-runtime-d3d12/src/texture.rs @@ -1,6 +1,8 @@ use crate::descriptor_heap::{CpuStagingHeap, RenderTargetHeap}; +use crate::resource::ResourceHandleStrategy; use d3d12_descriptor_heap::D3D12DescriptorHeapSlot; use librashader_common::{FilterMode, GetSize, Size, WrapMode}; +use std::mem::ManuallyDrop; use windows::Win32::Graphics::Direct3D12::{ID3D12Resource, D3D12_CPU_DESCRIPTOR_HANDLE}; use windows::Win32::Graphics::Dxgi::Common::DXGI_FORMAT; @@ -17,6 +19,12 @@ pub(crate) enum InputDescriptor { Raw(D3D12_CPU_DESCRIPTOR_HANDLE), } +impl InputDescriptor { + fn is_raw(&self) -> bool { + matches!(self, InputDescriptor::Raw(_)) + } +} + #[derive(Clone)] pub(crate) enum OutputDescriptor { Owned(D3D12DescriptorHeapSlot), @@ -80,19 +88,19 @@ impl D3D12OutputView { } } -#[derive(Clone)] pub struct InputTexture { - pub(crate) resource: ID3D12Resource, + pub(crate) resource: ManuallyDrop, pub(crate) descriptor: InputDescriptor, pub(crate) size: Size, pub(crate) format: DXGI_FORMAT, pub(crate) wrap_mode: WrapMode, pub(crate) filter: FilterMode, + drop_flag: bool, } impl InputTexture { - pub fn new( - resource: ID3D12Resource, + pub fn new, T>( + resource: &T, handle: D3D12DescriptorHeapSlot, size: Size, format: DXGI_FORMAT, @@ -101,12 +109,18 @@ impl InputTexture { ) -> InputTexture { let srv = InputDescriptor::Owned(handle); InputTexture { - resource, + // SAFETY: `new` is only used for owned textures. We know this because + // we also hold `handle`, so the texture is at least + // as valid for the lifetime of handle. + // Also, resource is non-null by construction. + // Option and have the same layout. + resource: unsafe { std::mem::transmute(S::obtain(resource)) }, descriptor: srv, size, format, wrap_mode, filter, + drop_flag: S::NEEDS_CLEANUP, } } @@ -118,12 +132,42 @@ impl InputTexture { ) -> InputTexture { let desc = unsafe { image.resource.GetDesc() }; InputTexture { - resource: image.resource, + resource: ManuallyDrop::new(image.resource.clone()), descriptor: InputDescriptor::Raw(image.descriptor), size: Size::new(desc.Width as u32, desc.Height), format: desc.Format, wrap_mode, filter, + drop_flag: true, + } + } +} + +impl Clone for InputTexture { + fn clone(&self) -> Self { + // ensure lifetime for raw resources or if there is a drop flag + if self.descriptor.is_raw() || self.drop_flag { + InputTexture { + resource: ManuallyDrop::clone(&self.resource), + descriptor: self.descriptor.clone(), + size: self.size, + format: self.format, + wrap_mode: self.wrap_mode, + filter: self.filter, + drop_flag: true, + } + } else { + // SAFETY: the parent doesn't have drop flag, so that means + // we don't need to handle drop. + InputTexture { + resource: unsafe { std::mem::transmute_copy(&self.resource) }, + descriptor: self.descriptor.clone(), + size: self.size, + format: self.format, + wrap_mode: self.wrap_mode, + filter: self.filter, + drop_flag: false, + } } } } @@ -141,3 +185,11 @@ impl GetSize for D3D12OutputView { Ok(self.size) } } + +impl Drop for InputTexture { + fn drop(&mut self) { + if self.drop_flag { + unsafe { ManuallyDrop::drop(&mut self.resource) } + } + } +} diff --git a/librashader-runtime-d3d12/src/util.rs b/librashader-runtime-d3d12/src/util.rs index 4376210..23dfb6e 100644 --- a/librashader-runtime-d3d12/src/util.rs +++ b/librashader-runtime-d3d12/src/util.rs @@ -7,8 +7,9 @@ use windows::Win32::Graphics::Direct3D::Dxc::{ DXC_CP_UTF8, }; +use crate::resource::ResourceHandleStrategy; use windows::Win32::Graphics::Direct3D12::{ - ID3D12Device, ID3D12GraphicsCommandList, ID3D12Resource, D3D12_FEATURE_DATA_FORMAT_SUPPORT, + ID3D12Device, ID3D12GraphicsCommandList, D3D12_FEATURE_DATA_FORMAT_SUPPORT, D3D12_FEATURE_FORMAT_SUPPORT, D3D12_RESOURCE_BARRIER, D3D12_RESOURCE_BARRIER_0, D3D12_RESOURCE_BARRIER_ALL_SUBRESOURCES, D3D12_RESOURCE_BARRIER_FLAG_NONE, D3D12_RESOURCE_BARRIER_TYPE_TRANSITION, D3D12_RESOURCE_STATES, @@ -174,9 +175,8 @@ pub fn dxc_validate_shader( } } -#[must_use = "Resource Barrier must be disposed"] -pub fn d3d12_get_resource_transition_subresource( - resource: &ID3D12Resource, +pub fn d3d12_get_resource_transition_subresource, T>( + resource: &T, before: D3D12_RESOURCE_STATES, after: D3D12_RESOURCE_STATES, subresource: u32, @@ -186,7 +186,7 @@ pub fn d3d12_get_resource_transition_subresource( Flags: D3D12_RESOURCE_BARRIER_FLAG_NONE, Anonymous: D3D12_RESOURCE_BARRIER_0 { Transition: ManuallyDrop::new(D3D12_RESOURCE_TRANSITION_BARRIER { - pResource: ManuallyDrop::new(Some(resource.clone())), + pResource: unsafe { S::obtain(resource) }, Subresource: subresource, StateBefore: before, StateAfter: after, @@ -195,15 +195,14 @@ pub fn d3d12_get_resource_transition_subresource( } } -#[must_use = "Resource Barrier must be disposed"] #[inline(always)] -pub fn d3d12_resource_transition( +pub fn d3d12_resource_transition, T>( cmd: &ID3D12GraphicsCommandList, - resource: &ID3D12Resource, + resource: &T, before: D3D12_RESOURCE_STATES, after: D3D12_RESOURCE_STATES, ) -> [D3D12_RESOURCE_BARRIER; 1] { - d3d12_resource_transition_subresource( + d3d12_resource_transition_subresource::( cmd, resource, before, @@ -214,14 +213,14 @@ pub fn d3d12_resource_transition( #[must_use = "Resource Barrier must be disposed"] #[inline(always)] -pub fn d3d12_resource_transition_subresource( +fn d3d12_resource_transition_subresource, T>( cmd: &ID3D12GraphicsCommandList, - resource: &ID3D12Resource, + resource: &T, before: D3D12_RESOURCE_STATES, after: D3D12_RESOURCE_STATES, subresource: u32, ) -> [D3D12_RESOURCE_BARRIER; 1] { - let barrier = [d3d12_get_resource_transition_subresource( + let barrier = [d3d12_get_resource_transition_subresource::( resource, before, after, diff --git a/librashader-runtime-d3d12/tests/hello_triangle/mod.rs b/librashader-runtime-d3d12/tests/hello_triangle/mod.rs index 825e3a3..354fdfa 100644 --- a/librashader-runtime-d3d12/tests/hello_triangle/mod.rs +++ b/librashader-runtime-d3d12/tests/hello_triangle/mod.rs @@ -188,7 +188,7 @@ fn get_hardware_adapter(factory: &IDXGIFactory4) -> Result { for i in 0.. { let adapter = unsafe { factory.EnumAdapters1(i)? }; - let mut desc = unsafe { adapter.GetDesc1()? }; + let desc = unsafe { adapter.GetDesc1()? }; if (desc.Flags & DXGI_ADAPTER_FLAG_SOFTWARE.0 as u32) != DXGI_ADAPTER_FLAG_NONE.0 as u32 { // Don't select the Basic Render Driver adapter. If you want a diff --git a/librashader-runtime-d3d12/tests/triangle.rs b/librashader-runtime-d3d12/tests/triangle.rs index f003026..ea7c188 100644 --- a/librashader-runtime-d3d12/tests/triangle.rs +++ b/librashader-runtime-d3d12/tests/triangle.rs @@ -5,8 +5,8 @@ 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/crt/crt-royale.slangp", + "../test/shaders_slang/bezel/Mega_Bezel/Presets/MBZ__0__SMOOTH-ADV.slangp", + // "../test/shaders_slang/crt/crt-royale.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",