Fix icon generation for PWA from png with transparent pixels.

SkBitmap used as a source for icon bitmaps generation may have color components premultiplied by alpha value. In that case utilizing SkBitmap pixels as is makes them appear darker in resulting icon where no premultiplication is expected.

With this CL premultiplied alpha pixels are converted to pixels where colors components are independent from alpha channel if required.

Bug: 1090720
Change-Id: Idfc34fe66fbd6d4ed1a44332c59b0f9bd6d9fc84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2345144Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799266}

ui/gfx/icon_util.cc

@@ -20,6 +20,7 @@
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_family.h"
+#include "ui/gfx/skbitmap_operations.h"
 
 namespace {
 
@@ -628,7 +629,11 @@ void IconUtil::SetSingleIconImageInformation(const SkBitmap& bitmap,
   // opaque.
   unsigned char* image_addr = reinterpret_cast<unsigned char*>(icon_image);
   unsigned char* xor_mask_addr = image_addr + sizeof(BITMAPINFOHEADER);
-  CopySkBitmapBitsIntoIconBuffer(bitmap, xor_mask_addr, xor_mask_size);
+
+  // Make sure pixels are not premultiplied by alpha.
+  SkBitmap unpremul_bitmap = SkBitmapOperations::UnPreMultiply(bitmap);
+  CopySkBitmapBitsIntoIconBuffer(unpremul_bitmap, xor_mask_addr, xor_mask_size);
+
   *image_byte_count = bytes_in_resource;
 }
 

ui/gfx/icon_util_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/path_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/icon_util_unittests_resource.h"
 #include "ui/gfx/image/image.h"
@@ -430,3 +431,36 @@ TEST_F(IconUtilTest, TestNumIconDimensionsUpToMediumSize) {
             IconUtil::kIconDimensions[
                 IconUtil::kNumIconDimensionsUpToMediumSize - 1]);
 }
+
+TEST_F(IconUtilTest, TestTransparentIcon) {
+  base::FilePath icon_filename =
+      temp_directory_.GetPath().AppendASCII(kTempIconFilename);
+  int size = 48;
+  auto semi_transparent_red = SkColorSetARGB(0x77, 0xFF, 0x00, 0x00);
+
+  // Create a bitmap with a semi transparent red dot.
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(size, size, false);
+  EXPECT_EQ(bitmap.alphaType(), kPremul_SkAlphaType);
+  {
+    SkCanvas canvas(bitmap);
+    canvas.drawColor(SK_ColorWHITE);
+    SkPaint paint;
+    paint.setColor(semi_transparent_red);
+    paint.setBlendMode(SkBlendMode::kSrc);
+    canvas.drawPoint(1, 1, paint);
+  }
+
+  // Create icon from that bitmap.
+  gfx::ImageFamily image_family;
+  image_family.Add(gfx::Image::CreateFrom1xBitmap(bitmap));
+  ASSERT_TRUE(
+      IconUtil::CreateIconFileFromImageFamily(image_family, icon_filename));
+
+  // Load icon and check that dot has same color.
+  ScopedHICON icon(LoadIconFromFile(icon_filename, size, size));
+  ASSERT_TRUE(icon.is_valid());
+  SkBitmap bitmap_loaded =
+      IconUtil::CreateSkBitmapFromHICON(icon.get(), gfx::Size(size, size));
+  EXPECT_EQ(bitmap_loaded.getColor(1, 1), semi_transparent_red);
+}

ui/gfx/skbitmap_operations.cc

@@ -628,11 +628,11 @@ SkBitmap SkBitmapOperations::DownsampleByTwo(const SkBitmap& bitmap) {
 SkBitmap SkBitmapOperations::UnPreMultiply(const SkBitmap& bitmap) {
   if (bitmap.isNull())
     return bitmap;
-  if (bitmap.isOpaque())
+  if (bitmap.alphaType() != kPremul_SkAlphaType)
     return bitmap;
 
   const SkImageInfo& opaque_info =
-      bitmap.info().makeAlphaType(kOpaque_SkAlphaType);
+      bitmap.info().makeAlphaType(kUnpremul_SkAlphaType);
   SkBitmap opaque_bitmap;
   opaque_bitmap.allocPixels(opaque_info);
 

ui/gfx/skbitmap_operations_unittest.cc

@@ -437,6 +437,7 @@ TEST(SkBitmapOperationsTest, DownsampleByTwoUntilSize) {
 TEST(SkBitmapOperationsTest, UnPreMultiply) {
   SkBitmap input;
   input.allocN32Pixels(2, 2);
+  EXPECT_EQ(input.alphaType(), kPremul_SkAlphaType);
 
   // Set PMColors into the bitmap
   *input.getAddr32(0, 0) = SkPackARGB32NoCheck(0x80, 0x00, 0x00, 0x00);
@@ -445,8 +446,10 @@ TEST(SkBitmapOperationsTest, UnPreMultiply) {
   *input.getAddr32(1, 1) = SkPackARGB32NoCheck(0x00, 0x00, 0xCC, 0x88);
 
   SkBitmap result = SkBitmapOperations::UnPreMultiply(input);
+  EXPECT_EQ(result.alphaType(), kUnpremul_SkAlphaType);
   EXPECT_EQ(2, result.width());
   EXPECT_EQ(2, result.height());
+  EXPECT_NE(result.getPixels(), input.getPixels());
 
   EXPECT_EQ(0x80000000, *result.getAddr32(0, 0));
   EXPECT_EQ(0x80FFFFFF, *result.getAddr32(1, 0));
@@ -454,6 +457,27 @@ TEST(SkBitmapOperationsTest, UnPreMultiply) {
   EXPECT_EQ(0x00000000u, *result.getAddr32(1, 1));  // "Division by zero".
 }
 
+TEST(SkBitmapOperationsTest, UnPreMultiplyOpaque) {
+  SkBitmap input;
+  input.allocN32Pixels(2, 2, true);
+  EXPECT_EQ(input.alphaType(), kOpaque_SkAlphaType);
+
+  SkBitmap result = SkBitmapOperations::UnPreMultiply(input);
+  EXPECT_EQ(result.alphaType(), kOpaque_SkAlphaType);
+  EXPECT_EQ(result.getPixels(), input.getPixels());
+}
+
+TEST(SkBitmapOperationsTest, UnPreMultiplyAlreadyUnPreMultiplied) {
+  SkBitmap input;
+  input.allocN32Pixels(2, 2);
+  input.setAlphaType(kUnpremul_SkAlphaType);
+  EXPECT_EQ(input.alphaType(), kUnpremul_SkAlphaType);
+
+  SkBitmap result = SkBitmapOperations::UnPreMultiply(input);
+  EXPECT_EQ(result.alphaType(), kUnpremul_SkAlphaType);
+  EXPECT_EQ(result.getPixels(), input.getPixels());
+}
+
 TEST(SkBitmapOperationsTest, CreateTransposedBitmap) {
   SkBitmap input;
   input.allocN32Pixels(2, 3);