fix multithread related crashes in skpdiff

tools/skpdiff/SkImageDiffer.h

Index: tools/skpdiff/SkImageDiffer.h
diff --git a/tools/skpdiff/SkImageDiffer.h b/tools/skpdiff/SkImageDiffer.h
index 2c1fa7e78853cdb3b15f7f97c79ce1719db45632..df736d90154f9dab56fcaa506e678be3caa7d262 100644
--- a/tools/skpdiff/SkImageDiffer.h
+++ b/tools/skpdiff/SkImageDiffer.h
@@ -8,8 +8,7 @@
 #ifndef SkImageDiffer_DEFINED
 #define SkImageDiffer_DEFINED
 
-class SkBitmap;
-struct SkIPoint;
+#include "SkBitmap.h"
 
 /**
  * Encapsulates an image difference metric algorithm that can be potentially run asynchronously.
@@ -22,6 +21,13 @@ public:
     static const double RESULT_CORRECT;
     static const double RESULT_INCORRECT;
 
+    struct Result {
+        double result;
+        int poiCount;
+        SkBitmap poiAlphaMask; // optional
+        double timeElapsed; // optional
+    };
+
     /**
      * Gets a unique and descriptive name of this differ
      * @return A statically allocated null terminated string that is the name of this differ
@@ -46,64 +52,12 @@ public:
     virtual bool enablePOIAlphaMask() { return false; }

[mtklein, 2013/11/08 15:06:13]

Param to diff?

[djsollen, 2013/11/12 16:40:43]

Done.

 
     /**
-     * Wraps a call to queueDiff by loading the given filenames into SkBitmaps
-     * @param  baseline The file path of the baseline image
-     * @param  test     The file path of the test image
-     * @return          The results of queueDiff with the loaded bitmaps
-     */
-    int queueDiffOfFile(const char baseline[], const char test[]);
-
-    /**
-     * Queues a diff on a pair of bitmaps to be done at some future time.
+     * diff on a pair of bitmaps.
      * @param  baseline The correct bitmap
      * @param  test     The bitmap whose difference is being tested
-     * @return          An non-negative diff ID on success, a negative integer on failure.
-     */
-    virtual int queueDiff(SkBitmap* baseline, SkBitmap* test) = 0;
-
-    /**
-     * Gets whether a queued diff of the given id has finished
-     * @param  id The id of the queued diff to query
-     * @return    True if the queued diff is finished and has results, false otherwise
-     */
-    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
-     */
-    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
+     * @return          true on success, and false in the case of failure
      */
-    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;
-
-    /*
-     * Gets a bitmap containing an alpha mask containing transparent pixels at 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 SkBitmap* getPointsOfInterestAlphaMask(int id) { return NULL; }
-
+    virtual bool diff(SkBitmap* baseline, SkBitmap* test, Result* result) = 0;

[mtklein, 2013/11/08 15:06:13]

can this be const?  if not, it's a fairly good sign it's not yet threadsafe.

[mtklein, 2013/11/08 15:06:13]

do these bitmaps need to be SkBitmap*?  const SkBitmap*?  const SkBitmap&?

[djsollen, 2013/11/12 16:40:43]

the function is now const but the input params can't be const as we need to call
lockPixels on them.  I could require that the caller lock the pixels to get
around this.

[mtklein, 2013/11/12 17:00:19]

gotcha.  seems fine as-is.

 
 protected:
     bool fIsGood;

[mtklein, 2013/11/08 15:06:13]

Can die?