On 29/04/16 05:36, Jean-Philippe André wrote:
> Hi Tom,
>
> On 29 April 2016 at 11:32, Jean-Philippe André <j...@videolan.org> 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.
>> Anyway, I don't think parts belong to EO outside of EFL
>>
>> 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.
>>
>
> I pushed a commit testing and showing off the proxy object lifecycle:
> 0c2027b2af67f91dc4a203c177345e769ec07decThe lifecycle is not that good in that commit, I spotted a few issues. Take a look at my email from 11:50 UTC, I think the scheme described there will solve them all. Just a few examples: table = efl_content_get(obj, "table"); fail_if(!table); other = efl_content_get(obj, "table"); fail_if(other != table); fail_if(eo_ref_get(other) != 1); That is wrong, consider the following code: table = efl_content_get(obj, "table"); a(table); trouble(obj); b(table); eo_del(table); The implementation of "trouble": table2 = efl_content_get(obj, "table"); a(table2); eo_del(table2); This means that the reference to table will be gone, and the proxy object will be dead by the time you get to "b(table);". As I said a few times already, with this scheme *THE PROXY OBJECT STAYS ALIVE* until the object dies or you do something ugly like clear on the first loop run. I prefer the scheme I described, but another (a bit less) acceptable version would be to take the GC approach. That is: don't allow the users to ref/unref/del the parts and just have an idler job to do it on the next loop run. I prefer the scheme I dsecribed in the other email better, but this would be acceptable too, and maybe even be nicer to use. > > >> >>> >>> 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 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). >> >> >>> >>> 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. >> >> >> 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. >> >> >>> 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. >> >> >> -- >> Jean-Philippe André >> > > > ------------------------------------------------------------------------------ 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
