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

Issue 728332 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 713891



Sign in to add a comment

color: display list canvas has incorrect colors with forced color profile

Project Member Reported by ccameron@chromium.org, May 31 2017

Issue description

1. Run chrome with
     --enable-color-correct-rendering \
     --force-color-profile=color-spin-gamma24 \
2. Open the attached canvas.html
3. Observe that you get actual.png, not expected.png

This example used to work -- I suspect something is going wrong with the color conversion for display list canvas.
 
expected.png
80.6 KB View Download
actual.png
86.8 KB View Download
canvas.html
851 bytes View Download
Cc: msarett@chromium.org
/facepalm ... this is a pretty obvious regression that I added in SkPictureImageFilter.
This bug also causes the layout test images/color-profile-layer-filter.html to fail.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/eb0e60f1ec2498f5cdba96708f25bd7929201aff

commit eb0e60f1ec2498f5cdba96708f25bd7929201aff
Author: Christopher Cameron <ccameron@chromium.org>
Date: Thu Jun 01 14:42:24 2017

Ensure SkPictureImageFilter::onFilterImage doesn't double-convert color

Consider the following sequence of events:
1. SkPictureImageFilter::onFilterImage creates a local canvas, wraps it
   in a SkColorSpaceXformCanvas, and passed to...
2. SkPictureImageFilter::drawPictureAtLocalResolution creates a local
   canvas (localCanvas) wraps that in a SkColorSpaceXformCanvas, draws
   the picture to it, doing sRGB->fColorSpace conversion
3. We then call makeImageSnapshot to produce a SkSpecialImage, localImg,
   which is not tagged with any color space.
4. When the draw that localImg to the passed-in SkColorSpaceXformCanvas,
   which then performs sRGB->fColorSpace conversion a second time.

We now have performed color conversion twice.

One fix for this would be to have the image produced by the call to
localSurface->makeImageSnapshot() be tagged with fColorSpace. This is
somewhat involved.

The less invasive fix is to remove the SkColorSpaceXformCanvas in
SkPictureImageFilter::onFilterImage, and push it down into the two
branches, SkPictureImageFilter::drawPictureAtLocalResolution and
SkPictureImageFilter::drawPictureAtDeviceResolution.

BUG= 728332 

Change-Id: If2aa32e18ad660b3e361f1d90845eeb8555fe404
Reviewed-on: https://skia-review.googlesource.com/18282
Reviewed-by: Matt Sarett <msarett@google.com>
Reviewed-by: Christopher Cameron <ccameron@google.com>
Commit-Queue: Christopher Cameron <ccameron@google.com>

[modify] https://crrev.com/eb0e60f1ec2498f5cdba96708f25bd7929201aff/src/effects/SkPictureImageFilter.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 1 2017

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

commit c3568295e5606b20908b6155fd3cd0052fdb53be
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Thu Jun 01 17:49:12 2017

Roll src/third_party/skia/ baf41bd1c..c6912f712 (5 commits)

https://skia.googlesource.com/skia.git/+log/baf41bd1c8d9..c6912f712f34

$ git log baf41bd1c..c6912f712 --date=short --no-merges --format='%ad %ae %s'
2017-05-31 mtklein make SkColorFilter::onAppendStages() pure
2017-06-01 robertphillips Remove GrSurface-based surfaceContext factories from GrContextPriv
2017-05-31 ccameron Ensure SkPictureImageFilter::onFilterImage doesn't double-convert color
2017-06-01 reed fix and test colorfiltershader
2017-06-01 robertphillips Update more GrOps to split-opList world

Created with:
  roll-dep src/third_party/skia
BUG= 728332 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=herb@chromium.org

Change-Id: I3396532654f54234d1bf8774470df6435539a50a
Reviewed-on: https://chromium-review.googlesource.com/521242
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476344}
[modify] https://crrev.com/c3568295e5606b20908b6155fd3cd0052fdb53be/DEPS

Status: Fixed (was: Assigned)

Sign in to add a comment