Color correct ImageBitmap(HTMLImageElement*) constructor

third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp

 /*
  * Copyright (c) 2013, 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 22 matching lines @@
 #include "SkPixelRef.h"  // FIXME: qualify this skia header file.
 #include "core/dom/Document.h"
 #include "core/fetch/ImageResource.h"
 #include "core/fetch/MemoryCache.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "platform/graphics/StaticBitmapImage.h"
 #include "platform/graphics/skia/SkiaUtils.h"
 #include "platform/heap/Handle.h"
+#include "platform/image-decoders/ImageDecoder.h"
 #include "platform/network/ResourceRequest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColorSpaceXform.h"
 #include "third_party/skia/include/core/SkImage.h"
 #include "third_party/skia/include/core/SkSurface.h"
 
 namespace blink {
 
 class ImageBitmapTest : public ::testing::Test {
  protected:
   virtual void SetUp() {
     sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(10, 10);
     surface->getCanvas()->clear(0xFFFFFFFF);
     m_image = surface->makeImageSnapshot();
 
     sk_sp<SkSurface> surface2 = SkSurface::MakeRasterN32Premul(5, 5);
     surface2->getCanvas()->clear(0xAAAAAAAA);
     m_image2 = surface2->makeImageSnapshot();
 
     // Save the global memory cache to restore it upon teardown.
     m_globalMemoryCache = replaceMemoryCacheForTesting(MemoryCache::create());
+
+    // Save the state of experimental canvas features and color correct
+    // rendering flags to restore them on teardown.
+    experimentalCanvasFeatures =
+        RuntimeEnabledFeatures::experimentalCanvasFeaturesEnabled();
+    colorCorrectRendering =
+        RuntimeEnabledFeatures::colorCorrectRenderingEnabled();
   }
   virtual void TearDown() {
     // Garbage collection is required prior to switching out the
     // test's memory cache; image resources are released, evicting
     // them from the cache.
     ThreadState::current()->collectGarbage(BlinkGC::NoHeapPointersOnStack,
                                            BlinkGC::GCWithSweep,
                                            BlinkGC::ForcedGC);
 
     replaceMemoryCacheForTesting(m_globalMemoryCache.release());
+    RuntimeEnabledFeatures::setExperimentalCanvasFeaturesEnabled(
+        experimentalCanvasFeatures);
+    RuntimeEnabledFeatures::setColorCorrectRenderingEnabled(
+        colorCorrectRendering);
   }
 
   sk_sp<SkImage> m_image, m_image2;
   Persistent<MemoryCache> m_globalMemoryCache;
+  bool experimentalCanvasFeatures;
+  bool colorCorrectRendering;
 };
 
 TEST_F(ImageBitmapTest, ImageResourceConsistency) {
   const ImageBitmapOptions defaultOptions;
   HTMLImageElement* imageElement =
       HTMLImageElement::create(*Document::create());
   ImageResource* image =
       ImageResource::create(StaticBitmapImage::create(m_image).get());
   imageElement->setImageResource(image);
 
   Optional<IntRect> cropRect =
       IntRect(0, 0, m_image->width(), m_image->height());
   ImageBitmap* imageBitmapNoCrop = ImageBitmap::create(
       imageElement, cropRect, &(imageElement->document()), defaultOptions);
   cropRect = IntRect(m_image->width() / 2, m_image->height() / 2,
                      m_image->width() / 2, m_image->height() / 2);
   ImageBitmap* imageBitmapInteriorCrop = ImageBitmap::create(
       imageElement, cropRect, &(imageElement->document()), defaultOptions);
   cropRect = IntRect(-m_image->width() / 2, -m_image->height() / 2,
                      m_image->width(), m_image->height());
   ImageBitmap* imageBitmapExteriorCrop = ImageBitmap::create(
       imageElement, cropRect, &(imageElement->document()), defaultOptions);
   cropRect = IntRect(-m_image->width(), -m_image->height(), m_image->width(),
                      m_image->height());
   ImageBitmap* imageBitmapOutsideCrop = ImageBitmap::create(
       imageElement, cropRect, &(imageElement->document()), defaultOptions);
 
-  ASSERT_EQ(imageBitmapNoCrop->bitmapImage()->imageForCurrentFrame(),
+  ASSERT_NE(imageBitmapNoCrop->bitmapImage()->imageForCurrentFrame(),
             imageElement->cachedImage()->getImage()->imageForCurrentFrame());
   ASSERT_NE(imageBitmapInteriorCrop->bitmapImage()->imageForCurrentFrame(),
             imageElement->cachedImage()->getImage()->imageForCurrentFrame());
   ASSERT_NE(imageBitmapExteriorCrop->bitmapImage()->imageForCurrentFrame(),
             imageElement->cachedImage()->getImage()->imageForCurrentFrame());
 
   StaticBitmapImage* emptyImage = imageBitmapOutsideCrop->bitmapImage();
   ASSERT_NE(emptyImage->imageForCurrentFrame(),
             imageElement->cachedImage()->getImage()->imageForCurrentFrame());
 }
 
 // Verifies that ImageBitmaps constructed from HTMLImageElements hold a
 // reference to the original Image if the HTMLImageElement src is changed.
 TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
   HTMLImageElement* image = HTMLImageElement::create(*Document::create());
   ImageResource* originalImageResource =
       ImageResource::create(StaticBitmapImage::create(m_image).get());
   image->setImageResource(originalImageResource);
 
   const ImageBitmapOptions defaultOptions;
   Optional<IntRect> cropRect =
       IntRect(0, 0, m_image->width(), m_image->height());
   ImageBitmap* imageBitmap = ImageBitmap::create(
       image, cropRect, &(image->document()), defaultOptions);
-  ASSERT_EQ(imageBitmap->bitmapImage()->imageForCurrentFrame(),
+  // As we are applying color space conversion for the "default" mode,
+  // this verifies that the color corrected image is not the same as the
+  // source.
+  ASSERT_NE(imageBitmap->bitmapImage()->imageForCurrentFrame(),
             originalImageResource->getImage()->imageForCurrentFrame());
 
   ImageResource* newImageResource =
       ImageResource::create(StaticBitmapImage::create(m_image2).get());
   image->setImageResource(newImageResource);
 
-  // The ImageBitmap should contain the same data as the original cached image
   {
-    ASSERT_EQ(imageBitmap->bitmapImage()->imageForCurrentFrame(),
+    ASSERT_NE(imageBitmap->bitmapImage()->imageForCurrentFrame(),
               originalImageResource->getImage()->imageForCurrentFrame());
     SkImage* image1 = imageBitmap->bitmapImage()->imageForCurrentFrame().get();
     ASSERT_NE(image1, nullptr);
     SkImage* image2 =
         originalImageResource->getImage()->imageForCurrentFrame().get();
     ASSERT_NE(image2, nullptr);
-    ASSERT_EQ(image1, image2);
+    ASSERT_NE(image1, image2);
   }
 
   {
     ASSERT_NE(imageBitmap->bitmapImage()->imageForCurrentFrame(),
               newImageResource->getImage()->imageForCurrentFrame());
     SkImage* image1 = imageBitmap->bitmapImage()->imageForCurrentFrame().get();
     ASSERT_NE(image1, nullptr);
     SkImage* image2 =
         newImageResource->getImage()->imageForCurrentFrame().get();
     ASSERT_NE(image2, nullptr);
     ASSERT_NE(image1, image2);
   }
 }
 
+enum class ColorSpaceConversion : uint8_t {
+  NONE = 0,
+  // this is the only mode that is not color corrected
+  DEFAULT_NOT_COLOR_CORRECTED = 1,
+  DEFAULT_COLOR_CORRECTED = 2,
+  SRGB = 3,
+  LINEAR_RGB = 4,
+
+  LAST = LINEAR_RGB
+};
+
+static ImageBitmap* createImageBitmapWithColorSpaceConversion(
+    HTMLImageElement* image,
+    Optional<IntRect>& cropRect,
+    Document* document,
+    const ColorSpaceConversion& colorSpaceConversion) {
+  // Set the color space conversion in ImageBitmapOptions
+  ImageBitmapOptions options;
+  static const std::vector<std::string> conversions = {

[dcheng, 2016/11/23 02:23:54]

Please use Vector and String in Blink (which will also simplify the conversion
code that is present below)

[zakerinasab, 2016/11/24 18:18:32]

Done.

+      "none", "default", "default", "srgb", "linear-rgb"};
+  options.setColorSpaceConversion(
+      WTF::String(conversions[(uint8_t)(colorSpaceConversion)].c_str()));

[dcheng, 2016/11/23 02:23:54]

Here and elsewhere, casts should be using C++-style casts (static_cast, etc)
rather than C-style casts.

[zakerinasab, 2016/11/24 18:18:33]

Done.

[dcheng, 2016/11/24 20:28:15]

Looks like there's still casts like (void*), etc.

[zakerinasab, 2016/11/24 20:51:12]

Done.

+
+  // Set the runtime flags
+  bool runtimeFlag = (colorSpaceConversion !=
+                      ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED);
+  RuntimeEnabledFeatures::setExperimentalCanvasFeaturesEnabled(runtimeFlag);
+  RuntimeEnabledFeatures::setColorCorrectRenderingEnabled(runtimeFlag);
+
+  // Create and return the ImageBitmap
+  return ImageBitmap::create(image, cropRect, &(image->document()), options);
+}
+
+TEST_F(ImageBitmapTest, ImageBitmapColorSpaceConversion) {
+  HTMLImageElement* imageElement =
+      HTMLImageElement::create(*Document::create());
+
+  SkPaint p;
+  p.setColor(SK_ColorRED);
+  sk_sp<SkColorSpace> srcRGBColorSpace =
+      SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+
+  SkImageInfo rasterImageInfo =
+      SkImageInfo::MakeN32Premul(100, 100, srcRGBColorSpace);
+  sk_sp<SkSurface> surface(SkSurface::MakeRaster(rasterImageInfo));
+  surface->getCanvas()->drawCircle(50, 50, 50, p);
+  sk_sp<SkImage> image = surface->makeImageSnapshot();
+
+  uint8_t* srcPixel = new uint8_t[rasterImageInfo.bytesPerPixel()];
+  image->readPixels(rasterImageInfo.makeWH(1, 1), (void*)(srcPixel),
+                    image->width() * rasterImageInfo.bytesPerPixel(), 50, 50);
+
+  ImageResource* originalImageResource =
+      ImageResource::create(StaticBitmapImage::create(image).get());
+  imageElement->setImageResource(originalImageResource);
+
+  Optional<IntRect> cropRect = IntRect(0, 0, image->width(), image->height());
+
+  // Create and test the ImageBitmap objects.
+  // We don't check "none" color space conversion as it requires the encoded
+  // data in a format readable by ImageDecoder. Furthermore, the code path for
+  // "none" color space conversion is not affected by this CL.
+
+  sk_sp<SkColorSpace> colorSpace = nullptr;
+  SkColorType colorType = SkColorType::kN32_SkColorType;
+  SkColorSpaceXform::ColorFormat colorFormat32 =
+      (colorType == kBGRA_8888_SkColorType)
+          ? SkColorSpaceXform::ColorFormat::kBGRA_8888_ColorFormat
+          : SkColorSpaceXform::ColorFormat::kRGBA_8888_ColorFormat;
+  SkColorSpaceXform::ColorFormat colorFormat = colorFormat32;
+
+  for (uint8_t i = (uint8_t)(ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED);
+       i <= (uint8_t)(ColorSpaceConversion::LAST); i++) {
+    ColorSpaceConversion csConversion = (ColorSpaceConversion)(i);

[xidachen, 2016/11/22 20:34:38]

Please avoid using abbreviation, use colorSpaceConversion instead of
csConversion.

[zakerinasab, 2016/11/24 18:18:32]

Done.

+    ImageBitmap* imageBitmap = createImageBitmapWithColorSpaceConversion(
+        imageElement, cropRect, &(imageElement->document()), csConversion);
+    SkImage* convertedImage =
+        imageBitmap->bitmapImage()->imageForCurrentFrame().get();
+
+    switch (csConversion) {
+      case ColorSpaceConversion::NONE:
+        NOTREACHED();
+        break;
+      case ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED:
+        colorSpace = ImageDecoder::globalTargetColorSpace();
+        colorFormat = colorFormat32;
+        break;
+      case ColorSpaceConversion::DEFAULT_COLOR_CORRECTED:
+      case ColorSpaceConversion::SRGB:
+        colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+        colorFormat = colorFormat32;
+        break;
+      case ColorSpaceConversion::LINEAR_RGB:
+        colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGBLinear_Named);
+        colorType = SkColorType::kRGBA_F16_SkColorType;
+        colorFormat = SkColorSpaceXform::ColorFormat::kRGBA_F16_ColorFormat;
+        break;
+      default:
+        NOTREACHED();
+    }
+
+    SkImageInfo imageInfo = SkImageInfo::Make(
+        1, 1, colorType, SkAlphaType::kPremul_SkAlphaType, colorSpace);
+    uint8_t* convertedPixel = new uint8_t[imageInfo.bytesPerPixel()];

[dcheng, 2016/11/23 02:23:54]

It's not clear if this leaks. I would recommend writing this as:

  auto convertedPixels = makeUnique<uint8_t[]>(imageInfo.bytesPerPixel());

[zakerinasab, 2016/11/24 18:18:33]

Done.

[dcheng, 2016/11/24 20:28:15]

There's a few more instances of this throughout that need to be fixed as well.

[zakerinasab, 2016/11/24 20:51:12]

Done.

+    convertedImage->readPixels(
+        imageInfo, (void*)(convertedPixel),
+        convertedImage->width() * imageInfo.bytesPerPixel(), 50, 50);
+
+    // Transform the source pixel and check if the image bitmap color conversion
+    // is done correctly.
+    std::unique_ptr<SkColorSpaceXform> colorSpaceXform =
+        SkColorSpaceXform::New(srcRGBColorSpace.get(), colorSpace.get());
+    uint8_t* transformedPixel = new uint8_t[imageInfo.bytesPerPixel()];
+    colorSpaceXform->apply(colorFormat, (void*)(transformedPixel),
+                           colorFormat32, (void*)(srcPixel), 1,
+                           SkAlphaType::kPremul_SkAlphaType);
+
+    int compare = std::memcmp(convertedPixel, transformedPixel,
+                              imageInfo.bytesPerPixel());
+    ASSERT_EQ(compare, 0);
+  }
+}
+
 }  // namespace blink