New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 773247 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 634542



Sign in to add a comment

Re-enable color correct rendering for canvas

Project Member Reported by zakerinasab@chromium.org, Oct 10 2017

Issue description

As explained in https://bugs.chromium.org/p/chromium/issues/detail?id=772189, color correct rendering in canvas seems to be incorrect, breaking a set of color conversion tests. To resolve the issue, the canvas default color space is set to legacy color space in https://chromium-review.googlesource.com/703993.

Resolve the problem and turn canvas color correct rendering again.
 

Comment 1 Deleted

Comment 2 Deleted

Observations:

1. I tapped into the PaintShader (which is used to store the image pattern inside the PaintFlags, which is sent to canvas->drawRect()). Looking at the SkImage storage containing red_at_12_oclock_with_color_profile.jpg in PaintShader, the red is already at 12 oclock, which IIUC means that the image is already color converted to sRGB in the decoder, and rules out the assumption that we are not doing any color conversion in the pipeline.

  * If the color profile of the image is not applied, I expect to see a bluish color at 12 oclock in the paint shader, like what we have here: https://fiddle.skia.org/c/58b0526ceb05f5c6efe948e8f8b38ea1

2. However, the internal SkImage in PaintShader does have a color space, but it is not sRGB. The other plausible option was that may be it is still ColorSpin even though the image is decoded to sRGB. We cannot rely on SkColorSpace::Equals() to check ColorSpin since there is no guarantee that the ICC profile created by SkImage::MakeFromEncoded() is the same as the one generated by the image decoder. FWIW, SkColorSpace::Equals() returns false when comparing the color space of the paint shader SkImage with the one extracted from the file by SkImage::MakeFromEncoded(). There is a ICCProfileForTestingColorSpin() in ui/gfx/test/icc_profiles.h, but apparently it cannot be accessed from cc/paint/paint_shader.cc.

To rule out the possibility of having double colorpsin->sRGB color correction, I imitated the behavior in a fiddle. The double color correction output can be seen at the bottom left of the output here: https://fiddle.skia.org/c/85fad125b51e5701ff93a4aee6ec888b.

3. Assuming that we already ruled out having no color conversion in step 1,
surprisingly, the only way that we might get what we see in https://chromium-review.googlesource.com/c/chromium/src/+/598500/11/third_party/WebKit/LayoutTests/images/color-profile-image-canvas-expected.png is to have a reverse color conversion from sRGB to ColorSpin, as shown in the bottom right corner of the output here: https://fiddle.skia.org/c/85fad125b51e5701ff93a4aee6ec888b. However, AFAICT this doesn't make sense.

I think the source of the problem is somewhere in the image decoder when the color space of the SkImage is not set correctly. This is okay for legacy canvas as there is no color conversion there but messes with the color managed canvas. Please let me know if anything comes to your mind.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03414af91b3af6aa6176b256ad77ec03aff592cd

commit 03414af91b3af6aa6176b256ad77ec03aff592cd
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Sat Oct 28 00:00:32 2017

Merge kLegacy and kSRGB canvas color spaces

This change merges kLegacy and kSRGB canvas color spaces and sets
the default canvas color space to sRGB.

Bug:  770330 , 770760 , 773247 , 740197 , 744636 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I06c5e87339a0a1c932e81630049cbd9718f170a5
Reviewed-on: https://chromium-review.googlesource.com/731466
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512356}
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/VirtualTestSuites
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-colorspace-arguments.html
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-linear-rgb.html
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-p3.html
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-rec2020.html
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/html/ImageData.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/html/ImageData.h
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/html/ImageDataTest.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.h
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/core/imagebitmap/ImageBitmapTest.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/modules/canvas/canvas2d/CanvasRenderingContext2DSettings.idl
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/modules/canvas/canvas2d/CanvasRenderingContext2DTest.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/CanvasColorParams.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/CanvasColorParams.h
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/CanvasColorParamsTest.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/ColorCorrectionTestUtils.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/ColorCorrectionTestUtils.h
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/GraphicsTypes.h
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/03414af91b3af6aa6176b256ad77ec03aff592cd/third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp

Status: Fixed (was: Assigned)
Blocking: 634542

Sign in to add a comment