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

Issue 754713 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocked on:
issue skia:6958

Blocking:
issue 634542



Sign in to add a comment

Fix null color space images in ImageBitmap=>ApplyColorSpaceConversion()

Project Member Reported by zakerinasab@chromium.org, Aug 11 2017

Issue description

In ImageBitmap=>ApplyColorSpaceConversion(), if the image does not have any color space info and we must color convert to SRGB, we only tag the image as SRGB.
Since we cannot use SkImage::makeColorSpace() on SkImages with no color space to get the image in SRGB, we have to do this using a slow code path (reading the pixels and creating a new SRGB SkImage). This is inefficient and also converts GPU-backed images to CPU-backed. For now, we ignore the tagging and let the images remain in null color space. This must be okay if color management is only supported for SRGB, but we need to fix this if SkImages with no color space info should be converted to non-SRGB color spaces.
 
Another solution suggested by brianosman@ is to make sure that Skia considers no color space SkImages as SRGB. According to brianosman@, this is the current behavior of SkImage::makeColorSpace(), which means if everything else is correct in Blink, we might only need an extra tag or something similar to guarantee the correct behavior of color management pipeline.
Blockedon: skia:6958
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 6 2017

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

commit af0188902ff0dada4e69be68c4c057717c36a2b1
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Wed Sep 06 16:19:30 2017

Add layout tests for color managed createImageBitmap

Along with some corrections in the color management code path,
this change adds three layout tests to test createImageBitmap in
linear-rgb, p3 and rec2020 color spaces from different sources in
different color spaces. The test coverage is not complete as some
sources (image, video, svg) are not decodable in non-srgb color spaces
for now. Furthermore, color correcting OffscreenCanvas works fine in
software code path, but fails in accelerated code path ( crbug.com/761424 ).

Bug:  668547 , 754713 
Change-Id: I5bbc23e00d8886c1c34e171101036195dd4a4e5c
Reviewed-on: https://chromium-review.googlesource.com/646508
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499983}
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-linear-rgb.html
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-p3.html
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/canvas-createImageBitmap-rec2020.html
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/resources/pattern-srgb-fullcolor.ogv
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/resources/pattern-srgb-transparent.png
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/resources/pattern-srgb.png
[add] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/LayoutTests/virtual/color_space/fast/canvas/color-space/resources/pattern-srgb.svg
[modify] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/Source/core/html/ImageData.cpp
[modify] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/Source/core/imagebitmap/ImageBitmapTest.cpp
[modify] https://crrev.com/af0188902ff0dada4e69be68c4c057717c36a2b1/third_party/WebKit/Source/core/offscreencanvas/OffscreenCanvas.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 18 2017

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

commit d16fd252591ff20f731c434badf16f2f8589f6b6
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Mon Sep 18 21:41:28 2017

Avoid GPU readbacks in ImageBitmap when tagging SkImages as sRGB

In chromium-review.googlesource.com/646508 SkImage objects without color space info are tagged
as sRGB through a readPixels - writePixels round-trip, converting accelerated SkImage objects
to de-accelerated ones. This change list resolves this by drawing onto a surface (instead of
using readPixels).

Bug:  754713 ,  740197 
Change-Id: I9609fe6bb2a8d32ba66a4eceacacb8de3a79f588
Reviewed-on: https://chromium-review.googlesource.com/670519
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502682}
[modify] https://crrev.com/d16fd252591ff20f731c434badf16f2f8589f6b6/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 26 2017

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

commit 982d26079cf7f293d3dd05cf6783e591ca937259
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Tue Sep 26 17:37:34 2017

Remove GPU readback in ImageBitmap when tagging SkImages as sRGB

In chromium-review.googlesource.com/670519 SkImage objects without color space
info are tagged as sRGB by drawin the image onto a sRGB canvas. This is not
needed anymore as makeColorSpace() assumes the no-color-space images as sRGB.

Bug:  754713 ,  740197 ,  767275 
Change-Id: Ic6a9461919bf49d3bf316d93ca6a5e10bc6f3d7c
Reviewed-on: https://chromium-review.googlesource.com/672023
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504414}
[modify] https://crrev.com/982d26079cf7f293d3dd05cf6783e591ca937259/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp

Status: Fixed (was: Assigned)
Blocking: 634542

Sign in to add a comment