add point of interest API

R=bsalomon@google.com

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

git-svn-id: http://skia.googlecode.com/svn/trunk@9806 2bbb7eff-a529-9590-31e7-b0007b416f81

8 files changed, 116 insertions, 31 deletions

diff --git a/experimental/skpdiff/SkCLImageDiffer.cpp b/experimental/skpdiff/SkCLImageDiffer.cpp
index 50eb2b6fc8..0c27e0989b 100644
--- a/experimental/skpdiff/SkCLImageDiffer.cpp
+++ b/experimental/skpdiff/SkCLImageDiffer.cpp
@@ -69,7 +69,8 @@ bool SkCLImageDiffer::loadKernelSource(const char source[], const char name[], c
 
         // Attempt to get information about why the build failed
         char buildLog[4096];
-        clGetProgramBuildInfo(program, fDevice, CL_PROGRAM_BUILD_LOG, sizeof(buildLog), buildLog, NULL);
+        clGetProgramBuildInfo(program, fDevice, CL_PROGRAM_BUILD_LOG, sizeof(buildLog),
+                              buildLog, NULL);
         SkDebugf("Build log: %s\n", buildLog);
 
         return false;
@@ -125,6 +126,17 @@ bool SkCLImageDiffer::makeImage2D(SkBitmap* bitmap, cl_mem* image) {
 
 ////////////////////////////////////////////////////////////////
 
+struct SkDifferentPixelsImageDiffer::QueuedDiff {
+    bool finished;
+    double result;
+    int numDiffPixels;
+    SkIPoint* poi;
+    cl_mem baseline;
+    cl_mem test;
+    cl_mem resultsBuffer;
+    cl_mem poiBuffer;
+};
+
 const char* SkDifferentPixelsImageDiffer::getName() {
     return "different_pixels";
 }
@@ -134,18 +146,22 @@ int SkDifferentPixelsImageDiffer::queueDiff(SkBitmap * baseline, SkBitmap * test
     double startTime = get_seconds();
     QueuedDiff* diff = fQueuedDiffs.push();
 
+    // If we never end up running the kernel, include some safe defaults in the result.
+    diff->finished = false;
+    diff->result = -1.0;
+    diff->numDiffPixels = 0;
+    diff->poi = NULL;
+
     // Ensure the images are comparable
     if (baseline->width() != test->width() || baseline->height() != test->height() ||
                     baseline->width() <= 0 || baseline->height() <= 0) {
         diff->finished = true;
-        diff->result = 0.0;
         return diffID;
     }
 
     // Upload images to the CL device
     if (!this->makeImage2D(baseline, &diff->baseline) || !this->makeImage2D(test, &diff->test)) {
         diff->finished = true;
-        diff->result = 0.0;
         fIsGood = false;
         return -1;
     }
@@ -153,15 +169,21 @@ int SkDifferentPixelsImageDiffer::queueDiff(SkBitmap * baseline, SkBitmap * test
     // A small hack that makes calculating percentage difference easier later on.
     diff->result = 1.0 / ((double)baseline->width() * baseline->height());
 
-    // Make a buffer to store results into
-    int numDiffPixels = 0;
+    // Make a buffer to store results into. It must be initialized with pointers to memory.
+    static const int kZero = 0;
+    // We know OpenCL won't write to it because we use CL_MEM_COPY_HOST_PTR
     diff->resultsBuffer = clCreateBuffer(fContext, CL_MEM_READ_WRITE | CL_MEM_COPY_HOST_PTR,
-                                         sizeof(int), &numDiffPixels, NULL);
+                                         sizeof(int), (int*)&kZero, NULL);
+
+    diff->poiBuffer = clCreateBuffer(fContext, CL_MEM_WRITE_ONLY,
+                                     sizeof(int) * 2 * baseline->width() * baseline->height(),
+                                     NULL, NULL);
 
     // Set all kernel arguments
     cl_int setArgErr = clSetKernelArg(fKernel, 0, sizeof(cl_mem), &diff->baseline);
     setArgErr       |= clSetKernelArg(fKernel, 1, sizeof(cl_mem), &diff->test);
     setArgErr       |= clSetKernelArg(fKernel, 2, sizeof(cl_mem), &diff->resultsBuffer);
+    setArgErr       |= clSetKernelArg(fKernel, 3, sizeof(cl_mem), &diff->poiBuffer);
     if (CL_SUCCESS != setArgErr) {
         SkDebugf("Set arg failed: %s\n", cl_error_to_string(setArgErr));
         fIsGood = false;
@@ -172,7 +194,8 @@ int SkDifferentPixelsImageDiffer::queueDiff(SkBitmap * baseline, SkBitmap * test
     cl_event event;
     const size_t workSize[] = { baseline->width(), baseline->height() };
     cl_int enqueueErr;
-    enqueueErr = clEnqueueNDRangeKernel(fCommandQueue, fKernel, 2, NULL, workSize, NULL, 0, NULL, &event);
+    enqueueErr = clEnqueueNDRangeKernel(fCommandQueue, fKernel, 2, NULL, workSize,
+                                        NULL, 0, NULL, &event);
     if (CL_SUCCESS != enqueueErr) {
         SkDebugf("Enqueue failed: %s\n", cl_error_to_string(enqueueErr));
         fIsGood = false;
@@ -184,11 +207,17 @@ int SkDifferentPixelsImageDiffer::queueDiff(SkBitmap * baseline, SkBitmap * test
     diff->finished = true;
 
     // Immediate read back the results
-    clEnqueueReadBuffer(fCommandQueue, diff->resultsBuffer, CL_TRUE, 0, sizeof(int), &numDiffPixels, 0, NULL, NULL);
-    diff->result *= (double)numDiffPixels;
+    clEnqueueReadBuffer(fCommandQueue, diff->resultsBuffer, CL_TRUE, 0,
+                        sizeof(int), &diff->numDiffPixels, 0, NULL, NULL);
+    diff->result *= (double)diff->numDiffPixels;
     diff->result = (1.0 - diff->result);
 
+    diff->poi = SkNEW_ARRAY(SkIPoint, diff->numDiffPixels);
+    clEnqueueReadBuffer(fCommandQueue, diff->poiBuffer, CL_TRUE, 0,
+                        sizeof(SkIPoint) * diff->numDiffPixels, diff->poi, 0, NULL, NULL);
+
     // Release all the buffers created
+    clReleaseMemObject(diff->poiBuffer);
     clReleaseMemObject(diff->resultsBuffer);
     clReleaseMemObject(diff->baseline);
     clReleaseMemObject(diff->test);
@@ -198,6 +227,14 @@ int SkDifferentPixelsImageDiffer::queueDiff(SkBitmap * baseline, SkBitmap * test
     return diffID;
 }
 
+void SkDifferentPixelsImageDiffer::deleteDiff(int id) {
+    QueuedDiff* diff = &fQueuedDiffs[id];
+    if (NULL != diff->poi) {
+        SkDELETE_ARRAY(diff->poi);
+        diff->poi = NULL;
+    }
+}
+
 bool SkDifferentPixelsImageDiffer::isFinished(int id) {
     return fQueuedDiffs[id].finished;
 }
@@ -206,6 +243,13 @@ double SkDifferentPixelsImageDiffer::getResult(int id) {
     return fQueuedDiffs[id].result;
 }
 
+int SkDifferentPixelsImageDiffer::getPointsOfInterestCount(int id) {
+    return fQueuedDiffs[id].numDiffPixels;
+}
+
+SkIPoint* SkDifferentPixelsImageDiffer::getPointsOfInterest(int id) {
+    return fQueuedDiffs[id].poi;
+}
 
 bool SkDifferentPixelsImageDiffer::onInit() {
     if (!loadKernelFile("experimental/skpdiff/diff_pixels.cl", "diff", &fKernel)) {
diff --git a/experimental/skpdiff/SkCLImageDiffer.h b/experimental/skpdiff/SkCLImageDiffer.h
index 565b3714ee..6509392a9c 100644
--- a/experimental/skpdiff/SkCLImageDiffer.h
+++ b/experimental/skpdiff/SkCLImageDiffer.h
@@ -93,20 +93,17 @@ class SkDifferentPixelsImageDiffer : public SkCLImageDiffer {
 public:
     virtual const char* getName() SK_OVERRIDE;
     virtual int queueDiff(SkBitmap* baseline, SkBitmap* test) SK_OVERRIDE;
+    virtual void deleteDiff(int id) SK_OVERRIDE;
     virtual bool isFinished(int id) SK_OVERRIDE;
     virtual double getResult(int id) SK_OVERRIDE;
+    virtual int getPointsOfInterestCount(int id) SK_OVERRIDE;
+    virtual SkIPoint* getPointsOfInterest(int id) SK_OVERRIDE;
 
 protected:
     virtual bool onInit() SK_OVERRIDE;
 
 private:
-    struct QueuedDiff {
-        bool finished;
-        double result;
-        cl_mem baseline;
-        cl_mem test;
-        cl_mem resultsBuffer;
-    };
+    struct QueuedDiff;
 
     SkTDArray<QueuedDiff> fQueuedDiffs;
     cl_kernel fKernel;
diff --git a/experimental/skpdiff/SkImageDiffer.cpp b/experimental/skpdiff/SkImageDiffer.cpp
index 41be949ac6..8a5c3c71a2 100644
--- a/experimental/skpdiff/SkImageDiffer.cpp
+++ b/experimental/skpdiff/SkImageDiffer.cpp
@@ -5,13 +5,10 @@
  * found in the LICENSE file.
  */
 
-#include <cstring>
-
 #include "SkBitmap.h"
 #include "SkImageDecoder.h"
 
 #include "SkImageDiffer.h"
-
 #include "skpdiff_util.h"
 
 
diff --git a/experimental/skpdiff/SkImageDiffer.h b/experimental/skpdiff/SkImageDiffer.h
index 86cf5bf801..c06538df17 100644
--- a/experimental/skpdiff/SkImageDiffer.h
+++ b/experimental/skpdiff/SkImageDiffer.h
@@ -9,6 +9,7 @@
 #define SkImageDiffer_DEFINED
 
 class SkBitmap;
+struct SkIPoint;
 
 /**
  * Encapsulates an image difference metric algorithm that can be potentially run asynchronously.
@@ -54,14 +55,35 @@ public:
     virtual bool isFinished(int id) = 0;
 
     /**
+     * Deletes memory associated with a diff and its results. This may block execution until the
+     * diff is finished,
+     * @param id The id of the diff to query
+     */
+    virtual void deleteDiff(int id) = 0;
+
+    /**
      * Gets the results of the queued diff of the given id. The results are only meaningful after
      * the queued diff has finished.
      * @param  id The id of the queued diff to query
-     * @return    A score between of how different the compared images are, with lower numbers being
-     *            more different.
      */
     virtual double getResult(int id) = 0;
 
+    /**
+     * Gets the number of points of interest for the diff of the given id. The results are only
+     * meaningful after the queued diff has finished.
+     * @param  id The id of the queued diff to query
+     */
+    virtual int getPointsOfInterestCount(int id) = 0;
+
+    /**
+     * Gets an array of the points of interest for the diff of the given id. The results are only
+     * meaningful after the queued diff has finished.
+     * @param  id The id of the queued diff to query
+     */
+    virtual SkIPoint* getPointsOfInterest(int id) = 0;
+
+
+
 protected:
     bool fIsGood;
 };
diff --git a/experimental/skpdiff/SkPMetric.cpp b/experimental/skpdiff/SkPMetric.cpp
index b674b15cca..710b18bd20 100644
--- a/experimental/skpdiff/SkPMetric.cpp
+++ b/experimental/skpdiff/SkPMetric.cpp
@@ -233,7 +233,7 @@ static void convolve(const ImageL* imageL,
     }
 }
 
-float pmetric(const ImageLAB* baselineLAB, const ImageLAB* testLAB) {
+float pmetric(const ImageLAB* baselineLAB, const ImageLAB* testLAB, SkTDArray<SkIPoint>* poi) {
     int width = baselineLAB->width;
     int height = baselineLAB->height;
     int maxLevels = (int)log2(width < height ? width : height);
@@ -371,6 +371,7 @@ float pmetric(const ImageLAB* baselineLAB, const ImageLAB* testLAB) {
 
             if (isFailure) {
                 failures++;
+                poi->push()->set(x, y);
             }
         }
     }
@@ -388,13 +389,13 @@ const char* SkPMetric::getName() {
 int SkPMetric::queueDiff(SkBitmap* baseline, SkBitmap* test) {
     int diffID = fQueuedDiffs.count();
     double startTime = get_seconds();
-    QueuedDiff* diff = fQueuedDiffs.push();
+    QueuedDiff& diff = fQueuedDiffs.push_back();
+    diff.result = 0.0;
 
     // Ensure the images are comparable
     if (baseline->width() != test->width() || baseline->height() != test->height() ||
                     baseline->width() <= 0 || baseline->height() <= 0) {
-        diff->finished = true;
-        diff->result = 0.0;
+        diff.finished = true;
         return diffID;
     }
 
@@ -404,7 +405,7 @@ int SkPMetric::queueDiff(SkBitmap* baseline, SkBitmap* test) {
     bitmap_to_cielab(baseline, &baselineLAB);
     bitmap_to_cielab(test, &testLAB);
 
-    diff->result = pmetric(&baselineLAB, &testLAB);
+    diff.result = pmetric(&baselineLAB, &testLAB, &diff.poi);
 
     SkDebugf("Time: %f\n", (get_seconds() - startTime));
 
@@ -412,6 +413,10 @@ int SkPMetric::queueDiff(SkBitmap* baseline, SkBitmap* test) {
 }
 
 
+void SkPMetric::deleteDiff(int id) {
+   fQueuedDiffs[id].poi.reset();
+}
+
 bool SkPMetric::isFinished(int id) {
     return fQueuedDiffs[id].finished;
 }
@@ -419,3 +424,11 @@ bool SkPMetric::isFinished(int id) {
 double SkPMetric::getResult(int id) {
     return fQueuedDiffs[id].result;
 }
+
+int SkPMetric::getPointsOfInterestCount(int id) {
+    return fQueuedDiffs[id].poi.count();
+}
+
+SkIPoint* SkPMetric::getPointsOfInterest(int id) {
+    return fQueuedDiffs[id].poi.begin();
+}
diff --git a/experimental/skpdiff/SkPMetric.h b/experimental/skpdiff/SkPMetric.h
index fb32db5568..5963c2f33c 100644
--- a/experimental/skpdiff/SkPMetric.h
+++ b/experimental/skpdiff/SkPMetric.h
@@ -8,6 +8,7 @@
 #ifndef SkPMetric_DEFINED
 #define SkPMetric_DEFINED
 
+#include "SkTArray.h"
 #include "SkTDArray.h"
 
 #include "SkImageDiffer.h"
@@ -19,16 +20,20 @@ class SkPMetric : public SkImageDiffer {
 public:
     virtual const char* getName() SK_OVERRIDE;
     virtual int queueDiff(SkBitmap* baseline, SkBitmap* test) SK_OVERRIDE;
+    virtual void deleteDiff(int id) SK_OVERRIDE;
     virtual bool isFinished(int id) SK_OVERRIDE;
     virtual double getResult(int id) SK_OVERRIDE;
+    virtual int getPointsOfInterestCount(int id) SK_OVERRIDE;
+    virtual SkIPoint* getPointsOfInterest(int id) SK_OVERRIDE;
 
 private:
     struct QueuedDiff {
         bool finished;
         double result;
+        SkTDArray<SkIPoint> poi;
     };
 
-    SkTDArray<QueuedDiff> fQueuedDiffs;
+    SkTArray<QueuedDiff> fQueuedDiffs;
 
     typedef SkImageDiffer INHERITED;
 };
diff --git a/experimental/skpdiff/diff_pixels.cl b/experimental/skpdiff/diff_pixels.cl
index c1ecb52356..1eed2a4216 100644
--- a/experimental/skpdiff/diff_pixels.cl
+++ b/experimental/skpdiff/diff_pixels.cl
@@ -11,7 +11,8 @@ const sampler_t gInSampler = CLK_NORMALIZED_COORDS_FALSE |
                              CLK_ADDRESS_CLAMP_TO_EDGE   |
                              CLK_FILTER_NEAREST;
 
-__kernel void diff(read_only image2d_t baseline, read_only image2d_t test, __global int* result) {
+__kernel void diff(read_only image2d_t baseline, read_only image2d_t test,
+                   __global int* result, __global int2* poi) {
     int2 coord = (int2)(get_global_id(0), get_global_id(1));
     uint4 baselinePixel = read_imageui(baseline, gInSampler, coord);
     uint4 testPixel = read_imageui(test, gInSampler, coord);
@@ -21,6 +22,7 @@ __kernel void diff(read_only image2d_t baseline, read_only image2d_t test, __glo
         baselinePixel.z != testPixel.z ||
         baselinePixel.w != testPixel.w) {
 
-        atom_inc(result);
+        int poiIndex = atomic_inc(result);
+        poi[poiIndex] = coord;
     }
 }
\ No newline at end of file
diff --git a/experimental/skpdiff/main.cpp b/experimental/skpdiff/main.cpp
index e5b8faea3a..b9d533df84 100644
--- a/experimental/skpdiff/main.cpp
+++ b/experimental/skpdiff/main.cpp
@@ -11,6 +11,7 @@
 
 #include "SkCommandLineFlags.h"
 #include "SkGraphics.h"
+#include "SkPoint.h"
 #include "SkOSFile.h"
 #include "SkString.h"
 #include "SkTArray.h"
@@ -83,7 +84,7 @@ static void diff_directories(const char baselinePath[], const char testPath[], S
     SkTDArray<int> queuedDiffIDs;
     for (int baselineIndex = 0; baselineIndex < baselineEntries.count(); baselineIndex++) {
         const char* baseFilename = baselineEntries[baselineIndex].c_str();
-        SkDebugf("%s\n", baseFilename);
+        SkDebugf("\n%s\n", baseFilename);
 
         // Find the real location of each file to compare
         SkString baselineFile = SkOSPath::SkPathJoin(baselinePath, baseFilename);
@@ -96,6 +97,8 @@ static void diff_directories(const char baselinePath[], const char testPath[], S
             if (diffID >= 0) {
                 queuedDiffIDs.push(diffID);
                 SkDebugf("Result: %f\n", differ->getResult(diffID));
+                SkDebugf("POI Count: %i\n", differ->getPointsOfInterestCount(diffID));
+                differ->deleteDiff(diffID);
             }
         } else {
             SkDebugf("Baseline file \"%s\" has no corresponding test file\n", baselineFile.c_str());
@@ -130,12 +133,14 @@ static void diff_patterns(const char baselinePattern[], const char testPattern[]
     for (int entryIndex = 0; entryIndex < baselineEntries.count(); entryIndex++) {
         const char* baselineFilename = baselineEntries[entryIndex].c_str();
         const char* testFilename     = testEntries    [entryIndex].c_str();
-        SkDebugf("%s %s\n", baselineFilename, testFilename);
+        SkDebugf("\n%s %s\n", baselineFilename, testFilename);
 
         int diffID = differ->queueDiffOfFile(baselineFilename, testFilename);
         if (diffID >= 0) {
             queuedDiffIDs.push(diffID);
             SkDebugf("Result: %f\n", differ->getResult(diffID));
+            SkDebugf("POI Count: %i\n", differ->getPointsOfInterestCount(diffID));
+            differ->deleteDiff(diffID);
         }
     }
 }