Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full Interface Support #110

Open
1 of 5 tasks
mjakeman opened this issue Nov 20, 2020 · 8 comments
Open
1 of 5 tasks

Full Interface Support #110

mjakeman opened this issue Nov 20, 2020 · 8 comments
Labels
Enhancement New feature or request
Milestone

Comments

@mjakeman
Copy link
Member

mjakeman commented Nov 20, 2020

While trying to implement TreeSelection, I ran into an issue where I needed to get a GObject of some (theoretically) unknown type and return it as an interface. We should make Try/WrapPointerAs support returning interfaces as well as objects.

After #102, I think interfaces are the only "big thing" left in terms of type integration before we've covered most of the GObject type system. Let's use this to track interface support.

To Do:

  • Support interface properties (GInterface properties #104)
  • Support interfaces in Try/WrapPointerAs
  • Allow user to implement GInterfaces for their subclasses
    • This will need to be done during type registration for GObject to recognise it

Future:

  • Allow the user to export custom C# interfaces as GInterfaces?
    • Would be similar to object registration
  • Prefix interfaces with I for consistency
    • Would be done alongside the generator rewrite
@mjakeman mjakeman added the Enhancement New feature or request label Nov 20, 2020
@na2axl
Copy link
Member

na2axl commented Nov 20, 2020

Hi @firox263, you made a good point with this, maybe you can see if what I'm trying to do in #103 it is useful to you. I have:

  1. Created an IObject interface that will be the real root for every GObjects, so currently in Widget Catalog Samples #103 the Object class implement IObject and also every generated GInterfaces.
  2. Used modifications from 1. to implement GInterface properties as described in GInterface properties #104, for now only static property descriptors,
    an empty C# property (which will require implementation in classes) and a Native class are generated.

Maybe the fact that the IObject interface is the root node of the class/interface type hierarchy may help you ?

@mjakeman
Copy link
Member Author

@na2axl This looks very good to me. Having interface properties generated already knocks one thing off this list.

The next thing would be mapping between GInterfaces and their GTypes like we're doing for objects. I'll experiment on top of your branch a bit, and see if I can get something working.

@badcel
Copy link
Member

badcel commented Nov 22, 2020

I did not check the code just in case it applies here: we could try do make smaller iterations on the develop branch and slowly get things working.

@na2axl if complete parts of your code are already working as expected let's put them into develop.

I'm thinking about the same for my type integration or. It should probably be split into several smaller ones as it contains several features.

@mjakeman mjakeman mentioned this issue Dec 16, 2020
8 tasks
@mjakeman
Copy link
Member Author

mjakeman commented Jan 2, 2021

Some example code:

Assume we have a subclass which implements the Orientable interface.

public class MySubclass : Gtk.Widget, Orientable
{
    public Orientation Orientation
    {
        get => GetProperty(Orientable.OrientationProperty);
        set => SetProperty(Orientable.OrientationProperty, value);
    }

    public void SetOrientation(Orientation orientation)
        => (this as Orientable).SetOrientation(orientation);

    public MySubclass()
    {
        
    }
}

If we try and use this:

var subclass = new MySubclass();
subclass.SetOrientation(Orientation.Horizontal);

We get the following critical:

(dotnet.exe:19384): Gtk-CRITICAL **: 19:20:33.858: gtk_orientable_set_orientation: assertion 'GTK_IS_ORIENTABLE (orientable)' failed

To resolve this we need to make sure MySubclass implements the corresponding GInterface for Orientable at type registration time so that this method, as well as interface properties work transparently.

@mjakeman
Copy link
Member Author

mjakeman commented Jan 2, 2021

@badcel
Copy link
Member

badcel commented Jan 3, 2021

To register an interface we need to do the registration in the corresponding subclass, to avoid heavy reflection. In my test I added a method like this to the CompositeWidget.cs sample:

private static void RegisterInterfaces(Type gClass)
{
    var orientableInterfaceType = Gtk.Orientable.GTypeDescriptor.GType;

    var interfaceStruct = new InterfaceInfo(
        initFunc: InterfaceInit
    );
    
    IntPtr ptr = Marshal.AllocHGlobal(Marshal.SizeOf(interfaceStruct));
    Marshal.StructureToPtr(interfaceStruct, ptr, true);

    GObject.Global.Native.type_add_interface_static(gClass.Value, orientableInterfaceType.Value, ptr);
    Marshal.FreeHGlobal(ptr);
}

private static void InterfaceInit(IntPtr g_iface, IntPtr iface_data)
{
    
}

The method is executed directly after the type registration (currently per reflection). and the InterfaceInit method is called correctly.

This fixes

(dotnet.exe:19384): Gtk-CRITICAL **: 19:20:33.858: gtk_orientable_set_orientation: assertion 'GTK_IS_ORIENTABLE (orientable)' failed

But now the next error appears:

(Template:18443): GLib-GObject-CRITICAL **: 08:13:36.824: Object class GtkDemo_CompositeWidget doesn't implement property 'orientation' from interface 'GtkOrientable'

and a stack overflow

Stack overflow.
Repeat 261784 times:
--------------------------------
   at GtkDemo.CompositeWidget.SetOrientation(Gtk.Orientation)
--------------------------------
   at GtkDemo.Program.Main(System.String[])

The hurdle is that if we implement an interface we want to call the methods defined in our subclass. According to the documentation we need to wire up our subclass methods into the interface structure. The properties should be defined in the default_init function and must be registered, too.

I put the code here: https://github.com/gircore/gir.core/tree/spike/interfacesupport1

@badcel
Copy link
Member

badcel commented Jan 17, 2021

I made some good progress with installing properties into the Subclasses which needs to be done to implement interfaces. It is not yet working but I think it is not much left to have some working prototype code.

@badcel
Copy link
Member

badcel commented Jan 17, 2021

This is now working in some preliminary way. I think it is not very bad. I would be interested in your opinions @gircore/coreteam .

I added two additional (for now optional) properties to the Property class. It is the kind of the property which tells us if it is an enum / int / ... and the 2nd is the GObject type-id as we need both to create the C property descriptor (ParamSpec). This would require the generator to be aware of those 2 properties as they are not part of the property in the gir but part of the type definition which is in another place.

Another probably better solution could be to add the "kind" property to the type descriptor. But it would require the TypeDescriptor of each class to be public. Currently I think this would be okay, as we made the Handle public, too and are already exposing some internals. This would make generating the code much easier as we could just access the Orientable.TypeDescriptor and pass it on to InstallProperty. This would match the differentiation between types and properties which is present in the gir file, too.

Additionally I implemented it in a way that the subclass itself has to handle the setting of their own custom properties which itself installed. The code is quite easy. I tried to move it down into object but this made it very complex as the object class does not know anything regarding how to set a property of a more derived type and the types needed.

I think the current code does not work if there is a even more derived class than CompositeWidget as the ClassInit is probably not invoked. Need to check what would be required to get this working.

See: #196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants