rhx / SwiftGtk

A Swift wrapper around gtk-3.x and gtk-4.x that is largely auto-generated from gobject-introspection
https://rhx.github.io/SwiftGtk/
BSD 2-Clause "Simplified" License
317 stars 26 forks source link

Extending Gtk Widgets #17

Closed lschmierer closed 4 years ago

lschmierer commented 4 years ago

I am trying to figure out a way of ergonomically extending Gtk widgets.

The following works and seems to be the nicest way I can figure out:

import Gtk

class ConnectPage: Box {
    init(wifiConfiguration: String) {
        super.init(BoxRef(orientation: .vertical, spacing: 60).box_ptr)

        let qrCode = QRCode(data: wifiConfiguration)
        qrCode.setSizeRequest(width: 200, height: 200)

        let label = Label(str: "<span size='larger'>Please Scan QR Code to connect</span>")
        label.useMarkup = true

        setValign(align: .center)
        add(widgets: [qrCode, label])
        showAll()
    }
}

I am using Box as example for any Gtk Widget.

Box has the designated initializer init(_ op: UnsafeMutablePointer<GtkBox>) and among others the convenience initializer convenience init( orientation: Orientation, spacing: CInt)

If I add another convenience initializer,

init(wifiConfiguration: String) {
    self.init(orientation: .vertical, spacing: 60)
    ...

the code looks much nicer. However, the super class convenience initializers like convenience init( orientation: Orientation, spacing: CInt) are still accessible and can be called. Thereby my initialization logic is bypassed. Thus, I need to override the super designated initializer.

I am not quite sure what super.init(BoxRef(orientation: .vertical, spacing: 60).box_ptr) does exactly. It seems to create a new GtkBox object, takes it as unowned reference (BoxRef) and then turns it into a Box by passing the pointer to a designated initializer of Box. When I try using a Box instead of a BoxRef, the application crashed. This makes sense, since Box being a owned reference to GtkBox, destroys the underlying Gtk object on deinitialization (which happens right after the super.init() call) while BoxRef does not.

All in all, I think my sample code above does not have any memory leaks. However, it is still inconvenient to use. Might it be possible to turn the convenience initializer convenience init( orientation: Orientation, spacing: CInt) of Box into a designated initializer?

I am also wondering, why the need the *Ref structs at all. For callbacks etc. it should be possible to use e.g. Unmanaged.fromOpaque(ptr).takeUnretainedValue() for turning a raw pointer back into a Swift object without increasing the reference count. (https://developer.apple.com/documentation/swift/unmanaged) But this is rather a discussion about gir2swift I guess.

rhx commented 4 years ago

The Ref structs are basically just wrappers around a pointer that the gtk functions return. They do not participate in Swift memory management and thus are not that useful to create manually in Swift. The main reason for their existence is that a lot of GTK functions return pointers to gtk/glib objects that have already been created by the underlying libraries.

What the Refs give you over the opaque pointers is type safety, i.e., they make it harder to pass them around to unrelated types or call unrelated methods. You could just stick to the corresponding Swift classes, but you would incur the extra heap allocation for a Swift object (using Unmanaged does not prevent that, it just gives you manual control over whether Swift reference counts should be updated or not, i.e. takeRetainedValue() vs takeUnretainedValue()).

So as a rule of thumb:

rhx commented 4 years ago

So looking at your code above:

super.init(BoxRef(orientation: .vertical, spacing: 60).box_ptr)

This is going to leak memory, because you are using Gtk to create a Box, but you never free it anywhere. So, yes

    self.init(orientation: .vertical, spacing: 60)

is the way to go. If you want to make superclass constructors unavailable, override them, but make them private or protect them through @availability

lschmierer commented 4 years ago

Thank you very much for clearing things up!

is the way to go. If you want to make superclass constructors unavailable, override them, but make them private or protect them through @availability

How would I override e.g. convenience init( orientation: Orientation, spacing: CInt)? A convenience initializer must delegate with 'self.init'. We can not call super.init(orientation: Orientation, spacing: CInt). So the only way I can come up with would be by copying the parents class initializer.

private init(orientation: Orientation, spacing: CInt) {
    let rv = gtk_box_new(orientation, gint(spacing))
    self.init(cast(rv))
}

Thats does not seem very Swifty. However we can't make superclass intitalizers unavailable this way anyway. Making them private, the parent intialzer can still be accessed from outside of the class. (Strictly speaking, we can't override convenience initializers, we can only "provide a matching implementation" Apple Docs.)

If we use @available, we can not access the initializer from other initializers (e.g. init(wifiConfiguration: String)) as well.

It is probably the easiest to just live with the fact that the parent initializers are accessible. There seems to be no way to avoid it as long as init(orientation: Orientation, spacing: CInt) remains a convenience initializer. Is there any reason why a designated initializer can not be used instead?

lschmierer commented 4 years ago

Since your last changes to gir2swift we can do:

init(wifiConfiguration: String) {
    super.init(box: Box(orientation: .vertical, spacing: 60))

which is quite nice, but should be documented somewhere as it still somewhat diverges from usual subclassing practice.

rhx commented 4 years ago

Yes. I'm currently experimenting whether I can turn the other (generated) convenience initialisers into designated initialisers as well (this is wip as it's somewhat tricky, though).

rhx commented 4 years ago

Okay, with gir2swift-8.0.0 all generated initialisers are now designated initialisers. So you should now be able to do something that looks a bit more swifty:

class ConnectPage: Box {
    private override init(orientation: Orientation, spacing: CInt) {
        super.init(orientation: orientation, spacing: spacing)
    }

    init(wifiConfiguration: String) {
        super.init(orientation: .vertical, spacing: 60)

        let qrCode = QRCode(data: wifiConfiguration)
        qrCode.setSizeRequest(width: 200, height: 200)

        let label = Label(str: "<span size='larger'>Please Scan QR Code to connect</span>")
        label.useMarkup = true

        setValign(align: .center)
        add(widgets: [qrCode, label])
        showAll()
    }
}

You need to clean, pull the latest SwiftGtk, and rebuild for this to work.

lschmierer commented 4 years ago

Thank you very much, this is really great!

Just for your information, I don't think privately overriding init(orientation: Orientation, spacing: CInt) is necessary. By providing a new designated initializer in the subclass, every object creation is funnelled through this new intializer and designated parent initializers can not be accessed anymore.

rhx commented 4 years ago

By providing a new designated initializer in the subclass, every object creation is funnelled through this new intializer and designated parent initializers can not be accessed anymore.

You are right of course. Obviously if you did want to make the superclass initialiser available, you'd now have to do something like

public override init(orientation: Orientation, spacing: CInt) {
        /// ... insert subclass initialisation here ...
        super.init(orientation: orientation, spacing: spacing)
    }