Skip to content

Commit

Permalink
rt(d3d12): stop leaking transition barriers
Browse files Browse the repository at this point in the history
  • Loading branch information
@chyyran
chyyran committed Aug 26, 2024
1 parent 2e7c3b3 commit e934f17
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 85 deletions.
63 changes: 41 additions & 22 deletions librashader-runtime-d3d12/src/filter_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -104,6 +106,7 @@ pub(crate) struct FrameResiduals {
mipmaps: Vec<D3D12DescriptorHeapSlot<ResourceWorkHeap>>,
mipmap_luts: Vec<D3D12MipmapGen>,
resources: Vec<ManuallyDrop<Option<ID3D12Resource>>>,
resource_barriers: Vec<D3D12_RESOURCE_BARRIER>,
}

impl FrameResiduals {
Expand All @@ -113,6 +116,7 @@ impl FrameResiduals {
mipmaps: Vec::new(),
mipmap_luts: Vec::new(),
resources: Vec::new(),
resource_barriers: Vec::new(),
}
}

Expand All @@ -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<Item = D3D12_RESOURCE_BARRIER>) {
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
}
}
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)?;
}
}
}
Expand Down Expand Up @@ -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);
Expand All @@ -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| {
Expand All @@ -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);
Expand Down
12 changes: 8 additions & 4 deletions librashader-runtime-d3d12/src/framebuffer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())),
Expand Down Expand Up @@ -236,31 +237,34 @@ impl OwnedImage {
cmd.ResourceBarrier(&barriers);
}

gc.dispose_barriers(barriers);

Ok(())
}

pub fn clear(
&self,
cmd: &ID3D12GraphicsCommandList,
heap: &mut D3D12DescriptorHeap<RenderTargetHeap>,
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(())
}
Expand Down
29 changes: 4 additions & 25 deletions librashader-runtime-d3d12/src/luts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,39 +157,18 @@ impl LutTexture {
resource_type: &ResourceType::Placed,
})?;

//
// let mut upload: Option<ID3D12Resource> = 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,
Expand All @@ -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(),
Expand Down
26 changes: 11 additions & 15 deletions librashader-runtime-d3d12/src/mipmap.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub struct MipmapGenContext<'a> {
cmd: &'a ID3D12GraphicsCommandList,
heap: &'a mut D3D12DescriptorHeap<ResourceWorkHeap>,
residuals: Vec<D3D12DescriptorHeapSlot<ResourceWorkHeap>>,
residual_uav_descs: Vec<D3D12_RESOURCE_UAV_BARRIER>,
residual_barriers: Vec<D3D12_RESOURCE_BARRIER>,
}

impl<'a> MipmapGenContext<'a> {
Expand All @@ -56,7 +56,7 @@ impl<'a> MipmapGenContext<'a> {
cmd,
heap,
residuals: Vec::new(),
residual_uav_descs: Vec::new(),
residual_barriers: Vec::new(),
}
}

Expand All @@ -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(())
Expand All @@ -84,9 +84,9 @@ impl<'a> MipmapGenContext<'a> {
self,
) -> (
Vec<D3D12DescriptorHeapSlot<ResourceWorkHeap>>,
Vec<D3D12_RESOURCE_UAV_BARRIER>,
Vec<D3D12_RESOURCE_BARRIER>,
) {
(self.residuals, self.residual_uav_descs)
(self.residuals, self.residual_barriers)
}
}

Expand Down Expand Up @@ -153,7 +153,7 @@ impl D3D12MipmapGen {
) -> Result<
(
Vec<D3D12DescriptorHeapSlot<ResourceWorkHeap>>,
Vec<D3D12_RESOURCE_UAV_BARRIER>,
Vec<D3D12_RESOURCE_BARRIER>,
),
E,
>
Expand Down Expand Up @@ -188,7 +188,7 @@ impl D3D12MipmapGen {
work_heap: &mut D3D12DescriptorHeap<ResourceWorkHeap>,
) -> error::Result<(
Vec<D3D12DescriptorHeapSlot<ResourceWorkHeap>>,
Vec<D3D12_RESOURCE_UAV_BARRIER>,
Vec<D3D12_RESOURCE_BARRIER>,
)> {
// create views for mipmap generation
let srv = work_heap.alloc_slot()?;
Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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))
}
}
38 changes: 21 additions & 17 deletions librashader-runtime-d3d12/src/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -216,21 +201,40 @@ 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,
resource: &ID3D12Resource,
before: D3D12_RESOURCE_STATES,
after: D3D12_RESOURCE_STATES,
subresource: u32,
) {
) -> [D3D12_RESOURCE_BARRIER; 1] {
let barrier = [d3d12_get_resource_transition_subresource(
resource,
before,
after,
subresource,
)];
unsafe { cmd.ResourceBarrier(&barrier) }
barrier
}

pub(crate) fn d3d12_update_subresources(
Expand Down
Loading

0 comments on commit e934f17

Please sign in to comment.