After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779942 - Make GimpPickButton honor monitor profile
Make GimpPickButton honor monitor profile
Status: RESOLVED OBSOLETE
Product: GIMP
Classification: Other
Component: General
gimp-2-8
Other All
: Normal normal
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
: 792617 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-03-12 14:58 UTC by Norbert Sievers
Modified: 2018-05-24 17:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
The Gimp file where I detect the colordrift and a screenshot from Color Management Dialog (351.50 KB, image/x-xcf)
2017-03-12 14:58 UTC, Norbert Sievers
Details
Prove, that Gimp uses the MonitorProfile for the Color Picker (149.22 KB, image/jpeg)
2017-03-12 18:40 UTC, Norbert Sievers
Details

Description Norbert Sievers 2017-03-12 14:58:10 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
Comment 1 Michael Natterer 2017-03-12 17:20:16 UTC
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?
Comment 2 Michael Natterer 2017-03-12 17:24:51 UTC
Oh, I just realize it doesn't look at the monitor profile in git
master either...
Comment 3 Norbert Sievers 2017-03-12 18:40:29 UTC
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.
Comment 4 Norbert Sievers 2017-03-12 18:54:51 UTC
(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.
Comment 5 Michael Natterer 2017-03-12 21:11:54 UTC
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.
Comment 6 Michael Natterer 2017-03-12 22:31:33 UTC
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(-)
Comment 7 Michael Natterer 2017-03-13 08:07:30 UTC
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(-)
Comment 8 Jehan 2017-12-06 23:47:58 UTC
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.
Comment 9 Jehan 2017-12-10 02:07:58 UTC
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.
Comment 10 Michael Natterer 2017-12-12 11:54:22 UTC
Jehan, this is exactly how I see the current state of affairs.
Comment 11 Jehan 2018-01-11 04:51:44 UTC
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? :-)
Comment 12 Michael Natterer 2018-01-18 00:15:14 UTC
*** Bug 792617 has been marked as a duplicate of this bug. ***
Comment 13 Jehan 2018-03-16 16:23:43 UTC
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(-)
Comment 14 Kristian Rietveld 2018-03-18 21:05:24 UTC
(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.
Comment 15 Jehan 2018-03-23 17:32:57 UTC
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.
Comment 16 Kristian Rietveld 2018-03-26 21:00:49 UTC
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.
Comment 17 GNOME Infrastructure Team 2018-05-24 17:33:52 UTC
-- 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.