Now split (and no re-introduced commented code, sorry for that): [gegl_buffer_rename.diff] Patch from Henrik Akesson that renames pset/pget to gegl_buffer_pixel_set and gegl_buffer_pixel_get in order to improve readability of code and to conform to gegl coding standards. * gegl/buffer/gegl-buffer-access.c * gegl/buffer/gegl-buffer-private.h [gegl_buffer_refactoring.diff] Patch from Henrik Akesson that extracts duplicated code from gegl_buffer_pixel_get and gegl_buffer_pixel_set into the function gegl_buffer_in_abyss to improve readability of code and maintainability. * gegl/buffer/gegl-buffer-access.c However for the get_rectangle_as_string functionality, I have to reconsider given that the function returns a string that is used in the GEGL_NOTE. You are right that g_string_free (string, FALSE) doesn't free the actual character data, but it cannot as it is used in the GEGL_NOTE function call. This means that the string has to be freed later after the GEGL_NOTE call. In the end it just makes a complicated code, so it's better not adding it. Thanks, Henrik 2009/3/24 Martin Nordholts <enselic@xxxxxxxxx>: > Henrik Akesson wrote: >> Two patches attached. I've made the below comments in a more suitable >> ChangeLog format: >> >> [gegl_buffer.diff] >> >> Patch from Henrik Akesson: >> - Extracted duplicated code from pset/pget into a separate function >> gegl_buffer_in_abyss to improve readability of code and maintainability. >> - Renamed pset/pget to gegl_buffer_pixel_set and gegl_buffer_pixel_get >> in order to improve readability of code and to be conformant to gegl >> coding standards. >> > > Hi, > > Do you think you can split up that patch into two patches? One with the > renames and one with the new utility function. It also added g_debug() > statements that were commented away, please remove those. Thanks! > >> [gegl_utils.diff] >> >> Patch from Henrik Akesson that adds a gegl_rectangle_as string >> function for retreiving a rectangle as a debug string. Log statements >> has been changed to use this said function as well as using GEGL_NOTE >> (see gegl-debug.h). > > That leaks memory because the data returned from g_string_free() needs > to be freed if FALSE is passed to the free_segment parameter (you pass > NULL (!)). Otherwise it is a nice change. Could you address the memory > leaks please? > > BR, > Martin >
Index: gegl/buffer/gegl-buffer-access.c =================================================================== --- gegl/buffer/gegl-buffer-access.c (revision 2986) +++ gegl/buffer/gegl-buffer-access.c (working copy) @@ -50,11 +50,11 @@ #if 0 static inline void -pset (GeglBuffer *buffer, - gint x, - gint y, - const Babl *format, - guchar *buf) +gegl_buffer_pixel_set (GeglBuffer *buffer, + gint x, + gint y, + const Babl *format, + guchar *buf) { gint tile_width = buffer->tile_storage->tile_width; gint tile_height = buffer->tile_storage->tile_width; @@ -116,11 +116,11 @@ #endif static inline void -pset (GeglBuffer *buffer, - gint x, - gint y, - const Babl *format, - gpointer data) +gegl_buffer_set_pixel (GeglBuffer *buffer, + gint x, + gint y, + const Babl *format, + gpointer data) { guchar *buf = data; gint tile_width = buffer->tile_storage->tile_width; @@ -200,11 +200,11 @@ } static inline void -pget (GeglBuffer *buffer, - gint x, - gint y, - const Babl *format, - gpointer data) +gegl_buffer_get_pixel (GeglBuffer *buffer, + gint x, + gint y, + const Babl *format, + gpointer data) { guchar *buf = data; gint tile_width = buffer->tile_storage->tile_width; @@ -595,7 +595,7 @@ if (rect && rect->width == 1 && rect->height == 1) /* fast path */ { - pset (buffer, rect->x, rect->y, format, src); + gegl_buffer_set_pixel (buffer, rect->x, rect->y, format, src); } else { @@ -949,7 +949,7 @@ rect->width == 1 && rect->height == 1) /* fast path */ { - pget (buffer, rect->x, rect->y, format, dest_buf); + gegl_buffer_get_pixel (buffer, rect->x, rect->y, format, dest_buf); #if ENABLE_MP g_static_rec_mutex_unlock (&mutex); #endif @@ -1084,7 +1084,7 @@ /*#define USE_WORKING_SHORTCUT*/ #ifdef USE_WORKING_SHORTCUT - pget (buffer, x, y, format, dest); + gegl_buffer_get_pixel (buffer, x, y, format, dest); return; #endif Index: gegl/buffer/gegl-buffer-private.h =================================================================== --- gegl/buffer/gegl-buffer-private.h (revision 2986) +++ gegl/buffer/gegl-buffer-private.h (working copy) @@ -47,8 +47,8 @@ GeglRectangle abyss; - GeglTile *hot_tile; /* cached tile for speeding up pget/pset (1x1 - sized gets/sets)*/ + GeglTile *hot_tile; /* cached tile for speeding up gegl_buffer_get_pixel + and gegl_buffer_set_pixel (1x1 sized gets/sets)*/ GeglSampler *sampler; /* cached sampler for speeding up random access interpolated fetches from the
117a118,139 > static gboolean > gegl_buffer_in_abyss( GeglBuffer *buffer, > gint x, > gint y ) > { > gint buffer_shift_x = buffer->shift_x; > gint buffer_shift_y = buffer->shift_y; > gint buffer_abyss_x = buffer->abyss.x + buffer_shift_x; > gint buffer_abyss_y = buffer->abyss.y + buffer_shift_y; > gint abyss_x_total = buffer_abyss_x + buffer->abyss.width; > gint abyss_y_total = buffer_abyss_y + buffer->abyss.height; > > > gint tiledy = y + buffer_shift_y; > gint tiledx = x + buffer_shift_x; > > return !(tiledy >= buffer_abyss_y && > tiledy < abyss_y_total && > tiledx >= buffer_abyss_x && > tiledx < abyss_x_total); > } > 133,136d154 < gint buffer_abyss_x = buffer->abyss.x + buffer_shift_x; < gint buffer_abyss_y = buffer->abyss.y + buffer_shift_y; < gint abyss_x_total = buffer_abyss_x + buffer->abyss.width; < gint abyss_y_total = buffer_abyss_y + buffer->abyss.height; 149,152c167 < if (!(tiledy >= buffer_abyss_y && < tiledy < abyss_y_total && < tiledx >= buffer_abyss_x && < tiledx < abyss_x_total)) --- > if (gegl_buffer_in_abyss( buffer, x, y)) 217,220d231 < gint buffer_abyss_x = buffer->abyss.x + buffer_shift_x; < gint buffer_abyss_y = buffer->abyss.y + buffer_shift_y; < gint abyss_x_total = buffer_abyss_x + buffer->abyss.width; < gint abyss_y_total = buffer_abyss_y + buffer->abyss.height; 233,236c244 < if (!(tiledy >= buffer_abyss_y && < tiledy < abyss_y_total && < tiledx >= buffer_abyss_x && < tiledx < abyss_x_total)) --- > if (gegl_buffer_in_abyss (buffer, x, y)) 319,321d326 < gint width = buffer->extent.width; < gint height = buffer->extent.height; < 333a339,340 > gint width = buffer->extent.width; > gint height = buffer->extent.height;
_______________________________________________ Gegl-developer mailing list Gegl-developer@xxxxxxxxxxxxxxxxxxxxxx https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer