When GtkBuilder builds a set of objects, described in a .glade file,
the function gtk_builder_get_type_from_name() is called each time an
<object class="xyz" id="abc"> element is parsed.
gtk_builder_get_type_from_name() calls the virtual function
get_type_from_name(). Gtk::Builder has overridden that function, and makes
GtkBuilder build gtkmm__xyz objects instead of xyz objects. So far, so good.

GtkListStore and GtkTreeStore call gtk_builder_get_type_from_name() when they
parse <columns> elements. Here it's not so good that GtkListStore is told
that a column shall contain gtkmm__GdkPixbuf objects when the .glade file says
it shall contain GdkPixbuf objects.
Gtk::TreeModelColumn<Glib::RefPtr<Gdk::Pixbuf>> tries to store GdkPixbuf
objects in the column, not gtkmm__GdkPixbuf objects.

GObject registers a transformation function in the GValue system, saying that
every GObject-derived object can be transformed to every other GObject-derived
object. That transformation is useless when it tries to transform a GdkPixbuf
to a gtkmm__GdkPixbuf, but because it exists, there is no warning when the
ListStore is populated.

In summary, Gtk::TreeViewColumn/ListStore/TreeStore is incompatible with
Gtk::Builder. I don't know how to fix it without unacceptable bad side effects.

Thanks for the test case. It simplified the troubleshooting a lot.

(In reply to comment #1)
> GtkListStore and GtkTreeStore call gtk_builder_get_type_from_name() when they
> parse <columns> elements. Here it's not so good that GtkListStore is told
> that a column shall contain gtkmm__GdkPixbuf objects when the .glade file says
> it shall contain GdkPixbuf objects.

I wonder if GtkListStore and GtkTreeStore should be checking for the type or derived types rather than just checking for the exact type.

Gdk::Pixbuf's constructors and create() methods create instances of
GdkPixbuf, not gtkmm__GdkPixbuf. The same goes for many other methods
that return a Glib::RefPtr<Gdk::Pixbuf>, such as Gtk::IconTheme::load_icon().
I think it would be very difficult to change that. There are a lot of
gdk_pixbuf_new*() methods, and it would be difficult to replace them with
calls to g_object_new() to create gtkmm__GdkPixbuf objects.

The problem here is that GtkListStore calls gtk_builder_get_type_from_name(),
which calls get_type_from_name_vfunc_callback() in builder.ccg.
GtkListStore is told that a TreeViewColumn shall contain gtkmm__GdkPixbuf
objects. Then it, quite reasonably, does not accept GdkPixbuf objects.

It's unfortunate that gtk_builder_get_type_from_name() is used for two quite
different purposes.

1. GtkBuilder calls it to find out what kind of object to create.
   A comment in builder.ccg tells why it's wise to ask for a gtkmm-derived
   type, if there is one:
   // Allow GtkBuilder to instantiate a gtkmm derived GType instead of the regular
   // GTK+ GType, so we can, for instance, use our vfuncs and default signal handlers.

2. GtkListStore and GtkTreeStore call it to find out what kind of objects are
   required in a TreeViewColumn. Then it would be wise *not* to require
   gtkmm-derived objects.

Created attachment 294696 [details] [review]
patch: Gtk::Builder: Don't request building of gtkmm__GdkPixbuf

Here's a patch that fixes the problem, but for GdkPixbuf only. I suppose
similar problems can occur for other types of objects in tree views.
I don't like this "solution". It's an ugly ad hoc solution.

A much better fix, from gtkmm's point of view, would be to add a new virtual
function in GtkBuilder, and let GtkListStore and GtkTreeStore call that
function instead of calling get_type_from_name().


An unrelated observation: When there is no gtkmm-derived type,
get_type_from_name_vfunc_callback() in builder.ccg calls g_type_from_name().
Wouldn't it be better to call the base class function()?
gtk_builder_real_get_type_from_name() does more, if g_type_from_name() can't
find a type.

(In reply to comment #4)
> Created an attachment (id=294696) [details] [review]
> patch: Gtk::Builder: Don't request building of gtkmm__GdkPixbuf
> 
> Here's a patch that fixes the problem, but for GdkPixbuf only. I suppose
> similar problems can occur for other types of objects in tree views.
> I don't like this "solution". It's an ugly ad hoc solution.

How would we describe the classes for which we shouldn't instantiate our gtkmm classes? Lack of vfuncs? Lack of signals?
 
> A much better fix, from gtkmm's point of view, would be to add a new virtual
> function in GtkBuilder, and let GtkListStore and GtkTreeStore call that
> function instead of calling get_type_from_name().

What would that function be called?
>
> An unrelated observation: When there is no gtkmm-derived type,
> get_type_from_name_vfunc_callback() in builder.ccg calls g_type_from_name().
> Wouldn't it be better to call the base class function()?
> gtk_builder_real_get_type_from_name() does more, if g_type_from_name() can't
> find a type.

Sure. That sounds wise. Maybe I thought the base class function couldn't be accessed when I implemented that years ago, but it should be doable.

(In reply to comment #5)
> How would we describe the classes for which we shouldn't instantiate our gtkmm
> classes? Lack of vfuncs? Lack of signals?

If a class has neither vfuncs nor signals, it doesn't matter if a gtkmm
derived object or a gtk+ object is created, I suppose.
Lack of signals can probably be tested with g_signal_list_ids(). I don't know
how to test for lack of vfuncs.

> What would that function be called?

I don't know. I don't even know if it should belong to GtkBuilder. Perhaps it
fits better somewhere else. In the GtkTreeModel interface? In GtkListStore and
GtkTreeStore?

Do you think we've got a chance to convince the gtk+ maintainers that such a
virtual function is needed?

> Sure. That sounds wise. Maybe I thought the base class function couldn't be
> accessed when I implemented that years ago, but it should be doable.

I have pushed the patch
https://git.gnome.org/browse/gtkmm/commit/?id=aecd310515b05a39b77635ef7f0c861c8587f7e0

I have filed the gtk+ bug 743516.

Created attachment 295924 [details] [review]
patch: Gtk::Builder: Make the creation of gtkmm-derived GTypes optional

The gtk+ developers don't want to make a separate version of
get_type_from_name() for the column types in GtkListStore and GtkTreeStore.
I can understand that.

The patch shows another idea. Make it possible to ask get_type_from_name() to
skip the search for a gtkmm-derived GType by prefixing the type name with '-'.

<object class="GtkListStore" id="treatstore">
  <columns>
    <!-- column-name col1 -->
    <column type="-GdkPixbuf"/>  <!-- Note -->
    <!-- column-name col2 -->
    <column type="gchararray"/>
  </columns>
</object>

Would this be acceptable? Do you prefer another prefix?

Thanks for working on this. The use of a prefix does seem a bit odd.

It would be nice if we could just somehow avoid the use of C++ column type whenever GTK+ instantiates a ListStore or TreeStore (or anything deriving from TreeModel).

But even that, or this use of a prefix, would need to be tested with various column types. I never expected this to work for even basic types such as strings.

(In reply to comment #9)
> It would be nice if we could just somehow avoid the use of C++ column type
> whenever GTK+ instantiates a ListStore or TreeStore (or anything deriving from
> TreeModel).

Agree. I just don't know how to achieve that.

Created attachment 296443 [details] [review]
patch: Gtk::Builder: Don't get gtkmm-derived GTypes while parsing <columns>

Here's a solution. Somewhat complicated perhaps, but I think it will work.
So far I have only tested it with the test case in comment 0.

Created attachment 298461 [details]
Test case 2

This test case is more extensive than the one in comment 0.
It works well with the patch in comment 11.
If no one objects soon, I'll push that patch.

(In reply to Kjell Ahlstedt from comment #12)
> Created attachment 298461 [details]
> Test case 2

If you need to define your ModelColumns in your code anyway, like in that test case:

class ModelColumns : public Gtk::TreeModelColumnRecord
{
public:
  ModelColumns()
  {
    add(m_pb);
    add(m_icon_name);
    add(m_int);
    add(m_icon);
  }
  Gtk::TreeModelColumn<Glib::RefPtr<Gdk::Pixbuf> > m_pb;
  Gtk::TreeModelColumn<Glib::ustring> m_icon_name;
  Gtk::TreeModelColumn<int> m_int;
  Gtk::TreeModelColumn<Glib::RefPtr<Gio::ThemedIcon> > m_icon;
};
...
static ModelColumns col;
...
Glib::RefPtr<Gtk::ListStore> store =
  Glib::RefPtr<Gtk::ListStore>::cast_static(
    builder->get_object("treatstore"));
...
row[col.m_icon_name] = "help-about";

Then is there not any particular benefit to defining the model types in the .glade file, and there is a danger of runtime error if you the code and the .glade file are out of sync.

Of course, there is some benefit (translation, for instance) to being able to specify the row data in the .glade file.


Before you commit this, could you please check that it works with pixbufs and icons specified in the .glade file. I don't see any. Then if we are really going to support this then we should add it as a variation of an example in gtkmm-documentation.

Created attachment 298568 [details]
Test case 4

Sorry for the unnatural numbering of test cases. As test case 2 contains
a test3.glade file, I thought it was better not to call this test case 3.

This is as far as I got with specifying a pixbuf and an icon in the .glade
file.

  <object class="GThemedIcon" id="icon1">
    <property name="name">document-print</property>
  </object>

  <object class="GtkListStore" id="treatstore">
    <columns>
      <column type="GdkPixbuf"/>
      <column type="gchararray"/>
      <column type="gint"/>
      <column type="GThemedIcon"/>
    </columns>
    <data>
      <row>
        <col id="0">rain.png</col>
        <col id="1" translatable="yes">list-add</col>
        <col id="2">42</col>
      </row>
    </data>
  </object>

I'm not sure it's possible to have builder add the icon to a list store row.
I had to add GThemedIcon manually. Glade does not like the file now. When
it's opened, glade complains about the GThemedIcon.

(In reply to comment #13)
> Then is there not any particular benefit to defining the model types in the
> .glade file, and there is a danger of runtime error if you the code and the
> .glade file are out of sync.

You have a good point there. In these test cases the benefit is not obvious.
I have expanded the test case, linked to in comment 0. There sawims, the
bug reporter, shows a workaround where

  Glib::RefPtr<Gtk::ListStore> store =
    Glib::RefPtr<Gtk::ListStore>::cast_static(
    builder->get_object("treatstore"));

has been replaced by

  Gtk::TreeView *treeview = 0;
    builder->get_widget("treattree", treeview);

  Glib::RefPtr<Gtk::ListStore> store = Gtk::ListStore::create(col);
  treeview->set_model(store);

and says that "this is not as flexible as hoped."


Questions to sawims:

What kind of flexibility do you want? In your test case, it does not seem to
be a great benefit to define the ListStore in the .glade file.

I can see that it would be useful to specify the ListStore with all its columns
and rows in the .glade file, if there are two programs involved: One that
writes .glade files, possibly with different sets of columns in different
files, and another program that can read any one of the .glade files.
The second program would not have to know exactly which columns the ListStore
contains. Is it something like this you want to do?

[snip[
> I'm not sure it's possible to have builder add the icon to a list store row.
> I had to add GThemedIcon manually. Glade does not like the file now. When
> it's opened, glade complains about the GThemedIcon.
[snip]

Then let's leave the GThemedIcon out of any example code for now, though it could stay in a test case. Or did someone ask for that to be supported?

(In reply to Kjell Ahlstedt from comment #14)
[snip]
> Questions to sawims:
> 
> What kind of flexibility do you want? In your test case, it does not seem to
> be a great benefit to define the ListStore in the .glade file.
[snip]

As said, my experience using Glade/Gtkmm was (and still is) quite limited.  By now, I can conclude that defining the store in code is not a big deal.  Flexibility is not an issue for this bug report.  At the start of my project I thought the store was necessary for creating the view with Glade (what can a display do if there is no data defined...).

For me, the major reason for filing the report is that the combination of the Glade defined store simply did not work in combination with gtkmm.  There was no crash, no warning or error message and I could not find any relevant information on the net.  The icons simply wouldn't appear in the GUI.  Hopefully this bug will safe others from the search, and better yet, it will solve the issue completely.

Created attachment 298772 [details]
Test case 5

I have removed the GThemedIcon and added a column with enum GtkAlign.

Probably this is yet another test case that will not convince anyone that it's
beneficial to let the builder create a ListStore or a TreeStore. It just shows
a static TreeView, defined in the .glade file.

There are some examples in the gtk+ demos, but they are equally unconvincing.
Either they define the columns in the .ui file (.glade file) and the rows
in C code, meaning that .ui file and C code must be in sync.
Or they define both columns and rows in the .ui file, and the program shows
a static GtkTreeView with no user interaction.

(In reply to comment #15)
> Then let's leave the GThemedIcon out of any example code for now, though it
> could stay in a test case. Or did someone ask for that to be supported?

No one asked specifically for GThemedIcon, but in comment 9 you asked for
tests with more column types.

(In reply to comment #13)
> Then if we are really going to support this then we should add it as a
> variation of an example in gtkmm-documentation.

Is it necessary to show it in the gtkmm tutorial, if we are not convinced
that it's useful? Sawims' main concern, according to comment 16, is that the
Pixbuf is not shown and there is no error message or other hint why it's not
shown.
(It depends on how the Pixbuf is added to the ListStore. If it's done in
code, like in test case 2 with test3.glade, there is no message, but the
Pixbuf is not shown. If it's done in the .glade file, like in test 5, there
is a critical message.)

Yes, please push the patch. I worry that some other code won't work when we allow this, but I guess this is how to find out.


I'll just explain the problem to myself in my own words to help me when I forget why we did this:

The actual critical

  (a.out:32005): GLib-GObject-CRITICAL **: g_value_set_object: assertion 'g_value_type_compatible (G_OBJECT_TYPE (v_object), G_VALUE_TYPE (value))' failed

happens at gtkliststore.c:2550, at this line:

  if (!gtk_builder_value_from_string_type (data->builder,
					   data->column_types[info->id],
					   string,
					   &data->values[i],
					   &tmp_error))


This is gtk_builder_value_from_string_type() from gtkbuilder.c:

  gboolean
  gtk_builder_value_from_string_type (GtkBuilder   *builder,
				    GType         type,
                                    const gchar  *string,
                                    GValue       *value,
				    GError      **error)


gtkliststore.c:2550 calls it with the 
  type=gtkmm__GdkPixbuf (well, you get that if you call g_type_name() on it)
  string="rain.png"

And it correctly initializes a GValue* with the gtkmm__GdkPixbuf type.

But then, when it actually tries to parse the data, such as:
  <col id="0">rain.png</col>
it uses some very specific gdk_pixbuf_*() API that always creates a GdkPixbuf, not a gtkmm__GdkPixbuf. The subsequent call to g_value_set_object() fails with the g_value_type_compatible() critical assertion because the GValue was inited to contain a gtkmm_GdkPixbuf but it's being given a GdkPixbuf.


Ideally gtk_builder_value_from_string_type() wouldn't use gdk_pixbuf_*_new*() functions, instead using g_object_new() and then calling gdk_pixbuf_*() functions on the object polymorphically. But those functions, such as gdk_pixbuf_new_from_stream() do more than just call public API, so this seems difficult to achieve. Ideally that's the correct way to fix it, but I certainly don't plan to spend time on that.

I have pushed https://git.gnome.org/browse/gtkmm/commit/?id=af2b13ab5ce5d316af0b0c19a299feba49d5f946