On 27/04/16 12:28, Carsten Haitzler wrote: > On Wed, 27 Apr 2016 10:35:29 +0100 Tom Hacohen <t...@osg.samsung.com> said: > >> On 26/04/16 22:59, Carsten Haitzler wrote: >>> On Tue, 26 Apr 2016 20:23:02 +0000 Mike Blumenkrantz >>> <michael.blumenkra...@gmail.com> said: >>> >>>> On Tue, Apr 26, 2016 at 2:38 PM Carsten Haitzler <ras...@rasterman.com> >>>> wrote: >>>> >>>>> On Tue, 26 Apr 2016 18:55:08 +0100 Tom Hacohen <t...@osg.samsung.com> >>>>> said: >>>>> >>>>>> On 26/04/16 18:48, Mike Blumenkrantz wrote: >>>>>>> To further elaborate, this currently breaks two known things in >>>>>>> Enlightenment: >>>>>>> * notifications now trigger valgrind errors upon deletion >>>>>>> * using the menu to activate desklock crashes the session >>>>>>> - I put in a small hack to avoid crashing for now, but it still >>>>> leaves >>>>>>> artifacts >>>>>>> >>>>>>> I would guess that there are many other things which also break since >>>>> the >>>>>>> cause of this breakage is this new hide. >>>>>>> >>>>>>> This change would effectively force me to have to rewrite the >>>>> compositor >>>>>>> object management system. It has also broken every single Enlightenment >>>>>>> release which has shipped since 2013. I am strongly opposed to leaving >>>>> this >>>>>>> in, and I would revert it right now if I wasn't reasonably certain >>>>> that my >>>>>>> revert would also be reverted. >>>>>> >>>>>> I would happily revert the revert of your revert if needed. >>>>>> >>>>>> So according to you, this change breaks the past and the present. >>>>>> According to me this change is bad for the present and the future. >>>>>> >>>>>> Carsten, could you please weigh in as to why this is at all wanted while >>>>>> addressing both Mike's and my concerns? >>>>> >>>>> talk with netstar - he had bugs in e EXACTLY because of dels not hiding >>>>> objects. the lifetime of an object shouldn't be tied to its visibility. >>>>> the visibility CAN be affected lifetime though. i would argue then >>>>> something is deeply wrong in the compositor object system code if this >>>>> causes bugs. >>>>> >>>>> if the code did an evas_object_hide() right before the del() it would then >>>>> also >>>>> break, so is already broken by design, and it worked by luck. >>>>> >>>> >>>> I think you may have read my mail hastily and missed my point about the >>>> timing being relevant here. The reason this breaks is because it hides >>>> objects on the call to evas_object_del() instead of on destruction. >>> >>> objects SHOULD hide on del. refs shouldn't keep them visible. this causes >>> actual bugs. netstar was reporting them on #edevelop right before my commit >>> there. >>> >> >> This assumes a "one manager" kind of thing. Which essentially means that >> we ignore refcounting for the lifecycle of evas objects. It's weird. Why > > no - a ref is a reference. there is an OWNER though. references are not > ignored > - they keep the object alive to data can be found/accessed etc. until the > reference is given up. but this reference shouldn't keep it visible when the > owner has said "i'm done with you" - eg the parent.
I just checked, and as I remembered, Mike is right. Since the dawn of time a ref would have prevented an object deletion. Check the commit where you introduced object reference in 2011: b893963ee87ff92b0b9a8ebce168b45d8cb4b423. So your definition of del is not only in disagreement with me, but with past you and EFL itself. > >> not have people do what they mean, that is hide it when they want to? >> I'm fine with making evas_object_del() behave "like it should" for >> legacy compat (though Mike disagrees with you on what that version may >> look like), but going on to the future, it doesn't make sense to me. > > then why do we have eo_del at all? nuke it. eo_ref and unref. we have been > through this before del is different to unref. I don't like eo_del, but it's a counterpart to eo_add(). It actually started as something better, it started as an automatic unref + unparent so you'd automatically give up on your ref and get rid of the parent if there is one. This made sense. However since the changes to how we deal with refs in eo. More specifically the request that eo_add doesn't lead to a ref, it was changed to be the counterpart of eo_add(). > >> -- >> Tom. >> >>>> I realize you're busy at the moment, so I've taken some time and pushed a >>>> small change to this which matches the pre-eo behavior ( >>>> https://git.enlightenment.org/legacy/evas.git/tree/src/lib/canvas/evas_object_main.c#n439) >>>> for you. >>>> >>>> >>>>> >>>>>> Thanks. >>>>>> >>>>>>> >>>>>>> On Tue, Apr 26, 2016 at 1:42 PM Tom Hacohen <t...@osg.samsung.com> >>>>> wrote: >>>>>>> >>>>>>>> That is exactly what I said. Carsten also wants to introduce this in >>>>> Eo. >>>>>>>> I don't like it. >>>>>>>> >>>>>>>> We finally have a proper object lifetime with refcounting, destructors >>>>>>>> and what not. The object shouldn't die unless it's dead. If for some >>>>>>>> reason you want to hide it immediately when killing it, by all means >>>>> do >>>>>>>> it, but don't make it the awkward norm that deviates from the clean >>>>> life >>>>>>>> cycle. >>>>>>>> >>>>>>>> -- >>>>>>>> Tom. >>>>>>>> >>>>>>>> On 26/04/16 18:11, Mike Blumenkrantz wrote: >>>>>>>>> This commit broke things by triggering an immediate hide call on >>>>> objects >>>>>>>>> which were expecting to receive a deferred call. For example, any >>>>> time >>>>>>>>> evas_object_ref() is used to preserve an object lifetime and then >>>>>>>>> evas_object_del() is called before the unref, the object will now be >>>>>>>> hidden >>>>>>>>> regardless of user intent. >>>>>>>>> >>>>>>>>> This seems wrong to me; hiding during the object destructor is fine, >>>>> but >>>>>>>>> enforcing hide on delete (which may or may not result in immediate >>>>>>>>> destruction) will cause unwanted behaviors. >>>>>>>>> >>>>>>>>> On Tue, Apr 26, 2016 at 12:46 PM Carsten Haitzler < >>>>> ras...@rasterman.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> On Tue, 26 Apr 2016 16:24:08 +0000 Mike Blumenkrantz >>>>>>>>>> <michael.blumenkra...@gmail.com> said: >>>>>>>>>> >>>>>>>>>>> FYI this broke a LOT of things in enlightenment, and I'd guess it >>>>> will >>>>>>>>>> also >>>>>>>>>>> break a lot of user applications too. >>>>>>>>>> >>>>>>>>>> the commit or fix broke thigns or the fact that things were not >>>>> hidde >>>>>>>>>> before >>>>>>>>>> on object_del due to the move to eo? >>>>>>>>>> >>>>>>>>>>> On Sat, Apr 23, 2016 at 10:07 AM Carsten Haitzler < >>>>>>>> ras...@rasterman.com> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> raster pushed a commit to branch master. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>> http://git.enlightenment.org/core/efl.git/commit/?id=df2b31b63eaed894601ba8126d1f43f07edb6332 >>>>>>>>>>>> >>>>>>>>>>>> commit df2b31b63eaed894601ba8126d1f43f07edb6332 >>>>>>>>>>>> Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com> >>>>>>>>>>>> Date: Sat Apr 23 23:06:13 2016 +0900 >>>>>>>>>>>> >>>>>>>>>>>> evas - legacy evas_object_del - always hide obj regardless >>>>> of >>>>>>>> refs >>>>>>>>>>>> >>>>>>>>>>>> if an object iot reffed or not hide on del. it should have >>>>> been >>>>>>>>>> this >>>>>>>>>>>> way before eo. eoifications i think messed a few things up. >>>>>>>>>>>> >>>>>>>>>>>> this does bring up an issue... in eo we have no way to >>>>> explicitly >>>>>>>>>> do >>>>>>>>>>>> stuff on eo_del regardless of references at the time. this >>>>> needs >>>>>>>>>> to be >>>>>>>>>>>> solved. >>>>>>>>>>>> >>>>>>>>>>>> @fix >>>>>>>>>>>> --- >>>>>>>>>>>> src/lib/evas/canvas/evas_object_main.c | 1 + >>>>>>>>>>>> 1 file changed, 1 insertion(+) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/src/lib/evas/canvas/evas_object_main.c >>>>>>>>>>>> b/src/lib/evas/canvas/evas_object_main.c >>>>>>>>>>>> index 6a1983f..8b9710a 100644 >>>>>>>>>>>> --- a/src/lib/evas/canvas/evas_object_main.c >>>>>>>>>>>> +++ b/src/lib/evas/canvas/evas_object_main.c >>>>>>>>>>>> @@ -725,6 +725,7 @@ evas_object_del(Evas_Object *eo_obj) >>>>>>>>>>>> Evas_Object_Protected_Data *obj = eo_data_scope_get(eo_obj, >>>>>>>>>> MY_CLASS); >>>>>>>>>>>> >>>>>>>>>>>> if (!obj) return; >>>>>>>>>>>> + evas_object_hide(eo_obj); >>>>>>>>>>>> evas_object_async_block(obj); >>>>>>>>>>>> if (obj->delete_me || obj->eo_del_called) return; >>>>>>>>>>>> if (obj->ref > 0) >>>>>>>>>>>> >>>>>>>>>>>> -- >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>> ------------------------------------------------------------------------------ >>>>>>>>>>> Find and fix application performance issues faster with >>>>> Applications >>>>>>>>>> Manager >>>>>>>>>>> Applications Manager provides deep performance insights into >>>>> multiple >>>>>>>>>> tiers of >>>>>>>>>>> your business applications. It resolves application problems >>>>> quickly >>>>>>>> and >>>>>>>>>>> reduces your MTTR. Get your free trial! >>>>>>>>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>>>>>>>> _______________________________________________ >>>>>>>>>>> enlightenment-devel mailing list >>>>>>>>>>> enlightenment-devel@lists.sourceforge.net >>>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> ------------- Codito, ergo sum - "I code, therefore I am" >>>>> -------------- >>>>>>>>>> The Rasterman (Carsten Haitzler) ras...@rasterman.com >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>> ------------------------------------------------------------------------------ >>>>>>>>> Find and fix application performance issues faster with Applications >>>>>>>> Manager >>>>>>>>> Applications Manager provides deep performance insights into multiple >>>>>>>> tiers of >>>>>>>>> your business applications. It resolves application problems quickly >>>>> and >>>>>>>>> reduces your MTTR. Get your free trial! >>>>>>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>>>>>> _______________________________________________ >>>>>>>>> enlightenment-devel mailing list >>>>>>>>> enlightenment-devel@lists.sourceforge.net >>>>>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>> ------------------------------------------------------------------------------ >>>>>>>> Find and fix application performance issues faster with Applications >>>>>>>> Manager >>>>>>>> Applications Manager provides deep performance insights into multiple >>>>>>>> tiers of >>>>>>>> your business applications. It resolves application problems quickly >>>>> and >>>>>>>> reduces your MTTR. Get your free trial! >>>>>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>>>>> _______________________________________________ >>>>>>>> enlightenment-devel mailing list >>>>>>>> enlightenment-devel@lists.sourceforge.net >>>>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>>>>> >>>>>>> >>>>> ------------------------------------------------------------------------------ >>>>>>> Find and fix application performance issues faster with Applications >>>>> Manager >>>>>>> Applications Manager provides deep performance insights into multiple >>>>> tiers >>>>>>> of your business applications. It resolves application problems >>>>> quickly and >>>>>>> reduces your MTTR. Get your free trial! >>>>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>>>> _______________________________________________ >>>>>>> enlightenment-devel mailing list >>>>>>> enlightenment-devel@lists.sourceforge.net >>>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>>>> >>>>>> >>>>>> >>>>>> >>>>> ------------------------------------------------------------------------------ >>>>>> Find and fix application performance issues faster with Applications >>>>> Manager >>>>>> Applications Manager provides deep performance insights into multiple >>>>> tiers of >>>>>> your business applications. It resolves application problems quickly and >>>>>> reduces your MTTR. Get your free trial! >>>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>>> _______________________________________________ >>>>>> enlightenment-devel mailing list >>>>>> enlightenment-devel@lists.sourceforge.net >>>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>>> >>>>> >>>>> >>>>> -- >>>>> ------------- Codito, ergo sum - "I code, therefore I am" -------------- >>>>> The Rasterman (Carsten Haitzler) ras...@rasterman.com >>>>> >>>>> >>>>> >>>>> ------------------------------------------------------------------------------ >>>>> Find and fix application performance issues faster with Applications >>>>> Manager >>>>> Applications Manager provides deep performance insights into multiple >>>>> tiers of >>>>> your business applications. It resolves application problems quickly and >>>>> reduces your MTTR. Get your free trial! >>>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>>> _______________________________________________ >>>>> enlightenment-devel mailing list >>>>> enlightenment-devel@lists.sourceforge.net >>>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>>> >>>> ------------------------------------------------------------------------------ >>>> Find and fix application performance issues faster with Applications >>>> Manager Applications Manager provides deep performance insights into >>>> multiple tiers of your business applications. It resolves application >>>> problems quickly and reduces your MTTR. Get your free trial! >>>> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >>>> _______________________________________________ >>>> enlightenment-devel mailing list >>>> enlightenment-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >>>> >>> >>> >> >> >> ------------------------------------------------------------------------------ >> Find and fix application performance issues faster with Applications Manager >> Applications Manager provides deep performance insights into multiple tiers >> of >> your business applications. It resolves application problems quickly and >> reduces your MTTR. Get your free trial! >> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >> _______________________________________________ >> enlightenment-devel mailing list >> enlightenment-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> > > ------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
