Queue any indirect data fetches to happen outside of a renderpass

* Since vkCmdCopyBuffer can't run inside a renderpass we have to delay it until
  the renderpass ends. Warn if there is a subpass dependency that would allow
  writing the indirect args inside the RP (which we would then miss)

renderdoc/driver/vulkan/vk_core.h

@@ -72,6 +72,17 @@ enum class VkIndirectPatchType
   DrawIndirectByteCount,
 };
 
+struct VkIndirectRecordData
+{
+  VkBufferMemoryBarrier paramsBarrier, countBarrier;
+
+  struct
+  {
+    VkBuffer src, dst;
+    VkBufferCopy copy;
+  } paramsCopy, countCopy;
+};
+
 struct VkIndirectPatchData
 {
   VkIndirectPatchType type = VkIndirectPatchType::NoPatch;
@@ -509,6 +520,8 @@ private:
     vector<DebugMessage> debugMessages;
     std::list<VulkanDrawcallTreeNode *> drawStack;
 
+    std::vector<VkIndirectRecordData> indirectCopies;
+
     uint32_t beginChunk = 0;
     uint32_t endChunk = 0;
 
@@ -732,6 +745,8 @@ private:
                                         VkBuffer dataBuffer, VkDeviceSize dataOffset, uint32_t count,
                                         uint32_t stride = 0, VkBuffer counterBuffer = VK_NULL_HANDLE,
                                         VkDeviceSize counterOffset = 0);
+  void ExecuteIndirectReadback(VkCommandBuffer commandBuffer,
+                               const VkIndirectRecordData &indirectcopy);
 
   WriteSerialiser &GetThreadSerialiser();
   template <typename SerialiserType>

renderdoc/driver/vulkan/wrappers/vk_cmd_funcs.cpp

@@ -1298,6 +1298,11 @@ bool WrappedVulkan::Serialise_vkCmdEndRenderPass(SerialiserType &ser, VkCommandB
     {
       ObjDisp(commandBuffer)->CmdEndRenderPass(Unwrap(commandBuffer));
 
+      // fetch any queued indirect readbacks here
+      for(const VkIndirectRecordData &indirectcopy :
+          m_BakedCmdBufferInfo[m_LastCmdBufferID].indirectCopies)
+        ExecuteIndirectReadback(commandBuffer, indirectcopy);
+
       std::vector<VkImageMemoryBarrier> imgBarriers = GetImplicitRenderPassBarriers(~0U);
 
       ResourceId cmd = GetResID(commandBuffer);

renderdoc/driver/vulkan/wrappers/vk_draw_funcs.cpp

@@ -84,13 +84,15 @@ VkIndirectPatchData WrappedVulkan::FetchIndirectData(VkIndirectPatchType type,
   if(type == VkIndirectPatchType::DrawIndirectByteCount)
     buf.srcAccessMask |= VK_ACCESS_TRANSFORM_FEEDBACK_COUNTER_READ_BIT_EXT;
 
-  ObjDisp(commandBuffer)
-      ->CmdPipelineBarrier(Unwrap(commandBuffer), VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
-                           VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 0, NULL, 1, &buf, 0, NULL);
+  VkIndirectRecordData indirectcopy = {};
+
+  indirectcopy.paramsBarrier = buf;
 
   VkBufferCopy copy = {dataOffset, 0, dataSize};
-  ObjDisp(commandBuffer)
-      ->CmdCopyBuffer(Unwrap(commandBuffer), Unwrap(dataBuffer), Unwrap(paramsbuf), 1, &copy);
+
+  indirectcopy.paramsCopy.src = dataBuffer;
+  indirectcopy.paramsCopy.dst = paramsbuf;
+  indirectcopy.paramsCopy.copy = copy;
 
   if(counterBuffer != VK_NULL_HANDLE)
   {
@@ -98,17 +100,23 @@ VkIndirectPatchData WrappedVulkan::FetchIndirectData(VkIndirectPatchType type,
     buf.offset = counterOffset;
     buf.size = 4;
 
-    ObjDisp(commandBuffer)
-        ->CmdPipelineBarrier(Unwrap(commandBuffer), VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
-                             VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 0, NULL, 1, &buf, 0, NULL);
+    indirectcopy.countBarrier = buf;
 
     copy.srcOffset = counterOffset;
     copy.dstOffset = bufInfo.size - 16;
     copy.size = 4;
-    ObjDisp(commandBuffer)
-        ->CmdCopyBuffer(Unwrap(commandBuffer), Unwrap(counterBuffer), Unwrap(paramsbuf), 1, &copy);
+
+    indirectcopy.countCopy.src = counterBuffer;
+    indirectcopy.countCopy.dst = paramsbuf;
+    indirectcopy.countCopy.copy = copy;
   }
 
+  // if it's a dispatch we can do it immediately, otherwise we delay to the end of the renderpass
+  if(type == VkIndirectPatchType::DispatchIndirect)
+    ExecuteIndirectReadback(commandBuffer, indirectcopy);
+  else
+    m_BakedCmdBufferInfo[m_LastCmdBufferID].indirectCopies.push_back(indirectcopy);
+
   VkIndirectPatchData indirectPatch;
   indirectPatch.type = type;
   indirectPatch.alloc = alloc;
@@ -119,6 +127,31 @@ VkIndirectPatchData WrappedVulkan::FetchIndirectData(VkIndirectPatchType type,
   return indirectPatch;
 }
 
+void WrappedVulkan::ExecuteIndirectReadback(VkCommandBuffer commandBuffer,
+                                            const VkIndirectRecordData &indirectcopy)
+{
+  ObjDisp(commandBuffer)
+      ->CmdPipelineBarrier(Unwrap(commandBuffer), VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                           VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 0, NULL, 1,
+                           &indirectcopy.paramsBarrier, 0, NULL);
+
+  ObjDisp(commandBuffer)
+      ->CmdCopyBuffer(Unwrap(commandBuffer), Unwrap(indirectcopy.paramsCopy.src),
+                      Unwrap(indirectcopy.paramsCopy.dst), 1, &indirectcopy.paramsCopy.copy);
+
+  if(indirectcopy.countCopy.src != VK_NULL_HANDLE)
+  {
+    ObjDisp(commandBuffer)
+        ->CmdPipelineBarrier(Unwrap(commandBuffer), VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
+                             VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, 0, NULL, 1,
+                             &indirectcopy.countBarrier, 0, NULL);
+
+    ObjDisp(commandBuffer)
+        ->CmdCopyBuffer(Unwrap(commandBuffer), Unwrap(indirectcopy.countCopy.src),
+                        Unwrap(indirectcopy.countCopy.dst), 1, &indirectcopy.countCopy.copy);
+  }
+}
+
 bool WrappedVulkan::IsDrawInRenderPass()
 {
   BakedCmdBufferInfo &cmd = m_BakedCmdBufferInfo[m_LastCmdBufferID];

renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp

@@ -767,6 +767,22 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass(SerialiserType &ser, VkDevice d
         live = GetResourceManager()->WrapResource(Unwrap(device), rp);
         GetResourceManager()->AddLiveResource(RenderPass, rp);
 
+        bool badIndirectArgDep = false;
+
+        for(uint32_t i = 0; i < CreateInfo.dependencyCount; i++)
+          if(CreateInfo.pDependencies[i].dstAccessMask & VK_ACCESS_INDIRECT_COMMAND_READ_BIT)
+            badIndirectArgDep = true;
+
+        if(badIndirectArgDep)
+          AddDebugMessage(MessageCategory::State_Creation, MessageSeverity::High,
+                          MessageSource::RuntimeWarning,
+                          StringFormat::Fmt("Creating renderpass %s contains a subpass dependency "
+                                            "that would allow writing indirect command arguments.\n"
+                                            "Indirect command contents are read at the end of the "
+                                            "render pass, so write-after-read overwrites will "
+                                            "cause incorrect display of indirect arguments.",
+                                            ToStr(RenderPass)));
+
         // make a version of the render pass that loads from its attachments,
         // so it can be used for replaying a single draw after a render pass
         // without doing a clear or a DONT_CARE load.