Add TreeStore and ListStore

[oakes]

This fixes #60 by wrapping TreeStore and ListStore, so TreeViews can display content. The idea is that it should work like this:

let mut project_tree = gtk::TreeView::new().unwrap();
let column_types = vec![glib::ffi::g_type_string];
let tree_store = gtk::TreeStore::new(column_types).unwrap();
let model = tree_store.get_model().unwrap();
project_tree.set_model(&model);

Here's what I think still needs to be done:

• Complete all the TreeStore/ListStore functions
• Create an enum that maps to the various GType constants
• TreeView's getter functions seem to have the "get_" omitted, which seems inconsistent with other widgets like IconView, so that should probably be fixed

[jeremyletang]

hey !

Thanks for working on this ! I think too that your implementation is made the good way.

Complete all the TreeStore/ListStore functions

Feel free to continue the work on this, be sure your code will be merge !

Create an enum that maps to the various GType constants

Maybe some static: static gtype_constant: i32 = 1;, Rust don't support binary operation, on enums.

TreeView's getter functions seem to have the "get_" omitted, which seems inconsistent with other widgets like IconView, so that should probably be fixed

Feel free to propose your change.

[oakes]

Great, I'll keep working on it. Static variables should work fine. I think I will need to bring all the constants into gtk_glue.c first, since they are all C macros.

[oakes]

I finished adding the TreeStore and ListStore functions. I have one thing I would like to ask about. I removed the TreeIter wrapper, because it seemed to not serve any purpose. I can bring it back if you prefer, but currently I'm just passing the underlying C struct to the functions like this:

let mut iter = gtk::ffi::C_GtkTreeIter;
list_store.append(&mut iter);
list_store.set_string(&mut iter, 0, "I'm in a list");

However, I noticed a problem. If I try to run the above code from a separate function, it crashes at runtime:

fn add_row(list_store: &gtk::ListStore) {
    let mut iter = gtk::ffi::C_GtkTreeIter;
    list_store.append(&mut iter);
    list_store.set_string(&mut iter, 0, "I'm in a list");
}

I created an example project to demonstrate creating treeviews. If you modify it to use the function above, you can reproduce the runtime crash. I would really appreciate any help in figuring out what is wrong.

[oakes]

I tried using the FFI functions directly. Interestingly, this works:

fn add_row(list_store: &gtk::ListStore) {
    let mut iter = gtk::ffi::C_GtkTreeIter;
    unsafe {
        let ptr = list_store.get_pointer();
        gtk::ffi::gtk_list_store_append(ptr, &mut iter);
        gtk::ffi::gtk_list_store_set(ptr, &mut iter, 0i, "I'm in a list".to_c_str().unwrap(), -1i);
    }
}

...but this crashes at runtime:

fn add_row(list_store: &gtk::ListStore) {
    let mut iter = gtk::ffi::C_GtkTreeIter;
    unsafe {
        let ptr = list_store.get_pointer();
        gtk::ffi::gtk_list_store_append(ptr, &mut iter);
        let ptr = list_store.get_pointer();
        gtk::ffi::gtk_list_store_set(ptr, &mut iter, 0i, "I'm in a list".to_c_str().unwrap(), -1i);
    }
}

So, getting a new copy of the pointer doesn't work! This is why it crashes when the wrapper functions are used. If you print out the two pointers, you would expect them to be equal, but in fact you get something like this:

0x7fa677913490
0x2

I'm mystified...

[GuillaumeGomez]

If you just get your pointer (without copying it) and the program leaves the original wrapper's scope, then all your objects which wrap the same pointer won't work properly anymore. Maybe your program comes from something like that.

[oakes]

I see, so we do in fact need the TreeIter wrapper. I added it back and will modify the functions to use it. However, how do you actually initialize it? It seems that the struct is normally initialized statically, but I hope there is a cleaner way than this:

let mut raw_iter = gtk::ffi::C_GtkTreeIter;
let iter = gtk::TreeIter::wrap_pointer(&mut raw_iter);

[GuillaumeGomez]

Then just do an empty new function. Do what seems better from your point of view.

[oakes]

Wouldn't that require storing the C_GtkTreeIter inside the TreeIter struct? Otherwise it would have to be dynamically allocated, it seems.

[GuillaumeGomez]

Yes, it does need C_GtkTreeIter anyway.

[oakes]

Ok I will work on that. I just realized, though, that TreeIter is not the cause of the crash, since I only messed with the TreeStore/ListStore pointer. So that is still a mystery to me. On Oct 11, 2014 3:47 PM, "Guillaume Gomez" notifications@github.com wrote:

Yes, it does need C_GtkTreeIter anyway.

Reply to this email directly or view it on GitHub https://github.com/jeremyletang/rgtk/pull/92#issuecomment-58762065.

[oakes]

I transitioned the code to use TreeIter. As expected, the runtime crash I described before is still happening, though. The ListStore/TreeStore pointers are becoming invalid after being used once inside a function scope. Please let me know if you have any thoughts. I think it is the last issue I have with this.

[GuillaumeGomez]

Hum... I don't see exactly how it works but since the wrapper is destroyed, I guess it's normal that its internal pointer is freed as well. Did I misunderstood something ?

[oakes]

Do you mean the ListStore wrapper is destroyed? I don't see why it would. The pointer is valid, then after calling gtk_list_store_append the pointer becomes invalid:

fn add_row(list_store: &gtk::ListStore) {
    let mut iter = gtk::ffi::C_GtkTreeIter;
    unsafe {
        let ptr = list_store.get_pointer(); // valid
        gtk::ffi::gtk_list_store_append(ptr, &mut iter);
        let ptr = list_store.get_pointer(); // invalid
        gtk::ffi::gtk_list_store_set(ptr, &mut iter, 0i, "I'm in a list".to_c_str().unwrap(), -1i);
    }
}

[GuillaumeGomez]

That's not normal at all. When you print the pointer's address, is it the same ? Does the ptr variable and the get_pointer function return the same one (after you call gtk_list_store_append). If that's not the case, we should think about returning a pointer on this pointer. But it seems weird : the pointer should still be the same.

[oakes]

No, they are not the same pointer. When I print them out, I get something like this:

0x7f072d54ec80
0x1

I'm running my treeview example project with the contents of the first for loop replaced by a call to the add_row function from above.

[GuillaumeGomez]

Oh, I just get it !

let ptr = list_store.get_pointer(); // valid
gtk::ffi::gtk_list_store_append(ptr, &mut iter);
let ptr = list_store.get_pointer(); // invalid

That's incorrect.

let mut ptr = list_store.get_pointer(); // valid
gtk::ffi::gtk_list_store_append(ptr, &mut iter);
ptr = list_store.get_pointer(); // invalid

Try with this.

[oakes]

It doesn't seem to make a difference; it crashes the same way and printing them out has the same result. Anyway, the ultimate problem is that self.pointer is changing, because when calling these functions from the ListStore wrapper it also crashes.

[GuillaumeGomez]

That's what's strange. Since it's a simple pointer and not a pointer on pointer, it shouldn't change. I just can't see how it could...

[oakes]

At any rate, this PR has all the additions I planned on making. Feel free to merge if you see no other problems and I'll make an issue about the pointer problem. Otherwise, we can try to fix it before merging if you prefer.

[GuillaumeGomez]

I prefer to only add fully functional source code. After it becomes a mess to know what's working and what's not. From my point of view, it's better to try to fix the problem before merging it to avoid future potential issue.

[oakes]

Fair enough. In case it helps, I made a minimal example project that reproduces the issue.

[GuillaumeGomez]

Thanks, I'll try to solve this issue in the next days (or, at least, look at it).

[oakes]

It looks like @afaur found that passing the TreeView to add_row prevents its from crashing, even though it's not being used. My knowledge of Rust is limited but I doubt this should ever be necessary, so it only deepens the mystery.

[GuillaumeGomez]

I got the problem : it's the difference between a library and another source code. From the library it must do some stuff and then it just breaks when you use va_args. I'm trying to find another way. Maybe on the macro side ?

PS: I was right :

let ptr = 0x1;
let ptr = 0x2;

println!("{}", ptr); // -> 0x1

Never do that again !

[oakes]

Do you mean I am using GTK+ incorrectly somewhere?

[afaur]

I think I am kinda of understanding what he's saying @oakes don't fully understand it but I went back after reading that and tried passing anything as the second arg to that add_row_via_wrapper and that makes it work.

... fn add_row_via_wrapper(store: &gtk::ListStore, tree: &str) { ... let test: &'static str = "the quick brown fox jumps over the lazy dog"; add_row_via_wrapper(&store,test); ...

I guess one of those methods needs two arguments from the function that it is being ran from for some reason? (http://www.cplusplus.com/reference/cstdarg/va_arg/)

[oakes]

Wow, that...is unbelievable. I don't see what va_arg has to do with it, though. The gtk_list_store_set function (wrapped by set_string) is indeed variadic, but the crash seems to happen even if you remove that and just call the append function twice, because the pointer to the internal list store becomes invalid. But yes, it seems that it does not become invalid if an additional argument is passed to the containing function...

[oakes]

Is there a chance this is a bug in Rust?

[oakes]

Success! With help from brandonson on IRC, the problem is fixed. It turns out that allocating C_GtkTreeIter statically was causing problems, so I added a new() function to TreeIter which allocates it dynamically. You can confirm it works by running the latest commit in the example project.

[jeremyletang]

hey @oakes ! Sorry to not be here since last week, i've been really busy.

First this seems really cool stuff !

I just see some things, that should be changed.

The functions which return boolean should use ffisafe type Gboolean.

You should move the Gtype const inside another module than ffi, the idea is that users of rgtk shouldn't use stuff from ffi as this stuff should be low level and not wrapped.

I will be happy to merge this stuff, after these littles changes.

[oakes]

I made the first two changes. Regarding GType, do you mean the type declaration and the list of constants should be moved somewhere else? I suppose src/glib/mod.rs would work, though we would have to create an extern block there. What do you think?

[oakes]

Anything else I need to do here?

[GuillaumeGomez]

If it works no. @jeremyletang will watch tomorrow I think.

[jeremyletang]

hey, It's okay to merge for me. Just one thing, you may want to add you name to authors in the Cargo.toml file ?

[oakes]

Sure, thanks.