Add filtering and zlib-level options to SkPngEncoder

Bug: skia:6409
Bug: 713862
Change-Id: If287e2bcad5af990fac11e9091305f45ec903dbf
Reviewed-on: https://skia-review.googlesource.com/15647
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>

3 files changed, 116 insertions, 6 deletions

diff --git a/src/images/SkPngEncoder.cpp b/src/images/SkPngEncoder.cpp
index 38c7d888c6..6c14f50930 100644
--- a/src/images/SkPngEncoder.cpp
+++ b/src/images/SkPngEncoder.cpp
@@ -17,6 +17,13 @@
 
 #include "png.h"
 
+static_assert(PNG_FILTER_NONE  == (int)SkPngEncoder::FilterFlag::kNone,  "Skia libpng filter err.");
+static_assert(PNG_FILTER_SUB   == (int)SkPngEncoder::FilterFlag::kSub,   "Skia libpng filter err.");
+static_assert(PNG_FILTER_UP    == (int)SkPngEncoder::FilterFlag::kUp,    "Skia libpng filter err.");
+static_assert(PNG_FILTER_AVG   == (int)SkPngEncoder::FilterFlag::kAvg,   "Skia libpng filter err.");
+static_assert(PNG_FILTER_PAETH == (int)SkPngEncoder::FilterFlag::kPaeth, "Skia libpng filter err.");
+static_assert(PNG_ALL_FILTERS  == (int)SkPngEncoder::FilterFlag::kAll,   "Skia libpng filter err.");
+
 static constexpr bool kSuppressPngEncodeWarnings = true;
 
 static void sk_error_fn(png_structp png_ptr, png_const_charp msg) {
@@ -43,7 +50,7 @@ public:
      */
     static std::unique_ptr<SkPngEncoderMgr> Make(SkWStream* stream);
 
-    bool setHeader(const SkImageInfo& srcInfo);
+    bool setHeader(const SkImageInfo& srcInfo, const SkPngEncoder::Options& options);
     bool setPalette(const SkImageInfo& srcInfo, SkColorTable* colorTable,
                     SkTransferFunctionBehavior);
     bool setColorSpace(SkColorSpace* colorSpace);
@@ -89,7 +96,7 @@ std::unique_ptr<SkPngEncoderMgr> SkPngEncoderMgr::Make(SkWStream* stream) {
     return std::unique_ptr<SkPngEncoderMgr>(new SkPngEncoderMgr(pngPtr, infoPtr));
 }
 
-bool SkPngEncoderMgr::setHeader(const SkImageInfo& srcInfo) {
+bool SkPngEncoderMgr::setHeader(const SkImageInfo& srcInfo, const SkPngEncoder::Options& options) {
     if (setjmp(png_jmpbuf(fPngPtr))) {
         return false;
     }
@@ -161,6 +168,13 @@ bool SkPngEncoderMgr::setHeader(const SkImageInfo& srcInfo) {
                  PNG_FILTER_TYPE_BASE);
     png_set_sBIT(fPngPtr, fInfoPtr, &sigBit);
 
+    int filters = (int)options.fFilterFlags & (int)SkPngEncoder::FilterFlag::kAll;
+    SkASSERT(filters == (int)options.fFilterFlags);
+    png_set_filter(fPngPtr, PNG_FILTER_TYPE_BASE, filters);
+
+    int zlibLevel = SkTMin(SkTMax(0, options.fZLibLevel), 9);
+    SkASSERT(zlibLevel == options.fZLibLevel);
+    png_set_compression_level(fPngPtr, zlibLevel);
     return true;
 }
 
@@ -379,7 +393,7 @@ std::unique_ptr<SkPngEncoder> SkPngEncoder::Make(SkWStream* dst, const SkPixmap&
         return nullptr;
     }
 
-    if (!encoderMgr->setHeader(src.info())) {
+    if (!encoderMgr->setHeader(src.info(), options)) {
         return nullptr;
     }
 
diff --git a/src/images/SkPngEncoder.h b/src/images/SkPngEncoder.h
index 813790e74b..0664c06149 100644
--- a/src/images/SkPngEncoder.h
+++ b/src/images/SkPngEncoder.h
@@ -16,11 +16,43 @@ class SkWStream;
 class SkPngEncoder : public SkEncoder {
 public:
 
-    // TODO (skbug.com/6409):
-    // Add options for png filters and zlib compression.
+    enum class FilterFlag : int {
+        kZero  = 0x00,
+        kNone  = 0x08,
+        kSub   = 0x10,
+        kUp    = 0x20,
+        kAvg   = 0x40,
+        kPaeth = 0x80,
+        kAll   = kNone | kSub | kUp | kAvg | kPaeth,
+    };
 
     struct Options {
         /**
+         *  Selects which filtering strategies to use.
+         *
+         *  If a single filter is chosen, libpng will use that filter for every row.
+         *
+         *  If multiple filters are chosen, libpng will use a heuristic to guess which filter
+         *  will encode smallest, then apply that filter.  This happens on a per row basis,
+         *  different rows can use different filters.
+         *
+         *  Using a single filter (or less filters) is typically faster.  Trying all of the
+         *  filters may help minimize the output file size.
+         *
+         *  Our default value matches libpng's default.
+         */
+        FilterFlag fFilterFlags = FilterFlag::kAll;
+
+        /**
+         *  Must be in [0, 9] where 9 corresponds to maximal compression.  This value is passed
+         *  directly to zlib.  0 is a special case to skip zlib entirely, creating dramatically
+         *  larger pngs.
+         *
+         *  Our default value matches libpng's default.
+         */
+        int fZLibLevel = 6;
+
+        /**
          *  If the input is premultiplied, this controls the unpremultiplication behavior.
          *  The encoder can convert to linear before unpremultiplying or ignore the transfer
          *  function and unpremultiply the input as is.
@@ -58,4 +90,9 @@ protected:
     typedef SkEncoder INHERITED;
 };
 
+static inline SkPngEncoder::FilterFlag operator|(SkPngEncoder::FilterFlag x,
+                                                 SkPngEncoder::FilterFlag y) {
+    return (SkPngEncoder::FilterFlag)((int)x | (int)y);
+}
+
 #endif
diff --git a/tests/EncodeTest.cpp b/tests/EncodeTest.cpp
index 7be0f6cdbf..45a0ff83aa 100644
--- a/tests/EncodeTest.cpp
+++ b/tests/EncodeTest.cpp
@@ -10,6 +10,7 @@
 
 #include "SkBitmap.h"
 #include "SkEncodedImageFormat.h"
+#include "SkImage.h"
 #include "SkJpegEncoder.h"
 #include "SkPngEncoder.h"
 #include "SkStream.h"
@@ -80,7 +81,65 @@ static void test_encode(skiatest::Reporter* r, SkEncodedImageFormat format) {
     REPORTER_ASSERT(r, data0->equals(data3.get()));
 }
 
-DEF_TEST(Encoder, r) {
+DEF_TEST(Encode, r) {
     test_encode(r, SkEncodedImageFormat::kJPEG);
     test_encode(r, SkEncodedImageFormat::kPNG);
 }
+
+static inline bool equals(const SkBitmap& a, const SkBitmap& b) {
+    if (a.info() != b.info()) {
+        return false;
+    }
+
+    SkASSERT(kN32_SkColorType == a.colorType());
+    for (int y = 0; y < a.height(); y++) {
+        for (int x = 0; x < a.width(); x++) {
+            if (*a.getAddr32(x, y) != *b.getAddr32(x, y)) {
+                return false;
+            }
+        }
+    }
+
+    return true;
+}
+
+DEF_TEST(Encode_PngOptions, r) {
+    SkBitmap bitmap;
+    bool success = GetResourceAsBitmap("mandrill_128.png", &bitmap);
+    if (!success) {
+        return;
+    }
+
+    SkPixmap src;
+    success = bitmap.peekPixels(&src);
+    REPORTER_ASSERT(r, success);
+    if (!success) {
+        return;
+    }
+
+    SkDynamicMemoryWStream dst0, dst1, dst2;
+    SkPngEncoder::Options options;
+    success = SkPngEncoder::Encode(&dst0, src, options);
+    REPORTER_ASSERT(r, success);
+
+    options.fFilterFlags = SkPngEncoder::FilterFlag::kUp;
+    success = SkPngEncoder::Encode(&dst1, src, options);
+    REPORTER_ASSERT(r, success);
+
+    options.fZLibLevel = 3;
+    success = SkPngEncoder::Encode(&dst2, src, options);
+    REPORTER_ASSERT(r, success);
+
+    sk_sp<SkData> data0 = dst0.detachAsData();
+    sk_sp<SkData> data1 = dst1.detachAsData();
+    sk_sp<SkData> data2 = dst2.detachAsData();
+    REPORTER_ASSERT(r, data0->size() < data1->size());
+    REPORTER_ASSERT(r, data1->size() < data2->size());
+
+    SkBitmap bm0, bm1, bm2;
+    SkImage::MakeFromEncoded(data0)->asLegacyBitmap(&bm0, SkImage::kRO_LegacyBitmapMode);
+    SkImage::MakeFromEncoded(data1)->asLegacyBitmap(&bm1, SkImage::kRO_LegacyBitmapMode);
+    SkImage::MakeFromEncoded(data2)->asLegacyBitmap(&bm2, SkImage::kRO_LegacyBitmapMode);
+    REPORTER_ASSERT(r, equals(bm0, bm1));
+    REPORTER_ASSERT(r, equals(bm0, bm2));
+}