Respect colorSpace in DecodingImageGenerator::onGetPixels()

third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp

Index: third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp b/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp
index 78dea66626650db17f55dd143817340770044d42..b00ba16b276c651b06e0ee66f0bda7c8a5673943 100644
--- a/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp
+++ b/third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp
@@ -70,34 +70,86 @@ SkData* DecodingImageGenerator::onRefEncodedData(GrContext* ctx) {
   return m_data->getAsSkData().release();
 }
 
-bool DecodingImageGenerator::onGetPixels(const SkImageInfo& info,
+bool DecodingImageGenerator::onGetPixels(const SkImageInfo& dstInfo,
                                          void* pixels,
                                          size_t rowBytes,
-                                         SkPMColor table[],
-                                         int* tableCount) {
+                                         SkPMColor*,
+                                         int*) {
   TRACE_EVENT1("blink", "DecodingImageGenerator::getPixels", "frame index",
                static_cast<int>(m_frameIndex));
 
   // Implementation doesn't support scaling yet, so make sure we're not given a
   // different size.
-  if (info.width() != getInfo().width() || info.height() != getInfo().height())
+  if (dstInfo.width() != getInfo().width() ||
+      dstInfo.height() != getInfo().height()) {
     return false;
+  }
 
-  if (info.colorType() != getInfo().colorType()) {
-    // blink::ImageFrame may have changed the owning SkBitmap to
-    // kOpaque_SkAlphaType after fully decoding the image frame, so if we see a
-    // request for opaque, that is ok even if our initial alpha type was not
-    // opaque.
+  if (kN32_SkColorType != dstInfo.colorType()) {
     return false;
   }
 
+  // Skip the check for alphaType.  blink::ImageFrame may have changed the
+  // owning SkBitmap to kOpaque_SkAlphaType after fully decoding the image
+  // frame, so if we see a request for opaque, that is ok even if our initial
+  // alpha type was not opaque.
+
+  // Pass decodeColorSpace to the decoder.  That is what we can expect the
+  // output to be.
+  sk_sp<SkColorSpace> decodeColorSpace = getInfo().refColorSpace();
+  SkImageInfo decodeInfo = dstInfo.makeColorSpace(decodeColorSpace);
+
+  bool needsColorXform =

[scroggo_chromium, 2017/04/05 19:59:01]

nit: can be const?

[msarett1, 2017/04/06 22:05:29]

Done.

+      decodeColorSpace && dstInfo.colorSpace() &&
+      !SkColorSpace::Equals(decodeColorSpace.get(), dstInfo.colorSpace());
+  if (needsColorXform) {
+    m_frameGenerator->setAlphaOption(ImageDecoder::AlphaNotPremultiplied);

[scroggo_chromium, 2017/04/05 19:59:01]

I think you can move this inside if (!decodeInfo.isOpaque()).

Then you can even combine the two if statements.

[msarett1, 2017/04/06 22:05:29]

Thanks, of course.

+    if (!decodeInfo.isOpaque()) {
+      decodeInfo = decodeInfo.makeAlphaType(kUnpremul_SkAlphaType);
+    }
+  }
+
   PlatformInstrumentation::willDecodeLazyPixelRef(uniqueID());
   bool decoded = m_frameGenerator->decodeAndScale(
-      m_data.get(), m_allDataReceived, m_frameIndex, getInfo(), pixels,
+      m_data.get(), m_allDataReceived, m_frameIndex, decodeInfo, pixels,
       rowBytes);
   PlatformInstrumentation::didDecodeLazyPixelRef();
 
-  return decoded;
+  if (!decoded) {
+    return false;
+  }
+
+  if (needsColorXform) {
+    std::unique_ptr<SkColorSpaceXform> xform =
+        SkColorSpaceXform::New(decodeColorSpace.get(), dstInfo.colorSpace());
+
+    uint32_t* row = (uint32_t*)pixels;
+    for (int y = 0; y < dstInfo.height(); y++) {
+      SkColorSpaceXform::ColorFormat format =
+          SkColorSpaceXform::kRGBA_8888_ColorFormat;
+      if (kN32_SkColorType == kBGRA_8888_SkColorType) {
+        format = SkColorSpaceXform::kBGRA_8888_ColorFormat;
+      }
+      SkAlphaType alphaType =
+          dstInfo.isOpaque() ? kOpaque_SkAlphaType : kUnpremul_SkAlphaType;
+      bool xformed =
+          xform->apply(format, row, format, row, dstInfo.width(), alphaType);
+      DCHECK(xformed);
+
+      // To be compatible with dst space blending, premultiply in the dst space.
+      if (kPremul_SkAlphaType == dstInfo.alphaType()) {
+        for (int x = 0; x < dstInfo.width(); x++) {
+          row[x] =
+              SkPreMultiplyARGB(SkGetPackedA32(row[x]), SkGetPackedR32(row[x]),
+                                SkGetPackedG32(row[x]), SkGetPackedB32(row[x]));
+        }
+      }
+
+      row = (uint32_t*)(((uint8_t*)row) + rowBytes);
+    }
+  }
+
+  return true;
 }
 
 bool DecodingImageGenerator::onQueryYUV8(SkYUVSizeInfo* sizeInfo,