On 29/04/16 03:32, Jean-Philippe André wrote:
> On 29 April 2016 at 00:51, Tom Hacohen <t...@osg.samsung.com> wrote:
>
>> On 28/04/16 06:25, Jean-Philippe André wrote:
>>> On 27 April 2016 at 23:44, Tom Hacohen <t...@osg.samsung.com> wrote:
>>>
>>>> On 26/04/16 06:28, Jean-Philippe André wrote:
>>>>> Hello,
>>>>>
>>>>>
>>>>> I've just merged a series of commits dealing with the box & table APIs
>>>> for
>>>>> Edje.Object and Elm.Layout. Since we decided not to implement anything
>>>> like
>>>>> eo_part at the core eo level, I've implemented part_box and part_table
>>>>> support using fake objects.
>>>>>
>>>>>
>>>>> This code:
>>>>>> elm_layout_table_blah(ly, "part", args);
>>>>>
>>>>> now becomes:
>>>>>> efl_pack_blah(efl_content_get(ly, "part"), args);
>>>>>
>>>>>
>>>>> The EO returned by efl_content_get is not a real Evas Object, it's
>> only a
>>>>> temporary proxy object that knows about its parent (ly) and the part
>> name
>>>>> it refers to ("part"). It is attached to the underlying Evas Box or
>> Table
>>>>> created by edje, and should live
>>>>>
>>>>> eo_del() is legal, just call efl_content_get() again to create a new
>>>> handle.
>>>>> eo_ref() is not a good idea.
>>>>>
>>>>>
>>>>> Note that efl_content_get() also returns real swallowed objects if the
>>>>> "part" is a SWALLOW.
>>>>>
>>>>>
>>>>> I believe text part APIs should eventually move to the same concept,
>> once
>>>>> the text interface is finalized (or, well, good enough). efl_text_set()
>>>> on
>>>>> a Layout object (or any Widget) should set the text of the "default"
>> part
>>>>> (whatever that means). Other parts can be accessed by
>> efl_content_get().
>>>>>
>>>>>
>>>>> Comments? Suggestions on how to improve this?
>>>>>
>>>>>
>>>>
>>>> Proxy objects were one of the original ideas to do eo_part. After that
>>>> we decided to do the more lightweight version I proposed and after that
>>>> it was rejected altogether.
>>>>
>>>> I don't like this solution because it's essentially what everyone
>>>> (raster?) said he didn't want to do. I prefer doing the part_* versions
>>>> (i.e double the functions), the normal versions (passing NULL as part
>>>> name) or maybe even an eo_part.
>>>>
>>>
>>> The part version means we should probably have a series of part_pack
>>> interfaces then.
>>> Or duplicate each and every API.
>>>
>>> Honestly, I'm not sure which solution is best. Proxy object or part APIs.
>>> But I believe the API looks pretty decent like this.
>>
>> I also like the part API, it's essentially the same API I suggested a
>> while back:
>>
>> efl_text_set(eo_part(obj, "bla"), "text"));
>>
>> Raster objected...
>>
>
> There's a difference between eo_part and the proxy object.
>
> With eo_part we would use a solution like eo_super(), which I have to say
> is not incredibly pretty (internally: spinlock and extra bit use). I
> believe that's the bit (sic) that raster didn't like much. Please correct
> me if I'm wrong.This is an implementation detail. Also, pretty or not, it works, tested, and we already rely on it, so what's wrong? > Anyway, I don't think parts belong to EO outside of EFL I think it's a cool thing, like composite objects. :) > > The proxy object is a real EO object so you can keep it around if you want. > It's always attached to its parent. > The internal Evas Object Box (or Edje_Real_Part that really is the part) is > the owner of the proxy object. > When the proxy dies (eo_del) then a del callback will remove the entry in > the owner. This way, content_get() does not return a dead object. > When the owner (Evas Box) dies, not only should it delete its children (the > proxy object), but it should also set the weak reference to NULL. That way, > even if a user called eo_ref() on a proxy object, internally the reference > to the original Edje or Elm layout is reset to NULL. Calls won't work, but > nothing should crash, and we can print nice error messages if we want. > > In terms of API, the advantage for both eo_part and proxy object is that it > looks like we have an object, and the API is the same for a normal box and > an edje box. > The advantage of proxy object over eo_part is that it's really an EO > object. It's just not a real canvas object. As described elsewhere in this thread, the problem is that with code like: efl_text_set(part(obj, "part"), "text"); The proxy object will never die. Given how we know no one will ever delete them, the proxy objects will just never die at all. That's bad. That's what I'm talking about with lifecycle. > > >> >> I don't like proxy object because of the implications I described about >> life-cycle, and I actually wonder about the implications of your >> solution for bindings. I think it just won't work because the bindings >> won't know which functions are available (with the part API, it's just >> part-enabled functions on the current class). >> > > The lifecycle is handled. If there's a bug then it's an implementation bug, > not a design bug (at least I believe so). > As I said before: If you delete the proxy after first usage, that is, only allow: efl_text_set(part(layout, "part"), "text"); Then you are fine, but then this: obj = part(layout, "part"); efl_text_set(obj, "text"); efl_text_get(obj); eo_del(obj); wouldn't work. You can't have both working, only one or the other. If you go with the first you have to delete after first use and the second will break, if you go with the second, the first will leak (proxy will remain alive until deleted explicitly, which it won't). Either way, it won't work. One way you could deal with it is implicitly unreffing after each call, so the first alternative would work, and make the second work like this: obj = eo_ref(part(layout, "part")); ... eo_unref(obj); > As for bindings, they need to be able to dynamically know which functions > an object support based on its class (here the object is an instance of a > class implementing Efl.Pack). Otherwise I don't see how any binding would > work at all. (In C++ I would cast to the efl_pack class, but that doesn't > work for Lua/JS). There are other options too for other languages, so actually I'm not too concerned about that, so don't mind this point. > > >> >> If you can convince raster, I'll happily go back to the original >> eo_part() solution. >> >>> >>> If we (or "people") really want the part APIs then I think we can just >> add >>> static inline / macros for C. >> >> Static inline, not macros. :) Yeah, that's the plan. >> >>> >>> Anyway, remember the scope of these proxy objects: Edje BOX and TABLE. >>> No one uses that. Proxy objects make the API behave the same as a >> swallowed >>> Box or Table. >>> >>> My biggest concern with this is the "eo_del() is legal, just call ..." >>>> paragraph. You need to describe the lifecycle properly and not leave it >>>> to chance because people will use it and you need to make sure they know >>>> what's allowed, what's not and what they should do. For example, it's >>>> not obvious to me if I get a ref out if I do content_get(). If I don't I >>>> risk the object being deleted underneath my feet. If I do, it becomes >>>> more of a PITA to deal with. Which one is it? >>>> >>> >>> You don't get a reference. The object keeps it. There is no own(). >>> content_get() creates an object if there is not one already, eo_del() >> nulls >>> out the reference inside the object. So another content_get() triggers a >>> new object creation. >>> >>> The proxy object can't die under your feet as it's bound to the main >>> object. Unless you eo_del() the container. >>> It's basically the same as eo_data_scope_get(). >>> >>> Is this a documentation issue or a design issue? >>> >> >> A design issue. >> > > Clearly this is a documentation issue, because your argument below is based > on assumptions. I showed above why it's not an assumption but a fact. > > > part = efl_content_get(obj, "bla"); >> >> how do you deal with the lifecycle of part? You mentioned eo_del(), if >> that's the case, you can't do: >> >> efl_text_set(efl_content_get(obj, "bla"), "test"); >> > > You can. > > Because it would never get deleted. You can only have one or the other >> unless you do some ugly magic (which you don't at the moment), so there >> is a problem there. >> > > Wrong assumption. This argument has been split a bit, please reply above, I've addressed and re-explained everything there. > > >> If it's kept alive and you don't del it at all you have a leak. If you >> del it implicitly under some conditions (you have an object pool of 5) >> it will die under the user's feet if they call it on more than 5 >> different parts. I'm talking about the "part = " option because that's >> the only relevant one for lifecycle. >> > > No pool. Each part represents a real object, so they can track each other's > death. > The "leak" side of it is that instead of just having an Edje_Real_Part > struct and an Evas_Object_Box EO you now also have a proxy EO in memory. > They all die together. > > > Finally, the thing is there was no clear consensus over the best solution > for part APIs. I implemented something that works NOW. And I'm pretty happy > with the result from a user point of view. > > What you implemented is what we'll be stuck with for all eternity, so "works NOW" is not good enough. :) -- Tom. ------------------------------------------------------------------------------ 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
