From 2c0b612c6f87d66f26f28db6db1bbefe27eac2a6 Mon Sep 17 00:00:00 2001
From: Benson Joeris <bjoeris@ad.corp.google.com>
Date: Wed, 29 Aug 2018 14:11:19 -0400
Subject: [PATCH] Fix order that InitialContents are applied for Vulkan.

This fixes a bug where the memory backing an image could be initialized
after the image itself, causing corruptions. ApplyInitialContents was
processing the resources in order of resource id. It is possible that a
VkImage and the VkDeviceMemory it is bound to both have InitialContent
(e.g. because the VkDeviceMemory also backs a buffer), and it is also
possible that the VkImage has a lower resource id than the
VkDeviceMemory. In this case, the VkImage's InitialContent is loaded,
and then the VkDeviceMemory's InitialContent is loaded. This direct
write of the backing memory causes the VkImage content to become
undefined and, at least on my AMD card, causes image corruption.

This is fixed by sorting the resource ids by type, which works because
eResDeviceMemory < eResImage. This sorting is applied only for
VulkanResourceManager.
---
 renderdoc/core/resource_manager.h      | 25 ++++++++++++++++++-------
 renderdoc/driver/vulkan/vk_manager.cpp | 10 ++++++++++
 renderdoc/driver/vulkan/vk_manager.h   |  1 +
 3 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/renderdoc/core/resource_manager.h b/renderdoc/core/resource_manager.h
index 7add1483c..5e3fedc4c 100644
--- a/renderdoc/core/resource_manager.h
+++ b/renderdoc/core/resource_manager.h
@@ -435,6 +435,7 @@ protected:
                                       WrappedResourceType res) = 0;
   virtual void Create_InitialState(ResourceId id, WrappedResourceType live, bool hasData) = 0;
   virtual void Apply_InitialState(WrappedResourceType live, InitialContentData initial) = 0;
+  virtual std::vector<ResourceId> InitialContentResources();
 
   // very coarse lock, protects EVERYTHING. This could certainly be improved and it may be a
   // bottleneck
@@ -764,21 +765,31 @@ template <typename Configuration>
 void ResourceManager<Configuration>::ApplyInitialContents()
 {
   RDCDEBUG("Applying initial contents");
-  uint32_t numContents = 0;
+  std::vector<ResourceId> resources = InitialContentResources();
+  for(auto it = resources.begin(); it != resources.end(); ++it)
+  {
+    ResourceId id = *it;
+    InitialContentData data = m_InitialContents[id];
+    WrappedResourceType live = GetLiveResource(id);
+    Apply_InitialState(live, data);
+  }
+  RDCDEBUG("Applied %d", (uint32_t)resources.size());
+}
+
+template <typename Configuration>
+std::vector<ResourceId> ResourceManager<Configuration>::InitialContentResources()
+{
+  std::vector<ResourceId> resources;
   for(auto it = m_InitialContents.begin(); it != m_InitialContents.end(); ++it)
   {
     ResourceId id = it->first;
 
     if(HasLiveResource(id))
     {
-      WrappedResourceType live = GetLiveResource(id);
-
-      numContents++;
-
-      Apply_InitialState(live, it->second);
+      resources.push_back(id);
     }
   }
-  RDCDEBUG("Applied %d", numContents);
+  return resources;
 }
 
 template <typename Configuration>
diff --git a/renderdoc/driver/vulkan/vk_manager.cpp b/renderdoc/driver/vulkan/vk_manager.cpp
index 3891866b9..46b24a497 100644
--- a/renderdoc/driver/vulkan/vk_manager.cpp
+++ b/renderdoc/driver/vulkan/vk_manager.cpp
@@ -612,6 +612,16 @@ void VulkanResourceManager::Apply_InitialState(WrappedVkRes *live, VkInitialCont
   return m_Core->Apply_InitialState(live, initial);
 }
 
+std::vector<ResourceId> VulkanResourceManager::InitialContentResources()
+{
+  std::vector<ResourceId> resources =
+      ResourceManager<VulkanResourceManagerConfiguration>::InitialContentResources();
+  std::sort(resources.begin(), resources.end(), [this](ResourceId a, ResourceId b) {
+    return m_InitialContents[a].type < m_InitialContents[b].type;
+  });
+  return resources;
+}
+
 bool VulkanResourceManager::ResourceTypeRelease(WrappedVkRes *res)
 {
   return m_Core->ReleaseResource(res);
diff --git a/renderdoc/driver/vulkan/vk_manager.h b/renderdoc/driver/vulkan/vk_manager.h
index 2984c3c65..75b8ac777 100644
--- a/renderdoc/driver/vulkan/vk_manager.h
+++ b/renderdoc/driver/vulkan/vk_manager.h
@@ -419,6 +419,7 @@ private:
   bool Serialise_InitialState(WriteSerialiser &ser, ResourceId resid, WrappedVkRes *res);
   void Create_InitialState(ResourceId id, WrappedVkRes *live, bool hasData);
   void Apply_InitialState(WrappedVkRes *live, VkInitialContents initial);
+  std::vector<ResourceId> InitialContentResources();
 
   CaptureState m_State;
   WrappedVulkan *m_Core;