Use SkPngEncoder in gfx jpeg_codec

ui/gfx/codec/png_codec.cc

Index: ui/gfx/codec/png_codec.cc
diff --git a/ui/gfx/codec/png_codec.cc b/ui/gfx/codec/png_codec.cc
index fe4020596fc9c3ba32609719060a727554b07e81..48ccb901077f72f5522e769b73806b615e19c23a 100644
--- a/ui/gfx/codec/png_codec.cc
+++ b/ui/gfx/codec/png_codec.cc
@@ -13,60 +13,14 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColorPriv.h"
 #include "third_party/skia/include/core/SkUnPreMultiply.h"
+#include "third_party/skia/include/encode/SkPngEncoder.h"
 #include "third_party/zlib/zlib.h"
+#include "ui/gfx/codec/vector_wstream.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/skia_util.h"
 
 namespace gfx {
 
-namespace {
-
-// Converts BGRA->RGBA and RGBA->BGRA.
-void ConvertBetweenBGRAandRGBA(const unsigned char* input, int pixel_width,
-                               unsigned char* output, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &input[x * 4];
-    unsigned char* pixel_out = &output[x * 4];
-    pixel_out[0] = pixel_in[2];
-    pixel_out[1] = pixel_in[1];
-    pixel_out[2] = pixel_in[0];
-    pixel_out[3] = pixel_in[3];
-  }
-}
-
-void ConvertRGBAtoRGB(const unsigned char* rgba, int pixel_width,
-                      unsigned char* rgb, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++)
-    memcpy(&rgb[x * 3], &rgba[x * 4], 3);
-}
-
-void ConvertSkiaToRGB(const unsigned char* skia, int pixel_width,
-                      unsigned char* rgb, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const uint32_t pixel_in = *reinterpret_cast<const uint32_t*>(&skia[x * 4]);
-    unsigned char* pixel_out = &rgb[x * 3];
-
-    int alpha = SkGetPackedA32(pixel_in);
-    if (alpha != 0 && alpha != 255) {
-      SkColor unmultiplied = SkUnPreMultiply::PMColorToColor(pixel_in);
-      pixel_out[0] = SkColorGetR(unmultiplied);
-      pixel_out[1] = SkColorGetG(unmultiplied);
-      pixel_out[2] = SkColorGetB(unmultiplied);
-    } else {
-      pixel_out[0] = SkGetPackedR32(pixel_in);
-      pixel_out[1] = SkGetPackedG32(pixel_in);
-      pixel_out[2] = SkGetPackedB32(pixel_in);
-    }
-  }
-}
-
-void ConvertSkiaToRGBA(const unsigned char* skia, int pixel_width,
-                       unsigned char* rgba, bool* is_opaque) {
-  gfx::ConvertSkiaToRGBA(skia, pixel_width, rgba);
-}
-
-}  // namespace
-
 // Decoder --------------------------------------------------------------------
 //
 // This code is based on WebKit libpng interface (PNGImageDecoder), which is
@@ -369,15 +323,6 @@ void LogLibPNGDecodeWarning(png_structp png_ptr, png_const_charp warning_msg) {
   DLOG(ERROR) << "libpng decode warning: " << warning_msg;
 }
 
-void LogLibPNGEncodeError(png_structp png_ptr, png_const_charp error_msg) {
-  DLOG(ERROR) << "libpng encode error: " << error_msg;
-  longjmp(png_jmpbuf(png_ptr), 1);
-}
-
-void LogLibPNGEncodeWarning(png_structp png_ptr, png_const_charp warning_msg) {
-  DLOG(ERROR) << "libpng encode warning: " << warning_msg;
-}
-
 }  // namespace
 
 // static
@@ -454,287 +399,63 @@ bool PNGCodec::Decode(const unsigned char* input, size_t input_size,
 }
 
 // Encoder --------------------------------------------------------------------
-//
-// This section of the code is based on nsPNGEncoder.cpp in Mozilla
-// (Copyright 2005 Google Inc.)
 
 namespace {
 
-// Passed around as the io_ptr in the png structs so our callbacks know where
-// to write data.
-struct PngEncoderState {
-  explicit PngEncoderState(std::vector<unsigned char>* o) : out(o) {}
-  std::vector<unsigned char>* out;
-};
-
-// Called by libpng to flush its internal buffer to ours.
-void EncoderWriteCallback(png_structp png, png_bytep data, png_size_t size) {
-  PngEncoderState* state = static_cast<PngEncoderState*>(png_get_io_ptr(png));
-  DCHECK(state->out);
-
-  size_t old_size = state->out->size();
-  state->out->resize(old_size + size);
-  memcpy(&(*state->out)[old_size], data, size);
-}
-
-void FakeFlushCallback(png_structp png) {
-  // We don't need to perform any flushing since we aren't doing real IO, but
-  // we're required to provide this function by libpng.
-}
-
-void ConvertBGRAtoRGB(const unsigned char* bgra, int pixel_width,
-                      unsigned char* rgb, bool* is_opaque) {
-  for (int x = 0; x < pixel_width; x++) {
-    const unsigned char* pixel_in = &bgra[x * 4];
-    unsigned char* pixel_out = &rgb[x * 3];
-    pixel_out[0] = pixel_in[2];
-    pixel_out[1] = pixel_in[1];
-    pixel_out[2] = pixel_in[0];
+static void AddComments(SkPngEncoder::Options& options,
+                        const std::vector<PNGCodec::Comment>& comments) {
+  std::vector<const char*> comment_pointers;
+  std::vector<size_t> comment_sizes;
+  for (const auto& comment : comments) {
+    comment_pointers.push_back(comment.key.c_str());
+    comment_pointers.push_back(comment.text.c_str());
+    comment_sizes.push_back(comment.key.length() + 1);
+    comment_sizes.push_back(comment.text.length() + 1);
   }
+  options.fComments = SkDataTable::MakeCopyArrays(
+      (void const* const*)comment_pointers.data(), comment_sizes.data(),
+      comment_pointers.size());
 }
 
-#ifdef PNG_TEXT_SUPPORTED
-class CommentWriter {
- public:
-  explicit CommentWriter(const std::vector<PNGCodec::Comment>& comments)
-      : comments_(comments),
-        png_text_(new png_text[comments.size()]) {
-    for (size_t i = 0; i < comments.size(); ++i)
-      AddComment(i, comments[i]);
-  }
-
-  ~CommentWriter() {
-    for (size_t i = 0; i < comments_.size(); ++i) {
-      free(png_text_[i].key);
-      free(png_text_[i].text);
-    }
-    delete [] png_text_;
-  }
-
-  bool HasComments() {
-    return !comments_.empty();
-  }
-
-  png_text* get_png_text() {
-    return png_text_;
-  }
-
-  int size() {
-    return static_cast<int>(comments_.size());
-  }
-
- private:
-  void AddComment(size_t pos, const PNGCodec::Comment& comment) {
-    png_text_[pos].compression = PNG_TEXT_COMPRESSION_NONE;
-    // A PNG comment's key can only be 79 characters long.
-    DCHECK(comment.key.length() < 79);
-    png_text_[pos].key = base::strdup(comment.key.substr(0, 78).c_str());
-    png_text_[pos].text = base::strdup(comment.text.c_str());
-    png_text_[pos].text_length = comment.text.length();
-#ifdef PNG_iTXt_SUPPORTED
-    png_text_[pos].itxt_length = 0;
-    png_text_[pos].lang = 0;
-    png_text_[pos].lang_key = 0;
-#endif
-  }
-
-  DISALLOW_COPY_AND_ASSIGN(CommentWriter);
-
-  const std::vector<PNGCodec::Comment> comments_;
-  png_text* png_text_;
-};
-#endif  // PNG_TEXT_SUPPORTED
-
-// The type of functions usable for converting between pixel formats.
-typedef void (*FormatConverter)(const unsigned char* in, int w,
-                                unsigned char* out, bool* is_opaque);
-
-// libpng uses a wacky setjmp-based API, which makes the compiler nervous.
-// We constrain all of the calls we make to libpng where the setjmp() is in
-// place to this function.
-// Returns true on success.
-bool DoLibpngWrite(png_struct* png_ptr, png_info* info_ptr,
-                   PngEncoderState* state,
-                   int width, int height, int row_byte_width,
-                   const unsigned char* input, int compression_level,
-                   int png_output_color_type, int output_color_components,
-                   FormatConverter converter,
-                   const std::vector<PNGCodec::Comment>& comments) {
-#ifdef PNG_TEXT_SUPPORTED
-  CommentWriter comment_writer(comments);
-#endif
-  unsigned char* row_buffer = NULL;
-
-  // Make sure to not declare any locals here -- locals in the presence
-  // of setjmp() in C++ code makes gcc complain.
-
-  if (setjmp(png_jmpbuf(png_ptr))) {
-    delete[] row_buffer;
-    return false;
-  }
-
-  png_set_compression_level(png_ptr, compression_level);
-
-  // Set our callback for libpng to give us the data.
-  png_set_write_fn(png_ptr, state, EncoderWriteCallback, FakeFlushCallback);
-  png_set_error_fn(png_ptr, NULL, LogLibPNGEncodeError, LogLibPNGEncodeWarning);
-
-  png_set_IHDR(png_ptr, info_ptr, width, height, 8, png_output_color_type,
-               PNG_INTERLACE_NONE, PNG_COMPRESSION_TYPE_DEFAULT,
-               PNG_FILTER_TYPE_DEFAULT);
-
-#ifdef PNG_TEXT_SUPPORTED
-  if (comment_writer.HasComments()) {
-    png_set_text(png_ptr, info_ptr, comment_writer.get_png_text(),
-                 comment_writer.size());
-  }
-#endif
-
-  png_write_info(png_ptr, info_ptr);
+}  // namespace
 
-  if (!converter) {
-    // No conversion needed, give the data directly to libpng.
-    for (int y = 0; y < height; y ++) {
-      png_write_row(png_ptr,
-                    const_cast<unsigned char*>(&input[y * row_byte_width]));
-    }
-  } else {
-    // Needs conversion using a separate buffer.
-    row_buffer = new unsigned char[width * output_color_components];
-    for (int y = 0; y < height; y ++) {
-      converter(&input[y * row_byte_width], width, row_buffer, NULL);
-      png_write_row(png_ptr, row_buffer);
-    }
-    delete[] row_buffer;
-  }
+static bool EncodeSkPixmap(const SkPixmap& src,
+                           const std::vector<PNGCodec::Comment>& comments,
+                           std::vector<unsigned char>* output,
+                           int zlib_level) {
+  output->clear();
+  VectorWStream dst(output);
 
-  png_write_end(png_ptr, info_ptr);
-  return true;
+  SkPngEncoder::Options options;
+  AddComments(options, comments);
+  options.fZLibLevel = zlib_level;
+  return SkPngEncoder::Encode(&dst, src, options);
 }
 
-bool EncodeWithCompressionLevel(const unsigned char* input,
-                                PNGCodec::ColorFormat format,
-                                const Size& size,
-                                int row_byte_width,
-                                bool discard_transparency,
-                                const std::vector<PNGCodec::Comment>& comments,
-                                int compression_level,
-                                std::vector<unsigned char>* output) {
-  // Run to convert an input row into the output row format, NULL means no
-  // conversion is necessary.
-  FormatConverter converter = NULL;
-
-  int input_color_components, output_color_components;
-  int png_output_color_type;
-  switch (format) {
-    case PNGCodec::FORMAT_RGBA:
-      input_color_components = 4;
-      if (discard_transparency) {
-        output_color_components = 3;
-        png_output_color_type = PNG_COLOR_TYPE_RGB;
-        converter = ConvertRGBAtoRGB;
-      } else {
-        output_color_components = 4;
-        png_output_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
-        converter = NULL;
-      }
-      break;
-
-    case PNGCodec::FORMAT_BGRA:
-      input_color_components = 4;
-      if (discard_transparency) {
-        output_color_components = 3;
-        png_output_color_type = PNG_COLOR_TYPE_RGB;
-        converter = ConvertBGRAtoRGB;
-      } else {
-        output_color_components = 4;
-        png_output_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
-        converter = ConvertBetweenBGRAandRGBA;
-      }
-      break;
-
-    case PNGCodec::FORMAT_SkBitmap:
-      // Compare row_byte_width and size.width() to detect the format of
-      // SkBitmap. kA8_Config (1bpp) and kARGB_8888_Config (4bpp) are the two
-      // supported formats.
-      if (row_byte_width < 4 * size.width()) {
-        // Not 4bpp, so must be 1bpp.
-        // Ignore discard_transparency - it doesn't make sense in this context,
-        // since alpha is the only thing we have and it needs to be used for
-        // color intensity.
-        input_color_components = 1;
-        output_color_components = 1;
-        png_output_color_type = PNG_COLOR_TYPE_GRAY;
-        // |converter| is left as null
-      } else {
-        input_color_components = 4;
-        if (discard_transparency) {
-          output_color_components = 3;
-          png_output_color_type = PNG_COLOR_TYPE_RGB;
-          converter = ConvertSkiaToRGB;
-        } else {
-          output_color_components = 4;
-          png_output_color_type = PNG_COLOR_TYPE_RGB_ALPHA;
-          converter = ConvertSkiaToRGBA;
-        }
-      }
-      break;
-
-    default:
-      NOTREACHED() << "Unknown pixel format";
+static bool EncodeSkPixmap(const SkPixmap& src,
+                           bool discard_transparency,
+                           const std::vector<PNGCodec::Comment>& comments,
+                           std::vector<unsigned char>* output,
+                           int zlib_level) {
+  if (discard_transparency) {
+    SkImageInfo opaque_info = src.info().makeAlphaType(kOpaque_SkAlphaType);
+    SkBitmap copy;
+    if (!copy.tryAllocPixels(opaque_info)) {
       return false;
+    }
+    SkPixmap opaque_pixmap;
+    SkASSERT(copy.peekPixels(&opaque_pixmap));

[scroggo_chromium, 2017/07/11 16:01:06]

This only happens in debug. Maybe you want:

bool success = copy.peekPixels(&opaque_pixmap);
DCHECK(success);

[liyuqian, 2017/07/11 19:24:55]

Done.

+    // The following step does the unpremul as we set the dst alpha type to be
+    // kUnpremul_SkAlphaType. Later, because opaque_pixmap has
+    // kOpaque_SkAlphaType, we'll discard the transparency as required.
+    SkASSERT(src.readPixels(opaque_info.makeAlphaType(kUnpremul_SkAlphaType),

[scroggo_chromium, 2017/07/11 16:01:06]

Again, this only happens in debug, and we should be using DCHECK anyway.

[liyuqian, 2017/07/11 19:24:55]

Done.

+                            opaque_pixmap.writable_addr(),
+                            opaque_pixmap.rowBytes()));
+    return EncodeSkPixmap(opaque_pixmap, comments, output, zlib_level);
   }
-
-  // Row stride should be at least as long as the length of the data.
-  DCHECK(input_color_components * size.width() <= row_byte_width);
-
-  PngWriteStructInfo si;
-  si.png_ptr_ = png_create_write_struct(PNG_LIBPNG_VER_STRING,
-                                        NULL, NULL, NULL);
-  if (!si.png_ptr_)
-    return false;
-
-  si.info_ptr_ = png_create_info_struct(si.png_ptr_);
-  if (!si.info_ptr_)
-    return false;
-
-  output->clear();
-
-  PngEncoderState state(output);
-  bool success = DoLibpngWrite(si.png_ptr_, si.info_ptr_, &state,
-                               size.width(), size.height(), row_byte_width,
-                               input, compression_level, png_output_color_type,
-                               output_color_components, converter, comments);
-
-  return success;
+  return EncodeSkPixmap(src, comments, output, zlib_level);
 }
 
-bool InternalEncodeSkBitmap(const SkBitmap& input,
-                            bool discard_transparency,
-                            int compression_level,
-                            std::vector<unsigned char>* output) {
-  if (input.empty() || input.isNull())
-    return false;
-  int bpp = input.bytesPerPixel();
-  DCHECK(bpp == 1 || bpp == 4);  // We support kA8_Config and kARGB_8888_Config.
-
-  unsigned char* inputAddr = bpp == 1 ?
-      reinterpret_cast<unsigned char*>(input.getAddr8(0, 0)) :
-      reinterpret_cast<unsigned char*>(input.getAddr32(0, 0));    // bpp = 4
-  return EncodeWithCompressionLevel(
-      inputAddr,
-      PNGCodec::FORMAT_SkBitmap,
-      Size(input.width(), input.height()),
-      static_cast<int>(input.rowBytes()),
-      discard_transparency,
-      std::vector<PNGCodec::Comment>(),
-      compression_level,
-      output);
-}
-
-
-}  // namespace
-
 // static
 bool PNGCodec::Encode(const unsigned char* input,
                       ColorFormat format,
@@ -743,43 +464,55 @@ bool PNGCodec::Encode(const unsigned char* input,
                       bool discard_transparency,
                       const std::vector<Comment>& comments,
                       std::vector<unsigned char>* output) {
-  return EncodeWithCompressionLevel(input,
-                                    format,
-                                    size,
-                                    row_byte_width,
-                                    discard_transparency,
-                                    comments,
-                                    Z_DEFAULT_COMPRESSION,
-                                    output);
+  auto colorType =
+      format == FORMAT_RGBA ? kRGBA_8888_SkColorType : kBGRA_8888_SkColorType;

[scroggo_chromium, 2017/07/11 16:01:06]

Just to make sure I understand - if format is FORMAT_SkBitmap, we also want
kBGRA_8888? The header says it's treated as kARGB_8888_Config (probably needs
updating).

[liyuqian, 2017/07/11 19:24:55]

I'm also very confused because:

1. The method is called EncodeBGRASkBitmap but the comment says
kARGB_8888_Config.

2. kARGB_8888_Config seems to no longer exist in Skia except in some
experimental code

Therefore, I'm going to change kARGB_8888_Config to kBGRA_8888_SkColorType in
all comments in this file.

[scroggo_chromium, 2017/07/11 19:33:09]

Yeah, Configs were replaced with SkColorTypes before you got here. When we had
kARGB_8888, I think it corresponded to whichever one was the native one.
Sgtm, so long as we don't accidentally flip the behavior!

[liyuqian, 2017/07/11 19:59:25]

Acknowledged.

+  auto alphaType =
+      format == FORMAT_SkBitmap ? kPremul_SkAlphaType : kUnpremul_SkAlphaType;
+  SkImageInfo info =
+      SkImageInfo::Make(size.width(), size.height(), colorType, alphaType);
+  SkPixmap src(info, input, row_byte_width);
+  return EncodeSkPixmap(src, discard_transparency, comments, output,
+                        DEFAULT_ZLIB_COMPRESSION);
+}
+
+static bool EncodeSkBitmap(const SkBitmap& input,
+                           bool discard_transparency,
+                           std::vector<unsigned char>* output,
+                           int zlib_level) {
+  SkPixmap src;
+  if (!input.peekPixels(&src)) {
+    return false;
+  }
+  std::vector<PNGCodec::Comment> empty_comments;

[scroggo_chromium, 2017/07/11 16:01:06]

Too bad we need to create this empty vector just to ignore it.

[liyuqian, 2017/07/11 19:24:55]

Agree. I can use {} in Mac but Linux can only compile with this ugly
empty_comments...

[scroggo_chromium, 2017/07/11 19:33:09]

Maybe it should take a pointer, which can be null?

[liyuqian, 2017/07/11 19:59:25]

That sounds good. However, it will require the public API to change and many
places that use this API will need to be changed. I'd prefer to postpone those
changes to a later CL so this CL doesn't get too big. Anyway, I just realized
that I can use `std::vector<PNGCodec::Comment>()` instead of empty_comments.
(It's strange that `{}` won't work but `std::vector<PNGCodec::Comment>()` will.)

[scroggo_chromium, 2017/07/11 20:09:30]

Sgtm

+  return EncodeSkPixmap(src, discard_transparency, empty_comments, output,
+                        zlib_level);
 }
 
 // static
 bool PNGCodec::EncodeBGRASkBitmap(const SkBitmap& input,
                                   bool discard_transparency,
                                   std::vector<unsigned char>* output) {
-  return InternalEncodeSkBitmap(input,
-                                discard_transparency,
-                                Z_DEFAULT_COMPRESSION,
-                                output);
+  return EncodeSkBitmap(input, discard_transparency, output,
+                        DEFAULT_ZLIB_COMPRESSION);
 }
 
 // static
 bool PNGCodec::EncodeA8SkBitmap(const SkBitmap& input,
                                 std::vector<unsigned char>* output) {
-  return InternalEncodeSkBitmap(input,
-                                false,
-                                Z_DEFAULT_COMPRESSION,
-                                output);
+  SkASSERT(input.colorType() == kAlpha_8_SkColorType);

[scroggo_chromium, 2017/07/11 16:01:06]

DCHECK. There might be a DCHECK_EQ, so this could be

  DCHECK_EQ(input.colorType(), kAlpha_8_SkColorType);

[liyuqian, 2017/07/11 19:24:55]

Done.

+  auto info = input.info()
+                  .makeColorType(kGray_8_SkColorType)
+                  .makeAlphaType(kOpaque_SkAlphaType);
+  SkPixmap src(info, input.getAddr(0, 0), input.rowBytes());

[scroggo_chromium, 2017/07/11 16:01:06]

You could instead use readPixels here, as you do elsewhere. Not sure why to
choose one or the other.

[liyuqian, 2017/07/11 19:24:55]

In the other place, we need to do unpremul and copy the pixels into a new
location so readPixels is used; here, we don't have to modify the memory and
we're not copying pixels, so readPixels is not used. 

[scroggo_chromium, 2017/07/11 19:33:09]

Oh, oops, you are correct.

+  std::vector<Comment> empty_comments;
+  return EncodeSkPixmap(src, empty_comments, output, DEFAULT_ZLIB_COMPRESSION);
 }
 
 // static
 bool PNGCodec::FastEncodeBGRASkBitmap(const SkBitmap& input,
                                       bool discard_transparency,
                                       std::vector<unsigned char>* output) {
-  return InternalEncodeSkBitmap(input,
-                                discard_transparency,
-                                Z_BEST_SPEED,
-                                output);
+  return EncodeSkBitmap(input, discard_transparency, output, Z_BEST_SPEED);
 }
 
 PNGCodec::Comment::Comment(const std::string& k, const std::string& t)