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 78265 - Add support for ICC color profiles
Add support for ICC color profiles
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: General
git master
Other All
: Urgent enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
: 167541 314874 343667 (view as bug list)
Depends on:
Blocks: 123598 319580 320447
 
 
Reported: 2002-04-10 09:34 UTC by Raphaël Quinet
Modified: 2010-09-06 11:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed icon for color management page in preferences dialog (3.21 KB, image/png)
2004-08-12 22:39 UTC, Alastair M. Robinson
  Details
Proof-of-concept - expands existing proof display module. (13.29 KB, patch)
2004-08-12 23:50 UTC, Alastair M. Robinson
needs-work Details | Review
Suggested patch to avoid prompting for null conversions (1.03 KB, patch)
2007-06-25 13:08 UTC, Tor Lillqvist
rejected Details | Review
Patch for reading the primary monitor profile on Windows (957 bytes, patch)
2007-08-12 03:27 UTC, Yoshinori Yamakawa
committed Details | Review
Add libgdi32 to cdisplay_lcms_libadd (for Win32) (544 bytes, patch)
2007-08-12 14:20 UTC, Yoshinori Yamakawa
none Details | Review

Description Raphaël Quinet 2002-04-10 09:34:27 UTC
This has been requested several times, but this improvement proposal was
not entered in Bugzilla yet: the GIMP should support ICC profiles and do
at least some limited color management so that the images are displayed
correctly on the screen.

As suggested by Piotr Legiecki <piotrlg@sci.pam.szczecin.pl> on the GIMP
developers' list, we could have a look at the "little CMS" library (LGPL):
http://www.littlecms.com/ or http://sourceforge.net/projects/lcms/.

This is probably a feature for 2.0, since it would make sense to introduce
it together with CMYK support.  But the CMYK part is not necessary, so it
could also be done in 1.4 if someone can contribute the code...
Comment 1 Alan Horkan 2003-07-23 18:41:45 UTC
Changes at the request of Dave Neary on the developer mailing list.  
I am changing many of the bugzilla reports that have not specified a target
milestone to Future milestone.  Hope that is acceptable.  
Comment 2 Sven Neumann 2003-07-24 08:48:02 UTC
This could be implemented as a pluggable display module. The framework
is in place for 2.0.
Comment 3 Dave Neary 2003-07-24 09:22:14 UTC
Moving to milestone 1.3.x to see if anyone is willing to do this
before the feature freeze.

Dave.
Comment 4 Dave Neary 2003-07-26 10:33:32 UTC
Bumping a bunch of enhancement requests which will not be done by the feature
freeze to Future. 

Dave.
Comment 5 Ed Halley 2003-10-13 14:15:36 UTC
(Adding a few google-friendly keywords in the hopes of revitalizing 
this bug.)

Adobe RGB 1998 (AdobeRGB) is a prominent example of an ICC color 
profile.  High-end digital cameras such as the popular Canon 10D 
often support AdobeRGB and sRGB colorspace handling for professional 
work.  Would like to see more support for assigning color profiles to 
a given image, and for viewing and printing the image accordingly.
Comment 6 Sven Neumann 2004-01-29 12:03:45 UTC
There's a display filter now that allows to proof an image based on
ICC color profiles. There's more to come in this area...
Comment 7 Hal V Engel 2004-05-03 15:35:33 UTC
I currently use PhotoShop and as a photographer color management is a must have
particularly for printing.  I build and use custom profiles for my printers. 
After playing with GIMP I have concluded that with full color management added
to the current release (2.01) I could give up PhotoShop and move to GIMP.  

I just wanted to add this comment to encourage those doing development to add
this ASAP.  There are many photographers, both pros and serious amateurs, that
will not consider using any photo editor that does not fully support color
management.
Comment 8 Helge Hielscher 2004-05-03 16:09:58 UTC
You might want to try out Cinepaint which supports littlecms.
http://cinepaint.sourceforge.net/
See also
http://www.tu-chemnitz.de/linux/tag/2004/vortraege/slides/behrmann/vortrag_farbmanagement_chemnitzer_linuxtag_2004.html
(in German).
Comment 9 weskaggs 2004-05-20 20:18:25 UTC
As a step toward this, I have set up a mechanism that permits adding a color
profile to a Gimp image so that it is saved when using the tiff format.  If you
have the right .icc file, you can use the MetaData plug-in
(http://registry.gimp.org/plugin?id=4013), and in the File menu select "Load ICC
Profile".  I have tested that a profile loaded this way shows up correctly in
Photoshop 6.0.

Comment 10 Alastair M. Robinson 2004-08-12 22:39:09 UTC
Created attachment 30492 [details]
Proposed icon for color management page in preferences dialog

I'm starting to explore in more detail the best way of implementing color
management - both from a technical and user interface perspective.

Just to get the ball rolling, here's an icon, based on the ICC 1934 xy
chromaticity diagram that typifies color-management.

I'm currently working on a prefs page for color management, and would like to
propose the following options:

Standard Options:  (a group)
  Enable color management  (checkbox)
  Monitor Profile  (GimpFileEntry)

Advanced Options:
  RenderingIntent (ComboBox)
  Working Profile (GimpFileEntry)
  Proof Profile (GimpFileEntry)

That should cover the display side of things, and gives new users a clear hint
as to what they do and don't need to play with.

I'm currently leaning towards the idea of making this a sub-section to the
existing display section within the preferences, but I'm not yet very familiar
with the GIMP codebase, so I apologise if these seem like silly questions:

How easy or difficult will it be, assuming these options are tacked on the end
of _GimpDisplayConfig, to get at them from within a display filter?

What's the best way of applying the display filter globally?  Do we simply
automatically enable the proof/color-management filter when an image is opened,
or should we attempt to share a filter between images (so we only have one set
of profiles and one transform in memory)?

Finally, what do we do about the color selector?
Comment 11 Alastair M. Robinson 2004-08-12 22:42:01 UTC
Sorry, slip of the keyboard there - the icon is, of course, based on the *CIE*
1934 xy Chromaticity diagram, not ICC!
Comment 12 Sven Neumann 2004-08-12 22:56:58 UTC
There is currently no way a plug-in or a display filter could access these
values if they become part of gimprc. If we would make them parasites, that
should work out of the box w/o any changes to the core. Since parasites can be
accessed from plug-ins, the configuration should also be done in a plug-in.
That's what I was expecting to happen outside the core GIMP development based on
the GIMP 2.0 API already. Of course we could decide to do it differently now
that we have the chance to do changes to the core.

Let's please discuss this further on the mailing list.
Comment 13 Alastair M. Robinson 2004-08-12 23:50:46 UTC
Created attachment 30494 [details] [review]
Proof-of-concept - expands existing proof display module.

This proof-of-concept patch expands the existing proof display module to
support regular source->monitor transformation, as well as expanding the
soft-proof ability.

The interface now has three GimpFileSelectors:
Image Profile - this will ultimately be replaced by the Working profile; for
now, it should either be left blank (in which case sRGB will be used), or set
to the profile of, for example, the scanner used to acquire the image.

Monitor Profile - speaks for itself; this is the most important setting, and
the only one that casual users will need to touch.

Proof Profile - if this profile is supplied, soft-proofing will be enabled; the
result of transforming image data from Image Profile -> Proof Profile will be
rendered as accurately as possible on Monitor Profile.
Comment 14 Sven Neumann 2004-08-13 23:27:28 UTC
This doesn't really belong to the proof filter but should be a new filter
module, doesn't it?
Comment 15 Raphaël Quinet 2004-08-15 16:05:00 UTC
Note: this is not directly related, but I am currently working on improved
support for EXIF and XMP metadata, which would allow the core and all plug-ins
to check easily the color space used by a file (e.g. sRGB or Adobe RGB, which
are quite different).  I will post some details in a couple of days on the
gimp-developer list, when my code gets stable enough to be shared with others.
Together with the appropriate display filters, this would provide a better
rendering of the image formats that support EXIF or XMP metadata (TIFF, JPEG,
GIF, PNG, ...).
Comment 16 Dave Neary 2005-01-24 08:02:20 UTC
Comment on attachment 30494 [details] [review]
Proof-of-concept - expands existing proof display module.


From the conversation, and the mailing list, there is some work needed on the
proof system to allow it to get at ICC profiles attached to the image. An
interface for profiling which goes through the preferences has also been
discussed on the list.
Comment 17 Sven Neumann 2005-02-06 23:53:48 UTC
I am working on this now. Detailed plan has been sent to the gimp-developer list.
Comment 18 Sven Neumann 2005-02-07 00:03:15 UTC
2005-02-07  Sven Neumann  <sven@gimp.org>

	* themes/Default/images/preferences/Makefile.am
	* themes/Default/images/preferences/color-management.png: added
	icon for the yet to be added color management preferences page.
	Icon kindly provided by Alastair M. Robinson (bug #78265).
Comment 19 Sven Neumann 2005-02-16 00:58:51 UTC
*** Bug 167541 has been marked as a duplicate of this bug. ***
Comment 20 Leon Brooks 2005-02-16 02:07:58 UTC
Thank you, Sven and Alastair. This is now looking hopeful. (-: 
Comment 21 Sven Neumann 2005-02-21 00:46:38 UTC
Here's the next step, more should follow soon. This incorporates the changes
suggested in comment #13.

2005-02-21  Sven Neumann  <sven@gimp.org>

	Another step towards color management:

	* modules/Makefile.am
	* modules/cdisplay_lcms.c: added new color display module that
	implements color management for the image displays. Still work
	in progress...

	* modules/cdisplay_proof.c: no need to include <string.h> here.

	* libgimpconfig/gimpcolorconfig.[ch]: added new property
	"display-module" to configure the display color management module.

	* app/display/gimpdisplayshell-filter.[ch]
	* app/display/gimpdisplayshell.c: create the configured color
	management display filter for each display.
Comment 22 Albert Cahalan 2005-02-25 21:35:14 UTC
Please remember to make the common case easy:

a. the screen is in sRGB
b. all unmarked RGB files are in sRGB
c. the printer drivers expect sRGB

Despite all that:

d. layer blending requires linear data
e. non-pencil paint tools require linear data
f. most plug-ins require linear data

Normal users should get accurate color handling without having
to make any obscure settings.
Comment 23 Hal V Engel 2005-08-04 23:42:25 UTC
I just installed 2.3.2 to test out the CM support.  Here is what I found:

1. The display appears to be double profiled.
  
    In File==>Preferences==>Color Management when I have Color managed display
or Print simulation selected for Mode of Operation changing either RGB profile
or Monitor profile a displayed image will undergo a color space tranformation
(change colors).  This should only happen for changes to the monitor profile. 
So it appears that both profiles are being used by the image display system.  To
prevent double profiling I had to set both to the same profile since if LCMS is
passed the same two profiles and asked to do a transform it will be not do
anything.  This works. 

    I am not sure what the RGB profile is for.  If it for the display then it is
redondant and should be removed.  If it is for the printer then it should not
affect the display unless it is set to print simulation (but then what is the
Print simulation profile setting for?)  Or is this intended to be what is called
a working space profile in Photoshop?

2.  In Image==>Image Properties the Color space property should include the
embedded profile of the image if one exists.

3. I was not able to find any menu item that would allow me to embed profiles
into images or do other color space work such as transformations between color
spaces.

4. It does not appear that there is any CM support in the printing system yet.  

At this point it appears that what is there is only related to color managing
the display.  If that is the case then of the existing code the only significant
problem I found was that the display is double profiled.  So this is a step
forward and with a little clean up the display CM stuff will be good to go.
Comment 24 Sven Neumann 2005-08-05 13:41:47 UTC
Noone has ever claimed that the color management would be doing anything
reasonable yet. It is far from being finished.

The RGB profile is indeed the working space profile and since the infrastructure
to use the profile from the image is not yet in place, the RGB profile is
currently being used. This is clearly marked as FIXME in the code. All other
aspects you raise are well-known and will be addressed before 2.4.
Comment 25 Sven Neumann 2005-08-31 14:12:38 UTC
*** Bug 314874 has been marked as a duplicate of this bug. ***
Comment 26 weskaggs 2006-05-20 22:24:36 UTC
Raising priority to Urgent since it is crucial to stabilize this for 2.4.
Comment 27 Sven Neumann 2006-06-06 07:20:55 UTC
*** Bug 343667 has been marked as a duplicate of this bug. ***
Comment 28 John Marshall 2006-11-28 20:54:26 UTC
The two options that I think would be most useful to add are:

1. An assign ICC profile dialog, possibly with a convert to working colour space check box where the file has no embedded profile.  This is particularly relevant with many digital cameras that can record images in the adobe RGB colour space but don't embed the profile. 

2. Drop profile - effectively do not colour manage.

Apologies in advance if these are current work in progress.
Comment 29 Tor Lillqvist 2007-06-25 13:08:19 UTC
Created attachment 90611 [details] [review]
Suggested patch to avoid prompting for null conversions

This patch prevents the lcms plug-in from prompting for pointless conversions from sRGB to sRGB by comparing the description of the source profile to well-known descriptions of the sRGB profile. Yeah, it would be better if there was a way to algorithmically check that two profiles are the same, but until we have code to do that, this works fine in practise at least for me;)
Comment 30 Alastair M. Robinson 2007-06-27 00:38:05 UTC
I asked Marti Maria some time ago on the LCMS mailing list how best to compare two profiles for equality.

His suggestion was to perform a binary comparison of everything in the profile past the ICC header.  What I do in my own PhotoPrint app is this (pseudocode):

<load ICC profile temporarily into memory>
md5=new MD5Digest(data+sizeof(icHeader),filelen-sizeof(icHeader));
<free temporary memory>

Thereafter, I compare the MD5 hashes to check for equality.

Comment 31 Sven Neumann 2007-06-27 06:36:21 UTC
What's the benefit of comparing the MD5 sums? You could as well compare the data directly. I expect that to be more efficient.

Anyway, does comparing the data work? Would it solve the problem that Tor's patch is addressing?
Comment 32 Alastair M. Robinson 2007-06-27 16:09:34 UTC
There are several reasons why I opted for MD5 sums - which may or may not be relevant for The GIMP.

1. ICC profiles are potentially large files - especially CMYK profiles, which can easily exceed a megabyte.

2. LCMS Profile handles are opaque - thus to do binary comparisons on a profile it's necessary to load it into memory, even if it's already "open" for LCMS.  Since some profiles might have a long lifetime in my app, I preferred to keep a small MD5 sum as part of my app's profile class than a second binary copy of the whole thing.

3. Transform caching.  Because I carry an MD5 sum with each profile, when I create a transform it's trivial to tag it with the MD5 sums of both source and destination profiles, and the intent used to create the transform.  Again, more practical than tagging the transform with a complete binary copy of both profiles!

Without those specific needs, a direct binary comparison is, as you say, probably the better bet.

Anyway, getting back on topic, yes, as I understand it, Tor's patch is to address the case where a source image has an embedded profile which matches the current working profile, and avoid creating a NULL transform.

Here's the brief exchange I had with Marti Maria on the subject some time ago:
http://sourceforge.net/mailarchive/forum.php?thread_name=410CD01E.2050209%40fakenhamweb.co.uk&forum_name=lcms-user

The pertinent part:

>Is there a recommended method of comparing ICC profiles for equality?

There is no exact way to do that, because same colorspace can be
encoded in different profiles otherwise numerically equivalent. In
practice, it is enough to check whatever both files are equal
DISCOUNTING THE HEADER. This is important because some
flags may be changed when embedding. V4 ProfileID is computed
taking the MD5 of whole profile after header, and this would be
enough in most cases.
Comment 33 Sven Neumann 2007-06-27 16:35:38 UTC
Looks like we should also do such a binary comparison then. Very soon we will also need to address caching and MD5 sums might be useful then. But caching is going to be important for the display correction filter only. The lcms plug-in is short-lived and we have the profile data already loaded anyway. So MD5 sums probably don't make sense here.
Comment 34 Sven Neumann 2007-07-09 21:23:30 UTC
Turns out to be quite difficult to do what Alastair suggests. The LCMS profile handle does indeed seem to be opaque. So how are we supposed to compare the embedded profile against the profile created using cmsCreate_sRGBProfile(). That is the most common case. It would be nice if lcms had a routine to compare two profiles. Can this be cooked up somehow using the lcms API?
Comment 35 Sven Neumann 2007-07-11 20:15:10 UTC
I have opened bug #456017 for the issue brought up by Tor. Please continue this discussion there.
Comment 36 Tony Mah 2007-08-05 02:23:12 UTC
Hi,

I have been trying to get files in Adobe Color space to work properly with Gimp (586 2.3.19) but am not totally successful.
If I open an adobe jpg in gimp, it will ask if I want to convert to srgb. If I say yes it will convert properly :-)

If I go to file>preferences and set the color management to  adobeRGB and open an adobe jpg, the colors will be washed out and the file will not convert properly.

It seems that Gimp just ignores the conversion. I have tried different color spaces and could not get the conversion to work properly. It seems like only adobe to srgb will work. 

Tony



Comment 37 Sven Neumann 2007-08-05 19:01:42 UTC
Tony, the color mamagement in GIMP is not yet fully implemented. The issues you are pointing out are well known. That's why this report is still open.
Comment 38 Yoshinori Yamakawa 2007-08-12 03:27:24 UTC
Created attachment 93523 [details] [review]
Patch for reading the primary monitor profile on Windows

Many of Windows users don't hope to specify the monitor profile in each application. 
I suggest the patch to obtain the color profile from a desktop of the primary monitor.
Comment 39 Sven Neumann 2007-08-12 09:41:17 UTC
This code has already been committed to SVN:

2007-08-12  Sven Neumann  <sven@gimp.org>

        * modules/cdisplay_lcms.c (cdisplay_lcms_get_display_profile):
        applied patch from Yoshinori Yamakawa that adds code to get the
        monitor profile from Win32.

        * app/dialogs/preferences-dialog.c: enabled the toggle for
        "display-profile-from-gdk" on all systems.
Comment 40 Michael Schumacher 2007-08-12 10:07:07 UTC
This patch breaks the MinGw build.
Comment 41 Yoshinori Yamakawa 2007-08-12 13:09:06 UTC
(In reply to comment #40)
> This patch breaks the MinGw build.
> 

GetICMProfile () needs the gdi32.dll.
Has gdi32 library been linked?
Comment 42 Yoshinori Yamakawa 2007-08-12 14:20:19 UTC
Created attachment 93541 [details] [review]
Add libgdi32 to cdisplay_lcms_libadd (for Win32)

I tried to build on Windows XP with MinGW.
It seems to be OK.
Comment 43 Yoshinori Yamakawa 2007-08-12 14:29:10 UTC
(In reply to comment #42)
> Add libgdi32 to cdisplay_libadd (for Win32)

Sorry,
That is not "cdisplay_libadd" but "cdisplay_lcms_libadd".
Comment 44 Michael Schumacher 2007-08-12 21:25:24 UTC
In file included from cdisplay_lcms.c:26:
c:/usr/src/include/lcms.h:188:1: warning: "MAX_PATH" redefined
In file included from c:/usr/src/include/lcms.h:72,
                 from cdisplay_lcms.c:26:
c:/Programme/MinGW/bin/../lib/gcc/mingw32/3.4.2/../../../../include/stdlib.h:46:1: warning: this is the location of the previous definition
In file included from c:/Programme/MinGW/bin/../lib/gcc/mingw32/3.4.2/../../../../include/windows.h:48,
                 from cdisplay_lcms.c:30:
c:/Programme/MinGW/bin/../lib/gcc/mingw32/3.4.2/../../../../include/windef.h:222: error: conflicting types for 'DWORD'
c:/usr/src/include/lcms.h:162: error: previous declaration of 'DWORD' was here
c:/Programme/MinGW/bin/../lib/gcc/mingw32/3.4.2/../../../../include/windef.h:241: error: conflicting types for 'LPDWORD'
c:/usr/src/include/lcms.h:162: error: previous declaration of 'LPDWORD' was here
cdisplay_lcms.c: In function `cdisplay_lcms_get_display_profile':
cdisplay_lcms.c:460: warning: passing arg 2 of `GetICMProfileA' makes integer from pointer without a cast
cdisplay_lcms.c:463: warning: passing arg 2 of `GetICMProfileA' makes integer from pointer without a cast
make: *** [cdisplay_lcms.lo] Error 1
Comment 45 Yoshinori Yamakawa 2007-08-13 02:17:59 UTC
(In reply to comment #44)
- Try to move "#include <windows.h>" to above "#include <lcms.h>".

- If you are using older version of lcms, please update it (1.15 or later is recommended).
Comment 46 Yoshinori Yamakawa 2007-08-13 03:04:01 UTC
(In reply to comment #44)

- Try to define "LCMS_WIN_TYPES_ALREADY_DEFINED".
Comment 47 Sven Neumann 2007-08-13 06:52:22 UTC
Yoshinori, next time, can you pretty please open a new bug report for such an enhancement?

I have committed changes to the include order and the linker options to SVN. Please test:

2007-08-13  Sven Neumann  <sven@gimp.org>

	* modules/Makefile.am (libcdisplay_lcms_la_LIBADD): link with
	gdi32 on PLATFORM_WIN32.

	* modules/cdisplay_lcms.c: changed include order to fix the build
	on MingW (see bug #78265).
Comment 48 Michael Schumacher 2007-08-13 08:09:53 UTC
I'm using 1.16, and have tried all this without success already.
I do think that the problem might be within lcms, though - #define NON_WINDOWS 1 sounds strange on a Windows system. Maybe their build system doesn't detect MinGw correctly.
Comment 49 Alastair M. Robinson 2007-08-14 01:25:02 UTC
(In reply to comment #48)
> I'm using 1.16, and have tried all this without success already.
> I do think that the problem might be within lcms, though - #define NON_WINDOWS
> 1 sounds strange on a Windows system. Maybe their build system doesn't detect
> MinGw correctly.

Just a thought - make sure that PACKAGE_NAME is defined before including lcms.h.  I seem to remember runninig into this or a similar issue once before with mingw32.  The LCMS header seems to define certain things differently if that variable is defined - perhaps in an attempt to identify an autotoolized build chain.
Comment 50 Yoshinori Yamakawa 2007-08-14 01:47:18 UTC
(In reply to comment #48)
> I'm using 1.16, and have tried all this without success already.
> I do think that the problem might be within lcms, though - #define NON_WINDOWS
> 1 sounds strange on a Windows system. Maybe their build system doesn't detect
> MinGw correctly.

I think NON_WINDOWS may be removed when using VC++ or BC++.
It is works correctly with MinGW.

The following code seems to be good:

 ...
 #include <glib.h>  /* lcms.h uses the "inline" keyword */

 #ifdef G_OS_WIN32
 #include <windows.h>
 #endif

 #define LCMS_WIN_TYPES_ALREADY_DEFINED

 #ifdef HAVE_LCMS_LCMS_H
 #include <lcms/lcms.h>
 #else
 #include <lcms.h>
 #endif

 #include <gtk/gtk.h>
 ...

But it cannot be built because the definitions of "LCMSHANDLE" and "cdecl" are missing.
(In lcms.h of lcms 1.17, "LCMSHANDLE" is defined correctly)
Comment 51 Michael Schumacher 2007-08-14 08:55:17 UTC
Yep, I moved the definition of cdecl around in lcms.h, and the build does work again. I guess it's a problem of littlecms. Let's go on, and if anything pops up again, we'll open a new bug for it.
Comment 52 Sven Neumann 2007-08-14 22:05:04 UTC
With the latest changes in SVN, I consider the ICC support ready for 2.4. I will close this bug now. New bug reports should be opened for issues with the implementation.