gm: force all pixels opaque earlier in the process

Review URL: https://codereview.appspot.com/7251043

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

1 files changed, 100 insertions, 79 deletions

diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 98234fa875..8ded7c1662 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -16,6 +16,7 @@
 #include "gm.h"
 #include "gm_expectations.h"
 #include "system_preferences.h"
+#include "SkBitmap.h"
 #include "SkBitmapChecksummer.h"
 #include "SkColorPriv.h"
 #include "SkData.h"
@@ -216,79 +217,26 @@ public:
         return name;
     }
 
-    /**
-     * All calls to get bitmap checksums must go through this
-     * wrapper, so that we can call force_all_opaque() first.
-     * THIS WILL MODIFY THE BITMAP IN-PLACE!
-     *
-     * It's too bad that we are throwing away alpha channel data
-     * we could otherwise be examining, but this had always been happening
-     * before... it was buried within the compare() method at
-     * https://code.google.com/p/skia/source/browse/trunk/gm/gmmain.cpp?r=7289#305 .
-     *
-     * Apparently we need this, at least for bitmaps that are either:
-     * (a) destined to be written out as PNG files, or
-     * (b) compared against bitmaps read in from PNG files
-     * for the reasons described just above the force_all_opaque() method.
-     *
-     * Neglecting to do this led to the difficult-to-diagnose
-     * http://code.google.com/p/skia/issues/detail?id=1079 ('gm generating
-     * spurious pixel_error messages as of r7258')
-     *
-     * TODO(epoger): Come up with a better solution that allows us to
-     * compare full pixel data, including alpha channel, while still being
-     * robust in the face of transformations to/from PNG files.
-     * Options include:
-     *
-     * 1. Continue to call force_all_opaque(), but ONLY for bitmaps that
-     *    will be written to, or compared against, PNG files.
-     *    PRO: Preserve/compare alpha channel info for the non-PNG cases
-     *         (comparing different renderModes in-memory)
-     *    CON: The bitmaps (and checksums) for these non-PNG cases would be
-     *         different than those for the PNG-compared cases, and in the
-     *         case of a failed renderMode comparison, how would we write the
-     *         image to disk for examination?
-     *
-     * 2. Always compute image checksums from PNG format (either
-     *    directly from the the bytes of a PNG file, or capturing the
-     *    bytes we would have written to disk if we were writing the
-     *    bitmap out as a PNG).
-     *    PRO: I think this would allow us to never force opaque, and to
-     *         the extent that alpha channel data can be preserved in a PNG
-     *         file, we could observe it.
-     *    CON: If we read a bitmap from disk, we need to take its checksum
-     *         from the source PNG (we can't compute it from the bitmap we
-     *         read out of the PNG, because we will have already premultiplied
-     *         the alpha).
-     *    CON: Seems wasteful to convert a bitmap to PNG format just to take
-     *         its checksum. (Although we're wasting lots of effort already
-     *         calling force_all_opaque().)
-     *
-     * 3. Make the alpha premultiply/unpremultiply routines 100% consistent,
-     *    so we can transform images back and forth without fear of off-by-one
-     *    errors.
-     *    CON: Math is hard.
-     *
-     * 4. Perform a "close enough" comparison of bitmaps (+/- 1 bit in each
-     *    channel), rather than demanding absolute equality.
-     *    CON: Can't do this with checksums.
-     */
-    static Checksum get_checksum(const SkBitmap& bitmap) {
-        SkBitmap copy;
-        const SkBitmap* bm = &bitmap;
-        if (bitmap.config() != SkBitmap::kARGB_8888_Config) {
-            bitmap.copyTo(&copy, SkBitmap::kARGB_8888_Config);
-            bm = ©
-        }
-        force_all_opaque(*bm);
-        return SkBitmapChecksummer::Compute64(*bm);
-    }
-
     /* since PNG insists on unpremultiplying our alpha, we take no
        precision chances and force all pixels to be 100% opaque,
        otherwise on compare we may not get a perfect match.
     */
     static void force_all_opaque(const SkBitmap& bitmap) {
+        SkBitmap::Config config = bitmap.config();
+        switch (config) {
+        case SkBitmap::kARGB_8888_Config:
+            force_all_opaque_8888(bitmap);
+            break;
+        case SkBitmap::kRGB_565_Config:
+            // nothing to do here; 565 bitmaps are inherently opaque
+            break;
+        default:
+            fprintf(stderr, "unsupported bitmap config %d\n", config);
+            SkDEBUGFAIL("unsupported bitmap config");
+        }
+    }
+
+    static void force_all_opaque_8888(const SkBitmap& bitmap) {
         SkAutoLockPixels lock(bitmap);
         for (int y = 0; y < bitmap.height(); y++) {
             for (int x = 0; x < bitmap.width(); x++) {
@@ -298,9 +246,11 @@ public:
     }
 
     static bool write_bitmap(const SkString& path, const SkBitmap& bitmap) {
+        // TODO(epoger): Now that we have removed force_all_opaque()
+        // from this method, we should be able to get rid of the
+        // transformation to 8888 format also.
         SkBitmap copy;
         bitmap.copyTo(&copy, SkBitmap::kARGB_8888_Config);
-        force_all_opaque(copy);
         return SkImageEncoder::EncodeFile(path.c_str(), copy,
                                           SkImageEncoder::kPNG_Type, 100);
     }
@@ -346,12 +296,17 @@ public:
         return stream.writeData(data.get());
     }
 
-    /// Returns true if processing should continue, false to skip the
-    /// remainder of this config for this GM.
-    //@todo thudson 22 April 2011 - could refactor this to take in
-    // a factory to generate the context, always call readPixels()
-    // (logically a noop for rasters, if wasted time), and thus collapse the
-    // GPU special case and also let this be used for SkPicture testing.
+    /**
+     * Prepare an SkBitmap to render a GM into.
+     *
+     * After you've rendered the GM into the SkBitmap, you must call
+     * complete_bitmap()!
+     *
+     * @todo thudson 22 April 2011 - could refactor this to take in
+     * a factory to generate the context, always call readPixels()
+     * (logically a noop for rasters, if wasted time), and thus collapse the
+     * GPU special case and also let this be used for SkPicture testing.
+     */
     static void setup_bitmap(const ConfigData& gRec, SkISize& size,
                              SkBitmap* bitmap) {
         bitmap->setConfig(gRec.fConfig, size.width(), size.height());
@@ -359,6 +314,66 @@ public:
         bitmap->eraseColor(SK_ColorTRANSPARENT);
     }
 
+    /**
+     * Any finalization steps we need to perform on the SkBitmap after
+     * we have rendered the GM into it.
+     *
+     * It's too bad that we are throwing away alpha channel data
+     * we could otherwise be examining, but this had always been happening
+     * before... it was buried within the compare() method at
+     * https://code.google.com/p/skia/source/browse/trunk/gm/gmmain.cpp?r=7289#305 .
+     *
+     * Apparently we need this, at least for bitmaps that are either:
+     * (a) destined to be written out as PNG files, or
+     * (b) compared against bitmaps read in from PNG files
+     * for the reasons described just above the force_all_opaque() method.
+     *
+     * Neglecting to do this led to the difficult-to-diagnose
+     * http://code.google.com/p/skia/issues/detail?id=1079 ('gm generating
+     * spurious pixel_error messages as of r7258')
+     *
+     * TODO(epoger): Come up with a better solution that allows us to
+     * compare full pixel data, including alpha channel, while still being
+     * robust in the face of transformations to/from PNG files.
+     * Options include:
+     *
+     * 1. Continue to call force_all_opaque(), but ONLY for bitmaps that
+     *    will be written to, or compared against, PNG files.
+     *    PRO: Preserve/compare alpha channel info for the non-PNG cases
+     *         (comparing different renderModes in-memory)
+     *    CON: The bitmaps (and checksums) for these non-PNG cases would be
+     *         different than those for the PNG-compared cases, and in the
+     *         case of a failed renderMode comparison, how would we write the
+     *         image to disk for examination?
+     *
+     * 2. Always compute image checksums from PNG format (either
+     *    directly from the the bytes of a PNG file, or capturing the
+     *    bytes we would have written to disk if we were writing the
+     *    bitmap out as a PNG).
+     *    PRO: I think this would allow us to never force opaque, and to
+     *         the extent that alpha channel data can be preserved in a PNG
+     *         file, we could observe it.
+     *    CON: If we read a bitmap from disk, we need to take its checksum
+     *         from the source PNG (we can't compute it from the bitmap we
+     *         read out of the PNG, because we will have already premultiplied
+     *         the alpha).
+     *    CON: Seems wasteful to convert a bitmap to PNG format just to take
+     *         its checksum. (Although we're wasting lots of effort already
+     *         calling force_all_opaque().)
+     *
+     * 3. Make the alpha premultiply/unpremultiply routines 100% consistent,
+     *    so we can transform images back and forth without fear of off-by-one
+     *    errors.
+     *    CON: Math is hard.
+     *
+     * 4. Perform a "close enough" comparison of bitmaps (+/- 1 bit in each
+     *    channel), rather than demanding absolute equality.
+     *    CON: Can't do this with checksums.
+     */
+    static void complete_bitmap(SkBitmap* bitmap) {
+        force_all_opaque(*bitmap);
+    }
+
     static void installFilter(SkCanvas* canvas) {
         if (gForceBWtext) {
             canvas->setDrawFilter(new BWTextDrawFilter)->unref();
@@ -417,6 +432,7 @@ public:
             canvas->readPixels(bitmap, 0, 0);
         }
 #endif
+        complete_bitmap(bitmap);
         return ERROR_NONE;
     }
 
@@ -429,6 +445,7 @@ public:
         installFilter(&canvas);
         canvas.scale(scale, scale);
         canvas.drawPicture(*pict);
+        complete_bitmap(bitmap);
     }
 
     static void generate_pdf(GM* gm, SkDynamicMemoryWStream& pdf) {
@@ -544,7 +561,7 @@ public:
                                           const char renderModeDescriptor[],
                                           bool addToJsonSummary=false) {
         ErrorBitfield retval;
-        Checksum actualChecksum = get_checksum(actualBitmap);
+        Checksum actualChecksum = SkBitmapChecksummer::Compute64(actualBitmap);
         SkString completeNameString = baseNameString;
         completeNameString.append(renderModeDescriptor);
         const char* completeName = completeNameString.c_str();
@@ -674,7 +691,7 @@ public:
              * not, the checksum returned here may not match the
              * checksum of actualBitmap, which *has* been run through
              * force_all_opaque().
-             * See comments above get_checksum() for more detail.
+             * See comments above complete_bitmap() for more detail.
              */
             Expectations expectations = expectationsSource->get(name.c_str());
             retval |= compare_to_expectations(expectations, actualBitmap,
@@ -682,7 +699,8 @@ public:
         } else {
             // If we are running without expectations, we still want to
             // record the actual results.
-            Checksum actualChecksum = get_checksum(actualBitmap);
+            Checksum actualChecksum =
+                SkBitmapChecksummer::Compute64(actualBitmap);
             add_actual_results_to_json_summary(name.c_str(), actualChecksum,
                                                ERROR_READING_REFERENCE_IMAGE,
                                                false);
@@ -715,7 +733,8 @@ public:
 
         SkASSERT(referenceBitmap);
         SkString name = make_name(gm->shortName(), gRec.fName);
-        Checksum referenceChecksum = get_checksum(*referenceBitmap);
+        Checksum referenceChecksum =
+            SkBitmapChecksummer::Compute64(*referenceBitmap);
         Expectations expectations(referenceChecksum);
         return compare_to_expectations(expectations, actualBitmap,
                                        name, renderModeDescriptor);
@@ -836,6 +855,7 @@ public:
             SkCanvas* pipeCanvas = writer.startRecording(
               &pipeController, gPipeWritingFlagCombos[i].flags);
             invokeGM(gm, pipeCanvas, false, false);
+            complete_bitmap(&bitmap);
             writer.endRecording();
             SkString string("-pipe");
             string.append(gPipeWritingFlagCombos[i].name);
@@ -861,6 +881,7 @@ public:
             SkCanvas* pipeCanvas = writer.startRecording(
               &pipeController, gPipeWritingFlagCombos[i].flags);
             invokeGM(gm, pipeCanvas, false, false);
+            complete_bitmap(&bitmap);
             writer.endRecording();
             SkString string("-tiled pipe");
             string.append(gPipeWritingFlagCombos[i].name);