Modify SkPngCodec to recognize 565 images from the sBIT chunk

Conveniently, SkPngImageEncoder already writes the sBIT chunk
appropriately.

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

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

diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index c5dc66a4743ff7443b18b97023da5b318bd36456..ced0c630e0816a1088bf1ec7f934a8c7e8548030 100644 (file)
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -526,6 +526,15 @@ protected:
             sk_sp<SkColorSpace> = nullptr,
             Origin = kTopLeft_Origin);
 
+    /**
+     *  Takes ownership of SkStream*
+     *  Allows the subclass to set the recommended SkImageInfo
+     */
+    SkCodec(const SkEncodedInfo&,
+            const SkImageInfo&,
+            SkStream*,
+            Origin = kTopLeft_Origin);
+
     virtual SkISize onGetScaledDimensions(float /*desiredScale*/) const {
         // By default, scaling is not supported.
         return this->getInfo().dimensions();

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index e5f9a07f6bcce9f1f34029a95081ca1009fc0b53..3192a6f4be8ad6127d1e1ba63f21d70f976ba6ff 100644 (file)
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -126,6 +126,18 @@ SkCodec::SkCodec(int width, int height, const SkEncodedInfo& info, SkStream* str
     , fCurrScanline(-1)
 {}
 
+SkCodec::SkCodec(const SkEncodedInfo& info, const SkImageInfo& imageInfo, SkStream* stream,
+        Origin origin)
+    : fEncodedInfo(info)
+    , fSrcInfo(imageInfo)
+    , fStream(stream)
+    , fNeedsRewind(false)
+    , fOrigin(origin)
+    , fDstInfo()
+    , fOptions()
+    , fCurrScanline(-1)
+{}
+
 SkCodec::~SkCodec() {}
 
 bool SkCodec::rewindIfNeeded() {

diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 60f7b9b866518dc5ee6679f0a8a1028524417f61..0d13e6ac2e70fc73606e99e65e3e15c36f67f6db 100644 (file)
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -370,11 +370,10 @@ void SkPngCodec::allocateStorage(const SkImageInfo& dstInfo) {
 
 class SkPngNormalCodec : public SkPngCodec {
 public:
-    SkPngNormalCodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
-            SkPngChunkReader* chunkReader, png_structp png_ptr, png_infop info_ptr, int bitDepth,
-            sk_sp<SkColorSpace> colorSpace)
-        : INHERITED(width, height, info, stream, chunkReader, png_ptr, info_ptr, bitDepth, 1,
-                    colorSpace)
+    SkPngNormalCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
+            SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
+            png_infop info_ptr, int bitDepth)
+        : INHERITED(encodedInfo, imageInfo, stream, chunkReader, png_ptr, info_ptr, bitDepth, 1)
     {}
 
     Result onStartScanlineDecode(const SkImageInfo& dstInfo, const Options& options,
@@ -447,11 +446,11 @@ public:
 
 class SkPngInterlacedCodec : public SkPngCodec {
 public:
-    SkPngInterlacedCodec(int width, int height, const SkEncodedInfo& info,
+    SkPngInterlacedCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
             SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
-            png_infop info_ptr, int bitDepth, int numberPasses, sk_sp<SkColorSpace> colorSpace)
-        : INHERITED(width, height, info, stream, chunkReader, png_ptr, info_ptr, bitDepth,
-                    numberPasses, colorSpace)
+            png_infop info_ptr, int bitDepth, int numberPasses)
+        : INHERITED(encodedInfo, imageInfo, stream, chunkReader, png_ptr, info_ptr, bitDepth,
+                    numberPasses)
         , fCanSkipRewind(false)
     {
         SkASSERT(numberPasses != 1);
@@ -723,30 +722,41 @@ static bool read_header(SkStream* stream, SkPngChunkReader* chunkReader, SkCodec
             colorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
         }
 
-        SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
+        SkEncodedInfo encodedInfo = SkEncodedInfo::Make(color, alpha, 8);
+        SkImageInfo imageInfo = encodedInfo.makeImageInfo(origWidth, origHeight, colorSpace);
+
+        if (SkEncodedInfo::kOpaque_Alpha == alpha) {
+            png_color_8p sigBits;
+            if (png_get_sBIT(png_ptr, info_ptr, &sigBits)) {
+                if (5 == sigBits->red && 6 == sigBits->green && 5 == sigBits->blue) {
+                    // Recommend a decode to 565 if the sBIT indicates 565.
+                    imageInfo = imageInfo.makeColorType(kRGB_565_SkColorType);
+                }
+            }
+        }
 
         if (1 == numberPasses) {
-            *outCodec = new SkPngNormalCodec(origWidth, origHeight, info, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth, colorSpace);
+            *outCodec = new SkPngNormalCodec(encodedInfo, imageInfo, stream,
+                    chunkReader, png_ptr, info_ptr, bitDepth);
         } else {
-            *outCodec = new SkPngInterlacedCodec(origWidth, origHeight, info, stream,
-                    chunkReader, png_ptr, info_ptr, bitDepth, numberPasses, colorSpace);
+            *outCodec = new SkPngInterlacedCodec(encodedInfo, imageInfo, stream,
+                    chunkReader, png_ptr, info_ptr, bitDepth, numberPasses);
         }
     }
 
     return true;
 }
 
-SkPngCodec::SkPngCodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
-                       SkPngChunkReader* chunkReader, png_structp png_ptr, png_infop info_ptr,
-                       int bitDepth, int numberPasses, sk_sp<SkColorSpace> colorSpace)
-    : INHERITED(width, height, info, stream, colorSpace)
+SkPngCodec::SkPngCodec(const SkEncodedInfo& encodedInfo, const SkImageInfo& imageInfo,
+                       SkStream* stream, SkPngChunkReader* chunkReader, png_structp png_ptr,
+                       png_infop info_ptr, int bitDepth, int numberPasses)
+    : INHERITED(encodedInfo, imageInfo, stream)
     , fPngChunkReader(SkSafeRef(chunkReader))
     , fPng_ptr(png_ptr)
     , fInfo_ptr(info_ptr)
     , fSwizzlerSrcRow(nullptr)
     , fColorXformSrcRow(nullptr)
-    , fSrcRowBytes(width * (bytes_per_pixel(this->getEncodedInfo().bitsPerPixel())))
+    , fSrcRowBytes(imageInfo.width() * (bytes_per_pixel(this->getEncodedInfo().bitsPerPixel())))
     , fNumberPasses(numberPasses)
     , fBitDepth(bitDepth)
 {}

diff --git a/src/codec/SkPngCodec.h b/src/codec/SkPngCodec.h
index 69ecef1313d0e3d73105a6608f0f790aa97adad1..b689f6fbae0aa6c692b849ddce259a9f0df0db87 100644 (file)
--- a/src/codec/SkPngCodec.h
+++ b/src/codec/SkPngCodec.h
@@ -46,8 +46,8 @@ protected:
     virtual int readRows(const SkImageInfo& dstInfo, void* dst, size_t rowBytes, int count,
                          int startRow) = 0;
 
-    SkPngCodec(int width, int height, const SkEncodedInfo&, SkStream*, SkPngChunkReader*,
-            png_structp, png_infop, int, int, sk_sp<SkColorSpace>);
+    SkPngCodec(const SkEncodedInfo&, const SkImageInfo&, SkStream*, SkPngChunkReader*,
+            png_structp, png_infop, int, int);
 
     SkAutoTUnref<SkPngChunkReader>     fPngChunkReader;
     png_structp                        fPng_ptr;

diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index a5df7e3b50a9e00291966f1a578b637bef7f7cce..a6b44eb9a7506187e5e248f74e2482b8351662da 100644 (file)
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -11,6 +11,7 @@
 #include "SkCodec.h"
 #include "SkCodecImageGenerator.h"
 #include "SkData.h"
+#include "SkImageEncoder.h"
 #include "SkFrontBufferedStream.h"
 #include "SkMD5.h"
 #include "SkRandom.h"
@@ -1042,3 +1043,34 @@ DEF_TEST(Codec_jpeg_rewind, r) {
     SkCodec::Result result = codec->getPixels(codec->getInfo(), pixelStorage.get(), rowBytes);
     REPORTER_ASSERT(r, SkCodec::kSuccess == result);
 }
+
+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);
+
+    // 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());
+
+    SkBitmap bm2;
+    bm2.allocPixels(codec->getInfo());
+    result = codec->getPixels(codec->getInfo(), bm2.getPixels(), bm2.rowBytes());
+    REPORTER_ASSERT(r, SkCodec::kSuccess == result);
+
+    SkMD5::Digest d1, d2;
+    md5(bm1, &d1);
+    md5(bm2, &d2);
+    REPORTER_ASSERT(r, d1 == d2);
+}