From 715ae0ea035ec4c3e6e318257a3feb1cd1163b85 Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Fri, 4 Jun 2021 11:25:04 +0100
Subject: [PATCH] Don't match popmarker draws with $draw() by default

* This is a little bit inconsistent because they are technically draws, but
  they're only draws so they can include the trailing API events in a region.
* Think of it this way:
  Imagine we instead made PopMarker just a normal API event (which is how we want to treat it) and instead inserted a second 'internal' draw at the end of each region to contain the popmarker. This internal draw would be hardcoded to never show up in filters no matter what - even with an empty include-all filter. But it contains the other API events so they can be selected individually and filtered in/out.
  In this scenario it's functionally equivalent to just using the PopMarker as that 'internal' draw containing its own API event and making it fail all draw filters. The only difference is that users can manually filter in the PopMarker draw with flags if they want.
---
 qrenderdoc/Windows/EventBrowser.cpp | 30 +++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/qrenderdoc/Windows/EventBrowser.cpp b/qrenderdoc/Windows/EventBrowser.cpp
index cc0c1f206..2c10d4e92 100644
--- a/qrenderdoc/Windows/EventBrowser.cpp
+++ b/qrenderdoc/Windows/EventBrowser.cpp
@@ -1747,13 +1747,17 @@ private:
                                                          ParseTrace &trace)
   {
     // $draw(...) => returns true only for draws, optionally with a particular property filter
+    // PopMarker draws are draws so they can contain the other non-draw events that might trail
+    // in a group, but we don't count them as real draws for any of this filtering except for flags
+    // filtering, where they can be manually included
     QList<Token> tokens = tokenise(parameters);
 
     // no parameters, just return if it's a draw
     if(tokens.isEmpty())
-      return
-          [](ICaptureContext *, const rdcstr &, const rdcstr &, uint32_t eventId, const SDChunk *,
-             const DrawcallDescription *draw, const rdcstr &) { return draw->eventId == eventId; };
+      return [](ICaptureContext *, const rdcstr &, const rdcstr &, uint32_t eventId,
+                const SDChunk *, const DrawcallDescription *draw, const rdcstr &) {
+        return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker);
+      };
 
     // we upcast to int64_t so we can compare both unsigned and signed values without losing any
     // precision (we don't have any uint64_ts to compare)
@@ -1836,37 +1840,43 @@ private:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) == value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) == value;
           };
         case 1:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) != value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) != value;
           };
         case 2:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) < value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) < value;
           };
         case 3:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) > value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) > value;
           };
         case 4:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) <= value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) <= value;
           };
         case 5:
           return [propGetter, value](ICaptureContext *, const rdcstr &, const rdcstr &,
                                      uint32_t eventId, const SDChunk *,
                                      const DrawcallDescription *draw, const rdcstr &) {
-            return draw->eventId == eventId && propGetter(draw) >= value;
+            return draw->eventId == eventId && !(draw->flags & DrawFlags::PopMarker) &&
+                   propGetter(draw) >= value;
           };
         default: trace.setError(tr("Internal error", "EventFilterModel")); return NULL;
       }
@@ -2945,7 +2955,7 @@ EventBrowser::EventBrowser(ICaptureContext &ctx, QWidget *parent)
   ui->filterExpression->enableCompletion();
 
   // set default filter, include only draws that aren't pop markers
-  ui->filterExpression->setText(lit("$draw() -$draw(flags & PopMarker)"));
+  ui->filterExpression->setText(lit("$draw()"));
 
   QObject::connect(ui->filterExpression, &RDTextEdit::completionBegin, [this](QString prefix) {
     if(m_FilterTimeout->isActive())