[PATCH] retrace: support for dumping multiple snapshots

[Rob Clark]

Usually if an app is using MRT, we want to dump (and diff) *all* the
render targets to track down where things are going wrong.

Only implemented for GLDumper, since I don't know anything about D3D.

TODO maybe cmdline arg to control this?  I'm not sure, I kinda tend to
think if you are using dump-images + diff-images you probably want to
compare all the render targets.  Maybe even depth/stencil.
---
 retrace/d3dretrace.hpp     |  7 ++++++-
 retrace/glretrace_main.cpp | 12 ++++++++++--
 retrace/glstate.hpp        |  5 ++++-
 retrace/glstate_images.cpp | 26 ++++++++++++++++++++++----
 retrace/retrace.hpp        |  5 ++++-
 retrace/retrace_main.cpp   | 34 +++++++++++++++++++++++++---------
 6 files changed, 71 insertions(+), 18 deletions(-)

diff --git a/retrace/d3dretrace.hpp b/retrace/d3dretrace.hpp
index d0be577..70f4b42 100644
--- a/retrace/d3dretrace.hpp
+++ b/retrace/d3dretrace.hpp
@@ -50,8 +50,13 @@ public:
         pLastDevice(NULL)
     {}
 
+    int
+    getSnapshotCount(void) override {
+        return 1;
+    }
+
     image::Image *
-    getSnapshot(void) {
+    getSnapshot(int n) {
         if (!pLastDevice) {
             return NULL;
         }
diff --git a/retrace/glretrace_main.cpp b/retrace/glretrace_main.cpp
index db94ce6..681a268 100755
--- a/retrace/glretrace_main.cpp
+++ b/retrace/glretrace_main.cpp
@@ -817,12 +817,20 @@ debugOutputCallback(GLenum source, GLenum type, GLuint id, GLenum severity,
 
 class GLDumper : public retrace::Dumper {
 public:
+    int
+    getSnapshotCount(void) override {
+        if (!glretrace::getCurrentContext()) {
+            return 0;
+        }
+        return glstate::getDrawBufferImageCount();
+    }
+
     image::Image *
-    getSnapshot(void) override {
+    getSnapshot(int n) override {
         if (!glretrace::getCurrentContext()) {
             return NULL;
         }
-        return glstate::getDrawBufferImage();
+        return glstate::getDrawBufferImage(n);
     }
 
     bool
diff --git a/retrace/glstate.hpp b/retrace/glstate.hpp
index 417a032..b6eb73a 100644
--- a/retrace/glstate.hpp
+++ b/retrace/glstate.hpp
@@ -64,8 +64,11 @@ dumpCurrentContext(StateWriter &writer);
 bool
 getDrawableBounds(GLint *width, GLint *height);
 
+int
+getDrawBufferImageCount(void);
+
 image::Image *
-getDrawBufferImage(void);
+getDrawBufferImage(int n);
 
 
 } /* namespace glstate */
diff --git a/retrace/glstate_images.cpp b/retrace/glstate_images.cpp
index 13cddb1..25d6ae7 100644
--- a/retrace/glstate_images.cpp
+++ b/retrace/glstate_images.cpp
@@ -959,9 +959,25 @@ getFramebufferAttachmentDesc(Context &context, GLenum target, GLenum attachment,
 }
 
 
+int
+getDrawBufferImageCount()
+{
+    Context context;
+    GLint count;
+
+    if (context.framebuffer_object) {
+        glGetIntegerv(GL_MAX_DRAW_BUFFERS, &count);
+        flushErrors();
+    } else {
+        return 0;
+    }
+
+    return count;
+}
+
 
 image::Image *
-getDrawBufferImage()
+getDrawBufferImage(int n)
 {
     Context context;
 
@@ -983,19 +999,19 @@ getDrawBufferImage()
     ImageDesc desc;
     if (draw_framebuffer) {
         if (context.ARB_draw_buffers) {
-            glGetIntegerv(GL_DRAW_BUFFER0, &draw_buffer);
+            glGetIntegerv(GL_DRAW_BUFFER0 + n, &draw_buffer);
             if (draw_buffer == GL_NONE) {
                 return NULL;
             }
         } else {
             // GL_COLOR_ATTACHMENT0 is implied
-            draw_buffer = GL_COLOR_ATTACHMENT0;
+            draw_buffer = GL_COLOR_ATTACHMENT0 + n;
         }
 
         if (!getFramebufferAttachmentDesc(context, framebuffer_target, draw_buffer, desc)) {
             return NULL;
         }
-    } else {
+    } else if (n == 0) {
         if (context.ES) {
             // XXX: Draw buffer is always FRONT for single buffer context, BACK
             // for double buffered contexts. There is no way to know which (as
@@ -1014,6 +1030,8 @@ getDrawBufferImage()
         }
 
         desc.depth = 1;
+    } else {
+        return NULL;
     }
 
     GLenum format = GL_RGB;
diff --git a/retrace/retrace.hpp b/retrace/retrace.hpp
index 356ad31..bbf5e51 100644
--- a/retrace/retrace.hpp
+++ b/retrace/retrace.hpp
@@ -223,8 +223,11 @@ public:
 class Dumper
 {
 public:
+    virtual int
+    getSnapshotCount(void) = 0;
+
     virtual image::Image *
-    getSnapshot(void) = 0;
+    getSnapshot(int n) = 0;
 
     virtual bool
     canDump(void) = 0;
diff --git a/retrace/retrace_main.cpp b/retrace/retrace_main.cpp
index 83b43ca..703b4df 100644
--- a/retrace/retrace_main.cpp
+++ b/retrace/retrace_main.cpp
@@ -133,8 +133,13 @@ frameComplete(trace::Call &call) {
 class DefaultDumper: public Dumper
 {
 public:
+    int
+    getSnapshotCount(void) override {
+        return 0;
+    }
+
     image::Image *
-    getSnapshot(void) override {
+    getSnapshot(int n) override {
         return NULL;
     }
 
@@ -166,15 +171,16 @@ static Snapshotter *snapshotter;
  * Take snapshots.
  */
 static void
-takeSnapshot(unsigned call_no) {
-    static unsigned snapshot_no = 0;
+takeSnapshot(unsigned call_no, unsigned mrt, unsigned snapshot_no) {
 
     assert(dumpingSnapshots);
     assert(snapshotPrefix);
 
-    std::unique_ptr<image::Image> src(dumper->getSnapshot());
+    std::unique_ptr<image::Image> src(dumper->getSnapshot(mrt));
     if (!src) {
-        std::cerr << call_no << ": warning: failed to get snapshot\n";
+        /* TODO for mrt>0 we probably don't want to treat this as an error: */
+        if (mrt == 0)
+            std::cerr << call_no << ": warning: failed to get snapshot\n";
         return;
     }
 
@@ -200,9 +206,10 @@ takeSnapshot(unsigned call_no) {
                 break;
             }
         } else {
-            os::String filename = os::String::format("%s%010u.png",
+            os::String filename = os::String::format("%s%010u-%u.png",
                                                      snapshotPrefix,
-                                                     useCallNos ? call_no : snapshot_no);
+                                                     useCallNos ? call_no : snapshot_no,
+                                                     mrt);
 
             // Here we release our ownership on the Image, it is now the
             // responsibility of the snapshotter to delete it.
@@ -210,11 +217,20 @@ takeSnapshot(unsigned call_no) {
         }
     }
 
-    snapshot_no++;
-
     return;
 }
 
+static void
+takeSnapshot(unsigned call_no) {
+    static unsigned snapshot_no = 0;
+    unsigned cnt = dumper->getSnapshotCount();
+
+    for (unsigned mrt = 0; mrt < cnt; mrt++) {
+        takeSnapshot(call_no, mrt, snapshot_no);
+    }
+
+    snapshot_no++;
+}
 
 /**
  * Retrace one call.
-- 
2.9.3