Re: two small patches

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

 



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

[Index of Archives]     [Yosemite News]     [Yosemite Photos]     [gtk]     [GIMP Users]     [KDE]     [Gimp's Home]     [Gimp on Windows]     [Steve's Art]

  Powered by Linux