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

Issue 634102 link

Starred by 4 users
Status: Fixed
Owner:
ccameron@chromium.org
Closed: May 2017
Cc:
█ zakerinasab@google.com
█ reed@chromium.org
█ hubbe@chromium.org
ccameron@chromium.org
█ msarett@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue skia:5695


Sign in to add a comment

Unify gfx::ColorSpace and SkColorSpace

Project Member Reported by junov@chromium.org, Aug 3 2016 
There are two parallel implementations of color space support in chromium. The implementation of SkColorSpace should be augmented to support all the colorspace representations that gfx::ColorSpace supports, and gfx::ColorSpace should be refactored to wrap an SkColorSpace.  This will help us avoid having to maintain parallel implementations and it will help us avoid having to implement and maintain complex conversion functions.

This will be useful for adding color management support in for canvases in blink, where we have no dependency on gfx and need to pass color-managed content to cc via mailboxes.

Comment 1 by junov@chromium.org, Aug 3 2016 

@zakerinasab: this issue is to be assigned to you as soon as your chromium account is set up.

Comment 2 by junov@chromium.org, Aug 3 2016

Cc: msarett@chromium.org

Comment 3 by msarett@chromium.org, Aug 3 2016

Cc: reed@chromium.org

Comment 4 by junov@chromium.org, Aug 15 2016

Owner: zakerinasab@chromium.org

Comment 5 by zakerinasab@chromium.org, Aug 30 2016

Blockedon: skia:5695
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 27 2016 

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

commit 87b65e9b67790efd6aaa2201bbee4485f9015dca
Author: ccameron <ccameron@chromium.org>
Date: Tue Dec 27 10:19:07 2016

Add SkColorSpace to gfx::ColorSpace

This allows more consistent and efficient conversion between the
two types.

Because we're including the header for SkColorSpace, targets that
include this can't play fast and loose with dependencies. Break off
the color space dependencies as //ui/gfx:color_space, and include
it in the media and remoting BUILD.gn that need it.

BUG= 634102 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2598833002
Cr-Commit-Position: refs/heads/master@{#440747}

[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/cc/resources/texture_mailbox.cc
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/cc/resources/texture_mailbox.h
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/base/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/base/android/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/base/mac/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/capture/content/android/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/capture/video/android/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/video/picture.cc
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/media/video/picture.h
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/remoting/codec/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/events/mojo/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/BUILD.gn
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/color_space.cc
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/color_space.h
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/icc_profile.cc
[modify] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/icc_profile.h
[add] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/icc_profile_unittest.cc
[add] https://crrev.com/87b65e9b67790efd6aaa2201bbee4485f9015dca/ui/gfx/icc_profile_unittest.h
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 29 2016 

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

commit 0b283e725b6489c7be426f1958419d19e8b348b7
Author: ccameron <ccameron@chromium.org>
Date: Thu Dec 29 23:44:57 2016

Use consistent types for ICC profiles

In Blink we convert from a gfx::ICCProfile to a raw vector of data.
Stop doing this, and, instead, pass the gfx::ICCProfile (or its gfx::ColorSpace
or SkColorSpace, as appropriate).

Merge several locations where we store ICC profiles for testing
into ui/gfx/test/icc_profiles.

BUG= 634102 
TBR=esprehn

Review-Url: https://codereview.chromium.org/2605743002
Cr-Commit-Position: refs/heads/master@{#441004}

[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/renderer/render_view_impl.cc
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/renderer/render_view_impl.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/renderer/render_widget.cc
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/renderer/render_widget.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/renderer/render_widget_owner_delegate.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/test/BUILD.gn
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/content/test/layouttest_support.cc
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/platform/graphics/ColorBehavior.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/third_party/WebKit/public/web/WebView.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/BUILD.gn
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/color_space.cc
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/color_space.h
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/icc_profile_unittest.cc
[add] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/test/icc_profiles.cc
[rename] https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7/ui/gfx/test/icc_profiles.h
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 2 2017 

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

commit 8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529
Author: engedy <engedy@chromium.org>
Date: Mon Jan 02 20:05:48 2017

Revert of Use consistent types for ICC profiles (patchset #5 id:80001 of https://codereview.chromium.org/2605743002/ )

Reason for revert:
Rebaselining not possible due to discrepancy between test outputs produced by continuous builders vs. try jobs. See crbug.com/677822 for details.

Speculatively reverting this CL as it has to do with ICC profiles and I am mostly seeing differences of color shades in pixel diffs.

Original issue's description:
> Use consistent types for ICC profiles
>
> In Blink we convert from a gfx::ICCProfile to a raw vector of data.
> Stop doing this, and, instead, pass the gfx::ICCProfile (or its gfx::ColorSpace
> or SkColorSpace, as appropriate).
>
> Merge several locations where we store ICC profiles for testing
> into ui/gfx/test/icc_profiles.
>
> BUG= 634102 
> TBR=esprehn
>
> Committed: https://crrev.com/0b283e725b6489c7be426f1958419d19e8b348b7
> Cr-Commit-Position: refs/heads/master@{#441004}

TBR=chrishtr@chromium.org,avi@chromium.org,esprehn@chromium.org,ccameron@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 634102 ,677822

Review-Url: https://codereview.chromium.org/2609093002
Cr-Commit-Position: refs/heads/master@{#441078}

[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/renderer/render_view_impl.cc
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/renderer/render_view_impl.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/renderer/render_widget.cc
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/renderer/render_widget.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/renderer/render_widget_owner_delegate.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/test/BUILD.gn
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/content/test/layouttest_support.cc
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/platform/graphics/ColorBehavior.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/web/WebViewImpl.cpp
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/third_party/WebKit/public/web/WebView.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/BUILD.gn
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/color_space.cc
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/color_space.h
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/icc_profile_unittest.cc
[rename] https://crrev.com/8d0ed9b51268dc2265f3fdf5c9a3b330d9a8a529/ui/gfx/icc_profile_unittest.h
[delete] https://crrev.com/d567eed7437e594b23d27d101bb740e2ac0fab28/ui/gfx/test/icc_profiles.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 4 2017 

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

commit 83e8e9a3c4597bac367eac402956b39fe6761b9b
Author: ccameron <ccameron@chromium.org>
Date: Wed Jan 04 01:31:20 2017

Merge all test ICC profiles to one location

This is a re-land of half of the code in
https://codereview.chromium.org/2605743002
which was reverted because it changed layout test color.

TBR=avi,esprehn
BUG= 634102 

Review-Url: https://codereview.chromium.org/2615523002
Cr-Commit-Position: refs/heads/master@{#441284}

[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/content/test/BUILD.gn
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/content/test/layouttest_support.cc
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/BUILD.gn
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/color_space.cc
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/color_space.h
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/icc_profile_unittest.cc
[add] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/test/icc_profiles.cc
[rename] https://crrev.com/83e8e9a3c4597bac367eac402956b39fe6761b9b/ui/gfx/test/icc_profiles.h
Project Member

Comment 12 by bugdroid1@chromium.org, Jan 19 2017 

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

commit c99fb6148dfdadf8f335462db33f2a520394c097
Author: ccameron <ccameron@chromium.org>
Date: Thu Jan 19 23:15:22 2017

Add histograms for ICC profile parsing

This will inform if we can restrict our color space support.

BUG= 634102 

Review-Url: https://codereview.chromium.org/2629723006
Cr-Commit-Position: refs/heads/master@{#444880}

[modify] https://crrev.com/c99fb6148dfdadf8f335462db33f2a520394c097/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
[modify] https://crrev.com/c99fb6148dfdadf8f335462db33f2a520394c097/tools/metrics/histograms/histograms.xml
Project Member

Comment 13 by bugdroid1@chromium.org, Jan 21 2017 

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

commit f976090c94c127ad92dee96dda3600bd88891b90
Author: ccameron <ccameron@chromium.org>
Date: Sat Jan 21 00:47:36 2017

Color: Use SkColorSpacePrimaries

This is towards unifying SkColorSpace and gfx::ColorSpace.

BUG= 634102 

Review-Url: https://codereview.chromium.org/2643773006
Cr-Commit-Position: refs/heads/master@{#445229}

[modify] https://crrev.com/f976090c94c127ad92dee96dda3600bd88891b90/ui/gfx/color_transform.cc
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 21 2017 

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

commit a7c18cdafd7db2229539a5e0b5b19c7cb8c6d862
Author: ccameron <ccameron@chromium.org>
Date: Sat Jan 21 02:45:08 2017

Use ICCProfile to construct SkColorSpace

This is a re-landing of parts of
https://codereview.chromium.org/2605743002/

To determine the reason for trybots not matching continuous bots.

TBR=esprehn
BUG= 634102 

Review-Url: https://codereview.chromium.org/2644233004
Cr-Commit-Position: refs/heads/master@{#445255}

[modify] https://crrev.com/a7c18cdafd7db2229539a5e0b5b19c7cb8c6d862/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
Project Member

Comment 15 by bugdroid1@chromium.org, Jan 21 2017 

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

commit 3d9c9a452905453d023cb1c632ee691ecbde6911
Author: ccameron <ccameron@chromium.org>
Date: Sat Jan 21 03:32:44 2017

Revert of Use ICCProfile to construct SkColorSpace (patchset #1 id:1 of https://codereview.chromium.org/2644233004/ )

Reason for revert:
Broke continuous bots, but not trybots
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11/builds/15367

Original issue's description:
> Use ICCProfile to construct SkColorSpace
>
> This is a re-landing of parts of
> https://codereview.chromium.org/2605743002/
>
> To determine the reason for trybots not matching continuous bots.
>
> TBR=esprehn
> BUG= 634102 
>
> Review-Url: https://codereview.chromium.org/2644233004
> Cr-Commit-Position: refs/heads/master@{#445255}
> Committed: https://chromium.googlesource.com/chromium/src/+/a7c18cdafd7db2229539a5e0b5b19c7cb8c6d862

TBR=esprehn@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 634102 

Review-Url: https://codereview.chromium.org/2646853007
Cr-Commit-Position: refs/heads/master@{#445265}

[modify] https://crrev.com/3d9c9a452905453d023cb1c632ee691ecbde6911/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
Project Member

Comment 16 by bugdroid1@chromium.org, Jan 26 2017 

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

commit 9791f229838cb7ebf67f111023350fef4bcaeef1
Author: ccameron <ccameron@chromium.org>
Date: Thu Jan 26 21:05:04 2017

Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG= 634102 

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446443}

[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/color_space.cc
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/color_space.h
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/color_transform.cc
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/color_transform.h
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/icc_profile.cc
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/icc_profile.h
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/ipc/color/gfx_param_traits.cc
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/ipc/color/gfx_param_traits.h
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/ipc/color/gfx_param_traits_macros.h
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/mojo/OWNERS
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/mojo/icc_profile.mojom
[modify] https://crrev.com/9791f229838cb7ebf67f111023350fef4bcaeef1/ui/gfx/mojo/icc_profile.typemap
[delete] https://crrev.com/4e4f32ae6ea8802694dd36543d4b18f080b5b231/ui/gfx/mojo/icc_profile_struct_traits.cc
[delete] https://crrev.com/4e4f32ae6ea8802694dd36543d4b18f080b5b231/ui/gfx/mojo/icc_profile_struct_traits.h
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 27 2017 

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

commit c3de81a0ca86c816b61189978d1c875875baafd5
Author: suzyh <suzyh@chromium.org>
Date: Fri Jan 27 00:28:10 2017

Revert of Use SkICC in gfx::ICCProfile and gfx::ColorSpace (patchset #7 id:110001 of https://codereview.chromium.org/2652503002/ )

Reason for revert:
Suspecting this patch is responsible for webkit_tests failures on Mac, where image diffs are showing slight colour changes.
e.g. https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.9/builds/41923

Original issue's description:
> Use SkICC in gfx::ICCProfile and gfx::ColorSpace
>
> This finally reaches the original goal of having gfx::ColorSpace be
> a (potentially lossily) compressed version of gfx::ICCProfile.
>
> The ICC profile's primaries and transfer funtion are extracted and
> stored in the gfx::ColorSpace, and are used in the event that the
> underlying ICC profile is purged from the cache.
>
> Update ICCProfile's IPC methods to ensure that the profile is touched
> in the cache and that its SkColorSpace is computed when it is
> received via IPC.
>
> BUG= 634102 
>
> Review-Url: https://codereview.chromium.org/2652503002
> Cr-Commit-Position: refs/heads/master@{#446443}
> Committed: https://chromium.googlesource.com/chromium/src/+/9791f229838cb7ebf67f111023350fef4bcaeef1

TBR=msarett@chromium.org,dcheng@chromium.org,hubbe@chromium.org,ccameron@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 634102 

Review-Url: https://codereview.chromium.org/2663453002
Cr-Commit-Position: refs/heads/master@{#446500}

[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/color_space.cc
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/color_space.h
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/color_transform.cc
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/color_transform.h
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/icc_profile.cc
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/icc_profile.h
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/ipc/color/gfx_param_traits.cc
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/ipc/color/gfx_param_traits.h
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/ipc/color/gfx_param_traits_macros.h
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/mojo/OWNERS
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/mojo/icc_profile.mojom
[modify] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/mojo/icc_profile.typemap
[add] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/mojo/icc_profile_struct_traits.cc
[add] https://crrev.com/c3de81a0ca86c816b61189978d1c875875baafd5/ui/gfx/mojo/icc_profile_struct_traits.h
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 28 2017 

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

commit 549738ba7a6df5e4872271233d40ecabc9857e7e
Author: ccameron <ccameron@chromium.org>
Date: Sat Jan 28 17:39:32 2017

Use SkICC in gfx::ICCProfile and gfx::ColorSpace

This finally reaches the original goal of having gfx::ColorSpace be
a (potentially lossily) compressed version of gfx::ICCProfile.

The ICC profile's primaries and transfer funtion are extracted and
stored in the gfx::ColorSpace, and are used in the event that the
underlying ICC profile is purged from the cache.

Update ICCProfile's IPC methods to ensure that the profile is touched
in the cache and that its SkColorSpace is computed when it is
received via IPC.

BUG= 634102 

Review-Url: https://codereview.chromium.org/2652503002
Cr-Commit-Position: refs/heads/master@{#446923}

[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/color_space.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/color_space.h
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/color_transform.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/color_transform.h
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/color_transform_unittest.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/icc_profile.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/icc_profile.h
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/icc_profile_unittest.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/ipc/color/gfx_param_traits.cc
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/ipc/color/gfx_param_traits.h
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/ipc/color/gfx_param_traits_macros.h
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/mojo/OWNERS
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/mojo/icc_profile.mojom
[modify] https://crrev.com/549738ba7a6df5e4872271233d40ecabc9857e7e/ui/gfx/mojo/icc_profile.typemap
[delete] https://crrev.com/d2355655832d2e34e5a9159dd2fd47b069823ddd/ui/gfx/mojo/icc_profile_struct_traits.cc
[delete] https://crrev.com/d2355655832d2e34e5a9159dd2fd47b069823ddd/ui/gfx/mojo/icc_profile_struct_traits.h

Comment 19 by jbroman@chromium.org, Feb 1 2017 

At least on mac_chromium_rel_ng, this is spammed quite a bit to stderr:

16:36:31.048 27463   [28085:771:0131/163630.974403:1016305086993:ERROR:icc_profile.cc(156)] Failed to find ICC profile matching SkColorSpace.

https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/379497/steps/webkit_tests%20%28with%20patch%29/logs/stdio

Comment 20 by ccameron@chromium.org, Feb 1 2017 

I'll check on cleaning that
Project Member

Comment 21 by bugdroid1@chromium.org, Feb 2 2017 

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

commit 9b2a0b1bc7034aa17ede1914e0620d3f71fe8dec
Author: ccameron <ccameron@chromium.org>
Date: Thu Feb 02 23:29:14 2017

Fix id tracking bug in ICCProfile

ICCProfiles are given an id when they are created, and are assumed to
be unique.

This breaks down in some tests, where we create ICCProfiles in the
browser and send them to the renderer (as usual), but then also create
test ICCProfiles in the in the renderer, that can conflict with the
ids provided by the browser.

Fix this by using hard-coded ids for the test ICCProfiles that are
created by the renderer.

BUG= 634102 

Review-Url: https://codereview.chromium.org/2663153002
Cr-Commit-Position: refs/heads/master@{#447874}

[modify] https://crrev.com/9b2a0b1bc7034aa17ede1914e0620d3f71fe8dec/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
[modify] https://crrev.com/9b2a0b1bc7034aa17ede1914e0620d3f71fe8dec/ui/gfx/icc_profile.cc
[modify] https://crrev.com/9b2a0b1bc7034aa17ede1914e0620d3f71fe8dec/ui/gfx/icc_profile.h
[modify] https://crrev.com/9b2a0b1bc7034aa17ede1914e0620d3f71fe8dec/ui/gfx/test/icc_profiles.cc
Project Member

Comment 22 by bugdroid1@chromium.org, Feb 3 2017 

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

commit 628896ee553990e2f0dc60a5a2c7b0b44535a10f
Author: ccameron <ccameron@chromium.org>
Date: Fri Feb 03 09:29:12 2017

Use gfx::ColorSpace instead of SkColorSpace in Blink

gfx::ColorSpace is used as input (speciying monitor color space) and
output (compositor texture mailbox) format for color spaces. Avoid
conversions by using this directly instead of SkColorSpace.

The SkColorSpace to pass to Skia APIs is accessible via the method
gfx::ColorSpace::ToSkColorSpace.

Delete gfx::ColorSpace::FromSkColorSpace and
gfx::ICCProfile::FromSkColorSpace, since they could not be implemented
reliably.

For CanvasRenderingContext and Canvas2DLayerBridge, create a
distinction between the gfx::ColorSpace that the canvas is to be
interpreted in by the compositor, and the SkColorSpace that is to be
used by SkSurfaces during rendering (for canvases that are not using
Skia's color conversion, but still need to specify their color space to
the compositor).

BUG= 634102 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2660393002
Cr-Commit-Position: refs/heads/master@{#447969}

[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/PRESUBMIT.py
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/html/ImageData.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/html/canvas/CanvasRenderingContext.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/core/paint/HTMLCanvasPainterTest.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2DTest.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/Canvas2DImageBufferSurface.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridgeTest.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/ColorBehavior.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/ColorBehavior.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/graphics/ContentLayerDelegate.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/color_space.cc
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/color_space.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/color_space_win.cc
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/icc_profile.cc
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/icc_profile.h
[modify] https://crrev.com/628896ee553990e2f0dc60a5a2c7b0b44535a10f/ui/gfx/icc_profile_unittest.cc

Comment 23 by zakerinasab@chromium.org, Mar 29 2017

Owner: ccameron@chromium.org

Comment 24 by ccameron@chromium.org, May 3 2017

Status: Fixed (was: Assigned)
Marking this as fixed.

We pass gfx::ColorSpace around now everywhere except in the boundary into Skia, where we convert to an SkColorSpace at the last moment.
►

Sign in to add a comment