Move bookmark storage centrally and save it with capture. Refs #501

qrenderdoc/Code/CaptureContext.cpp

@@ -323,6 +323,13 @@ void CaptureContext::LoadCaptureThreaded(const QString &captureFile, const QStri
       bytebuf buf = access->GetSectionContents(idx);
       LoadRenames(QString::fromUtf8((const char *)buf.data(), buf.count()));
     }
+
+    idx = access->FindSectionByType(SectionType::Bookmarks);
+    if(idx >= 0)
+    {
+      bytebuf buf = access->GetSectionContents(idx);
+      LoadBookmarks(QString::fromUtf8((const char *)buf.data(), buf.count()));
+    }
   }
 
   m_LoadInProgress = false;
@@ -618,6 +625,14 @@ bool CaptureContext::SaveCaptureTo(const QString &captureFile)
 
     Replay().GetCaptureAccess()->WriteSection(props, SaveRenames().toUtf8());
   }
+  if(m_CaptureMods & CaptureModifications::Bookmarks)
+  {
+    SectionProperties props;
+    props.type = SectionType::Bookmarks;
+    props.version = 1;
+
+    Replay().GetCaptureAccess()->WriteSection(props, SaveBookmarks().toUtf8());
+  }
 
   m_CaptureMods = CaptureModifications::NoModifications;
 
@@ -645,6 +660,7 @@ void CaptureContext::CloseCapture()
   m_ResourceList.clear();
 
   m_CustomNames.clear();
+  m_Bookmarks.clear();
 
   m_Drawcalls.clear();
   m_FirstDrawcall = m_LastDrawcall = NULL;
@@ -722,6 +738,38 @@ void CaptureContext::AddMessages(const rdcarray<DebugMessage> &msgs)
   }
 }
 
+void CaptureContext::SetBookmark(const EventBookmark &mark)
+{
+  int index = m_Bookmarks.indexOf(mark);
+  if(index >= 0)
+  {
+    // ignore no-op bookmarks
+    if(m_Bookmarks[index].text == mark.text)
+      return;
+
+    m_Bookmarks[index] = mark;
+  }
+  else
+  {
+    m_Bookmarks.push_back(mark);
+  }
+
+  m_CaptureMods |= CaptureModifications::Bookmarks;
+  m_MainWindow->captureModified();
+
+  RefreshUIStatus({}, true, true);
+}
+
+void CaptureContext::RemoveBookmark(uint32_t EID)
+{
+  m_Bookmarks.removeOne(EventBookmark(EID));
+
+  m_CaptureMods |= CaptureModifications::Bookmarks;
+  m_MainWindow->captureModified();
+
+  RefreshUIStatus({}, true, true);
+}
+
 QString CaptureContext::SaveRenames()
 {
   QVariantMap resources;
@@ -764,6 +812,46 @@ void CaptureContext::LoadRenames(const QString &data)
   }
 }
 
+QString CaptureContext::SaveBookmarks()
+{
+  QVariantList bookmarks;
+  for(const EventBookmark &mark : m_Bookmarks)
+  {
+    QVariantMap variantmark;
+    variantmark[lit("EID")] = mark.EID;
+    variantmark[lit("text")] = mark.text;
+
+    bookmarks.push_back(variantmark);
+  }
+
+  QVariantMap root;
+  root[lit("Bookmarks")] = bookmarks;
+
+  return VariantToJSON(root);
+}
+
+void CaptureContext::LoadBookmarks(const QString &data)
+{
+  QVariantMap root = JSONToVariant(data);
+
+  if(root.contains(lit("Bookmarks")))
+  {
+    QVariantList bookmarks = root[lit("Bookmarks")].toList();
+
+    for(QVariant v : bookmarks)
+    {
+      QVariantMap variantmark = v.toMap();
+
+      EventBookmark mark;
+      mark.EID = variantmark[lit("EID")].toUInt();
+      mark.text = variantmark[lit("text")].toString();
+
+      if(mark.EID != 0)
+        m_Bookmarks.push_back(mark);
+    }
+  }
+}
+
 QString CaptureContext::GetResourceName(ResourceId id)
 {
   if(id == ResourceId())

qrenderdoc/Code/CaptureContext.h

@@ -140,6 +140,10 @@ public:
   void MarkMessagesRead() override { m_UnreadMessageCount = 0; }
   void AddMessages(const rdcarray<DebugMessage> &msgs) override;
 
+  QList<EventBookmark> GetBookmarks() override { return m_Bookmarks; }
+  void SetBookmark(const EventBookmark &mark) override;
+  void RemoveBookmark(uint32_t EID) override;
+
   IMainWindow *GetMainWindow() override;
   IEventBrowser *GetEventBrowser() override;
   IAPIInspector *GetAPIInspector() override;
@@ -246,6 +250,9 @@ private:
   QString SaveRenames();
   void LoadRenames(const QString &data);
 
+  QString SaveBookmarks();
+  void LoadBookmarks(const QString &data);
+
   float m_LoadProgress = 0.0f;
   float m_PostloadProgress = 0.0f;
   float UpdateLoadProgress();
@@ -290,6 +297,8 @@ private:
   QMap<ResourceId, ResourceDescription *> m_Resources;
   rdcarray<ResourceDescription> m_ResourceList;
 
+  QList<EventBookmark> m_Bookmarks;
+
   QMap<ResourceId, QString> m_CustomNames;
   int m_CustomNameCachedID = 1;
 

qrenderdoc/Code/Interface/QRDInterface.h

@@ -905,6 +905,23 @@ enum class CaptureModifications : uint32_t
 
 BITMASK_OPERATORS(CaptureModifications);
 
+DOCUMENT("A description of a bookmark on an event");
+struct EventBookmark
+{
+  DOCUMENT("The EID at which this bookmark is placed.");
+  uint32_t EID = 0;
+
+  DOCUMENT("The text associated with this bookmark - could be empty");
+  QString text;
+
+  DOCUMENT("");
+  EventBookmark() = default;
+  EventBookmark(uint32_t e) : EID(e) {}
+  bool operator==(const EventBookmark &o) { return EID == o.EID; }
+  bool operator!=(const EventBookmark &o) const { return EID != o.EID; }
+  bool operator<(const EventBookmark &o) const { return EID < o.EID; }
+};
+
 DOCUMENT("The capture context that the python script is running in.")
 struct ICaptureContext
 {
@@ -1279,6 +1296,34 @@ as well as messages generated during replay and analysis.
 )");
   virtual void AddMessages(const rdcarray<DebugMessage> &msgs) = 0;
 
+  DOCUMENT(R"(Get the current list of bookmarks in the capture. Each bookmark is associated with an
+EID and has some text attached. There will only be at most one bookmark for any given EID.
+
+The list of bookmarks is not necessarily sorted by EID. Thus, bookmark 1 is always bookmark 1 until
+it is removed, the indices do not shift as new bookmarks are added or removed.
+
+:return: The currently set bookmarks.
+:rtype: ``list`` of :class:`BookMark`
+)");
+  virtual QList<EventBookmark> GetBookmarks() = 0;
+
+  DOCUMENT(R"(Set or update a bookmark.
+
+A bookmark will be added at the specified EID, or if one already exists then the attached text will
+be replaced.
+
+:param Bookmark mark: The bookmark to add.
+)");
+  virtual void SetBookmark(const EventBookmark &mark) = 0;
+
+  DOCUMENT(R"(Remove a bookmark at a given EID.
+
+If no bookmark exists, this function will do nothing.
+
+:param int EID: The EID of the bookmark to remove.
+)");
+  virtual void RemoveBookmark(uint32_t EID) = 0;
+
   DOCUMENT(R"(Retrieve the current singleton :class:`MainWindow`.
 
 :return: The current window.

qrenderdoc/Windows/EventBrowser.cpp

@@ -176,8 +176,6 @@ void EventBrowser::OnCaptureLoaded()
   RDTreeWidgetItem *frame = new RDTreeWidgetItem(
       {QFormatStr("Frame #%1").arg(m_Ctx.FrameInfo().frameNumber), QString(), QString(), QString()});
 
-  clearBookmarks();
-
   RDTreeWidgetItem *framestart =
       new RDTreeWidgetItem({tr("Frame Start"), lit("0"), lit("0"), QString()});
   framestart->setTag(QVariant::fromValue(EventItemTag(0, 0)));
@@ -191,6 +189,9 @@ void EventBrowser::OnCaptureLoaded()
 
   ui->events->expandItem(frame);
 
+  clearBookmarks();
+  repopulateBookmarks();
+
   ui->find->setEnabled(true);
   ui->gotoEID->setEnabled(true);
   ui->timeDraws->setEnabled(true);
@@ -220,6 +221,7 @@ void EventBrowser::OnCaptureClosed()
 void EventBrowser::OnEventChanged(uint32_t eventID)
 {
   SelectEvent(eventID);
+  repopulateBookmarks();
   highlightBookmarks();
 }
 
@@ -892,84 +894,107 @@ void EventBrowser::clearBookmarks()
   for(QToolButton *b : m_BookmarkButtons)
     delete b;
 
-  m_Bookmarks.clear();
   m_BookmarkButtons.clear();
 
   ui->bookmarkStrip->setVisible(false);
 }
 
+void EventBrowser::repopulateBookmarks()
+{
+  const QList<EventBookmark> bookmarks = m_Ctx.GetBookmarks();
+
+  // add any bookmark markers that we don't have
+  for(const EventBookmark &mark : bookmarks)
+  {
+    if(!m_BookmarkButtons.contains(mark.EID))
+    {
+      uint32_t EID = mark.EID;
+
+      QToolButton *but = new QToolButton(this);
+
+      but->setText(QString::number(EID));
+      but->setCheckable(true);
+      but->setAutoRaise(true);
+      but->setProperty("eid", EID);
+      QObject::connect(but, &QToolButton::clicked, [this, but, EID]() {
+        but->setChecked(true);
+        SelectEvent(EID);
+        highlightBookmarks();
+      });
+
+      m_BookmarkButtons[EID] = but;
+
+      highlightBookmarks();
+
+      RDTreeWidgetItem *found = NULL;
+      FindEventNode(found, ui->events->topLevelItem(0), EID);
+
+      if(found)
+      {
+        EventItemTag tag = found->tag().value<EventItemTag>();
+        tag.bookmark = true;
+        found->setTag(QVariant::fromValue(tag));
+        RefreshIcon(found, tag);
+      }
+
+      m_BookmarkStripLayout->removeItem(m_BookmarkSpacer);
+      m_BookmarkStripLayout->addWidget(but);
+      m_BookmarkStripLayout->addItem(m_BookmarkSpacer);
+    }
+  }
+
+  // remove any bookmark markers we shouldn't have
+  for(uint32_t EID : m_BookmarkButtons.keys())
+  {
+    if(!bookmarks.contains(EventBookmark(EID)))
+    {
+      delete m_BookmarkButtons[EID];
+      m_BookmarkButtons.remove(EID);
+
+      RDTreeWidgetItem *found = NULL;
+      FindEventNode(found, ui->events->topLevelItem(0), EID);
+
+      if(found)
+      {
+        EventItemTag tag = found->tag().value<EventItemTag>();
+        tag.bookmark = false;
+        found->setTag(QVariant::fromValue(tag));
+        RefreshIcon(found, tag);
+      }
+    }
+  }
+
+  ui->bookmarkStrip->setVisible(!bookmarks.isEmpty());
+}
+
 void EventBrowser::toggleBookmark(uint32_t EID)
 {
-  int index = m_Bookmarks.indexOf(EID);
+  EventBookmark mark(EID);
 
-  RDTreeWidgetItem *found = NULL;
-  FindEventNode(found, ui->events->topLevelItem(0), EID);
-
-  if(index >= 0)
-  {
-    delete m_BookmarkButtons.takeAt(index);
-    m_Bookmarks.removeAt(index);
-
-    if(found)
-    {
-      EventItemTag tag = found->tag().value<EventItemTag>();
-      tag.bookmark = false;
-      found->setTag(QVariant::fromValue(tag));
-      RefreshIcon(found, tag);
-    }
-  }
+  if(m_Ctx.GetBookmarks().contains(mark))
+    m_Ctx.RemoveBookmark(EID);
   else
-  {
-    QToolButton *but = new QToolButton(this);
-
-    but->setText(QString::number(EID));
-    but->setCheckable(true);
-    but->setAutoRaise(true);
-    but->setProperty("eid", EID);
-    QObject::connect(but, &QToolButton::clicked, [this, but, EID]() {
-      but->setChecked(true);
-      SelectEvent(EID);
-      highlightBookmarks();
-    });
-
-    m_BookmarkButtons.push_back(but);
-    m_Bookmarks.push_back(EID);
-
-    highlightBookmarks();
-
-    if(found)
-    {
-      EventItemTag tag = found->tag().value<EventItemTag>();
-      tag.bookmark = true;
-      found->setTag(QVariant::fromValue(tag));
-      RefreshIcon(found, tag);
-    }
-
-    m_BookmarkStripLayout->removeItem(m_BookmarkSpacer);
-    m_BookmarkStripLayout->addWidget(but);
-    m_BookmarkStripLayout->addItem(m_BookmarkSpacer);
-  }
-
-  ui->bookmarkStrip->setVisible(!m_BookmarkButtons.isEmpty());
+    m_Ctx.SetBookmark(mark);
 }
 
 void EventBrowser::jumpToBookmark(int idx)
 {
-  if(idx < 0 || idx >= m_Bookmarks.count() || !m_Ctx.IsCaptureLoaded())
+  const QList<EventBookmark> bookmarks = m_Ctx.GetBookmarks();
+  if(idx < 0 || idx >= bookmarks.count() || !m_Ctx.IsCaptureLoaded())
     return;
 
   // don't exclude ourselves, so we're updated as normal
-  SelectEvent(m_Bookmarks[idx]);
+  SelectEvent(bookmarks[idx].EID);
 }
 
 void EventBrowser::highlightBookmarks()
 {
-  for(QToolButton *b : m_BookmarkButtons)
+  for(uint32_t eid : m_BookmarkButtons.keys())
   {
-    if(b->property("eid").toUInt() == m_Ctx.CurEvent())
-      b->setChecked(true);
+    if(eid == m_Ctx.CurEvent())
+      m_BookmarkButtons[eid]->setChecked(true);
     else
-      b->setChecked(false);
+      m_BookmarkButtons[eid]->setChecked(false);
   }
 }
 
@@ -983,7 +1008,7 @@ bool EventBrowser::hasBookmark(RDTreeWidgetItem *node)
 
 bool EventBrowser::hasBookmark(uint32_t EID)
 {
-  return m_Bookmarks.contains(EID);
+  return m_Ctx.GetBookmarks().contains(EventBookmark(EID));
 }
 
 void EventBrowser::RefreshIcon(RDTreeWidgetItem *item, EventItemTag tag)

qrenderdoc/Windows/EventBrowser.h

@@ -110,6 +110,7 @@ private:
   int SetFindIcons(RDTreeWidgetItem *parent, QString filter);
   int SetFindIcons(QString filter);
 
+  void repopulateBookmarks();
   void highlightBookmarks();
   bool hasBookmark(RDTreeWidgetItem *node);
 
@@ -135,8 +136,7 @@ private:
 
   FlowLayout *m_BookmarkStripLayout;
   QSpacerItem *m_BookmarkSpacer;
-  QList<int> m_Bookmarks;
-  QList<QToolButton *> m_BookmarkButtons;
+  QMap<uint32_t, QToolButton *> m_BookmarkButtons;
 
   void RefreshIcon(RDTreeWidgetItem *item, EventItemTag tag);
 

qrenderdoc/Windows/PythonShell.cpp

@@ -111,6 +111,7 @@ struct CaptureContextInvoker : ICaptureContext
   virtual const QVector<DebugMessage> &DebugMessages() override { return m_Ctx.DebugMessages(); }
   virtual int UnreadMessageCount() override { return m_Ctx.UnreadMessageCount(); }
   virtual void MarkMessagesRead() override { return m_Ctx.MarkMessagesRead(); }
+  virtual QList<EventBookmark> GetBookmarks() override { return m_Ctx.GetBookmarks(); }
   virtual const D3D11Pipe::State &CurD3D11PipelineState() override
   {
     return m_Ctx.CurD3D11PipelineState();
@@ -190,6 +191,15 @@ struct CaptureContextInvoker : ICaptureContext
   {
     InvokeVoidFunction(&ICaptureContext::SetResourceCustomName, id, name);
   }
+
+  virtual void SetBookmark(const EventBookmark &mark) override
+  {
+    InvokeVoidFunction(&ICaptureContext::SetBookmark, mark);
+  }
+  virtual void RemoveBookmark(uint32_t EID) override
+  {
+    InvokeVoidFunction(&ICaptureContext::RemoveBookmark, EID);
+  }
   virtual IMainWindow *GetMainWindow() override
   {
     return InvokeRetFunction<IMainWindow *>(&ICaptureContext::GetMainWindow);