Re: [Gegl-developer] speeding up GEGL operations in GIMP
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 <= 0.0) - { -c = 0; -s = 0; - } -else 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 <= 0.0) +{ + c = 0; + s = 0; +} + else 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_r
Re: [Gegl-developer] speeding up GEGL operations in GIMP
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 ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] speeding up GEGL operations in GIMP
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 ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Color temperature correction GeglOperation
Hi, On 18:36, Sat 26 Apr 08, Kevin Cozens wrote: > Jan Heller wrote: > > On 11:09, Sat 26 Apr 08, Øyvind Kolås wrote: > >> It would also be nice to replace the planckian locus lookup table with > >> a function that approximates it. > > > > I played with MATLAB for a while and came up with rational > > functions of degree 5 that approximate the Planckian locus > > dataset reasonably well. Attached is a modified version of > > the operation using these approximations. > > I haven't looked at the code yet, but I would have thought execution would be > faster using the lookup table than having to calculate values using a formula > especially given that the formula only approximates the values of the table. > In the previous version of the code the values outside the lookup table got interpolated anyway. The approximating functions are really quite good and pretty fast. Since this computation only takes place only at the beginning of the operation and not per pixel basis, the speed impairment is really negligible. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Color temperature correction GeglOperation
Hi, On 11:09, Sat 26 Apr 08, Øyvind Kolås wrote: > On Fri, Apr 25, 2008 at 6:52 PM, Jan Heller <[EMAIL PROTECTED]> wrote: > > > I wrote it to better familiarize myself with > > Gegl and I am posting it here in hope it will be useful for > > others. > > I think it is a good operation to have, so I have commited a slightly > modified version to svn. Nice, thanks! > > GEGL and babl deals with the out of gamut handling themselves at a > later stage, during processing we preserve headroom and footroom. This > allows us to change the contrast of the image to bring details back in > later. The implemented gamut handling also seemed to introduce banding > that the conversions in babl does not. Good to know. > > It would also be nice to replace the planckian locus lookup table with > a function that approximates it. I played with MATLAB for a while and came up with rational functions of degree 5 that approximate the Planckian locus dataset reasonably well. Attached is a modified version of the operation using these approximations. Regards, Jan /* This file is an image processing operation for GEGL * * GEGL is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 3 of the License, or (at your option) any later version. * * GEGL is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with GEGL; if not, see <http://www.gnu.org/licenses/>. * * Copyright 2008 Jan Heller */ #ifdef GEGL_CHANT_PROPERTIES gegl_chant_double (original_temp, "Original temperature", 1000, 12000, 6500, "Estimated temperature of the light source in K the image was taken with.") gegl_chant_double (intended_temp, "Intended temperature", 1000, 12000, 6500, "Corrected estimation of the temperature of the light source in K.") #else #define GEGL_CHANT_TYPE_POINT_FILTER #define GEGL_CHANT_C_FILE "color-temperature.c" #include "gegl-chant.h" #define LOWEST_TEMPERATURE 1000 #define HIGHEST_TEMPERATURE 12000 gfloat rgb_r55[][12]; static void convert_k_to_rgb (gfloat temperature, gfloat *rgb) { gfloat nomin, denom; intchannel, deg; if (temperature < LOWEST_TEMPERATURE) temperature = LOWEST_TEMPERATURE; if (temperature > HIGHEST_TEMPERATURE) temperature = HIGHEST_TEMPERATURE; /* Evaluation of an approximation of the Planckian locus in linear RGB space * by rational functions of degree 5 using Horner's scheme * f(x) = (p1*x^5 + p2*x^4 + p3*x^3 + p4*x^2 + p5*x + p6) / *(x^5 + q1*x^4 + q2*x^3 + q3*x^2 + q4*x + q5) */ for (channel = 0; channel < 3; channel++) { nomin = rgb_r55[channel][0]; for (deg = 1; deg < 6; deg++) nomin = nomin * temperature + rgb_r55[channel][deg]; denom = rgb_r55[channel][6]; for (deg = 1; deg < 6; deg++) denom = denom * temperature + rgb_r55[channel][6 + deg]; rgb[channel] = nomin / denom; } } static void prepare (GeglOperation *operation) { Babl *format = babl_format ("RGBA float"); gegl_operation_set_format (operation, "input", format); gegl_operation_set_format (operation, "output", format); } /* GeglOperationPointFilter gives us a linear buffer to operate on * in our requested pixel format */ static gboolean process (GeglOperation *op, void *in_buf, void *out_buf, glong n_pixels) { GeglChantO *o = GEGL_CHANT_PROPERTIES (op); gfloat *in_pixel; gfloat *out_pixel; gfloat original_temp, original_temp_rgb[3]; gfloat intended_temp, intended_temp_rgb[3]; gfloat coefs[3]; glong i; in_pixel = in_buf; out_pixel = out_buf; original_temp = o->original_temp; intended_temp = o->intended_temp; convert_k_to_rgb (original_temp, original_temp_rgb); convert_k_to_rgb (intended_temp, intended_temp_rgb); coefs[0] = original_temp_rgb[0] / intended_temp_rgb[0]; coefs[1] = original_temp_rgb[1] / intended_temp_rgb[1]; coefs[2] = original_temp_rgb[2] / intended_temp_rgb[2]; for (i = 0; i < n_pixels; i++) { out_pixel[0] = in_pixel[0] * coefs[0]; out_pixel[1] = in_pixel[1] * coefs[1]; out_pixel[2] = in_pixel[2] * coefs[2]; in_pixel += 4; out_pixel += 4; } return TRUE; } static void gegl_chant_class_init (GeglChantClass *klass) { GeglOperationClass*operation_class; GeglOperationPointFilterClass *point_fi
[Gegl-developer] Color temperature correction GeglOperation
Hi, attached is a GeglOperation performing color temperature correction. This is of course analogical to the whitebalance operation that is already in the Gegl source tree, though it uses photographer friendly input values. It has two parameters, estimated temperature of the light source in K the image was taken with and Corrected estimation of the temperature of the light source in K. The computation is a rather simple linear interpolation of a data table. I wrote it to better familiarize myself with Gegl and I am posting it here in hope it will be useful for others. Regards, Jan /* This file is an image processing operation for GEGL * * GEGL is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public * License as published by the Free Software Foundation; either * version 3 of the License, or (at your option) any later version. * * GEGL is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * Lesser General Public License for more details. * * You should have received a copy of the GNU Lesser General Public * License along with GEGL; if not, see <http://www.gnu.org/licenses/>. * * Copyright 2008 Jan Heller <[EMAIL PROTECTED]> */ #ifdef GEGL_CHANT_PROPERTIES gegl_chant_double (original_temp, "Original temperature", 1000, 12000, 6500, "Estimated temperature of the light source in K the image was taken with.") gegl_chant_double (intended_temp, "Intended temperature", 1000, 12000, 6500, "Corrected estimation of the temperatureof the light source in K.") #else #define GEGL_CHANT_TYPE_POINT_FILTER #define GEGL_CHANT_C_FILE "color-temperature.c" #include "gegl-chant.h" #define LOWEST_TEMPERATURE 1000 #define HIGHEST_TEMPERATURE 12000 #define TEMPERATURE_STEP 20 #define LERP(x, x0, y0, x1, y1) (y0 + (x - x0) * (gfloat) (y1 - y0) / (gfloat) (x1 - x0)) gfloat planckian_locus[][3]; static void convert_k_to_rgb (gfloat temperature, gfloat *rgb) { intilow, ihigh; intdelta_temperature; gfloat resid_temperature; if (temperature < LOWEST_TEMPERATURE) temperature = LOWEST_TEMPERATURE; if (temperature > HIGHEST_TEMPERATURE) temperature = HIGHEST_TEMPERATURE; delta_temperature = temperature - LOWEST_TEMPERATURE; ilow = delta_temperature / TEMPERATURE_STEP; ihigh = (delta_temperature % TEMPERATURE_STEP) ? ilow + 1 : ilow; resid_temperature = temperature - (ilow * TEMPERATURE_STEP + LOWEST_TEMPERATURE); rgb[0] = LERP(resid_temperature, 0, planckian_locus[ilow][0], TEMPERATURE_STEP, planckian_locus[ihigh][0]); rgb[1] = LERP(resid_temperature, 0, planckian_locus[ilow][1], TEMPERATURE_STEP, planckian_locus[ihigh][1]); rgb[2] = LERP(resid_temperature, 0, planckian_locus[ilow][2], TEMPERATURE_STEP, planckian_locus[ihigh][2]); } static void prepare (GeglOperation *operation) { Babl *format = babl_format ("RGBA float"); gegl_operation_set_format (operation, "input", format); gegl_operation_set_format (operation, "output", format); } /* GeglOperationPointFilter gives us a linear buffer to operate on * in our requested pixel format */ static gboolean process (GeglOperation *op, void *in_buf, void *out_buf, glong n_pixels) { GeglChantO *o = GEGL_CHANT_PROPERTIES (op); gfloat *in_pixel; gfloat *out_pixel; gfloat original_temp, original_temp_rgb[3]; gfloat intended_temp, intended_temp_rgb[3]; gfloat coefs[3]; glong i; gfloat min, max; in_pixel = in_buf; out_pixel = out_buf; original_temp = o->original_temp; intended_temp = o->intended_temp; convert_k_to_rgb (original_temp, original_temp_rgb); convert_k_to_rgb (intended_temp, intended_temp_rgb); coefs[0] = original_temp_rgb[0] / intended_temp_rgb[0]; coefs[1] = original_temp_rgb[1] / intended_temp_rgb[1]; coefs[2] = original_temp_rgb[2] / intended_temp_rgb[2]; for (i = 0; i < n_pixels; i++) { out_pixel[0] = in_pixel[0] * coefs[0]; out_pixel[1] = in_pixel[1] * coefs[1]; out_pixel[2] = in_pixel[2] * coefs[2]; /* handle out-of-gamut colors */ min = -MIN(MIN(out_pixel[0], out_pixel[1]), MIN(out_pixel[2], 0)); out_pixel[0] += min; out_pixel[1] += min; out_pixel[2] += min; max = MAX(MAX(out_pixel[0], out_pixel[1]), MAX(out_pixel[2], 1)) - 1; out_pixel[0] -= max; out_pixel[1] -= max; out_pixel[2] -= max; in_pixel += 4; out_
Re: [Gegl-developer] BABL path vs. reference fish
On 00:51, Mon 14 Apr 08, Øyvind Kolås wrote: > One issue in babl's current design is that other activity on the > system at the same time as measuring is performed can skew results > quite a lot for a given conversion, not sure if there is a good way to > deal with this at runtime though. Hi, yes, the gettimeofday function is indeed not the best profiling method. I have put some though into this and came up with several alternatives how to tackle this problem. The first possibility is to replace gettimeofday by system function that accounts only for time of the given process, such as getrusage. Although the getrusage function provides way to measure process time in microseconds, the real resolution is far from 1 microsecond. Using getrusage would mean that the runtime profiling would have to take approx. 100x longer than in the current state to get reasonable results for the fastest conversions. And that is far from ideal. The second possibility is to profile during the compilation, which would allow for much longer and precise profiling. This would generate some profiling data in a form of a C file and would compile into babl in the second compile pass. This might work in a situation when the compiling machine is the target machine, however, the results can be of course skewed while this is not the case. Maybe the biggest drawback would be the fact that this contradicts the dynamic paradigm of the library. The third possibility is again to profile longer using getrusage or similar function, this time once per user. The profiling results would be output e.g. to ~/.babl/prof.data and would be reloaded next time. Once new .so with conversion would be found or new babl version installed, the profiling would be done again. So these are the alternatives I came up with. If you prefer one of these or have a better idea, I can investigate it further. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
[Gegl-developer] BABL path vs. reference fish
Hi, attached is a path that introduces logic to disallow creation of a fish path that is actually slower than appropriate reference fish. Suggested Changelog entry: * babl/babl-fish-path.c (get_conversion_path), (babl_fish_path), (test_create), (init_path_instrumentation), (destroy_path_instrumentation), (get_path_instrumentation): Improved fish path instrumentation during the search for a new one to optimize for speed. Added logic to disallow creation of a fish path that is actually slower than appropriate reference fish. * babl/babl-util.[ch] (babl_process_cost): New function for unified timing formula for babl processings. * babl/babl-conversion.c (babl_conversion_error): Make use of the new babl_process_cost function. * extensions/gimp-8bit.c: Added a function declaration to prevent compiler warning. Regards, Jan Index: extensions/gimp-8bit.c === --- extensions/gimp-8bit.c (revision 304) +++ extensions/gimp-8bit.c (working copy) @@ -296,6 +296,7 @@ conv_g8_gamma_2_2_rgbaF_linear (unsigned return samples; } +int init (void); int init (void) Index: babl/babl-fish-path.c === --- babl/babl-fish-path.c (revision 304) +++ babl/babl-fish-path.c (working copy) @@ -23,7 +23,17 @@ #define BABL_MAX_COST_VALUE 200 static double -get_conversion_path_error (BablList *path); +init_path_instrumentation (Babl *fmt_source, + Babl *fmt_destination); + +static void +destroy_path_instrumentation (void); + +static void +get_path_instrumentation (BablList *path, + double *path_cost, + double *ref_cost, + double *path_error); static long process_conversion_path (BablList *path, @@ -118,28 +128,32 @@ get_conversion_path (Babl *current_forma { /* We have found a candidate path, let's * see about it's properties */ - double temp_cost = 0.0; - double temp_error = 1.0; + double path_cost = 0.0; + double ref_cost = 0.0; + double path_error = 1.0; inti; for (i = 0; i < babl_list_size (current_path); i++) { - temp_error *= (1.0 + babl_conversion_error ((BablConversion *) current_path->items[i])); - temp_cost += babl_conversion_cost ((BablConversion *) current_path->items[i]); + path_error *= (1.0 + babl_conversion_error ((BablConversion *) current_path->items[i])); } - if (temp_cost < fish_path->fish_path.cost && - temp_error - 1.0 <= legal_error () && /* check this before the next; - which does a more accurate - measurement of the error */ - (temp_error = get_conversion_path_error (current_path)) <= legal_error () - ) + if (path_error - 1.0 <= legal_error ()) /* check this before the next; + which does a more accurate + measurement of the error */ { - /* We have found the best path so far, - * let's copy it into our new fish */ - fish_path->fish_path.cost = temp_cost; - fish_path->fish.error = temp_error; - babl_list_copy (current_path, fish_path->fish_path.conversion_list); + get_path_instrumentation (current_path, &path_cost, &ref_cost, &path_error); + + if ((path_cost < ref_cost) && /* do not use paths that took longer to compute than reference */ + (path_cost < fish_path->fish_path.cost) && + (path_error <= legal_error ())) +{ + /* We have found the best path so far, + * let's copy it into our new fish */ + fish_path->fish_path.cost = path_cost; + fish_path->fish.error = path_error; + babl_list_copy (current_path, fish_path->fish_path.conversion_list); +} } } else @@ -163,11 +177,10 @@ get_conversion_path (Babl *current_forma if (!next_format->format.visited) { /* next_format is not in the current path, we can pay a visit */ - babl_list_insert_last (current_path, next_conversion); - get_conversion_path (next_format, current_length + 1, max_length); - babl_list_remove_last (current_path); - } - + babl_list_insert_last (current_path, next_conversion); + get_conversion_path (next_format, current_length + 1, max_length); + babl_list_remove_last (current_path); +} }
[Gegl-developer] BABL fishing patch
Hi, attached is a patch that improves logic of go_fishing code in babl-fish.c. The current code searches list of all fishes while looking for suitable preexistent BABL_FISH_PATH instance. The new code only searches relevant part of database's hash table. Further, the current code searches for fish path every time such a babl fish is requested, even though the same fish path has been requested before and not found. The new code creates dummy BABL_FISH instance with appropriate source/destination formats and inserts it into the fish database to indicate that fish path has been searched for and not found. See comments in the patch for details. Regards, Jan Index: babl/babl-internal.h === --- babl/babl-internal.h(revision 302) +++ babl/babl-internal.h(working copy) @@ -88,6 +88,8 @@ long babl_fish_path_process void *source, void *destination, longn); +int babl_fish_get_id (const Babl *source, + const Babl *destination); double babl_format_loss (Babl *babl); Babl * babl_image_from_linear (char *buffer, Index: babl/babl-fish-path.c === --- babl/babl-fish-path.c (revision 302) +++ babl/babl-fish-path.c (working copy) @@ -193,9 +193,10 @@ Babl * babl_fish_path (const Babl *source, const Babl *destination) { - Babl *babl = NULL; - char *name = create_name (source, destination, 1); + Babl *babl = NULL; + char *name; + name = create_name (source, destination, 1); babl = babl_db_exist_by_name (babl_fish_db (), name); if (babl) { @@ -205,17 +206,11 @@ babl_fish_path (const Babl *source, return babl; } - babl_assert (BABL_IS_BABL (source)); - babl_assert (BABL_IS_BABL (destination)); - - babl_assert (source->class_type == BABL_FORMAT); - babl_assert (destination->class_type == BABL_FORMAT); - babl = babl_calloc (1, sizeof (BablFishPath) + strlen (name) + 1); babl->class_type= BABL_FISH_PATH; - babl->instance.id = 0; + babl->instance.id = babl_fish_get_id (source, destination); babl->instance.name = ((char *) babl) + sizeof (BablFishPath); strcpy (babl->instance.name, name); babl->fish.source = source; Index: babl/babl-fish-simple.c === --- babl/babl-fish-simple.c (revision 302) +++ babl/babl-fish-simple.c (working copy) @@ -33,7 +33,6 @@ babl_fish_simple (BablConversion *conver babl_assert (BABL_IS_BABL (conversion)); name = create_name (conversion); - babl = babl_db_exist_by_name (babl_fish_db (), name); if (babl) { @@ -46,7 +45,7 @@ babl_fish_simple (BablConversion *conver babl = babl_malloc (sizeof (BablFishSimple) + strlen (name) + 1); babl->class_type= BABL_FISH_SIMPLE; - babl->instance.id = 0; + babl->instance.id = babl_fish_get_id (conversion->source, conversion->destination); babl->instance.name = ((char *) babl) + sizeof (BablFishSimple); strcpy (babl->instance.name, name); babl->fish.source = conversion->source; Index: babl/babl-fish-reference.c === --- babl/babl-fish-reference.c (revision 302) +++ babl/babl-fish-reference.c (working copy) @@ -69,7 +69,7 @@ babl_fish_reference (const Babl *source, babl = babl_malloc (sizeof (BablFishReference) + strlen (name) + 1); babl->class_type= BABL_FISH_REFERENCE; - babl->instance.id = 0; + babl->instance.id = babl_fish_get_id (source, destination); babl->instance.name = ((char *) babl) + sizeof (BablFishReference); strcpy (babl->instance.name, name); babl->fish.source = source; Index: babl/babl-fish.c === --- babl/babl-fish.c(revision 302) +++ babl/babl-fish.c(working copy) @@ -20,6 +20,87 @@ #include #include +typedef struct _BablFindFish BablFindFish; + +typedef struct _BablFindFish +{ + Babl *fish_path; + Babl *fish_ref; + Babl *fish_fish; + intfishes; + const Babl *source; + const Babl *destination; +} _BablFishFish; + + +static int +match_conversion (Babl *conversion, + void *inout); + +static int +find_fish_path (Babl *item, +void *data); + +static int +find_memcpy_fish (Babl *item, + void *data); + +static int +each_babl_fish_destroy (Babl *babl, +void *data); + + +/*
[Gegl-developer] BABL fish path patch
Hi, attached is a patch that ports BablFishPath class to the new list API and the list API is a bit expanded. Further, the algorithm for generating the shortest conversion path is reformulated to be more readable and comprehensible and thoroughly commented. The algorithm for processing the conversion paths is reformulated and commented. The patch also contains minor readability cleanups end speedups. Regards, Jan Index: babl/babl-list.c === --- babl/babl-list.c(revision 300) +++ babl/babl-list.c(working copy) @@ -63,29 +63,78 @@ int babl_list_size (BablList *list) { babl_assert (list); + return list->count; } -void -babl_list_insert (BablList *list, - Babl *item) +inline void +babl_list_insert_last (BablList *list, + Babl *item) { babl_assert(list); babl_assert(BABL_IS_BABL(item)); if (list->size < list->count + 1) { -Babl **new_items; + Babl **new_items; -new_items = babl_realloc (list->items, (list->size * 2) * sizeof (BablInstance *)); -babl_assert (new_items); -list->items = new_items; -memset (list->items + list->size, 0, list->size * sizeof (BablInstance *)); -list->size *= 2; + new_items = babl_realloc (list->items, (list->size * 2) * sizeof (BablInstance *)); + babl_assert (new_items); + list->items = new_items; + memset (list->items + list->size, 0, list->size * sizeof (BablInstance *)); + list->size *= 2; } list->items[list->count++] = item; } +inline void +babl_list_remove_last (BablList *list) +{ + babl_assert (list); + babl_assert (list->count > 0); + + list->count--; +} + +inline Babl * +babl_list_get_first (BablList *list) +{ + babl_assert (list); + babl_assert (list->count > 0); + + return (list->items[0]); +} + +inline Babl * +babl_list_get_last (BablList *list) +{ + babl_assert (list); + babl_assert (list->count > 0); + + return (list->items[list->count - 1]); +} + +inline void +babl_list_copy (BablList *from, +BablList *to) +{ + babl_assert (from); + babl_assert (to); + + if (to->size < from->count) +{ + Babl **new_items; + + new_items = babl_realloc (to->items, from->count * sizeof (BablInstance *)); + babl_assert (new_items); + to->items = new_items; + to->size = from->count; +} + +memcpy (to->items, from->items, from->count * sizeof (BablInstance *)); +to->count = from->count; +} + void babl_list_each (BablList *list, BablEachFunction each_fun, Index: babl/babl-list.h === --- babl/babl-list.h(revision 300) +++ babl/babl-list.h(working copy) @@ -47,9 +47,22 @@ babl_list_destroy (BablList *list); int babl_list_size (BablList *list); -void -babl_list_insert (BablList *list, - Babl *item); +inline void +babl_list_insert_last (BablList *list, + Babl *item); + +inline void +babl_list_remove_last (BablList *list); + +inline Babl * +babl_list_get_first (BablList *list); + +inline Babl * +babl_list_get_last (BablList *list); + +inline void +babl_list_copy (BablList *from, +BablList *to); void babl_list_each (BablList *list, Index: babl/babl-fish-path.c === --- babl/babl-fish-path.c (revision 300) +++ babl/babl-fish-path.c (working copy) @@ -19,14 +19,35 @@ #include #include "babl-internal.h" +#define BABL_LEGAL_ERROR0.01 +#define BABL_MAX_COST_VALUE 200 + static double -chain_error (const Babl *fmt_source, - const Babl *fmt_destination, - BablConversion **chain, - int conversions); +get_conversion_path_error (BablList *path); + +static long +process_conversion_path (BablList *path, + void *source_buffer, + void *destination_buffer, + long n); + +static void +get_conversion_path (Babl *current_format, + int current_length, + int max_length); + +static double * +test_create (void); + +static char * +create_name (const Babl *source, + const Babl *destination, + int is_reference); + +static double legal_error (void); + +static int max_path_length (void); -//#define BABL_LEGAL_ERROR 0.01 -//#define BABL_LEGAL_ERROR 0.01 static double legal_error (void) { @@ -40,7 +61,7 @@ static double legal_error (void) if (env) error = atof (env); else -error = 0.01; +error = BABL_LEGAL_ERROR; return error; } @@ -64,207 +85,99 @@ static int max_path_length (void) return max_length; } -typedef struct BablChainContext -{ - const Babl *from; - const Babl *to; - do
[Gegl-developer] BABL list API patch
Hi, attached is a patch that introduces minimal changes needed for removal of the old list routines. It ports several lists to the new API and deletes the unused old functions. There are still several plain C arrays in the code used as lists, however, these do not depend on the old routines and are left untouched by the patch. I can try to identify and port these as well. This would certainly improve things from the readability point of view, although it wouldn't IMO do much about the speed of the code. To Sven: Thanks for including me into the AUTHORS file :) Regards, Jan Index: babl/babl-model.c === --- babl/babl-model.c (revision 299) +++ babl/babl-model.c (working copy) @@ -27,7 +27,8 @@ static int each_babl_model_destroy (Babl *babl, void *data) { - babl_free (babl->model.from); + if (babl->model.from_list) +babl_list_destroy (babl->model.from_list); babl_free (babl); return 0; /* continue iterating */ } @@ -72,7 +73,7 @@ model_new (const char *name, strcpy (babl->instance.name, name); memcpy (babl->model.component, component, sizeof (BablComponent *) * components); - babl->model.from = NULL; + babl->model.from_list = NULL; return babl; } Index: babl/babl-sampling.c === --- babl/babl-sampling.c(revision 299) +++ babl/babl-sampling.c(working copy) @@ -46,7 +46,8 @@ static int each_babl_sampling_destroy (Babl *babl, void *data) { - babl_free (babl->sampling.from); + if (babl->sampling.from_list) +babl_list_destroy (babl->sampling.from_list); return 0; /* continue iterating */ } Index: babl/babl-list.c === --- babl/babl-list.c(revision 299) +++ babl/babl-list.c(working copy) @@ -16,11 +16,8 @@ * <http://www.gnu.org/licenses/>. */ -/* Implementation of list data structure. This is a bit superior - * to the list implementation in babl-util.c. +/* Implementation of list data structure. * Copyright (C) 2008, Jan Heller - * - * TODO: migrate babl to BablList */ #include "babl-internal.h" @@ -30,11 +27,19 @@ BablList * babl_list_init (void) { + return babl_list_init_with_size (BABL_LIST_INITIAL_SIZE); +} + +BablList * +babl_list_init_with_size (int initial_size) +{ BablList *list = babl_calloc (sizeof (BablList), 1); babl_assert (list); - list->size = BABL_LIST_INITIAL_SIZE; + if (initial_size == 0) +initial_size = 1; + list->size = initial_size; list->count = 0; list->items = NULL; if (list->size) @@ -81,13 +86,10 @@ babl_list_insert (BablList *list, list->items[list->count++] = item; } -/* TODO: Rename babl_list_each_temp to babl_list_each after the list migration - */ - void -babl_list_each_temp (BablList *list, - BablEachFunction each_fun, - void *user_data) +babl_list_each (BablList *list, +BablEachFunction each_fun, +void *user_data) { int i; Index: babl/babl-list.h === --- babl/babl-list.h(revision 299) +++ babl/babl-list.h(working copy) @@ -20,12 +20,13 @@ #define _BABL_LIST_H #ifndef _BABL_CLASSES_H +/* babl-classes.h contains forward declaration + * typedef struct _BablList BablList; + */ #error babl-list.h is only to be included after babl-classes.h #endif -typedef struct _BablList BablList; - typedef struct _BablList { int count; @@ -37,6 +38,9 @@ typedef struct _BablList BablList * babl_list_init (void); +BablList * +babl_list_init_with_size (int initial_size); + void babl_list_destroy (BablList *list); @@ -48,9 +52,9 @@ babl_list_insert (BablList *list, Babl *item); void -babl_list_each_temp (BablList *list, - BablEachFunction each_fun, - void *user_data); +babl_list_each (BablList *list, +BablEachFunction each_fun, +void *user_data); #endif Index: babl/babl-internal.h === --- babl/babl-internal.h(revision 299) +++ babl/babl-internal.h(working copy) @@ -25,6 +25,7 @@ #define BABL_MAX_COMPONENTS 32 #define BABL_HARD_MAX_PATH_LENGTH 8 +#define BABL_CONVERSIONS 5 #include #include Index: babl/babl-fish-path.c === --- babl/babl-fish-path.c (revision 299) +++ babl/babl-fish-path.c (working copy) @@ -141,10 +141,10 @@ get_conversion_chain (const Babl *f temp_chain[temp_conversions] = NULL;
Re: [Gegl-developer] BABL constructors speedup patch
Hi On 20:04, Sat 29 Mar 08, Sven Neumann wrote: > BTW, wasn't there some additional changes needed after your last patch? > If I remember correctly, some lists still need to be ported to the new > list API. Is that correct? That is correct. I will definitely port the rest of the code to the new API, I'm just contemplating the best course of action. There are other related changes I would want to make and I was thinking I'd make them all at once. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
[Gegl-developer] BABL constructors speedup patch
Hi, Here is an another patch I came up with while playing with BABL code. It concerns constructors of the BABL classes. The original code prepares the whole instance of a class before trying to insert it into the BABL database. The insertion might fail in case there is an instance of the same name already present in the database. This is a bit wasteful, so the patch tries to test the preexistent instance as soon as possible. It showed up that this brought a considerable speedup. It speeds babl_fish_path_dhtml by up to 9%. Speedup of everyday BABL usage won't be as significant, but measurable nonetheless. I sneaked two minor typo fixups into the patch as well. Regards, Jan Index: babl/babl-model.c === --- babl/babl-model.c (revision 297) +++ babl/babl-model.c (working copy) @@ -161,14 +161,24 @@ babl_model_new (void *first_argument, va_end (varg); - babl = model_new (create_name (name, components, component), id, components, component); + name = create_name (name, components, component); - { -Babl *ret = babl_db_insert (db, babl); -if (ret != babl) - babl_free (babl); -return ret; - } + babl = babl_db_exist (db, id, name); + if (babl) +{ + /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */ + return babl; +} + + babl = model_new (name, id, components, component); + + /* Since there is not an already registered instance by the required + * id/name, inserting newly created class into database. + */ + babl_db_insert (db, babl); + return babl; } Index: babl/babl-fish-path.c === --- babl/babl-fish-path.c (revision 297) +++ babl/babl-fish-path.c (working copy) @@ -284,6 +284,15 @@ babl_fish_path (const Babl *source, char *name = create_name (source, destination, 1); BablConversion *temp_chain[BABL_HARD_MAX_PATH_LENGTH]; + babl = babl_db_exist_by_name (babl_fish_db (), name); + if (babl) +{ + /* There is an instance already registered by the required name, + * returning the preexistent one instead. + */ + return babl; +} + babl_assert (BABL_IS_BABL (source)); babl_assert (BABL_IS_BABL (destination)); @@ -329,12 +338,11 @@ babl_fish_path (const Babl *source, return NULL; } - { -Babl *ret = babl_db_insert (babl_fish_db (), babl); -if (ret != babl) - babl_free (babl); -return ret; - } + /* Since there is not an already registered instance by the required + * name, inserting newly created class into database. + */ + babl_db_insert (babl_fish_db (), babl); + return babl; } static long Index: babl/babl-format.c === --- babl/babl-format.c (revision 297) +++ babl/babl-format.c (working copy) @@ -45,24 +45,25 @@ format_new (const char *name, { Babl *babl; - { -int i; -/* i is desintation position */ -for (i = 0; i < model->components; i++) - { -int j; - -for (j = 0; j < components; j++) - { -if (component[j] == model->component[i]) - goto component_found; - } -babl_fatal ("matching source component for %s in model %s not found", -model->component[i]->instance.name, model->instance.name); -component_found: -; - } - } + /* i is desintation position */ + int i, j, component_found = 0; + for (i = 0; i < model->components; i++) +{ + for (j = 0; j < components; j++) +{ + if (component[j] == model->component[i]) +{ + component_found = 1; + break; +} +} + if (!component_found) +{ + component_found = 0; + babl_fatal ("matching source component for %s in model %s not found", + model->component[i]->instance.name, model->instance.name); +} +} /* allocate all memory in one chunk */ babl = babl_malloc (sizeof (BablFormat) + @@ -295,17 +296,28 @@ babl_format_new (void *first_arg, va_end (varg); - babl = format_new (name ? name : create_name (model, components, component, type), + if (!name) +name = create_name (model, components, component, type); + + babl = babl_db_exist (db, id, name); + if (babl) +{ + /* There is an instance already registered by the required id/name, + * returning the preexistent one instead. + */ + return babl; +} + + babl = format_new (name, id, planar, components, model, component, sampling, type); - { -Babl *ret = babl_db_insert (db, babl); -if (ret != babl) - babl_free (babl); -return ret; - } + /* Since t
Re: [Gegl-developer] Hi, need any help?
On 21:09, Mon 10 Mar 08, Sven Neumann wrote: > I would prefer if this functionality could be added in new files > babl-list.[ch] and babl-hash-table.[ch]. Also the hash-table functions > should be prefixed with babl_hash_table_ instead of just babl_hash_ as > we should keep the babl_hash prefix for hash functions. Then I don't Such changes can be easily and happily arranged in case the patch would make it to the babl. > understand the purpose of babl_list_each_NEW(). This is just temporary, > isn't it? Yes, that is correct. There is an another implementation of the list data structure already present in the babl with a colliding function name. The next step after the patch would be migrating the code to the new implementation of the list data structure, which would not necessarily improve speed of the code, however, this would IMHO improve readability and maintainability a bit. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Hi, need any help?
On 17:55, Mon 10 Mar 08, Martin Nordholts wrote: > That looks very interesting. Do you think you could provide some > benchmarking data of the performance improvements in variuos situations? > It would be useful to have Hi, I used babl/tests/babl_fish_path_dhtml as a simple benchmark. Here are running times of babl_fish_path_dhtml w/o and w/ the patch applied: [EMAIL PROTECTED] ~/projects/gegl/babl/babl/tests $ time ./babl_fish_path_dhtml > babl_fish_path_0.0.20.html real0m10.463s user0m9.441s sys 0m0.040s [EMAIL PROTECTED] ~/projects/gegl/babl-patch/babl/tests $ time ./babl_fish_path_dhtml > babl_fish_path_patch.html real0m3.844s user0m3.416s sys 0m0.048s Here are the resulting HTML files: http://www.ms.mff.cuni.cz/~hellj1am/WWW/babl_fish_path_0.0.20.html http://www.ms.mff.cuni.cz/~hellj1am/WWW/babl_fish_path_patch.html Such a dramatic performance gain is not to be expected from any practical usage of the babl library, since the database code is not as heavily used. A simple 512x512 16-bit/color RGBA PNG image load/save test using gegl gives following results for runs w/o and w/ the patch. I took the respective best results from several tries: W/O real0m6.367s user0m3.476s sys 0m0.168s W real0m2.930s user0m2.400s sys 0m0.096s I hope these results give a better idea about the performance of the changes. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
Re: [Gegl-developer] Hi, need any help?
On 19:07, Sun 09 Mar 08, Martin Nordholts wrote: > Hi Jan > > We're absolutely interested in your contributions! Eagerly awaiting the > patch :) > > Best regards, > Martin Nordholts > Glad to hear it :) So here is the patch. Essentially, it implements coalesced hashing as BablHashTable structure and uses it as the babl database data structure. The code is not long and is quite self-explanatory I believe, although I wouldn't mind adding a few comments if needed. Regards, Jan Index: babl/babl-util.c === --- babl/babl-util.c(revision 290) +++ babl/babl-util.c(working copy) @@ -22,6 +22,287 @@ #include #include "babl-memory.h" #include "babl-internal.h" +#include "babl-util.h" + +#define UTIL_INITIAL_LIST_SIZE 0x7F +#define UTIL_INITIAL_HT_MASK 0x7F + +/* static functions declarations */ +static inline int +hash_by_name (BablHashTable *htab, + const char*str); + +static inline int +hash_by_id (BablHashTable *htab, +int id); + +static inline int +hash_insert (BablHashTable *htab, + Babl *item); + +static void +hash_rehash (BablHashTable *htab); + +/* BablList functions */ + +BablList * +babl_list_init (void) +{ + BablList *list = babl_calloc (sizeof (BablList), 1); + + list->size = UTIL_INITIAL_LIST_SIZE; + list->count = 0; + list->items = NULL; + if (list->size) +{ + list->items = babl_calloc (sizeof (BablInstance *), list->size); +} + + return list; +} + +void +babl_list_destroy (BablList *list) +{ +babl_assert(list); + +babl_free (list->items); +babl_free (list); +} + +void +babl_list_insert (BablList *list, + Babl *item) +{ + babl_assert(list); + babl_assert(BABL_IS_BABL(item)); + + if (list->size < list->count + 1) +{ +Babl **new_items; + +new_items = babl_realloc (list->items, (list->size * 2) * sizeof (BablInstance *)); +babl_assert (new_items); +list->items = new_items; +memset (list->items + list->size, 0, list->size * sizeof (BablInstance *)); +list->size *= 2; +} +list->items[list->count++] = item; +} + +void +babl_list_each_NEW (BablList *list, +BablEachFunction each_fun, +void *user_data) +{ + int i; + + babl_assert(list); + babl_assert(each_fun); + + for (i = 0; i < list->count; i++) +{ + if (list->items[i]) +{ + if (each_fun ((Babl *) list->items[i], user_data)) +break; +} +} +} + +/* BablHashTable functions */ + + +inline int +babl_hash_by_str (BablHashTable *htab, + const char*str) +{ + int hash = 0; + + while (*str) + { +hash += *str++; +hash += (hash << 10); +hash ^= (hash >> 6); + } + hash += (hash << 3); + hash ^= (hash >> 11); + hash += (hash << 15); + + return (hash & htab->mask); +} + +inline int +babl_hash_by_int (BablHashTable *htab, + int id) +{ + int hash = 0; + int i; + + for (i = 0; i < sizeof (int); i++) + { +hash += id & 0xFF; +hash += (hash << 10); +hash ^= (hash >> 6); +id >>= 8; + } + hash += (hash << 3); + hash ^= (hash >> 11); + hash += (hash << 15); + + return (hash & htab->mask); +} + +static inline int +hash_insert (BablHashTable *htab, + Babl *item) +{ + int hash = htab->hash_func(htab, item); + + if (htab->data_table[hash] == NULL) +{ + /* create new chain */ + htab->data_table[hash] = item; +} + else +{ + int it, oit, cursor = 0; + + while ((cursor < (htab->mask + 1)) && (htab->data_table[cursor] != NULL)) +++cursor; + + htab->data_table[cursor] = item; + + for (oit = hash, it = htab->chain_table[oit]; it != -1; oit = it, it = htab->chain_table[oit]) +; + htab->chain_table[oit] = cursor; +} + + htab->count++; + return 0; +} + +static void +hash_rehash (BablHashTable *htab) +{ + Babl *item; + int i; + BablHashTable *nhtab = babl_calloc (sizeof (BablHashTable), 1); + + nhtab->data_table = NULL; + nhtab->chain_table = NULL; + nhtab->mask = (htab->mask << 1) + 1; + nhtab->count = 0; + nhtab->hash_func = htab->hash_func; + nhtab->find_func = htab->find_func; + if (nhtab->mask) +{ + nhtab->data_table = babl_calloc (sizeof (BablInstance *), babl_hash_size(nhtab)); + nhtab->chain_table = babl_malloc (sizeof (int *) * babl_hash_size(nhtab)); + memset (nhtab->chain_table, -1, sizeof (int) * babl_hash_size(nhtab)); +} + + for (i = 0; i < babl_hash_size (htab); i++) +{ + item = htab->data_table[i]; + babl_hash_insert (nhtab, item); +} + + htab->mask = nhtab->mask; + babl_free (htab->data_table); + babl_free (htab->chain_table); + htab->data_table = nhtab->data_table; + htab->chain_table = nhtab->chain_table; + babl_free (nhtab); +} + +inline
[Gegl-developer] Hi, need any help?
Hi all, I am new to this list and I would like to help with the GEGL development. I am a programmer and a computer graphics enthusiast. I started to look into the GEGL/BABL code base and have been able to make some improvements in babl database code that speed things up a bit, without loss of readability. Should you guys be interested I can post the patch to this list and I can try to look into things further. If you have other ideas as to where any help might be needed I'll be glad to help. Regards, Jan ___ Gegl-developer mailing list Gegl-developer@lists.XCF.Berkeley.EDU https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer