Re: Displaying linear gamma images

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/12/13, Jon Nordby <jononor@xxxxxxxxx> wrote:
> On 12 September 2013 16:08, Elle Stone <l.elle.stone@xxxxxxxxx> 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 <l.elle.stone@xxxxxxxxx>
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 <glib.h>  /* elle lcms.h uses the "inline" keyword */
+
+#include <string.h> /* elle */
+/* this might be needed for Windows
+#ifdef G_OS_WIN32
+#define STRICT
+#include <windows.h>
+#define LCMS_WIN_TYPES_ALREADY_DEFINED
+#endif
+*/
+#include <lcms2.h> /* elle */
 
 #include <gegl.h>
 #include <gtk/gtk.h>
@@ -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;
+  gint                loops;
+  /* gint                projection_alpha; this variable will eventually be needed. */
+  gint                projection_bpp;
+  GeglBufferIterator *iter;
+  const Babl         *iter_format;
+  GeglBuffer         *buffer_clone;
+  gint                projection_width;
+  gint                projection_height;
+  /* gint                projection_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", 
+            projection_width, projection_height);
+
+  /* This code creates a hard-coded ICC profile transformation. It reads an image working space from disk. It also reads a monitor profile from disk. It should be modified to reflect similar profiles on your computers. The test image should be 32-bit floating point with an alpha channel. */
+  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_FLT,
+                                                   INTENT_RELATIVE_COLORIMETRIC,
+                                                   /* cmsFLAGS_NOOPTIMIZE |*/
+                                                   cmsFLAGS_BLACKPOINTCOMPENSATION ); 
+  if (image_to_monitor_transform)
+     {
+  iter_format       = babl_format ("R'G'B'A float");
+  projection_bpp    = babl_format_get_bytes_per_pixel (iter_format);
+  buffer_clone      = buffer;
+  /* projection_alpha  = babl_format_has_alpha (iter_format); */
+  /* printf("gimpdisplayshell-render.c projection_bpp, projection_alpha= %i %i \n", 
+            projection_bpp, projection_alpha); */
+  iter = gegl_buffer_iterator_new (buffer_clone, NULL, 0, iter_format,
+                                       GEGL_BUFFER_READ, GEGL_ABYSS_NONE);
+                                       
+  gegl_buffer_iterator_add (iter, buffer_clone, NULL, 0, iter_format,
+                                GEGL_BUFFER_WRITE, GEGL_ABYSS_NONE);
+
+     while (gegl_buffer_iterator_next (iter))
+          {
+           memcpy (iter->data[1], iter->data[0], iter->length * projection_bpp); 
+           cmsDoTransform (image_to_monitor_transform, iter->data[0], iter->data[1], iter->length);
+          }
+     }
+  else {printf("image_to_monitor_transform was NOT created in gimpdisplayshell-render.c.\n");}  
+      /*     gegl_buffer_flush (buffer_clone);
+      gimp_projection_flush_now (projection); */
+      
+      
 #ifdef GIMP_DISPLAY_RENDER_ENABLE_SCALING
-  /* if we had this future API, things would look pretty on hires (retina) */
+/*   if we had this future API, things would look pretty on hires (retina) */
   window_scale = gdk_window_get_scale_factor (gtk_widget_get_window (gtk_widget_get_toplevel (GTK_WIDGET (shell))));
 #endif
 
@@ -89,7 +150,7 @@ gimp_display_shell_render (GimpDisplayShell *shell,
                                                  &viewport_width,
                                                  &viewport_height);
   if (shell->rotate_transform)
-    {
+    {  printf("gimpdisplayshell-render.c if (shell->rotate_transform) \n");
       xfer = cairo_surface_create_similar_image (cairo_get_target (cr),
                                                  CAIRO_FORMAT_ARGB32,
                                                  w * window_scale,
@@ -99,7 +160,7 @@ gimp_display_shell_render (GimpDisplayShell *shell,
       src_y = 0;
     }
   else
-    {
+    {  printf("gimpdisplayshell-render.c if (shell->rotate_transform) else\n");/* here */
       xfer = gimp_display_xfer_get_surface (shell->xfer,
                                             w * window_scale,
                                             h * window_scale,
@@ -122,9 +183,9 @@ gimp_display_shell_render (GimpDisplayShell *shell,
 
   /*  apply filters to the rendered projection  */
   if (shell->filter_stack)
-    {
+    {  printf("gimpdisplayshell-render.c if (shell->filter_stack) \n"); /* here */
       cairo_surface_t *image =
-	cairo_image_surface_create_for_data (data, CAIRO_FORMAT_ARGB32,
+	  cairo_image_surface_create_for_data (data, CAIRO_FORMAT_ARGB32,
 					     w * window_scale,
 					     h * window_scale,
 					     stride);
@@ -133,11 +194,11 @@ gimp_display_shell_render (GimpDisplayShell *shell,
     }
 
   if (shell->mask)
-    {
+    {  printf("gimpdisplayshell-render.c if (shell->mask) \n");
       gint mask_height;
 
       if (! shell->mask_surface)
-        {
+        {  printf("gimpdisplayshell-render.c if (! shell->mask_surface) \n");
           shell->mask_surface =
             cairo_image_surface_create (CAIRO_FORMAT_A8,
                                         GIMP_DISPLAY_RENDER_BUF_WIDTH  *
diff --git a/libgimpwidgets/gimpcolordisplay.c b/libgimpwidgets/gimpcolordisplay.c
index 37e8dc0..4046c3c 100644
--- a/libgimpwidgets/gimpcolordisplay.c
+++ b/libgimpwidgets/gimpcolordisplay.c
@@ -347,7 +347,7 @@ gimp_color_display_clone (GimpColorDisplay *display)
 void
 gimp_color_display_convert_surface (GimpColorDisplay *display,
                                     cairo_surface_t  *surface)
-{
+{  printf("gimpcolordisplay.c gimp_color_display_convert_surface \n");
   g_return_if_fail (GIMP_IS_COLOR_DISPLAY (display));
   g_return_if_fail (surface != NULL);
   g_return_if_fail (cairo_surface_get_type (surface) ==
@@ -355,7 +355,7 @@ gimp_color_display_convert_surface (GimpColorDisplay *display,
 
   if (display->enabled &&
       GIMP_COLOR_DISPLAY_GET_CLASS (display)->convert_surface)
-    {
+    {  printf("gimpcolordisplay.c gimp_color_display_convert_surface if (display->enabled &&\n");
       cairo_surface_flush (surface);
       GIMP_COLOR_DISPLAY_GET_CLASS (display)->convert_surface (display, surface);
       cairo_surface_mark_dirty (surface);
diff --git a/libgimpwidgets/gimpcolordisplaystack.c b/libgimpwidgets/gimpcolordisplaystack.c
index 5a0401d..3527284 100644
--- a/libgimpwidgets/gimpcolordisplaystack.c
+++ b/libgimpwidgets/gimpcolordisplaystack.c
@@ -304,7 +304,7 @@ gimp_color_display_stack_convert_surface (GimpColorDisplayStack *stack,
   g_return_if_fail (surface != NULL);
   g_return_if_fail (cairo_surface_get_type (surface) ==
                     CAIRO_SURFACE_TYPE_IMAGE);
-
+  printf("gimpcolordisplaystack.c gimp_color_display_stack_convert_surface \n");
   for (list = stack->filters; list; list = g_list_next (list))
     {
       GimpColorDisplay *display = list->data;
diff --git a/modules/display-filter-lcms.c b/modules/display-filter-lcms.c
index ab047c4..415b9a9 100644
--- a/modules/display-filter-lcms.c
+++ b/modules/display-filter-lcms.c
@@ -308,7 +308,7 @@ cdisplay_lcms_configure (GimpColorDisplay *display)
 static void
 cdisplay_lcms_convert_surface (GimpColorDisplay *display,
                                cairo_surface_t  *surface)
-{
+{  printf("display-filter-lcms.c cdisplay_lcms_convert_surface \n");
   CdisplayLcms   *lcms   = CDISPLAY_LCMS (display);
   gint            width  = cairo_image_surface_get_width (surface);
   gint            height = cairo_image_surface_get_height (surface);
@@ -341,7 +341,7 @@ cdisplay_lcms_convert_surface (GimpColorDisplay *display,
           rowbuf[4*x+3] = b;
         }
 
-      cmsDoTransform (lcms->transform, rowbuf, rowbuf, width);
+      /* cmsDoTransform (lcms->transform, rowbuf, rowbuf, width); */
 
       /* And back to ARGB premul */
       for (x = 0; x < width; x++)
@@ -359,7 +359,7 @@ cdisplay_lcms_convert_surface (GimpColorDisplay *display,
 
 static void
 cdisplay_lcms_changed (GimpColorDisplay *display)
-{
+{  printf("display-filter-lcms.c cdisplay_lcms_changed \n");
   CdisplayLcms    *lcms   = CDISPLAY_LCMS (display);
   GimpColorConfig *config = gimp_color_display_get_config (display);
 
_______________________________________________
gimp-developer-list mailing list
List address:    gimp-developer-list@xxxxxxxxx
List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list

[Index of Archives]     [Video For Linux]     [Photo]     [Yosemite News]     [gtk]     [GIMP for Windows]     [KDE]     [GEGL]     [Gimp's Home]     [Gimp on GUI]     [Gimp on Windows]     [Steve's Art]

  Powered by Linux