GNOME Bugzilla – Bug 478528
Layer and Image previews are not color managed
Last modified: 2016-04-15 15:57:10 UTC
Please describe the problem: When opening an image with color managment the little preview in the layer overview does not use color managment! Steps to reproduce: 1. Open http://www.fc-prints.de/fileadmin/daten/fcPrints_Testbild_100ppi.jpg 2. Choose to KEEP the profile 3. Look at the image in big and the thumbnail in the Layer Overview Actual results: The image in the layer overview does show a non color managed image which is different to the one in big. Expected results: Both the Image in normal size and in the layers dialog have the same colors. Does this happen every time? Yes Other information:
I am very sorry, it is much more better visible here (and I intended to post this first): http://www.fc-prints.de/fileadmin/daten/farbkreis.jpg
Yes, and we aren't going to change that, at least not in the near future.
You are supposed to convert to sRGB. Working in other color spaces is strongly discouraged until all parts of GIMP have been made aware of color management. This is not likely going to happen anytime soon. See also bug #467930.
Assuming the layer overview preview image uses the same preview widget that the rest of gimp users, this is probably a bug in the preview widget (same as bug 556608).
*** Bug 727038 has been marked as a duplicate of this bug. ***
Fixed in master. Does not fix previews in plug-ins, which is tracked in bug 556608. commit 09fed0a0e5553e1febe41fe358b97860c45eb9d8 Author: Michael Natterer <mitch@gimp.org> Date: Thu Sep 3 02:25:51 2015 +0200 Bug 478528 - Layer and Image previews are not color managed Implement color management in GimpViewRenderer: if the viewable is a GimpColorManaged (true for images and layers), keep around a GimpColorTransform and convert the preview image to display colors. app/widgets/gimpviewrenderer.c | 128 +++++++++++++++++++++++++++++++++++------ app/widgets/gimpviewrenderer.h | 4 ++ 2 files changed, 116 insertions(+), 16 deletions(-)
(In reply to Michael Natterer from comment #6) > Fixed in master. Does not fix previews in plug-ins, which is tracked > in bug 556608. > > commit 09fed0a0e5553e1febe41fe358b97860c45eb9d8 > Author: Michael Natterer <mitch@gimp.org> > Date: Thu Sep 3 02:25:51 2015 +0200 > > Bug 478528 - Layer and Image previews are not color managed > > Implement color management in GimpViewRenderer: if the viewable is a > GimpColorManaged (true for images and layers), keep around a > GimpColorTransform and convert the preview image to display colors. > > app/widgets/gimpviewrenderer.c | 128 > +++++++++++++++++++++++++++++++++++------ > app/widgets/gimpviewrenderer.h | 4 ++ > 2 files changed, 116 insertions(+), 16 deletions(-) Since this commit layer previews in the preview dialog are skewed. The problem is here: https://git.gnome.org/browse/gimp/tree/libgimpcolor/gimpcolorprofile.c#n963 The babl_format "cairo-RGB24" has babl_model "R'G'B'", that means that its 'else if' branch is never executed and this function return babl and lcms formats incompatible (cairo-RGB24 has a padding byte). A solution could be to move the "cairo-RGB24" 'else if' before the "R'G'B'" 'else if'.
Argh of course. I looked at this code like 20 times to track down the bug, but only tested alpha layers, which have a premultiplied Babl model, and are not affected by the bug... Will fix, thanks!
Fixed in master: commit f9da5cc6ccaa8fa2ad4ec277837312ecba7101cb Author: Michael Natterer <mitch@gimp.org> Date: Sun Sep 6 14:12:58 2015 +0200 Bug 478528 - Layer and Image previews are not color managed Fix gimp_color_profile_get_format() to return the right format for "cairo-RGB24": simply check for the more special cairo formats first, then check for the general RGB models. Found by Massimo. libgimpcolor/gimpcolorprofile.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)
The layer previews for images in linear precision are wrong, here: https://git.gnome.org/browse/gimp/tree/app/widgets/gimpviewrenderer.c#n1265 the configured image profile is used, for example 'GIMP built-in Linear RGB', but the data that is passed to lcms has been converted to "R'G'B'/A u8" because 'gimp_drawable_get_preview_format' always returns a gamma corrected format: https://git.gnome.org/browse/gimp/tree/app/core/gimpdrawable-preview.c#n63 similarly for the projection previews: https://git.gnome.org/browse/gimp/tree/app/core/gimpimage-preview.c#n115 Another source of errors generating previews (as seen with Brasseur's dalay lama jpegs) is GEGL computing the pyramid in the buffer color space, but that's another issue probably already tracked in a GEGL bug report.
Layer previews in the layers dialog do not update correctly after adding an alpha channel and erasing an elliptical selection. steps to reproduce: A new opaque image (R'G'B' u8) <Ctrl>-n <Enter> Layer->Transparency->Add alpha Channel <Alt>-l a h Select Elliptical tool E Select an ellipse and press <Del> the preview is not updated, whereas gimp-2.8 shows the checkerboard in the selected area. BTW lcms from master adds a cmsFLAGS_COPY_ALPHA flag so that a transform copies also the alpha channel probably simplifying GIMP code in few places
Indeed, I forgot about the preview_format() function. About wrong updates when adding/removing alpha, that won't be fixed by cmsFLAGS_COPY_ALPHA because we realy need a new transform here, the pixel format has changed.
I just pushed this for fixing color picking when there is a profile: commit 9550fbff3cdfbd6ee037f880e0ec711be5b49275 Author: Michael Natterer <mitch@gimp.org> Date: Sun Sep 20 21:17:54 2015 +0200 Bug 748749 - picked colors don't match image colors... ...when a color profile is active This commit adds more (still unused) infrastructure to fix this bug: Ee now keep around color transforms from layer pixels to "R'G'B'A double" (which is GimpRGB's format) and back. Also add utility function gimp_image_color_profile_pixel_to_srgb() which converts a picked pixel to GimpRGB, using the cached color transform. app/core/gimpimage-color-profile.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++- app/core/gimpimage-color-profile.h | 9 ++++- app/core/gimpimage-private.h | 5 +++ app/core/gimpimage.c | 5 +-- 4 files changed, 118 insertions(+), 7 deletions(-) Which makes me thing that maybe we should simply convert image pixels to sRGB before handing them out as previews, and in GimpViewRenderer only convert from sRGB to the monitor profile. this would involve two conversions per peview though, and I don't really like that. Any comments?
(In reply to Michael Natterer from comment #12) > Indeed, I forgot about the preview_format() function. > > About wrong updates when adding/removing alpha, that won't be fixed by > cmsFLAGS_COPY_ALPHA because we realy need a new transform here, the > pixel format has changed. (In reply to Michael Natterer from comment #13) > I just pushed this for fixing color picking when there is a profile: > ... > > Which makes me thing that maybe we should simply convert image pixels > to sRGB before handing them out as previews, and in GimpViewRenderer > only convert from sRGB to the monitor profile. this would involve > two conversions per peview though, and I don't really like that. > > Any comments? My comment on the cmsFLAGS_COPY_ALPHA was based on the observation that there are already many conversions per view because of the alpha copy done in GIMP: https://git.gnome.org/browse/gimp/tree/app/gegl/gimp-gegl-loops.c#n753 when the destination buffer is a wrapper around a cairo surface with alpha channel gegl_buffer_copy performs one conversion and gegl_buffer_iter_next with READ_WRITE two. I tried to use cmsFLAGS_COPY_ALPHA, but it appeared to do nothing because the renderer format is not updated promptly
This fixes preview rendering when alpha changes, now you can try the cmsFLAGS_COPY_ALPHA which will save us copies once we can use the new lcms. The same separate alpha copying code is in all places that do an actual lcms transform, which really sucks. commit 35367359136ef51fb6db8120ca0d0341ae6c32b2 Author: Michael Natterer <mitch@gimp.org> Date: Mon Sep 21 20:20:02 2015 +0200 Bug 478528 - Layer and Image previews are not color managed Fix color managed layer previews when adding/removing alpha: Implement GimpDrawable::alpha_changed() in GimpLayer and emit GimpColorManaged::profile_changed() so all cached color transforms are nuked. app/core/gimplayer.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
That was too quick, the commit is 616c57b8cf94283e85fccaa5a7847c2cb251e3b8.
Created attachment 311890 [details] [review] use cmsFLAGS_COPY_ALPHA (In reply to Michael Natterer from comment #15) > This fixes preview rendering when alpha changes, now you can try > the cmsFLAGS_COPY_ALPHA which will save us copies once we can use > the new lcms. The same separate alpha copying code is in all places > that do an actual lcms transform, which really sucks. > Thanks, attached the patch showing that GIMP code is simplified when lcms 2.8 cmsFLAGS_COPY_ALPHA will be available. It seems to work, but benchmarking the projection (File->Debug->Benchmark Projection) it shows slight improvement only for images in half linear mode where gimp_display_shell_profile_convert_buffer executes a gamma conversion to just copy the alpha channel and falls back to a reference babl_fish.
Yeah I don't expect huge improvements, but that separate copying is just too ugly, I look forward to depend on the new lcms :)
*** Bug 760761 has been marked as a duplicate of this bug. ***
This is a real bug now...
Fixed again in master: commit 9e8499bb48c8ceda5864e0b877ffb7a43a22d605 Author: Michael Natterer <mitch@gimp.org> Date: Fri Apr 15 16:52:25 2016 +0100 Bug 478528 - Layer and Image previews are not color managed Change gimp_viewable_get_[new]_preview() to return buffers of the image's and layers' colorspace, but in u8. This way layer and image previews can transform them correctly to the display profile. Note: this makes plug-ins receive thumbnail buffers in that pixel format too. Also change gimp_viewable_get_[new]_pixbuf() to always return sRGB buffers that can reasonably be put to screen directly, or put into DND buffers. This is at least more correct now. app/core/gimpdrawable-preview.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- app/core/gimpdrawable-preview.h | 14 +++++- app/core/gimpdrawable.c | 1 + app/core/gimpimage-preview.c | 83 +++++++++++++++++++++++++++++++- app/core/gimpimage-preview.h | 4 ++ app/core/gimpimage.c | 1 + 6 files changed, 244 insertions(+), 8 deletions(-)