Update fake markers to work with new event browser model

* Fake markers now need a real event ID. We don't want to have to remap all
  events in a capture between UI and replay driver, so instead we assign non-
  contiguous events above the normal range and expect the UI to handle it.

qrenderdoc/Code/RGPInterop.cpp

@@ -256,7 +256,8 @@ void RGPInterop::CreateMapping(const rdcarray<DrawcallDescription> &drawcalls)
   {
     for(const APIEvent &ev : draw.events)
     {
-      if(ev.chunkIndex == 0 || ev.chunkIndex >= file.chunks.size())
+      if(ev.chunkIndex == 0 || ev.chunkIndex == APIEvent::NoChunk ||
+         ev.chunkIndex >= file.chunks.size())
         continue;
 
       const SDChunk *chunk = file.chunks[ev.chunkIndex];

qrenderdoc/Windows/APIInspector.cpp

@@ -210,9 +210,19 @@ void APIInspector::fillAPIView()
 
   if(draw != NULL && !draw->events.isEmpty())
   {
-    for(const APIEvent &ev : draw->events)
+    if(draw->IsFakeMarker())
     {
-      addEvent(ev, ev.eventId == draw->eventId);
+      RDTreeWidgetItem *root = new RDTreeWidgetItem({lit("---"), QString(draw->name)});
+      root->setBold(true);
+      ui->apiEvents->addTopLevelItem(root);
+      ui->apiEvents->setSelectedItem(root);
+    }
+    else
+    {
+      for(const APIEvent &ev : draw->events)
+      {
+        addEvent(ev, ev.eventId == draw->eventId);
+      }
     }
   }
   else
@@ -228,6 +238,9 @@ void APIInspector::fillAPIView()
 
 void APIInspector::addEvent(const APIEvent &ev, bool primary)
 {
+  if(ev.chunkIndex == APIEvent::NoChunk)
+    return;
+
   const SDFile &file = m_Ctx.GetStructuredFile();
 
   RDTreeWidgetItem *root = new RDTreeWidgetItem({QString::number(ev.eventId), QString()});

qrenderdoc/Windows/EventBrowser.cpp

@@ -307,6 +307,14 @@ struct EventItemModel : public QAbstractItemModel
         // the first draw with an EID greater than the one we're searching for should contain it.
         if(d.eventId >= eid)
         {
+          // except if the draw is a fake marker. In this case its own event ID is invalid, so we
+          // check the range of its children (knowing it only has one layer of children)
+          if(d.IsFakeMarker() && d.children[0].eventId < eid)
+          {
+            rowInParent++;
+            continue;
+          }
+
           // keep counting until we get to the row within this draw
           for(size_t i = 0; i < d.events.size(); i++)
           {
@@ -745,7 +753,8 @@ private:
         m_Draws.resize_for_index(e.eventId);
         m_Chunks.resize_for_index(e.eventId);
         m_Draws[e.eventId] = &d;
-        m_Chunks[e.eventId] = sdfile.chunks[e.chunkIndex];
+        if(e.chunkIndex != APIEvent::NoChunk && e.chunkIndex < sdfile.chunks.size())
+          m_Chunks[e.eventId] = sdfile.chunks[e.chunkIndex];
       }
 
       row += d.events.count();
@@ -844,7 +853,7 @@ private:
 
     QString name;
 
-    if(draw->eventId == eid)
+    if(draw->eventId == eid || draw->IsFakeMarker())
     {
       name = draw->name;
     }
@@ -2067,6 +2076,9 @@ void EventBrowser::events_currentChanged(const QModelIndex &current, const QMode
 
   const DrawcallDescription *draw = m_Ctx.GetDrawcall(effectiveEID);
 
+  if(draw && draw->IsFakeMarker())
+    draw = &draw->children.back();
+
   ui->stepPrev->setEnabled(draw && draw->previous);
   ui->stepNext->setEnabled(draw && draw->next);
 

renderdoc/api/replay/data_types.h

@@ -701,7 +701,12 @@ struct TextureDescription
 
 DECLARE_REFLECTION_STRUCT(TextureDescription);
 
-DOCUMENT("An individual API-level event, generally corresponds one-to-one with an API call.");
+DOCUMENT(R"(An individual API-level event, generally corresponds one-to-one with an API call.
+
+.. data:: NoChunk
+
+  No chunk is available.
+)");
 struct APIEvent
 {
   DOCUMENT("");
@@ -727,7 +732,11 @@ of results part way through the multi draw.
 )");
   uint32_t eventId = 0;
 
-  DOCUMENT("The chunk index for this function call in the structured file.");
+  DOCUMENT(R"(The chunk index for this function call in the structured file.
+
+If no chunk index is available this will be set to :data:`NoChunk`. This will only happen for fake
+markers added to the capture after load.
+)");
   uint32_t chunkIndex = 0;
 
   DOCUMENT(R"(A byte offset in the data stream where this event happens.
@@ -736,6 +745,8 @@ of results part way through the multi draw.
   the start of the file on disk.
 )");
   uint64_t fileOffset = 0;
+
+  static const uint32_t NoChunk = ~0U;
 };
 
 DECLARE_REFLECTION_STRUCT(APIEvent);
@@ -1534,45 +1545,31 @@ DOCUMENT("Describes the properties of a drawcall, dispatch, debug marker, or sim
 struct DrawcallDescription
 {
   DOCUMENT("");
-  DrawcallDescription() { Reset(); }
+  DrawcallDescription() = default;
   DrawcallDescription(const DrawcallDescription &) = default;
   DrawcallDescription &operator=(const DrawcallDescription &) = default;
 
-  DOCUMENT("Resets the drawcall back to a default/empty state.");
-  void Reset()
+  DOCUMENT(R"(Returns whether or not this drawcall corresponds to a fake marker added by
+:meth:`ReplayController.AddFakeMarkers`.
+
+Such draws may break expectations of event IDs and drawcall IDs, so it is recommended to avoid
+processing them wherever possible.
+
+:return: Returns whether or not this drawcall is a fake marker.
+:rtype: bool
+)");
+  bool IsFakeMarker() const
   {
-    eventId = 0;
-    drawcallId = 0;
-    flags = DrawFlags::NoFlags;
-    markerColor = FloatVector();
-    numIndices = 0;
-    numInstances = 0;
-    indexOffset = 0;
-    baseVertex = 0;
-    vertexOffset = 0;
-    instanceOffset = 0;
-    drawIndex = 0;
-
-    dispatchDimension[0] = dispatchDimension[1] = dispatchDimension[2] = 0;
-    dispatchThreadsDimension[0] = dispatchThreadsDimension[1] = dispatchThreadsDimension[2] = 0;
-    dispatchBase[0] = dispatchBase[1] = dispatchBase[2] = 0;
-
-    copySource = ResourceId();
-    copyDestination = ResourceId();
-
-    parent = previous = next = NULL;
-
-    for(int i = 0; i < 8; i++)
-      outputs[i] = ResourceId();
-    depthOut = ResourceId();
+    return events.size() == 1 && events[0].chunkIndex == APIEvent::NoChunk;
   }
+
   DOCUMENT("");
   bool operator==(const DrawcallDescription &o) const { return eventId == o.eventId; }
   bool operator<(const DrawcallDescription &o) const { return eventId < o.eventId; }
   DOCUMENT("The :data:`eventId <APIEvent.eventId>` that actually produced the drawcall.");
-  uint32_t eventId;
+  uint32_t eventId = 0;
   DOCUMENT("A 1-based index of this drawcall relative to other drawcalls.");
-  uint32_t drawcallId;
+  uint32_t drawcallId = 0;
 
   DOCUMENT(R"(The name of this drawcall. Typically a summarised/concise list of parameters.
 
@@ -1582,7 +1579,7 @@ struct DrawcallDescription
   rdcstr name;
 
   DOCUMENT("A set of :class:`DrawFlags` properties describing what kind of drawcall this is.");
-  DrawFlags flags;
+  DrawFlags flags = DrawFlags::NoFlags;
 
   DOCUMENT(R"(A RGBA color specified by a debug marker call.
 
@@ -1591,47 +1588,47 @@ struct DrawcallDescription
   FloatVector markerColor;
 
   DOCUMENT("The number of indices or vertices as appropriate for the drawcall. 0 if not used.");
-  uint32_t numIndices;
+  uint32_t numIndices = 0;
 
   DOCUMENT("The number of instances for the drawcall. 0 if not used.");
-  uint32_t numInstances;
+  uint32_t numInstances = 0;
 
   DOCUMENT("For indexed drawcalls, the offset added to each index after fetching.");
-  int32_t baseVertex;
+  int32_t baseVertex = 0;
 
   DOCUMENT("For indexed drawcalls, the first index to fetch from the index buffer.");
-  uint32_t indexOffset;
+  uint32_t indexOffset = 0;
 
   DOCUMENT("For non-indexed drawcalls, the offset applied before looking up each vertex input.");
-  uint32_t vertexOffset;
+  uint32_t vertexOffset = 0;
 
   DOCUMENT(
       "For instanced drawcalls, the offset applied before looking up instanced vertex inputs.");
-  uint32_t instanceOffset;
+  uint32_t instanceOffset = 0;
 
   DOCUMENT(R"(The index of this draw in an call with multiple draws, e.g. an indirect draw.
 
 0 if not part of a multi-draw.
 )");
-  uint32_t drawIndex;
+  uint32_t drawIndex = 0;
 
   DOCUMENT(R"(The 3D number of workgroups to dispatch in a dispatch call.
 
 :type: Tuple[int,int,int]
 )");
-  rdcfixedarray<uint32_t, 3> dispatchDimension;
+  rdcfixedarray<uint32_t, 3> dispatchDimension = {0, 0, 0};
 
   DOCUMENT(R"(The 3D size of each workgroup in threads if the call allows an override, or 0 if not.
 
 :type: Tuple[int,int,int]
 )");
-  rdcfixedarray<uint32_t, 3> dispatchThreadsDimension;
+  rdcfixedarray<uint32_t, 3> dispatchThreadsDimension = {0, 0, 0};
 
   DOCUMENT(R"(The 3D base offset of the workgroup ID if the call allows an override, or 0 if not.
 
 :type: Tuple[int,int,int]
 )");
-  rdcfixedarray<uint32_t, 3> dispatchBase;
+  rdcfixedarray<uint32_t, 3> dispatchBase = {0, 0, 0};
 
   DOCUMENT(R"(The :class:`ResourceId` identifying the source object in a copy, resolve or blit
 operation.
@@ -1659,19 +1656,19 @@ operation.
 
 :type: DrawcallDescription
 )");
-  const DrawcallDescription *parent;
+  const DrawcallDescription *parent = NULL;
 
   DOCUMENT(R"(The previous drawcall in the frame, or ``None`` if this is the first drawcall in the
 frame.
 
 :type: DrawcallDescription
 )");
-  const DrawcallDescription *previous;
+  const DrawcallDescription *previous = NULL;
   DOCUMENT(R"(The next drawcall in the frame, or ``None`` if this is the last drawcall in the frame.
 
 :type: DrawcallDescription
 )");
-  const DrawcallDescription *next;
+  const DrawcallDescription *next = NULL;
 
   DOCUMENT(R"(An 8-tuple of the :class:`ResourceId` ids for the color outputs, which can be used
 for very coarse bucketing of drawcalls into similar passes by their outputs.

renderdoc/api/replay/renderdoc_replay.h

@@ -680,7 +680,19 @@ See :meth:`BuildTargetShader`.
   virtual const SDFile &GetStructuredFile() = 0;
 
   DOCUMENT(R"(Add fake marker regions to the list of drawcalls in the capture, based on which
-textures are bound as outputs.
+textures are bound as outputs. Will not do anything if the capture already contains user marker
+regions.
+
+.. warning::
+  This must be called *immediately* after capture load, calling it at a later time will cause
+  corruption. No other functions should be called between load and this one.
+
+.. note::
+  The event IDs for fake marker pushes and pops will not be contiguous with the surrounding draws
+  and will be set to values above the last real event in the capture. This also means they break the
+  typical rules that event IDs always increase. It's recommended that these events are not
+  referenced directly in other calls such as SetFrameEvent, and fake markers should be used 
+  sparingly at all compared to proper application-provided markers.
 )");
   virtual void AddFakeMarkers() = 0;
 

renderdoc/replay/replay_controller.cpp

@@ -72,6 +72,11 @@ void ReplayController::SetFrameEvent(uint32_t eventId, bool force)
   CHECK_REPLAY_THREAD();
   RENDERDOC_PROFILEFUNCTION();
 
+  // use remapped event if there's a match
+  auto it = m_EventRemap.find(eventId);
+  if(it != m_EventRemap.end())
+    eventId = it->second;
+
   if(eventId != m_EventID || force)
   {
     m_EventID = eventId;
@@ -288,6 +293,9 @@ void ReplayController::AddFakeMarkers()
   if(ContainsMarker(draws))
     return;
 
+  uint32_t newEventId = draws.back().events.back().eventId + 1;
+  uint32_t newDrawcallId = draws.back().drawcallId + 1;
+
   rdcarray<DrawcallDescription> ret;
 
   int depthpassID = 1;
@@ -348,15 +356,13 @@ void ReplayController::AddFakeMarkers()
 
     DrawcallDescription mark;
 
-    mark.eventId = draws[start].eventId;
-    mark.drawcallId = draws[start].drawcallId;
+    mark.eventId = newEventId++;
+    mark.drawcallId = newDrawcallId++;
 
     mark.flags = DrawFlags::PushMarker;
     mark.outputs = draws[end].outputs;
     mark.depthOut = draws[end].depthOut;
 
-    mark.name = "Guessed Pass";
-
     minOutCount = RDCMAX(1, minOutCount);
 
     const char *targets = draws[end].depthOut == ResourceId() ? "Targets" : "Targets + Depth";
@@ -378,6 +384,16 @@ void ReplayController::AddFakeMarkers()
     for(int j = start; j <= end; j++)
       mark.children[j - start] = draws[j];
 
+    APIEvent ev;
+    ev.eventId = mark.eventId;
+    ev.fileOffset = mark.children[0].events.back().fileOffset;
+    ev.chunkIndex = APIEvent::NoChunk;
+    mark.events.push_back(ev);
+
+    // when this event is selected, instead select the first the event before the first child's
+    // first event. This is effectively what would be the case if there was a real marker here.
+    m_EventRemap[mark.eventId] = mark.children[0].events[0].eventId - 1;
+
     ret.push_back(mark);
 
     start = i;

renderdoc/replay/replay_controller.h

@@ -242,6 +242,8 @@ private:
 
   uint32_t m_EventID;
 
+  std::map<uint32_t, uint32_t> m_EventRemap;
+
   const D3D11Pipe::State *m_D3D11PipelineState;
   const D3D12Pipe::State *m_D3D12PipelineState;
   const GLPipe::State *m_GLPipelineState;

renderdoc/replay/replay_driver.cpp

@@ -94,7 +94,8 @@ static DrawcallDescription *SetupDrawcallPointers(rdcarray<DrawcallDescription *
     if(!draw->children.empty())
     {
       {
-        RDCASSERT(drawcallTable.empty() || draw->eventId > drawcallTable.back()->eventId);
+        RDCASSERT(drawcallTable.empty() || draw->eventId > drawcallTable.back()->eventId ||
+                  drawcallTable.back()->IsFakeMarker());
         drawcallTable.resize(RDCMAX(drawcallTable.size(), size_t(draw->eventId) + 1));
         drawcallTable[draw->eventId] = draw;
       }
@@ -107,10 +108,9 @@ static DrawcallDescription *SetupDrawcallPointers(rdcarray<DrawcallDescription *
       // Some markers like Present should have previous/next, but API Calls we also skip
 
       {
-        // we also allow equal EIDs for fake markers that don't have their own EIDs
+        // we also allow non-contiguous EIDs for fake markers that have high EIDs
         RDCASSERT(drawcallTable.empty() || draw->eventId > drawcallTable.back()->eventId ||
-                  (draw->eventId == drawcallTable.back()->eventId &&
-                   (drawcallTable.back()->flags & DrawFlags::PushMarker)));
+                  drawcallTable.back()->IsFakeMarker());
         drawcallTable.resize(RDCMAX(drawcallTable.size(), size_t(draw->eventId) + 1));
         drawcallTable[draw->eventId] = draw;
       }
@@ -122,10 +122,9 @@ static DrawcallDescription *SetupDrawcallPointers(rdcarray<DrawcallDescription *
       draw->previous = previous;
 
       {
-        // we also allow equal EIDs for fake markers that don't have their own EIDs
+        // we also allow non-contiguous EIDs for fake markers that have high EIDs
         RDCASSERT(drawcallTable.empty() || draw->eventId > drawcallTable.back()->eventId ||
-                  (draw->eventId == drawcallTable.back()->eventId &&
-                   (drawcallTable.back()->flags & DrawFlags::PushMarker)));
+                  drawcallTable.back()->IsFakeMarker());
         drawcallTable.resize(RDCMAX(drawcallTable.size(), size_t(draw->eventId) + 1));
         drawcallTable[draw->eventId] = draw;
       }