GNOME Bugzilla – Bug 751553
Linear precision doesn't display the image correctly
Last modified: 2015-10-21 18:28:20 UTC
Created attachment 306178 [details] Screenshot showing what is displayed at 32f linear vs gamma precision Linear precision displays the image as if it had been given an approximately gamma=0.45 "correction". This is for GIMP (and babl and GEGL) updated June 26, 2015. The behavior is probably connected to the new ICC profile conversion code added beginning roughly the middle of May. I thought the problem had been corrected recently, but apparently not. The screenshot is of a 32-bit floating point tiff exported by GIMP. Then GIMP was closed and reopened, and the tiff imported and converted to the GIMP built-in sRGB profile. The image is displayed correctly at 32f gamma precision, but not after converting to 32f linear precision. The same issue affect 16-bit integer precisions, and so probably all precisions, and also affects images converted to a regular sRGB profile from disk.
I observed it as well and in my tests it displays correctly if you select 'None' as 'RGB profile' in Edit->Preferences Color Management In practice selecting a profile (also RGB built-in) is incompatible with the linear image precision. Probably the built-in RGB profile should adjust to the gamma/linear requested (behave as if None was selected), whereas a disk profile should only offer different type precisions (u8, u16, float etc).
Created attachment 306304 [details] [review] quick hack Sorry I did not understand correctly the problem reported here. Here the problem is that two built-in sRGB color profiles created in two different processes (gimp-2.9 and lcms) do not compare equal despite having the same characteristics. Probably they differ in the creation date. This means that here: https://git.gnome.org/browse/gimp/tree/app/core/gimpimage-profile.c#n346 gimp-2.9 tags the image with a color profile, apparently different from the built-in and so successively behave as if the image has a color profile read from disk. Hard-coding in libgimpcolor/gimpcolorprofile.c the sRGB color profile as dumped in a run fixes the issue.
OMG that looks horrible :) is there really no better way to do that? But I actually had a longer comment for this bug in a tab and got distracted... Will re-write it, or find it again ;)
Found the tab :) This comment was written assuming the profile compare bug didn't exist. So once we fixed the compare we will hit this one... Good that you filed this bug, because I missed you on IRC when I wanted to discuss this. It's the last remaining "big piece" I don't know how to fix. Of course the problem is that a profile can't work for both linear and gamma corrected pixels. If the image is not tagged with any profile, and there is no RGB profile configured in prefs, GIMP makes the right choice and uses either gimp_color_profile_new_srgb() or gimp_color_profile_new_linear_rgb(). For any other profile, when we convert the image between gamma and integer, we have these options: - create a new profile that matches the old one "plus/minus gamma" (is that possible at all?) - create an additional profile that does just that gamma conversion and use it in a chain with the image's profile (is that what cmsCreateMultiprofileTransform() is for?) - discard the profile, but tell the user (bad) Did I miss an option? Is this heading in the right direction at all?
About the profile comparison, this thread: http://sourceforge.net/p/lcms/mailman/message/6461782/ says that profiles should be compared by comparing byte-by-byte *except* the header. gimp_color_profile_is_equal() seems to compare *only* the header.
(In reply to Michael Natterer from comment #4) > Found the tab :) This comment was written assuming the profile compare > bug didn't exist. So once we fixed the compare we will hit this one... > > > Good that you filed this bug, because I missed you on IRC when I wanted to > discuss this. It's the last remaining "big piece" I don't know how to fix. Mitch, if you meant me, is there a time tomorrow that I should stop by IRC? > Of course the problem is that a profile can't work for both linear and > gamma corrected pixels. If the image is not tagged with any profile, > and there is no RGB profile configured in prefs, GIMP makes the right > choice and uses either gimp_color_profile_new_srgb() or > gimp_color_profile_new_linear_rgb(). > > For any other profile, when we convert the image between gamma and integer, > we have these options: > > - create a new profile that matches the old one "plus/minus gamma" > (is that possible at all?) Yes, this is often, maybe even usually possible, but not always possible. > - create an additional profile that does just that gamma conversion > and use it in a chain with the image's profile I think maybe this wouldn't work because you won't always be able to "extract" TRC information (what if it's a LUT profile, or a matrix profile with a very odd set of TRCs that don't even match each other?). You can't assume the profile has the sRGB TRC. > (is that what cmsCreateMultiprofileTransform() is for?) I'll ask Marti. The API doc wasn't very informative. But the phrase "device profile" would seem to indicate some other purpose. > - discard the profile, but tell the user > (bad) Very bad! > > Did I miss an option? Is this heading in the right direction at all? I've been thinking about this problem. Pippin was correct when he noted that there are a lot of color spaces out there that really shouldn't be used for editing. For example, what if the image has a LUT profile, or a matrix profile that's not well-behaved and might even have a non-zero black point? You can't do much with these profiles except ask the user to convert to a different RGB working space profile and hope it's better than the first one. Here are some possibilities that might be worth considering singly or in combination: First, give the user the option to opt out completely from the gamma conversions. This would make it possible for the user to edit in "any old color space" they had in mind, including a well-behaved true linear gamma color space. This could be considered as completely in keeping with the future goal of allowing the user a choice of TRCs, such as the sRGB TRC, BT709, gamma=2.2, the Lab "L" TRC, various log curves used in the VFX industry, and etc. If the user elected this option, a conversion between "linear" and "gamma" would be a null action. Second, if the image's embedded ICC profile is well-behaved (can be tested for using LCMS), one option is to use LCMS to make a new profile with the image color space's XYZ values and the sRGB TRC, and then convert the image to this color space. I think the user should be told what's happening. Third, given the plethora of "not well-behaved and not even a working space" ICC profiles that any given image might be in when opened with GIMP, instead of constructing an ICC profile "on the fly" from the image's embedded ICC profile (which is not always going to be possible), it might be even better to give the user the option to convert the image from their (possibly really inappropriate color space) to a selection of hard-coded internal GIMP RGB working spaces, including at least the following primaries (all paired with the sRGB TRC): sRGB AdobeRGB1998 ProPhotoRGB Rec.2020 ACEScg ACES Fourth, if considering the third option, it would be nice to allow the user the option to "plug in" their own XYZ values. This would only be useful for advanced users, but advanced users would find it invaluable. And then GIMP could convert the image to a profile made with the user-supplied primaries and the sRGB TRC. In this case, if the user enters stupid values, that's the user's responsibility.
(In reply to Michael Natterer from comment #3) > OMG that looks horrible :) is there really no better way to do that? > There you are comparing a generic color profile to the builtin. You can compare the tags that you set in the builtin Description, DeviceMfg, DeviceModel and Copyright. They'll hardly match a color profile different from the builtin.
Massimo: Seen my comment 5? Elle: yes I meant you but I'm on a trip currently, will be more responsive on tuesday again.
(In reply to Michael Natterer from comment #8) > Massimo: Seen my comment 5? > yes (In reply to Michael Natterer from comment #5) > About the profile comparison, this thread: > > http://sourceforge.net/p/lcms/mailman/message/6461782/ > > says that profiles should be compared by comparing byte-by-byte > *except* the header. gimp_color_profile_is_equal() seems to compare > *only* the header. If that is the road to follow then you need a double representation: the profile in memory and the functional cmsHPROFILE, or you have to save to memory the cmsHPROFILE every time you compare two of them or keep the in memory and create the cmsHPROFILE everytime you need a cmsTRANSFORM from it. Hoping that it works as intended
Yes, that's involved. All this stuff, and the number of times we convert from cmsHPROFILE to ICC blobs and the other was around, brings me one step closer to just hacking up an opaque wrapper around this shit, so we can cache it at least within one process.
Created attachment 306789 [details] [review] compare byte-by-byte profiles in color_profile_is_equal I don't know if it is better to implement the comparison byte-by-byte in gimp_color_profile_is_equal or keep _is_equal as is and introduce a gimp_color_profile_is_builtin for this special comparison. The attached patch re-implements gimp_color_profile_is_equal and fixes the problem described here. It fixes also a typo in comments P.S: in this file there are memory leaks in gimp_color_profile_open_from_file and gimp_color_profile_get_summary
I really did that change locally (changing GimpColorProfile to be a proper GObject), will test a bit, plug the leaks you mentioned and also add your profile comparison code, stay tuned...
Pushed your patch to master (not really your patch, but in the spirit of your patch...), now we can address the more evil bugs... commit 688861cd34aaf0214306ed570ce096b075e75ce2 Author: Massimo Valentini <mvalentini@src.gnome.org> Date: Fri Jul 10 23:06:53 2015 +0200 Bug 751553: Linear precision doesn't display the image correctly gimp_color_profile_is_equal(): byte-by-byte compare the entire profile *except* the header, instead of only the header, which was wrong. libgimpcolor/gimpcolorprofile.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-)
What is missing here is a function that takes a GimpColorProfile, adds/removes gamma, and returns a new GimpColorProfile, or returns NULL if that is impossible.
(In reply to Michael Natterer from comment #14) > What is missing here is a function that takes a GimpColorProfile, > adds/removes gamma, and returns a new GimpColorProfile, or returns NULL > if that is impossible. A couple of relevent recent posts to the LCMS mailing list (the links are to specific posts, but the threads as a whole are worth reading): Re: [Lcms-user] Obtaining a linear variety of an arbitrary icc profile. http://sourceforge.net/p/lcms/mailman/message/34474383/ Re: [Lcms-user] Fwd: Trying to make a UI for colorspaces, have some questions. http://sourceforge.net/p/lcms/mailman/message/34492122/ Making a linear gamma (or other TRC) RGB working space profile from a matrix profile is easy, assuming the original matrix profile is a well-behaved working space: What Makes a Color Space Well Behaved? http://ninedegreesbelow.com/photography/well-behaved-profile.html An RGB working space profile is 1. neutral up and down the gray axis (R=G=B corresponds to LAB a=0,b=0) 2. reaching LAB=0,0,0 at RGB (0,0,0) and LAB=100,0,0 at RGB (1.0,1.0,1.0). The problem is: 1. Many RGB matrix "working space profiles", including ones floating around free software, aren't actually completely well-behaved: Are Your Working Space Profiles Well Behaved? http://ninedegreesbelow.com/photography/are-your-working-space-profiles-well-behaved.html Survey of Free and Open Source ICC RGB Working Space Profiles http://ninedegreesbelow.com/photography/linux-icc-profiles.html 3. It's entirely possible to make shaper matrix RGB profiles (for example using ArgyllCMS to profile a camera or a monitor), where the TRC is different in each channel, hence completely NOT well-behaved (this would be an RGB device profile). 4. The RGB profile might not be a matrix profile. * It might be a LookUp Table device profile such as you make using ArgyllCMS for a printer, or as a specialized camera input or monitor profile. * It might be one of the new color.org LUT profiles that have confusing names that suggest they are actually the same as old RGB matrix profiles (there is such an sRGB profile, and people do download it and try to use it as a working space profile): http://www.color.org/profiles2.xalter The more I think about it, the more I think it might be better to check to see if the image's embedded ICC profile is "close enough" to well-behaved, and if so, make a profile, but if not, invite the user to choose another profile from disk or else convert to one of a set of hard-coded RGB working space profiles (suggested list given in comment 6 above). And offer the same invitation even the embedded profile really is "close enough" to well-behaved. Otherwise, maybe Noel Carboni's suggestion on the LCMS mailing list is worth exploring.
These new functions seem to do the job, thanks for providing the first version. commit 6eb9f9d4aab93909bfafcc21f82719b4f366927b Author: Michael Natterer <mitch@gimp.org> Date: Tue Oct 20 20:12:18 2015 +0200 libgimpcolor: add API to create profile variants with linear/sRGB gamma and the original profile's RGB chromacities and whitepoint. libgimpcolor/gimpcolor.def | 2 ++ libgimpcolor/gimpcolorprofile.c | 79 ++++++++++++++++++++++++++++++++++------- libgimpcolor/gimpcolorprofile.h | 5 +++ 3 files changed, 74 insertions(+), 12 deletions(-)
Fixed in master. Still some GUI parts missing, but the bug is fixed and the image should always display correctly and have a profile that matches the layer data. commit ecd47520728f9a51aafaecad102791f575086e2f Author: Michael Natterer <mitch@gimp.org> Date: Tue Oct 20 23:53:47 2015 +0200 Bug 751553 - Linear precision doesn't display the image correctly When converting and image with a color profileimage between linear and gamma, create a new profile using the new API in GimpColorProfile, convert the layers to that profile and tag the image with the new profile. If creating a new profile fails, convert to the right builtin profile (linear rgb or sRGB from GimpColorProfile), but that code should be considered a fallback that will be prevented from happening in the convert dialog (at least the user will be informed). app/core/gimpimage-convert-precision.c | 51 ++++++++++++++++++++++++++---- app/core/gimplayer.c | 57 +++++++++++++++++++++++++++++++--- 2 files changed, 98 insertions(+), 10 deletions(-)
That was premature... commit 61ae6b10de231d28422ff1e3f627340970a1d6b5 Author: Michael Natterer <mitch@gimp.org> Date: Wed Oct 21 00:27:34 2015 +0200 Bug 751553 - Linear precision doesn't display the image correctly Disable the new conversion code, something is broken... app/core/gimpimage-convert-precision.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
(In reply to Michael Natterer from comment #18) > That was premature... > > commit 61ae6b10de231d28422ff1e3f627340970a1d6b5 > Author: Michael Natterer <mitch@gimp.org> > Date: Wed Oct 21 00:27:34 2015 +0200 > > Bug 751553 - Linear precision doesn't display the image correctly > > Disable the new conversion code, something is broken... > > app/core/gimpimage-convert-precision.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) I reverted this commit and played a bit, I spotted two possible problems, one is here: https://git.gnome.org/browse/gimp/tree/libgimpcolor/gimpcolorprofile.c#n695 and the other is here: https://git.gnome.org/browse/gimp/tree/app/core/gimpimage-color-profile.c#n523 using {get,merge,free}_shadow_buffer makes it possible to undo a color profile conversion. HTH
You found two bugs, thanks :) The first one looks like it's the reason here. The second one... well I killed undo when getting rid of that code duplication, will fix...
Fixed in master: commit 4e04e2ff30506160e409e72d625acf76d64685c2 Author: Michael Natterer <mitch@gimp.org> Date: Wed Oct 21 20:27:06 2015 +0200 Revert "Bug 751553 - Linear precision doesn't display the image correctly" This reverts commit 61ae6b10de231d28422ff1e3f627340970a1d6b5. Re-enable color profile conversion on precision conversion, it's fixed now. app/core/gimpimage-convert-precision.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) commit e2f3a05d441d2d0aafce682744cc113a4182ef02 Author: Michael Natterer <mitch@gimp.org> Date: Wed Oct 21 20:25:06 2015 +0200 Bug 751553 - Linear precision doesn't display the image correctly Fix copy/paste bug in gimp_color_profile_get_rgb_matrix_colorants() which returned a broken XYZ triple for the blue component. Spotted by Massimo. libgimpcolor/gimpcolorprofile.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)