From d61f10d27c9bba4cbfbdc37ebe86110d63fe8ee4 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 15 Sep 2022 14:49:03 +0100 Subject: [PATCH] Insert command buffer references after descriptor references on vulkan * During capture we don't insert references as we're recording descriptor binds, just once at queue submit time, as an optimisation. However this loses ordering relative to more direct references. Since descriptor binds are more conservative (either read, or read before write) we ensure to insert them first before any direct references like copies or fills which could mark the resource as completely written before read. --- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index dbec8ee9e..22bb0313e 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -866,6 +866,7 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, std::set> capDescriptors; std::set> descriptorSets; + std::set cmdsWithReferences; // pull in any copy sources, conservatively if(capframe) @@ -905,12 +906,19 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, it != record->bakedCommands->cmdInfo->sparse.end(); ++it) GetResourceManager()->MarkSparseMapReferenced(*it); - // pull in frame refs from this baked command buffer - record->bakedCommands->AddResourceReferences(GetResourceManager()); + // can't pull in frame refs here, we need to do these last, since they are disjoint from + // descriptor references and we have to apply those first to be conservative. + // For example say a buffer is referenced in a descriptor for read, then completely + // overwritten in a later CmdFillBuffer. The fill will be listed in the normal references + // here, the descriptor reference is lazy tracked and only added below when we handle those. + // Since we don't know the ordering, we do it in the most conservative order - descriptor + // references can never be CompleteWrite, they're either read-only or read-before-write for + // storage descriptors. + // record->bakedCommands->AddResourceReferences(GetResourceManager()); + // GetResourceManager()->MergeReferencedMemory(record->bakedCommands->cmdInfo->memFrameRefs); + cmdsWithReferences.insert(record->bakedCommands); record->bakedCommands->AddReferencedIDs(refdIDs); - GetResourceManager()->MergeReferencedMemory(record->bakedCommands->cmdInfo->memFrameRefs); - // ref the parent command buffer's alloc record, this will pull in the cmd buffer pool GetResourceManager()->MarkResourceFrameReferenced( record->cmdInfo->allocRecord->GetResourceID(), eFrameRef_Read); @@ -920,10 +928,15 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, for(size_t sub = 0; sub < subcmds.size(); sub++) { VkResourceRecord *bakedSubcmds = subcmds[sub]->bakedCommands; - bakedSubcmds->AddResourceReferences(GetResourceManager()); + + // cannot add references here until after we've done descriptor sets later, see comment + // above + // bakedSubcmds->AddResourceReferences(GetResourceManager()); + // GetResourceManager()->MergeReferencedMemory(bakedSubcmds->cmdInfo->memFrameRefs); + cmdsWithReferences.insert(bakedSubcmds); + bakedSubcmds->AddReferencedIDs(refdIDs); UpdateImageStates(bakedSubcmds->cmdInfo->imageStates); - GetResourceManager()->MergeReferencedMemory(bakedSubcmds->cmdInfo->memFrameRefs); GetResourceManager()->MarkResourceFrameReferenced( subcmds[sub]->cmdInfo->allocRecord->GetResourceID(), eFrameRef_Read); @@ -1032,6 +1045,14 @@ void WrappedVulkan::CaptureQueueSubmit(VkQueue queue, } } + // now we can insert frame references from command buffers, to have a conservative ordering vs. + // descriptor references since we don't know which happened first + for(auto it = cmdsWithReferences.begin(); it != cmdsWithReferences.end(); ++it) + { + (*it)->AddResourceReferences(GetResourceManager()); + GetResourceManager()->MergeReferencedMemory((*it)->cmdInfo->memFrameRefs); + } + GetResourceManager()->MarkResourceFrameReferenced(GetResID(queue), eFrameRef_Read); if(fence != VK_NULL_HANDLE)