Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

[RFC] GObject wrappers refactoring #59

Closed
wants to merge 5 commits into from
Closed

[RFC] GObject wrappers refactoring #59

wants to merge 5 commits into from

Conversation

gkoz
Copy link
Member

@gkoz gkoz commented May 5, 2015

Do not merge!

Here's my latest and likely final (modulo small improvements) version of the GObject wrapper framework first proposed at jeremyletang/rgtk#197 that's supposed to replace the current approach of using macros and a C glue module.

Objectives:

  • reduce code duplication by encapsulating reference counting and containers management;
  • reduce boilerplate by factoring out most common conversions and checks;
  • reduce reliance on macros in favor of generic programming that allows better type checking;
  • improve support for implementing multiple interfaces (e.g. GtkTreeModel is an interface implemented by GtkTreeStore, it shouldn't be represented as a separate widget);
  • support implementing some widgets in separate crates (e.g. libvte);
  • remove the need for gtk_glue.c and build complications it brings;
  • move toward automatic code generation from GIR by unifying the translations to/from the FFI types.

The glib::translate module shows all its power now: you can have a string, a widget, a nullable widget or a GList of widgets, doesn't matter, it just works, all applicable asserts included:

    pub fn new() -> Button {
        unsafe { Widget::borrow_from_glib(ffi::gtk_button_new()).downcast_unchecked() }
    }

    fn get_name(&self) -> Option<String> {
        unsafe {
            from_glib_none(ffi::gtk_widget_get_name(self.upcast().to_glib_none().0))
        }
    }

    fn set_name(&self, name: &str) {
        unsafe {
            ffi::gtk_widget_set_name(self.upcast().to_glib_none().0, name.to_glib_none().0)
        }
    }

    fn get_ancestor(&self, widget_type: Type) -> Option<Widget> {
        unsafe {
            from_glib_none(
                 ffi::gtk_widget_get_ancestor(self.upcast().to_glib_none().0, widget_type.to_glib()))
        }
    }

    fn get_children(&self) -> Vec<Widget> {
        unsafe {
            Vec::from_glib_container(
                ffi::gtk_container_get_children(self.upcast().to_glib_none().0))
        }
    }

The basic example (not present in this PR) demonstrates the use of the only two widgets supported in this proof of concept - Window and Button each implementing its own trait as well as WidgetTrait and ContainerTrait, and downcasting from Widget to Button.

The changes touch almost all crates, you can find the principal changes here: glib, gtk, examples but for the example to build other crates need patching too.

If you want to run the code I suggest using the git-repo tool:

> cd some_empty_dir
> repo init -u https://github.com/gkoz/rust-gnome-manifest -b refactor_objects
> repo sync
  • Run:
> cd examples
> cargo run

@gkoz gkoz changed the title [RFC] Proof of concept GObject wrappers refactoring [RFC] GObject wrappers refactoring May 5, 2015
@GuillaumeGomez
Copy link
Member

The biggest current problem is the fact we can't convert an object into another one. For example, converting a GtkListStore into a GtkTreeModel. I wanted to add two functions for this:

trait GObject {
    pub fn cast_to<T: GObject>(&self) -> T;
    pub fn cast_from<T: GObject>(object: &T) -> Self;
}

Your solution seems to allow such a thing in a better way. I'll test your example by the end of the week but I think I'll appreciate it. Very nice job !

@gkoz
Copy link
Member Author

gkoz commented May 6, 2015

I've now changed the FromGlibPtr/ToGlibPtr methods to avoid possibly confusing ownership terminology and use the transfer: none/container/full convention from GIR and gnome docs.

@gkoz
Copy link
Member Author

gkoz commented May 21, 2015

I've started implementing the conversion. The progress can be reviewed here: https://github.com/gkoz/rust-gnome-gtk/tree/object_reform

@GuillaumeGomez
Copy link
Member

Okay, I'll look it this week-end. Thanks for your work ! =D

@gsingh93
Copy link
Member

gsingh93 commented Jun 4, 2015

What's the status of this?

@gkoz
Copy link
Member Author

gkoz commented Jun 4, 2015

I've been somewhat distracted by other stuff including rustdoc (gtk-rs/gdk#49) but this is in progress.

@gsingh93
Copy link
Member

What do you think of merging just the object.rs file and letting any new widgets I add use the new framework? I think that works better than me adding new objects using the old framework or sending pull requests to your work in progress branch.

There would be a bit of inconsistency between previous and new widgets, but I don't think that's a big deal.

@gkoz
Copy link
Member Author

gkoz commented Jun 13, 2015

We could merge that file but without cooperation from numerous traits that may not be enough. You can't use the widget unless it implements FFIWidget and WidgetTrait and some of the traits are implemented with macros... Straightening all that out takes away time from the refactoring.

@gsingh93
Copy link
Member

Hey @gkoz, any update on how this is going? I saw the work going on with GIR and generating bindings, any update on that?

@GuillaumeGomez
Copy link
Member

@gsingh93: @gkoz is finalizing it, he said it should be done in a few days/weeks. I'll bring my help on this once I'm back from holidays to speed it up.

@gkoz
Copy link
Member Author

gkoz commented Dec 28, 2015

@gsingh93 this is mostly done (with the majority of bindings autogenerated thanks to great work by @EPashkin). I've been swamped with other stuff lately but will catch up soon.

@gkoz gkoz mentioned this pull request Jan 11, 2016
@gkoz
Copy link
Member Author

gkoz commented Jan 11, 2016

The implementation is at #221.

@gkoz gkoz closed this Jan 11, 2016
alex179ohm pushed a commit to alex179ohm/gtk that referenced this pull request Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants