[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

@@ -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);
 }

idd/LGIdd/CD3D12Device.h

@@ -80,5 +80,5 @@ struct CD3D12Device
     bool IsIndirectCopy() { return m_indirectCopy; }
 
     CD3D12CommandSlot * GetCopySlot   (unsigned frameIndex);
-    CD3D12CommandSlot * GetComputeSlot();
+    CD3D12CommandSlot * GetComputeSlot(unsigned frameIndex);
 };

idd/LGIdd/CPostProcessor.cpp

@@ -131,22 +131,43 @@ void CPostProcessor::Reset()
   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  &&
+  if (!NeedsReconfigure(srcFormat))
-      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)
   {
     // Static HDR metadata may change independently of the resource format.
     // Propagate it without recreating textures or post-processing state.
@@ -157,8 +178,7 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha
 
   D12FrameFormat oldDst        = m_dstFormat;
   D12FrameFormat cur           = srcFormat;
-  m_srcFormat = srcFormat;
+  bool           effectsActive = false;
-  m_effectsActive = false;
 
   for (const auto& effect : m_effects)
   {
@@ -167,7 +187,7 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha
     {
     case PostProcessStatus::SUCCESS:
       effect->Enabled = true;
-      m_effectsActive = true;
+      effectsActive   = true;
       cur             = dst;
       DEBUG_INFO("Post-processing effect active: %s", effect->GetName());
       break;
@@ -182,9 +202,13 @@ bool CPostProcessor::Configure(const D12FrameFormat& srcFormat, bool * formatCha
     }
   }
 
+  m_srcFormat     = srcFormat;
   m_dstFormat     = cur;
+  m_effectsActive = effectsActive;
+  m_configured    = true;
   if (formatChanged)
-    *formatChanged = oldDst.desc.Width    != m_dstFormat.desc.Width  ||
+    *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         ||

idd/LGIdd/CPostProcessor.h

@@ -103,11 +103,14 @@ private:
   D12FrameFormat                                   m_srcFormat     = {};
   D12FrameFormat                                   m_dstFormat     = {};
   bool                                             m_effectsActive = false;
+  bool                                             m_configured    = false;
 
 public:
   bool Init(std::shared_ptr<CD3D12Device> 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<ID3D12Resource> Run(

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))
+  else
-    DEBUG_ERROR("Failed to initialize post processor");
+  {
+    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<IDXGIResource> 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))
+  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<IDXGIResource> 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<IDXGIResource> acquiredBuffer
     memcpy(frameDirtyRects, currentDirtyRects, nbDirtyRects * sizeof(*frameDirtyRects));
   }
   unsigned frameDirtyRectCount = nbDirtyRects;
-  m_postProcessor.AdjustFrameDamage(frameDirtyRects, &frameDirtyRectCount);
+  m_postProcessors[0].AdjustFrameDamage(
-
+    frameDirtyRects, &frameDirtyRectCount);
-  ComPtr<ID3D12Resource> 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);
 
   auto buffer = m_devContext->PrepareFrameBuffer(
     (unsigned)layout.Footprint.RowPitch,
@@ -886,6 +910,8 @@ bool CSwapChainProcessor::SwapChainNewFrame(ComPtr<IDXGIResource> 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<IDXGIResource> acquiredBuffer
     return false;
   }
 
-  const bool syncResult = computeSlot ?
+  ComPtr<ID3D12Resource> copySrcResource = srcRes->GetRes();
-    copySlot->WaitFor(*computeSlot) : srcRes->Sync(*copySlot);
+  CD3D12CommandSlot    * computeSlot     = nullptr;
-  if (!syncResult)
+  if (postProcessor.HasActiveEffects())
+  {
+    computeSlot = m_dx12Device->GetComputeSlot(buffer.frameIndex);
+    if (!computeSlot)
     {
       copySlot->Cancel();
       m_devContext->AbortFrameBuffer(buffer.frameIndex);
-    DEBUG_ERROR("Failed to queue copy synchronization");
+      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 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<IDXGIResource> acquiredBuffer
   if (!copySlot->Execute())
   {
     if (!copySlot->HasSubmittedWork())
+    {
+      if (computeSlot)
+        m_dx12Device->WaitForIdle();
       m_devContext->AbortFrameBuffer(buffer.frameIndex);
+    }
     SetFullPendingDamage();
     return false;
   }

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<Wrappers::HandleTraits::HANDLENullTraits> m_thread[2];
   Wrappers::Event m_terminateEvent;

idd/LGIdd/effect/CColorTransformEffect.cpp

@@ -280,9 +280,9 @@ ComPtr<ID3D12Resource> CColorTransformEffect::Run(
   UNREFERENCED_PARAMETER(dirtyRects);
   UNREFERENCED_PARAMETER(nbDirtyRects);
 
-  // GetComputeSlot waits for the previous submission before Run is called,
+  // The framebuffer-indexed compute slot waits for this chain's previous
-  // so this is the first point where the shared upload buffers are guaranteed
+  // submission before Run is called, so this is the first point where its
-  // not to be in use by the GPU.
+  // upload buffers are guaranteed not to be in use by the GPU.
   if (m_uploadPending)
   {
     if (!Upload(m_constBuffer, &m_consts, sizeof(m_consts)) ||