On Tue, 18 Jul 2017 21:57:23 -0700 Jean-Philippe ANDRÉ <j...@videolan.org> said:
After much bisecting late into the evening... this patch broke enlightenment and makes it reliably crash when you run virtualbox... in texture uploading a seemingly garbage pointer in the image data... at least for now this is a note to "look into this" ... filing > jpeg pushed a commit to branch master. > > http://git.enlightenment.org/core/efl.git/commit/?id=45c8e5e983d24b23b244513087799edcda862411 > > commit 45c8e5e983d24b23b244513087799edcda862411 > Author: Jean-Philippe Andre <jp.an...@samsung.com> > Date: Wed Jul 19 11:43:35 2017 +0900 > > evas: Fix support for image_data_get on snapshot > > evas_object_image_data_get() is a legacy API that I made work > with snapshot objects (evas_object_image_snapshot_set()). Some > changes in the engine broke the behaviour and this patch fixes > it. > > When getting the pixels from an FBO, in read-only mode, we need > to create a temporary image (pixels surface) that contains the > copy of the pixels we get from glReadPixels. This image needs > to be deleted afterwards. It is thus stored by the image object > and freed upon _image_data_set() (good) or object deletion (bad). > > FBO + read-write is not supported by this API (it is supported > through buffer_map as the filters had to use that). > > Fixes T5754 > --- > src/lib/evas/canvas/evas_image_legacy.c | 84 ++++++++++++++++++++++++++ > +----- src/lib/evas/canvas/evas_image_private.h | 1 + > src/lib/evas/canvas/evas_object_image.c | 7 +++ > 3 files changed, 79 insertions(+), 13 deletions(-) > > diff --git a/src/lib/evas/canvas/evas_image_legacy.c > b/src/lib/evas/canvas/evas_image_legacy.c index a8593db29b..3f69dca4e6 100644 > --- a/src/lib/evas/canvas/evas_image_legacy.c > +++ b/src/lib/evas/canvas/evas_image_legacy.c > @@ -14,6 +14,14 @@ > EVAS_IMAGE_API(_o, __VA_ARGS__); \ > } while (0) > > +typedef struct _Evas_Image_Legacy_Pixels_Entry > Evas_Image_Legacy_Pixels_Entry; + > +struct _Evas_Image_Legacy_Pixels_Entry > +{ > + Eo *object; > + void *image; > +}; > + > EAPI Evas_Object * > evas_object_image_add(Evas *eo_e) > { > @@ -552,7 +560,7 @@ evas_object_image_data_set(Eo *eo_obj, void *data) > > Evas_Object_Protected_Data *obj = efl_data_scope_get(eo_obj, > EFL_CANVAS_OBJECT_CLASS); Evas_Image_Data *o = efl_data_scope_get(eo_obj, > EFL_CANVAS_IMAGE_INTERNAL_CLASS); > - void *p_data; > + void *p_data, *pixels; > Eina_Bool resize_call = EINA_FALSE; > > > @@ -563,6 +571,13 @@ evas_object_image_data_set(Eo *eo_obj, void *data) > p_data = o->engine_data; > if (data) > { > + // r/o FBO data_get: only free the image, don't update pixels > + if ((pixels = eina_hash_find(o->pixels->images_to_free, data)) != > NULL) > + { > + eina_hash_del(o->pixels->images_to_free, data, pixels); > + return; > + } > + > if (o->engine_data) > { > o->engine_data = ENFN->image_data_put(ENDT, o->engine_data, > data); @@ -640,14 +655,28 @@ evas_object_image_data_set(Eo *eo_obj, void > *data) if (resize_call) evas_object_inform_call_image_resize(eo_obj); > } > > +static void > +_image_to_free_del_cb(void *data) > +{ > + Evas_Image_Legacy_Pixels_Entry *px_entry = data; > + Evas_Object_Protected_Data *obj; > + > + obj = efl_data_scope_safe_get(px_entry->object, EFL_CANVAS_OBJECT_CLASS); > + EINA_SAFETY_ON_NULL_RETURN(obj); > + ENFN->image_free(ENDT, px_entry->image); > + free(px_entry); > +} > + > EAPI void* > evas_object_image_data_get(const Eo *eo_obj, Eina_Bool for_writing) > { > EVAS_IMAGE_API(eo_obj, NULL); > > Evas_Image_Data *o = efl_data_scope_get(eo_obj, > EFL_CANVAS_IMAGE_INTERNAL_CLASS); > + Evas_Image_Legacy_Pixels_Entry *px_entry = NULL; > + Eina_Bool tofree = 0; > + void *pixels = NULL; > int stride = 0; > - void *pixels; > DATA32 *data; > > if (!o->engine_data) return NULL; > @@ -662,25 +691,48 @@ evas_object_image_data_get(const Eo *eo_obj, Eina_Bool > for_writing) ENFN->image_scale_hint_set(ENDT, o->engine_data, o->scale_hint); > if (ENFN->image_content_hint_set) > ENFN->image_content_hint_set(ENDT, o->engine_data, o->content_hint); > - pixels = ENFN->image_data_get(ENDT, o->engine_data, for_writing, &data, > &o->load_error, NULL); > + pixels = ENFN->image_data_get(ENDT, o->engine_data, for_writing, &data, > &o->load_error, &tofree); > /* if we fail to get engine_data, we have to return NULL */ > if (!pixels) return NULL; > > - o->engine_data = pixels; > - if (ENFN->image_stride_get) > - ENFN->image_stride_get(ENDT, o->engine_data, &stride); > - else > - stride = o->cur->image.w * 4; > + if (!tofree) > + { > + o->engine_data = pixels; > + if (ENFN->image_stride_get) > + ENFN->image_stride_get(ENDT, o->engine_data, &stride); > + else > + stride = o->cur->image.w * 4; > > - if (o->cur->image.stride != stride) > + if (o->cur->image.stride != stride) > + { > + EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write) > + state_write->image.stride = stride; > + EINA_COW_IMAGE_STATE_WRITE_END(o, state_write); > + } > + > + o->pixels_checked_out++; > + } > + else > { > - EINA_COW_IMAGE_STATE_WRITE_BEGIN(o, state_write) > - state_write->image.stride = stride; > - EINA_COW_IMAGE_STATE_WRITE_END(o, state_write); > + Eina_Hash *hash = o->pixels->images_to_free; > + > + if (!hash) > + { > + hash = eina_hash_pointer_new(_image_to_free_del_cb); > + if (!hash) goto error; > + EINA_COW_PIXEL_WRITE_BEGIN(o, pixi_write) > + pixi_write->images_to_free = hash; > + EINA_COW_PIXEL_WRITE_END(o, pixi_write); > + } > + > + px_entry = calloc(1, sizeof(*px_entry)); > + px_entry->object = (Eo *) eo_obj; > + px_entry->image = pixels; > + if (!eina_hash_add(hash, data, px_entry)) > + goto error; > } > > - o->pixels_checked_out++; > if (for_writing) > { > o->written = EINA_TRUE; > @@ -688,6 +740,12 @@ evas_object_image_data_get(const Eo *eo_obj, Eina_Bool > for_writing) } > > return data; > + > +error: > + free(px_entry); > + if (tofree && pixels) > + ENFN->image_free(ENDT, pixels); > + return NULL; > } > > EAPI void > diff --git a/src/lib/evas/canvas/evas_image_private.h > b/src/lib/evas/canvas/evas_image_private.h index 67cfee41d1..91c7e22af1 100644 > --- a/src/lib/evas/canvas/evas_image_private.h > +++ b/src/lib/evas/canvas/evas_image_private.h > @@ -61,6 +61,7 @@ struct _Evas_Object_Image_Pixels > Evas_Object_Image_Pixels_Get_Cb get_pixels; > void *get_pixels_data; > } func; > + Eina_Hash *images_to_free; /* pixel void* -> > Evas_Image_Legacy_Pixels_Entry */ > Evas_Video_Surface video; > unsigned int video_caps; > diff --git a/src/lib/evas/canvas/evas_object_image.c > b/src/lib/evas/canvas/evas_object_image.c index c522742d7a..5506290ba6 100644 > --- a/src/lib/evas/canvas/evas_object_image.c > +++ b/src/lib/evas/canvas/evas_object_image.c > @@ -1511,6 +1511,13 @@ evas_object_image_free(Evas_Object *eo_obj, > Evas_Object_Protected_Data *obj) efl_del(o->file_obj); > o->file_obj = NULL; > } > + if (o->pixels->images_to_free) > + { > + eina_hash_free(o->pixels->images_to_free); > + EINA_COW_PIXEL_WRITE_BEGIN(o, pixi_write) > + pixi_write->images_to_free = NULL; > + EINA_COW_PIXEL_WRITE_END(o, pixi_write); > + } > if (o->pixels->pixel_updates) > { > EINA_COW_PIXEL_WRITE_BEGIN(o, pixi_write) > > -- > > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
