Changes to make SkPDiff more robost.
authorscroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 12 Nov 2013 14:41:20 +0000 (14:41 +0000)
committerscroggo@google.com <scroggo@google.com@2bbb7eff-a529-9590-31e7-b0007b416f81>
Tue, 12 Nov 2013 14:41:20 +0000 (14:41 +0000)
bitmap_to_cielab now returns a boolean. Instead of asserting that the
config is 8888, copy to 8888 (and return false on failure). This allows
skpdiff to work on Index8 bitmaps (without changing the code that does
the real work). If it returns false, do not attempt to create a pmetric.

In pmetric, exit early if maxLevels is <= 2, since this would crash later
in the function (creation of an ImageL3D if maxLevels is 0, and then the
creation of an array of a negative number of entries). maxLevels is small
if the width or height of the image is small (i.e. a bitmap with width 1
has maxLevels of 0).

R=djsollen@google.com

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

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

tools/skpdiff/SkPMetric.cpp

index a19ed82..5120e12 100644 (file)
@@ -123,8 +123,14 @@ static void adobergb_to_cielab(float r, float g, float b, LAB* lab) {
 }
 
 /// Converts a 8888 bitmap to LAB color space and puts it into the output
-static void bitmap_to_cielab(const SkBitmap* bitmap, ImageLAB* outImageLAB) {
-    SkASSERT(bitmap->config() == SkBitmap::kARGB_8888_Config);
+static bool bitmap_to_cielab(const SkBitmap* bitmap, ImageLAB* outImageLAB) {
+    SkBitmap bm8888;
+    if (bitmap->config() != SkBitmap::kARGB_8888_Config) {
+        if (!bitmap->copyTo(&bm8888, SkBitmap::kARGB_8888_Config)) {
+            return false;
+        }
+        bitmap = &bm8888;
+    }
 
     int width = bitmap->width();
     int height = bitmap->height();
@@ -146,6 +152,7 @@ static void bitmap_to_cielab(const SkBitmap* bitmap, ImageLAB* outImageLAB) {
         }
     }
     bitmap->unlockPixels();
+    return true;
 }
 
 // From Barten SPIE 1989
@@ -269,6 +276,12 @@ static double pmetric(const ImageLAB* baselineLAB, const ImageLAB* testLAB, SkTD
         maxLevels++;
     }
 
+    // We'll be creating new arrays with maxLevels - 2, and ImageL3D requires maxLevels > 0,
+    // so just return failure if we're less than 3.
+    if (maxLevels <= 2) {
+        return 0.0;
+    }
+
     const float fov = SK_ScalarPI / 180.0f * 45.0f;
     float contrastSensitivityMax = contrast_sensitivity(3.248f, 100.0f);
     float pixelsPerDegree = width / (2.0f * tanf(fov * 0.5f) * 180.0f / SK_ScalarPI);
@@ -444,8 +457,9 @@ int SkPMetric::queueDiff(SkBitmap* baseline, SkBitmap* test) {
     ImageLAB baselineLAB(baseline->width(), baseline->height());
     ImageLAB testLAB(baseline->width(), baseline->height());
 
-    bitmap_to_cielab(baseline, &baselineLAB);
-    bitmap_to_cielab(test, &testLAB);
+    if (!bitmap_to_cielab(baseline, &baselineLAB) || !bitmap_to_cielab(test, &testLAB)) {
+        return diffID;
+    }
 
     diff.result = pmetric(&baselineLAB, &testLAB, &diff.poi);