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

[RFC-1] Type System Integration with GObject #46

Closed
mjakeman opened this issue Aug 6, 2020 · 14 comments · Fixed by #52 or #60
Closed

[RFC-1] Type System Integration with GObject #46

mjakeman opened this issue Aug 6, 2020 · 14 comments · Fixed by #52 or #60
Labels
Discussion Plan the future Enhancement New feature or request

Comments

@mjakeman
Copy link
Member

mjakeman commented Aug 6, 2020

Integrating C# and GObject

RFC

Please see the RFC here: https://github.com/gircore/rfcs/blob/master/rfc-001-type-integration.md
It explains the proposal and some potential solutions.

Summary

Type Integration means that when the user creates a class that derives from GObject.Object, the library will automatically create a corresponding GType behind the scenes. Effectively, it allows us to write "native" GObject code using C#.

This enables us to implement the following features, among many others:

  • GObject Properties - Subclasses can expose properties to the gobject type system, allowing for them to be viewable/changeable in GtkInspector, etc.
  • Overriding Virtual Functions - Subclasses can override virtual functions (See GtkWidgetClass). For example, creating a custom container widget in Gtk with full control over sizing and layout.
  • Composite Template Support - Supports binding a subclass to a composite template defined in an XML file. Particularly useful for complex custom widgets.
  • Bidirectional Bindings - The possibility of using C# defined types from C and/or other language bindings. With .NET 5 we can use source generators to output native-callable functions.

Like with the namespace refactor (#27, PR #45), this will be a huge change that affects almost the entire codebase. For this reason, it is probably best to focus on completing this before we write more bindings.

@mjakeman mjakeman added Enhancement New feature or request Discussion Plan the future labels Aug 6, 2020
@badcel
Copy link
Member

badcel commented Aug 6, 2020

Thank you for your document, it is amazing! I agree with the content of your document. Just some remarks we should consider:

  • We should add as few restrictions on the creator of C# subclasses as possible (regarding the hijacked constructor callback)
  • I don't like the constructor MyWindow(params Property[] properties). It is enforced but does not matter to a C# dev in any way. With .NET5 there will be source generators, which will allow source code generation at compile time. Perhaps we could generate the necessary connecting bits between C# and C via code generation.

Subclass constructor
I assume that we do not need to care about properties and their registration in the type system in the constructor? The properties are based on the class and could be registred in the GTypeInformation of the class during class initialization (See: GTypeInfo -> class_init parameter).

@mjakeman
Copy link
Member Author

mjakeman commented Aug 6, 2020

We should add as few restrictions on the creator of C# subclasses as possible (regarding the hijacked constructor callback)

Fully agree.

I assume that we do not need to care about properties and their registration in the type system in the constructor?

We shouldn't, thankfully. We'll need access to the class struct(s) in order to override virtual functions, so having class_init is a natural extension of that. In terms of properties, we'd probably use reflection against the class to find any properties to register and do that whenever the first instance is created.

Perhaps we could generate the necessary connecting bits between C# and C via code generation.

It isn't necessarily the connecting bits that are the problem. We really just need a way to create objects through an interface. The factory pattern would normally work for this, but forcing devs to write an entire factory for every object seems ridiculous. Ultimately, we could just require that there is an empty constructor present and let the user do whatever they like with it. The only downside then is that if we create a subclass with g_object_new() for whatever reason, we lose all of the properties that were set.

@badcel
Copy link
Member

badcel commented Aug 6, 2020

The only downside then is that if we create a subclass with g_object_new() for whatever reason, we lose all of the properties that were set

The toggle ref should keep our instance alive so there is no need to recreate an object which existed before or do I miss s.th.?

@mjakeman
Copy link
Member Author

mjakeman commented Aug 7, 2020

The toggle ref should keep our instance alive so there is no need to recreate an object which existed before or do I miss s.th.?

Don't think I worded that properly. If someone (either code from C/language bindings/GLib or some kind of GObjectFactory) calls g_object_new() to create a brand new instance of our class, they can pass in a list of property values to assign. For example:

GtkWidget widget = g_object_new (MY_TYPE_SUBCLASS,
                                 "width", 800, 
                                 "title", "Hello", 
                                 NULL);

Which should create an instance of MySubclass with properties Width = 800 and Title = "Hello".

If we're hijacking the constructor callback to call the C# constructor, we want these properties to be automatically set. It's just a question of how to do this. If we set them ourselves (i.e. using the returned object), they won't be accessible in the constructor. If we let the user handle (or conversely not handle) it, we lose the guarantee that the properties are set.

That being said, maybe this shouldn't even be considered a problem at all. With C#'s auto initialiser syntax, you can't access the properties in the constructor anyway:

public class Subclass
{
    public string Title;
    public int Width;

    public Subclass()
    {
        // The following lines will not work as the properties
        // haven't been assigned yet.
        Console.WriteLine(Title);
        Console.WriteLine(Width);
    }
}

// Somewhere Else
var obj = new Subclass {
    Title = "Hello",
    Width = 1024,
};

We could simply follow C# convention and make the user read properties later (maybe on 'gtk_widget_realize', which would be a virtual function?). It would certainly simplify the API and isn't unreasonable to expect.

EDIT:
Now thinking about it, this would simply force users to define an empty 'Default' constructor. The pattern for this could be to just chain to their preferred constructor: I think this is a clean-ish solution given it's already C# convention.

public Subclass() : this(my, default, values) {}

@badcel
Copy link
Member

badcel commented Aug 7, 2020

This topic is quite interesting, but very hard to follow along. At least for me. So the following text could be wrong or make wrong assumptions. Please keep that in mind.

I'm not sure if we need to consider this because, because the properties are registered in the type system and if g_object_new is called, the properties can be set inside the method g_object_new itself.

  • g_object_new calls g_object_new_valist: See
  • g_object_new_valist calls g_object_new_internal: See
  • g_object_new_internal sets the properties directly on the new object (it does not know the type) : See

I believe if we have a constructor which just accepts an IntPtr handle this handle could already be completly initzialized including the custom C# properties.

If the code is like i suggested above:
Creating an object in C: Object is created, Properties are set inside g_object_new and the C# constructor(IntPtr handle) is called (handle is already initialized), which executes additional initialization steps for the GObject.

Creating an object in C#: The object is created in the C world (through the inheritance chain ..:base(@new(...))) and afterwards the C# constructor continues automatically which executes additional initialization steps for the GObject.

This would look like:

public class Subclass : Window
{
    public IProperty<string> Title { get; }
    public IProperty<int> Width { get; }

    public Subclass() //: base() Implicit call of base constructor which calls the other constructor later
    {
        //This constructor is for public access
    }
    internal Subclass(IntPtr handle) : base(handle)
    {
        //This constructor is to initialize
        Title = PropertyOfString("title"); //Connects the properties to the C world
        With = PropertyOfInt("width");
        Title.Value = "MyTitle set from C# in the constructor"
    }
}

There is one other path, for custom constructors: See. This is probably the path you have in mind in your analysis? I believe, if we register a custom constructor the code would look like you suggested.

Are my assumptions correkt? If yes: Which way do you prefer and why?


Just as a side note: In the code it reads a little bit like this path is not very performant, but i don't know the performance implications of this.

If we have ->constructed() then we have to do a lot more work.

if G_UNLIKELY (CLASS_HAS_CUSTOM_CONSTRUCTOR (class))
    return g_object_new_with_custom_constructor (class, params, n_params);

@mjakeman
Copy link
Member Author

mjakeman commented Aug 7, 2020

Are my assumptions correkt?

You're absolutely right there, as far as I'm aware.

My proposal was indeed using custom constructors, as it gives us a much nicer API (i.e. the user doesn't have to define the internal Subclass(IntPtr handle) : base(handle) in order for it to work), but at the cost of a bit more complexity in the GObject bindings.

public class Subclass : GObject
{
    public Subclass()
    {
        // Init
        Console.WriteLine("We can do everything here *except* access properties");
    }
}

The above is all we need for a custom GObject-based class, everything is handled behind the scenes. All creation paths would go through the default constructor, and in a similar manner to GLib properties, would be assigned after we have made the object.

However I think you're right about the constructor pathway possibly being slower. I'm not too sure myself actually, as it seems most of the overhead involves copying construct properties. Since we'll only ever have one set (the inhibit property) perhaps the overhead will be minimal. I think it's definitely worth investigating this, because we don't want to sacrifice performance for a slightly nicer API.

Which way do you prefer and why?

I don't feel strongly about either way, although I'd prefer the constructor method since it has a much more organic feeling API. If there is a performance hit, we should definitely go with the IntPtr one. Luckily, this isn't something that needs to be done for a while, so we could always experiment with both?

@mjakeman
Copy link
Member Author

mjakeman commented Aug 7, 2020

Some exciting news: I had a go at implementing basic proof-of-concept type system integration and it appears to be working quite well actually.

I added a type dictionary and a static method to register a new class with GLib (see here). It now creates a new GType behind the scenes for every derived class of GObject. I've also managed to override class_init and print out to the console.

The library automatically creates a GType for the following class, as you'd expect:

class Subclass : GObject.Object
{
    public Subclass() {}
}

We can retrieve the class name to verify:

// Create a new instance of the object
var obj = new Subclass();

// The Library automatically generates a GType for Subclass
// and calls it `{Namespace}{ClassName}`, in this case
// "GObjectTestSubclass"

// To confirm it's working, let's retrieve the name back from GLib
IntPtr ptr = Sys.Methods.type_name_from_instance(obj.Handle);
string? name = Marshal.PtrToStringAnsi(ptr);

// Assert passes: we have indeed created a new type
Debug.Assert("GObjectTestSubclass" == name);

// Nothing very useful at the moment, but the groundwork is there :)

I'm working on it over here: https://github.com/firox263/gir.core/tree/gobject-rewrite.

@badcel
Copy link
Member

badcel commented Aug 8, 2020

Luckily, this isn't something that needs to be done for a while, so we could always experiment with both?

Absolutely 👍

With C#'s auto initialiser syntax, you can't access the properties in the constructor anyway:

You are right. This approach does match with the gobject approach as both are based on initializing properties, after the object is created.

We could simply follow C# convention and make the user read properties later (maybe on 'gtk_widget_realize', which would be a virtual function?)

I like the thought of virtual functions, but we should be working this out in the GObject layer and not depend on any GTK functions, as all GObject based libraries will have properties which we probably want to access somehow during/after initialization.

I think there are different approaches to initialize objects in C#, two of them are:

  • Initialize the object in the constructor (but not the properties) and the properties are initialized afterwards via auto initializers
  • Initialize the object only in the constructor because the properties depend on the constructor parameters.

I like your aproach and the simple look of the subclass, but I think currently it limits the possibilities a little bit. If we could find a way to mitigate this it would be an absolutely amazing solution.

Another possible solution would be just an explanation in the documentation. As we could simply state that GObjects are created in exactly this way: Properties are set via auto initializers, and not inside the constructor, because this is how the GObject system works. If someone wants to create objects in another style, it would simply not be a GObject.

One more thought: There is the Constructed class method which is called during object creation. Perhaps we could override the constructed method and call back into a virtual function of the C# object. In this case it would be possible to access all "constructor properties" and we are alligned with the GObject system.

I'm going to test a little bit with your code.

@mjakeman
Copy link
Member Author

mjakeman commented Aug 8, 2020

I like the thought of virtual functions, but we should be working this out in the GObject layer and not depend on any GTK functions

Yep, you're right. I was mostly thinking of this in terms of Gtk widgets, but it'd make the most sense for us to use ->constructed(). It looks like we can use virtual functions for everything except class_init and class_finalize (as they need to be static) so it should feel fairly natural to use.

Another possible solution would be just an explanation in the documentation. As we could simply state that GObjects are created in exactly this way: Properties are set via auto initializers, and not inside the constructor, because this is how the GObject system works. If someone wants to create objects in another style, it would simply not be a GObject.

I think this would be okay. Hopefully we can use ->constructed as a means of making it a little more flexible.

I'm going to test a little bit with your code.

Have fun :) Just to let you know, only Libs/GObject compiles since I've changed how handles work (and will do again once I work on object lifetimes).


I've been experimenting with getting the rest of the bindings to work and I've encountered a few sticking points. Would greatly appreciate feedback on the following:

  1. How do we link wrapper types with the underlying GType? Currently as a placeholder I'm just using a [Wrapper("GtkWindow")] attribute, but since we can't look up GTypes by name until they've been created at least once, we'll need a better method.
  2. As far as I can tell, we aren't generating the get_type() function for each type. Ideally we'd use this towards (1), but I'm not sure if it's even included in Gir data. Maybe we can automatically generate a prefix_name_get_type() function since the signature will never change? See https://developer.gnome.org/gobject/stable/gtype-instantiable-classed.html for the macro-less type function.
  3. We're creating objects inline in constructors and passing the resulting pointer down to the base class. This results in the subclass type being registered correctly, but never used as we simply wrap the pointer from the wrapper type. To illustrate:

With `DemoWindow':

DemoWindow() -> ApplicationWindow(Sys.ApplicationWindow.@new(...)) -> Window(IntPtr) -> ... -> GObject(IntPtr)

So while we are registering a GType for DemoWindow, our object is initialised by the IntPtr from GtkApplicationWindow. The result is that we simply get an app window, rather than our custom subclass.

We need some way of not calling @new() when subclassing from a wrapper, and instead letting the base class handle object creation. My preferred option would be to do all creation in GObject(), but I'm not sure how we'd set the my_type_new() method, unless maybe by using attributes, which seems a bit messy.

@badcel
Copy link
Member

badcel commented Aug 8, 2020

Regarding point 1:

  • An idea would be to use the class constructor to register the type, before any object is created, it could be s.th. like GObject.Type.Register(typeof(DemoWindow));. (This code can potentially generated, in .NET 5)
  • In which situations do we actually need this link and the Wrapper-Attribute is not sufficient?

Regarding point 2:

  • It is in the gir-file, but it is currently not generated and I never tried to call it: <class name="ApplicationWindow" ... glib:get-type="gtk_application_window_get_type" ...>. If this method is helping, I can add it to the generator.

Regarding point 3

  • Yes we need to get rid of @new. But I assume that we do not need to call any @new() method from the inheritance chain? If we register our type beforehand, we could just call g_object_new(TYPE,...) and get the constructed callback?

@mjakeman
Copy link
Member Author

mjakeman commented Aug 9, 2020

An idea would be to use the class constructor to register the type, before any object is created, it could be s.th. like GObject.Type.Register(typeof(DemoWindow));. (This code can potentially generated, in .NET 5)

In a way we're doing that already. The main problem is in order to link with the GType for GtkApplicationWindow, we need someway of retrieving that type. If we look up "GtkApplicationWindow" by name, the type most likely hasn't been created yet, and therefore won't be registered in the GLib Type system, let alone ours. I think we don't have a choice other than to use the glib:get-type function, since that creates the type if it hasn't been registered yet.

If this method is helping, I can add it to the generator.

That would be great thanks!

we could just call g_object_new(TYPE,...) and get the constructed callback?

We're doing this for Subclass types already. The only problem I see with using g_object_new over say gtk_window_new is I believe the creation methods set default properties which we'd lose by replicating it ourselves. In Gtk.NET, I created a delegate called defaultConstructor which I overrode in the object constructor, however I'd like to avoid lazy initialisation if possible. It makes everything too complicated.

We need some way to have both the get_type function, and optionally the constructor, associated with the class statically. If no constructor is present, we fall back to g_object_new_with_properties.


I've pushed my working branch to gobject-rewrite and created a draft PR. I think we should keep this issue for high-level discussions of how to integrate it (e.g. what we're doing now) and use the PR for progress updates and discussions about specific code.

@badcel
Copy link
Member

badcel commented Aug 9, 2020

Sorry I used the term class constructor before, but actually meant static constructor

If we look up "GtkApplicationWindow" by name, the type most likely hasn't been created yet, and therefore won't be registered in the GLib Type system, let alone ours.

In which point in time or method do we need to look up "GtkApplicationWindow"? Because if the static constructor already run at this point in time, the type would be registred. As far as I can see it at the moment, we need to check the RegisterType method which is called during initialization of the object. So the static constructor should already be executed.

Perhaps it is enough to call

  • Wrapper classes: the get_type methods in the static constructor
  • Subclasses: the Object.RegisterType(type) in the static constructor

to register the types early enough.

@mjakeman
Copy link
Member Author

I think you're right. For wrappers, calling get_type() and registering it in the static constructor would be a very good method. The only thing I'm worried about is it's not compile time check-able, but it doesn't seem like we'll get that regardless of the option we choose. It's much better than using [Wrapper("TypeName")] anyway.

When we create a subclass, the static constructor will be called automatically and thus the type will be registered by the time it comes to creating the subclass object. The only flaw in this is that we won't be able to guarantee access to the gtype of an object from outside the object/it's inheritance chain. If we enforce this as a rule and make the GType API private then it should be fine (perhaps we should be doing this anyway? User code doesn't need access to glib internals).

We probably don't need to call RegisterType() in the static constructor for Subclasses as we can do that in the GObject constructor itself on first run and avoid the user having to write more boilerplate.

@badcel badcel linked a pull request Aug 18, 2020 that will close this issue
@mjakeman
Copy link
Member Author

Quick idea in light of #56

I agree we should try and get this landed soon, since the entire library depends on this. I've been doing some thinking about the proposal and I feel that in trying to make the bindings bidirectional, we're sacrificing a lot of nice C# ergonomics.

New Idea regarding ->constructor():

We only need to use ->constructor() if we could reasonably expect that the object is create-able from GObject. Since constructor() appears to introduce non-trivial overhead, and I'm not even sure anyone would practically need to create a C# subclass from C, we could simply not override by default.

Later on, if we find a situation in which this is useful, we can get away with adding this without breaking API compatibility, as the setup is entirely contained within class_init. We could even make it optional per-class (e.g. if some attribute is declared).

What it means in the short term is we can focus on making the library usable, and worry about this edge-case feature down the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Plan the future Enhancement New feature or request
Projects
None yet
2 participants