Set previous/next pointers on marker drawcalls

* Even if marker drawcalls aren't themselves in the linked list, it's very
  convenient to have their previous/next link to the next node on the list
  itself, so that if you search and find one from the list of drawcalls you'll
  still be able to iterate normally.

renderdoc/core/replay_proxy.cpp

@@ -387,8 +387,7 @@ FrameRecord ReplayProxy::Proxied_GetFrameRecord(ParamSerialiser &paramser, Retur
   if(paramser.IsWriting())
   {
     // re-configure the drawcall pointers, since they will be invalid
-    DrawcallDescription *previous = NULL;
-    SetupDrawcallPointers(m_Drawcalls, ret.drawcallList, NULL, previous);
+    SetupDrawcallPointers(m_Drawcalls, ret.drawcallList);
   }
 
   return ret;

renderdoc/driver/d3d11/d3d11_device.cpp

@@ -1081,8 +1081,7 @@ ReplayStatus WrappedID3D11Device::ReadLogInitialisation(RDCFile *rdc, bool store
 
   if(!IsStructuredExporting(m_State))
   {
-    DrawcallDescription *previous = NULL;
-    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList, NULL, previous);
+    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList);
   }
 
 #if ENABLED(RDOC_DEVEL)

renderdoc/driver/d3d12/d3d12_device.cpp

@@ -2742,8 +2742,7 @@ ReplayStatus WrappedID3D12Device::ReadLogInitialisation(RDCFile *rdc, bool store
 
     m_Queue->GetParentDrawcall().children.clear();
 
-    DrawcallDescription *previous = NULL;
-    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList, NULL, previous);
+    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList);
 
     D3D12CommandData &cmd = *m_Queue->GetCommandData();
 

renderdoc/driver/gl/gl_driver.cpp

@@ -4523,8 +4523,7 @@ ReplayStatus WrappedOpenGL::ContextReplayLog(CaptureState readType, uint32_t sta
     GetFrameRecord().drawcallList = m_ParentDrawcall.children;
     GetFrameRecord().frameInfo.debugMessages = GetDebugMessages();
 
-    DrawcallDescription *previous = NULL;
-    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList, NULL, previous);
+    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList);
 
     // it's easier to remove duplicate usages here than check it as we go.
     // this means if textures are bound in multiple places in the same draw

renderdoc/driver/vulkan/vk_core.cpp

@@ -2031,8 +2031,7 @@ ReplayStatus WrappedVulkan::ContextReplayLog(CaptureState readType, uint32_t sta
   {
     GetFrameRecord().drawcallList = m_ParentDrawcall.Bake();
 
-    DrawcallDescription *previous = NULL;
-    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList, NULL, previous);
+    SetupDrawcallPointers(m_Drawcalls, GetFrameRecord().drawcallList);
 
     struct SortEID
     {

renderdoc/replay/replay_controller.cpp

@@ -496,9 +496,8 @@ void ReplayController::AddFakeMarkers()
   m_FrameRecord.drawcallList = ret;
 
   // re-configure the previous/next pointeres
-  DrawcallDescription *previous = NULL;
   m_Drawcalls.clear();
-  SetupDrawcallPointers(m_Drawcalls, m_FrameRecord.drawcallList, NULL, previous);
+  SetupDrawcallPointers(m_Drawcalls, m_FrameRecord.drawcallList);
 }
 
 rdcarray<CounterResult> ReplayController::FetchCounters(const rdcarray<GPUCounter> &counters)
@@ -1936,9 +1935,8 @@ ReplayStatus ReplayController::PostCreateInit(IReplayDriver *device, RDCFile *rd
   if(m_FrameRecord.drawcallList.empty())
     return ReplayStatus::APIReplayFailed;
 
-  DrawcallDescription *previous = NULL;
   m_Drawcalls.clear();
-  SetupDrawcallPointers(m_Drawcalls, m_FrameRecord.drawcallList, NULL, previous);
+  SetupDrawcallPointers(m_Drawcalls, m_FrameRecord.drawcallList);
 
   return ReplayStatus::Succeeded;
 }

renderdoc/replay/replay_driver.cpp

@@ -53,10 +53,16 @@ void DoSerialise(SerialiserType &ser, GetTextureDataParams &el)
 
 INSTANTIATE_SERIALISE_TYPE(GetTextureDataParams);
 
-DrawcallDescription *SetupDrawcallPointers(vector<DrawcallDescription *> &drawcallTable,
-                                           rdcarray<DrawcallDescription> &draws,
-                                           DrawcallDescription *parent,
-                                           DrawcallDescription *&previous)
+static bool PreviousNextExcludedMarker(DrawcallDescription *draw)
+{
+  return bool(draw->flags & (DrawFlags::PushMarker | DrawFlags::SetMarker | DrawFlags::MultiDraw |
+                             DrawFlags::APICalls));
+}
+
+static DrawcallDescription *SetupDrawcallPointers(vector<DrawcallDescription *> &drawcallTable,
+                                                  rdcarray<DrawcallDescription> &draws,
+                                                  DrawcallDescription *parent,
+                                                  DrawcallDescription *&previous)
 {
   DrawcallDescription *ret = NULL;
 
@@ -76,8 +82,7 @@ DrawcallDescription *SetupDrawcallPointers(vector<DrawcallDescription *> &drawca
 
       ret = SetupDrawcallPointers(drawcallTable, draw->children, draw, previous);
     }
-    else if(draw->flags & (DrawFlags::PushMarker | DrawFlags::SetMarker | DrawFlags::MultiDraw |
-                           DrawFlags::APICalls))
+    else if(PreviousNextExcludedMarker(draw))
     {
       // don't want to set up previous/next links for markers, but still add them to the table
       // Some markers like Present should have previous/next, but API Calls we also skip
@@ -113,6 +118,53 @@ DrawcallDescription *SetupDrawcallPointers(vector<DrawcallDescription *> &drawca
   return ret;
 }
 
+void SetupDrawcallPointers(std::vector<DrawcallDescription *> &drawcallTable,
+                           rdcarray<DrawcallDescription> &draws)
+{
+  DrawcallDescription *previous = NULL;
+  SetupDrawcallPointers(drawcallTable, draws, NULL, previous);
+
+  // markers don't enter the previous/next chain, but we still want pointers for them that point to
+  // the next or previous actual draw (skipping any markers). This means that draw->next->previous
+  // != draw sometimes, but it's more useful than draw->next being NULL in the middle of the list.
+  // This enables searching for a marker string and then being able to navigate from there and
+  // joining the 'real' linked list after one step.
+
+  previous = NULL;
+  std::vector<DrawcallDescription *> markers;
+
+  for(DrawcallDescription *draw : drawcallTable)
+  {
+    if(!draw)
+      continue;
+
+    bool marker = PreviousNextExcludedMarker(draw);
+
+    if(marker)
+    {
+      // point the previous pointer to the last non-marker draw we got. If we haven't hit one yet
+      // because this is near the start, this will just be NULL.
+      draw->previous = previous;
+
+      // because there can be multiple markers consecutively we want to point all of their nexts to
+      // the next draw we encounter. Accumulate this list, though in most cases it will only be 1
+      // long as it's uncommon to have multiple markers one after the other
+      markers.push_back(draw);
+    }
+    else
+    {
+      // the next markers we encounter should point their previous to this.
+      previous = draw;
+
+      // all previous markers point to this one
+      for(DrawcallDescription *m : markers)
+        m->next = draw;
+
+      markers.clear();
+    }
+  }
+}
+
 void PatchLineStripIndexBuffer(const DrawcallDescription *draw, uint8_t *idx8, uint16_t *idx16,
                                uint32_t *idx32, std::vector<uint32_t> &patchedIndices)
 {

renderdoc/replay/replay_driver.h

@@ -226,10 +226,8 @@ public:
 };
 
 // utility functions useful in any driver implementation
-DrawcallDescription *SetupDrawcallPointers(std::vector<DrawcallDescription *> &drawcallTable,
-                                           rdcarray<DrawcallDescription> &draws,
-                                           DrawcallDescription *parent,
-                                           DrawcallDescription *&previous);
+void SetupDrawcallPointers(std::vector<DrawcallDescription *> &drawcallTable,
+                           rdcarray<DrawcallDescription> &draws);
 
 // for hardware/APIs that can't do line rasterization, manually expand any triangle input topology
 // to a linestrip with strip restart indices.