rebaseline_server: use just skpdiff, not Python Image Library

BUG=skia:2414
R=djsollen@google.com, borenet@google.com

Author: epoger@google.com

Review URL: https://codereview.chromium.org/325413003

9 files changed, 263 insertions, 40 deletions

diff --git a/tools/skpdiff/SkDiffContext.cpp b/tools/skpdiff/SkDiffContext.cpp
index 6f0b09f08..26898c92f 100644
--- a/tools/skpdiff/SkDiffContext.cpp
+++ b/tools/skpdiff/SkDiffContext.cpp
@@ -9,11 +9,13 @@
 #include "SkImageDecoder.h"
 #include "SkOSFile.h"
 #include "SkRunnable.h"
+#include "SkSize.h"
 #include "SkStream.h"
 #include "SkTDict.h"
 #include "SkThreadPool.h"
 
 #include "SkDiffContext.h"
+#include "SkImageDiffer.h"
 #include "skpdiff_util.h"
 
 SkDiffContext::SkDiffContext() {
@@ -28,9 +30,21 @@ SkDiffContext::~SkDiffContext() {
     }
 }
 
-void SkDiffContext::setDifferenceDir(const SkString& path) {
+void SkDiffContext::setAlphaMaskDir(const SkString& path) {
     if (!path.isEmpty() && sk_mkdir(path.c_str())) {
-        fDifferenceDir = path;
+        fAlphaMaskDir = path;
+    }
+}
+
+void SkDiffContext::setRgbDiffDir(const SkString& path) {
+    if (!path.isEmpty() && sk_mkdir(path.c_str())) {
+        fRgbDiffDir = path;
+    }
+}
+
+void SkDiffContext::setWhiteDiffDir(const SkString& path) {
+    if (!path.isEmpty() && sk_mkdir(path.c_str())) {
+        fWhiteDiffDir = path;
     }
 }
 
@@ -90,13 +104,13 @@ void SkDiffContext::addDiff(const char* baselinePath, const char* testPath) {
 
     newRecord->fBaselinePath = baselinePath;
     newRecord->fTestPath = testPath;
+    newRecord->fSize = SkISize::Make(baselineBitmap.width(), baselineBitmap.height());
 
-    bool alphaMaskPending = false;
-
-    // only enable alpha masks if a difference dir has been provided
-    if (!fDifferenceDir.isEmpty()) {
-        alphaMaskPending = true;
-    }
+    // only generate diff images if we have a place to store them
+    SkImageDiffer::BitmapsToCreate bitmapsToCreate;
+    bitmapsToCreate.alphaMask = !fAlphaMaskDir.isEmpty();
+    bitmapsToCreate.rgbDiff = !fRgbDiffDir.isEmpty();
+    bitmapsToCreate.whiteDiff = !fWhiteDiffDir.isEmpty();
 
     // Perform each diff
     for (int differIndex = 0; differIndex < fDifferCount; differIndex++) {
@@ -106,30 +120,69 @@ void SkDiffContext::addDiff(const char* baselinePath, const char* testPath) {
         DiffData& diffData = newRecord->fDiffs.push_back();
         diffData.fDiffName = differ->getName();
 
-        if (!differ->diff(&baselineBitmap, &testBitmap, alphaMaskPending, &diffData.fResult)) {
-            // if the diff failed record -1 as the result
+        if (!differ->diff(&baselineBitmap, &testBitmap, bitmapsToCreate, &diffData.fResult)) {
+            // if the diff failed, record -1 as the result
+            // TODO(djsollen): Record more detailed information about exactly what failed.
+            // (Image dimension mismatch? etc.)  See http://skbug.com/2710 ('make skpdiff
+            // report more detail when it fails to compare two images')
             diffData.fResult.result = -1;
             continue;
         }
 
-        if (alphaMaskPending
+        if (bitmapsToCreate.alphaMask
                 && SkImageDiffer::RESULT_CORRECT != diffData.fResult.result
                 && !diffData.fResult.poiAlphaMask.empty()
                 && !newRecord->fCommonName.isEmpty()) {
 
-            newRecord->fDifferencePath = SkOSPath::SkPathJoin(fDifferenceDir.c_str(),
-                                                              newRecord->fCommonName.c_str());
+            newRecord->fAlphaMaskPath = SkOSPath::SkPathJoin(fAlphaMaskDir.c_str(),
+                                                             newRecord->fCommonName.c_str());
 
             // compute the image diff and output it
             SkBitmap copy;
             diffData.fResult.poiAlphaMask.copyTo(&copy, kN32_SkColorType);
-            SkImageEncoder::EncodeFile(newRecord->fDifferencePath.c_str(), copy,
+            SkImageEncoder::EncodeFile(newRecord->fAlphaMaskPath.c_str(), copy,
                                        SkImageEncoder::kPNG_Type, 100);
 
             // cleanup the existing bitmap to free up resources;
             diffData.fResult.poiAlphaMask.reset();
 
-            alphaMaskPending = false;
+            bitmapsToCreate.alphaMask = false;
+        }
+
+        if (bitmapsToCreate.rgbDiff
+                && SkImageDiffer::RESULT_CORRECT != diffData.fResult.result
+                && !diffData.fResult.rgbDiffBitmap.empty()
+                && !newRecord->fCommonName.isEmpty()) {
+            // TODO(djsollen): Rather than taking the max r/g/b diffs that come back from
+            // a particular differ and storing them as toplevel fields within
+            // newRecord, we should extend outputRecords() to report optional
+            // fields for each differ (not just "result" and "pointsOfInterest").
+            // See http://skbug.com/2712 ('allow skpdiff to report different sets
+            // of result fields for different comparison algorithms')
+            newRecord->fMaxRedDiff = diffData.fResult.maxRedDiff;
+            newRecord->fMaxGreenDiff = diffData.fResult.maxGreenDiff;
+            newRecord->fMaxBlueDiff = diffData.fResult.maxBlueDiff;
+
+            newRecord->fRgbDiffPath = SkOSPath::SkPathJoin(fRgbDiffDir.c_str(),
+                                                           newRecord->fCommonName.c_str());
+            SkImageEncoder::EncodeFile(newRecord->fRgbDiffPath.c_str(),
+                                       diffData.fResult.rgbDiffBitmap,
+                                       SkImageEncoder::kPNG_Type, 100);
+            diffData.fResult.rgbDiffBitmap.reset();
+            bitmapsToCreate.rgbDiff = false;
+        }
+
+        if (bitmapsToCreate.whiteDiff
+                && SkImageDiffer::RESULT_CORRECT != diffData.fResult.result
+                && !diffData.fResult.whiteDiffBitmap.empty()
+                && !newRecord->fCommonName.isEmpty()) {
+            newRecord->fWhiteDiffPath = SkOSPath::SkPathJoin(fWhiteDiffDir.c_str(),
+                                                             newRecord->fCommonName.c_str());
+            SkImageEncoder::EncodeFile(newRecord->fWhiteDiffPath.c_str(),
+                                       diffData.fResult.whiteDiffBitmap,
+                                       SkImageEncoder::kPNG_Type, 100);
+            diffData.fResult.whiteDiffBitmap.reset();
+            bitmapsToCreate.whiteDiff = false;
         }
     }
 }
@@ -229,11 +282,15 @@ void SkDiffContext::outputRecords(SkWStream& stream, bool useJSONP) {
     } else {
         stream.writeText("{\n");
     }
+
+    // TODO(djsollen): Would it be better to use the jsoncpp library to write out the JSON?
+    // This manual approach is probably more efficient, but it sure is ugly.
+    // See http://skbug.com/2713 ('make skpdiff use jsoncpp library to write out
+    // JSON output, instead of manual writeText() calls?')
     stream.writeText("    \"records\": [\n");
     while (NULL != currentRecord) {
         stream.writeText("        {\n");
 
-            SkString differenceAbsPath = get_absolute_path(currentRecord->fDifferencePath);
             SkString baselineAbsPath = get_absolute_path(currentRecord->fBaselinePath);
             SkString testAbsPath = get_absolute_path(currentRecord->fTestPath);
 
@@ -242,7 +299,15 @@ void SkDiffContext::outputRecords(SkWStream& stream, bool useJSONP) {
             stream.writeText("\",\n");
 
             stream.writeText("            \"differencePath\": \"");
-            stream.writeText(differenceAbsPath.c_str());
+            stream.writeText(get_absolute_path(currentRecord->fAlphaMaskPath).c_str());
+            stream.writeText("\",\n");
+
+            stream.writeText("            \"rgbDiffPath\": \"");
+            stream.writeText(get_absolute_path(currentRecord->fRgbDiffPath).c_str());
+            stream.writeText("\",\n");
+
+            stream.writeText("            \"whiteDiffPath\": \"");
+            stream.writeText(get_absolute_path(currentRecord->fWhiteDiffPath).c_str());
             stream.writeText("\",\n");
 
             stream.writeText("            \"baselinePath\": \"");
@@ -253,6 +318,23 @@ void SkDiffContext::outputRecords(SkWStream& stream, bool useJSONP) {
             stream.writeText(testAbsPath.c_str());
             stream.writeText("\",\n");
 
+            stream.writeText("            \"width\": ");
+            stream.writeDecAsText(currentRecord->fSize.width());
+            stream.writeText(",\n");
+            stream.writeText("            \"height\": ");
+            stream.writeDecAsText(currentRecord->fSize.height());
+            stream.writeText(",\n");
+
+            stream.writeText("            \"maxRedDiff\": ");
+            stream.writeDecAsText(currentRecord->fMaxRedDiff);
+            stream.writeText(",\n");
+            stream.writeText("            \"maxGreenDiff\": ");
+            stream.writeDecAsText(currentRecord->fMaxGreenDiff);
+            stream.writeText(",\n");
+            stream.writeText("            \"maxBlueDiff\": ");
+            stream.writeDecAsText(currentRecord->fMaxBlueDiff);
+            stream.writeText(",\n");
+
             stream.writeText("            \"diffs\": [\n");
             for (int diffIndex = 0; diffIndex < currentRecord->fDiffs.count(); diffIndex++) {
                 DiffData& data = currentRecord->fDiffs[diffIndex];
diff --git a/tools/skpdiff/SkDiffContext.h b/tools/skpdiff/SkDiffContext.h
index c036c2ef6..996737f2e 100644
--- a/tools/skpdiff/SkDiffContext.h
+++ b/tools/skpdiff/SkDiffContext.h
@@ -28,10 +28,28 @@ public:
     void setThreadCount(int threadCount) { fThreadCount = threadCount; }
 
     /**
-     * Creates the directory if it does not exist and uses it to store differences
-     * between images.
+     * Sets the directory within which to store alphaMasks (images that
+     * are transparent for each pixel that differs between baseline and test).
+     *
+     * If the directory does not exist yet, it will be created.
+     */
+    void setAlphaMaskDir(const SkString& directory);
+
+    /**
+     * Sets the directory within which to store rgbDiffs (images showing the
+     * per-channel difference between baseline and test at each pixel).
+     *
+     * If the directory does not exist yet, it will be created.
+     */
+    void setRgbDiffDir(const SkString& directory);
+
+    /**
+     * Sets the directory within which to store whiteDiffs (images showing white
+     * for each pixel that differs between baseline and test).
+     *
+     * If the directory does not exist yet, it will be created.
      */
-    void setDifferenceDir(const SkString& directory);
+    void setWhiteDiffDir(const SkString& directory);
 
     /**
      * Sets the differs to be used in each diff. Already started diffs will not retroactively use
@@ -74,6 +92,14 @@ public:
      *    "differencePath" : (optional) string containing the path to an alpha
      *                       mask of the pixel difference between the baseline
      *                       and test images
+     *                       TODO(epoger): consider renaming this "alphaMaskPath"
+     *                       to distinguish from other difference types?
+     *    "rgbDiffPath"    : (optional) string containing the path to a bitmap
+     *                       showing per-channel differences between the
+     *                       baseline and test images at each pixel
+     *    "whiteDiffPath"  : (optional) string containing the path to a bitmap
+     *                       showing every pixel that differs between the
+     *                       baseline and test images as white
      *
      * They also have an array named "diffs" with each element being one diff record for the two
      * images indicated in the above field.
@@ -117,10 +143,21 @@ private:
     };
 
     struct DiffRecord {
+        // TODO(djsollen): Some of these fields are required, while others are optional
+        // (e.g., fRgbDiffPath is only filled in if SkDifferentPixelsMetric
+        // was run).  Figure out a way to note that.  See http://skbug.com/2712
+        // ('allow skpdiff to report different sets of result fields for
+        // different comparison algorithms')
         SkString           fCommonName;
-        SkString           fDifferencePath;
+        SkString           fAlphaMaskPath;
+        SkString           fRgbDiffPath;
+        SkString           fWhiteDiffPath;
         SkString           fBaselinePath;
         SkString               fTestPath;
+        SkISize                    fSize;
+        int                  fMaxRedDiff;
+        int                fMaxGreenDiff;
+        int                 fMaxBlueDiff;
         SkTArray<DiffData>        fDiffs;
     };
 
@@ -137,7 +174,9 @@ private:
     int fDifferCount;
     int fThreadCount;
 
-    SkString fDifferenceDir;
+    SkString fAlphaMaskDir;
+    SkString fRgbDiffDir;
+    SkString fWhiteDiffDir;
 };
 
 #endif
diff --git a/tools/skpdiff/SkDifferentPixelsMetric.h b/tools/skpdiff/SkDifferentPixelsMetric.h
index 06c56b1cc..9302d9e06 100644
--- a/tools/skpdiff/SkDifferentPixelsMetric.h
+++ b/tools/skpdiff/SkDifferentPixelsMetric.h
@@ -28,7 +28,8 @@ class SkDifferentPixelsMetric :
 #endif
 public:
     virtual const char* getName() const SK_OVERRIDE;
-    virtual bool diff(SkBitmap* baseline, SkBitmap* test, bool computeMask,
+    virtual bool diff(SkBitmap* baseline, SkBitmap* test,
+                      const BitmapsToCreate& bitmapsToCreate,
                       Result* result) const SK_OVERRIDE;
 
 protected:
diff --git a/tools/skpdiff/SkDifferentPixelsMetric_cpu.cpp b/tools/skpdiff/SkDifferentPixelsMetric_cpu.cpp
index e528b78f4..7194e805b 100644
--- a/tools/skpdiff/SkDifferentPixelsMetric_cpu.cpp
+++ b/tools/skpdiff/SkDifferentPixelsMetric_cpu.cpp
@@ -14,7 +14,8 @@ const char* SkDifferentPixelsMetric::getName() const {
     return "different_pixels";
 }
 
-bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test, bool computeMask,
+bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test,
+                                   const BitmapsToCreate& bitmapsToCreate,
                                    Result* result) const {
     double startTime = get_seconds();
 
@@ -22,17 +23,34 @@ bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test, bool comp
     if (baseline->width() != test->width() || baseline->height() != test->height() ||
         baseline->width() <= 0 || baseline->height() <= 0 ||
         baseline->colorType() != test->colorType()) {
+        SkASSERT(baseline->width() == test->width());
+        SkASSERT(baseline->height() == test->height());
+        SkASSERT(baseline->width() > 0);
+        SkASSERT(baseline->height() > 0);
+        SkASSERT(baseline->colorType() == test->colorType());
         return false;
     }
 
     int width = baseline->width();
     int height = baseline->height();
+    int maxRedDiff = 0;
+    int maxGreenDiff = 0;
+    int maxBlueDiff = 0;
 
-    // Prepare the POI alpha mask if needed
-    if (computeMask) {
+    // Prepare any bitmaps we will be filling in
+    if (bitmapsToCreate.alphaMask) {
         result->poiAlphaMask.allocPixels(SkImageInfo::MakeA8(width, height));
         result->poiAlphaMask.eraseARGB(SK_AlphaOPAQUE, 0, 0, 0);
     }
+    if (bitmapsToCreate.rgbDiff) {
+        result->rgbDiffBitmap.allocPixels(SkImageInfo::Make(width, height, baseline->colorType(),
+                                                            kPremul_SkAlphaType));
+        result->rgbDiffBitmap.eraseARGB(SK_AlphaTRANSPARENT, 0, 0, 0);
+    }
+    if (bitmapsToCreate.whiteDiff) {
+        result->whiteDiffBitmap.allocPixels(SkImageInfo::MakeN32Premul(width, height));
+        result->whiteDiffBitmap.eraseARGB(SK_AlphaOPAQUE, 0, 0, 0);
+    }
 
     // Prepare the pixels for comparison
     result->poiCount = 0;
@@ -40,24 +58,60 @@ bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test, bool comp
     test->lockPixels();
     for (int y = 0; y < height; y++) {
         // Grab a row from each image for easy comparison
-        unsigned char* baselineRow = (unsigned char*)baseline->getAddr(0, y);
-        unsigned char* testRow = (unsigned char*)test->getAddr(0, y);
+        // TODO(epoger): The code below already assumes 4 bytes per pixel, so I think
+        // we could just call getAddr32() to save a little time.
+        // OR, if we want to play it safe, call ComputeBytesPerPixel instead
+        // of assuming 4 bytes per pixel.
+        uint32_t* baselineRow = static_cast<uint32_t *>(baseline->getAddr(0, y));
+        uint32_t* testRow = static_cast<uint32_t *>(test->getAddr(0, y));
         for (int x = 0; x < width; x++) {
             // Compare one pixel at a time so each differing pixel can be noted
-            if (memcmp(&baselineRow[x * 4], &testRow[x * 4], 4) != 0) {
+            // TODO(epoger): This loop looks like a good place to work on performance,
+            // but we should run the code through a profiler to be sure.
+            uint32_t baselinePixel = baselineRow[x];
+            uint32_t testPixel = testRow[x];
+            if (baselinePixel != testPixel) {
                 result->poiCount++;
-                if (computeMask) {
+
+                int redDiff = abs(static_cast<int>(SkColorGetR(baselinePixel) -
+                                                   SkColorGetR(testPixel)));
+                if (redDiff > maxRedDiff) {maxRedDiff = redDiff;}
+                int greenDiff = abs(static_cast<int>(SkColorGetG(baselinePixel) -
+                                                     SkColorGetG(testPixel)));
+                if (greenDiff > maxGreenDiff) {maxGreenDiff = greenDiff;}
+                int blueDiff = abs(static_cast<int>(SkColorGetB(baselinePixel) -
+                                                    SkColorGetB(testPixel)));
+                if (blueDiff > maxBlueDiff) {maxBlueDiff = blueDiff;}
+
+                if (bitmapsToCreate.alphaMask) {
                     *result->poiAlphaMask.getAddr8(x,y) = SK_AlphaTRANSPARENT;
                 }
+                if (bitmapsToCreate.rgbDiff) {
+                    *result->rgbDiffBitmap.getAddr32(x,y) =
+                        SkColorSetRGB(redDiff, greenDiff, blueDiff);
+                }
+                if (bitmapsToCreate.whiteDiff) {
+                    *result->whiteDiffBitmap.getAddr32(x,y) = SK_ColorWHITE;
+                }
             }
         }
     }
     test->unlockPixels();
     baseline->unlockPixels();
 
-    if (computeMask) {
+    result->maxRedDiff = maxRedDiff;
+    result->maxGreenDiff = maxGreenDiff;
+    result->maxBlueDiff = maxBlueDiff;
+
+    if (bitmapsToCreate.alphaMask) {
         result->poiAlphaMask.unlockPixels();
     }
+    if (bitmapsToCreate.rgbDiff) {
+        result->rgbDiffBitmap.unlockPixels();
+    }
+    if (bitmapsToCreate.whiteDiff) {
+        result->whiteDiffBitmap.unlockPixels();
+    }
 
     // Calculates the percentage of identical pixels
     result->result = 1.0 - ((double)result->poiCount / (width * height));
diff --git a/tools/skpdiff/SkDifferentPixelsMetric_opencl.cpp b/tools/skpdiff/SkDifferentPixelsMetric_opencl.cpp
index 14225055f..2e5ab8f72 100644
--- a/tools/skpdiff/SkDifferentPixelsMetric_opencl.cpp
+++ b/tools/skpdiff/SkDifferentPixelsMetric_opencl.cpp
@@ -36,7 +36,8 @@ const char* SkDifferentPixelsMetric::getName() const {
     return "different_pixels";
 }
 
-bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test, bool computeMask,
+bool SkDifferentPixelsMetric::diff(SkBitmap* baseline, SkBitmap* test, bool computeAlphaMask,
+                                   bool computeRgbDiff, bool computeWhiteDiff,
                                    Result* result) const {
     double startTime = get_seconds();
 
diff --git a/tools/skpdiff/SkImageDiffer.h b/tools/skpdiff/SkImageDiffer.h
index 641bbe8f8..1f9ffd188 100644
--- a/tools/skpdiff/SkImageDiffer.h
+++ b/tools/skpdiff/SkImageDiffer.h
@@ -24,10 +24,30 @@ public:
     struct Result {
         double result;
         int poiCount;
+        // TODO(djsollen): Figure out a way that the differ can report which of the
+        // optional fields it has filled in.  See http://skbug.com/2712 ('allow
+        // skpdiff to report different sets of result fields for different comparison algorithms')
         SkBitmap poiAlphaMask; // optional
+        SkBitmap rgbDiffBitmap; // optional
+        SkBitmap whiteDiffBitmap; // optional
+        int maxRedDiff; // optional
+        int maxGreenDiff; // optional
+        int maxBlueDiff; // optional
         double timeElapsed; // optional
     };
 
+    // A bitfield indicating which bitmap types we want a differ to create.
+    //
+    // TODO(epoger): Remove whiteDiffBitmap, because alphaMask can provide
+    // the same functionality and more.
+    // It will be a little bit tricky, because the rebaseline_server client
+    // and server side code will both need to change to use the alphaMask.
+    struct BitmapsToCreate {
+        bool alphaMask;
+        bool rgbDiff;
+        bool whiteDiff;
+    };
+
     /**
      * Gets a unique and descriptive name of this differ
      * @return A statically allocated null terminated string that is the name of this differ
@@ -43,10 +63,10 @@ public:
      * diff on a pair of bitmaps.
      * @param  baseline    The correct bitmap
      * @param  test        The bitmap whose difference is being tested
-     * @param  computeMask true if the differ is to attempt to create poiAlphaMask
+     * @param  bitmapsToCreate  Which bitmaps the differ should attempt to create
      * @return             true on success, and false in the case of failure
      */
-    virtual bool diff(SkBitmap* baseline, SkBitmap* test, bool computeMask,
+    virtual bool diff(SkBitmap* baseline, SkBitmap* test, const BitmapsToCreate& bitmapsToCreate,
                       Result* result) const = 0;
 };
 
diff --git a/tools/skpdiff/SkPMetric.cpp b/tools/skpdiff/SkPMetric.cpp
index a433d9f4b..392a34221 100644
--- a/tools/skpdiff/SkPMetric.cpp
+++ b/tools/skpdiff/SkPMetric.cpp
@@ -442,7 +442,8 @@ static double pmetric(const ImageLAB* baselineLAB, const ImageLAB* testLAB, int*
     return 1.0 - (double)(*poiCount) / (width * height);
 }
 
-bool SkPMetric::diff(SkBitmap* baseline, SkBitmap* test, bool computeMask, Result* result) const {
+bool SkPMetric::diff(SkBitmap* baseline, SkBitmap* test, const BitmapsToCreate& bitmapsToCreate,
+                     Result* result) const {
     double startTime = get_seconds();
 
     // Ensure the images are comparable
diff --git a/tools/skpdiff/SkPMetric.h b/tools/skpdiff/SkPMetric.h
index b60858a65..00a3d7ae6 100644
--- a/tools/skpdiff/SkPMetric.h
+++ b/tools/skpdiff/SkPMetric.h
@@ -19,7 +19,7 @@
 class SkPMetric : public SkImageDiffer {
 public:
     virtual const char* getName() const SK_OVERRIDE { return "perceptual"; }
-    virtual bool diff(SkBitmap* baseline, SkBitmap* test, bool computeMask,
+    virtual bool diff(SkBitmap* baseline, SkBitmap* test, const BitmapsToCreate& bitmapsToCreate,
                       Result* result) const SK_OVERRIDE;
 
 private:
diff --git a/tools/skpdiff/skpdiff_main.cpp b/tools/skpdiff/skpdiff_main.cpp
index b1bf9173c..3d1bcda0f 100644
--- a/tools/skpdiff/skpdiff_main.cpp
+++ b/tools/skpdiff/skpdiff_main.cpp
@@ -5,6 +5,11 @@
  * found in the LICENSE file.
  */
 
+// TODO(djsollen): Rename this whole package (perhaps to "SkMultiDiffer").
+// It's not just for "pdiff" (perceptual diffs)--it's a harness that allows
+// the execution of an arbitrary set of difference algorithms.
+// See http://skbug.com/2711 ('rename skpdiff')
+
 #if SK_SUPPORT_OPENCL
 
 #define __NO_STD_VECTOR // Uses cl::vectpr instead of std::vectpr
@@ -37,10 +42,12 @@ DEFINE_bool2(list, l, false, "List out available differs");
 DEFINE_string2(differs, d, "", "The names of the differs to use or all of them by default");
 DEFINE_string2(folders, f, "", "Compare two folders with identical subfile names: <baseline folder> <test folder>");
 DEFINE_string2(patterns, p, "", "Use two patterns to compare images: <baseline> <test>");
-DEFINE_string2(output, o, "", "Writes the output of these diffs to output: <output>");
-DEFINE_string(alphaDir, "", "Writes the alpha mask of these diffs to output: <output>");
+DEFINE_string2(output, o, "", "Writes a JSON summary of these diffs to file: <filepath>");
+DEFINE_string(alphaDir, "", "If the differ can generate an alpha mask, write it into directory: <dirpath>");
+DEFINE_string(rgbDiffDir, "", "If the differ can generate an image showing the RGB diff at each pixel, write it into directory: <dirpath>");
+DEFINE_string(whiteDiffDir, "", "If the differ can generate an image showing every changed pixel in white, write it into directory: <dirpath>");
 DEFINE_bool(jsonp, true, "Output JSON with padding");
-DEFINE_string(csv, "", "Writes the output of these diffs to a csv file");
+DEFINE_string(csv, "", "Writes the output of these diffs to a csv file: <filepath>");
 DEFINE_int32(threads, -1, "run N threads in parallel [default is derived from CPUs available]");
 
 #if SK_SUPPORT_OPENCL
@@ -193,12 +200,30 @@ int tool_main(int argc, char * argv[]) {
             return 1;
         }
     }
+    if (!FLAGS_rgbDiffDir.isEmpty()) {
+        if (1 != FLAGS_rgbDiffDir.count()) {
+            SkDebugf("rgbDiffDir flag expects one argument: <directory>\n");
+            return 1;
+        }
+    }
+    if (!FLAGS_whiteDiffDir.isEmpty()) {
+        if (1 != FLAGS_whiteDiffDir.count()) {
+            SkDebugf("whiteDiffDir flag expects one argument: <directory>\n");
+            return 1;
+        }
+    }
 
     SkDiffContext ctx;
     ctx.setDiffers(chosenDiffers);
 
     if (!FLAGS_alphaDir.isEmpty()) {
-        ctx.setDifferenceDir(SkString(FLAGS_alphaDir[0]));
+        ctx.setAlphaMaskDir(SkString(FLAGS_alphaDir[0]));
+    }
+    if (!FLAGS_rgbDiffDir.isEmpty()) {
+        ctx.setRgbDiffDir(SkString(FLAGS_rgbDiffDir[0]));
+    }
+    if (!FLAGS_whiteDiffDir.isEmpty()) {
+        ctx.setWhiteDiffDir(SkString(FLAGS_whiteDiffDir[0]));
     }
 
     if (FLAGS_threads >= 0) {