diff --git a/renderdoc/driver/vulkan/vk_core.cpp b/renderdoc/driver/vulkan/vk_core.cpp index 6cfd4bd52..a9d974a68 100644 --- a/renderdoc/driver/vulkan/vk_core.cpp +++ b/renderdoc/driver/vulkan/vk_core.cpp @@ -185,9 +185,6 @@ WrappedVulkan::~WrappedVulkan() SAFE_DELETE(m_FrameReader); - for(size_t i = 0; i < m_MemIdxMaps.size(); i++) - delete[] m_MemIdxMaps[i]; - for(size_t i = 0; i < m_ThreadSerialisers.size(); i++) delete m_ThreadSerialisers[i]; diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h index 086c1e772..62e8940fc 100644 --- a/renderdoc/driver/vulkan/vk_core.h +++ b/renderdoc/driver/vulkan/vk_core.h @@ -369,9 +369,6 @@ private: uint32_t uploadMemIndex = 0; uint32_t GPULocalMemIndex = 0; - VkPhysicalDeviceMemoryProperties *fakeMemProps = NULL; - uint32_t *memIdxMap = NULL; - VkPhysicalDeviceFeatures features = {}; VkPhysicalDeviceProperties props = {}; VkPhysicalDeviceMemoryProperties memProps = {}; @@ -453,9 +450,6 @@ private: // (targetQueueFamily << 32) | (targetQueueIndex) std::vector m_QueueRemapping[16]; - std::vector m_MemIdxMaps; - void RemapMemoryIndices(VkPhysicalDeviceMemoryProperties *memProps, uint32_t **memIdxMap); - void WrapAndProcessCreatedSwapchain(VkDevice device, const VkSwapchainCreateInfoKHR *pCreateInfo, VkSwapchainKHR *pSwapChain); diff --git a/renderdoc/driver/vulkan/vk_memory.cpp b/renderdoc/driver/vulkan/vk_memory.cpp index b96c8e342..75c91bf5b 100644 --- a/renderdoc/driver/vulkan/vk_memory.cpp +++ b/renderdoc/driver/vulkan/vk_memory.cpp @@ -83,98 +83,6 @@ uint32_t WrappedVulkan::PhysicalDeviceData::GetMemoryIndex(uint32_t resourceRequ return best; } -#define CREATE_NON_COHERENT_ATTRACTIVE_MEMORY 0 - -void WrappedVulkan::RemapMemoryIndices(VkPhysicalDeviceMemoryProperties *memProps, - uint32_t **memIdxMap) -{ - uint32_t *memmap = new uint32_t[VK_MAX_MEMORY_TYPES]; - *memIdxMap = memmap; - m_MemIdxMaps.push_back(memmap); - - for(size_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) - memmap[i] = ~0U; - -// basic idea here: -// We want to discourage coherent memory maps as much as possible while capturing, -// as they're painful to track. Unfortunately the spec guarantees that at least -// one such memory type will be available, and we must follow that. -// -// So, rather than removing the coherent memory type we make it as unappealing as -// possible and try and ensure that only someone looking specifically for a coherent -// memory type will find it. That way hopefully memory selection algorithms will -// pick non-coherent memory and do proper flushing as necessary. - -// we want to add a new heap, hopefully there is room -#if CREATE_NON_COHERENT_ATTRACTIVE_MEMORY - RDCASSERT(memProps->memoryHeapCount < VK_MAX_MEMORY_HEAPS - 1); - - uint32_t coherentHeap = memProps->memoryHeapCount; - memProps->memoryHeapCount++; - - // make a new heap that's tiny. If any applications look at heap sizes to determine - // viability, they'll dislike the look of this one (the real heaps should be much - // bigger). - memProps->memoryHeaps[coherentHeap].flags = 0; // not device local - memProps->memoryHeaps[coherentHeap].size = 32 * 1024 * 1024; -#endif - - // for every coherent memory type, add a non-coherent type first, then - // mark the coherent type with our crappy heap - - uint32_t origCount = memProps->memoryTypeCount; - VkMemoryType origTypes[VK_MAX_MEMORY_TYPES]; - memcpy(origTypes, memProps->memoryTypes, sizeof(origTypes)); - - uint32_t newtypeidx = 0; - - for(uint32_t i = 0; i < origCount; i++) - { -#if CREATE_NON_COHERENT_ATTRACTIVE_MEMORY - if((origTypes[i].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT) != 0) - { - // coherent type found. - - // can we still add a new type without exceeding the max? - if(memProps->memoryTypeCount + 1 <= VK_MAX_MEMORY_TYPES) - { - // copy both types from the original type - memProps->memoryTypes[newtypeidx] = origTypes[i]; - memProps->memoryTypes[newtypeidx + 1] = origTypes[i]; - - // mark first as non-coherent, cached - memProps->memoryTypes[newtypeidx].propertyFlags &= ~VK_MEMORY_PROPERTY_HOST_COHERENT_BIT; - memProps->memoryTypes[newtypeidx].propertyFlags |= VK_MEMORY_PROPERTY_HOST_CACHED_BIT; - - // point second at bad heap - memProps->memoryTypes[newtypeidx + 1].heapIndex = coherentHeap; - - // point both new types at this original type - memmap[newtypeidx++] = i; - memmap[newtypeidx++] = i; - - // we added a type - memProps->memoryTypeCount++; - } - else - { - // can't add a new type, but we can at least repoint this coherent - // type at the bad heap to discourage use - memProps->memoryTypes[newtypeidx] = origTypes[i]; - memProps->memoryTypes[newtypeidx].heapIndex = coherentHeap; - memmap[newtypeidx++] = i; - } - } - else -#endif - { - // non-coherent already or non-hostvisible, just copy through - memProps->memoryTypes[newtypeidx] = origTypes[i]; - memmap[newtypeidx++] = i; - } - } -} - MemoryAllocation WrappedVulkan::AllocateMemoryForResource(bool buffer, VkMemoryRequirements mrq, MemoryScope scope, MemoryType type) { diff --git a/renderdoc/driver/vulkan/vk_resources.cpp b/renderdoc/driver/vulkan/vk_resources.cpp index 940479816..e73d69263 100644 --- a/renderdoc/driver/vulkan/vk_resources.cpp +++ b/renderdoc/driver/vulkan/vk_resources.cpp @@ -3391,14 +3391,11 @@ VkResourceRecord::~VkResourceRecord() { VkResourceType resType = Resource != NULL ? IdentifyTypeByPtr(Resource) : eResUnknown; - if(resType == eResPhysicalDevice) - SAFE_DELETE(memProps); - // bufferviews and imageviews have non-owning pointers to the sparseinfo struct if(resType == eResBuffer || resType == eResImage) SAFE_DELETE(resInfo); - if(resType == eResInstance || resType == eResDevice) + if(resType == eResInstance || resType == eResDevice || resType == eResPhysicalDevice) SAFE_DELETE(instDevInfo); if(resType == eResSwapchain) diff --git a/renderdoc/driver/vulkan/vk_resources.h b/renderdoc/driver/vulkan/vk_resources.h index bf898afb4..9d6f8ade4 100644 --- a/renderdoc/driver/vulkan/vk_resources.h +++ b/renderdoc/driver/vulkan/vk_resources.h @@ -1422,12 +1422,7 @@ public: static byte markerValue[32]; VkResourceRecord(ResourceId id) - : ResourceRecord(id, true), - Resource(NULL), - bakedCommands(NULL), - pool(NULL), - memIdxMap(NULL), - ptrunion(NULL) + : ResourceRecord(id, true), Resource(NULL), bakedCommands(NULL), pool(NULL), ptrunion(NULL) { } @@ -1551,11 +1546,6 @@ public: WrappedVkRes *Resource; - // externally allocated/freed, a mapping from memory idx - // in our modified properties that were passed to the app - // to the memory indices that actually exist - uint32_t *memIdxMap; - // this points to the base resource, either memory or an image - // ie. the resource that can be modified or changes (or can become dirty) // since typical memory bindings are immutable and must happen before @@ -1582,15 +1572,14 @@ public: // allocation type of the Resource union { - void *ptrunion; // for initialisation to NULL - VkPhysicalDeviceMemoryProperties *memProps; // only for physical devices - InstanceDeviceInfo *instDevInfo; // only for logical devices or instances - ResourceInfo *resInfo; // only for buffers, images, and views of them - SwapchainInfo *swapInfo; // only for swapchains - MemMapState *memMapState; // only for device memory - CmdBufferRecordingInfo *cmdInfo; // only for command buffers - AttachmentInfo *imageAttachments; // only for framebuffers and render passes - PipelineLayoutData *pipeLayoutInfo; // only for pipeline layouts + void *ptrunion; // for initialisation to NULL + InstanceDeviceInfo *instDevInfo; // only for instances or physical/logical devices + ResourceInfo *resInfo; // only for buffers, images, and views of them + SwapchainInfo *swapInfo; // only for swapchains + MemMapState *memMapState; // only for device memory + CmdBufferRecordingInfo *cmdInfo; // only for command buffers + AttachmentInfo *imageAttachments; // only for framebuffers and render passes + PipelineLayoutData *pipeLayoutInfo; // only for pipeline layouts DescriptorSetData *descInfo; // only for descriptor sets and descriptor set layouts DescUpdateTemplate *descTemplateInfo; // only for descriptor update templates uint32_t queueFamilyIndex; // only for queues diff --git a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp index 11243cf75..cb39e96f8 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp @@ -472,7 +472,6 @@ ReplayStatus WrappedVulkan::Initialise(VkInitParams ¶ms, uint64_t sectionVer m_ReplayPhysicalDevices.resize(count); m_ReplayPhysicalDevicesUsed.resize(count); m_OriginalPhysicalDevices.resize(count); - m_MemIdxMaps.resize(count); vkr = ObjDisp(m_Instance) ->EnumeratePhysicalDevices(Unwrap(m_Instance), &count, &m_ReplayPhysicalDevices[0]); @@ -914,7 +913,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk SERIALISE_ELEMENT_LOCAL(PhysicalDevice, GetResID(*pPhysicalDevices)) .TypedAs("VkPhysicalDevice"_lit); - uint32_t memIdxMap[VK_MAX_MEMORY_TYPES] = {0}; + uint32_t legacyUnused_memIdxMap[VK_MAX_MEMORY_TYPES] = {0}; // not used at the moment but useful for reference and might be used // in the future VkPhysicalDeviceProperties physProps = {}; @@ -929,8 +928,6 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk if(ser.IsWriting()) { - memcpy(memIdxMap, GetRecord(*pPhysicalDevices)->memIdxMap, sizeof(memIdxMap)); - ObjDisp(instance)->GetPhysicalDeviceProperties(Unwrap(*pPhysicalDevices), &physProps); ObjDisp(instance)->GetPhysicalDeviceMemoryProperties(Unwrap(*pPhysicalDevices), &memProps); ObjDisp(instance)->GetPhysicalDeviceFeatures(Unwrap(*pPhysicalDevices), &physFeatures); @@ -975,7 +972,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk } } - SERIALISE_ELEMENT(memIdxMap); + SERIALISE_ELEMENT(legacyUnused_memIdxMap).Hidden(); // was never used SERIALISE_ELEMENT(physProps); SERIALISE_ELEMENT(memProps); SERIALISE_ELEMENT(physFeatures); @@ -1019,7 +1016,7 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk // hopefully the most common case is when there's a precise match, and maybe the order changed. // // If more GPUs were present on replay than during capture, we map many-to-one which might have - // bad side-effects as e.g. we have to pick one memidxmap, but this is as good as we can do. + // bad side-effects, but this is as good as we can do. uint32_t bestIdx = 0; VkPhysicalDeviceProperties bestPhysProps = {}; @@ -1309,17 +1306,6 @@ bool WrappedVulkan::Serialise_vkEnumeratePhysicalDevices(SerialiserType &ser, Vk "Mapping multiple capture-time physical devices to a single replay-time physical device." "This means the HW has changed between capture and replay and may cause bugs."); } - else if(m_MemIdxMaps[bestIdx] == NULL) - { - // the first physical device 'wins' for the memory index map - uint32_t *storedMap = new uint32_t[32]; - memcpy(storedMap, memIdxMap, sizeof(memIdxMap)); - - for(uint32_t i = 0; i < 32; i++) - storedMap[i] = i; - - m_MemIdxMaps[bestIdx] = storedMap; - } m_ReplayPhysicalDevicesUsed[bestIdx] = true; } @@ -1367,10 +1353,6 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance, VkResourceRecord *record = GetResourceManager()->AddResourceRecord(devices[i]); RDCASSERT(record); - record->memProps = new VkPhysicalDeviceMemoryProperties(); - - ObjDisp(devices[i])->GetPhysicalDeviceMemoryProperties(Unwrap(devices[i]), record->memProps); - VkPhysicalDeviceProperties physProps; ObjDisp(devices[i])->GetPhysicalDeviceProperties(Unwrap(devices[i]), &physProps); @@ -1383,9 +1365,6 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance, m_PhysicalDevices[i] = devices[i]; - // we remap memory indices to discourage coherent maps as much as possible - RemapMemoryIndices(record->memProps, &record->memIdxMap); - { CACHE_THREAD_SERIALISER(); @@ -1399,6 +1378,9 @@ VkResult WrappedVulkan::vkEnumeratePhysicalDevices(VkInstance instance, instrecord->AddParent(record); + // copy the instance's setup directly + record->instDevInfo = new InstanceDeviceInfo(*instrecord->instDevInfo); + // treat physical devices as pool members of the instance (ie. freed when the instance dies) { instrecord->LockChunks(); @@ -2758,15 +2740,6 @@ bool WrappedVulkan::Serialise_vkCreateDevice(SerialiserType &ser, VkPhysicalDevi m_PhysicalDeviceData.GPULocalMemIndex = m_PhysicalDeviceData.GetMemoryIndex( ~0U, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT); - for(size_t i = 0; i < m_ReplayPhysicalDevices.size(); i++) - { - if(physicalDevice == m_ReplayPhysicalDevices[i]) - { - m_PhysicalDeviceData.memIdxMap = m_MemIdxMaps[i]; - break; - } - } - APIProps.vendor = GetDriverInfo().Vendor(); // temporarily disable the debug message sink, to ignore any false positive messages from our @@ -3086,8 +3059,6 @@ VkResult WrappedVulkan::vkCreateDevice(VkPhysicalDevice physicalDevice, record->AddChunk(chunk); - record->memIdxMap = GetRecord(physicalDevice)->memIdxMap; - record->instDevInfo = new InstanceDeviceInfo(); record->instDevInfo->brokenGetDeviceProcAddr = @@ -3217,8 +3188,6 @@ VkResult WrappedVulkan::vkCreateDevice(VkPhysicalDevice physicalDevice, m_PhysicalDeviceData.queueCount = qCount; memcpy(m_PhysicalDeviceData.queueProps, props, qCount * sizeof(VkQueueFamilyProperties)); - m_PhysicalDeviceData.fakeMemProps = GetRecord(physicalDevice)->memProps; - m_ShaderCache = new VulkanShaderCache(this); m_TextRenderer = new VulkanTextRenderer(this); diff --git a/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp index a42507086..12833b5cc 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_get_funcs.cpp @@ -211,12 +211,6 @@ void WrappedVulkan::vkGetPhysicalDeviceQueueFamilyProperties( void WrappedVulkan::vkGetPhysicalDeviceMemoryProperties( VkPhysicalDevice physicalDevice, VkPhysicalDeviceMemoryProperties *pMemoryProperties) { - if(pMemoryProperties) - { - *pMemoryProperties = *GetRecord(physicalDevice)->memProps; - return; - } - ObjDisp(physicalDevice)->GetPhysicalDeviceMemoryProperties(Unwrap(physicalDevice), pMemoryProperties); } @@ -237,21 +231,6 @@ void WrappedVulkan::vkGetBufferMemoryRequirements(VkDevice device, VkBuffer buff *pMemoryRequirements = GetRecord(buffer)->resInfo->memreqs; else ObjDisp(device)->GetBufferMemoryRequirements(Unwrap(device), Unwrap(buffer), pMemoryRequirements); - - // don't do remapping here on replay. - if(IsReplayMode(m_State)) - return; - - uint32_t bits = pMemoryRequirements->memoryTypeBits; - uint32_t *memIdxMap = GetRecord(device)->memIdxMap; - - pMemoryRequirements->memoryTypeBits = 0; - - // for each of our fake memory indices, check if the real - // memory type it points to is set - if so, set our fake bit - for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) - if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) - pMemoryRequirements->memoryTypeBits |= (1U << i); } void WrappedVulkan::vkGetImageMemoryRequirements(VkDevice device, VkImage image, @@ -265,21 +244,6 @@ void WrappedVulkan::vkGetImageMemoryRequirements(VkDevice device, VkImage image, else ObjDisp(device)->GetImageMemoryRequirements(Unwrap(device), Unwrap(image), pMemoryRequirements); - // don't do remapping here on replay. - if(IsReplayMode(m_State)) - return; - - uint32_t bits = pMemoryRequirements->memoryTypeBits; - uint32_t *memIdxMap = GetRecord(device)->memIdxMap; - - pMemoryRequirements->memoryTypeBits = 0; - - // for each of our fake memory indices, check if the real - // memory type it points to is set - if so, set our fake bit - for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) - if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) - pMemoryRequirements->memoryTypeBits |= (1U << i); - // AMD can have some variability in the returned size, so we need to pad the reported size to // allow for this. The variability isn't quite clear, but for now we assume aligning size to // alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it @@ -326,21 +290,6 @@ void WrappedVulkan::vkGetBufferMemoryRequirements2(VkDevice device, // pessimistic for the case of external memory bound resources. See vkCreateBuffer/vkCreateImage if(IsCaptureMode(m_State) && GetRecord(pInfo->buffer)->resInfo) pMemoryRequirements->memoryRequirements = GetRecord(pInfo->buffer)->resInfo->memreqs; - - // don't do remapping here on replay. - if(IsReplayMode(m_State)) - return; - - uint32_t bits = pMemoryRequirements->memoryRequirements.memoryTypeBits; - uint32_t *memIdxMap = GetRecord(device)->memIdxMap; - - pMemoryRequirements->memoryRequirements.memoryTypeBits = 0; - - // for each of our fake memory indices, check if the real - // memory type it points to is set - if so, set our fake bit - for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) - if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) - pMemoryRequirements->memoryRequirements.memoryTypeBits |= (1U << i); } void WrappedVulkan::vkGetImageMemoryRequirements2(VkDevice device, @@ -361,17 +310,6 @@ void WrappedVulkan::vkGetImageMemoryRequirements2(VkDevice device, if(IsReplayMode(m_State)) return; - uint32_t bits = pMemoryRequirements->memoryRequirements.memoryTypeBits; - uint32_t *memIdxMap = GetRecord(device)->memIdxMap; - - pMemoryRequirements->memoryRequirements.memoryTypeBits = 0; - - // for each of our fake memory indices, check if the real - // memory type it points to is set - if so, set our fake bit - for(uint32_t i = 0; i < VK_MAX_MEMORY_TYPES; i++) - if(memIdxMap[i] < 32U && (bits & (1U << memIdxMap[i]))) - pMemoryRequirements->memoryRequirements.memoryTypeBits |= (1U << i); - // AMD can have some variability in the returned size, so we need to pad the reported size to // allow for this. The variability isn't quite clear, but for now we assume aligning size to // alignment * 4 should be sufficient (adding on a fixed padding won't help the problem as it diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp index 0e9a05bc7..e1eea8c34 100644 --- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp +++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp @@ -263,11 +263,6 @@ bool WrappedVulkan::Serialise_vkAllocateMemory(SerialiserType &ser, VkDevice dev { VkDeviceMemory mem = VK_NULL_HANDLE; - // serialised memory type index is non-remapped, so we remap now. - // PORTABILITY may need to re-write info to change memory type index to the - // appropriate index on replay - AllocateInfo.memoryTypeIndex = m_PhysicalDeviceData.memIdxMap[AllocateInfo.memoryTypeIndex]; - VkMemoryAllocateInfo patched = AllocateInfo; byte *tempMem = GetTempMemory(GetNextPatchSize(patched.pNext)); @@ -345,8 +340,6 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate VkDeviceMemory *pMemory) { VkMemoryAllocateInfo info = *pAllocateInfo; - if(IsCaptureMode(m_State)) - info.memoryTypeIndex = GetRecord(device)->memIdxMap[info.memoryTypeIndex]; { // we need to be able to allocate a buffer that covers the whole memory range. However @@ -469,7 +462,7 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate record->Length = info.allocationSize; uint32_t memProps = - m_PhysicalDeviceData.fakeMemProps->memoryTypes[info.memoryTypeIndex].propertyFlags; + m_PhysicalDeviceData.memProps.memoryTypes[info.memoryTypeIndex].propertyFlags; // if memory is not host visible, so not mappable, don't create map state at all if((memProps & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) != 0)