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

Issue 770760 link

Starred by 0 users
Status: Fixed
Owner:
█ zakerinasab@chromium.org
Last visit > 30 days ago
Closed: Oct 2017
Cc:
█ junov@chromium.org
brianosman@chromium.org
fs...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocking:
issue 634542
issue 770330



Sign in to add a comment

ImageData color conversion must use gamma encoded unpremul

Project Member Reported by zakerinasab@chromium.org, Oct 2 2017 
Assume the user creates an ImageData in color space A and puts the ImageData onto a canvas in color space B, where A or B or both have a non-linear transfer function. The correct procedure must be as follows:

- ImageData pixels are "gamma encoded unpremul" in color space A.
- If the transfer function of A is non-linear, pixels must get converted to "premul". Otherwise, do nothing.
- If the transfer function of color space B is non-linear, pixels must get color converted to "premul in color space B" and then "read in gamma encoded unpremul".
- Otherwise, if the transfer function of color space B is linear, pixels can directly get color converted to "unpremul in color space B".

It seems that the current color managed ImageData does not properly handle the premul/unpremul in gamma encoded space. This seems to be the root of the trybot failures observed in https://chromium-review.googlesource.com/c/chromium/src/+/693219/2.

Comment 1 by zakerinasab@chromium.org, Oct 2 2017 

A skia feature bug filed to see if Skia can extend the alpha conversion modes supported in SkColorSpaceXform: https://bugs.chromium.org/p/skia/issues/detail?id=7101

Comment 2 by zakerinasab@chromium.org, Oct 3 2017 

The rounding error seems to be higher than expected if we keep the color values in SRGB (specially when the pixels are close to transparent). To clarify the problem, consider this scenario:

* We have a sRGB canvas with a sRGB picture on it.
* We decide to use putImageData to change a pixel to this gamma-encoded unpremul pixel: [93,117,205,41]
* To do the "right" thing, we convert the pixel to premul by multiplying each color component by 41/255=0.16, resulting in premul pixel: [14,18,32,41]
* Then in ImageBuffer::GetImageData we read the premul pixel and divide each color component by 41/255=0.16, resulting in gamma-encoded unpremul pixel: [87,111,199,41]
* We have these rounding errors: 93->87, 117->111, 205->199

Math: 93.0*0.16=14.88, 14.0/0.16=87.5

What can we do about this?

Comment 3 by brianosman@chromium.org, Oct 3 2017 

#2: Is that what you're seeing? It looks like you rounded down (rather than round-to-nearest) when you did the premul math:

93 * (41 / 255) = 14.95

So I'd expect 15 in the buffer, and unpremuling 15 will round-trip perfectly. Or are you assuming that the transfer function gets applied somewhere?

Comment 4 by zakerinasab@chromium.org, Oct 3 2017 

Rounding to the nearest is a good suggestion. It should reduce the error range.
Fore the record, after discussing with junov@, we move forward with correct sRGB storage and retrieval. Unit tests and layout tests should change to be alpha-aware, i.e., we expect no conversion error for opaque pixels, and higher error levels as the pixels get more transparent.

Comment 5 by zakerinasab@chromium.org, Oct 25 2017

Labels: -Pri-3 Pri-2
Project Member

Comment 6 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

Comment 7 by zakerinasab@chromium.org, Oct 30 2017

Status: Fixed (was: Assigned)

Comment 8 by zakerinasab@chromium.org, Nov 27 2017

Blocking: 634542
►

Sign in to add a comment