Re: [Gimp-developer] Displaying linear gamma images
Massimo made a patch that was so much better than my poor effort. His patch worked really well. Then Mitch did a whole bunch of code writing and now Gimp from git can properly display linear gamma images. Thank you Massimo! Thank you Mitch! ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
Hi Daniel, Thanks! for the input. I'm pondering your suggestions about the code and I'll check in with you and Mitch on IRC in the next couple of days. -- http://ninedegreesbelow.com Just because it's a standard, doesn't mean it's right. ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
On 9/12/13, Jon Nordby wrote:
> On 12 September 2013 16:08, Elle Stone wrote:
>> The problem at this point is that the image won't display properly
>> until something like doing a very small levels correction forces a
>> screen redraw. After forcing a screen redraw, the image is displayed
>> without any posterization, but with magenta lines (outlining the
>> tiles?). The screen redraw lasts until the level dialog is closed.
>>
>> I think the problem is that I'm not properly merging and updating
>> "buffer" after the hard-coded transform. The corresponding code from
>> the lcms.c file uses layer buffers, which seems not applicable to a
>> projection:
>> gimp_drawable_merge_shadow (layer_id, TRUE);
>> gimp_drawable_update (layer_id, 0, 0, layer_width, layer_height);
>
> I do not know the GimpDisplayShell code well, but try to just read out
> data from the projection GeglBuffer instead of modifying it.
I created a copy of "buffer" and converted it to the monitor profile,
with the same results. I'm pretty sure the problem is what happens to
the buffer after it's been used. It doesn't quietly disappear. In a
small test image the code that does the transform gets executed 11
times per anything that changes the screen. It takes up "pipes" and a
larger image eventually crashes Gimp ("unable to open pipe: Too many
open files").
> And
> instead of the the "gegl_buffer_get (buffer, ... "cairo-ARGB32", ...
> data ...)" that you have marked, do the lcms transform such that the
> 8bit ready-for-display ends up in the "data" buffer.
That code is the pre-existing code that sends the buffer to cairo, so
I haven't tried to modify it. There's probably a way to get "buffer"
to be converted from the image color space at 32f to the monitor
profile at 8i in one fell swoop - I'm pretty sure lcms can do the
conversion in one fell swoop but I don't have the gegl buffers set up
correctly. But "gegl_buffer_get (buffer, ... "cairo-ARGB32", ..." does
convert "buffer" to 8-bits.
> Also, can you please post your changes as a (git formatted) diff?
> It is much easier to read and apply for another contributor trying to
> help you out.
I posted the git patch, the two ICC profiles in the hard-coded
transform, and a test image to
http://ninedegreesbelow.com/temp/convert-buffer-before-cairo.html. I
hope I did the git patch correctly! The git patch (but not the
profiles or test image) is also attached to this email.
> Jon Nordby - www.jonnor.com
Thanks! Jon, for taking an interest in this project.
Elle
--
http://ninedegreesbelow.com
commit 23c4619a16d2b3ef5c05fd086a93a8ae3f28bd85
Author: Elle Stone
Date: Thu Sep 12 17:30:29 2013 -0400
elle - convert before cairo - modified:
app/display/gimpdisplayshell-render.c
elle - write statements added - modified:
libgimpwidgets/gimpcolordisplay.c
elle - write statements added - modified:
libgimpwidgets/gimpcolordisplaystack.c
elle - comment out actual conversion, write statements added -
modified: modules/display-filter-lcms.c
diff --git a/app/display/gimpdisplayshell-render.c
b/app/display/gimpdisplayshell-render.c
index 9c3f6c1..101bba7 100644
--- a/app/display/gimpdisplayshell-render.c
+++ b/app/display/gimpdisplayshell-render.c
@@ -16,6 +16,17 @@
*/
#include "config.h"
+#include /* elle lcms.h uses the "inline" keyword */
+
+#include /* elle */
+/* this might be needed for Windows
+#ifdef G_OS_WIN32
+#define STRICT
+#include
+#define LCMS_WIN_TYPES_ALREADY_DEFINED
+#endif
+*/
+#include /* elle */
#include
#include
@@ -43,7 +54,6 @@
#include "gimpdisplayshell-scroll.h"
#include "gimpdisplayxfer.h"
-
void
gimp_display_shell_render (GimpDisplayShell *shell,
cairo_t *cr,
@@ -68,6 +78,19 @@ gimp_display_shell_render (GimpDisplayShell *shell,
gint stride;
guchar *data;
+ cmsHTRANSFORM image_to_monitor_transform = NULL;
+ cmsHPROFILE image_space;
+ cmsHPROFILE monitor_profile;
+ gintloops;
+ /* gintprojection_alpha; this variable will eventually be
needed. */
+ gintprojection_bpp;
+ GeglBufferIterator *iter;
+ const Babl *iter_format;
+ GeglBuffer *buffer_clone;
+ gintprojection_width;
+ gintprojection_height;
+ /* gintprojection_alpha; this variable will eventually be
needed. */
+
g_return_if_fail (GIMP_IS_DISPLAY_SHELL (shell));
g_return_if_fail (cr != NULL);
g_return_if_fail (w > 0 && h > 0);
@@ -76,8 +99,46 @@ gimp_display_shell_render (GimpDisplayShell *shell,
projection = gimp_image_get_projection (image);
buffer = gimp_pickable_get_buffer (GIMP_PICKABLE (projection));
+ projection_width = gegl_buffer_get_width (buffer);
+ projection_height = gegl_buffer_get_height (buffer);
+ printf("gimpdisplayshell-render.c projection_width, projection_height= %i %i
\n",
+pro
Re: [Gimp-developer] Displaying linear gamma images
You should bug mitch or me IRC for faster responses :). Issues with this code: Don't modify the buffer you got from gimp_pickable_get_buffer (GIMP_PICKABLE (projection)), it's a live part of the projection. Adding the buffer to the iterator twice probably doesn't do what you want it to do. Depending on the format pointers may be to the same buffer, so writing may immediately modify your read buffer, or it may not. Use READWRITE if interacting with the same buffer twice. For best results you should be writing to the cairo image in xfer. The call of gegl_buffer_get wirting to xfer *is* the gamma conversion, you want to replace it completely. Alternativly you could create a temp gegl buffer here and then use buffer_get from that to write to xfer. Finally, you need to detect weather the projection's format is linear or gamma and read in that, rather than always using "R'G'B'A float". Using gimp_drawable_get_linear is probably sufficient for now. On Thu, Sep 12, 2013 at 9:12 AM, Jon Nordby wrote: > On 12 September 2013 16:08, Elle Stone wrote: > > I've made progress getting a linear gamma image to display without > > posterization in the shadows. I put up a temporary web page with the > > modified code and a picture: > > http://ninedegreesbelow.com/temp/convert-buffer-before-cairo.html > > > > The problem at this point is that the image won't display properly > > until something like doing a very small levels correction forces a > > screen redraw. After forcing a screen redraw, the image is displayed > > without any posterization, but with magenta lines (outlining the > > tiles?). The screen redraw lasts until the level dialog is closed. > > > > I think the problem is that I'm not properly merging and updating > > "buffer" after the hard-coded transform. The corresponding code from > > the lcms.c file uses layer buffers, which seems not applicable to a > > projection: > > gimp_drawable_merge_shadow (layer_id, TRUE); > > gimp_drawable_update (layer_id, 0, 0, layer_width, layer_height); > > I do not know the GimpDisplayShell code well, but try to just read out > data from the projection GeglBuffer instead of modifying it. And > instead of the the "gegl_buffer_get (buffer, ... "cairo-ARGB32", ... > data ...)" that you have marked, do the lcms transform such that the > 8bit ready-for-display ends up in the "data" buffer. > > Also, can you please post your changes as a (git formatted) diff? > It is much easier to read and apply for another contributor trying to > help you out. > > -- > Jon Nordby - www.jonnor.com > ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
On 12 September 2013 16:08, Elle Stone wrote: > I've made progress getting a linear gamma image to display without > posterization in the shadows. I put up a temporary web page with the > modified code and a picture: > http://ninedegreesbelow.com/temp/convert-buffer-before-cairo.html > > The problem at this point is that the image won't display properly > until something like doing a very small levels correction forces a > screen redraw. After forcing a screen redraw, the image is displayed > without any posterization, but with magenta lines (outlining the > tiles?). The screen redraw lasts until the level dialog is closed. > > I think the problem is that I'm not properly merging and updating > "buffer" after the hard-coded transform. The corresponding code from > the lcms.c file uses layer buffers, which seems not applicable to a > projection: > gimp_drawable_merge_shadow (layer_id, TRUE); > gimp_drawable_update (layer_id, 0, 0, layer_width, layer_height); I do not know the GimpDisplayShell code well, but try to just read out data from the projection GeglBuffer instead of modifying it. And instead of the the "gegl_buffer_get (buffer, ... "cairo-ARGB32", ... data ...)" that you have marked, do the lcms transform such that the 8bit ready-for-display ends up in the "data" buffer. Also, can you please post your changes as a (git formatted) diff? It is much easier to read and apply for another contributor trying to help you out. -- Jon Nordby - www.jonnor.com ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
I've made progress getting a linear gamma image to display without posterization in the shadows. I put up a temporary web page with the modified code and a picture: http://ninedegreesbelow.com/temp/convert-buffer-before-cairo.html The problem at this point is that the image won't display properly until something like doing a very small levels correction forces a screen redraw. After forcing a screen redraw, the image is displayed without any posterization, but with magenta lines (outlining the tiles?). The screen redraw lasts until the level dialog is closed. I think the problem is that I'm not properly merging and updating "buffer" after the hard-coded transform. The corresponding code from the lcms.c file uses layer buffers, which seems not applicable to a projection: gimp_drawable_merge_shadow (layer_id, TRUE); gimp_drawable_update (layer_id, 0, 0, layer_width, layer_height); If I can get the screen to consistently display properly, then the next step would be to merge the code in "modules/display-filter-lcms.c" with the code in "app/display/gimpdisplayshell-render.c". But I'm somewhat at a stand-still until serendipity or someone with more knowledge about gegl buffers can solve the problem of getting the screen to display all the way all the time correctly rather than only after using levels to force a redraw. I've been searching the Gimp code base looking for that "serendipitous" example code to follow, and came up with "gimp_projection_flush_now (projection);" which seemed to help, but only a little. Elle -- http://ninedegreesbelow.com ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
Hmm, I managed to get past the problem of not linking to lcms2. The solution was to compile Gimp using MAKEOPTS="-j3" CFLAGS=-Wl,--no-as-needed LDFLAGS=-llcms2 ./autogen.sh . . . not that I know why that seems to have worked. On to the next step. -- http://ninedegreesbelow.com - articles on color management & open source photography ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images
Hi Jon, Daniel, all,
On 9/10/13, Daniel Sabo wrote:
> The conversion happens in app/display/gimpdisplayshell-render.c :
> gimp_display_shell_render() .
> Look for the gegl_buffer_get call that converts to babl_format
> ("cairo-ARGB32").
On 9/11/13, Jon Nordby wrote:
>> Quoting Jon's suggestion:
>> (1)call a "GeglBuffer from the projection (or possibly a copy)" and then
>> So how does one "call a GeglBuffer from the projection"?
>
> I said "operate on", not "call". :)
Sorry!
> The projection has a GeglBuffer associated with it, see
> https://git.gnome.org/browse/gimp/tree/app/display/gimpdisplayshell-render.c#n77
> That is what the color conversion needs to use as an input, basically*.
I set up a babl/gegl/gimp prefix for working on this issue and wrote a
function that creates a hard-coded ICC profile transform:
static void image_to_monitor ()
{
cmsHTRANSFORMimage_to_monitor_transform = NULL;
cmsHPROFILE image_space;
cmsHPROFILE monitor_profile;
image_space =
cmsOpenProfileFromFile("/usr/share/color/icc/V4/AllColors-elleV4-g100.icc","r");
monitor_profile =
cmsOpenProfileFromFile("/usr/share/color/icc/monitor-as-qh.icc","r");
image_to_monitor_transform = cmsCreateTransform (image_space, TYPE_RGBA_FLT,
monitor_profile, TYPE_RGBA_8,
INTENT_RELATIVE_COLORIMETRIC,
cmsFLAGS_NOOPTIMIZE |
cmsFLAGS_BLACKPOINTCOMPENSATION );
if (image_to_monitor_transform)
{printf("Image_to_monitor_transform WAS created in
display-filter-lcms.c.\n");}
else {printf("Image_to_monitor_transform was NOT created in
display-filter-lcms.c.\n");}
}
The purpose of this function is (eventually) to be able to convert the
projection from the image profile to the monitor profile without
bringing in all the overhead that allows Gimp to detect which profile
is which. The overhead can be added later. That's my theory, anyway.
To go along with this hard-coded transform, I made a 32-bit floating
point image with an alpha channel. So in my dedicated prefix version
of Gimp, that's the only image that gets opened.
My first goal was to get the "image_to_monitor" function to work, not
to do anything with it, just to get evidence that the transform was
created.
I put the "image_to_monitor" function in "display-filter-lcms.c" and
added this line - "image_to_monitor ();" - to the function
"cdisplay_lcms_convert_surface". Gimp compiles and runs just fine,
printing out lines that the transform was indeed created.
I also added the exact same "image_to_monitor" function to the file
"gimpdisplayshell-render.c", except modified so that the printf lines
say "WAS/was NOT created in gimpdisplayshell-render.c". Again, Gimp
compiles and runs with no problems, but no lines are printed from this
function because at this point it isn't called by the function
"gimp_display_shell_render".
Unfortunately, as soon as I add the line "image_to_monitor ();" to the
function "gimp_display_shell_render", then Gimp won't compile, but
rather exits with these lines:
make[4]: Leaving directory `/home/elle/code/gimp292/build/gimp/app/gui'
Making all in .
make[4]: Entering directory `/home/elle/code/gimp292/build/gimp/app'
CCLD gimp-2.9
display/libappdisplay.a(gimpdisplayshell-render.o): In function
`image_to_monitor':
/home/elle/code/gimp292/build/gimp/app/display/gimpdisplayshell-render.c:79:
undefined reference to `cmsOpenProfileFromFile'
/home/elle/code/gimp292/build/gimp/app/display/gimpdisplayshell-render.c:80:
undefined reference to `cmsOpenProfileFromFile'
/home/elle/code/gimp292/build/gimp/app/display/gimpdisplayshell-render.c:81:
undefined reference to `cmsCreateTransform'
Any ideas as to why Gimp won't compile when I add the line
"image_to_monitor ();" to the function "gimp_display_shell_render"?
It's pretty obvious that it's not finding lcms2.h. But I did add the
following "#include" lines to the top of gimpdisplayshell-render.c:
#include /* lcms.h uses the "inline" keyword */
#include
#include
Suggestions regarding other/better ways to approach the problem are
also welcome!
Elle
___
gimp-developer-list mailing list
List address:gimp-developer-list@gnome.org
List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images (Was Re: Update on my Gimp color management coding efforts)
On 10 September 2013 17:15, Elle Stone wrote: > On 11/12/12, Elle Stone wrote: >> On 11/10/12, Michael Natterer wrote: >>> On Sat, 2012-11-10 at 15:17 -0500, Elle Stone wrote: On 11/8/12, Jon Nordby wrote: > * Change the lcms-based conversion (modules/display-filter-lcms.c) > from being a generic display filter to be something that takes a > GeglBuffer in and blits into a cairo_surface_t. > * Change the display filter interface to accept a GeglBuffer instead > of a cairo_surface_t. As gimp_color_display_convert_surface is public > API, it should probably become a stub and be marked as deprecated. New > interface could for instance be called > "gimp_color_display_convert_buffer" > * Adapt all the display filter operations (modules/display-filter-*.c) > to the new interface and to working on 32bit floating point. If any of > the operations are no longer useful, now would be the time to drop > them. > * In the use of the display filter stack (in > gimp_display_shell_render), first let the filter stack operate on the > GeglBuffer from the projection (or possibly a copy), and then pass it > to the lcms-based color conversion, and then pass that to cairo. I'm looking forward to taking another look at the monitor display code path. Your suggestions sound very helpful. >>> >>> It does, but it's clearly step 2 (or step n). IMO we should first >>> get the lcms plug-in right so the data GIMP is dealing with is >>> correct in the first place. >> Hi Elle, nice to see this picked up again! > Step 1 happened a long time ago. I'm trying to implement Jon Norby's > suggestions because it would be nice to see a linear gamma image > displayed without posterization in the shadows from the conversion to > 8-bits that happens before the conversion to the monitor profile. > Quoting Jon's suggestion: > > (1)call a "GeglBuffer from the projection (or possibly a copy)" and then > (2)pass it to the lcms-based color conversion, and then > (3)pass that to cairo. > > So how does one "call a GeglBuffer from the projection"? I said "operate on", not "call". :) The projection has a GeglBuffer associated with it, see https://git.gnome.org/browse/gimp/tree/app/display/gimpdisplayshell-render.c#n77 That is what the color conversion needs to use as an input, basically*. * It should probably go through the display filters first, if any exists. But that can probably be solved later. -- Jon Nordby - www.jonnor.com ___ gimp-developer-list mailing list List address:gimp-developer-list@gnome.org List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
Re: [Gimp-developer] Displaying linear gamma images (Was Re: Update on my Gimp color management coding efforts)
The conversion happens in app/display/gimpdisplayshell-render.c :
gimp_display_shell_render() .
Look for the gegl_buffer_get call that converts to babl_format
("cairo-ARGB32").
It's more complicated that just that because the display filters all expect
sRGB u8 data still, but I suspect the best solution is for someone to just
start hacking at it.
___
gimp-developer-list mailing list
List address:gimp-developer-list@gnome.org
List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
