Page MenuHomePhabricator

Refactoring Edje/Elm_Layout
Open, HighPublic

Description

Edje and Elm_Layout do share quite a lot of interface. They should have a common interface and duplication should be avoided.

List of things that would make sense :

  • Provide an Efl.Canvas.Layout.Signal (For everything related to signal).
  • Message API should be legacy only as we will rely on style to do that for bindings.
  • Edje should become Efl.Canvas.Layout.
  • Elm.Layout should be renamed Efl.Ui.Layout.
  • Efl.Ui.Layout should compose with Efl.Canvas.Layout.

Details

Commits
Restricted Diffusion Commit
Restricted Diffusion Commit
rEFLfa786667b8c9: edje: Rename events according to recent rename
rEFL73e3a82bc204: edje: Move perspective to legacy
rEFL8faa65d90b5e: edje: Move text_change_cb to legacy only
rEFL6225293d2ec7: edje: Move "preload" to legacy only
rEFL19dff855194f: edje_object: Mark access_part_iterate as @beta
rEFLa97329482059: edje: Rename part external class
rEFLd5a31f3f307c: edje/elm: Rename _internal_ to _part_ (EO)
rEFL3add24fa45b3: elm: Continue elm_layout renaming
rEFL9a2d4928f09c: elm: Rename elm_layout to Efl.Ui.Layout
rEFLf79960839d22: layout: Implement missing edje APIs
rEFL6864495c993c: elm: Move elm_layout_sizing_eval to legacy
rEFL4c8f87974c67: layout: Implement data_get from edje
rEFL240cc9e5012a: edje: Move size_min/max and data to an interface
rEFL3e5cfb83c092: elm: Remove custom layout signal APIs
rEFLefac7d523ac4: edje: Move signal APIs to an interface
rEFLad7e16bcf4af: layout: Use only legacy "elm_layout_sizing_eval"
rEFLfb941c457c9c: layout: Improve doc for theme_set
rEFL82e610032229: layout: Move Part_Alias struct to legacy
rEFLf0730f6f4ce7: layout: Move edje_get to legacy only
rEFLaebd37cab88c: layout: Remove sub_object_add_enable
rEFLa4940ae6c980: layout: Remove method theme_enable
rEFLa4c392989400: layout: Implement cursor part APIs with efl_part
rEFLaeacb54c35a6: cursor: EO-ify elm_cursor API
rEFL6df17b2ed00b: edje: Split off calc APIs to an interface (EO)
rEFL796d4b138ba3: elm: Make content and text aliases internal only
rEFL07a25fc88ce2: edje: Move base_scale to Efl.Ui.Base
rEFL602039cf836d: edje: Move some functions to Efl.Ui.Base (EO)
rEFLce0d15ecb330: edje: Reshuffle a bit edje_object.eo
rEFLaa5ac8eed71f: edje: Rename "data" to "group_data" for EO
cedric created this task.Mar 30 2017, 2:00 AM
conr2d added a subscriber: conr2d.May 1 2017, 11:19 PM

@cedric
Does "Efl.Ui.Layout should compose with Efl.Canvas.Layout." mean efl_composite_attach()? When I wrote patch for that, subclasses inheriting from Elm.Layout cannot override Edje.Object methods.

The signal API needs to be discussed with @singh.amitesh so we can find the right balance between a specific signal API and a more generic asynchronous message API (with payload, eg. for edje messages).

jpeg added a comment.May 11 2017, 8:29 PM
In T5315#86266, @conr2d wrote:

@cedric
Does "Efl.Ui.Layout should compose with Efl.Canvas.Layout." mean efl_composite_attach()? When I wrote patch for that, subclasses inheriting from Elm.Layout cannot override Edje.Object methods.

I'm afraid composite is a shortcut that we can not take here. Inheritance of the interface and manual implementation might be the way to go. Most functions in elm_layout will be just a simple call forward.

jpeg added a comment.May 26 2017, 1:57 AM
In T5315#86906, @jpeg wrote:
In T5315#86266, @conr2d wrote:

@cedric
Does "Efl.Ui.Layout should compose with Efl.Canvas.Layout." mean efl_composite_attach()? When I wrote patch for that, subclasses inheriting from Elm.Layout cannot override Edje.Object methods.

I'm afraid composite is a shortcut that we can not take here. Inheritance of the interface and manual implementation might be the way to go. Most functions in elm_layout will be just a simple call forward.

Ok so after thinking a bit more and talking with @cedric, composition may be the way to go. According to @conr2d there might be some API clash of some sort. Let's figure out what the exact problems are and whether this is really relevant. We have no Elm.Layout child class that overrides Edje.Object methods at the moment (Efl.Ui.Image is not a layout but overrides Edje.Object functions).
But the most important point is that we get rid of the internal object API (I have a pending patch).

In T5315#86904, @jpeg wrote:

The signal API needs to be discussed with @singh.amitesh so we can find the right balance between a specific signal API and a more generic asynchronous message API (with payload, eg. for edje messages).

The signal callback API will be manually bound. This means 2 extra APIs to bind manually: signal_cb_add and del. signal_emit does not require manual binding as it only deals with strings.
We also don't need a generic infrastructure to send async messages right now. Edje will keep its current implementation for the foreseable future. Anyway it is bound to what EDC supports.

jpeg added a comment.EditedMay 26 2017, 2:16 AM

Now in Edje.Object we have the following methods:

  1. No problem:

Mirrored, language (may need common interface? but which one?)
Seat functions (specific to edje object - all good)

  1. signal & message functions

Signal cb functions need manual binding
There is no EO API to listen to messages from EDC but it could easily be implemented as an event)

  1. Specific layout & calc functions

Maybe need an interface? Maybe not? Some of those functions would not be shared with other objects - specific to edje objects.
They might need a prefix like "layout_" or "calc_" so that they don't disturb too much autocompletion.

  1. Text

This needs to be done. And sync'ed with the Text EO API. @herdsman are you working on this? Should I just do the Efl.Part implementation and then let you deal with pure text stuff? You might even be able to do a lot of the work using composition inside the efl_part implementation.

  1. Missing global functions

edje_file/mmap_data_get and such

  1. Color/text/size classes

@conr2d I think we are still missing some of your patches

  1. Preload, load_error, file_set

Once the futures work well, the proper common file API needs to be implemented. load_error then becomes irrelevant (future error cb).

  1. animation vs. play

The documentation is not clear. My understanding is that "animation" stops animations from happening, in the sense that transitions are now instant state changes. But programs still run. Disabling "play" means that programs don't run. The edje is frozen in time (not sure how resizing affects its parts).

  1. part drag APIs

need an interface and efl_part

  1. part external APIs

need efl_part and maybe an interface (TBD as an interface means the widgets themselves should implement it).
part_external_content may not be required

In T5315#87811, @jpeg wrote:
  1. Text This needs to be done. And sync'ed with the Text EO API. @herdsman are you working on this? Should I just do the Efl.Part implementation and then let you deal with pure text stuff? You might even be able to do a lot of the work using composition inside the efl_part implementation.

I have done it. Will reference my branch with the changes here for your approval.

jpeg added a comment.May 28 2017, 9:32 PM
In T5315#87811, @jpeg wrote:
  1. Text This needs to be done. And sync'ed with the Text EO API. @herdsman are you working on this? Should I just do the Efl.Part implementation and then let you deal with pure text stuff? You might even be able to do a lot of the work using composition inside the efl_part implementation.

I have done it. Will reference my branch with the changes here for your approval.

Ah. Then I hope my recent changes are not going to impact your merge. Let me know if you need any help, I'd like to rid edje object of all the text APIs asap.

cedric raised the priority of this task from TODO to High.Jul 10 2017, 3:07 PM
jpeg claimed this task.Aug 2 2017, 2:37 AM

Already working on it. Almost done.

jpeg added a comment.Aug 3 2017, 2:33 AM

I found some massively problematic APIs:

  • sizing_eval
  • sizing_restricted_eval

sizing_restricted_eval is actually quite fine, as it's not implemented anywhere outside of elm_layout.c
sizing_eval is a massive problem as its definition is:

  • #1. Mark an object as requiring recalc, but don't do anything now (async).

This is similar to Efl.Canvas.Group.group_need_recalculate.set.

The problem is that some widgets do:

  • #2. Calculate min/max/aspect size hints synchronously.

Or even:

  • #3. Calculate size hints and geometry synchronously.

Some widgets even implement Efl.Canvas.Group.group_calculate along with sizing_eval. The exact behavior from the caller point of view is completely undefined: was the object just flagged (1)? were the hints updated (2)? or was the object moved & resized (3)?

elm_layout_sizing_eval() is used a lot throughout EFL and by applications...

jpeg added a comment.Aug 3 2017, 6:36 PM

There is no EO binding for edje file APIs.

jpeg added a comment.Aug 7 2017, 9:45 PM

I would say that Efl.Ui.Layout is now done, with two exceptions:

  1. It doesn't implement all the functions in its API (ie. some interfaces are partially implemented), and doesn't provide a way to get the internal Edje object
  2. It inherits from Elm.Widget which is not done at all: See T5363
jpeg added a comment.Aug 7 2017, 11:50 PM

@herdsman in Edje.Object there are still a lot of remaining text part APIs.

jpeg added a comment.Sep 12 2017, 10:02 PM

@herdsman in Edje.Object there are still a lot of remaining text part APIs.

This is still true to this day.

jpeg added a comment.Oct 12 2017, 1:25 AM

This is now mostly done! Remaining to-do:

edje_object:

  • Rename Edje.Object to Efl.Canvas.Layout (@jpeg)
  • Two "text" APIs remain, including a function cb: text_change_cb, item_provider (@herdsman)
  • All the Font/Size/Color class interfaces need to be finalized (@conr2d)
  • Sync/Async file interface needs to be done (@cedric)

elm_layout:

  • theme.set API may change if the theme structure is specifically adapted to EO
jpeg added a comment.Dec 6 2017, 5:18 PM

commit 54ae9cc18b6c0fdb9a10f03ad2681587ca062c6c
Author: Amitesh Singh <amitesh.sh@samsung.com>
Date: Wed Dec 6 12:15:39 2017 +0900

edje: rename Edje.Object to Efl.Canvas.Layout

commit bdc396945281290c3d4b0622d9f9a3945af01097
Author: Amitesh Singh <amitesh.sh@samsung.com>
Date: Tue Dec 5 16:00:08 2017 +0900

edje: rename intf Efl.Canvas.Layout_Group to Efl.Layout.Group

commit c7aa3b2f83bfed886cf5bcd53dea21d0c9d11a99
Author: Amitesh Singh <amitesh.sh@samsung.com>
Date: Tue Dec 5 15:29:07 2017 +0900

edje: rename intf Efl.Canvas.Layout_Calc to Efl.Layout.Calc

commit 7b3fde4d4b9047b39a1dc0c7794500364f689325
Author: Amitesh Singh <amitesh.sh@samsung.com>
Date: Tue Dec 5 14:39:20 2017 +0900

edje: rename intf Efl.Canvas.Layout.Signal to Efl.Layout.Signal

Still missing the landing of @herdsman branch remotes/origin/devs/herdsman/annotation_item which still has a few issue. I might take over and finish the last bit as their isn't much there left.

jpeg reassigned this task from jpeg to herdsman.Jan 17 2018, 11:50 PM
jpeg edited subscribers, added: taxi2se; removed: conr2d.

Last missing bit for me is @herdsman's removal of item_provider_cb, and maybe @taxi2se's work with Playable interface.

With 4a905a22a485388a5e4ba9fb25ff5ca381420ba7 we have the provider_cb out of the way.

jpeg added a comment.Jan 18 2018, 10:09 PM

Efl.Ui.Layout work is completed (only one API: theme).

Remaining non-interface API's in Efl.Canvas.Layout are:

  • animation --> @taxi2se I think this is related to your Animation/Playable changes
  • seat & seat_name -> this could be in an interface so that Efl.Ui.Layout can also implement it
  • access_part_iterate [BETA] -> this could be marked as stable and put in an interface as well

Thoughts?

zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:54 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 9:15 AM
zmike added a subscriber: zmike.

A #Goal ticket should not be set to a milestone.

Diffusion added a commit: Restricted Diffusion Commit.Aug 20 2018, 1:00 PM
Diffusion added a commit: Restricted Diffusion Commit.
zmike added a comment.Jan 15 2019, 6:07 AM

@cedric this looks like it's done?