diff --git a/renderdoc/common/wrapped_pool.h b/renderdoc/common/wrapped_pool.h index 104204847..5ab7f331a 100644 --- a/renderdoc/common/wrapped_pool.h +++ b/renderdoc/common/wrapped_pool.h @@ -41,7 +41,7 @@ template class GetTypeName { public: static const char *Name(); }; template class FriendMaker { public: typedef C Type; }; // allocate each class in its own pool so we can identify the type by the pointer -template +template class WrappingPool { public: @@ -215,7 +215,8 @@ class WrappingPool allocated[idx] = false; #if !defined(RELEASE) - memset(p, 0xfe, AllocByteSize); + if(DebugClear) + memset(p, 0xfe, AllocByteSize); #endif } diff --git a/renderdoc/driver/vulkan/vk_common.cpp b/renderdoc/driver/vulkan/vk_common.cpp index 20410f3e3..275982584 100644 --- a/renderdoc/driver/vulkan/vk_common.cpp +++ b/renderdoc/driver/vulkan/vk_common.cpp @@ -1961,7 +1961,7 @@ string ToStrHelper::Get(const VkPresentModeWSI &el) ResourceId id; \ if(m_Mode >= WRITING) id = GetResID(obj); \ Serialise(name, id); \ - if(m_Mode < WRITING) obj = (id == ResourceId()) ? VK_NULL_HANDLE : Unwrap(VKMGR()->GetLiveHandle(id)); \ + if(m_Mode < WRITING) obj = (id == ResourceId() || !VKMGR()->HasLiveResource(id)) ? VK_NULL_HANDLE : Unwrap(VKMGR()->GetLiveHandle(id)); \ } static void SerialiseNext(Serialiser *ser, const void *&pNext) diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp index ddefbc887..b59f8bf04 100644 --- a/renderdoc/driver/vulkan/vk_initstate.cpp +++ b/renderdoc/driver/vulkan/vk_initstate.cpp @@ -238,21 +238,73 @@ bool WrappedVulkan::Serialise_InitialState(WrappedVkRes *res) info += layout.bindings[j].arraySize; - // VKTODOMED this should check to see if the actual descriptor - // bits used by this element in the layout are set. For now - // we're mostly concerned with descriptors that were alloc'd and - // never written to, so are completely empty. - // Also needs to handle arrays properly - we are very - // all-or-nothing with this check + // check that the resources we need for this write are present, + // as some might have been skipped due to stale descriptor set + // slots or otherwise unreferenced objects (the descriptor set + // initial contents do not cause a frame reference for their + // resources + bool valid = true; + + // quick check for slots that were completely uninitialised + // and so don't have valid data if(writes[i].pDescriptors->bufferView == VK_NULL_HANDLE && writes[i].pDescriptors->sampler == VK_NULL_HANDLE && writes[i].pDescriptors->imageView == VK_NULL_HANDLE && writes[i].pDescriptors->attachmentView == VK_NULL_HANDLE) - writes[i].count = 0; + { + valid = false; + } + else + { + switch(writes[i].descriptorType) + { + case VK_DESCRIPTOR_TYPE_SAMPLER: + { + for(uint32_t d=0; d < writes[i].count; d++) + valid &= (writes[i].pDescriptors[d].sampler != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + { + for(uint32_t d=0; d < writes[i].count; d++) + { + valid &= (writes[i].pDescriptors[d].sampler != VK_NULL_HANDLE); + valid &= (writes[i].pDescriptors[d].imageView != VK_NULL_HANDLE); + } + break; + } + case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: + case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: + { + for(uint32_t d=0; d < writes[i].count; d++) + valid &= (writes[i].pDescriptors[d].imageView != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER: + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: + { + for(uint32_t d=0; d < writes[i].count; d++) + valid &= (writes[i].pDescriptors[d].bufferView != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: + { + for(uint32_t d=0; d < writes[i].count; d++) + valid &= (writes[i].pDescriptors[d].attachmentView != VK_NULL_HANDLE); + break; + } + default: + RDCERR("Unexpected descriptor type %d", writes[i].descriptorType); + } + } - // if this write became completely null and void, skip it + // if this write is not valid, skip it // and start writing the next one in here - if(writes[i].count == 0) + if(!valid) validBinds--; else i++; diff --git a/renderdoc/driver/vulkan/vk_manager.h b/renderdoc/driver/vulkan/vk_manager.h index 7ef486bb6..cdea3cc1a 100644 --- a/renderdoc/driver/vulkan/vk_manager.h +++ b/renderdoc/driver/vulkan/vk_manager.h @@ -130,7 +130,7 @@ class VulkanResourceManager : public ResourceManager - void ReleaseWrappedResource(realtype obj) + void ReleaseWrappedResource(realtype obj, bool clearID = false) { ResourceId id = GetResID(obj); @@ -142,6 +142,24 @@ class VulkanResourceManager : public ResourceManagerDelete(this); + if(clearID) + { + // note the nulling of the wrapped object's ID here is rather unpleasant, + // but the lesser of two evils to ensure that stale descriptor set slots + // referencing the object behave safely. To do this correctly we would need + // to maintain a list of back-references to every descriptor set that has + // this object bound, and invalidate them. Instead we just make sure the ID + // is always something sensible, since we know the deallocation doesn't + // free the memory - the object is pool-allocated. + // If a new object is allocated in that pool slot, it will still be a valid + // ID and if the resource isn't ever referenced elsewhere, it will just be + // a non-live ID to be ignored. + + if(IsDispatchableRes(GetWrapped(obj))) + ((WrappedVkDispRes *)GetWrapped(obj))->id = ResourceId(); + else + ((WrappedVkNonDispRes *)GetWrapped(obj))->id = ResourceId(); + } delete GetWrapped(obj); } diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index 2748b1221..28a6223c7 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -276,7 +276,7 @@ struct WrappedVkBufferView : WrappedVkNonDispRes typedef VkBufferView InnerType; static const int AllocPoolCount = 128*1024; static const int AllocPoolMaxByteSize = 3*1024*1024; - ALLOCATE_WITH_WRAPPED_POOL(WrappedVkBufferView, AllocPoolCount, AllocPoolMaxByteSize); + ALLOCATE_WITH_WRAPPED_POOL(WrappedVkBufferView, AllocPoolCount, AllocPoolMaxByteSize, false); enum { TypeEnum = eResBufferView, }; }; struct WrappedVkImageView : WrappedVkNonDispRes @@ -285,7 +285,7 @@ struct WrappedVkImageView : WrappedVkNonDispRes typedef VkImageView InnerType; static const int AllocPoolCount = 128*1024; static const int AllocPoolMaxByteSize = 3*1024*1024; - ALLOCATE_WITH_WRAPPED_POOL(WrappedVkImageView, AllocPoolCount, AllocPoolMaxByteSize); + ALLOCATE_WITH_WRAPPED_POOL(WrappedVkImageView, AllocPoolCount, AllocPoolMaxByteSize, false); enum { TypeEnum = eResImageView, }; }; struct WrappedVkAttachmentView : WrappedVkNonDispRes @@ -294,7 +294,7 @@ struct WrappedVkAttachmentView : WrappedVkNonDispRes typedef VkAttachmentView InnerType; static const int AllocPoolCount = 128*1024; static const int AllocPoolMaxByteSize = 3*1024*1024; - ALLOCATE_WITH_WRAPPED_POOL(WrappedVkAttachmentView, AllocPoolCount, AllocPoolMaxByteSize); + ALLOCATE_WITH_WRAPPED_POOL(WrappedVkAttachmentView, AllocPoolCount, AllocPoolMaxByteSize, false); enum { TypeEnum = eResAttachmentView, }; }; struct WrappedVkShaderModule : WrappedVkNonDispRes @@ -352,7 +352,9 @@ struct WrappedVkDescriptorSetLayout : WrappedVkNonDispRes struct WrappedVkSampler : WrappedVkNonDispRes { WrappedVkSampler(VkSampler obj, ResourceId objId) : WrappedVkNonDispRes(obj, objId) {} - typedef VkSampler InnerType; ALLOCATE_WITH_WRAPPED_POOL(WrappedVkSampler); + static const int AllocPoolCount = 8192; + static const int AllocPoolMaxByteSize = 1024*1024; + typedef VkSampler InnerType; ALLOCATE_WITH_WRAPPED_POOL(WrappedVkSampler, AllocPoolCount, AllocPoolMaxByteSize, false); enum { TypeEnum = eResSampler, }; }; struct WrappedVkDescriptorPool : WrappedVkNonDispRes diff --git a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp index be0ef037f..96c08b844 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_descriptor_funcs.cpp @@ -344,9 +344,66 @@ bool WrappedVulkan::Serialise_vkUpdateDescriptorSets( device = GetResourceManager()->GetLiveHandle(devId); if(writes) - ObjDisp(device)->UpdateDescriptorSets(Unwrap(device), 1, &writeDesc, 0, NULL); + { + // check for validity - if a resource wasn't referenced other than in this update + // (ie. the descriptor set was overwritten or never bound), then the write descriptor + // will be invalid with some missing handles. It's safe though to just skip this + // update as we only get here if it's never used. + + bool valid = true; + + switch(writeDesc.descriptorType) + { + case VK_DESCRIPTOR_TYPE_SAMPLER: + { + for(uint32_t i=0; i < writeDesc.count; i++) + valid &= (writeDesc.pDescriptors[i].sampler != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER: + { + for(uint32_t i=0; i < writeDesc.count; i++) + { + valid &= (writeDesc.pDescriptors[i].sampler != VK_NULL_HANDLE); + valid &= (writeDesc.pDescriptors[i].imageView != VK_NULL_HANDLE); + } + break; + } + case VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE: + case VK_DESCRIPTOR_TYPE_STORAGE_IMAGE: + { + for(uint32_t i=0; i < writeDesc.count; i++) + valid &= (writeDesc.pDescriptors[i].imageView != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER: + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER: + case VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC: + case VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC: + { + for(uint32_t i=0; i < writeDesc.count; i++) + valid &= (writeDesc.pDescriptors[i].bufferView != VK_NULL_HANDLE); + break; + } + case VK_DESCRIPTOR_TYPE_INPUT_ATTACHMENT: + { + for(uint32_t i=0; i < writeDesc.count; i++) + valid &= (writeDesc.pDescriptors[i].attachmentView != VK_NULL_HANDLE); + break; + } + default: + RDCERR("Unexpected descriptor type %d", writeDesc.descriptorType); + } + + if(valid) + ObjDisp(device)->UpdateDescriptorSets(Unwrap(device), 1, &writeDesc, 0, NULL); + } else + { ObjDisp(device)->UpdateDescriptorSets(Unwrap(device), 0, NULL, 1, ©Desc); + } } return true; @@ -436,8 +493,6 @@ VkResult WrappedVulkan::vkUpdateDescriptorSets( m_FrameCaptureRecord->AddChunk(scope.Get()); } - // don't have to mark referenced any of the resources pointed to by the descriptor sets - that's handled - // on queue submission by marking ref'd all the current bindings of the sets referenced by the cmd buffer GetResourceManager()->MarkResourceFrameReferenced(GetResID(pDescriptorCopies[i].destSet), eFrameRef_Write); GetResourceManager()->MarkResourceFrameReferenced(GetResID(pDescriptorCopies[i].srcSet), eFrameRef_Read); } diff --git a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp index b324a4136..2eebb1bdb 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_misc_funcs.cpp @@ -29,7 +29,7 @@ { \ if(m_ImageInfo.find(GetResID(obj)) != m_ImageInfo.end()) m_ImageInfo.erase(GetResID(obj)); \ VkResult ret = ObjDisp(device)->func(Unwrap(device), Unwrap(obj)); \ - GetResourceManager()->ReleaseWrappedResource(obj); \ + GetResourceManager()->ReleaseWrappedResource(obj, true); \ return ret; \ }