Detect mapped memory writes to tiled images and skip. Closes #1863

* We track memory bindings to see which regions of a memory object are only used
  for tiled images, and discard any writes in case this was accidental detection
  of changes by the GPU which we don't want to replay. In the case of aliasing
  if there's linear and tiled resources then we still replay the writes.
* Note that we have to take a slower path involving a copy since we can't
  serialise straight into memory in this case, so applications should avoid
  mapping memory behind

renderdoc/driver/vulkan/vk_core.cpp

@@ -2382,6 +2382,9 @@ ReplayStatus WrappedVulkan::ReadLogInitialisation(RDCFile *rdc, bool storeStruct
 
   FreeAllMemory(MemoryScope::IndirectReadback);
 
+  for(auto it = m_CreationInfo.m_Memory.begin(); it != m_CreationInfo.m_Memory.end(); ++it)
+    it->second.SimplifyBindings();
+
   return ReplayStatus::Succeeded;
 }
 

renderdoc/driver/vulkan/vk_core.h

@@ -804,6 +804,8 @@ private:
   std::map<ResourceId, rdcarray<EventUsage>> m_ResourceUses;
   std::map<uint32_t, EventFlags> m_EventFlags;
 
+  bytebuf m_MaskedMapData;
+
   // returns thread-local temporary memory
   byte *GetTempMemory(size_t s);
   template <class T>

renderdoc/driver/vulkan/vk_info.cpp

@@ -986,6 +986,41 @@ void VulkanCreationInfo::Memory::Init(VulkanResourceManager *resourceMan, Vulkan
   size = pAllocInfo->allocationSize;
 }
 
+void VulkanCreationInfo::Memory::SimplifyBindings()
+{
+  // after initialisation we're likely to end up with a lot of gaps of 'none' in between tiled or
+  // linear resources. Regions of memory with no bindings are not visible in any meaningful way
+  // (memory can only be read with an image or buffer bound to it) so we perform a pass collapsing
+  // any 'None' intervals into the previous to be able to simplify the set of intervals. This means
+  // we might promote some regions to tiled, but that's fine since as above their contents are
+  // essentially meaningless.
+
+  // if the first entry is None and we have a second entry, then set the first to whatever the
+  // second is
+  if(bindings.size() > 1 && bindings.begin()->value() == VulkanCreationInfo::Memory::None)
+  {
+    auto it = bindings.begin();
+    it++;
+    bindings.begin()->setValue(it->value());
+  }
+
+  for(auto it = bindings.begin(); it != bindings.end(); it++)
+  {
+    // if we're not at the begining and the current range is None, copy whatever was in the previous
+    // range
+    if(it != bindings.begin() && it->value() == VulkanCreationInfo::Memory::None)
+    {
+      auto previt = it;
+      previt--;
+
+      it->setValue(previt->value());
+    }
+
+    // merge left when possible
+    it->mergeLeft();
+  }
+}
+
 void VulkanCreationInfo::Buffer::Init(VulkanResourceManager *resourceMan, VulkanCreationInfo &info,
                                       const VkBufferCreateInfo *pCreateInfo)
 {
@@ -1014,6 +1049,8 @@ void VulkanCreationInfo::Image::Init(VulkanResourceManager *resourceMan, VulkanC
   mipLevels = pCreateInfo->mipLevels;
   samples = RDCMAX(VK_SAMPLE_COUNT_1_BIT, pCreateInfo->samples);
 
+  linear = pCreateInfo->tiling == VK_IMAGE_TILING_LINEAR;
+
   creationFlags = TextureCategory::NoFlags;
 
   if(pCreateInfo->usage & VK_IMAGE_USAGE_SAMPLED_BIT)

renderdoc/driver/vulkan/vk_info.h

@@ -447,6 +447,24 @@ struct VulkanCreationInfo
     uint64_t size;
 
     VkBuffer wholeMemBuf;
+
+    enum MemoryBinding
+    {
+      None = 0x0,
+      Linear = 0x1,
+      Tiled = 0x2,
+      LinearAndTiled = 0x3,
+    };
+
+    Intervals<MemoryBinding> bindings;
+
+    void BindMemory(uint64_t offs, uint64_t sz, MemoryBinding b)
+    {
+      bindings.update(offs, offs + sz, b,
+                      [](MemoryBinding a, MemoryBinding b) { return MemoryBinding(a | b); });
+    }
+
+    void SimplifyBindings();
   };
   std::map<ResourceId, Memory> m_Memory;
 
@@ -484,6 +502,7 @@ struct VulkanCreationInfo
     uint32_t arrayLayers, mipLevels;
     VkSampleCountFlagBits samples;
 
+    bool linear;
     bool cube;
     TextureCategory creationFlags;
   };

renderdoc/driver/vulkan/wrappers/vk_resource_funcs.cpp

@@ -708,6 +708,8 @@ bool WrappedVulkan::Serialise_vkUnmapMemory(SerialiserType &ser, VkDevice device
   SERIALISE_ELEMENT(MapOffset);
   SERIALISE_ELEMENT(MapSize);
 
+  bool directStream = true;
+
   if(IsReplayingAndReading() && memory != VK_NULL_HANDLE)
   {
     if(IsLoading(m_State))
@@ -717,11 +719,78 @@ bool WrappedVulkan::Serialise_vkUnmapMemory(SerialiserType &ser, VkDevice device
                                               (void **)&MapData);
     if(vkr != VK_SUCCESS)
       RDCERR("Error mapping memory on replay: %s", ToStr(vkr).c_str());
+
+    const Intervals<VulkanCreationInfo::Memory::MemoryBinding> &bindings =
+        m_CreationInfo.m_Memory[GetResID(memory)].bindings;
+
+    uint64_t finish = MapOffset + MapSize;
+
+    auto it = bindings.find(MapOffset);
+
+    // iterate the bindings that this map region overlaps, if we overlap with any tiled memory we
+    // need to take the slow path
+    while(it->finish() < finish)
+    {
+      if(it->value() == VulkanCreationInfo::Memory::Tiled)
+      {
+        if(IsLoading(m_State))
+        {
+          AddDebugMessage(MessageCategory::Performance, MessageSeverity::Medium,
+                          MessageSource::GeneralPerformance,
+                          "Unmapped memory overlaps tiled-only memory region. "
+                          "Taking slow path to mask tiled memory writes");
+        }
+        directStream = false;
+        m_MaskedMapData.resize((size_t)MapSize);
+        break;
+      }
+
+      it++;
+    }
   }
 
-  // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise
-  // directly into upload memory
-  ser.Serialise("MapData"_lit, MapData, MapSize, SerialiserFlags::NoFlags);
+  if(directStream)
+  {
+    // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise
+    // directly into upload memory
+    ser.Serialise("MapData"_lit, MapData, MapSize, SerialiserFlags::NoFlags);
+  }
+  else
+  {
+    // serialise into temp storage
+    byte *tmp = m_MaskedMapData.data();
+    ser.Serialise("MapData"_lit, tmp, MapSize, SerialiserFlags::NoFlags);
+
+    const Intervals<VulkanCreationInfo::Memory::MemoryBinding> &bindings =
+        m_CreationInfo.m_Memory[GetResID(memory)].bindings;
+
+    uint64_t finish = MapOffset + MapSize;
+
+    auto it = bindings.find(MapOffset);
+
+    // iterate the bindings that this map region overlaps, and only memcpy the bits that we overlap
+    // which are linear
+    while(it->finish() < finish)
+    {
+      if(it->value() != VulkanCreationInfo::Memory::Tiled)
+      {
+        // start at the map offset or the region offset, whichever is *later*. E.g. if the region is
+        // larger than the map we only start where the map started, and vice-versa if the map
+        // started earlier than the region.
+        // We also rebase it so that it's relative to the map, so it's the byte offset for the
+        // memcpy
+        size_t offs = size_t(RDCMAX(it->start(), MapOffset) - MapOffset);
+
+        // similarly, only copy up to the end of the region or the end ofthe map whichever is
+        // *sooner*.
+        size_t size = size_t(RDCMIN(it->finish(), finish) - offs);
+
+        memcpy(MapData + offs, m_MaskedMapData.data() + offs, size);
+      }
+
+      it++;
+    }
+  }
 
   if(IsReplayingAndReading() && MapData && memory != VK_NULL_HANDLE)
     ObjDisp(device)->UnmapMemory(Unwrap(device), Unwrap(memory));
@@ -840,6 +909,8 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD
     MappedData = state->cpuReadPtr + (size_t)MemRange.offset;
   }
 
+  bool directStream = true;
+
   if(IsReplayingAndReading() && MemRange.memory != VK_NULL_HANDLE && MemRange.size > 0)
   {
     if(IsLoading(m_State))
@@ -851,11 +922,78 @@ bool WrappedVulkan::Serialise_vkFlushMappedMemoryRanges(SerialiserType &ser, VkD
                                    MemRange.size, 0, (void **)&MappedData);
     if(ret != VK_SUCCESS)
       RDCERR("Error mapping memory on replay: %s", ToStr(ret).c_str());
+
+    const Intervals<VulkanCreationInfo::Memory::MemoryBinding> &bindings =
+        m_CreationInfo.m_Memory[GetResID(MemRange.memory)].bindings;
+
+    uint64_t finish = MemRange.offset + MemRange.size;
+
+    auto it = bindings.find(MemRange.offset);
+
+    // iterate the bindings that this map region overlaps, if we overlap with any tiled memory we
+    // need to take the slow path
+    while(it->finish() < finish)
+    {
+      if(it->value() == VulkanCreationInfo::Memory::Tiled)
+      {
+        if(IsLoading(m_State))
+        {
+          AddDebugMessage(MessageCategory::Performance, MessageSeverity::Medium,
+                          MessageSource::GeneralPerformance,
+                          "Unmapped memory overlaps tiled-only memory region. "
+                          "Taking slow path to mask tiled memory writes");
+        }
+        directStream = false;
+        m_MaskedMapData.resize((size_t)MemRange.size);
+        break;
+      }
+
+      it++;
+    }
   }
 
-  // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise
-  // directly into upload memory
-  ser.Serialise("MappedData"_lit, MappedData, memRangeSize, SerialiserFlags::NoFlags);
+  if(directStream)
+  {
+    // not using SERIALISE_ELEMENT_ARRAY so we can deliberately avoid allocation - we serialise
+    // directly into upload memory
+    ser.Serialise("MappedData"_lit, MappedData, memRangeSize, SerialiserFlags::NoFlags);
+  }
+  else
+  {
+    // serialise into temp storage
+    byte *tmp = m_MaskedMapData.data();
+    ser.Serialise("MapData"_lit, tmp, MemRange.size, SerialiserFlags::NoFlags);
+
+    const Intervals<VulkanCreationInfo::Memory::MemoryBinding> &bindings =
+        m_CreationInfo.m_Memory[GetResID(MemRange.memory)].bindings;
+
+    uint64_t finish = MemRange.offset + MemRange.size;
+
+    auto it = bindings.find(MemRange.offset);
+
+    // iterate the bindings that this map region overlaps, and only memcpy the bits that we overlap
+    // which are linear
+    while(it->finish() < finish)
+    {
+      if(it->value() != VulkanCreationInfo::Memory::Tiled)
+      {
+        // start at the map offset or the region offset, whichever is *later*. E.g. if the region is
+        // larger than the map we only start where the map started, and vice-versa if the map
+        // started earlier than the region.
+        // We also rebase it so that it's relative to the map, so it's the byte offset for the
+        // memcpy
+        size_t offs = size_t(RDCMAX(it->start(), MemRange.offset) - MemRange.offset);
+
+        // similarly, only copy up to the end of the region or the end ofthe map whichever is
+        // *sooner*.
+        size_t size = size_t(RDCMIN(it->finish(), finish) - offs);
+
+        memcpy(MappedData + offs, m_MaskedMapData.data() + offs, size);
+      }
+
+      it++;
+    }
+  }
 
   if(IsReplayingAndReading() && MappedData && MemRange.memory != VK_NULL_HANDLE && MemRange.size > 0)
     ObjDisp(device)->UnmapMemory(Unwrap(device), Unwrap(MemRange.memory));
@@ -1053,6 +1191,9 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory(SerialiserType &ser, VkDevice d
       else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address)
         bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo);
     }
+
+    m_CreationInfo.m_Memory[GetResID(memory)].BindMemory(memoryOffset, mrq.size,
+                                                         VulkanCreationInfo::Memory::Linear);
   }
 
   return true;
@@ -1153,6 +1294,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory(SerialiserType &ser, VkDevice de
 
     AddResourceCurChunk(memOrigId);
     AddResourceCurChunk(resOrigId);
+
+    VulkanCreationInfo::Image &imgInfo = m_CreationInfo.m_Image[GetResID(image)];
+    m_CreationInfo.m_Memory[GetResID(memory)].BindMemory(
+        memoryOffset, mrq.size,
+        imgInfo.linear ? VulkanCreationInfo::Memory::Linear : VulkanCreationInfo::Memory::Tiled);
   }
 
   return true;
@@ -2170,6 +2316,9 @@ bool WrappedVulkan::Serialise_vkBindBufferMemory2(SerialiserType &ser, VkDevice
         else if(GetExtensions(GetRecord(device)).ext_EXT_buffer_device_address)
           bufInfo.gpuAddress = ObjDisp(device)->GetBufferDeviceAddressEXT(Unwrap(device), &getInfo);
       }
+
+      m_CreationInfo.m_Memory[GetResID(bindInfo.memory)].BindMemory(
+          bindInfo.memoryOffset, mrq.size, VulkanCreationInfo::Memory::Linear);
     }
 
     VkBindBufferMemoryInfo *unwrapped = UnwrapInfos(pBindInfos, bindInfoCount);
@@ -2283,6 +2432,11 @@ bool WrappedVulkan::Serialise_vkBindImageMemory2(SerialiserType &ser, VkDevice d
 
       AddResourceCurChunk(memOrigId);
       AddResourceCurChunk(resOrigId);
+
+      VulkanCreationInfo::Image &imgInfo = m_CreationInfo.m_Image[GetResID(bindInfo.image)];
+      m_CreationInfo.m_Memory[GetResID(bindInfo.memory)].BindMemory(
+          bindInfo.memoryOffset, mrq.size,
+          imgInfo.linear ? VulkanCreationInfo::Memory::Linear : VulkanCreationInfo::Memory::Tiled);
     }
 
     VkBindImageMemoryInfo *unwrapped = UnwrapInfos(pBindInfos, bindInfoCount);