Ensure reference data for map comparisons is taken from serialised data

* If we serialise from the map, and then separately memcpy from it to populate
  the reference data then we risk another thread's writes happening in between
  and not being detected on subsequence checks. Instead we need to copy from the
  serialised data, that way regardless of what we snapshot there we always
  compare against it to detect any further writes.

renderdoc/driver/d3d12/d3d12_command_queue_wrap.cpp

@@ -920,8 +920,6 @@ void WrappedID3D12CommandQueue::ExecuteCommandListsInternal(UINT NumCommandLists
 
             D3D12_RANGE range = {diffStart, diffEnd};
 
-            m_pDevice->MapDataWrite(res, subres, data, range);
-
             if(ref == NULL)
             {
               res->AllocShadow(subres, size);
@@ -929,8 +927,9 @@ void WrappedID3D12CommandQueue::ExecuteCommandListsInternal(UINT NumCommandLists
               ref = res->GetShadow(subres);
             }
 
-            // update comparison shadow for next time
-            memcpy(ref, data, size);
+            // passing true here asks the serialisation function to update the shadow pointer for
+            // this resource
+            m_pDevice->MapDataWrite(res, subres, data, range, true);
 
             GetResourceManager()->MarkDirtyResource(res->GetResourceID());
           }

renderdoc/driver/d3d12/d3d12_commands.cpp

@@ -792,8 +792,9 @@ bool WrappedID3D12CommandQueue::ProcessChunk(ReadSerialiser &ser, D3D12Chunk chu
     case D3D12Chunk::PopMarker: ret = m_ReplayList->Serialise_EndEvent(ser); break;
     case D3D12Chunk::SetMarker: ret = m_ReplayList->Serialise_SetMarker(ser, 0, NULL, 0); break;
 
+    case D3D12Chunk::CoherentMapWrite:
     case D3D12Chunk::Resource_Unmap:
-      ret = m_pDevice->Serialise_MapDataWrite(ser, NULL, 0, NULL, D3D12_RANGE());
+      ret = m_pDevice->Serialise_MapDataWrite(ser, NULL, 0, NULL, D3D12_RANGE(), false);
       break;
     case D3D12Chunk::Resource_WriteToSubresource:
       ret = m_pDevice->Serialise_WriteToSubresource(ser, NULL, 0, NULL, NULL, 0, 0);

renderdoc/driver/d3d12/d3d12_common.h

@@ -902,5 +902,6 @@ enum class D3D12Chunk : uint32_t
   Device_CreateCommittedResource2,
   Device_CreatePlacedResource1,
   Device_CreateCommandQueue1,
+  CoherentMapWrite,
   Max,
 };

renderdoc/driver/d3d12/d3d12_device.cpp

@@ -1694,13 +1694,13 @@ void WrappedID3D12Device::Unmap(ID3D12Resource *Resource, UINT Subresource, byte
   }
 
   if(capframe)
-    MapDataWrite(Resource, Subresource, mapPtr, range);
+    MapDataWrite(Resource, Subresource, mapPtr, range, false);
 }
 
 template <typename SerialiserType>
 bool WrappedID3D12Device::Serialise_MapDataWrite(SerialiserType &ser, ID3D12Resource *Resource,
                                                  UINT Subresource, byte *MappedData,
-                                                 D3D12_RANGE range)
+                                                 D3D12_RANGE range, bool coherentFlush)
 {
   SERIALISE_ELEMENT(Resource).Important();
   SERIALISE_ELEMENT(Subresource);
@@ -1737,12 +1737,31 @@ bool WrappedID3D12Device::Serialise_MapDataWrite(SerialiserType &ser, ID3D12Reso
 
   ser.Serialise("MappedData"_lit, MappedData, range.End - range.Begin, flags).Important();
 
+  size_t dataOffset = 0;
+  if(ser.IsWriting())
+    dataOffset = ser.GetWriter()->GetOffset() - (range.End - range.Begin);
+
   SERIALISE_ELEMENT(range);
 
   uint64_t rangeSize = range.End - range.Begin;
 
   SERIALISE_CHECK_READ_ERRORS();
 
+  if(ser.IsWriting() && coherentFlush)
+  {
+    byte *ref = GetWrapped(Resource)->GetShadow(Subresource);
+    const byte *serialisedData = ser.GetWriter()->GetData() + dataOffset;
+
+    if(ref)
+    {
+      memcpy(ref + range.Begin, serialisedData, size_t(range.End - range.Begin));
+    }
+    else
+    {
+      RDCERR("Shadow data not allocated when coherent map flush is processed!");
+    }
+  }
+
   // don't do anything if end <= begin because the range is empty.
   if(IsReplayingAndReading() && Resource && range.End > range.Begin)
   {
@@ -1824,19 +1843,21 @@ bool WrappedID3D12Device::Serialise_MapDataWrite(SerialiserType &ser, ID3D12Reso
 }
 
 template bool WrappedID3D12Device::Serialise_MapDataWrite(ReadSerialiser &ser,
-                                                          ID3D12Resource *Resource, UINT Subresource,
-                                                          byte *MappedData, D3D12_RANGE range);
+                                                          ID3D12Resource *Resource,
+                                                          UINT Subresource, byte *MappedData,
+                                                          D3D12_RANGE range, bool coherentFlush);
 template bool WrappedID3D12Device::Serialise_MapDataWrite(WriteSerialiser &ser,
-                                                          ID3D12Resource *Resource, UINT Subresource,
-                                                          byte *MappedData, D3D12_RANGE range);
+                                                          ID3D12Resource *Resource,
+                                                          UINT Subresource, byte *MappedData,
+                                                          D3D12_RANGE range, bool coherentFlush);
 
 void WrappedID3D12Device::MapDataWrite(ID3D12Resource *Resource, UINT Subresource, byte *mapPtr,
-                                       D3D12_RANGE range)
+                                       D3D12_RANGE range, bool coherentFlush)
 {
   CACHE_THREAD_SERIALISER();
 
-  SCOPED_SERIALISE_CHUNK(D3D12Chunk::Resource_Unmap);
-  Serialise_MapDataWrite(ser, Resource, Subresource, mapPtr, range);
+  SCOPED_SERIALISE_CHUNK(coherentFlush ? D3D12Chunk::CoherentMapWrite : D3D12Chunk::Resource_Unmap);
+  Serialise_MapDataWrite(ser, Resource, Subresource, mapPtr, range, coherentFlush);
 
   m_FrameCaptureRecord->AddChunk(scope.Get());
 
@@ -3814,6 +3835,7 @@ bool WrappedID3D12Device::ProcessChunk(ReadSerialiser &ser, D3D12Chunk context)
     case D3D12Chunk::List_IndirectSubCommand:
     case D3D12Chunk::Swapchain_Present:
     case D3D12Chunk::List_ClearState:
+    case D3D12Chunk::CoherentMapWrite:
       RDCERR("Unexpected chunk while processing initialisation: %s", ToStr(context).c_str());
       return false;
 

renderdoc/driver/d3d12/d3d12_device.h

@@ -1072,8 +1072,8 @@ public:
     return E_NOINTERFACE;
   }
 
-  IMPLEMENT_FUNCTION_THREAD_SERIALISED(void, MapDataWrite, ID3D12Resource *Resource,
-                                       UINT Subresource, byte *mapPtr, D3D12_RANGE range);
+  IMPLEMENT_FUNCTION_THREAD_SERIALISED(void, MapDataWrite, ID3D12Resource *Resource, UINT Subresource,
+                                       byte *mapPtr, D3D12_RANGE range, bool coherentFlush);
   IMPLEMENT_FUNCTION_THREAD_SERIALISED(void, WriteToSubresource, ID3D12Resource *Resource,
                                        UINT Subresource, const D3D12_BOX *pDstBox,
                                        const void *pSrcData, UINT SrcRowPitch, UINT SrcDepthPitch);

renderdoc/driver/d3d12/d3d12_stringise.cpp

@@ -28,7 +28,7 @@
 template <>
 rdcstr DoStringise(const D3D12Chunk &el)
 {
-  RDCCOMPILE_ASSERT((uint32_t)D3D12Chunk::Max == 1113, "Chunks changed without updating names");
+  RDCCOMPILE_ASSERT((uint32_t)D3D12Chunk::Max == 1114, "Chunks changed without updating names");
 
   BEGIN_ENUM_STRINGISE(D3D12Chunk)
   {
@@ -204,6 +204,7 @@ rdcstr DoStringise(const D3D12Chunk &el)
     STRINGISE_ENUM_CLASS_NAMED(Device_CreatePlacedResource1,
                                "ID3D12Device8::CreatePlacedResource1");
     STRINGISE_ENUM_CLASS_NAMED(Device_CreateCommandQueue1, "ID3D12Device9::CreateCommandQueue1");
+    STRINGISE_ENUM_CLASS_NAMED(CoherentMapWrite, "Internal::Coherent Mapped Memory Write");
     STRINGISE_ENUM_CLASS_NAMED(Max, "Max Chunk");
   }
   END_ENUM_STRINGISE()