Add Gray support to SkPNGImageEncoder

BUG=skia:5616
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2290843002

Review-Url: https://codereview.chromium.org/2290843002

diff --git a/src/images/SkPNGImageEncoder.cpp b/src/images/SkPNGImageEncoder.cpp
index 1932e66..a9c8b3d 100644 (file)
--- a/src/images/SkPNGImageEncoder.cpp
+++ b/src/images/SkPNGImageEncoder.cpp
@@ -76,6 +76,7 @@ static transform_scanline_proc choose_proc(SkColorType ct, bool hasAlpha) {
         { kARGB_4444_SkColorType,   false,  transform_scanline_444 },
         { kARGB_4444_SkColorType,   true,   transform_scanline_4444 },
         { kIndex_8_SkColorType,     false,  transform_scanline_memcpy },
+        { kGray_8_SkColorType,      false,  transform_scanline_memcpy },
     };
 
     for (int i = SK_ARRAY_COUNT(gMap) - 1; i >= 0; --i) {
@@ -178,13 +179,13 @@ bool SkPNGImageEncoder::onEncode(SkWStream* stream,
     const SkBitmap* bitmap = &originalBitmap;
     switch (originalBitmap.colorType()) {
         case kIndex_8_SkColorType:
+        case kGray_8_SkColorType:
         case kN32_SkColorType:
         case kARGB_4444_SkColorType:
         case kRGB_565_SkColorType:
             break;
         default:
             // TODO(scroggo): support 8888-but-not-N32 natively.
-            // TODO(scroggo): support kGray_8 directly.
             // TODO(scroggo): support Alpha_8 as Grayscale(black)+Alpha
             if (originalBitmap.copyTo(&copy, kN32_SkColorType)) {
                 bitmap = ©
@@ -193,45 +194,49 @@ bool SkPNGImageEncoder::onEncode(SkWStream* stream,
     SkColorType ct = bitmap->colorType();
 
     const bool hasAlpha = !bitmap->isOpaque();
-    int colorType = PNG_COLOR_MASK_COLOR;
     int bitDepth = 8;   // default for color
     png_color_8 sig_bit;
+    sk_bzero(&sig_bit, sizeof(png_color_8));
 
+    int colorType;
     switch (ct) {
         case kIndex_8_SkColorType:
-            colorType |= PNG_COLOR_MASK_PALETTE;
-            // fall through to the ARGB_8888 case
+            sig_bit.red = 8;
+            sig_bit.green = 8;
+            sig_bit.blue = 8;
+            sig_bit.alpha = 8;
+            colorType = PNG_COLOR_TYPE_PALETTE;
+            break;
+        case kGray_8_SkColorType:
+            sig_bit.gray = 8;
+            colorType = PNG_COLOR_TYPE_GRAY;
+            SkASSERT(!hasAlpha);
+            break;
         case kN32_SkColorType:
             sig_bit.red = 8;
             sig_bit.green = 8;
             sig_bit.blue = 8;
             sig_bit.alpha = 8;
+            colorType = hasAlpha ? PNG_COLOR_TYPE_RGB_ALPHA : PNG_COLOR_TYPE_RGB;
             break;
         case kARGB_4444_SkColorType:
             sig_bit.red = 4;
             sig_bit.green = 4;
             sig_bit.blue = 4;
             sig_bit.alpha = 4;
+            colorType = hasAlpha ? PNG_COLOR_TYPE_RGB_ALPHA : PNG_COLOR_TYPE_RGB;
             break;
         case kRGB_565_SkColorType:
             sig_bit.red = 5;
             sig_bit.green = 6;
             sig_bit.blue = 5;
-            sig_bit.alpha = 0;
+            colorType = PNG_COLOR_TYPE_RGB;
+            SkASSERT(!hasAlpha);
             break;
         default:
             return false;
     }
 
-    if (hasAlpha) {
-        // don't specify alpha if we're a palette, even if our ctable has alpha
-        if (!(colorType & PNG_COLOR_MASK_PALETTE)) {
-            colorType |= PNG_COLOR_MASK_ALPHA;
-        }
-    } else {
-        sig_bit.alpha = 0;
-    }
-
     SkAutoLockPixels alp(*bitmap);
     // readyToDraw checks for pixels (and colortable if that is required)
     if (!bitmap->readyToDraw()) {
@@ -306,9 +311,8 @@ bool SkPNGImageEncoder::doEncode(SkWStream* stream, const SkBitmap& bitmap,
             png_set_tRNS(png_ptr, info_ptr, trans, numTrans, nullptr);
         }
     }
-#ifdef PNG_sBIT_SUPPORTED
+
     png_set_sBIT(png_ptr, info_ptr, &sig_bit);
-#endif
     png_write_info(png_ptr, info_ptr);
 
     const char* srcImage = (const char*)bitmap.getPixels();

diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 8023ff2..fcbfadd 100644 (file)
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1074,29 +1074,22 @@ DEF_TEST(Codec_ColorXform, r) {
     check_color_xform(r, "mandrill_512.png");
 }
 
-DEF_TEST(Codec_Png565, r) {
-    // Create an arbitrary 565 bitmap.
-    const char* path = "mandrill_512_q075.jpg";
-    SkAutoTDelete<SkStream> stream(resource(path));
-    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
-    SkImageInfo info565 = codec->getInfo().makeColorType(kRGB_565_SkColorType);
-    SkBitmap bm1;
-    bm1.allocPixels(info565);
-    SkCodec::Result result = codec->getPixels(info565, bm1.getPixels(), bm1.rowBytes());
-    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+static void check_round_trip(skiatest::Reporter* r, const SkBitmap& bm1) {
+    SkColorType origColorType = bm1.colorType();
+    SkAlphaType origAlphaType = bm1.alphaType();
 
     // Encode the image to png.
     sk_sp<SkData> data =
             sk_sp<SkData>(SkImageEncoder::EncodeData(bm1, SkImageEncoder::kPNG_Type, 100));
 
     // Prepare to decode.  The codec should recognize that the PNG is 565.
-    codec.reset(SkCodec::NewFromData(data.get()));
-    REPORTER_ASSERT(r, kRGB_565_SkColorType == codec->getInfo().colorType());
-    REPORTER_ASSERT(r, kOpaque_SkAlphaType == codec->getInfo().alphaType());
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(data.get()));
+    REPORTER_ASSERT(r, origColorType == codec->getInfo().colorType());
+    REPORTER_ASSERT(r, origAlphaType == codec->getInfo().alphaType());
 
     SkBitmap bm2;
     bm2.allocPixels(codec->getInfo());
-    result = codec->getPixels(codec->getInfo(), bm2.getPixels(), bm2.rowBytes());
+    SkCodec::Result result = codec->getPixels(codec->getInfo(), bm2.getPixels(), bm2.rowBytes());
     REPORTER_ASSERT(r, SkCodec::kSuccess == result);
 
     SkMD5::Digest d1, d2;
@@ -1104,3 +1097,26 @@ DEF_TEST(Codec_Png565, r) {
     md5(bm2, &d2);
     REPORTER_ASSERT(r, d1 == d2);
 }
+
+DEF_TEST(Codec_PngRoundTrip, r) {
+    // Create an arbitrary 565 bitmap.
+    const char* path = "mandrill_512_q075.jpg";
+    SkAutoTDelete<SkStream> stream(resource(path));
+    SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.release()));
+    SkImageInfo info565 = codec->getInfo().makeColorType(kRGB_565_SkColorType);
+    SkBitmap bm1;
+    bm1.allocPixels(info565);
+    SkCodec::Result result = codec->getPixels(info565, bm1.getPixels(), bm1.rowBytes());
+    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+    check_round_trip(r, bm1);
+
+    // Create an arbitrary gray bitmap.
+    path = "grayscale.jpg";
+    stream.reset(resource(path));
+    codec.reset(SkCodec::NewFromStream(stream.release()));
+    SkBitmap bm2;
+    bm2.allocPixels(codec->getInfo());
+    result = codec->getPixels(codec->getInfo(), bm2.getPixels(), bm2.rowBytes());
+    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+    check_round_trip(r, bm2);
+}