GNOME Bugzilla – Bug 779942
Make GimpPickButton honor monitor profile
Last modified: 2018-05-24 17:33:52 UTC
Created attachment 347755 [details] The Gimp file where I detect the colordrift and a screenshot from Color Management Dialog I know that this behavior is related to colormanagement. But it seems strange to me, that for picking the color from the screen with the color picker the RGB-Profile is used. I except, that for picking the color from the monitor, the Monitor-Profile should be used. What I did: * I made a colored dot in a Gimp picture. (See attachment) * Afterwards I took the color picker tool and filled it with the color from this dot. The color was a little bit different from the dot I made. * I made a second dot with the new color beside the first dot. * Then I filled again the color picker with the color of the second dot. And again it was differnt. So the color was drifting each time I made a new dot. Greetings, Norbert
Thanks for the report. In 2.8, it uses no profile at all, it just takes the RGB values from the screen pixel, and uses them directly as if they were sRGB values. There is no reasonable way to fix this in 2.8, but in git master all of that has changed. Can you check a git master build, e.g. from http://nightly.darkrefraction.com/gimp/ and check if the problem is gone there?
Oh, I just realize it doesn't look at the monitor profile in git master either...
Created attachment 347767 [details] Prove, that Gimp uses the MonitorProfile for the Color Picker In the upper part you can see that there is no color drift if there is no monitor profile. in the lower part you can see that there is a color drift with the monitor profile.
(In reply to Michael Natterer from comment #1) > Thanks for the report. > > In 2.8, it uses no profile at all, it just takes the RGB values from > the screen pixel, and uses them directly as if they were sRGB values. > > There is no reasonable way to fix this in 2.8, but in git master > all of that has changed. Can you check a git master build, e.g. > from http://nightly.darkrefraction.com/gimp/ and check if the > problem is gone there? Thanks for your reply. But I'm sure Gimp uses the RGB-Profile for the color picker.I've done one test, which you can find in the new jpg attachment: In the upper part you can see that there is no color drift if there is no RGB profile. in the lower part you can see that there is a color drift with the RGB profile. I think it's fine, that Gimp uses profiles. But I suggest to use the Monitor profile for the color picker, not the RGB profile. The monitor profile was made with x-rite calibration for my laptop. It's an ICC V4, because Windows 10 didn't accept the old ICC V2 any more. Can you please check it again. Thanks.
We have a misunderstanding here, but we don't disagree :) The pixels as they are on screen are in the color space of the monitor profile. GIMP just takes one of the pixels and puts the R, G, B values into its internal color representation which is supposed to be sRGB. It *should* convert the picked pixel from the monitor color space to the sRGB color space in order to pick the right color. "Right" as in a roundtrip of painting, picking, painting should results in the same color.
Fixed for the default picking backend: commit 064c4527cb53e473f0582dc6f72fe1465030b16e Author: Michael Natterer <mitch@gimp.org> Date: Sun Mar 12 23:29:25 2017 +0100 Bug 779942 - Make GimpPickButton honor monitor profile Convert the picked pixel from the monitor color profile to sRGB. Only changed the default impl, not the quartz code. libgimpwidgets/gimppickbutton-default.c | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-)
Fix the last commit: commit e2ff186861896c102cf777d13f51573b19a96d0f Author: Michael Natterer <mitch@gimp.org> Date: Mon Mar 13 09:06:00 2017 +0100 Bug 779942 - Make GimpPickButton honor monitor profile Pass the right flags to gimp_color_transform_new(), and handle a NULL return value. libgimpwidgets/gimppickbutton-default.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
Is it finished? Looking at the commit in comment 6, I guess that maybe all what remains is fixing the quartz code? Something else? Anyway making it a blocker. This goes with all the color-managed changes.
Ok so from what I saw in current code: - This should work on X11 and Windows. - gimppickbutton-quartz.c does not have any color-management code yet. - GNOME/Wayland doesn't have color picking feature yet anyway (cf. bug 789756). - We implemented KDE/Wayland color-picking but the code is not color managed. I opened a bug at KDE's: https://bugs.kde.org/show_bug.cgi?id=387757 Also regarding Wayland, it seems anyway gimp_screen_get_color_profile() has only X11/Quartz/Win32 implementations right now. Can we get ICC profiles in Wayland or is this data private there? As a conclusion, it seems the only thing we can do right now is implementing color management on color picker for Quartz.
Jehan, this is exactly how I see the current state of affairs.
Is there anyone who uses macOS who can implement this so that we can get rid of this blocker? Kris, maybe? If you have a bit of time between diaper renewals? :-)
*** Bug 792617 has been marked as a duplicate of this bug. ***
Ok just trying to go forward. I have thoroughly double-read my code, but this is untested, and not even compiled (I don't have access to a macOS machine). Yet I know a few packagers compile for macOS and if I made any mistake, we should see bug reports or emails on mailing lists in the next few days, which I am relying on. Hopefully the logics is right too. Please anyone, review my commit! I am not closing the bug report because there are still 2 implementations not right yet (GNOME/Wayland which has simply no color picker API yet and KDE/Wayland not color managed), yet we can't do anything about these for the time being, so that should not be considered a blocker. commit 6c8300923d0deb24286492859e52ed13e1919e62 (HEAD -> master) Author: Jehan <jehan@girinstud.io> Date: Fri Mar 16 17:06:32 2018 +0100 Bug 779942 - Make GimpPickButton honor monitor profile. Quartz/macOS implementation for the color picker. This is untested but I am trying to advance our 2.10 blockers by implementing the base code for this feature. Please anyone with macOS machine access, review and fix if needed! libgimpwidgets/gimppickbutton-quartz.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 5 deletions(-)
(In reply to Jehan from comment #13) > Ok just trying to go forward. > > I have thoroughly double-read my code, but this is untested, and not even > compiled (I don't have access to a macOS machine). Yet I know a few > packagers compile for macOS and if I made any mistake, we should see bug > reports or emails on mailing lists in the next few days, which I am relying > on. > > Hopefully the logics is right too. Please anyone, review my commit! I will try to compile and if necessary fix it somewhere this week.
Hi Kris, (In reply to Kristian Rietveld from comment #14) > (In reply to Jehan from comment #13) > > Hopefully the logics is right too. Please anyone, review my commit! > > I will try to compile and if necessary fix it somewhere this week. You had any chance to try? :-) We are planning to release a dev version (or RC) very soon. Would be awesome to know if that is working properly.
It did build, but when trying to run it the code crashed. Two fixes have been pushed: commit ce3899e722c8079f18508a892f91114ba1ca8417 Author: Kristian Rietveld <kris@loopnest.org> Date: Mon Mar 26 22:55:49 2018 +0200 libgimpwidgets: gimppickbutton-quartz: do not release color_space The color_space reference should not be released because it was obtained with a Get-function, which means we do not have ownership. commit 0e9e167da8626766aeafcf9172b038caacd6a1ba Author: Kristian Rietveld <kris@loopnest.org> Date: Mon Mar 26 22:49:27 2018 +0200 libgimpwidgets: make pick button quartz work on macOS < 10.12 The symbol CGColorSpaceCopyICCData() is only available on macOS 10.12 and higher. We want GIMP to run from 10.9 onwards, so use the older symbol CGColorSpaceCopyICCProfile() even though this one is deprecated.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/1068.