Respect colorSpace in DecodingImageGenerator::onGetPixels()

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 52 matching lines @@
   // this. skbug.com/5485
   if (ctx && !m_allDataReceived)
     return nullptr;
 
   // Other clients are serializers, which want the data even if it requires
   // copying, and even if the data is incomplete. (Otherwise they would
   // potentially need to decode the partial image in order to re-encode it.)
   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,
                                          SkYUVColorSpace* colorSpace) const {
   // YUV decoding does not currently support progressive decoding. See comment
   // in ImageFrameGenerator.h.
   if (!m_canYUVDecode || !m_allDataReceived)
     return false;
 
   TRACE_EVENT1("blink", "DecodingImageGenerator::queryYUV8", "sizes",
@@ skipping 44 matching lines @@
       ImageFrameGenerator::create(SkISize::Make(size.width(), size.height()),
                                   false, decoder->colorBehavior());
   if (!frame)
     return nullptr;
 
   return new DecodingImageGenerator(frame, info, segmentReader.release(), true,
                                     0);
 }
 
 }  // namespace blink