From 0da2d93304ead1ba09a7dbdbc4fa77dc90933664 Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 23 Oct 2015 18:55:11 +0200 Subject: [PATCH] Clean up instance/device/queue handling and init & shutdown order --- renderdoc/core/resource_manager.h | 12 +- renderdoc/driver/vulkan/vk_core.cpp | 82 ++--- renderdoc/driver/vulkan/vk_core.h | 66 ++-- renderdoc/driver/vulkan/vk_debug.cpp | 12 +- renderdoc/driver/vulkan/vk_manager.cpp | 4 +- renderdoc/driver/vulkan/vk_manager.h | 32 +- renderdoc/driver/vulkan/vk_memory.cpp | 18 +- renderdoc/driver/vulkan/vk_replay.cpp | 4 +- renderdoc/driver/vulkan/vk_resources.h | 3 - .../vulkan/wrappers/vk_device_funcs.cpp | 316 +++++++++--------- .../driver/vulkan/wrappers/vk_misc_funcs.cpp | 93 +++++- .../driver/vulkan/wrappers/vk_queue_funcs.cpp | 44 ++- .../driver/vulkan/wrappers/vk_wsi_funcs.cpp | 25 +- 13 files changed, 405 insertions(+), 306 deletions(-) diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h index 49a368b86..622e4f0c4 100644 --- a/renderdoc/core/resource_manager.h +++ b/renderdoc/core/resource_manager.h @@ -501,16 +501,20 @@ void ResourceManager::Shutdow { auto it = m_LiveResourceMap.begin(); ResourceTypeRelease(it->second); - if(!m_LiveResourceMap.empty()) - m_LiveResourceMap.erase(m_LiveResourceMap.begin()); + + auto removeit = m_LiveResourceMap.find(it->first); + if(removeit != m_LiveResourceMap.end()) + m_LiveResourceMap.erase(removeit); } while(!m_InframeResourceMap.empty()) { auto it = m_InframeResourceMap.begin(); ResourceTypeRelease(it->second); - if(!m_InframeResourceMap.empty()) - m_InframeResourceMap.erase(m_InframeResourceMap.begin()); + + auto removeit = m_InframeResourceMap.find(it->first); + if(removeit != m_InframeResourceMap.end()) + m_InframeResourceMap.erase(removeit); } FreeInitialContents(); diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 81b7009ed..1b7862261 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -263,8 +263,6 @@ WrappedVulkan::WrappedVulkan(const char *logFilename) m_pSerialiser = new Serialiser(NULL, Serialiser::WRITING, debugSerialiser); } - m_SwapPhysDevice = -1; - m_Replay.SetDriver(this); m_FrameCounter = 0; @@ -330,32 +328,20 @@ WrappedVulkan::WrappedVulkan(const char *logFilename) WrappedVulkan::~WrappedVulkan() { - // VKTODOLOW shutdown order is really up in the air - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) + // records must be deleted before resource manager shutdown + if(m_FrameCaptureRecord) { - SAFE_DELETE(m_PhysicalDeviceData[i].debugMan); - - if(m_PhysicalDeviceData[i].cmdpool != VK_NULL_HANDLE) - { - ObjDisp(m_PhysicalDeviceData[i].dev)->DestroyCommandPool(Unwrap(m_PhysicalDeviceData[i].dev), Unwrap(m_PhysicalDeviceData[i].cmdpool)); - GetResourceManager()->ReleaseWrappedResource(m_PhysicalDeviceData[i].cmdpool); - } + RDCASSERT(m_FrameCaptureRecord->GetRefCount() == 1); + m_FrameCaptureRecord->Delete(GetResourceManager()); + m_FrameCaptureRecord = NULL; } + // in case the application leaked some objects, avoid crashing trying + // to release them ourselves by clearing the resource manager. + // In a well-behaved application, this should be a no-op. + m_ResourceManager->ClearWithoutReleasing(); SAFE_DELETE(m_ResourceManager); - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - if(m_PhysicalDeviceData[i].dev != VK_NULL_HANDLE) - vkDestroyDevice(m_PhysicalDeviceData[i].dev); - - // VKTODOLOW only one instance - if(m_PhysicalDeviceData[0].inst != VK_NULL_HANDLE) - { - VkInstance instance = Unwrap(m_PhysicalDeviceData[0].inst); - - ObjDisp(m_PhysicalDeviceData[0].inst)->DestroyInstance(instance); - } - SAFE_DELETE(m_pSerialiser); for(size_t i=0; i < m_ThreadSerialisers.size(); i++) @@ -370,64 +356,55 @@ WrappedVulkan::~WrappedVulkan() VkCmdBuffer WrappedVulkan::GetNextCmd() { - RDCASSERT(m_SwapPhysDevice >= 0); - PhysicalDeviceData &rd = m_PhysicalDeviceData[m_SwapPhysDevice]; - VkCmdBuffer ret; - if(!rd.freecmds.empty()) + if(!m_InternalCmds.freecmds.empty()) { - ret = rd.freecmds.back(); - rd.freecmds.pop_back(); + ret = m_InternalCmds.freecmds.back(); + m_InternalCmds.freecmds.pop_back(); ObjDisp(ret)->ResetCommandBuffer(Unwrap(ret), 0); } else { - VkCmdBufferCreateInfo cmdInfo = { VK_STRUCTURE_TYPE_CMD_BUFFER_CREATE_INFO, NULL, Unwrap(rd.cmdpool), VK_CMD_BUFFER_LEVEL_PRIMARY, 0 }; - VkResult vkr = ObjDisp(rd.dev)->CreateCommandBuffer(Unwrap(rd.dev), &cmdInfo, &ret); + VkCmdBufferCreateInfo cmdInfo = { VK_STRUCTURE_TYPE_CMD_BUFFER_CREATE_INFO, NULL, Unwrap(m_InternalCmds.m_CmdPool), VK_CMD_BUFFER_LEVEL_PRIMARY, 0 }; + VkResult vkr = ObjDisp(m_Device)->CreateCommandBuffer(Unwrap(m_Device), &cmdInfo, &ret); RDCASSERT(vkr == VK_SUCCESS); - GetResourceManager()->WrapResource(Unwrap(rd.dev), ret); + GetResourceManager()->WrapResource(Unwrap(m_Device), ret); } - rd.pendingcmds.push_back(ret); + m_InternalCmds.pendingcmds.push_back(ret); return ret; } void WrappedVulkan::SubmitCmds() { - RDCASSERT(m_SwapPhysDevice >= 0); - PhysicalDeviceData &rd = m_PhysicalDeviceData[m_SwapPhysDevice]; - // nothing to do - if(rd.pendingcmds.empty()) + if(m_InternalCmds.pendingcmds.empty()) return; - vector cmds = rd.pendingcmds; + vector cmds = m_InternalCmds.pendingcmds; for(size_t i=0; i < cmds.size(); i++) cmds[i] = Unwrap(cmds[i]); - ObjDisp(rd.q)->QueueSubmit(Unwrap(rd.q), (uint32_t)cmds.size(), &cmds[0], VK_NULL_HANDLE); + ObjDisp(m_Queue)->QueueSubmit(Unwrap(m_Queue), (uint32_t)cmds.size(), &cmds[0], VK_NULL_HANDLE); - rd.submittedcmds.insert(rd.submittedcmds.end(), rd.pendingcmds.begin(), rd.pendingcmds.end()); - rd.pendingcmds.clear(); + m_InternalCmds.submittedcmds.insert(m_InternalCmds.submittedcmds.end(), m_InternalCmds.pendingcmds.begin(), m_InternalCmds.pendingcmds.end()); + m_InternalCmds.pendingcmds.clear(); } void WrappedVulkan::FlushQ() { - RDCASSERT(m_SwapPhysDevice >= 0); - PhysicalDeviceData &rd = m_PhysicalDeviceData[m_SwapPhysDevice]; - // VKTODOLOW could do away with the need for this function by keeping // commands until N presents later, or something, or checking on fences - ObjDisp(rd.q)->QueueWaitIdle(Unwrap(rd.q)); + ObjDisp(m_Queue)->QueueWaitIdle(Unwrap(m_Queue)); - if(!rd.submittedcmds.empty()) + if(!m_InternalCmds.submittedcmds.empty()) { - rd.freecmds.insert(rd.freecmds.end(), rd.submittedcmds.begin(), rd.submittedcmds.end()); - rd.submittedcmds.clear(); + m_InternalCmds.freecmds.insert(m_InternalCmds.freecmds.end(), m_InternalCmds.submittedcmds.begin(), m_InternalCmds.submittedcmds.end()); + m_InternalCmds.submittedcmds.clear(); } } @@ -746,13 +723,8 @@ void WrappedVulkan::ReadLogInitialisation() m_pSerialiser->SetDebugText(false); - RDCASSERT(m_SwapPhysDevice >= 0 && - m_PhysicalDeviceData[m_SwapPhysDevice].dev != VK_NULL_HANDLE && - m_PhysicalDeviceData[m_SwapPhysDevice].q != VK_NULL_HANDLE && - m_PhysicalDeviceData[m_SwapPhysDevice].cmdpool != VK_NULL_HANDLE); - - // VKTODOLOW maybe better place to put this? - m_PhysicalDeviceData[m_SwapPhysDevice].debugMan = new VulkanDebugManager(this, GetDev()); + // ensure the capture at least created a device and fetched a queue. + RDCASSERT(m_Device != VK_NULL_HANDLE && m_Queue != VK_NULL_HANDLE && m_InternalCmds.m_CmdPool != VK_NULL_HANDLE); } void WrappedVulkan::ContextReplayLog(LogState readType, uint32_t startEventID, uint32_t endEventID, bool partial) diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 6ea80b684..28fa30961 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -128,8 +128,6 @@ private: VulkanReplay m_Replay; VkInitParams m_InitParams; - - VkResourceRecord *m_InstanceRecord; VkResourceRecord *m_FrameCaptureRecord; Chunk *m_HeaderChunk; @@ -144,6 +142,7 @@ private: vector m_CmdBufferRecords; VulkanResourceManager *m_ResourceManager; + VulkanDebugManager *m_DebugManager; Threading::CriticalSection m_CapTransitionLock; @@ -158,27 +157,12 @@ private: struct PhysicalDeviceData { PhysicalDeviceData() - : inst(VK_NULL_HANDLE), phys(VK_NULL_HANDLE) - , qFamilyIdx(0), dev(VK_NULL_HANDLE), q(VK_NULL_HANDLE) - , cmdpool(VK_NULL_HANDLE), debugMan(NULL) {} - - VkInstance inst; - VkPhysicalDevice phys; - VkDevice dev; - uint32_t qFamilyIdx; - VkQueue q; + : readbackMemIndex(0), uploadMemIndex(0), GPULocalMemIndex(0) + { + RDCEraseEl(features); + RDCEraseEl(memProps); + } - vector freecmds; - // -> record -> - vector pendingcmds; - // -> submit -> - vector submittedcmds; - // -> flush/waitidle -> freecmds - - VkCmdPool cmdpool; - - VulkanDebugManager *debugMan; - uint32_t GetMemoryIndex(uint32_t resourceRequiredBitmask, uint32_t allocRequiredProps, uint32_t allocUndesiredProps); // store the three most common memory indices: @@ -192,26 +176,45 @@ private: VkPhysicalDeviceFeatures features; VkPhysicalDeviceMemoryProperties memProps; }; - vector m_PhysicalDeviceData; - int m_SwapPhysDevice; VkInstance m_Instance; // the instance corresponding to this WrappedVulkan - vector m_Devices; // the devices created under this instance VkDbgMsgCallback m_DbgMsgCallback; // the instance's dbg msg callback handle + VkDevice m_Device; // the device used for our own command buffer work + PhysicalDeviceData m_PhysicalDeviceData; // the data about the physical device used for the above device; + uint32_t m_QueueFamilyIdx; // the family index that we've selected in CreateDevice for our queue + VkQueue m_Queue; // the queue used for our own command buffer work + + struct + { + void Reset() + { + m_CmdPool = VK_NULL_HANDLE; + freecmds.clear(); + pendingcmds.clear(); + submittedcmds.clear(); + } + + VkCmdPool m_CmdPool; // the command pool used for allocating our own command buffers + + vector freecmds; + // -> record -> + vector pendingcmds; + // -> submit -> + vector submittedcmds; + // -> flush/waitidle -> freecmds + } m_InternalCmds; vector m_FreeMems; - - VulkanDebugManager *GetDebugManager() - { RDCASSERT(m_SwapPhysDevice >= 0); return m_PhysicalDeviceData[m_SwapPhysDevice].debugMan; } - VkDevice GetDev() { RDCASSERT(m_SwapPhysDevice >= 0); return m_PhysicalDeviceData[m_SwapPhysDevice].dev; } - VkQueue GetQ() { RDCASSERT(m_SwapPhysDevice >= 0); return m_PhysicalDeviceData[m_SwapPhysDevice].q; } + // return the pre-selected device and queue + VkDevice GetDev() { RDCASSERT(m_Device != VK_NULL_HANDLE); return m_Device; } + VkQueue GetQ() { RDCASSERT(m_Device != VK_NULL_HANDLE); return m_Queue; } VkCmdBuffer GetNextCmd(); void SubmitCmds(); void FlushQ(); const VkPhysicalDeviceFeatures &GetDeviceFeatures() - { RDCASSERT(m_SwapPhysDevice >= 0); return m_PhysicalDeviceData[m_SwapPhysDevice].features; } + { return m_PhysicalDeviceData.features; } uint32_t GetReadbackMemoryIndex(uint32_t resourceRequiredBitmask); uint32_t GetUploadMemoryIndex(uint32_t resourceRequiredBitmask); @@ -495,6 +498,7 @@ public: ResourceId GetContextResourceID() { return m_FrameCaptureRecord->GetResourceID(); } VulkanResourceManager *GetResourceManager() { return m_ResourceManager; } + VulkanDebugManager *GetDebugManager() { return m_DebugManager; } VulkanReplay *GetReplay() { return &m_Replay; } diff --git a/renderdoc/driver/vulkan/vk_debug.cpp b/renderdoc/driver/vulkan/vk_debug.cpp index f06043a55..5be7e0caa 100644 --- a/renderdoc/driver/vulkan/vk_debug.cpp +++ b/renderdoc/driver/vulkan/vk_debug.cpp @@ -1026,7 +1026,6 @@ VulkanDebugManager::VulkanDebugManager(WrappedVulkan *driver, VkDevice dev) vt->UpdateDescriptorSets(Unwrap(dev), ARRAY_COUNT(writeSet), writeSet, 0, NULL); vt->EndCommandBuffer(Unwrap(cmd)); - driver->SubmitCmds(); } VulkanDebugManager::~VulkanDebugManager() @@ -1036,6 +1035,17 @@ VulkanDebugManager::~VulkanDebugManager() VkResult vkr = VK_SUCCESS; + // since we don't have properly registered resources, releasing our descriptor + // pool here won't remove the descriptor sets, so we need to free our own + // tracking data (not the API objects) for descriptor sets. + + GetResourceManager()->ReleaseWrappedResource(m_CheckerboardDescSet); + GetResourceManager()->ReleaseWrappedResource(m_GenericDescSet); + GetResourceManager()->ReleaseWrappedResource(m_TextDescSet); + + for(size_t i=0; i < ARRAY_COUNT(m_TexDisplayDescSet); i++) + GetResourceManager()->ReleaseWrappedResource(m_TexDisplayDescSet[i]); + if(m_DescriptorPool != VK_NULL_HANDLE) { vt->DestroyDescriptorPool(Unwrap(dev), Unwrap(m_DescriptorPool)); diff --git a/renderdoc/driver/vulkan/vk_manager.cpp b/renderdoc/driver/vulkan/vk_manager.cpp index 911eb9937..625550300 100644 --- a/renderdoc/driver/vulkan/vk_manager.cpp +++ b/renderdoc/driver/vulkan/vk_manager.cpp @@ -519,7 +519,5 @@ void VulkanResourceManager::Apply_InitialState(WrappedVkRes *live, InitialConten bool VulkanResourceManager::ResourceTypeRelease(WrappedVkRes *res) { - bool ret = m_Core->ReleaseResource(res); - ReleaseWrappedResource(res); - return ret; + return m_Core->ReleaseResource(res); } diff --git a/renderdoc/driver/vulkan/vk_manager.h b/renderdoc/driver/vulkan/vk_manager.h index fe0b9c49b..aa435e4de 100644 --- a/renderdoc/driver/vulkan/vk_manager.h +++ b/renderdoc/driver/vulkan/vk_manager.h @@ -42,6 +42,26 @@ class VulkanResourceManager : public ResourceManager void AddLiveResource(ResourceId id, realtype obj) @@ -166,7 +186,17 @@ class VulkanResourceManager : public ResourceManagerpool so we don't recurse (*it)->pool = NULL; - ReleaseWrappedResource((VkDescriptorSet)(uint64_t)(*it)->Resource, true); + VkResourceType restype = IdentifyTypeByPtr((*it)->Resource); + if(restype == eResDescriptorSet) + ReleaseWrappedResource((VkDescriptorSet)(uint64_t)(*it)->Resource, true); + else if(restype == eResCmdBuffer) + ReleaseWrappedResource((VkCmdBuffer)(*it)->Resource, true); + else if(restype == eResQueue) + ReleaseWrappedResource((VkQueue)(*it)->Resource, true); + else if(restype == eResPhysicalDevice) + ReleaseWrappedResource((VkPhysicalDevice)(*it)->Resource, true); + else + RDCERR("Unexpected resource type %d as pooled child!", restype); } record->pooledChildren.clear(); } diff --git a/renderdoc/driver/vulkan/vk_memory.cpp b/renderdoc/driver/vulkan/vk_memory.cpp index 14d06a91d..350745579 100644 --- a/renderdoc/driver/vulkan/vk_memory.cpp +++ b/renderdoc/driver/vulkan/vk_memory.cpp @@ -26,30 +26,30 @@ uint32_t WrappedVulkan::GetReadbackMemoryIndex(uint32_t resourceRequiredBitmask) { - if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData[m_SwapPhysDevice].readbackMemIndex)) - return m_PhysicalDeviceData[m_SwapPhysDevice].readbackMemIndex; + if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData.readbackMemIndex)) + return m_PhysicalDeviceData.readbackMemIndex; - return m_PhysicalDeviceData[m_SwapPhysDevice].GetMemoryIndex( + return m_PhysicalDeviceData.GetMemoryIndex( resourceRequiredBitmask, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, VK_MEMORY_PROPERTY_HOST_WRITE_COMBINED_BIT); } uint32_t WrappedVulkan::GetUploadMemoryIndex(uint32_t resourceRequiredBitmask) { - if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData[m_SwapPhysDevice].uploadMemIndex)) - return m_PhysicalDeviceData[m_SwapPhysDevice].uploadMemIndex; + if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData.uploadMemIndex)) + return m_PhysicalDeviceData.uploadMemIndex; - return m_PhysicalDeviceData[m_SwapPhysDevice].GetMemoryIndex( + return m_PhysicalDeviceData.GetMemoryIndex( resourceRequiredBitmask, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, 0); } uint32_t WrappedVulkan::GetGPULocalMemoryIndex(uint32_t resourceRequiredBitmask) { - if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData[m_SwapPhysDevice].GPULocalMemIndex)) - return m_PhysicalDeviceData[m_SwapPhysDevice].GPULocalMemIndex; + if(resourceRequiredBitmask & (1 << m_PhysicalDeviceData.GPULocalMemIndex)) + return m_PhysicalDeviceData.GPULocalMemIndex; - return m_PhysicalDeviceData[m_SwapPhysDevice].GetMemoryIndex( + return m_PhysicalDeviceData.GetMemoryIndex( resourceRequiredBitmask, VK_MEMORY_PROPERTY_DEVICE_ONLY, 0); } diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 2813c5708..959cd9183 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -702,7 +702,9 @@ bool VulkanReplay::RenderTextureInternal(TextureDisplay cfg, VkRenderPassBeginIn VkResult vkr = ObjDisp(dev)->CreateImageView(Unwrap(dev), &viewInfo, &iminfo.view); RDCASSERT(vkr == VK_SUCCESS); - m_pDriver->GetResourceManager()->WrapResource(Unwrap(dev), iminfo.view); + ResourceId viewid = m_pDriver->GetResourceManager()->WrapResource(Unwrap(dev), iminfo.view); + // register as a live-only resource, so it is cleaned up properly + m_pDriver->GetResourceManager()->AddLiveResource(viewid, iminfo.view); liveImView = iminfo.view; } diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 0de1aba43..71ed87925 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -671,9 +671,6 @@ struct VkResourceRecord : public ResourceRecord SwapchainInfo *swapInfo; CmdBufferRecordingInfo *cmdInfo; - // queues associated with this instance, so they can be shut down on destruction - vector queues; - // pointer to either the pool this item is allocated from, or the children allocated // from this pool. Protected by the chunk lock VkResourceRecord *pool; diff --git a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp index d42ff2a95..b114d6676 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp @@ -33,6 +33,11 @@ // The class constructor and destructor handle only *non-API* work. All API objects must be created and // torn down in the latter functions (vkCreateInstance/vkDestroyInstance during capture, and // Initialise/Shutdown during replay). +// +// Note that during capture we have vkDestroyDevice before vkDestroyDevice that does most of the work. +// +// Also we assume correctness from the application, that all objects are destroyed before the device and +// instance are destroyed. We only clean up after our own objects. void WrappedVulkan::Initialise(VkInitParams ¶ms) { @@ -79,6 +84,10 @@ void WrappedVulkan::Initialise(VkInitParams ¶ms) GetResourceManager()->AddLiveResource(params.InstanceID, m_Instance); m_DbgMsgCallback = VK_NULL_HANDLE; + m_Device = VK_NULL_HANDLE; + m_QueueFamilyIdx = ~0U; + m_Queue = VK_NULL_HANDLE; + m_InternalCmds.Reset(); if(ObjDisp(m_Instance)->DbgCreateMsgCallback) { @@ -108,92 +117,90 @@ VkResult WrappedVulkan::vkCreateInstance( if(ret != VK_SUCCESS) return ret; - - if(m_State >= WRITING) - { - m_InitParams.Set(pCreateInfo, GetResID(m_Instance)); - m_InstanceRecord = GetResourceManager()->AddResourceRecord(m_Instance); - } + + // should only be called during capture + RDCASSERT(m_State >= WRITING); + + m_InitParams.Set(pCreateInfo, GetResID(m_Instance)); + GetResourceManager()->AddResourceRecord(m_Instance); *pInstance = m_Instance; + + m_DbgMsgCallback = VK_NULL_HANDLE; + m_Device = VK_NULL_HANDLE; + m_QueueFamilyIdx = ~0U; + m_Queue = VK_NULL_HANDLE; + m_InternalCmds.Reset(); return VK_SUCCESS; } void WrappedVulkan::Shutdown() { + // flush out any pending commands + SubmitCmds(); + FlushQ(); + + // since we didn't create proper registered resources for our command buffers, + // they won't be taken down properly with the pool. So we release them (just our + // data) here. + for(size_t i=0; i < m_InternalCmds.freecmds.size(); i++) + GetResourceManager()->ReleaseWrappedResource(m_InternalCmds.freecmds[i]); + + // destroy the pool + ObjDisp(m_Device)->DestroyCommandPool(Unwrap(m_Device), Unwrap(m_InternalCmds.m_CmdPool)); + GetResourceManager()->ReleaseWrappedResource(m_InternalCmds.m_CmdPool); + + // we do more in Shutdown than the equivalent vkDestroyInstance since on replay there's + // no explicit vkDestroyDevice, we destroy the device here then the instance + // destroy any replay objects that aren't specifically to do with the frame capture - VkDevice dev = GetDev(); for(size_t i=0; i < m_FreeMems.size(); i++) { - ObjDisp(dev)->FreeMemory(Unwrap(dev), Unwrap(m_FreeMems[i])); + ObjDisp(m_Device)->FreeMemory(Unwrap(m_Device), Unwrap(m_FreeMems[i])); GetResourceManager()->ReleaseWrappedResource(m_FreeMems[i]); } m_FreeMems.clear(); - + + // destroy debug manager and any objects it created + SAFE_DELETE(m_DebugManager); + if(ObjDisp(m_Instance)->DbgDestroyMsgCallback && m_DbgMsgCallback != VK_NULL_HANDLE) ObjDisp(m_Instance)->DbgDestroyMsgCallback(Unwrap(m_Instance), m_DbgMsgCallback); - // need to store the unwrapped devices and instance to destroy the + // need to store the unwrapped device and instance to destroy the // API object after resource manager shutdown - vector devs; VkInstance inst = Unwrap(m_Instance); + VkDevice dev = Unwrap(m_Device); - for(size_t i=0; i < m_Devices.size(); i++) - devs.push_back(Unwrap(m_Devices[i])); - - m_Devices.clear(); - m_Instance = VK_NULL_HANDLE; + const VkLayerDispatchTable *vt = ObjDisp(m_Device); + const VkLayerInstanceDispatchTable *vit = ObjDisp(m_Instance); // this destroys the wrapped objects for the devices and instances m_ResourceManager->Shutdown(); - // second last step, destroy devices - for(size_t i=0; i < devs.size(); i++) - { - VkDevice dev = devs[i]; - ObjDisp(dev)->DestroyDevice(Unwrap(dev)); - } + delete GetWrapped(m_Device); + delete GetWrapped(m_Instance); - // finally destroy instance - ObjDisp(inst)->DestroyInstance(Unwrap(inst)); + m_Device = VK_NULL_HANDLE; + m_Instance = VK_NULL_HANDLE; + + // finally destroy device then instance + vt->DestroyDevice(dev); + vit->DestroyInstance(inst); } void WrappedVulkan::vkDestroyInstance(VkInstance instance) { RDCASSERT(m_Instance == instance); - // records must be deleted before resource manager shutdown - if(m_FrameCaptureRecord) - { - RDCASSERT(m_FrameCaptureRecord->GetRefCount() == 1); - m_FrameCaptureRecord->Delete(GetResourceManager()); - m_FrameCaptureRecord = NULL; - } + // the device should already have been destroyed, assuming that the + // application is well behaved. If not, we just leak. - // need to store the unwrapped devices and instance to destroy the - // API object after resource manager shutdown - vector devs; - VkInstance inst = Unwrap(m_Instance); - - for(size_t i=0; i < m_Devices.size(); i++) - devs.push_back(Unwrap(m_Devices[i])); + ObjDisp(m_Instance)->DestroyInstance(Unwrap(m_Instance)); + GetResourceManager()->ReleaseWrappedResource(m_Instance); - m_Devices.clear(); m_Instance = VK_NULL_HANDLE; - - // this destroys the wrapped objects for the devices and instances - m_ResourceManager->Shutdown(); - - // second last step, destroy devices - for(size_t i=0; i < devs.size(); i++) - { - VkDevice dev = devs[i]; - ObjDisp(dev)->DestroyDevice(Unwrap(dev)); - } - - // finally destroy instance - ObjDisp(inst)->DestroyInstance(Unwrap(inst)); } bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices( @@ -237,20 +244,6 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices( GetResourceManager()->AddLiveResource(physId, pd); } - PhysicalDeviceData data; - data.inst = instance; - data.phys = pd; - - ObjDisp(pd)->GetPhysicalDeviceMemoryProperties(Unwrap(pd), &data.memProps); - - ObjDisp(pd)->GetPhysicalDeviceFeatures(Unwrap(pd), &data.features); - - data.readbackMemIndex = data.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, VK_MEMORY_PROPERTY_HOST_WRITE_COMBINED_BIT); - data.uploadMemIndex = data.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, 0); - data.GPULocalMemIndex = data.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_DEVICE_ONLY, 0); - - m_PhysicalDeviceData.push_back(data); - return true; } @@ -291,7 +284,21 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices( SCOPED_SERIALISE_CONTEXT(ENUM_PHYSICALS); Serialise_vkEnumeratePhysicalDevices(localSerialiser, instance, &i, &devices[i]); - m_InstanceRecord->AddChunk(scope.Get()); + VkResourceRecord *record = GetResourceManager()->AddResourceRecord(devices[i]); + RDCASSERT(record); + + record->AddChunk(scope.Get()); + + VkResourceRecord *instrecord = GetRecord(instance); + + instrecord->AddParent(record); + + // treat physical devices as pool members of the instance (ie. freed when the instance dies) + { + instrecord->LockChunks(); + instrecord->pooledChildren.push_back(record); + instrecord->UnlockChunks(); + } } } } @@ -395,15 +402,7 @@ bool WrappedVulkan::Serialise_vkCreateDevice( createInfo.pEnabledFeatures = &enabledFeatures; VkPhysicalDeviceFeatures availFeatures = {0}; - - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - { - if(m_PhysicalDeviceData[i].phys == physicalDevice) - { - availFeatures = m_PhysicalDeviceData[i].features; - break; - } - } + ObjDisp(physicalDevice)->GetPhysicalDeviceFeatures(Unwrap(physicalDevice), &availFeatures); if(availFeatures.fillModeNonSolid) enabledFeatures.fillModeNonSolid = true; @@ -421,41 +420,35 @@ bool WrappedVulkan::Serialise_vkCreateDevice( GetResourceManager()->WrapResource(device, device); GetResourceManager()->AddLiveResource(devId, device); + + InitDeviceReplayTables(Unwrap(device)); - found = false; + RDCASSERT(m_Device == VK_NULL_HANDLE); // VKTODOLOW multiple devices are not supported - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) + m_Device = device; + + m_QueueFamilyIdx = qFamilyIdx; + + if(m_InternalCmds.m_CmdPool == VK_NULL_HANDLE) { - if(m_PhysicalDeviceData[i].phys == physicalDevice) - { - InitDeviceReplayTables(Unwrap(device)); + VkCmdPoolCreateInfo poolInfo = { VK_STRUCTURE_TYPE_CMD_POOL_CREATE_INFO, NULL, qFamilyIdx, VK_CMD_POOL_CREATE_RESET_COMMAND_BUFFER_BIT }; + vkr = ObjDisp(device)->CreateCommandPool(Unwrap(device), &poolInfo, &m_InternalCmds.m_CmdPool); + RDCASSERT(vkr == VK_SUCCESS); - // VKTODOLOW not handling multiple devices per physical devices - RDCASSERT(m_PhysicalDeviceData[i].dev == VK_NULL_HANDLE); - m_PhysicalDeviceData[i].dev = device; - - m_PhysicalDeviceData[i].qFamilyIdx = qFamilyIdx; - - // VKTODOMED this is a hack - need a more reliable way of doing this - // when we serialise the relevant vkGetDeviceQueue, we'll search for this handle and replace it with the wrapped version - VkResult vkr = ObjDisp(device)->GetDeviceQueue(Unwrap(device), qFamilyIdx, 0, &m_PhysicalDeviceData[i].q); - RDCASSERT(vkr == VK_SUCCESS); - - VkCmdPoolCreateInfo poolInfo = { VK_STRUCTURE_TYPE_CMD_POOL_CREATE_INFO, NULL, qFamilyIdx, VK_CMD_POOL_CREATE_RESET_COMMAND_BUFFER_BIT }; - vkr = ObjDisp(device)->CreateCommandPool(Unwrap(device), &poolInfo, &m_PhysicalDeviceData[i].cmdpool); - RDCASSERT(vkr == VK_SUCCESS); - - GetResourceManager()->WrapResource(Unwrap(device), m_PhysicalDeviceData[i].cmdpool); - - found = true; - break; - } + GetResourceManager()->WrapResource(Unwrap(device), m_InternalCmds.m_CmdPool); } - SAFE_DELETE_ARRAY(modQueues); + ObjDisp(physicalDevice)->GetPhysicalDeviceMemoryProperties(Unwrap(physicalDevice), &m_PhysicalDeviceData.memProps); - if(!found) - RDCERR("Couldn't find VkPhysicalDevice for vkCreateDevice!"); + ObjDisp(physicalDevice)->GetPhysicalDeviceFeatures(Unwrap(physicalDevice), &m_PhysicalDeviceData.features); + + m_PhysicalDeviceData.readbackMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, VK_MEMORY_PROPERTY_HOST_WRITE_COMBINED_BIT); + m_PhysicalDeviceData.uploadMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, 0); + m_PhysicalDeviceData.GPULocalMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_DEVICE_ONLY, 0); + + m_DebugManager = new VulkanDebugManager(this, device); + + SAFE_DELETE_ARRAY(modQueues); } return true; @@ -555,48 +548,39 @@ VkResult WrappedVulkan::vkCreateDevice( chunk = scope.Get(); } - m_InstanceRecord->AddChunk(chunk); + GetRecord(m_Instance)->AddChunk(chunk); } else { GetResourceManager()->AddLiveResource(id, *pDevice); } - - found = false; - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) + VkDevice device = *pDevice; + + RDCASSERT(m_Device == VK_NULL_HANDLE); // VKTODOLOW multiple devices are not supported + + m_Device = device; + + m_QueueFamilyIdx = qFamilyIdx; + + if(m_InternalCmds.m_CmdPool == VK_NULL_HANDLE) { - if(m_PhysicalDeviceData[i].phys == physicalDevice) - { - m_PhysicalDeviceData[i].dev = *pDevice; + VkCmdPoolCreateInfo poolInfo = { VK_STRUCTURE_TYPE_CMD_POOL_CREATE_INFO, NULL, qFamilyIdx, VK_CMD_POOL_CREATE_RESET_COMMAND_BUFFER_BIT }; + vkr = ObjDisp(device)->CreateCommandPool(Unwrap(device), &poolInfo, &m_InternalCmds.m_CmdPool); + RDCASSERT(vkr == VK_SUCCESS); - m_PhysicalDeviceData[i].qFamilyIdx = qFamilyIdx; - - // we call our own vkGetDeviceQueue so that its initialisation is properly serialised in case when - // the application fetches this queue it gets the same handle - the already wrapped one created - // here will be returned. - vkr = vkGetDeviceQueue(*pDevice, qFamilyIdx, 0, &m_PhysicalDeviceData[i].q); - RDCASSERT(vkr == VK_SUCCESS); - - VkCmdPoolCreateInfo poolInfo = { VK_STRUCTURE_TYPE_CMD_POOL_CREATE_INFO, NULL, qFamilyIdx, VK_CMD_POOL_CREATE_RESET_COMMAND_BUFFER_BIT }; - vkr = ObjDisp(*pDevice)->CreateCommandPool(Unwrap(*pDevice), &poolInfo, &m_PhysicalDeviceData[i].cmdpool); - RDCASSERT(vkr == VK_SUCCESS); - - GetResourceManager()->WrapResource(Unwrap(*pDevice), m_PhysicalDeviceData[i].cmdpool); - - // VKTODOHIGH hack, need to properly handle multiple devices etc and - // not have this 'current swap chain device' thing. - m_SwapPhysDevice = (int)i; - - m_PhysicalDeviceData[i].debugMan = new VulkanDebugManager(this, *pDevice); - - found = true; - break; - } + GetResourceManager()->WrapResource(Unwrap(device), m_InternalCmds.m_CmdPool); } + + ObjDisp(physicalDevice)->GetPhysicalDeviceMemoryProperties(Unwrap(physicalDevice), &m_PhysicalDeviceData.memProps); - if(!found) - RDCERR("Couldn't find VkPhysicalDevice for vkCreateDevice!"); + ObjDisp(physicalDevice)->GetPhysicalDeviceFeatures(Unwrap(physicalDevice), &m_PhysicalDeviceData.features); + + m_PhysicalDeviceData.readbackMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, VK_MEMORY_PROPERTY_HOST_WRITE_COMBINED_BIT); + m_PhysicalDeviceData.uploadMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, 0); + m_PhysicalDeviceData.GPULocalMemIndex = m_PhysicalDeviceData.GetMemoryIndex(~0U, VK_MEMORY_PROPERTY_DEVICE_ONLY, 0); + + m_DebugManager = new VulkanDebugManager(this, device); } SAFE_DELETE_ARRAY(modQueues); @@ -606,32 +590,46 @@ VkResult WrappedVulkan::vkCreateDevice( void WrappedVulkan::vkDestroyDevice(VkDevice device) { - // VKTODOHIGH this stuff should all be in vkDestroyInstance - if(m_State >= WRITING) + // flush out any pending commands + SubmitCmds(); + FlushQ(); + + // VKTODOLOW handle multiple devices - this function will need to check + // if the device is the one we used for debugmanager/cmd pool etc, and + // only remove child queues and resources (instead of doing full resource + // manager shutdown) + RDCASSERT(m_Device == device); + + // delete all debug manager objects + SAFE_DELETE(m_DebugManager); + + // since we didn't create proper registered resources for our command buffers, + // they won't be taken down properly with the pool. So we release them (just our + // data) here. + for(size_t i=0; i < m_InternalCmds.freecmds.size(); i++) + GetResourceManager()->ReleaseWrappedResource(m_InternalCmds.freecmds[i]); + + // destroy our command pool + if(m_InternalCmds.m_CmdPool != VK_NULL_HANDLE) { - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - { - if(m_PhysicalDeviceData[i].dev == device) - { - if(m_PhysicalDeviceData[i].cmdpool != VK_NULL_HANDLE) - ObjDisp(device)->DestroyCommandPool(Unwrap(device), Unwrap(m_PhysicalDeviceData[i].cmdpool)); - - // VKTODOHIGH this data is needed in destructor for swapchains - order of shutdown needs to be revamped - break; - } - } - - vector &queues = m_InstanceRecord->queues; - for(size_t i=0; i < queues.size(); i++) - GetResourceManager()->ReleaseWrappedResource(queues[i]); + ObjDisp(m_Device)->DestroyCommandPool(Unwrap(m_Device), Unwrap(m_InternalCmds.m_CmdPool)); + GetResourceManager()->ReleaseWrappedResource(m_InternalCmds.m_CmdPool); } - ObjDisp(device)->DestroyDevice(Unwrap(device)); - - // VKTODOLOW on replay we're releasing this after resource manager - // shutdown. Yes it's a hack, yes it's ugly. - if(m_State >= WRITING) - GetResourceManager()->ReleaseWrappedResource(device); + m_InternalCmds.Reset(); + + m_QueueFamilyIdx = ~0U; + m_Queue = VK_NULL_HANDLE; + + // destroy the API device immediately. There should be no more + // resources left in the resource manager device/physical device/instance. + // Anything we created should be gone and anything the application created + // should be deleted by now. + // If there were any leaks, we will leak them ourselves in vkDestroyInstance + // rather than try to delete API objects after the device has gone + ObjDisp(m_Device)->DestroyDevice(Unwrap(m_Device)); + GetResourceManager()->ReleaseWrappedResource(m_Device); + m_Device = VK_NULL_HANDLE; } bool WrappedVulkan::Serialise_vkDeviceWaitIdle(Serialiser* localSerialiser, VkDevice device) diff --git a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp index f10290edb..b06aa80fa 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp @@ -60,6 +60,26 @@ DESTROY_IMPL(VkRenderPass, DestroyRenderPass) // needs to be separate because it returns VkResult still VkResult WrappedVulkan::vkDestroySwapchainKHR(VkDevice device, VkSwapchainKHR obj) { + // release internal rendering objects we created for rendering the overlay + { + SwapchainInfo &info = *GetRecord(obj)->swapInfo; + VkRenderPass unwrappedRP = Unwrap(info.rp); + GetResourceManager()->ReleaseWrappedResource(info.rp, true); + ObjDisp(device)->DestroyRenderPass(Unwrap(device), unwrappedRP); + + for(size_t i=0; i < info.images.size(); i++) + { + VkFramebuffer unwrappedFB = Unwrap(info.images[i].fb); + VkImageView unwrappedView = Unwrap(info.images[i].view); + GetResourceManager()->ReleaseWrappedResource(info.images[i].fb, true); + // note, image doesn't have to be destroyed, just untracked + GetResourceManager()->ReleaseWrappedResource(info.images[i].im, true); + GetResourceManager()->ReleaseWrappedResource(info.images[i].view, true); + ObjDisp(device)->DestroyFramebuffer(Unwrap(device), unwrappedFB); + ObjDisp(device)->DestroyImageView(Unwrap(device), unwrappedView); + } + } + VkSwapchainKHR unwrappedObj = Unwrap(obj); if(GetResourceManager()->HasWrapper(ToTypedHandle(unwrappedObj))) GetResourceManager()->ReleaseWrappedResource(obj, true); return ObjDisp(device)->DestroySwapchainKHR(Unwrap(device), unwrappedObj); @@ -106,8 +126,9 @@ bool WrappedVulkan::ReleaseResource(WrappedVkRes *res) VkDevice dev = GetDev(); const VkLayerDispatchTable *vt = ObjDisp(dev); - WrappedVkDispRes *disp = (WrappedVkDispRes *)res; WrappedVkNonDispRes *nondisp = (WrappedVkNonDispRes *)res; + WrappedVkDispRes *disp = (WrappedVkDispRes *)res; + uint64_t handle = (uint64_t)nondisp; switch(IdentifyTypeByPtr(res)) { @@ -121,79 +142,125 @@ bool WrappedVulkan::ReleaseResource(WrappedVkRes *res) case eResUnknown: RDCERR("Unknown resource type!"); break; - - case eResPhysicalDevice: - case eResQueue: - case eResDescriptorSet: + case eResCmdBuffer: - // nothing to do - destroyed with parent object + // special case here, on replay we don't have the tracking + // to remove these with the parent object so do it here. + // This ensures we clean up after ourselves with a well- + // behaved application. + if(m_State < WRITING) + GetResourceManager()->ReleaseWrappedResource((VkCmdBuffer)res); + break; + case eResDescriptorSet: + if(m_State < WRITING) + GetResourceManager()->ReleaseWrappedResource(VkDescriptorSet(handle)); + break; + case eResPhysicalDevice: + if(m_State < WRITING) + GetResourceManager()->ReleaseWrappedResource((VkPhysicalDevice)disp); + break; + case eResQueue: + if(m_State < WRITING) + GetResourceManager()->ReleaseWrappedResource((VkQueue)disp); break; - case eResInstance: case eResDevice: // these are explicitly released elsewhere, do not need to destroy - // any API objects + // any API objects. + // On replay though we do need to tidy up book-keeping for these. + if(m_State < WRITING) + { + GetResourceManager()->ReleaseCurrentResource(disp->id); + GetResourceManager()->RemoveWrapper(ToTypedHandle(disp->real.As())); + } + break; + case eResInstance: + if(m_State < WRITING) + { + GetResourceManager()->ReleaseCurrentResource(disp->id); + GetResourceManager()->RemoveWrapper(ToTypedHandle(disp->real.As())); + } break; case eResDeviceMemory: vt->FreeMemory(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkDeviceMemory(handle)); break; case eResBuffer: vt->DestroyBuffer(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkBuffer(handle)); break; case eResBufferView: vt->DestroyBufferView(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkBufferView(handle)); break; case eResImage: vt->DestroyImage(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkImage(handle)); break; case eResImageView: vt->DestroyImageView(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkImageView(handle)); break; case eResFramebuffer: vt->DestroyFramebuffer(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkFramebuffer(handle)); break; case eResRenderPass: vt->DestroyRenderPass(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkRenderPass(handle)); break; case eResShaderModule: vt->DestroyShaderModule(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkShaderModule(handle)); break; case eResShader: vt->DestroyShader(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkShader(handle)); break; case eResPipelineCache: vt->DestroyPipelineCache(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkPipelineCache(handle)); break; case eResPipelineLayout: vt->DestroyPipelineLayout(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkPipelineLayout(handle)); break; case eResPipeline: vt->DestroyPipeline(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkPipeline(handle)); break; case eResSampler: vt->DestroySampler(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkSampler(handle)); break; case eResDescriptorPool: vt->DestroyDescriptorPool(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkDescriptorPool(handle)); break; case eResDescriptorSetLayout: vt->DestroyDescriptorSetLayout(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkDescriptorSetLayout(handle)); break; case eResCmdPool: vt->DestroyCommandPool(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkCmdPool(handle)); break; case eResFence: vt->DestroyFence(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkFence(handle)); break; case eResEvent: vt->DestroyEvent(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkEvent(handle)); break; case eResQueryPool: vt->DestroyQueryPool(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkQueryPool(handle)); break; case eResSemaphore: vt->DestroySemaphore(Unwrap(dev), nondisp->real.As()); + GetResourceManager()->ReleaseWrappedResource(VkSemaphore(handle)); break; } @@ -405,7 +472,10 @@ bool WrappedVulkan::Serialise_vkCreateRenderPass( ret = ObjDisp(device)->CreateRenderPass(Unwrap(device), &info, &rpinfo.loadRP); RDCASSERT(ret == VK_SUCCESS); - GetResourceManager()->WrapResource(Unwrap(device), rpinfo.loadRP); + ResourceId loadRPid = GetResourceManager()->WrapResource(Unwrap(device), rpinfo.loadRP); + + // register as a live-only resource, so it is cleaned up properly + GetResourceManager()->AddLiveResource(loadRPid, rpinfo.loadRP); m_CreationInfo.m_RenderPass[live] = rpinfo; } @@ -468,7 +538,10 @@ VkResult WrappedVulkan::vkCreateRenderPass( ret = ObjDisp(device)->CreateRenderPass(Unwrap(device), &info, &rpinfo.loadRP); RDCASSERT(ret == VK_SUCCESS); - GetResourceManager()->WrapResource(Unwrap(device), rpinfo.loadRP); + ResourceId loadRPid = GetResourceManager()->WrapResource(Unwrap(device), rpinfo.loadRP); + + // register as a live-only resource, so it is cleaned up properly + GetResourceManager()->AddLiveResource(loadRPid, rpinfo.loadRP); m_CreationInfo.m_RenderPass[id] = rpinfo; } diff --git a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp index 0f9cc53b6..8ea7dde05 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_queue_funcs.cpp @@ -27,12 +27,12 @@ bool WrappedVulkan::Serialise_vkGetDeviceQueue( Serialiser* localSerialiser, VkDevice device, - uint32_t queueNodeIndex, + uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue* pQueue) { SERIALISE_ELEMENT(ResourceId, devId, GetResID(device)); - SERIALISE_ELEMENT(uint32_t, nodeIdx, queueNodeIndex); + SERIALISE_ELEMENT(uint32_t, familyIdx, queueFamilyIndex); SERIALISE_ELEMENT(uint32_t, idx, queueIndex); SERIALISE_ELEMENT(ResourceId, queueId, GetResID(*pQueue)); @@ -41,17 +41,21 @@ bool WrappedVulkan::Serialise_vkGetDeviceQueue( device = GetResourceManager()->GetLiveHandle(devId); VkQueue queue; - VkResult ret = ObjDisp(device)->GetDeviceQueue(Unwrap(device), nodeIdx, idx, &queue); + VkResult ret = ObjDisp(device)->GetDeviceQueue(Unwrap(device), familyIdx, idx, &queue); VkQueue real = queue; GetResourceManager()->WrapResource(Unwrap(device), queue); GetResourceManager()->AddLiveResource(queueId, queue); - - // VKTODOMED hack - fixup unwrapped queue objects, because we tried to fill them - // out early - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - if(m_PhysicalDeviceData[i].q == real) m_PhysicalDeviceData[i].q = queue; + + if(familyIdx == m_QueueFamilyIdx) + { + m_Queue = queue; + + // we can now submit any cmds that were queued (e.g. from creating debug + // manager on vkCreateDevice) + SubmitCmds(); + } } return true; @@ -59,11 +63,11 @@ bool WrappedVulkan::Serialise_vkGetDeviceQueue( VkResult WrappedVulkan::vkGetDeviceQueue( VkDevice device, - uint32_t queueNodeIndex, + uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue* pQueue) { - VkResult ret = ObjDisp(device)->GetDeviceQueue(Unwrap(device), queueNodeIndex, queueIndex, pQueue); + VkResult ret = ObjDisp(device)->GetDeviceQueue(Unwrap(device), queueFamilyIndex, queueIndex, pQueue); if(ret == VK_SUCCESS) { @@ -86,7 +90,7 @@ VkResult WrappedVulkan::vkGetDeviceQueue( CACHE_THREAD_SERIALISER(); SCOPED_SERIALISE_CONTEXT(GET_DEVICE_QUEUE); - Serialise_vkGetDeviceQueue(localSerialiser, device, queueNodeIndex, queueIndex, pQueue); + Serialise_vkGetDeviceQueue(localSerialiser, device, queueFamilyIndex, queueIndex, pQueue); chunk = scope.Get(); } @@ -94,7 +98,14 @@ VkResult WrappedVulkan::vkGetDeviceQueue( VkResourceRecord *record = GetResourceManager()->AddResourceRecord(*pQueue); RDCASSERT(record); - m_InstanceRecord->queues.push_back(*pQueue); + VkResourceRecord *instrecord = GetRecord(m_Instance); + + // treat queues as pool members of the instance (ie. freed when the instance dies) + { + instrecord->LockChunks(); + instrecord->pooledChildren.push_back(record); + instrecord->UnlockChunks(); + } record->AddChunk(chunk); } @@ -102,6 +113,15 @@ VkResult WrappedVulkan::vkGetDeviceQueue( { GetResourceManager()->AddLiveResource(id, *pQueue); } + + if(queueFamilyIndex == m_QueueFamilyIdx) + { + m_Queue = *pQueue; + + // we can now submit any cmds that were queued (e.g. from creating debug + // manager on vkCreateDevice) + SubmitCmds(); + } } } diff --git a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp index 022774f8d..4a2b55363 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_wsi_funcs.cpp @@ -153,9 +153,6 @@ VkResult WrappedVulkan::vkGetSwapchainImagesKHR( VkResourceRecord *swaprecord = GetRecord(swapchain); swaprecord->AddParent(record); - // decrement refcount on swap images, so that they are only ref'd from the swapchain - // (and will be deleted when it is deleted) - record->Delete(GetResourceManager()); } else { @@ -243,12 +240,6 @@ bool WrappedVulkan::Serialise_vkCreateSwapchainKHR( VK_IMAGE_LAYOUT_UNDEFINED, }; - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - { - if(m_PhysicalDeviceData[i].dev == device) - m_SwapPhysDevice = (int)i; - } - for(uint32_t i=0; i < numSwapImages; i++) { VkDeviceMemory mem = VK_NULL_HANDLE; @@ -272,7 +263,9 @@ bool WrappedVulkan::Serialise_vkCreateSwapchainKHR( vkr = ObjDisp(device)->AllocMemory(Unwrap(device), &allocInfo, &mem); RDCASSERT(vkr == VK_SUCCESS); - GetResourceManager()->WrapResource(Unwrap(device), mem); + ResourceId memid = GetResourceManager()->WrapResource(Unwrap(device), mem); + // register as a live-only resource, so it is cleaned up properly + GetResourceManager()->AddLiveResource(memid, mem); vkr = ObjDisp(device)->BindImageMemory(Unwrap(device), Unwrap(im), Unwrap(mem), 0); RDCASSERT(vkr == VK_SUCCESS); @@ -346,12 +339,6 @@ VkResult WrappedVulkan::vkCreateSwapchainKHR( VkResourceRecord *record = GetResourceManager()->AddResourceRecord(*pSwapChain); record->AddChunk(chunk); - - for(size_t i=0; i < m_PhysicalDeviceData.size(); i++) - { - if(m_PhysicalDeviceData[i].dev == device) - m_SwapPhysDevice = (int)i; - } record->swapInfo = new SwapchainInfo(); SwapchainInfo &swapInfo = *record->swapInfo; @@ -631,6 +618,10 @@ VkResult WrappedVulkan::vkQueuePresentKHR( GetDebugManager()->EndText(textstate); SubmitCmds(); + + // VKTODOLOW once we have a more sophisticated way of re-using submitted command + // buffers once they've executed and are safe to recycle, this can be removed + FlushQ(); } } @@ -980,7 +971,7 @@ VkResult WrappedVulkan::vkQueuePresentKHR( GetResourceManager()->ClearReferencedResources(); - GetResourceManager()->MarkResourceFrameReferenced(m_InstanceRecord->GetResourceID(), eFrameRef_Read); + GetResourceManager()->MarkResourceFrameReferenced(GetResID(m_Instance), eFrameRef_Read); // need to do all this atomically so that no other commands // will check to see if they need to markdirty or markpendingdirty