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

Issue 781908 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 634542
issue 762559



Sign in to add a comment

With {premultiplyAlpha: "none"} we should never pass through premul format

Project Member Reported by zakerinasab@chromium.org, Nov 6 2017

Issue description

When premultiplyAlpha is set to "none" in ImageBitmapOptions, we should never pass through premul format in color managed ImageBitmap code.

This requires special attention as many Skia APIs either silently go through premul mode or only work in premul mode.
 
Labels: -Pri-3 Pri-2
We need layout tests with premultiply="none", color components > 0 and alpha = 0 for color-managed and non-color-managed code path.
Blockedon: skia:5733
Fixing scaling requires Skia to support unpremul mode in SkImage::scalePixels().
Blocking: 762559
Unpremul ImageBitmap resize code path in Blink should respect non-zero color components with zero alpha value when the option premultiplyAlpha is set to "none". This requires skia:5733 to get fixed. Meanwhile, I can think of two workarounds:

1. Keep RGB values intact, map alpha from 0-255 to, like, 32-255:
Map(A) = (A+32)*255/(255+32)
Code path: Map(A), premul, scale, unpremul, Unmap(A)

2. Do the scaling twice. Break RGBA to RGB/255 and A/255,255,255. This workaround does not need additional premul/unpremul step, but requires additional memory.
Code path: Break(RGB,A), scale(RGB), scale(A), Merge(RGB,A).

Thoughts?
Cc: brianosman@chromium.org
Regarding comment 5: The first solution also requires a copy of pixels as Map/Unmap cannot happen on the original data (as we might end up modifying alpha in the round-trip).

Comment 8 by junov@chromium.org, Nov 7 2017

Option 1) is unacceptably lossy IMHO, unless you use an FP16 intermediate. In that case, might as well do 2).
Cc: mtklein@chromium.org reed@google.com
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 20 2017

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

commit ec67cae3b77ca0f9d641a2722d8d1f6090b346fd
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Mon Nov 20 17:54:39 2017

With {premultiplyAlpha: "none"} ImageBitmap should avoid premul code path

When premultiplyAlpha is set to "none" in ImageBitmapOptions, we should never pass through premul
format in color managed ImageBitmap code.

Bug:  781908 ,  785313 
Change-Id: I3a2fc1664dd57530bdc31694c5f9d61e95a9501b
Reviewed-on: https://chromium-review.googlesource.com/759116
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517858}
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/core/html/ImageData.cpp
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/core/html/ImageData.h
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/core/html/ImageDataTest.cpp
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.h
[modify] https://crrev.com/ec67cae3b77ca0f9d641a2722d8d1f6090b346fd/third_party/WebKit/Source/modules/canvas/canvas2d/BaseRenderingContext2D.cpp

Blockedon: -skia:5733
Status: Fixed (was: Assigned)
Blocking: 634542

Sign in to add a comment