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

Issue 811318 link

Starred by 1 user
Status: Fixed
Owner:
█ zakerinasab@chromium.org
Last visit > 30 days ago
Closed: Mar 2018
Cc:
█ junov@chromium.org
mtklein@chromium.org
fs...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 634542



Sign in to add a comment

Don't use nullptr as the destination color space in readPixels

Project Member Reported by zakerinasab@chromium.org, Feb 12 2018 
In multiple locations in Blink->Canvas code we use nullptr as the color space of the destination SkImageInfo when calling SkImage::readPixels(), trusting Skia to interpret this as "don't do any color conversion when reading the pixels".

This is how Skia works now (Look at can_memcpy() in SkConvertPixels.cpp). However, it is safer to set the destination color space the same as the source instead, so things don't get messy if Skia changed this behavior in their ongoing color space refactoring effort.
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 19 2018 

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

commit dd3337e03c70a1b6289ea4206c8d89c3bdb2558f
Author: Reza.Zakerinasab <zakerinasab@chromium.org>
Date: Mon Mar 19 15:50:20 2018

Always pass a valid destination color space to SkImage::readPixels()

In a few locations inside canvas code we are passing nullptr color space to
SkImage::readPixels() for two purposes:
- Avoid color conversion
- Enforce premul / unpremul to happen in linear space

This CL removes these calls and passes the correct color space. To avoid color
conversion, we should always pass the color space of the source image to readPixels().

Bug:  811318 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I6be1791d2d1ce82c7c444c140a4f43e572096150
Reviewed-on: https://chromium-review.googlesource.com/946678
Reviewed-by: Justin Novosad <junov@chromium.org>
Reviewed-by: Olivia Lai <xlai@chromium.org>
Commit-Queue: Mohammad Reza Zakerinasab <zakerinasab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544048}
[modify] https://crrev.com/dd3337e03c70a1b6289ea4206c8d89c3bdb2558f/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/dd3337e03c70a1b6289ea4206c8d89c3bdb2558f/third_party/WebKit/Source/platform/graphics/OffscreenCanvasResourceProvider.cpp
[modify] https://crrev.com/dd3337e03c70a1b6289ea4206c8d89c3bdb2558f/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/dd3337e03c70a1b6289ea4206c8d89c3bdb2558f/third_party/WebKit/Source/platform/graphics/gpu/ImageLayerBridge.cpp

Comment 2 by zakerinasab@chromium.org, Mar 19 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment