fix multithread related crashes in skpdiff

tools/skpdiff/SkImageDiffer.h

 /*
  * Copyright 2013 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #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.
  */
 class SkImageDiffer {
 public:
     SkImageDiffer();
     virtual ~SkImageDiffer();
 
     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
      */
     virtual const char* getName() = 0;

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

Make this const?

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

Done.

 
     /**
      * Gets if this differ is in a usable state
      * @return True if this differ can be used, false otherwise
      */
     bool isGood() { return fIsGood; }

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

Can go away now?

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

Done.

 
     /**
      * Gets if this differ needs to be initialized with and OpenCL device and context.
      */
     virtual bool requiresOpenCL() { return false; }

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

const?

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

Done.

 
     /**
      * Enables the generation of an alpha mask for all points of interest.
      * @return True if the differ supports generating an alpha mask and false otherwise.
      */
     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.
+     * @return          true on success, and false in the case of 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
-     */
-    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?

 };
 
 
 #endif