[gegl] buffer, tests: in access.c, streamline tile alignment; fix unaligned buffer_clear()

[N/A <ell src gnome org>]

commit 2da0690a0aa0ab2d960726db64afec3013c25922
Author: Ell <ell_se yahoo com>
Date:   Fri Mar 9 04:43:24 2018 -0500

    buffer, tests: in access.c, streamline tile alignment; fix unaligned buffer_clear()
    
    In gegl_buffer_copy() and gegl_buffer_clear(), calculate tile-grid-
    aligned cow_rect directly instead of using loops, avoid splitting
    non-COWed area when cow_rect is empty, and some cleanup.
    
    Fix a bug in gegl_buffer_clear(), where clearing a region that
    doesn't cross tile boundary in either direction, erroneously
    cleared the entire area from the top/left edge of the region to
    the right/bottom edge of the containing tile, in the respective
    direction.
    
    Add a test for the above bug, for all of gegl_buffer_{clear,
    set_color,set_pattern,copy}().

 gegl/buffer/gegl-buffer-access.c            |  177 +++++++++++--------------
 tests/simple/Makefile.am                    |    1 +
 tests/simple/test-buffer-unaligned-access.c |  196 +++++++++++++++++++++++++++
 3 files changed, 274 insertions(+), 100 deletions(-)
---
diff --git a/gegl/buffer/gegl-buffer-access.c b/gegl/buffer/gegl-buffer-access.c
index ad4530e..06f62a3 100644
--- a/gegl/buffer/gegl-buffer-access.c
+++ b/gegl/buffer/gegl-buffer-access.c
@@ -2194,43 +2194,21 @@ gegl_buffer_copy2 (GeglBuffer          *src,
                    GeglBuffer          *dst,
                    const GeglRectangle *dst_rect)
 {
-  g_return_if_fail (GEGL_IS_BUFFER (src));
-  g_return_if_fail (GEGL_IS_BUFFER (dst));
-
-  if (!src_rect)
-    {
-      src_rect = gegl_buffer_get_extent (src);
-    }
+  GeglBufferIterator *i;
+  gint offset_x = src_rect->x - dst_rect->x;
+  gint offset_y = src_rect->y - dst_rect->y;
 
-  if (!dst_rect)
+  i = gegl_buffer_iterator_new (dst, dst_rect, 0, dst->soft_format,
+                                GEGL_ACCESS_WRITE | GEGL_ITERATOR_NO_NOTIFY,
+                                repeat_mode);
+  while (gegl_buffer_iterator_next (i))
     {
-      dst_rect = src_rect;
+      GeglRectangle src_rect = i->roi[0];
+      src_rect.x += offset_x;
+      src_rect.y += offset_y;
+      gegl_buffer_iterate_read_dispatch (src, &src_rect, i->data[0], 0,
+                                         dst->soft_format, 0, repeat_mode);
     }
-
-  if (src_rect->width == 0 || src_rect->height == 0)
-    return;
-
-  {
-    GeglRectangle dest_rect_r = *dst_rect;
-    GeglBufferIterator *i;
-    gint offset_x = src_rect->x - dst_rect->x;
-    gint offset_y = src_rect->y - dst_rect->y;
-
-    dest_rect_r.width = src_rect->width;
-    dest_rect_r.height = src_rect->height;
-
-    i = gegl_buffer_iterator_new (dst, &dest_rect_r, 0, dst->soft_format,
-                                  GEGL_ACCESS_WRITE | GEGL_ITERATOR_NO_NOTIFY,
-                                  repeat_mode);
-    while (gegl_buffer_iterator_next (i))
-      {
-        GeglRectangle src_rect = i->roi[0];
-        src_rect.x += offset_x;
-        src_rect.y += offset_y;
-        gegl_buffer_iterate_read_dispatch (src, &src_rect, i->data[0], 0,
-                                           dst->soft_format, 0, repeat_mode);
-      }
-  }
 }
 
 void
@@ -2240,6 +2218,8 @@ gegl_buffer_copy (GeglBuffer          *src,
                   GeglBuffer          *dst,
                   const GeglRectangle *dst_rect)
 {
+  GeglRectangle dest_rect_r;
+
   g_return_if_fail (GEGL_IS_BUFFER (src));
   g_return_if_fail (GEGL_IS_BUFFER (dst));
 
@@ -2247,12 +2227,20 @@ gegl_buffer_copy (GeglBuffer          *src,
     {
       src_rect = gegl_buffer_get_extent (src);
     }
+  if (src_rect->width <= 0 ||
+      src_rect->height <= 0)
+    return;
 
   if (!dst_rect)
     {
       dst_rect = src_rect;
     }
 
+  dest_rect_r = *dst_rect;
+  dest_rect_r.width = src_rect->width;
+  dest_rect_r.height = src_rect->height;
+  dst_rect = &dest_rect_r;
+
   if (src->soft_format == dst->soft_format &&
       src_rect->width >= src->tile_width &&
       src_rect->height >= src->tile_height &&
@@ -2262,37 +2250,30 @@ gegl_buffer_copy (GeglBuffer          *src,
       gegl_buffer_scan_compatible (src, src_rect->x, src_rect->y,
                                    dst, dst_rect->x, dst_rect->y))
     {
-      GeglRectangle dest_rect_r = *dst_rect;
-
-      gint tile_width = dst->tile_width;
+      gint tile_width  = dst->tile_width;
       gint tile_height = dst->tile_height;
 
-      dest_rect_r.width = src_rect->width;
-      dest_rect_r.height = src_rect->height;
-      dst_rect = &dest_rect_r;
+      GeglRectangle cow_rect = *dst_rect;
+      gint          rem;
 
-      {
-        GeglRectangle cow_rect = *dst_rect;
+      /* adjust origin to match the start of tile alignment */
+      rem = (cow_rect.x + dst->shift_x) % tile_width;
+      if (rem > 0)
+        rem -= tile_width;
+      cow_rect.x      -= rem;
+      cow_rect.width  += rem;
 
-        /* adjust origin until we match the start of tile alignment */
-        while ( (cow_rect.x + dst->shift_x) % tile_width)
-          {
-            cow_rect.x ++;
-            cow_rect.width --;
-          }
-        while ( (cow_rect.y + dst->shift_y) % tile_height)
-          {
-            cow_rect.y ++;
-            cow_rect.height --;
-          }
-        /* adjust size of rect to match multiple of tiles */
+      rem = (cow_rect.y + dst->shift_y) % tile_height;
+      if (rem > 0)
+        rem -= tile_height;
+      cow_rect.y      -= rem;
+      cow_rect.height += rem;
 
-        cow_rect.width  = cow_rect.width  - (cow_rect.width  % tile_width);
-        cow_rect.height = cow_rect.height - (cow_rect.height % tile_height);
-
-        g_assert (cow_rect.width >= 0);
-        g_assert (cow_rect.height >= 0);
+      /* adjust size of rect to match multiple of tiles */
+      cow_rect.width  -= cow_rect.width  % tile_width;
+      cow_rect.height -= cow_rect.height % tile_height;
 
+      if (cow_rect.width > 0 && cow_rect.height > 0)
         {
           GeglRectangle top, bottom, left, right;
 
@@ -2381,7 +2362,7 @@ gegl_buffer_copy (GeglBuffer          *src,
                                              src_rect->y + (bottom.y-dst_rect->y),
                                  bottom.width, bottom.height),
                              repeat_mode, dst, &bottom);
-          if (left.width)
+          if (left.width && left.height)
           gegl_buffer_copy2 (src,
                              GEGL_RECTANGLE (src_rect->x + (left.x-dst_rect->x),
                                              src_rect->y + (left.y-dst_rect->y),
@@ -2394,7 +2375,10 @@ gegl_buffer_copy (GeglBuffer          *src,
                                  right.width, right.height),
                              repeat_mode, dst, &right);
         }
-      }
+      else
+        {
+          gegl_buffer_copy2 (src, src_rect, repeat_mode, dst, dst_rect);
+        }
     }
   else
     {
@@ -2411,24 +2395,11 @@ gegl_buffer_clear2 (GeglBuffer          *dst,
   GeglBufferIterator *i;
   gint                pxsize;
 
-  g_return_if_fail (GEGL_IS_BUFFER (dst));
-
-  if (!dst_rect)
-    {
-      dst_rect = gegl_buffer_get_extent (dst);
-    }
-  if (dst_rect->width == 0 ||
-      dst_rect->height == 0)
-    return;
-
   pxsize = babl_format_get_bytes_per_pixel (dst->soft_format);
 
   if (gegl_cl_is_accelerated ())
     gegl_buffer_cl_cache_invalidate (dst, dst_rect);
 
-  /* FIXME: this can be even further optimized by special casing it so
-   * that fully voided tiles are dropped.
-   */
   i = gegl_buffer_iterator_new (dst, dst_rect, 0, dst->soft_format,
                                 GEGL_ACCESS_WRITE | GEGL_ITERATOR_NO_NOTIFY,
                                 GEGL_ABYSS_NONE);
@@ -2448,37 +2419,40 @@ gegl_buffer_clear (GeglBuffer          *dst,
     {
       dst_rect = gegl_buffer_get_extent (dst);
     }
+  if (dst_rect->width <= 0 ||
+      dst_rect->height <= 0)
+    return;
 
 #if 1
   /* cow for clearing is currently broken */
-  if (!g_object_get_data (G_OBJECT (dst), "is-linear"))
+  if (dst_rect->width >= dst->tile_width &&
+      dst_rect->height >= dst->tile_height &&
+      !g_object_get_data (G_OBJECT (dst), "is-linear"))
     {
-      gint tile_width = dst->tile_width;
+      gint tile_width  = dst->tile_width;
       gint tile_height = dst->tile_height;
 
-      {
-        GeglRectangle cow_rect = *dst_rect;
-
-        /* adjust origin until we match the start of tile alignment */
-        while ( (cow_rect.x + dst->shift_x) % tile_width)
-          {
-            cow_rect.x ++;
-            cow_rect.width --;
-          }
-        while ( (cow_rect.y + dst->shift_y) % tile_height)
-          {
-            cow_rect.y ++;
-            cow_rect.height --;
-          }
-        /* adjust size of rect to match multiple of tiles */
+      GeglRectangle cow_rect = *dst_rect;
+      gint          rem;
 
-        cow_rect.width  = cow_rect.width  - (cow_rect.width  % tile_width);
-        cow_rect.height = cow_rect.height - (cow_rect.height % tile_height);
+      /* adjust origin to match the start of tile alignment */
+      rem = (cow_rect.x + dst->shift_x) % tile_width;
+      if (rem > 0)
+        rem -= tile_width;
+      cow_rect.x      -= rem;
+      cow_rect.width  += rem;
 
+      rem = (cow_rect.y + dst->shift_y) % tile_height;
+      if (rem > 0)
+        rem -= tile_height;
+      cow_rect.y      -= rem;
+      cow_rect.height += rem;
 
-        g_assert (cow_rect.width >= 0);
-        g_assert (cow_rect.height >= 0);
+      /* adjust size of rect to match multiple of tiles */
+      cow_rect.width  -= cow_rect.width  % tile_width;
+      cow_rect.height -= cow_rect.height % tile_height;
 
+      if (cow_rect.width > 0 && cow_rect.height > 0)
         {
           GeglRectangle top, bottom, left, right;
 
@@ -2535,12 +2509,15 @@ gegl_buffer_clear (GeglBuffer          *dst,
           if (right.width < 0)
             right.width = 0;
 
-          if (top.height)     gegl_buffer_clear2 (dst, &top);
-          if (bottom.height)  gegl_buffer_clear2 (dst, &bottom);
-          if (left.width)     gegl_buffer_clear2 (dst, &left);
-          if (right.width)    gegl_buffer_clear2 (dst, &right);
+          if (top.height)                  gegl_buffer_clear2 (dst, &top);
+          if (bottom.height)               gegl_buffer_clear2 (dst, &bottom);
+          if (left.width && left.height)   gegl_buffer_clear2 (dst, &left);
+          if (right.width && right.height) gegl_buffer_clear2 (dst, &right);
+        }
+      else
+        {
+          gegl_buffer_clear2 (dst, dst_rect);
         }
-      }
     }
   else
 #endif
diff --git a/tests/simple/Makefile.am b/tests/simple/Makefile.am
index 482f79a..c27296d 100644
--- a/tests/simple/Makefile.am
+++ b/tests/simple/Makefile.am
@@ -7,6 +7,7 @@ noinst_PROGRAMS =                       \
        test-buffer-hot-tile    \
        test-buffer-sharing     \
        test-buffer-tile-voiding        \
+       test-buffer-unaligned-access    \
        test-change-processor-rect      \
        test-convert-format             \
        test-color-op                   \
diff --git a/tests/simple/test-buffer-unaligned-access.c b/tests/simple/test-buffer-unaligned-access.c
new file mode 100644
index 0000000..669488d
--- /dev/null
+++ b/tests/simple/test-buffer-unaligned-access.c
@@ -0,0 +1,196 @@
+/* This file is a test-case 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 (C) 2018 Ell
+ */
+
+#include "config.h"
+
+#include <stddef.h>
+#include <stdio.h>
+
+#include "gegl.h"
+
+
+#define SUCCESS    0
+#define FAILURE    -1
+
+
+typedef GeglColor * (* FillFunc) (GeglBuffer          *buffer,
+                                  const GeglRectangle *rect);
+
+
+static GeglColor *
+clear (GeglBuffer          *buffer,
+       const GeglRectangle *rect)
+{
+  gegl_buffer_clear (buffer, rect);
+
+  return gegl_color_new ("transparent");
+}
+
+static GeglColor *
+set_color (GeglBuffer          *buffer,
+           const GeglRectangle *rect)
+{
+  GeglColor *color = gegl_color_new ("red");
+
+  gegl_buffer_set_color (buffer, rect, color);
+
+  return color;
+}
+
+static GeglColor *
+set_pattern (GeglBuffer          *buffer,
+             const GeglRectangle *rect)
+{
+  GeglColor  *color = gegl_color_new ("green");
+  GeglBuffer *pattern;
+
+  pattern = gegl_buffer_new (GEGL_RECTANGLE (0, 0, 1, 1),
+                             gegl_buffer_get_format (buffer));
+
+  gegl_buffer_set_color (pattern, NULL, color);
+
+  gegl_buffer_set_pattern (buffer, rect, pattern, 0, 0);
+
+  g_object_unref (pattern);
+
+  return color;
+}
+
+static GeglColor *
+copy (GeglBuffer          *buffer,
+      const GeglRectangle *rect)
+{
+  GeglColor  *color = gegl_color_new ("blue");
+  GeglBuffer *src;
+
+  src = gegl_buffer_new (gegl_buffer_get_extent (buffer),
+                         gegl_buffer_get_format (buffer));
+
+  gegl_buffer_set_color (src, NULL, color);
+
+  gegl_buffer_copy (src, rect, GEGL_ABYSS_NONE, buffer, rect);
+
+  g_object_unref (src);
+
+  return color;
+}
+
+
+/* test that modifying a non-tile-grid-aligned area of a buffer using
+ * fill_func() only affects the said area.
+ */
+static gint
+test_unaligned_fill (FillFunc fill_func)
+{
+  gint                result = SUCCESS;
+  const Babl         *format = babl_format ("RGBA float");
+  gint                bpp    = babl_format_get_bytes_per_pixel (format);
+  gint                width, height;
+  GeglRectangle       rect;
+  GeglBuffer         *buffer;
+  GeglBufferIterator *iter;
+  GeglColor          *color;
+  guchar              pixel1[bpp];
+  guchar              pixel2[bpp];
+
+  g_object_get (gegl_config(),
+                "tile-width",  &width,
+                "tile-height", &height,
+                NULL);
+
+  rect.x      = width  / 4;
+  rect.y      = height / 4;
+  rect.width  = width  / 2;
+  rect.height = height / 2;
+
+  buffer = gegl_buffer_new (GEGL_RECTANGLE (0, 0, width, height), format);
+
+  color = gegl_color_new ("white");
+  gegl_buffer_set_color (buffer, NULL, color);
+  gegl_color_get_pixel (color, format, pixel1);
+  g_object_unref (color);
+
+  color = fill_func (buffer, &rect);
+  gegl_color_get_pixel (color, format, pixel2);
+  g_object_unref (color);
+
+  iter = gegl_buffer_iterator_new (buffer, NULL, 0, format,
+                                   GEGL_BUFFER_READ, GEGL_ABYSS_NONE);
+
+  while (gegl_buffer_iterator_next (iter))
+    {
+      const guchar        *src = iter->data[0];
+      const GeglRectangle *roi = &iter->roi[0];
+      gint                 x;
+      gint                 y;
+
+      for (y = roi->y; y < roi->y + roi->height; y++)
+        {
+          for (x = roi->x; x < roi->x + roi->width; x++)
+            {
+              const guchar *pixel = pixel1;
+
+              if (x >= rect.x && x < rect.x + rect.width &&
+                  y >= rect.y && y < rect.y + rect.height)
+                {
+                  pixel = pixel2;
+                }
+
+              if (memcmp (src, pixel, bpp))
+                result = FAILURE;
+
+              src += bpp;
+            }
+        }
+    }
+
+  g_object_unref (buffer);
+
+  return result;
+}
+
+#define RUN_TEST(test, ...) \
+  do \
+  { \
+    printf (#test " (" #__VA_ARGS__ ")..."); \
+    fflush (stdout); \
+    \
+    if (test_##test (__VA_ARGS__) == SUCCESS) \
+      printf (" passed\n"); \
+    else \
+      { \
+        printf (" FAILED\n"); \
+        result = FAILURE; \
+      } \
+  } while (FALSE)
+
+int main (int argc, char *argv[])
+{
+  gint result = SUCCESS;
+
+  gegl_init (&argc, &argv);
+
+  RUN_TEST (unaligned_fill, clear);
+  RUN_TEST (unaligned_fill, set_color);
+  RUN_TEST (unaligned_fill, set_pattern);
+  RUN_TEST (unaligned_fill, copy);
+
+  gegl_exit ();
+
+  return result;
+}