On 6 May 2016 at 21:09, Tom Hacohen <t...@osg.samsung.com> wrote:
> 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:
> > 0c2027b2af67f91dc4a203c177345e769ec07dec
>
>
> The 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:
>
It is wrong, yes, I know. The code was testing the safety of the API, not
demonstrating how it should behave exactly.
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 fell outside the intended use. Or rather, this is clearly a misuse,
just like keeping a eo_data_scope_get() pointer and messing with the eo
object references (directly or indirectly).
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.
>
Ok, so I'm refining the validity of the proxy object to be for a single
call. Like you proposed earlier. Not changing the implementation yet, only
the doc.
I guess all my "safety" mechanism and explaining how it's handled has
probably brought more confusion than anything else. Maybe my doc wasn't
clear either, although I admit that supporting multiple function calls and
eo_unref() was too ambitious (for the general case).
I fixed the usage in the test case, but kept the explicitely bad code to
still check for safety. It falls outside the scope of the API, though, so
we can break it.
I've also updated the wiki page.
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.
>
Just note that we have no way to disallow eo_ref (or do we??). unref/del
can trigger the del_callback for safety, caching or other purposes.
Anyway, raster has some new ideas (temp flag & maybe lock).
Thanks for your inputs, let's try to see if temp objects make more sense.
I'm also not a big fan of GC.
> >
> >
> >>
> >>>
> >>> 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
>
--
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
