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 488170 - use system monitor profile on OSX
use system monitor profile on OSX
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
git master
Other Mac OS
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2007-10-19 07:41 UTC by Sven Neumann
Modified: 2007-10-30 15:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
attempt to implement retrieval of the system monitor profile on OSX (1.32 KB, patch)
2007-10-19 07:45 UTC, Sven Neumann
none Details | Review
Added correct includes to patch so it compiles (1.56 KB, patch)
2007-10-23 01:19 UTC, Marianne Gagnon
none Details | Review
Patch to get CMS auto-detect profile on OS X (works on my computer) (2.78 KB, patch)
2007-10-29 19:06 UTC, Marianne Gagnon
reviewed Details | Review
Same as previous one but with a memory leak fixed (2.94 KB, patch)
2007-10-30 00:27 UTC, Marianne Gagnon
none Details | Review
cleaned up patch (2.96 KB, patch)
2007-10-30 07:06 UTC, Sven Neumann
none Details | Review
cleaned up patch that avoids the use of global variables (2.86 KB, patch)
2007-10-30 11:58 UTC, Sven Neumann
none Details | Review
Patch that compiles (2.88 KB, patch)
2007-10-30 13:06 UTC, Michael Natterer
committed Details | Review

Description Sven Neumann 2007-10-19 07:41:50 UTC
The lcms display filter which is responsible for color-correction of the image window in GIMP 2.4 currently has code to retrieve the system monitor profile on the X11 and Windows platforms. We should also add code to do this on Mac OSX.

I will attach a patch that I cooked up after looking at Oyranos (http://www.oyranos.org/) and consulting the ColorSync documentation offered by Apple (http://developer.apple.com/documentation/GraphicsImaging/Reference/ColorSync_Manager/).
Since I don't have access to an OSX system, I cannot even attempt to compile this code. Someone should test it, fix it if necessary and try if it does the right thing.
Comment 1 Sven Neumann 2007-10-19 07:45:57 UTC
Created attachment 97463 [details] [review]
attempt to implement retrieval of the system monitor profile on OSX
Comment 2 Sven Neumann 2007-10-19 07:47:13 UTC
I would like to get this in for 2.4. If we can't test it before 2.4.0, then we should consider adding it for 2.4.1. If it turns out to be more difficult we can still bump it to 2.6.
Comment 3 Marianne Gagnon 2007-10-23 01:18:32 UTC
Hi,

I tested the patch and it currently does not compile because the correct headers are not included :

#if defined GDK_WINDOWING_QUARTZ
#include <Carbon/Carbon.h>
#include <ApplicationServices/ApplicationServices.h>
#include <CoreServices/CoreServices.h>
#endif

I will attach a patch that makes it compile correctly and run without error.

However i am currently unable to test whether it works because i have no yet got around to learning how gimp deals with cms on a stable platform and as such i do not know how to test such feature.
Comment 4 Marianne Gagnon 2007-10-23 01:19:20 UTC
Created attachment 97683 [details] [review]
Added correct includes to patch so it compiles
Comment 5 Sven Neumann 2007-10-23 16:29:04 UTC
Thanks for fixing the patch.

I wonder if it is safe to include ApplicationServices.h and CoreServices.h or if we need to check for their presence. The GDK Quartz backend seems to include ApplicationServices.h, so that should be fine. Will CoreServices.h be available on all Mac OS X systems?
Comment 6 Marianne Gagnon 2007-10-23 16:39:52 UTC
All includes i added are present on all Mac OS X systems (and, further than that, cannot be removed without breaking the system)

what somewhat suprised me is that i did not have to set the header search paths, but i suppose it works because GTD-quartz does set them.

(I checked quickly in the color management section of the preferences but did not see anything different - should have something appeared? Or perhaps i need to reset the preference files before it takes effect?)
Comment 7 Sven Neumann 2007-10-23 19:19:22 UTC
If you have a system monitor profile, you should see it shown in the Color Management display filter. Make sure you have "Color Managed Display" enabled in the Preferences, then open an image and go to View->Display Filters. You should see the Color Management display filter there and if you select it, the dialog should display the names of the profiles it uses.
Comment 8 Sven Neumann 2007-10-23 19:25:33 UTC
To explain this further, the idea here is that you don't configure a monitor profile in GIMP, only enable the "Use system monitor profile" check-box. The display filter should then use the system-wide configured profile.
Comment 9 Marianne Gagnon 2007-10-24 00:15:36 UTC
Ok i see let's try.

In the preferences, "Color Managed Display" and "Use system monitor profile" are both checked. I then go to "View->Display Filters". "Color management" is in "active filters" section and checked. When i click on it, all the profiles in the info read "none". It does not seem to work at all, since when i select my monitor's profile manually i see a visual change, but when using the automatic profile detection i do not.

Further investigation showed that cdisplay_lcms_get_screen returns 'monitor' as '0', because 'GTK_WIDGET_DRAWABLE (managed)' returns false. This may need to be discussed with the gtk-mac developers as i myself do not know GTK (unless you have directives or a patch i can try).

Fortunately, mac users can probably find their profile in directory /Library/ColorSync/Profiles/Displays/ (i have not searched for documentation saying it is supposed to be there, i just found mine there.)
Comment 10 Marianne Gagnon 2007-10-24 00:17:18 UTC
I would also like to mention i only tested the code with the GTK-quartz backend and not with the X11.app version, that may act differently since this port is more stable.
Comment 11 Sven Neumann 2007-10-24 15:18:59 UTC
0 is the correct value for a single-monitor configuration. GTK_WIDGET_DRAWABLE (managed) should have be TRUE though. The question is whether the monitor number is the correct value to pass to CMGetProfileByAVID(). If it is, and CMGetProfileByAVID() doesn't receive a profile handle, then it looks as if your system doesn't have a system-wide monitor profile set.
Comment 12 Marianne Gagnon 2007-10-24 23:40:07 UTC
I have a monitor profile set in the system preferences (it's set by default) unless ColorSync expects soemthing else i can't think about.

It may be needed to deal this issue with a more knowledgeable mac programmer - but there's still one thing i can confirm, is that 'CMGetProfileByAVID' returns '0' wich means 'no error'.

(On a side note, for some reason when i tried it again today 'GTK_IS_WIDGET (managed)' did not return false anymore... i'm a bit puzzled as to why the same code may run differently today than yesterday but i guess these issues can be sorted out when the mac port of GTK becomes stable if they are still there.)
Comment 13 Marianne Gagnon 2007-10-29 18:19:28 UTC
For the interest that anyone that may want to look into this, i suggest looking at:
http://developer.apple.com/documentation/mac/ACIReference/ACIReference-162.html

from my experiments, it would seem like the 'lcms_cdisplay_lcms_flatten_profile' function will not be called once, but several times and must act a bit like a stream. The first time the function is called, only 128 bytes are given, and it seems like write is called 3 times before the data is completely sent in. I will try to make it work.
Comment 14 Marianne Gagnon 2007-10-29 19:06:24 UTC
Created attachment 98129 [details] [review]
Patch to get CMS auto-detect profile on OS X (works on my computer)

Ok i got it working! Patch attached. (It's probably not on par with your coding standards, however. also i'm not very used to working in C so i may have done something wrong. but at least the base is there to build upon)
Comment 15 Sven Neumann 2007-10-29 19:29:31 UTC
Very nice, thank you. This will need some changes to conform to our coding standards but I hope that Mitch can do this and commit the patch in time for 2.4.1.
Comment 16 Marianne Gagnon 2007-10-30 00:27:19 UTC
Created attachment 98148 [details] [review]
Same as previous one but with a memory leak fixed

Hi, i re-read the code i had written and found a memory leak in it - fixed.
Comment 17 Sven Neumann 2007-10-30 07:06:18 UTC
Created attachment 98157 [details] [review]
cleaned up patch

This is a cleaned up version of your patch. I am afraid though that it needs further work. The use of static variables to track the state of the flatten process is unacceptable. I will have a look at this again later.
Comment 18 Sven Neumann 2007-10-30 11:58:04 UTC
Created attachment 98172 [details] [review]
cleaned up patch that avoids the use of global variables

Please test if this patch compiles and works for you.
Comment 19 Michael Natterer 2007-10-30 13:06:03 UTC
Created attachment 98175 [details] [review]
Patch that compiles
Comment 20 Michael Natterer 2007-10-30 13:55:45 UTC
It seems to load the profile from the system correctly. At least i can
see the name of my configured profile in the filter config UI and the
display clearly changes when i enable/disable the filter. Please commit
that latest patch.
Comment 21 Michael Natterer 2007-10-30 15:16:26 UTC
Fixed in SVN:

2007-10-30  Michael Natterer  <mitch@gimp.org>

	* modules/cdisplay_lcms.c: applied patch from Sven Neumann and
	Marianne Gagnon which adds support for getting the system monitor
	profile on OS/X. Fixes bug #488170.