Bug 177185 – [WPE][GTK] Add support for ICC color management

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alex]

Reporting bug to Webkit as GNOME Web browser developer suggested:
https://bugzilla.gnome.org/show_bug.cgi?id=787876

Safari and Chrome have color management and are Webkit based. Can WebkitGTK+ based GNOME Web have it to?

[Alberto Garcia]

More details here:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=880697

In short, WebKitGTK does not show this image with the right colors (compare it with other browsers to see the difference):

http://okgo.net/build/wp-content/uploads/2016/11/Email-Blast-Photo.jpg

[Michael Catanzaro]

So a quick internet search suggests we need this for BMP, JPEG, JPEG 2000, PNG, and WebP. But not for GIF or ICO.

[Alberto Garcia]

Still happens with 2.30.0

[Carlos Garcia Campos]

Created attachment 422825 [details]
Patch

[Alberto Garcia]

(In reply to Carlos Garcia Campos from comment #4)
> Created attachment 422825 [details]
> Patch

I tested it with WebKitGTK 2.30.5 and I confirm that it solves the problem, thanks!!

[Adrian Perez]

Comment on attachment 422825 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422825&action=review

> Source/WebCore/platform/graphics/PlatformDisplay.cpp:278
> +{

It would be nice here to fall-back to asking colord what's the color profile for
the screen. That would work also for Wayland-based compositors and desktop
environments which populate colord's view of the available screens (at least
GNOME Shell and Weston with the cms-colord plug-in do this).

Returning a hardwired RGB profile would be in my mind the last of resorts
after trying colord.

By the way, I think it's fine to skip adding support for colord in this patch,
but if you agree that it would be nice to consider it, let's create a new bug
to track that and maybe add a TODO/FIXME in the code ^_^

> Source/cmake/FindLCMS2.cmake:70
> +

I would rather prefer to use an imported target in this CMake find-module.
We have been transitioning towards using imported targets, for example
FindManette.cmake uses an imported target and also does the dance of
regex-matching the version from the headers as a fallback so I think it
would be a good one to use as template here :)

[Michael Catanzaro]

Comment on attachment 422825 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422825&action=review

>> Source/WebCore/platform/graphics/PlatformDisplay.cpp:278
>> +{
> 
> It would be nice here to fall-back to asking colord what's the color profile for
> the screen. That would work also for Wayland-based compositors and desktop
> environments which populate colord's view of the available screens (at least
> GNOME Shell and Weston with the cms-colord plug-in do this).
> 
> Returning a hardwired RGB profile would be in my mind the last of resorts
> after trying colord.
> 
> By the way, I think it's fine to skip adding support for colord in this patch,
> but if you agree that it would be nice to consider it, let's create a new bug
> to track that and maybe add a TODO/FIXME in the code ^_^

Beware: talking to colord will only work if you do it in the UI process.

It might seem to work during development, because I know cgarcia has the web process sandbox disabled. It won't work for users, though. :P

[Adrian Perez]

Comment on attachment 422825 [details]
Patch

Changed to r+, after some discussion with Carlos Garcia, we agreed on
landing this as it covers common cases (as most consumer displays are
sRGB anyway), and we will file a separate bug regarding colord. The
CMake find-module can be rewritten later on as a follow-up, too.

:)

[Carlos Garcia Campos]

Committed r274273 (235170@main): <https://commits.webkit.org/235170@main>