Hi Sven,
I can give it a try. If I understand it correctly, gimp-8bit.c only implements u8->float conversions, which seem to be picked up by BABL correctly, so the problem lies in the float->u8 conversions that are computed by ReferenceFish. Is that correct?

Regards, Jan

On 09:14, Wed 14 May 08, Sven Neumann wrote:

Hi,

currently the GEGL operations in GIMP are very slow. I have done some basic profiling yesterday and it appears that the main problem is the conversion from floating point to 8bit. Here is where the most time is being spent. So it doesn't really make much sense to optimize the operations in GIMP or the point filters in GEGL. We need to look at the babl conversions first.

Here's an interesting paper that outlines how to do conversion from linear light floating point image data to 8-bit sRGB using a relatively small lookup table:

http://mysite.verizon.net/spitzak/conversion/sketches_0265.pdf

That is exactly the conversion that the tile sink executes. It would help us a lot if someone could implement this. The file extensions/gimp-8bit.c would probably be the right place to put this code. I am afraid I am not going to find time for this. Is anyone else interested in working on this?

Sven

Hello,

I can give it a try. If I understand it correctly, gimp-8bit.c only implements u8->float conversions, which seem to be picked up by BABL correctly, so the problem lies in the float->u8 conversions that are computed by ReferenceFish. Is that correct?

I just had a short look into the babl code; it looks like there is a float->u8 conversion implemented using the 16bit lookup table in extensions/gegl-fixups.c. In fact, it looks like the same algorithm as described in the pdf paper. I do not have much time to look at it now, I just noticed that the function gggl_float_to_index16 (line 130) is not inlined, but I guess it should be. Could you try to check if that helps?

Greets Richard

Regards,
Jan

On 09:14, Wed 14 May 08, Sven Neumann wrote:

Hi,

currently the GEGL operations in GIMP are very slow. I have done some basic profiling yesterday and it appears that the main problem is the conversion from floating point to 8bit. Here is where the most time is being spent. So it doesn't really make much sense to optimize the operations in GIMP or the point filters in GEGL. We need to look at the babl conversions first.

Here's an interesting paper that outlines how to do conversion from linear light floating point image data to 8-bit sRGB using a relatively small lookup table:

http://mysite.verizon.net/spitzak/conversion/sketches_0265.pdf

That is exactly the conversion that the tile sink executes. It would help us a lot if someone could implement this. The file extensions/gimp-8bit.c would probably be the right place to put this code. I am afraid I am not going to find time for this. Is anyone else interested in working on this?

Sven

Hi,

On 11:19, Wed 14 May 08, Richard Kralovic wrote:

I just had a short look into the babl code; it looks like there is a float->u8 conversion implemented using the 16bit lookup table in extensions/gegl-fixups.c. In fact, it looks like the same algorithm as described in the pdf paper. I do not have much time to look at it now, I just noticed that the function gggl_float_to_index16 (line 130) is not inlined, but I guess it should be. Could you try to check if that helps?

The gegl-fixups.c code deals with gamma corrected RGB u8 conversions, so the inlining would affect RGBA float -> R'G'B' u8 conversion. However, after loading a jpeg file into gimp-2.5 and the using a color tool, conversions from RGB u8->RGBA float and RGBA float -> RGB u8 are requested, that is non-gamma corrected RGB u8. Is there a simple way how to make gimp request conversions from/to R'G'B'u8 space?

On 09:14, Wed 14 May 08, Sven Neumann wrote:

Hi,

currently the GEGL operations in GIMP are very slow. I have done some basic profiling yesterday and it appears that the main problem is the conversion from floating point to 8bit. Here is where the most time is being spent. So it doesn't really make much sense to optimize the operations in GIMP or the point filters in GEGL. We need to look at the babl conversions first.

As you correctly pointed out on IRC, there is currently no speedup code to conform to 0.0001 default BABL error setting, so the RGBA float -> RGB u8 is handled by ReferenceFish and that is the reason the code is so slow. After setting the BABL_ERROR env. variable to 0.1, the RGBA float->RGB float->RGB u8 is selected, making it a bit faster. There is already RGBA float->RGB u8 shortcut in the babl codebase in the gggl-lies extension, however this one seems to work slower and introducing higher error than the RGBA float->RGB float->RGB u8 path.

The RGB u8->RGBA float conversion is correctly handled by the gimp-8bit extension, performing approx. twice as fast as the RGBA float->RGB float->RGB u8 path.

Regards Jan

Hi,

On 10:06, Thu 15 May 08, Sven Neumann wrote:

Changing this in GIMP is not feasible. What needs to be done is to implement shortcuts for the conversions that GIMP actually uses. Eventually he gimp-8bit extension should provide shortcuts for both gamma-corrected and linear 8bit double conversions.

attached is a patch that adds RGBA float->RGB u8 conversion using code already present in gggl-lies extension based on lookup table. I can try to move the code to the gimp-8bit extension, although I don't think there is a point to doing so, since this would only introduce duplicity and whould increase memory footprint of BABL. However, for this conversion to be selected, BABL_ERROR must be set at least to 0.0005.

The lookup tables in gegl-fixups are rather large, btw. As pointed out in the paper, a much smaller lookup table would be sufficient to cover the range from 0.0 to 1.0. I guess that adding a clever range check and reducing the size of the lookup tables would yield a performance improvement due to better cache coherency.

The lookup tables for float->u8 conversions are rather quite large and it is indeed possible to add a range check to size them down. However this range check would have to be present in the inner loop of a conversion, which would rather slow things down. Further, it wouldn't IMHO improve cache coherency much, since the pixels already are in the 0-1 range most of the time meaning only approx. 2.5% of the lookup table items are frequently touched.

Regards, Jan

Index: extensions/gggl-lies.c =================================================================== --- extensions/gggl-lies.c (revision 311) +++ extensions/gggl-lies.c (working copy) @@ -62,13 +62,17 @@ static float table_16_F[1 << 16 static unsigned char table_F_8[1 << 16]; static unsigned short table_F_16[1 << 16];
-
static int table_inited = 0;

static void
table_init (void)
{
int i;
+ union
+ {
+ float f;
+ unsigned short s[2];
+ } u;

if (table_inited)
return;
@@ -84,64 +88,48 @@ table_init (void) table_16_F[i] = (i * 1.0) / 65535.0; }
/* fill tables for conversion from float to integer */ - {
- union
+
+ u.s[0] = 0;
+ for (i = 0; i < 1 << 16; i++)
{
- float f;
- unsigned short s[2];
- } u;
- u.f = 0.0;
-
- u.s[0] = 0.0;
-
- for (i = 0; i < 1 << 16; i++)
- {
- unsigned char c;
- unsigned short s;
+ unsigned char c;
+ unsigned short s;

- u.s[1] = i;
+ u.s[1] = i;

- if (u.f = 1.0)
- {
- c = 255;
- s = 65535;
- }
- else
- {
- c = rint (u.f * 255.0); - s = rint (u.f * 65535.0); - }
+ if (u.f = 1.0)
+ {
+ c = 255;
+ s = 65535;
+ }
+ else
+ {
+ c = rint (u.f * 255.0);
+ s = rint (u.f * 65535.0); + }

/*fprintf (stderr, "%2.3f=%03i %05i ", f, c, (*hi)); / if (! ((*hi)%9))
/ fprintf (stderr, "\n"); */
- table_F_8[u.s[1]] = c;
- table_F_16[u.s[1]] = s;
- }
- }
- /* fix tables to ensure 1:1 conversions back and forth */ - if (0)
- { /*FIXME: probably not the right way to do it,.. must sit down and scribble on paper */ - int i;
- for (i = 0; i < 256; i++)
- {
- float f = table_8_F[i]; - unsigned short *hi = ((unsigned short *) (void *) &f); - unsigned short *lo = ((unsigned short *) (void *) &f); - *lo = 0;
- table_F_8[(*hi)] = i;
- }
+ table_F_8[u.s[1]] = c;
+ table_F_16[u.s[1]] = s;
}
+ /* fix tables to ensure 1:1 conversions back and forth */ + for (i = 0; i < 256; i++)
+ {
+ u.f = table_8_F[i];
+ table_F_8[u.s[1]] = i;
+ }
}

/* function to find the index in table for a float */ -static unsigned int
+static INLINE unsigned int
gggl_float_to_index16 (float f)
{
union
@@ -904,6 +892,33 @@ conv_rgbaF_rgbA16 (unsigned char *src, u return samples;
}

+#ifdef USE_TABLES
+
+static INLINE long
+conv_rgbaF_rgb8 (unsigned char *src, unsigned char *dst, long samples) +{
+ long n = samples;
+ register float f;
+ float *fsrc = (float *) src;
+
+ while (n--)
+ {
+ f = (*(float *) fsrc++);
+ *(unsigned char *) dst++ = table_F_8[gggl_float_to_index16 (f)]; +
+ f = (*(float *) fsrc++);
+ *(unsigned char *) dst++ = table_F_8[gggl_float_to_index16 (f)]; +
+ f = (*(float *) fsrc++);
+ *(unsigned char *) dst++ = table_F_8[gggl_float_to_index16 (f)]; +
+ fsrc++;
+ }
+ return samples;
+}
+
+#else
+
static INLINE long
conv_rgbaF_rgb8 (unsigned char *src, unsigned char *dst, long samples) {
@@ -924,6 +939,8 @@ conv_rgbaF_rgb8 (unsigned char *src, uns return samples;
}

+#endif
+
static INLINE long
conv_rgbaF_g8 (unsigned char *src, unsigned char *dst, long samples) {
@@ -1951,6 +1968,8 @@ init (void)
babl_component ("A"),
NULL);

+ table_init();
+
#define o(src, dst) \
babl_conversion_new (src, dst, "linear", conv_ ## src ## _ ## dst, NULL)

On Wed, May 14, 2008 at 8:14 AM, Sven Neumann wrote:

currently the GEGL operations in GIMP are very slow. I have done some basic profiling yesterday and it appears that the main problem is the conversion from floating point to 8bit. Here is where the most time is being spent. So it doesn't really make much sense to optimize the operations in GIMP or the point filters in GEGL. We need to look at the babl conversions first.

Some of the extra overhead spent in the GeglOperationPointFilter base class has now been optimized away in a fast code path. The new code paths is markedly faster.

/Øyvind K.