Evas filters: Fix some crash and reduce insanity

In a rare situation the filter would access an invalid buffer.

Solution: Stop messing with buffer references by properly
referencing and releasing them when not needed, rather
than stealing references and hoping for the best. (There were
flags tracking stolen references, but that was still madness)

diff --git a/src/lib/evas/canvas/evas_filter_mixin.c b/src/lib/evas/canvas/evas_filter_mixin.c
index 46bf5fe..2722a48 100644 (file)
--- a/src/lib/evas/canvas/evas_filter_mixin.c
+++ b/src/lib/evas/canvas/evas_filter_mixin.c
@@ -238,8 +238,7 @@ evas_filter_object_render(Eo *eo_obj, Evas_Object_Protected_Data *obj,
 
         // Steal output and release previous
         filter_output = evas_filter_buffer_backing_steal(filter, EVAS_FILTER_BUFFER_OUTPUT_ID);
-        if (filter_output != previous)
-          evas_filter_buffer_backing_release(filter, previous);
+        evas_filter_buffer_backing_release(filter, previous);
 
         // Request rendering from the object itself (child class)
         evas_filter_program_padding_get(pd->data->chain, &l, &r, &t, &b);

diff --git a/src/lib/evas/filters/evas_filter.c b/src/lib/evas/filters/evas_filter.c
index bd2ccfc..484f44d 100644 (file)
--- a/src/lib/evas/filters/evas_filter.c
+++ b/src/lib/evas/filters/evas_filter.c
@@ -125,38 +125,21 @@ _filter_buffer_backing_free(Evas_Filter_Buffer *fb)
 {
    if (!fb) return;
 
-   if (!fb->stolen)
-     {
-        if (fb->allocated)
-          _backing_free(fb->ctx, fb->backing);
-        if (fb->glimage && fb->allocated_gl)
-          fb->ENFN->image_free(fb->ENDT, fb->glimage);
-        fb->backing = NULL;
-        fb->glimage = NULL;
-     }
-   else
-     {
-        if (!fb->ctx->gl_engine)
-          {
-             fb->delete_me = fb->allocated;
-          }
-        else if (fb->glimage && fb->allocated)
-          {
-             _backing_free(fb->ctx, fb->backing);
-             fb->backing = NULL;
-          }
-     }
+   _backing_free(fb->ctx, fb->backing);
+   if (fb->glimage)
+     fb->ENFN->image_free(fb->ENDT, fb->glimage);
+   fb->backing = NULL;
+   fb->glimage = NULL;
 }
 
 /* GL engine stuff: read-back from texture */
 static Eina_Bool
-_filter_buffer_glimage_pixels_read(Evas_Filter_Buffer *fb)
+_filter_buffer_glimage_pixels_read(Evas_Filter_Buffer *fb, void *glimage)
 {
    Eina_Bool ok;
 
    EINA_SAFETY_ON_NULL_RETURN_VAL(fb, EINA_FALSE);
    EINA_SAFETY_ON_FALSE_RETURN_VAL(fb->ctx->gl_engine, EINA_FALSE);
-   EINA_SAFETY_ON_NULL_RETURN_VAL(fb->glimage, EINA_FALSE);
 
    if (fb->backing)
      return EINA_TRUE;
@@ -166,17 +149,16 @@ _filter_buffer_glimage_pixels_read(Evas_Filter_Buffer *fb)
    EINA_SAFETY_ON_NULL_RETURN_VAL(fb->ENFN->gl_surface_unlock, EINA_FALSE);
 
    fb->backing = _rgba_image_alloc(fb, NULL);
-   fb->allocated = EINA_TRUE;
    EINA_SAFETY_ON_NULL_RETURN_VAL(fb->backing, EINA_FALSE);
 
-   ok = fb->ENFN->gl_surface_lock(fb->ENDT, fb->glimage);
+   ok = fb->ENFN->gl_surface_lock(fb->ENDT, glimage);
    if (!ok)
      {
         ERR("Failed to lock the image pixels");
         return EINA_FALSE;
      }
 
-   ok = fb->ENFN->gl_surface_read_pixels(fb->ENDT, fb->glimage,
+   ok = fb->ENFN->gl_surface_read_pixels(fb->ENDT, glimage,
                                          0, 0, fb->w, fb->h, fb->alpha_only
                                          ? EVAS_COLORSPACE_GRY8
                                          : EVAS_COLORSPACE_ARGB8888,
@@ -184,7 +166,7 @@ _filter_buffer_glimage_pixels_read(Evas_Filter_Buffer *fb)
    if (!ok)
      ERR("Could not read the image pixels!");
 
-   ok &= fb->ENFN->gl_surface_unlock(fb->ENDT, fb->glimage);
+   ok &= fb->ENFN->gl_surface_unlock(fb->ENDT, glimage);
    return ok;
 }
 
@@ -212,19 +194,6 @@ evas_filter_context_proxy_render_all(Evas_Filter_Context *ctx, Eo *eo_obj,
             {
                XDBG("Source already rendered: '%s' of type '%s'",
                    fb->source_name, eo_class_name_get(eo_class_get(fb->source)));
-               _filter_buffer_backing_free(fb);
-               if (!ctx->gl_engine)
-                 {
-                    fb->backing = source->proxy->surface;
-                    fb->allocated = EINA_FALSE;
-                 }
-               else
-                 {
-                    fb->glimage = source->proxy->surface;
-                    fb->allocated_gl = EINA_FALSE;
-                    _filter_buffer_glimage_pixels_read(fb);
-                 }
-               fb->alpha_only = EINA_FALSE;
             }
           else
             {
@@ -232,20 +201,16 @@ evas_filter_context_proxy_render_all(Evas_Filter_Context *ctx, Eo *eo_obj,
                    fb->source_name, eo_class_name_get(eo_class_get(fb->source)),
                    source->proxy->redraw ? "redraw" : "no surface");
                evas_render_proxy_subrender(ctx->evas->evas, fb->source, eo_obj, obj, do_async);
-               _filter_buffer_backing_free(fb);
-               if (!ctx->gl_engine)
-                 {
-                    fb->backing = source->proxy->surface;
-                    fb->allocated = EINA_FALSE;
-                 }
-               else
-                 {
-                    fb->glimage = source->proxy->surface;
-                    fb->allocated_gl = EINA_FALSE;
-                    _filter_buffer_glimage_pixels_read(fb);
-                 }
-               fb->alpha_only = EINA_FALSE;
             }
+          _filter_buffer_backing_free(fb);
+          if (!ctx->gl_engine)
+            fb->backing = ENFN->image_ref(ENDT, source->proxy->surface);
+          else
+            {
+               fb->glimage = ENFN->image_ref(ENDT, source->proxy->surface);
+               _filter_buffer_glimage_pixels_read(fb, fb->glimage);
+            }
+          fb->alpha_only = EINA_FALSE;
           XDBG("Source has dimensions %dx%d (buffer %d)", fb->w, fb->h, fb->id);
        }
 }
@@ -442,9 +407,7 @@ evas_filter_context_buffers_allocate_all(Evas_Filter_Context *ctx)
 
    EINA_LIST_FOREACH(ctx->buffers, li, fb)
      {
-        RGBA_Image *im;
-        im = fb->backing;
-        if (im || fb->source || fb->glimage)
+        if (fb->backing || fb->source || fb->glimage)
           continue;
 
         if (!fb->w && !fb->h)
@@ -454,11 +417,8 @@ evas_filter_context_buffers_allocate_all(Evas_Filter_Context *ctx)
           }
 
         XDBG("Allocating buffer of size %ux%u %s", fb->w, fb->h, fb->alpha_only ? "alpha" : "rgba");
-        im = _rgba_image_alloc(fb, NULL);
-        if (!im) goto alloc_fail;
-
-        fb->backing = im;
-        fb->allocated = (im != NULL);
+        fb->backing = _rgba_image_alloc(fb, NULL);
+        if (!fb->backing) goto alloc_fail;
      }
 
    return EINA_TRUE;
@@ -506,8 +466,7 @@ _filter_buffer_data_set(Evas_Filter_Context *ctx, int bufid, void *data,
    fb->h = h;
 
    fb->backing = _rgba_image_alloc(fb, data);
-   fb->allocated = (fb->backing != NULL);
-   return fb->allocated;
+   return (fb->backing != NULL);
 }
 
 int
@@ -516,6 +475,8 @@ evas_filter_buffer_image_new(Evas_Filter_Context *ctx, void *image)
    Evas_Filter_Buffer *fb;
 
    EINA_SAFETY_ON_NULL_RETURN_VAL(ctx, -1);
+
+   image = ENFN->image_ref(ENDT, image);
    EINA_SAFETY_ON_NULL_RETURN_VAL(image, -1);
 
    fb = calloc(1, sizeof(Evas_Filter_Buffer));
@@ -523,16 +484,10 @@ evas_filter_buffer_image_new(Evas_Filter_Context *ctx, void *image)
 
    fb->id = ++(ctx->last_buffer_id);
    fb->ctx = ctx;
-   if (!fb->ctx->gl_engine)
-     {
-        fb->backing = image;
-        fb->allocated = EINA_FALSE;
-     }
+   if (!ctx->gl_engine)
+     fb->backing = image;
    else
-     {
-        fb->glimage = image;
-        fb->allocated_gl = EINA_FALSE;
-     }
+     fb->glimage = image;
    ENFN->image_size_get(ENDT, image, &fb->w, &fb->h);
    fb->alpha_only = (ENFN->image_colorspace_get(ENDT, image)
                      == EVAS_COLORSPACE_GRY8);
@@ -602,47 +557,26 @@ evas_filter_buffer_backing_get(Evas_Filter_Context *ctx, int bufid)
 void *
 evas_filter_buffer_backing_steal(Evas_Filter_Context *ctx, int bufid)
 {
-   Evas_Filter_Buffer *buffer;
+   Evas_Filter_Buffer *fb;
 
    EINA_SAFETY_ON_NULL_RETURN_VAL(ctx, NULL);
 
-   buffer = _filter_buffer_get(ctx, bufid);
-   if (!buffer) return NULL;
-
-   // we don't hold any reference on this buffer anymore
-   buffer->stolen = EINA_TRUE;
+   fb = _filter_buffer_get(ctx, bufid);
+   if (!fb) return NULL;
 
    if (ctx->gl_engine)
-     return buffer->glimage;
-
-   return buffer->backing;
+     return fb->ENFN->image_ref(fb->ENDT, fb->glimage);
+   else
+     return fb->ENFN->image_ref(fb->ENDT, fb->backing);
 }
 
 Eina_Bool
 evas_filter_buffer_backing_release(Evas_Filter_Context *ctx,
                                    void *stolen_buffer)
 {
-   Evas_Filter_Buffer *fb;
-   Eina_List *li;
-
    if (!stolen_buffer) return EINA_FALSE;
    EINA_SAFETY_ON_NULL_RETURN_VAL(ctx, EINA_FALSE);
 
-   EINA_LIST_FOREACH(ctx->buffers, li, fb)
-     {
-        if ((fb->backing == stolen_buffer) || (fb->glimage == stolen_buffer))
-          {
-             fb->stolen = EINA_FALSE;
-             if (fb->delete_me)
-               {
-                  ctx->buffers = eina_list_remove_list(ctx->buffers, li);
-                  _buffer_free(fb);
-                  return EINA_TRUE;
-               }
-             return EINA_TRUE;
-          }
-     }
-
    if (ctx->async)
      evas_unref_queue_image_put(ctx->evas, stolen_buffer);
    else if (ctx->gl_engine)
@@ -1710,13 +1644,7 @@ evas_filter_image_draw(Evas_Filter_Context *ctx, void *draw_context, int bufid,
                                        EINA_TRUE, do_async);
         if (do_async && async_unref)
           {
-#ifdef EVAS_CSERVE2
-             if (evas_cserve2_use_get())
-               evas_cache2_image_ref((Image_Entry *)image);
-             else
-#endif
-               evas_cache_image_ref((Image_Entry *)image);
-
+             ENFN->image_ref(ENDT, image);
              evas_unref_queue_image_put(ctx->evas, image);
           }
      }
@@ -1726,10 +1654,7 @@ evas_filter_image_draw(Evas_Filter_Context *ctx, void *draw_context, int bufid,
 
         fb = _filter_buffer_get(ctx, bufid);
         _filter_buffer_backing_free(fb);
-        fb->glimage = image;
-        fb->allocated_gl = EINA_FALSE;
-        _filter_buffer_glimage_pixels_read(fb);
-        fb->glimage = NULL;
+        _filter_buffer_glimage_pixels_read(fb, image);
      }
 
    return EINA_TRUE;

diff --git a/src/lib/evas/filters/evas_filter_private.h b/src/lib/evas/filters/evas_filter_private.h
index 683e35a..5770972 100644 (file)
--- a/src/lib/evas/filters/evas_filter_private.h
+++ b/src/lib/evas/filters/evas_filter_private.h
@@ -237,11 +237,8 @@ struct _Evas_Filter_Buffer
    Evas_Object *proxy;
 
    Eina_Bool alpha_only : 1;  // 1 channel (A) instead of 4 (RGBA)
-   Eina_Bool allocated : 1;   // allocated on demand, belongs to this context
-   Eina_Bool allocated_gl : 1; // allocated on demand the glimage
    Eina_Bool transient : 1;   // temporary buffer (automatic allocation)
    Eina_Bool locked : 1;      // internal flag
-   Eina_Bool stolen : 1;      // stolen by the client
    Eina_Bool delete_me : 1;   // request delete asap (after released by client)
    Eina_Bool dirty : 1;       // Marked as dirty as soon as a command writes to it
 };