Summary: | [WPE][GTK] Add support for ICC color management | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex <eneeen> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Enhancement | CC: | annulen, aperez, berto, bugs-noreply, cgarcia, ews-watchlist, gyuyoung.kim, mcatanzaro, ryuan.choi, sami, sergio | ||||
Priority: | P2 | ||||||
Version: | Other | ||||||
Hardware: | PC | ||||||
OS: | Linux | ||||||
See Also: |
https://bugzilla.gnome.org/show_bug.cgi?id=787876 https://bugs.webkit.org/show_bug.cgi?id=223065 |
||||||
Bug Depends on: | |||||||
Bug Blocks: | 223442 | ||||||
Attachments: |
|
Description
Alex
2017-09-19 13:27:16 PDT
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 So a quick internet search suggests we need this for BMP, JPEG, JPEG 2000, PNG, and WebP. But not for GIF or ICO. Still happens with 2.30.0 Created attachment 422825 [details]
Patch
(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!! 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 :) 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 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.
:)
Committed r274273 (235170@main): <https://commits.webkit.org/235170@main> |