Support for interlaced PNGs (in gfx::PNGCodec)

ui/gfx/codec/png_codec_unittest.cc

Index: ui/gfx/codec/png_codec_unittest.cc
===================================================================
--- ui/gfx/codec/png_codec_unittest.cc	(revision 123937)
+++ ui/gfx/codec/png_codec_unittest.cc	(working copy)
@@ -2,11 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#if defined(USE_SYSTEM_LIBPNG)
+#include <png.h>
+#else
+#include "third_party/libpng/png.h"
+#endif

[Francois, 2012/02/29 09:12:30]

Required in order to encode interlaced, palette-based, and grayscale images (not
supported by PNGCodec::Encode). It was necessary to add tests for these types
because PNGCodec is actually used to decode such images.

+
 #include <algorithm>
 #include <cmath>
 
+#include "base/logging.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #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/zlib/zlib.h"
 #include "ui/gfx/codec/png_codec.h"
@@ -14,7 +22,9 @@
 
 namespace gfx {
 
-static void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
+namespace {
+
+void MakeRGBImage(int w, int h, std::vector<unsigned char>* dat) {
   dat->resize(w * h * 3);
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -30,8 +40,8 @@
 // be filled with 0xff. With the alpha channel stripped, this should yield the
 // same image as MakeRGBImage above, so the code below can make reference
 // images for conversion testing.
-static void MakeRGBAImage(int w, int h, bool use_transparency,
-                          std::vector<unsigned char>* dat) {
+void MakeRGBAImage(int w, int h, bool use_transparency,
+                   std::vector<unsigned char>* dat) {
   dat->resize(w * h * 4);
   for (int y = 0; y < h; y++) {
     for (int x = 0; x < w; x++) {
@@ -47,6 +57,138 @@
   }
 }
 
+// User write function (to be passed to libpng by EncodeImage) which writes
+// into a buffer instead of to a file.
+void WriteImageData(png_structp png_ptr,
+                    png_bytep data,
+                    png_size_t length) {
+  std::vector<unsigned char>& v =
+      *static_cast<std::vector<unsigned char>*>(png_get_io_ptr(png_ptr));
+  v.resize(v.size() + length);
+  memcpy(&v[v.size() - length], data, length);
+}
+
+// User flush function; goes with WriteImageData, above.
+void FlushImageData(png_structp /*png_ptr*/) {}

[tony, 2012/02/29 18:57:57]

Nit: closing } goes on its own line.

[Francois, 2012/03/01 07:53:15]

Done.

+
+// Libpng user error function which allows us to print libpng errors using
+// Chrome's logging facilities instead of stderr.
+void LogLibPNGError(png_structp png_ptr,
+                    png_const_charp error_msg) {
+  DLOG(ERROR) << "libpng encode error: " << error_msg;
+  longjmp(png_jmpbuf(png_ptr), 1);
+}
+
+// Goes with LogLibPNGError, above.
+void LogLibPNGWarning(png_structp png_ptr,
+                      png_const_charp warning_msg) {
+  DLOG(ERROR) << "libpng encode warning: " << warning_msg;
+}
+
+// Color types supported by EncodeImage. Required because neither libpng nor
+// PNGCodec::Encode supports all of the required values.
+enum ColorType {
+  COLOR_TYPE_GRAY = PNG_COLOR_TYPE_GRAY,
+  COLOR_TYPE_GRAY_ALPHA = PNG_COLOR_TYPE_GRAY_ALPHA,
+  COLOR_TYPE_PALETTE = PNG_COLOR_TYPE_PALETTE,
+  COLOR_TYPE_RGB = PNG_COLOR_TYPE_RGB,
+  COLOR_TYPE_RGBA = PNG_COLOR_TYPE_RGBA,
+  COLOR_TYPE_BGR,
+  COLOR_TYPE_BGRA
+};
+
+// PNG encoder used for testing. Required because PNGCodec::Encode doesn't do
+// interlaced, palette-based, or grayscale images, but PNGCodec::Decode is
+// actually asked to decode these types of images by Chrome.
+bool EncodeImage(const std::vector<unsigned char>& input,
+                 const int width,
+                 const int height,
+                 ColorType output_color_type,
+                 std::vector<unsigned char>* output,
+                 const int interlace_type = PNG_INTERLACE_NONE,
+                 std::vector<png_color>* palette = 0,
+                 std::vector<unsigned char>* palette_alpha = 0) {
+  struct ScopedPNGStructs {
+    ScopedPNGStructs(png_struct** s, png_info** i) : s_(s), i_(i) {}
+    ~ScopedPNGStructs() { png_destroy_write_struct(s_, i_); }
+    png_struct** s_;
+    png_info** i_;
+  };
+
+  DCHECK(output);
+  png_struct* png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING,
+                                                NULL, NULL, NULL);
+  if (!png_ptr) return false;

[tony, 2012/02/29 18:57:57]

Nit: return false on its own line.

[Francois, 2012/03/01 07:53:15]

Done.

+
+  png_infop info_ptr = png_create_info_struct(png_ptr);
+  if (!info_ptr) {
+    png_destroy_write_struct(&png_ptr, NULL);
+    return false;
+  }
+
+  ScopedPNGStructs scoped_png_structs(&png_ptr, &info_ptr);
+
+  if (setjmp(png_jmpbuf(png_ptr)))
+    return false;
+
+  png_set_error_fn(png_ptr, NULL, LogLibPNGError, LogLibPNGWarning);
+
+  int input_rowbytes = 0;
+  int transforms = PNG_TRANSFORM_IDENTITY;
+
+  switch (output_color_type) {
+    case COLOR_TYPE_GRAY:
+      input_rowbytes = width;
+      break;
+    case COLOR_TYPE_GRAY_ALPHA:
+      input_rowbytes = width * 2;
+      break;
+    case COLOR_TYPE_PALETTE:
+      if (!palette) return false;
+      input_rowbytes = width;
+      break;
+    case COLOR_TYPE_RGB:
+      input_rowbytes = width * 3;
+      break;
+    case COLOR_TYPE_RGBA:
+      input_rowbytes = width * 4;
+      break;
+    case COLOR_TYPE_BGRA:
+      input_rowbytes = width * 4;
+      output_color_type = static_cast<ColorType>(PNG_COLOR_TYPE_RGBA);
+      transforms |= PNG_TRANSFORM_BGR;
+      break;
+    default:

[tony, 2012/02/29 18:57:57]

Nit: It's better to not have a default so you get a compiler error when a new
enum value is added.

[Francois, 2012/03/01 07:53:15]

Done.

+      return false;
+      break;
+  };
+
+  std::vector<png_bytep> row_pointers(height);
+  for (int y = 0 ; y < height; y++) {
+    row_pointers[y] = const_cast<unsigned char*>(&input[y * input_rowbytes]);
+  }
+  png_set_rows(png_ptr, info_ptr, &row_pointers[0]);
+  png_set_write_fn(png_ptr, output, WriteImageData, FlushImageData);
+  png_set_IHDR(png_ptr, info_ptr, width, height, 8, output_color_type,
+               interlace_type, PNG_COMPRESSION_TYPE_DEFAULT,
+               PNG_FILTER_TYPE_DEFAULT);
+  if (output_color_type == COLOR_TYPE_PALETTE) {
+    png_set_PLTE(png_ptr, info_ptr, &palette->front(), palette->size());
+    if (palette_alpha)
+      png_set_tRNS(png_ptr,

[tony, 2012/02/29 18:57:57]

Nit: I think this if requires {} because the body is more than one line.

[Francois, 2012/03/01 07:53:15]

Done.

+                   info_ptr,
+                   &palette_alpha->front(),
+                   palette_alpha->size(),
+                   NULL);
+  }
+
+  png_write_png(png_ptr, info_ptr, transforms, NULL);
+
+  return true;
+}
+
+}  // namespace
+
 // Returns true if each channel of the given two colors are "close." This is
 // used for comparing colors where rounding errors may cause off-by-one.
 bool ColorsClose(uint32_t a, uint32_t b) {
@@ -82,7 +224,7 @@
 
   // encode
   std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
                                Size(w, h), w * 3, false,
                                std::vector<PNGCodec::Comment>(),
                                &encoded));
@@ -90,7 +232,7 @@
   // decode, it should have the same size as the original
   std::vector<unsigned char> decoded;
   int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
                                PNGCodec::FORMAT_RGB, &decoded,
                                &outw, &outh));
   ASSERT_EQ(w, outw);
@@ -111,7 +253,7 @@
 
   // encode
   std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGBA,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGBA,
                                Size(w, h), w * 4, false,
                                std::vector<PNGCodec::Comment>(),
                                &encoded));
@@ -119,7 +261,7 @@
   // decode, it should have the same size as the original
   std::vector<unsigned char> decoded;
   int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
                                PNGCodec::FORMAT_RGBA, &decoded,
                                &outw, &outh));
   ASSERT_EQ(w, outw);
@@ -130,6 +272,227 @@
   ASSERT_TRUE(original == decoded);
 }
 
+TEST(PNGCodec, EncodeDecodeBGRA) {
+  const int w = 20, h = 20;
+
+  // Create an image with known values, alpha must be opaque because it will be
+  // lost during encoding.
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, true, &original);
+
+  // Encode.
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_BGRA,
+                               Size(w, h), w * 4, false,
+                               std::vector<PNGCodec::Comment>(),
+                               &encoded));
+
+  // Decode, it should have the same size as the original.
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_BGRA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be exactly equal.
+  ASSERT_TRUE(original == decoded);
+}

[Francois, 2012/02/29 09:12:30]

Old code moved from further down.

+
+TEST(PNGCodec, DecodeInterlacedRGB) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBImage(w, h, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGB,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGB, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBA) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGBA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBADiscardAlpha) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_RGB, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(decoded.size(), w * h * 3U);
+
+  // Images must be equal
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      unsigned char* orig_px = &original[(y * w + x) * 4];
+      unsigned char* dec_px = &decoded[(y * w + x) * 3];
+      ASSERT_EQ(dec_px[0], orig_px[0]);
+      ASSERT_EQ(dec_px[1], orig_px[1]);
+      ASSERT_EQ(dec_px[2], orig_px[2]);
+    }
+  }
+}
+
+TEST(PNGCodec, DecodeInterlacedBGRA) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_BGRA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // decode, it should have the same size as the original
+  std::vector<unsigned char> decoded;
+  int outw, outh;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
+                               PNGCodec::FORMAT_BGRA, &decoded,
+                               &outw, &outh));
+  ASSERT_EQ(w, outw);
+  ASSERT_EQ(h, outh);
+  ASSERT_EQ(original.size(), decoded.size());
+
+  // Images must be equal
+  ASSERT_EQ(original, decoded);
+}
+
+// Not encoding an interlaced PNG from SkBitmap because we don't do it
+// anywhere, and the ability to do that requires more code changes.
+TEST(PNGCodec, DecodeInterlacedRGBtoSkBitmap) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBImage(w, h, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGB,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // Decode the encoded string.
+  SkBitmap decoded_bitmap;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded.front(), encoded.size(),
+                               &decoded_bitmap));
+
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      const unsigned char* original_pixel = &original[(y * w + x) * 3];
+      const uint32_t original_pixel_sk = SkPackARGB32(0xFF,
+                                                      original_pixel[0],
+                                                      original_pixel[1],
+                                                      original_pixel[2]);
+      const uint32_t decoded_pixel = decoded_bitmap.getAddr32(0, y)[x];
+      ASSERT_EQ(original_pixel_sk, decoded_pixel);
+    }
+  }
+}
+
+TEST(PNGCodec, DecodeInterlacedRGBAtoSkBitmap) {
+  const int w = 20, h = 20;
+
+  // create an image with known values
+  std::vector<unsigned char> original;
+  MakeRGBAImage(w, h, false, &original);
+
+  // encode
+  std::vector<unsigned char> encoded;
+  ASSERT_TRUE(EncodeImage(original,
+                          w, h,
+                          COLOR_TYPE_RGBA,
+                          &encoded,
+                          PNG_INTERLACE_ADAM7));
+
+  // Decode the encoded string.
+  SkBitmap decoded_bitmap;
+  ASSERT_TRUE(PNGCodec::Decode(&encoded.front(), encoded.size(),
+                               &decoded_bitmap));
+
+  for (int x = 0; x < w; x++) {
+    for (int y = 0; y < h; y++) {
+      const unsigned char* original_pixel = &original[(y * w + x) * 4];
+      const uint32_t original_pixel_sk = SkPackARGB32(original_pixel[3],
+                                                      original_pixel[0],
+                                                      original_pixel[1],
+                                                      original_pixel[2]);
+      const uint32_t decoded_pixel = decoded_bitmap.getAddr32(0, y)[x];
+      ASSERT_EQ(original_pixel_sk, decoded_pixel);
+    }
+  }
+}
+
 // Test that corrupted data decompression causes failures.
 TEST(PNGCodec, DecodeCorrupted) {
   int w = 20, h = 20;
@@ -147,7 +510,7 @@
 
   // Make some compressed data.
   std::vector<unsigned char> compressed;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
+  ASSERT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_RGB,
                                Size(w, h), w * 3, false,
                                std::vector<PNGCodec::Comment>(),
                                &compressed));
@@ -165,35 +528,6 @@
                                 &outw, &outh));
 }
 
-TEST(PNGCodec, EncodeDecodeBGRA) {
-  const int w = 20, h = 20;
-
-  // Create an image with known values, alpha must be opaque because it will be
-  // lost during encoding.
-  std::vector<unsigned char> original;
-  MakeRGBAImage(w, h, true, &original);
-
-  // Encode.
-  std::vector<unsigned char> encoded;
-  EXPECT_TRUE(PNGCodec::Encode(&original[0], PNGCodec::FORMAT_BGRA,
-                               Size(w, h), w * 4, false,
-                               std::vector<PNGCodec::Comment>(),
-                               &encoded));
-
-  // Decode, it should have the same size as the original.
-  std::vector<unsigned char> decoded;
-  int outw, outh;
-  EXPECT_TRUE(PNGCodec::Decode(&encoded[0], encoded.size(),
-                               PNGCodec::FORMAT_BGRA, &decoded,
-                               &outw, &outh));
-  ASSERT_EQ(w, outw);
-  ASSERT_EQ(h, outh);
-  ASSERT_EQ(original.size(), decoded.size());
-
-  // Images must be exactly equal.
-  ASSERT_TRUE(original == decoded);

[Francois, 2012/02/29 09:12:30]

This function was moved further up.

-}
-
 TEST(PNGCodec, StripAddAlpha) {
   const int w = 20, h = 20;
 
@@ -221,7 +555,7 @@
   ASSERT_EQ(w, outw);
   ASSERT_EQ(h, outh);
   ASSERT_EQ(original_rgba.size(), decoded.size());
-  ASSERT_TRUE(original_rgba == decoded);
+  ASSERT_EQ(original_rgba, decoded);
 
   // Encode RGBA to RGBA.
   EXPECT_TRUE(PNGCodec::Encode(&original_rgba[0], PNGCodec::FORMAT_RGBA,
@@ -238,7 +572,7 @@
   ASSERT_EQ(w, outw);
   ASSERT_EQ(h, outh);
   ASSERT_EQ(original_rgb.size(), decoded.size());
-  ASSERT_TRUE(original_rgb == decoded);
+  ASSERT_EQ(original_rgb, decoded);
 }
 
 TEST(PNGCodec, EncodeBGRASkBitmap) {