From 72d2ba9c2b5ba2e2516c0959bf66eafd7b054892 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Jos=C3=A9=20Fonseca?= <jfonseca@vmware.com>
Date: Tue, 4 Jun 2013 16:59:21 +0100
Subject: [PATCH] retrace: Remove the -c/--compare=PREFIX .

That is, remove the ability to compare images while replaying.

For several reasons:

- This duplicates diff-images command.

- It is less versatile -- it is unable to output diff images.

- It will be necessary to change how snapshots are done to properly
  accommodate APIs that have no concept of current context, like
  Direct3D APIs, so the less fat on this code the better.
---
 image/CMakeLists.txt     |  1 -
 image/image.cpp          | 84 ----------------------------------------
 image/image.hpp          |  2 -
 retrace/retrace_main.cpp | 46 ++--------------------
 4 files changed, 4 insertions(+), 129 deletions(-)
 delete mode 100644 image/image.cpp

diff --git a/image/CMakeLists.txt b/image/CMakeLists.txt
index 37203f8..51d7ed8 100644
--- a/image/CMakeLists.txt
+++ b/image/CMakeLists.txt
@@ -3,7 +3,6 @@ include_directories (
 )
 
 add_library (image STATIC
-    image.cpp
     image_bmp.cpp
     image_png.cpp
     image_pnm.cpp
diff --git a/image/image.cpp b/image/image.cpp
deleted file mode 100644
index e692313..0000000
--- a/image/image.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-/**************************************************************************
- *
- * Copyright 2011 Jose Fonseca
- * Copyright 2008-2010 VMware, Inc.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- **************************************************************************/
-
-
-#include <assert.h>
-#include <math.h>
-
-#include <algorithm>
-
-#include "image.hpp"
-
-
-namespace image {
-
-
-double Image::compare(Image &ref)
-{
-    if (width != ref.width ||
-        height != ref.height ||
-        channels < 3 ||
-        ref.channels < 3) {
-        return 0.0;
-    }
-
-    // Ignore missing alpha when comparing RGB w/ RGBA, but enforce an equal
-    // number of channels otherwise.
-    unsigned minChannels = std::min(channels, ref.channels);
-    if (channels != ref.channels && minChannels < 3) {
-        return 0.0;
-    }
-
-    const unsigned char *pSrc = start();
-    const unsigned char *pRef = ref.start();
-
-    unsigned long long error = 0;
-    for (unsigned y = 0; y < height; ++y) {
-        for (unsigned  x = 0; x < width; ++x) {
-            // FIXME: Ignore alpha channel until we are able to pick a visual
-            // that matches the traces
-            for (unsigned  c = 0; c < minChannels; ++c) {
-                int delta = pSrc[x*channels + c] - pRef[x*ref.channels + c];
-                error += delta*delta;
-            }
-        }
-
-        pSrc += stride();
-        pRef += ref.stride();
-    }
-
-    double numerator = error*2 + 1;
-    double denominator = height*width*minChannels*255ULL*255ULL*2;
-    double quotient = numerator/denominator;
-
-    // Precision in bits
-    double precision = -log(quotient)/log(2.0);
-
-    return precision;
-}
-
-
-} /* namespace image */
diff --git a/image/image.hpp b/image/image.hpp
index 11bfc63..7e13dd6 100644
--- a/image/image.hpp
+++ b/image/image.hpp
@@ -118,8 +118,6 @@ public:
 	writeRAW(os);
 	return true;
     }
-
-    double compare(Image &ref);
 };
 
 
diff --git a/retrace/retrace_main.cpp b/retrace/retrace_main.cpp
index 8079cd6..d1e00db 100644
--- a/retrace/retrace_main.cpp
+++ b/retrace/retrace_main.cpp
@@ -44,7 +44,6 @@
 static bool waitOnFinish = false;
 static bool loopOnFinish = false;
 
-static const char *comparePrefix = NULL;
 static const char *snapshotPrefix = NULL;
 static enum {
     PNM_FMT,
@@ -52,7 +51,6 @@ static enum {
 } snapshotFormat = PNM_FMT;
 
 static trace::CallSet snapshotFrequency;
-static trace::CallSet compareFrequency;
 static trace::ParseBookmark lastFrameStart;
 
 static unsigned dumpStateCallNo = ~0;
@@ -101,26 +99,13 @@ Dumper *dumper = &defaultDumper;
 
 
 /**
- * Take/compare snapshots.
+ * Take snapshots.
  */
 static void
 takeSnapshot(unsigned call_no) {
     static unsigned snapshot_no = 0;
 
-    assert(snapshotPrefix || comparePrefix);
-
-    image::Image *ref = NULL;
-
-    if (comparePrefix) {
-        os::String filename = os::String::format("%s%010u.png", comparePrefix, call_no);
-        ref = image::readPNG(filename);
-        if (!ref) {
-            return;
-        }
-        if (retrace::verbosity >= 0) {
-            std::cout << "Read " << filename << "\n";
-        }
-    }
+    assert(snapshotPrefix);
 
     image::Image *src = dumper->getSnapshot();
     if (!src) {
@@ -147,11 +132,6 @@ takeSnapshot(unsigned call_no) {
         }
     }
 
-    if (ref) {
-        std::cout << "Snapshot " << call_no << " average precision of " << src->compare(*ref) << " bits\n";
-        delete ref;
-    }
-
     delete src;
 
     snapshot_no++;
@@ -170,8 +150,7 @@ static void
 retraceCall(trace::Call *call) {
     bool swapRenderTarget = call->flags &
         trace::CALL_FLAG_SWAP_RENDERTARGET;
-    bool doSnapshot = snapshotFrequency.contains(*call) ||
-        compareFrequency.contains(*call);
+    bool doSnapshot = snapshotFrequency.contains(*call);
 
     // For calls which cause rendertargets to be swaped, we take the
     // snapshot _before_ swapping the rendertargets.
@@ -573,8 +552,6 @@ usage(const char *argv0) {
         "      --pgpu              gpu profiling (gpu times per draw call)\n"
         "      --ppd               pixels drawn profiling (pixels drawn per draw call)\n"
         "      --pmem              memory usage profiling (vsize rss per call)\n"
-        "  -c, --compare=PREFIX    compare against snapshots with given PREFIX\n"
-        "  -C, --calls=CALLSET     calls to compare (default is every frame)\n"
         "      --call-nos[=BOOL]   use call numbers in snapshot filenames\n"
         "      --core              use core profile\n"
         "      --db                use a double buffer visual (default)\n"
@@ -606,14 +583,12 @@ enum {
 };
 
 const static char *
-shortOptions = "bc:C:D:hs:S:vw";
+shortOptions = "bD:hs:S:vw";
 
 const static struct option
 longOptions[] = {
     {"benchmark", no_argument, 0, 'b'},
     {"call-nos", optional_argument, 0, CALL_NOS_OPT },
-    {"calls", required_argument, 0, 'C'},
-    {"compare", required_argument, 0, 'c'},
     {"core", no_argument, 0, CORE_OPT},
     {"db", no_argument, 0, DB_OPT},
     {"driver", required_argument, 0, DRIVER_OPT},
@@ -647,7 +622,6 @@ int main(int argc, char **argv)
     using namespace retrace;
     int i;
 
-    assert(compareFrequency.empty());
     assert(snapshotFrequency.empty());
 
     int opt;
@@ -663,18 +637,6 @@ int main(int argc, char **argv)
         case CALL_NOS_OPT:
             useCallNos = trace::boolOption(optarg);
             break;
-        case 'c':
-            comparePrefix = optarg;
-            if (compareFrequency.empty()) {
-                compareFrequency = trace::CallSet(trace::FREQUENCY_FRAME);
-            }
-            break;
-        case 'C':
-            compareFrequency = trace::CallSet(optarg);
-            if (comparePrefix == NULL) {
-                comparePrefix = "";
-            }
-            break;
         case 'D':
             dumpStateCallNo = atoi(optarg);
             dumpingState = true;
-- 
2.45.2