From 795d7e0910056bf1971dbb8752581dc5bfa06fdf Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Mon, 22 Feb 2021 12:37:50 +0000
Subject: [PATCH] Handle a memory object that can't be covered by a buffer.
 Closes #2185

* On some drivers like older NV hardware, there are memory types which can only
  be bound to images and never to buffers, so ensure we don't try to create a
  memory-spanning buffer for such allocations and avoid marking memory behind
  images as written or it will become dirty in subsequent captures.
---
 renderdoc/driver/vulkan/vk_initstate.cpp      | 11 +++++++-
 renderdoc/driver/vulkan/vk_resources.cpp      |  9 +++----
 .../vulkan/wrappers/vk_resource_funcs.cpp     | 27 ++++++++++++++++---
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/renderdoc/driver/vulkan/vk_initstate.cpp b/renderdoc/driver/vulkan/vk_initstate.cpp
index 8ac136f87..a47b798ae 100644
--- a/renderdoc/driver/vulkan/vk_initstate.cpp
+++ b/renderdoc/driver/vulkan/vk_initstate.cpp
@@ -423,13 +423,22 @@ bool WrappedVulkan::Prepare_InitialState(WrappedVkRes *res)
   }
   else if(type == eResDeviceMemory)
   {
+    VkResourceRecord *record = GetResourceManager()->GetResourceRecord(id);
+
+    // if the memory has no wholeMemBuf we cannot fetch its contents. We shouldn't get here with
+    // only images bound to the memory so something has gone wrong
+    if(record->memMapState->wholeMemBuf == VK_NULL_HANDLE)
+    {
+      RDCERR("Trying to fetch device memory initial states without wholeMemBuf");
+      return true;
+    }
+
     VkResult vkr = VK_SUCCESS;
 
     VkDevice d = GetDev();
     // INITSTATEBATCH
     VkCommandBuffer cmd = GetNextCmd();
 
-    VkResourceRecord *record = GetResourceManager()->GetResourceRecord(id);
     VkDeviceMemory datamem = ToUnwrappedHandle<VkDeviceMemory>(res);
     VkDeviceSize datasize = record->Length;
 
diff --git a/renderdoc/driver/vulkan/vk_resources.cpp b/renderdoc/driver/vulkan/vk_resources.cpp
index 9c08dbd06..f17fae238 100644
--- a/renderdoc/driver/vulkan/vk_resources.cpp
+++ b/renderdoc/driver/vulkan/vk_resources.cpp
@@ -3854,12 +3854,9 @@ void VkResourceRecord::MarkImageViewFrameReferenced(VkResourceRecord *view, cons
   // mark image view as read
   MarkResourceFrameReferenced(view->GetResourceID(), eFrameRef_Read);
 
-  // mark memory backing image. For dedicated images we always treat the memory as read only so
-  // we don't try and include its initial contents.
-  if(view->dedicated)
-    MarkResourceFrameReferenced(mem, eFrameRef_Read);
-  else
-    MarkResourceFrameReferenced(mem, refType);
+  // mark memory backing image as read only so we don't try and include its initial contents just
+  // because of an image's writes
+  MarkResourceFrameReferenced(mem, eFrameRef_Read);
 
   ImageSubresourceRange imgRange;
   imgRange.aspectMask = view->viewRange.aspectMask;
diff --git a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp
index 19081bd8d..468a01e62 100644
--- a/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp
+++ b/renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp
@@ -535,9 +535,20 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate
 
       RDCASSERTEQUAL(mrq.size, info.allocationSize);
 
-      bufid = GetResourceManager()->WrapResource(Unwrap(device), wholeMemBuf);
+      if((mrq.memoryTypeBits & (1U << info.memoryTypeIndex)) != 0)
+      {
+        bufid = GetResourceManager()->WrapResource(Unwrap(device), wholeMemBuf);
 
-      ObjDisp(device)->BindBufferMemory(Unwrap(device), Unwrap(wholeMemBuf), Unwrap(*pMemory), 0);
+        ObjDisp(device)->BindBufferMemory(Unwrap(device), Unwrap(wholeMemBuf), Unwrap(*pMemory), 0);
+      }
+      else
+      {
+        // can't create a memory-spanning buffer for this allocation. Assume this is a case where
+        // this memory type is only available to images and is not mappable - in which case the
+        // whole memory buffer won't be needed so we can skip this.
+        ObjDisp(device)->DestroyBuffer(Unwrap(device), wholeMemBuf, NULL);
+        wholeMemBuf = VK_NULL_HANDLE;
+      }
     }
 
     if((dedicated != NULL || dedicatedNV != NULL) && wholeMemBuf != VK_NULL_HANDLE)
@@ -626,8 +637,18 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate
         // command buffer.
         if(Vulkan_GPUReadbackDeviceLocal() &&
            m_PhysicalDeviceData.props.deviceType == VK_PHYSICAL_DEVICE_TYPE_DISCRETE_GPU)
+        {
           record->memMapState->readbackOnGPU =
               ((memProps & VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT) != 0);
+
+          // we need a wholeMemBuf to readback on the GPU
+          if(record->memMapState->readbackOnGPU && wholeMemBuf == VK_NULL_HANDLE)
+          {
+            RDCWARN(
+                "Memory allocation would have been readback on GPU, but can't without wholeMemBuf");
+            record->memMapState->readbackOnGPU = false;
+          }
+        }
       }
 
       GetResourceManager()->AddDeviceMemory(id);
@@ -638,7 +659,7 @@ VkResult WrappedVulkan::vkAllocateMemory(VkDevice device, const VkMemoryAllocate
 
       m_CreationInfo.m_Memory[id].Init(GetResourceManager(), m_CreationInfo, &info);
 
-      if(dedicated == NULL && dedicatedNV == NULL)
+      if(dedicated == NULL && dedicatedNV == NULL && wholeMemBuf != VK_NULL_HANDLE)
       {
         // register as a live-only resource, so it is cleaned up properly
         GetResourceManager()->AddLiveResource(bufid, wholeMemBuf);