From fa2769b332d7c8d6a5b120829d982c277aabfb1a Mon Sep 17 00:00:00 2001 From: Geoffrey McRae Date: Mon, 3 Aug 2026 19:01:38 +1000 Subject: [PATCH] [idd] isolate post-processing by framebuffer slot Give each in-flight framebuffer a complete post-processing chain and its own COMPUTE recording slot while retaining one physical queue. Claim framebuffer ownership before submitting compute work, and use the frame index for both COMPUTE and COPY recording state. Drain in-flight work before reconfiguring either chain so COPY never references replaced effect resources. Require both chains to contain the same effects, preserve pending damage, and release ownership on every pre-copy failure path. --- idd/LGIdd/CD3D12Device.cpp | 6 +- idd/LGIdd/CD3D12Device.h | 4 +- idd/LGIdd/CPostProcessor.cpp | 82 ++++++---- idd/LGIdd/CPostProcessor.h | 13 +- idd/LGIdd/CSwapChainProcessor.cpp | 171 +++++++++++++++------ idd/LGIdd/CSwapChainProcessor.h | 2 +- idd/LGIdd/effect/CColorTransformEffect.cpp | 6 +- 7 files changed, 198 insertions(+), 86 deletions(-) diff --git a/idd/LGIdd/CD3D12Device.cpp b/idd/LGIdd/CD3D12Device.cpp index ef9fff9c..bdb5fb99 100644 --- a/idd/LGIdd/CD3D12Device.cpp +++ b/idd/LGIdd/CD3D12Device.cpp @@ -137,7 +137,7 @@ CD3D12Device::InitResult CD3D12Device::Init(CIVSHMEM &ivshmem, if (m_computeEnabled && !m_computeQueue.Init(m_device.Get(), D3D12_COMMAND_LIST_TYPE_COMPUTE, - L"Compute", CD3D12CommandSlot::FAST, 1)) + L"Compute", CD3D12CommandSlot::FAST, 2)) return InitResult::FAILURE; DEBUG_INFO("Created CD3D12Device"); @@ -207,7 +207,7 @@ CD3D12CommandSlot * CD3D12Device::GetCopySlot(unsigned frameIndex) return m_copyQueue.Acquire(frameIndex); } -CD3D12CommandSlot * CD3D12Device::GetComputeSlot() +CD3D12CommandSlot * CD3D12Device::GetComputeSlot(unsigned frameIndex) { if (!m_computeEnabled) { @@ -215,5 +215,5 @@ CD3D12CommandSlot * CD3D12Device::GetComputeSlot() return nullptr; } - return m_computeQueue.Acquire(); + return m_computeQueue.Acquire(frameIndex); } diff --git a/idd/LGIdd/CD3D12Device.h b/idd/LGIdd/CD3D12Device.h index 60ec44fb..60ae39ef 100644 --- a/idd/LGIdd/CD3D12Device.h +++ b/idd/LGIdd/CD3D12Device.h @@ -79,6 +79,6 @@ struct CD3D12Device ComPtr GetHeap() { return m_ivshmemHeap; } bool IsIndirectCopy() { return m_indirectCopy; } - CD3D12CommandSlot * GetCopySlot(unsigned frameIndex); - CD3D12CommandSlot * GetComputeSlot(); + CD3D12CommandSlot * GetCopySlot (unsigned frameIndex); + CD3D12CommandSlot * GetComputeSlot(unsigned frameIndex); }; diff --git a/idd/LGIdd/CPostProcessor.cpp b/idd/LGIdd/CPostProcessor.cpp index a1fbf9bd..98138f2d 100644 --- a/idd/LGIdd/CPostProcessor.cpp +++ b/idd/LGIdd/CPostProcessor.cpp @@ -128,25 +128,46 @@ void CPostProcessor::Reset() m_effects.clear(); m_dx12Device.reset(); m_device.Reset(); - m_srcFormat = {}; - m_dstFormat = {}; + m_srcFormat = {}; + m_dstFormat = {}; m_effectsActive = false; + m_configured = false; } -bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatChanged) +bool CPostProcessor::HasSameEffectChain(const CPostProcessor& other) const +{ + if (m_effects.size() != other.m_effects.size()) + return false; + + for (size_t i = 0; i < m_effects.size(); ++i) + if (std::strcmp(m_effects[i]->GetName(), + other.m_effects[i]->GetName()) != 0) + return false; + + return true; +} + +bool CPostProcessor::NeedsReconfigure(const D12FrameFormat& srcFormat) const +{ + return !m_configured || + srcFormat.desc.Width != m_srcFormat.desc.Width || + srcFormat.desc.Height != m_srcFormat.desc.Height || + srcFormat.desc.Format != m_srcFormat.desc.Format || + srcFormat.format != m_srcFormat.format || + srcFormat.width != m_srcFormat.width || + srcFormat.height != m_srcFormat.height || + srcFormat.hdr != m_srcFormat.hdr || + srcFormat.hdrPQ != m_srcFormat.hdrPQ || + srcFormat.colorTransform != m_srcFormat.colorTransform; +} + +bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, + bool * formatChanged) { if (formatChanged) *formatChanged = false; - if (srcFormat.desc.Width == m_srcFormat.desc.Width && - srcFormat.desc.Height == m_srcFormat.desc.Height && - srcFormat.desc.Format == m_srcFormat.desc.Format && - srcFormat.format == m_srcFormat.format && - srcFormat.width == m_srcFormat.width && - srcFormat.height == m_srcFormat.height && - srcFormat.hdr == m_srcFormat.hdr && - srcFormat.hdrPQ == m_srcFormat.hdrPQ && - srcFormat.colorTransform == m_srcFormat.colorTransform) + if (!NeedsReconfigure(srcFormat)) { // Static HDR metadata may change independently of the resource format. // Propagate it without recreating textures or post-processing state. @@ -155,10 +176,9 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha return true; } - D12FrameFormat oldDst = m_dstFormat; - D12FrameFormat cur = srcFormat; - m_srcFormat = srcFormat; - m_effectsActive = false; + D12FrameFormat oldDst = m_dstFormat; + D12FrameFormat cur = srcFormat; + bool effectsActive = false; for (const auto& effect : m_effects) { @@ -167,8 +187,8 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha { case PostProcessStatus::SUCCESS: effect->Enabled = true; - m_effectsActive = true; - cur = dst; + effectsActive = true; + cur = dst; DEBUG_INFO("Post-processing effect active: %s", effect->GetName()); break; @@ -182,18 +202,22 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha } } - m_dstFormat = cur; + m_srcFormat = srcFormat; + m_dstFormat = cur; + m_effectsActive = effectsActive; + m_configured = true; if (formatChanged) - *formatChanged = oldDst.desc.Width != m_dstFormat.desc.Width || - oldDst.desc.Height != m_dstFormat.desc.Height || - oldDst.desc.Format != m_dstFormat.desc.Format || - oldDst.format != m_dstFormat.format || - oldDst.width != m_dstFormat.width || - oldDst.height != m_dstFormat.height || - oldDst.hdr != m_dstFormat.hdr || - oldDst.hdrPQ != m_dstFormat.hdrPQ || - oldDst.sdrWhiteLevel != m_dstFormat.sdrWhiteLevel || - oldDst.colorTransform != m_dstFormat.colorTransform; + *formatChanged = + oldDst.desc.Width != m_dstFormat.desc.Width || + oldDst.desc.Height != m_dstFormat.desc.Height || + oldDst.desc.Format != m_dstFormat.desc.Format || + oldDst.format != m_dstFormat.format || + oldDst.width != m_dstFormat.width || + oldDst.height != m_dstFormat.height || + oldDst.hdr != m_dstFormat.hdr || + oldDst.hdrPQ != m_dstFormat.hdrPQ || + oldDst.sdrWhiteLevel != m_dstFormat.sdrWhiteLevel || + oldDst.colorTransform != m_dstFormat.colorTransform; return true; } diff --git a/idd/LGIdd/CPostProcessor.h b/idd/LGIdd/CPostProcessor.h index 7df3c572..04472c4f 100644 --- a/idd/LGIdd/CPostProcessor.h +++ b/idd/LGIdd/CPostProcessor.h @@ -97,17 +97,20 @@ public: class CPostProcessor { private: - std::shared_ptr m_dx12Device; - ComPtr m_device; + std::shared_ptr m_dx12Device; + ComPtr m_device; std::vector> m_effects; - D12FrameFormat m_srcFormat = {}; - D12FrameFormat m_dstFormat = {}; - bool m_effectsActive = false; + D12FrameFormat m_srcFormat = {}; + D12FrameFormat m_dstFormat = {}; + bool m_effectsActive = false; + bool m_configured = false; public: bool Init(std::shared_ptr dx12Device); void Reset(); + bool HasSameEffectChain(const CPostProcessor& other) const; + bool NeedsReconfigure(const D12FrameFormat& srcFormat) const; bool Configure(const D12FrameFormat& srcFormat, bool * formatChanged); void AdjustFrameDamage(RECT dirtyRects[], unsigned * nbDirtyRects); ComPtr Run( diff --git a/idd/LGIdd/CSwapChainProcessor.cpp b/idd/LGIdd/CSwapChainProcessor.cpp index 827635bf..9b488e66 100644 --- a/idd/LGIdd/CSwapChainProcessor.cpp +++ b/idd/LGIdd/CSwapChainProcessor.cpp @@ -81,8 +81,32 @@ CSwapChainProcessor::CSwapChainProcessor(CIndirectMonitorContext * monitorContex m_fbPool.Init(this); if (m_dx11Device->IsSoftware()) DEBUG_INFO("Software render adapter: post-processing disabled"); - else if (!m_postProcessor.Init(dx12Device)) - DEBUG_ERROR("Failed to initialize post processor"); + else + { + bool initialized = true; + for (CPostProcessor& postProcessor : m_postProcessors) + if (!postProcessor.Init(dx12Device)) + { + initialized = false; + break; + } + + if (initialized) + for (unsigned i = 1; i < ARRAYSIZE(m_postProcessors); ++i) + if (!m_postProcessors[0].HasSameEffectChain(m_postProcessors[i])) + { + DEBUG_ERROR("Post processor effect chains do not match"); + initialized = false; + break; + } + + if (!initialized) + { + for (CPostProcessor& postProcessor : m_postProcessors) + postProcessor.Reset(); + DEBUG_ERROR("Failed to initialize post processors"); + } + } // Manual-reset: both worker threads wait on this, so it must stay signalled // once set or only one thread would ever observe termination. @@ -107,7 +131,8 @@ CSwapChainProcessor::~CSwapChainProcessor() // worker epilogue, so this does not hold an IddCx frame. m_dx12Device->WaitForIdle(); - m_postProcessor.Reset(); + for (CPostProcessor& postProcessor : m_postProcessors) + postProcessor.Reset(); m_resPool.Reset(); m_fbPool.Reset(); delete[] m_shapeBuffer; @@ -792,10 +817,38 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer } const bool frameMetadataChanged = noImageUpdate && - FrameMetadataChanged(m_postProcessor.GetOutputFormat(), srcFormat); + FrameMetadataChanged(m_postProcessors[0].GetOutputFormat(), srcFormat); + + bool needsReconfigure = false; + for (const CPostProcessor& postProcessor : m_postProcessors) + if (postProcessor.NeedsReconfigure(srcFormat)) + { + needsReconfigure = true; + break; + } + + // SetFormat can replace resources still being read by the COPY queue. Format + // changes are rare, so drain both queues before updating either frame chain. + if (needsReconfigure) + { + m_nbDirtyRects = 0; + SetFullPendingDamage(); + m_dx12Device->WaitForIdle(); + } + bool postProcessFormatChanged = false; - if (!m_postProcessor.Configure(srcFormat, &postProcessFormatChanged)) - return false; + for (unsigned i = 0; i < ARRAYSIZE(m_postProcessors); ++i) + { + bool formatChanged = false; + if (!m_postProcessors[i].Configure(srcFormat, &formatChanged)) + { + SetFullPendingDamage(); + return false; + } + + if (i == 0) + postProcessFormatChanged = formatChanged; + } if (postProcessFormatChanged) { @@ -808,7 +861,8 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer if (noImageUpdate && !m_hasPendingDamage) return true; - const D12FrameFormat& dstFormat = m_postProcessor.GetOutputFormat(); + const D12FrameFormat& dstFormat = + m_postProcessors[0].GetOutputFormat(); D3D12_PLACED_SUBRESOURCE_FOOTPRINT layout; m_dx12Device->GetDevice()->GetCopyableFootprints( @@ -830,38 +884,8 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer memcpy(frameDirtyRects, currentDirtyRects, nbDirtyRects * sizeof(*frameDirtyRects)); } unsigned frameDirtyRectCount = nbDirtyRects; - m_postProcessor.AdjustFrameDamage(frameDirtyRects, &frameDirtyRectCount); - - ComPtr copySrcResource = srcRes->GetRes(); - CD3D12CommandSlot * computeSlot = nullptr; - if (m_postProcessor.HasActiveEffects()) - { - computeSlot = m_dx12Device->GetComputeSlot(); - if (!computeSlot) - { - DEBUG_ERROR("Failed to get a compute CommandSlot"); - return false; - } - - if (!srcRes->Sync(*computeSlot)) - { - computeSlot->Cancel(); - SetFullPendingDamage(); - return false; - } - - copySrcResource = m_postProcessor.Run( - computeSlot->GetGfxList(), copySrcResource, - currentDirtyRects, &nbDirtyRects); - - if (!computeSlot->Execute()) - { - SetFullPendingDamage(); - return false; - } - } - - ClipDirtyRects(currentDirtyRects, &nbDirtyRects, dstFormat.desc); + m_postProcessors[0].AdjustFrameDamage( + frameDirtyRects, &frameDirtyRectCount); auto buffer = m_devContext->PrepareFrameBuffer( (unsigned)layout.Footprint.RowPitch, @@ -886,6 +910,8 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer return false; } + CPostProcessor& postProcessor = m_postProcessors[buffer.frameIndex]; + CD3D12CommandSlot * copySlot = m_dx12Device->GetCopySlot(buffer.frameIndex); if (!copySlot) @@ -896,17 +922,72 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer return false; } - const bool syncResult = computeSlot ? - copySlot->WaitFor(*computeSlot) : srcRes->Sync(*copySlot); - if (!syncResult) + ComPtr copySrcResource = srcRes->GetRes(); + CD3D12CommandSlot * computeSlot = nullptr; + if (postProcessor.HasActiveEffects()) + { + computeSlot = m_dx12Device->GetComputeSlot(buffer.frameIndex); + if (!computeSlot) + { + copySlot->Cancel(); + m_devContext->AbortFrameBuffer(buffer.frameIndex); + DEBUG_ERROR("Failed to get a compute CommandSlot"); + SetFullPendingDamage(); + return false; + } + + if (!srcRes->Sync(*computeSlot)) + { + computeSlot->Cancel(); + copySlot->Cancel(); + m_devContext->AbortFrameBuffer(buffer.frameIndex); + SetFullPendingDamage(); + return false; + } + + copySrcResource = postProcessor.Run( + computeSlot->GetGfxList(), copySrcResource, + currentDirtyRects, &nbDirtyRects); + if (!copySrcResource) + { + computeSlot->Cancel(); + copySlot->Cancel(); + m_devContext->AbortFrameBuffer(buffer.frameIndex); + DEBUG_ERROR("Post processor returned no output resource"); + SetFullPendingDamage(); + return false; + } + + if (!computeSlot->Execute()) + { + copySlot->Cancel(); + m_dx12Device->WaitForIdle(); + m_devContext->AbortFrameBuffer(buffer.frameIndex); + SetFullPendingDamage(); + return false; + } + + if (!copySlot->WaitFor(*computeSlot)) + { + copySlot->Cancel(); + m_dx12Device->WaitForIdle(); + m_devContext->AbortFrameBuffer(buffer.frameIndex); + DEBUG_ERROR("Failed to queue compute synchronization"); + SetFullPendingDamage(); + return false; + } + } + else if (!srcRes->Sync(*copySlot)) { copySlot->Cancel(); m_devContext->AbortFrameBuffer(buffer.frameIndex); - DEBUG_ERROR("Failed to queue copy synchronization"); + DEBUG_ERROR("Failed to queue source synchronization"); SetFullPendingDamage(); return false; } + ClipDirtyRects(currentDirtyRects, &nbDirtyRects, dstFormat.desc); + const uint64_t copyStart = Nanotime(); fbRes->SetTiming(captureTime, postProcessStart, copyStart); @@ -974,7 +1055,11 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr acquiredBuffer if (!copySlot->Execute()) { if (!copySlot->HasSubmittedWork()) + { + if (computeSlot) + m_dx12Device->WaitForIdle(); m_devContext->AbortFrameBuffer(buffer.frameIndex); + } SetFullPendingDamage(); return false; } diff --git a/idd/LGIdd/CSwapChainProcessor.h b/idd/LGIdd/CSwapChainProcessor.h index 6f732bf2..1ea3a4af 100644 --- a/idd/LGIdd/CSwapChainProcessor.h +++ b/idd/LGIdd/CSwapChainProcessor.h @@ -53,7 +53,7 @@ private: CInteropResourcePool m_resPool; CFrameBufferPool m_fbPool; - CPostProcessor m_postProcessor; + CPostProcessor m_postProcessors[LGMP_Q_FRAME_LEN]; Wrappers::HandleT m_thread[2]; Wrappers::Event m_terminateEvent; diff --git a/idd/LGIdd/effect/CColorTransformEffect.cpp b/idd/LGIdd/effect/CColorTransformEffect.cpp index 7c9b74ba..381ca680 100644 --- a/idd/LGIdd/effect/CColorTransformEffect.cpp +++ b/idd/LGIdd/effect/CColorTransformEffect.cpp @@ -280,9 +280,9 @@ ComPtr CColorTransformEffect::Run( UNREFERENCED_PARAMETER(dirtyRects); UNREFERENCED_PARAMETER(nbDirtyRects); - // GetComputeSlot waits for the previous submission before Run is called, - // so this is the first point where the shared upload buffers are guaranteed - // not to be in use by the GPU. + // The framebuffer-indexed compute slot waits for this chain's previous + // submission before Run is called, so this is the first point where its + // upload buffers are guaranteed not to be in use by the GPU. if (m_uploadPending) { if (!Upload(m_constBuffer, &m_consts, sizeof(m_consts)) ||