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 <[email protected]>:
> 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
[email protected]
https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer