[gegl] buffer: don't use cached sampler for gegl_buffer_sample[_at_level]()



commit 26f13cbfe9aaaa8c176162e54fdbb8af6876538e
Author: Ell <ell_se yahoo com>
Date:   Sun May 13 17:05:19 2018 -0400

    buffer: don't use cached sampler for gegl_buffer_sample[_at_level]()
    
    The use of a per-buffer cached sampler is problematic, especially
    since the cached sampler may cache stale data after the buffer is
    modified.  Avoiding this requires careful use of
    gegl_buffer_sample_cleanup().  On the one hand, when using
    gegl_buffer_sample[_at_level]() for one-off samples in loosely-
    coupled code, this effectively means calling
    gegl_buffer_sample_cleanup() before/after each sample, defeating
    the purpose of caching the sampler in the first place.  On the
    other hand, when performing multiple samples in performance-
    critical code, using a sampler object is much more efficient
    anyway.
    
    Remove the per-buffer cached sampler, and simply use a per-call
    temporary sampler in gegl_buffer_sample[_at_level]().  Advise users
    to use gegl_buffer_sampler_new[_at_level]() to create a sampler
    object instead when performance is a concern.
    
    Deprecate gegl_buffer_sample_cleanup(), which is not necessary
    anymore, and keep it as a NOP for now.

 gegl/buffer/gegl-buffer-private.h |    7 ---
 gegl/buffer/gegl-buffer.c         |    2 -
 gegl/buffer/gegl-buffer.h         |   23 +++++++----
 gegl/buffer/gegl-sampler.c        |   74 ++++--------------------------------
 4 files changed, 24 insertions(+), 82 deletions(-)
---
diff --git a/gegl/buffer/gegl-buffer-private.h b/gegl/buffer/gegl-buffer-private.h
index d7e700d..e4fe3de 100644
--- a/gegl/buffer/gegl-buffer-private.h
+++ b/gegl/buffer/gegl-buffer-private.h
@@ -53,13 +53,6 @@ struct _GeglBuffer
                                             should track any modifications to the
                                             extent rectangle */
 
-
-  GeglSampler      *sampler; /* cached sampler for speeding up random
-                                access interpolated fetches from the
-                                buffer */
-  GeglSamplerType   sampler_type;
-  const Babl       *sampler_format; /* the format of the cached sampler */
-
   GeglTileStorage  *tile_storage;
 
   gint              tile_width;
diff --git a/gegl/buffer/gegl-buffer.c b/gegl/buffer/gegl-buffer.c
index a2480cf..b4fa962 100644
--- a/gegl/buffer/gegl-buffer.c
+++ b/gegl/buffer/gegl-buffer.c
@@ -378,8 +378,6 @@ gegl_buffer_dispose (GObject *object)
   GeglBuffer  *buffer  = GEGL_BUFFER (object);
   GeglTileHandler *handler = GEGL_TILE_HANDLER (object);
 
-  gegl_buffer_sample_cleanup (buffer);
-
   if (gegl_cl_is_accelerated ())
     gegl_buffer_cl_cache_invalidate (GEGL_BUFFER (object), NULL);
 
diff --git a/gegl/buffer/gegl-buffer.h b/gegl/buffer/gegl-buffer.h
index 6df6b9d..41d931e 100644
--- a/gegl/buffer/gegl-buffer.h
+++ b/gegl/buffer/gegl-buffer.h
@@ -415,8 +415,11 @@ GeglBuffer *    gegl_buffer_dup               (GeglBuffer       *buffer);
  * GEGL_ABYSS_LOOP (buffer contents are tiled if outside of the abyss rectangle).
  *
  * Query interpolate pixel values at a given coordinate using a specified form
- * of interpolation. The samplers used cache for a small neighbourhood of the
- * buffer for more efficient access.
+ * of interpolation.
+ *
+ * If you intend to take multiple samples, consider using
+ * gegl_buffer_sampler_new_at_level() to create a sampler object instead, which
+ * is more efficient.
  */
 
 void              gegl_buffer_sample_at_level (GeglBuffer       *buffer,
@@ -449,8 +452,11 @@ void              gegl_buffer_sample_at_level (GeglBuffer       *buffer,
  * GEGL_ABYSS_LOOP (buffer contents are tiled if outside of the abyss rectangle).
  *
  * Query interpolate pixel values at a given coordinate using a specified form
- * of interpolation. The samplers used cache for a small neighbourhood of the
- * buffer for more efficient access.
+ * of interpolation.
+ *
+ * If you intend to take multiple samples, consider using
+ * gegl_buffer_sampler_new() to create a sampler object instead, which is more
+ * efficient.
  */
 void              gegl_buffer_sample          (GeglBuffer       *buffer,
                                                gdouble           x,
@@ -467,11 +473,12 @@ void              gegl_buffer_sample          (GeglBuffer       *buffer,
  * gegl_buffer_sample_cleanup:
  * @buffer: the GeglBuffer to sample from
  *
- * Clean up resources used by sampling framework of buffer (will be freed
- * automatically later when the buffer is destroyed, for long lived buffers
- * cleaning up the sampling infrastructure when it has been used for its
- * purpose will sometimes be more efficient).
+ * Clean up resources used by sampling framework of buffer.
+ *
+ * Deprecated: 0.4.2: This function has no effect. It is not necessary to call
+ * it after using gegl_buffer_sample() or gegl_buffer_sample_at_level().
  */
+G_DEPRECATED
 void            gegl_buffer_sample_cleanup    (GeglBuffer *buffer);
 
 typedef void (*GeglSamplerGetFun)  (GeglSampler     *self,
diff --git a/gegl/buffer/gegl-sampler.c b/gegl/buffer/gegl-sampler.c
index 4f7eb49..a9a4246 100644
--- a/gegl/buffer/gegl-sampler.c
+++ b/gegl/buffer/gegl-sampler.c
@@ -414,8 +414,6 @@ gegl_sampler_gtype_from_enum (GeglSamplerType sampler_type)
     }
 }
 
-static GMutex gegl_buffer_sampler_mutex = {0,};
-
 void
 gegl_buffer_sample_at_level (GeglBuffer       *buffer,
                              gdouble           x,
@@ -427,55 +425,17 @@ gegl_buffer_sample_at_level (GeglBuffer       *buffer,
                              GeglSamplerType   sampler_type,
                              GeglAbyssPolicy   repeat_mode)
 {
-  /*
-  if (sampler_type == GEGL_SAMPLER_NEAREST && format == buffer->soft_format)
-  {
-    GeglRectangle rect = {floorf (x), floorf(y), 1, 1};
-    gegl_buffer_get (buffer, &rect, 1, NULL, dest, GEGL_AUTO_ROWSTRIDE, repeat_mode);
-    return;
-  }*/
+  GeglSampler *sampler;
 
   if (!format)
     format = buffer->soft_format;
 
-  if (gegl_cl_is_accelerated ())
-  {
-    GeglRectangle rect = {floorf (x), floorf(y), 1, 1};
-    gegl_buffer_cl_cache_flush (buffer, &rect);
-  }
-
-  /* unset the cached sampler if it dosn't match the needs */
-  if (buffer->sampler == NULL ||
-      (buffer->sampler != NULL &&
-          (buffer->sampler_type != sampler_type || buffer->sampler_format != format)))
-    {
-      gboolean  threaded = gegl_config_threads () > 1;
-      GType desired_type = gegl_sampler_gtype_from_enum (sampler_type);
+  sampler = gegl_buffer_sampler_new_at_level (buffer,
+                                              format, sampler_type, level);
 
-      if (threaded)
-        g_mutex_lock (&gegl_buffer_sampler_mutex);
-
-      if (buffer->sampler)
-      {
-        g_object_unref (buffer->sampler);
-        buffer->sampler = NULL;
-        buffer->sampler_type = 0;
-      }
+  gegl_sampler_get (sampler, x, y, scale, dest, repeat_mode);
 
-      buffer->sampler_type = sampler_type;
-      buffer->sampler = g_object_new (desired_type,
-                                      "buffer", buffer,
-                                      "format", format,
-                                      "level", level,
-                                      NULL);
-      buffer->sampler_format = format;
-      gegl_sampler_prepare (buffer->sampler);
-
-      if (threaded)
-        g_mutex_unlock (&gegl_buffer_sampler_mutex);
-    }
-
-  buffer->sampler->get(buffer->sampler, x, y, scale, dest, repeat_mode);
+  g_object_unref (sampler);
 }
 
 
@@ -495,26 +455,10 @@ gegl_buffer_sample (GeglBuffer       *buffer,
 void
 gegl_buffer_sample_cleanup (GeglBuffer *buffer)
 {
-  gboolean threaded;
-
-  g_return_if_fail (GEGL_IS_BUFFER (buffer));
-
-  if (! buffer->sampler)
-    return;
-
-  threaded = gegl_config_threads ()>1;
-
-  if (threaded)
-    g_mutex_lock (&gegl_buffer_sampler_mutex);
-
-  if (buffer->sampler)
-    {
-      g_object_unref (buffer->sampler);
-      buffer->sampler = NULL;
-    }
-
-  if (threaded)
-    g_mutex_unlock (&gegl_buffer_sampler_mutex);
+  /* this is a nop.  we used to have a per-buffer cached sampler, for use by
+   * gegl_buffer_sample[_at_level](), which this function would clear, but we
+   * don't anymore.
+   */
 }
 
 GeglSampler *


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