Page MenuHomePhabricator

efl_access: change Efl.Access.Object from mixin to interface
Needs ReviewPublic

Authored by Jaehyun_Cho on Tue, Nov 27, 9:46 PM.

Details

Summary

mixin Efl.Access.Object inherits from abstract Efl.Object. So it is
regarded as class in some languages and it causes multi class
inheritance which is not allowed in some languages. (e.g. C#)

To resolve above, mixin Efl.Access.Object is changed to interface and
class Efl.Access.Widget is newly introduced.
class Efl.Access.Widget works like mixin Efl.Access.Object.

Efl.Ui.Widget attaches Efl.Access.Widget as its composite object.
So if Efl.Access.Object's method is called with Efl.Ui.Widget, then the
attached composite object Efl.Access.Widget works instead of
Efl.Ui.Widget.

To introduce class Efl.Access.Widget, mixin Efl.Access.Widget.Action is
renamed to Efl.Access.Widget_Action to resolve namespace conflict.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8239
Build 7523: arc lint + arc unit
Jaehyun_Cho created this revision.Tue, Nov 27, 9:46 PM
Jaehyun_Cho requested review of this revision.Tue, Nov 27, 9:46 PM
bu5hm4n added a subscriber: q66.Wed, Nov 28, 12:12 AM

I think i have said that before, but a regular class in the mixin inherit list should more be counted as a requiredment for the inherited class, so if class A inherits from mixin M then class A needs to have the regular classes of M in its inheritance chain. So it should not cause a problem with C#.

Anyways, this is also a possibility to implement these things :)

A few other points:

  • No one can ever get again the access pd when you hand him a efl_ui_widget, is this okay ?
  • We need a solution in eolian to indicate that API is getting dragged in via composition, so we can check if all APIs are implemented or not. (@q66 what's the state there?)

-

src/lib/elementary/efl_ui_widget.c
4033

You might want to redirect selected events from it->access_widget to eo_item.

5362

You might want to redirect selected events from it->access_widget to eo_item

5410

Due to the fact that the parent of sd->access_widget is obj, you don't need the detaching reccount checking you did here, I am also not too sure if you really need to keep another ref on the object or not.

Also, detaching this here means, that the efl.access API will only be here until the invaliding, calls to the api on obj in the destructor will throw errors :)

Do we really need to use object composition? If possible, I would prefer a "static" hierarchy.
I have not thought about this very thoroughly, but, can't we make Efl.Access.Object a regular class, inheriting from Efl.Object, and make all classes currently including the Efl.Access.Object mixin just inherit from it instead?
It looks to me like all classes including the Efl.Access.Object mixin are already inheriting from Efl.Object.

This comment was removed by segfaultxavi.

@bu5hm4n

Thank you for review :)

I want to explain a bit more about why I made this patch.

There is no "mixin" in C# so "mixin" in eo should be converted to either "interface" or "class".
Since Efl.Access.Object inherits from abstract "Efl.Object", Efl.Access.Object should be converted to "class".
However, some eo classes inherits from Efl.Access.Object and other regular class as well. (e.g. Efl.Ui.Widget inherits from Efl.Canvas.Group and Efl.Access.Object)
As a result, this multi class inheritance is not allowed in C#.

@segfaultxavi

I understand your point.
I think that you mean that Efl.Access.Object should be changed to a regular class and all elementary widget classes inherit from Efl.Access.Object to inherit from Efl.Object.

But all elementary widget classes should inherit from Efl.Canvas.Group which is a regular class.
And Efl.Access.Object cannot be in inheritance tree of Efl.Canvas.Group. (as far as I know, Efl.Access.Object is valid only for elementary widgets.)

So I am afraid that your suggestion is not suitable for this case.

OK, I had not noticed that Efl.Ui.Widget inherits from both Efl.Canvas.Groupand Efl.Access.Object.

I still think that the composition mechanism should only be used for rare cases, and not to solve the general "mixin problem" (the problem discussed in T7240).
If we use composition to solve the mixin problem for the Efl.Access.Object class, then, for consistency, we have to use composition to solve all other mixins.
Maybe it is a good idea, but we would need to generalize it so it is usable for all other classes with mixin problems.

@segfaultxavi

Yes, you are right. It is not easy to apply composite object to all mixin cases automatically. So it is a problem..

Also in this case, I've found that the original mixin Efl.Access.Object was designed to use widget object itself. (e.g. widget object's parent and child are searched and used inside the access logic)
However, since the composite object does not have the same parent nor child with widget object, the access logic does not work correctly...

Now I am looking for how to resolve the above issue..
If there is no feasible solution for the above issue, then I think we need to find another way to deal with mixins..

I agree. It would be better if we found a general solution to T7240 before trying to fix individual cases one by one.

@Jaehyun_Cho Now that I think of it, mixins ARE working in EFL, right? In a very complicated way, but they are working. What is exactly the problem that you are trying to solve with this patch?

The C# bindings are building and you can use them to build simple apps. What do you want to do that the current state of EFL does not allow?