Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(82)

Unified Diff: third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp

Issue 2522693002: Color correct ImageBitmap(HTMLImageElement*) constructor (Closed)
Patch Set: Addressing crash on memory sanitizer trybot Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp b/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
index 4fb11fbac461365827a76721cd006280cf303930..c9666be3fd693be5d6f54526b5a20171dacab3ea 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
@@ -40,9 +40,11 @@
#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"
@@ -61,6 +63,15 @@ class ImageBitmapTest : public ::testing::Test {
// 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();
+ colorCorrectRenderingDefaultMode =
+ RuntimeEnabledFeatures::colorCorrectRenderingDefaultModeEnabled();
}
virtual void TearDown() {
// Garbage collection is required prior to switching out the
@@ -71,10 +82,19 @@ class ImageBitmapTest : public ::testing::Test {
BlinkGC::ForcedGC);
replaceMemoryCacheForTesting(m_globalMemoryCache.release());
+ RuntimeEnabledFeatures::setExperimentalCanvasFeaturesEnabled(
+ experimentalCanvasFeatures);
+ RuntimeEnabledFeatures::setColorCorrectRenderingEnabled(
+ colorCorrectRendering);
+ RuntimeEnabledFeatures::setColorCorrectRenderingDefaultModeEnabled(
+ colorCorrectRenderingDefaultMode);
}
sk_sp<SkImage> m_image, m_image2;
Persistent<MemoryCache> m_globalMemoryCache;
+ bool experimentalCanvasFeatures;
+ bool colorCorrectRendering;
+ bool colorCorrectRenderingDefaultMode;
};
TEST_F(ImageBitmapTest, ImageResourceConsistency) {
@@ -102,7 +122,7 @@ TEST_F(ImageBitmapTest, ImageResourceConsistency) {
ImageBitmap* imageBitmapOutsideCrop = ImageBitmap::create(
imageElement, cropRect, &(imageElement->document()), defaultOptions);
- ASSERT_EQ(imageBitmapNoCrop->bitmapImage()->imageForCurrentFrame(
+ ASSERT_NE(imageBitmapNoCrop->bitmapImage()->imageForCurrentFrame(
ColorBehavior::transformToTargetForTesting()),
imageElement->cachedImage()->getImage()->imageForCurrentFrame(
ColorBehavior::transformToTargetForTesting()));
@@ -135,7 +155,10 @@ TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
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(
ColorBehavior::transformToTargetForTesting()),
originalImageResource->getImage()->imageForCurrentFrame(
ColorBehavior::transformToTargetForTesting()));
@@ -144,9 +167,8 @@ TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
ImageResourceContent::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(
ColorBehavior::transformToTargetForTesting()),
originalImageResource->getImage()->imageForCurrentFrame(
ColorBehavior::transformToTargetForTesting()));
@@ -160,7 +182,7 @@ TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
->imageForCurrentFrame(ColorBehavior::transformToTargetForTesting())
.get();
ASSERT_NE(image2, nullptr);
- ASSERT_EQ(image1, image2);
+ ASSERT_NE(image1, image2);
}
{
@@ -182,4 +204,137 @@ TEST_F(ImageBitmapTest, ImageBitmapSourceChanged) {
}
}
+enum class ColorSpaceConversion : uint8_t {
+ NONE = 0,
+ 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 Vector<String> conversions = {"none", "default", "default",
+ "srgb", "linear-rgb"};
+ options.setColorSpaceConversion(
+ conversions[static_cast<uint8_t>(colorSpaceConversion)]);
+
+ // Set the runtime flags
+ bool flag = (colorSpaceConversion !=
+ ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED);
+ RuntimeEnabledFeatures::setExperimentalCanvasFeaturesEnabled(true);
+ RuntimeEnabledFeatures::setColorCorrectRenderingEnabled(flag);
+ RuntimeEnabledFeatures::setColorCorrectRenderingDefaultModeEnabled(!flag);
+
+ // 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();
+
+ std::unique_ptr<uint8_t[]> srcPixel(
+ new uint8_t[rasterImageInfo.bytesPerPixel()]());
+ image->readPixels(rasterImageInfo.makeWH(1, 1), srcPixel.get(),
+ image->width() * rasterImageInfo.bytesPerPixel(), 50, 50);
+
+ ImageResourceContent* originalImageResource =
+ ImageResourceContent::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 = static_cast<uint8_t>(
+ ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED);
+ i <= static_cast<uint8_t>(ColorSpaceConversion::LAST); i++) {
+ ColorSpaceConversion colorSpaceConversion =
+ static_cast<ColorSpaceConversion>(i);
+ ImageBitmap* imageBitmap = createImageBitmapWithColorSpaceConversion(
+ imageElement, cropRect, &(imageElement->document()),
+ colorSpaceConversion);
+ SkImage* convertedImage =
+ imageBitmap->bitmapImage()
+ ->imageForCurrentFrame(ColorBehavior::ignore())
+ .get();
+
+ switch (colorSpaceConversion) {
+ case ColorSpaceConversion::NONE:
+ NOTREACHED();
+ break;
+ case ColorSpaceConversion::DEFAULT_NOT_COLOR_CORRECTED:
+ // TODO(zakerinasab): Replace sRGB with a call to
+ // ImageDecoder::globalTargetColorSpace() when the crash problem on Mac
+ // is fixed. crbug.com/668546.
+ colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+ 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);
+ std::unique_ptr<uint8_t[]> convertedPixel(
+ new uint8_t[imageInfo.bytesPerPixel()]());
+ convertedImage->readPixels(
+ imageInfo, convertedPixel.get(),
+ 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());
+ std::unique_ptr<uint8_t[]> transformedPixel(
+ new uint8_t[imageInfo.bytesPerPixel()]());
+ colorSpaceXform->apply(colorFormat, transformedPixel.get(), colorFormat32,
+ srcPixel.get(), 1, SkAlphaType::kPremul_SkAlphaType);
+
+ int compare = std::memcmp(convertedPixel.get(), transformedPixel.get(),
+ imageInfo.bytesPerPixel());
+ ASSERT_EQ(compare, 0);
+ }
+}
+
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698