GNOME Bugzilla – Bug 639073
Many issues in gtkmm 3 affect the custom widget example program
Last modified: 2011-02-12 13:33:05 UTC
Overview: When I tried to run the example program gtkmm-documentation/examples/book/custom/custom_widget with gtkmm 3 code, I found a bunch of issues that affect that program, quite apart from the fact that the program itself has not been completely adapted to gtkmm 3. The program crashes with a segmentation fault, but that's not the only problem. 1. stylecontext.ccg There are a number of functions that end with return Gdk::RGBA(&crgba, false); or return Border(&cborder, false); where &crgba and &cborder are pointers to local data on the stack. This is one reason why the program can crash. 2. widget.hg In addition to the new functions gtk_widget_get_request_mode, gtk_widget_get_preferred_{width, height_for_width, height, width_for_height, size} which are correctly wrapped with _WRAP_METHOD, there are also the new virtual functions get_request_mode, get_preferred_{width, height_for_width, height, width_for_height} (but no virtual get_preferred_size). These must be wrapped with _WRAP_VFUNC. (get_request_mode is perhaps not that simple. See a following comment.) 3. cssprovider.hg get_default and get_named need 'refreturn'. 4. cssprovider.hg Shouldn't Gtk::CssProvider have a 'create' method and a protected default constructor? 5. gtkcssprovider.c When gtk_css_provider_load_from_path parses the file with style properties, it assumes that class names begin with an uppercase letter. Identifiers that begin with a lowercase letter (e.g. gtkmm__CustomObject_mywidget) are assumed to be names of regions within a widget. See functions parse_rule and parse_selector in gtkcssprovider.c. 6. The custom_widget program This program must be modified in some ways, but I'd like to defer a detailed discussion until the other problems have been solved. Steps to reproduce: Actual results: Expected results: Build date and platform: Ubuntu 10.10, source code of gtkmm, gtk+, etc. built with jhbuild on 2011-01-04. Additional information: Further discussion of these issues will follow right away in a comment.
I stumbled upon all these issues just because I tried to check if the changes in gtk+ 3 and gtkmm 3 have fixed bug 606903. I can make a patch with corrections of some or all of the issues, but I don't yet know what to do with some of them. 1. stylecontext.ccg Quite obvious what to do. Set make_a_copy = true in the constructors. Otherwise the functions return an object that contains a pointer to local data. An alternative solution that avoids a copy (get_color used as an example): GdkRGBA* crgba = g_slice_new(GdkRGBA); gtk_style_context_get_color(....., crgba); return Gdk::RGBA(crgba, false); 2. widget.hg I don't know what's the best way to wrap the virtual function get_request_mode. It's possible to let klass->get_request_mode = NULL, and I think that's what most gtk+ widgets do. When the request mode of a widget is required, gtk_widget_get_request_mode is called. It's defined in gtksizerequest.c: GtkSizeRequestMode gtk_widget_get_request_mode (GtkWidget *widget) { GtkWidgetClass *klass; g_return_val_if_fail (GTK_IS_WIDGET (widget), GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH); klass = GTK_WIDGET_GET_CLASS (widget); if (klass->get_request_mode) return klass->get_request_mode (widget); /* By default widgets are height-for-width. */ return GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH; } If get_request_mode is wrapped with _WRAP_VFUNC, then get_request_mode_vfunc and get_request_mode_vfunc_callback end with typedef GtkSizeRequestMode RType; // or typedef SizeRequestMode RType; return RType(); Now this return value happens to be the same as the default return value from gtk_widget_get_request_mode, but that's pure luck, I believe. It shouldn't be relied on. A better way would be to manually code get_request_mode_vfunc and get_request_mode_vfunc_callback, and have them return Gtk::SIZE_REQUEST_HEIGHT_FOR_WIDTH and GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH. A still better way would be to call gtk_widget_get_request_mode to get its default value. But then you must create a widget object of a class with klass->get_request_mode == NULL. The input object (self in vfunc_callback, gobject_ in vfunc) won't do, will it? But isn't it too much work to create a new object just to get this default return value? It would be nice if gtk+ contained a constant GTK_SIZE_REQUEST_DEFAULT or a function gtk_widget_get_default_request_mode(). 2.1 gtk_vfuncs.defs When I added virtual functions to widget.hg, I became aware that I also had to change gtk_vfuncs.defs. The tutorial "Programming with gtkmm" says that gtk_vfuncs.defs is written manually. Is that still true, or is it generated by a script? 3 and 4. cssprovider.hg Obvious solution, I think. Or is there a reason not to create a Gtk::CssProvider with a 'create' method? 5. gtkcssprovider.c Does this mean that the prefix gtkmm__ must be changed? E.g. to Gtkmm__? gtkcssprovider.c contains a long description of the syntax and semantics of a css file. At the very end of that description it's mentioned how to set a style property that has been registered with a call to gtk_widget_class_install_style_property. But nowhere does it say that a class name must begin with an uppercase letter. Only the source code shows that. 6. The custom_widget program Some necessary changes: Gtk::Widget::on_size_request does not exist any more. MyWidget::on_size_request shall be replaced by some or all of the get_preferred_xxx_vfunc functions. It's not correct to use Gtk::CssProvider::get_default(), and read the style property file with that CssProvider. You must create a new CssProvider and add it to the style context. The description of gtk_css_provider_load_from_path says: "Loads the data contained in @path into @css_provider, making it clear any previously loaded information." For a reason unknown to me, the program later crashes with a segmentation fault, if load_from_path is called on the default css provider. The style property file must be changed, probably like this: * { -Gtkmm__CustomObject_mywidget-example-scale: 800; } assuming the gtkmm__ prefix is changed to Gtkmm__ in glibmm/class.ccg and some other files. I don't know if such a change can have bad side effects. And what about bug 606903? No, it has not been fixed.
The removal of Gtk::Widget::on_size_request() affects also the custom_container example and the description in "Programming with gtkmm" chapter 26. MyContainer::on_size_request() is never called. ExampleWindow::m_Button_Two is a Gtk::Label in the custom_container example!
(In reply to comment #1) > I stumbled upon all these issues just because I tried to check if the changes > in gtk+ 3 and gtkmm 3 have fixed bug 606903. > > I can make a patch with corrections of some or all of the issues, That would be very welcome. Actually, if you request git access then I'll gladly provide it. > 2.1 gtk_vfuncs.defs > When I added virtual functions to widget.hg, I became aware that I also had to > change gtk_vfuncs.defs. The tutorial "Programming with gtkmm" says that > gtk_vfuncs.defs is written manually. Is that still true, or is it generated by > a script? It's still manual. I don't know of any way to generate it. > 3 and 4. cssprovider.hg > Obvious solution, I think. Or is there a reason not to create a > Gtk::CssProvider with a 'create' method? When the C API has get_default() we generally do the same. create() suggests a new instance, while get_*() suggests returning the same instance. If there is a *_new() function too then there should be a create() method in our API. > 5. gtkcssprovider.c > Does this mean that the prefix gtkmm__ must be changed? E.g. to Gtkmm__? We have had a problem with this before with GtkRCStyle, but I think we fixed it. We can change it in gtkmm 3 if necessary, but I'd like to see some documentation saying that we need to. You could file a bug about that.
(In reply to comment #3) > (In reply to comment #1) > > I can make a patch with corrections of some or all of the issues, > > That would be very welcome. Actually, if you request git access then I'll > gladly provide it. Thanks for your offer, but I won't request git access, at least not yet. I'm a bit of a coward, and it's fine not to be able to mess up the git repository, although I think the risk would be low. I'm preparing a patch that solves items 1, 2, 3, and 4. Concerning item 2 I have a question about constness of member functions. The new functions gtk_widget_get_preferred_xxx and gtk_widget_get_request_mode, and possibly also the corresponding virtual functions, have what I think Bjarne Stroustrup calls physical but not logical constness. The compiler does not complain if they are declared const (well, for the vfuncs it of course depends upon the overriding functions in the subclass), but they can do a fair amount of calculations, and save the results in the underlying C object. Are there any rules for this kind of function? Shall they be declared const? I'm inclined to remove const from get_preferred_xxx and get_request_mode. Is it possible that anyone is already dependent on they being const? I can imagine that it's next to impossible to get the constness right on all wrapped functions. The easy way is to declare all get_xxx and is_xxx functions const, except for some functions that return a pointer to a non-const object.
(In reply to comment #4) > I'm inclined to remove const from get_preferred_xxx and get_request_mode. > Is it possible that anyone is already dependent on they being const? No, I think the constness is appropriate. Whether it changes some bits in the memory is not really relevant. const means what we say it means. If it recalculates some cache values then that's not interesting to the user of the API. That's why C++ has the mutable keyword.
Created attachment 178710 [details] [review] Patch that solves items 1, 2, 3, 4. 1. make_a_copy = true in stylecontext.ccg. 2. Added vfuncs in widget.[hg|ccg]. 3, 4. Added create() and some refreturn in cssprovider.hg. As requested in comment 5 I have not removed any const declarations. The new vfuncs are also const. The output parameters in get_preferred_xxx_vfunc are int*, not int&. That's because these functions are sometimes called from the C code with one of the pointers = NULL. Item 5 has probably been solved by a change in gtk+. See bug 639754. Item 6 remains, and the issues I mentioned in comment 2.
Created attachment 179091 [details] [review] gtkmm-documentation patch: custom_container and custom_widget The attached patch modifies the custom_container and custom_widget examples and the "Custom Widgets" chapter in "Programming with gtkmm". The new versions of the example programs depend on the gtkmm patch in comment 6. If my English in the Custom Widgets chapter needs some polishing, please go ahead and improve it. I tried to add at least a one-sentence description of each virtual method mentioned in the chapter, but I gave up on on_map() and on_unmap(). They have some relation to show() and hide(), but the exact relationship beats me. I have not updated custom_container.png and custom_widget.png. Am I right, if I think that many figures need updating, and that they will all be regenerated some time with a recommended theme, so they all get a good and consistent look-and-feel? If this patch and the one in comment 6 are both accepted, all issues in this bug have been fixed.
(In reply to comment #6) > Created an attachment (id=178710) [details] [review] > Patch that solves items 1, 2, 3, 4. It looks good, but could you tell me more about this, please: +// This vfunc is custom implemented, because the return value RType(), +// generated by _WRAP_VFUNC may not be appropriate. +SizeRequestMode Gtk::Widget::get_request_mode_vfunc() const
(In reply to comment #7) > Created an attachment (id=179091) [details] [review] > gtkmm-documentation patch: custom_container and custom_widget This look good. Thanks for taking the time to fix this. However, this makes me notice that we are using int* where int& would probably be nicer: + virtual void get_preferred_width_vfunc(int* minimum_width, int* natural_width) const; + virtual void get_preferred_height_for_width_vfunc(int width, int* minimum_height, int* natural_height) const;
(In reply to comment #8) > It looks good, but could you tell me more about this, please: > +// This vfunc is custom implemented, because the return value RType(), > +// generated by _WRAP_VFUNC may not be appropriate. > +SizeRequestMode Gtk::Widget::get_request_mode_vfunc() const I discussed this issue in comment 1, item 2. Consider this case: gtk_widget_get_request_mode(GtkWidget*) is called for a C++ widget derived from Gtk::Widget (actually of course for the underlying C widget). It will call Gtk::Widget_Class::get_request_mode_vfunc_callback(). If it's a custom widget which has not overridden get_request_mode_vfunc() or if it's a gtkmmproc- generated class, the virtual get_request_mode() method of the parent C class is called, if it's defined. If it's not defined (if base->get_request_mode == 0, where base is a pointer to the parent C class), then get_request_mode_vfunc_callback() returns a default value. Now it seems that base->get_request_mode == 0 is a legitimate and perhaps even common case in gtk+ widgets. For such widgets gtk_widget_get_request_mode() returns GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH. Gtk::Widget_Class::get_request_mode_vfunc_callback() and Gtk::Widget::get_request_mode_vfunc() shall do the same. The code generated by _WRAP_VFUNC ends with typedef GtkSizeRequestMode RType; return RType(); and typedef SizeRequestMode RType; return RType(); Now enum GtkSizeRequestMode is defined in such a way that RType() is actually GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH, so the code generated by _WRAP_VFUNC returns the same value as my slightly modified code. To me that seems just like a coincidence that we should not rely on. Or am I wrong? Would _WRAP_VFUNC be ok for get_request_mode_vfunc()? As discussed in comment 1, my version of get_request_mode_vfunc() is not perfect either. It assumes that we know the default value of gtk_widget_get_request_mode(). (In reply to comment #9) > However, this makes me notice that we are using int* where int& would probably > be nicer: I agree that int& is usually nicer than int*. In comment 6 I mentioned very briefly the reason for using int*. Widget_Class::get_preferred_width_vfunc_callback() and the other get_preferred_xxx_vfunc_callback() functions are called from gtk+. In some calls the pointer to one of the output arguments is NULL. It's even less nice to convert a NULL int* to an int& than it is to pass the int* directly to the get_preferred_xxx_vfunc() C++ functions.
Thanks. I commited both these patches, but I'm keeping this bug open until I have time to look at those issues in some more detail.
Yet another comment! I'm afraid that my comments in this bug are becoming so verbose that you, Murray, will never have time to read them. It's difficult to be both clear and concise, and I'm not even sure my comments are clear. (In reply to comment 8) > It looks good, but could you tell me more about this, please: > +// This vfunc is custom implemented, because the return value RType(), > +// generated by _WRAP_VFUNC may not be appropriate. > +SizeRequestMode Gtk::Widget::get_request_mode_vfunc() const Why bother with the last return statement in get_request_mode_vfunc[_callback]? In other vfuncs, no one cares much. The test if(base && base->gtk_virtual_func) is just a safety measure. It should never fail, both base and base->gtk_virtual_func should always be non-null. If they are null, something is fundamentally wrong, and it doesn't matter much what value is returned. Or am I wrong? Isn't this the normal case? get_request_mode is different. Many gtk+ widgets leave widget_class->get_request_mode == NULL, and accept the default value from gtk_widget_get_request_mode(). Therefore I think it's important that gtk_widget_get_request_mode() returns the same value when it's called on a gtkmm-generated widget as it does when it's called on a gtk+ widget of the corresponding type. (In reply to comment 9) > However, this makes me notice that we are using int* where int& would probably > be nicer: It's possible to avoid int* by modifying the code that _WRAP_VFUNC generates. Take Widget_Class::get_preferred_width_vfunc_callback as an example. Replace obj->get_preferred_width_vfunc(minimum_width, natural_width); by int min_width = 0; int nat_width = 0; obj->get_preferred_width_vfunc(min_width, nat_width); if (minimum_width) *minimum_width = min_width; if (natural_width) *natural_width = nat_width; or int min_width = 0; int nat_width = 0; obj->get_preferred_width_vfunc((minimum_width ? *minimum_width : min_width), (natural_width ? *natural_width : nat_width); Here get_preferred_width_vfunc() would take int& arguments instead of int*. (The initial values of min_width and nat_width are necessary only if there is a compiler that would print a warning message if they are not initialized.) If you think it's worth the trouble to do this in order to avoid int*, I can make the two patches that are required (one for gtkmm, the other for gtkmm-documentation). I've noticed that GtkCellArea and GtkCellRenderer, which don't derive from GtkWidget, also contain the virtual get_request_mode function and the 4 virtual get_preferred_xxx functions.
(In reply to comment #12) > The test > if(base && base->gtk_virtual_func) > is just a safety measure. It should never fail, both base and > base->gtk_virtual_func should always be non-null. If they are null, something > is fundamentally wrong, I don't think that's true. It's not unheard of for a vfunc to be NULL in a GObject. And it seems wise to be careful in case one is. What's the value in risking a NULL dereference? > get_request_mode is different. Many gtk+ widgets leave > widget_class->get_request_mode == NULL, So there you go. That proves my point. > and accept the default value from > gtk_widget_get_request_mode(). Therefore I think it's important that > gtk_widget_get_request_mode() returns the same value when it's called on a > gtkmm-generated widget as it does when it's called on a gtk+ widget of the > corresponding type. It's unwise for a GObject to test for NULLness of its vfunc. A correct default vfunc implementation would be easier for us. CCing Tristan, who has dealt with a similar issue in GTK+ before.
A default implementation for get_request_mode() could certainly be added to GTK+ with no harm done. But please note that you should not call the ->get_preferred_width() family of vfuncs directly except when you are calling it directly on the object you are implementing from inside another implementation of the ->get_preferred_height_for_width() vfunc family. This is documented here: http://library.gnome.org/devel/gtk/2.91/GtkWidget.html (around the text "Widget calling its own size request method") For similar reasons why it was incorrect to call ->size_request() vfunc directly on a widget in GTK+ 2.x o Direct calls to vfuncs of this family will not cache the results, which can be a bad performance hit. o Direct calls if made, are not complete requests, when requesting a child widget one would need to use GtkWidgetClass->adjust_size_request() afterwards. o When calling ->get_preferred_height_for_width() vfunc, the 'for_width' argument needs to be treated by GtkWidgetClass->adjust_size_allocation() before sending that for_width to the implementing class (this is to compensate for changes made by ->adjust_size_request() and to cap the for_width value at the natural width for the case that GtkWidget:xalign is not GTK_ALIGN_FILL). All in all, it's very important to pass through the gtk_widget_get_preferred_height_for_width() APIs rather than calling the vfuncs directly.
While it's important to pay attention to the fact that you *must* call the wrapper functions for those APIs, it's possible I missed the point here. If you are wrapping the widget's vfuncs and chaining up to the native C implementation I suppose the above text is irrelevant. So long as when a caller calls widget.get_preferred_width(), gtk_widget_get_preferred_width() is actually called, and then the C++ wrapper vfunc invoked by virtue of it being a derived class, and then possibly chaining up. As I mentioned, if it's any help I can easily add a default implementation for get_request_mode() instead of casing it from the wrapper function.
> As I mentioned, if it's any help I can easily add a default > implementation for get_request_mode() instead of casing > it from the wrapper function. Yes, that seems safer than putting that code in a check for a NULL vfunc. I don't think we are talking about calling the vfunc directly from an application, or even from outside the class itself.
I added a default implementation to GtkWidgetClass that returns the default GTK_SIZE_REQUEST_HEIGHT_FOR_WIDTH instead of checking the vfunc from gtk_widget_get_request_mode() today. Hope this helps.
Thanks, Tristan. (It was http://git.gnome.org/browse/gtk+/commit/?id=5a5854f6f64231be8df028dbcb87f75a8025609e ) So I have now simplified that code in widget.[hg|ccg] to just use _WRAP_VFUNC(): http://git.gnome.org/browse/gtkmm/commit/?id=3fadc846ec4fb1ce9736a96919a372380e006449
(In reply to comment #12) > If you think it's worth the trouble to do this in order to avoid int*, I can > make the two patches that are required (one for gtkmm, the other for > gtkmm-documentation). That would be nice. Thanks. I guess you'd need to hand-code the vfunc wrappers, because it would be hard to make gmmproc guess when to do this. > I've noticed that GtkCellArea and GtkCellRenderer, which don't derive from > GtkWidget, also contain the virtual get_request_mode function and the 4 > virtual get_preferred_xxx functions. Ah, then we should wrap those vfuncs too, in the same way.
Created attachment 179635 [details] [review] gtkmm patch: Change int* to int& in get_preferred_xxx_vfunc(). (In reply to comment 19) > I guess you'd need to hand-code the vfunc wrappers, because it would be hard to > make gmmproc guess when to do this. I've hand-coded the vfunc_callbacks, but not the vfuncs. The vfuncs can be generated by _VFUNC_CC. _WRAP_SIGNAL understands the optional arguments custom_default_handler, no_default_handler and custom_c_callback. If _WRAP_VFUNC had understood custom_vfunc_callback (and custom_vfunc, although not necessary in this case), I could have used _WRAP_VFUNC instead of the more obscure m4 macros _VFUNC_PH and VFUNC_CC. I could perhaps file a bug with a suggestion for a future enhancement of gtkmm-proc. But I don't know where to file such a bug. In Bugzilla glibmm does not contain a 'tools' component.
Created attachment 179636 [details] [review] gtkmm-documentation patch: Change int* to int& in get_preferred_xxx_vfunc(). I've included two changes in addition to the change from int* to int&. 1. Added '*~' in .gitignore, to make git ignore gedit's backup files. 2. Added 'mm-common-prepare --version' in autogen.sh, as suggested in bug 628713. If this patch is committed, bug 628713 can be closed.
(In reply to comment 19) > (In reply to comment 12) >> I've noticed that GtkCellArea and GtkCellRenderer, which don't derive from >> GtkWidget, also contain the virtual get_request_mode function and the 4 >> virtual get_preferred_xxx functions. > Ah, then we should wrap those vfuncs too, in the same way. -- Tristan, if you still read comments in this bug: What you did in GtkWidgetClass helps gtkmm to make the C++ wrappers behave just like the underlying C classes. I.e. if a virtual function (e.g. get_xyz) is supposed to be called via gtk_baseclassname_get_xyz(), then put the default action in gtk_baseclassname_real_get_xyz(), and store a pointer to it in the virtual function table. In the cases where the virtual function returns void, and there is no default action, it's ok to have a NULL pointer in the vfunc table, but in other cases it simplifies for gtkmm if the default action is put in a function reached from the vfunc table. Now there are NULL vfunc pointers also in GtkCellArea and GtkCellRenderer. At least these ones: GtkCellArea::get_request_mode, get_preferred_width, get_preferred_height, GtkCellRenderer::get_request_mode, but there may be others too. (get_preferred_[width|height] return void, but there is still a default action in GtkCellArea, consisting of a warning message.) And I think I've found an error in gtk_cell_area_real_get_preferred_height_for_width(). It calls get_preferred_width(), but I think it should be get_preferred_height(). -- The rest of this comment is probably of no interest to gtk+ developers. Ok, I can add get_request_mode_vfunc() and get_preferred_xxx_vfunc() in CellArea and CellRenderer, but after that I think it's time to close this bug, although there are more functions in CellArea that have not been wrapped in C++ code. Just a question before I supply the 5th patch in this bug: The non-virtual CellArea::get_preferred_xxx() functions are non-const. Is that correct? Shall the virtual CellArea::get_preferred_xxx_vfunc() also be non-const? And an observation: GtkCellRenderer, GtkTreeViewColumn and GtkWidget are derived from GInitiallyUnowned in gtk+ 3. In gtk+ 2 they are derived from GtkObject. Gtk::CellRenderer, Gtk::TreeViewColumn and Gtk::Widget are still derived from Gtk::Object.
The first part of comment 22 is incomplete. Let me correct myself: From gtkmm's point of view it's ok to have a NULL pointer in the vfunc table if - either the virtual function returns void, and there is no default action, - or the default action consists of returning one of the "natural" default values 0, 0.0, FALSE, NULL (function's return value, not values set in output arguments). In other cases it simplifies for gtkmm if the default action is put in a function reached from the vfunc table.
Tristan, it would be helpful if you could make the same fixes to these virtual functions.
(In reply to comment #23) > Shall the virtual CellArea::get_preferred_xxx_vfunc() also be non-const? Sure, that would be good. > And an observation: GtkCellRenderer, GtkTreeViewColumn and GtkWidget are > derived from GInitiallyUnowned in gtk+ 3. In gtk+ 2 they are derived from > GtkObject. Gtk::CellRenderer, Gtk::TreeViewColumn and Gtk::Widget are still > derived from Gtk::Object. Yes, Gtk::Object in gtkmm 3 is a hacky class that doesn't correspond directly to any GtkObject type in GTK+. It lets us not force the use of RefPtr for these. I might change my mind about using it for CellRenderer. > I could perhaps file a bug with a suggestion for a future > enhancement of gtkmm-proc. But I don't know where to file such a bug. In > Bugzilla glibmm does not contain a 'tools' component. So just use any component - that's not a big deal. But will this even be necessary if Tristan fixes the GTK+ vfuncs?
(In reply to comment #21) > 2. Added 'mm-common-prepare --version' in autogen.sh, as suggested in > bug 628713. If this patch is committed, bug 628713 can be closed. I have committed that separately. Thanks. It was an unrelated bug so it should not be in the same commit.
Created attachment 179976 [details] [review] gtkmm patch: Widget, CellArea, CellRenderer: Modify and add some vfuncs. This patch replaces the patch in comment 20. - CellArea and CellRenderer: Add vfuncs get_request_mode, get_preferred_[width|height_for_width|height|width_for_height]. - Widget: Change int* to int& in get_preferred_[width|height_for_width|height|width_for_height]_vfunc. This patch uses the _WRAP_VFUNC argument custom_vfunc_callback, implemented in glibmm bug 641165. This patch can be used even if GtkCellArea and GtkCellRenderer are not changed as requested in comment 22 and comment 24. But then there would be slight differences between Gtk::Cell[Area|Renderer] and GtkCell[Area|Renderer]: - If anyone derives a custom CellArea from Gtk::CellArea and does not override get_request_mode, get_preferred_width and get_preferred_height, there will be no warning messages. - If the default value of GtkCell[Area|Renderer]::get_request_mode() will ever be changed (very unlikely, I think), then the default value of Gtk::Cell[Area|Renderer]::get_request_mode() will not automatically change accordingly. Shall I also make a replacement for the gtkmm-documentation patch in comment 21? A patch that does not change autogen.sh?
Thanks. To avoid too much code churn, I'll wait until Tristan can fix GTK+. Hopefully he'll get to that soon. > Shall I also make a replacement for the gtkmm-documentation patch in > comment 21? A patch that does not change autogen.sh? If you like, but it won't be a big problem to remove that later.
(In reply to comment #22) > (In reply to comment 19) > > (In reply to comment 12) > >> I've noticed that GtkCellArea and GtkCellRenderer, which don't derive from > >> GtkWidget, also contain the virtual get_request_mode function and the 4 > >> virtual get_preferred_xxx functions. > > > Ah, then we should wrap those vfuncs too, in the same way. > > -- Tristan, if you still read comments in this bug: > > What you did in GtkWidgetClass helps gtkmm to make the C++ wrappers behave > just like the underlying C classes. I.e. if a virtual function (e.g. get_xyz) > is supposed to be called via gtk_baseclassname_get_xyz(), then put the default > action in gtk_baseclassname_real_get_xyz(), and store a pointer to it in the > virtual function table. In the cases where the virtual function returns void, > and there is no default action, it's ok to have a NULL pointer in the vfunc > table, but in other cases it simplifies for gtkmm if the default action is put > in a function reached from the vfunc table. > > Now there are NULL vfunc pointers also in GtkCellArea and GtkCellRenderer. > At least these ones: GtkCellArea::get_request_mode, get_preferred_width, > get_preferred_height, GtkCellRenderer::get_request_mode, but there may be > others too. (get_preferred_[width|height] return void, but there is still a > default action in GtkCellArea, consisting of a warning message.) > > And I think I've found an error in > gtk_cell_area_real_get_preferred_height_for_width(). It calls > get_preferred_width(), but I think it should be get_preferred_height(). > Indeed, thanks for pointing that out, fixed. I just committed something that should hopefully make binding GtkCellArea a bit easier, As GtkCellArea and GtkCellRenderer are abstract classes they leave a few things unimplemented, now instead of checking the vfunc I've provided a default implementation for these vfuncs that simply prints a warning (the same warning that would have been printed had the vfunc not been there). I've done this for the following cases: GtkCellRendererClass.get_request_mode GtkCellAreaClass.get_request_mode GtkCellAreaClass.create_context GtkCellAreaClass.copy_context GtkCellAreaClass.get_preferred_height GtkCellAreaClass.get_preferred_width GtkCellAreaClass.add GtkCellAreaClass.remove GtkCellAreaClass.foreach GtkCellAreaClass.foreach_alloc GtkCellAreaClass.focus The special case of GtkCellRendererClass.get_size remains unimplemented, this case is a little odd because it's a deprecated api that we fall back on in the case that .get_preferred_xxx apis are still unimplemented. The relevant commits are: ce000db7f65a2066ea2c7e1806ec5830a37ee27f and b01fc35c189c8f934e0a63d09691ce1a6d7e01db Hope this helps in the quest to wrap GtkCellArea classes in gtkmm.
Thanks, Tristan. > The special case of GtkCellRendererClass.get_size remains > unimplemented, this case is a little odd because it's a deprecated > api that we fall back on in the case that .get_preferred_xxx apis > are still unimplemented. Then we should not wrap that vfunc in gtkmm at all. We are trying to not wrap any deprecated API in gtkmm 3.0.0.
Murray, are you waiting for me to do anything here? I decided not to replace the gtkmm-documentation patch in comment 21, when you said it's good enough. The only possible problem I can think of with the gtkmm patch in comment 27 is that it depends on the upgrade of _WRAP_VFUNC in bug 641165. That is, anyone who wants to build gtkmm with the patch must first update glibmm.
Thanks. Sorry. Yes, I've pushed this now. And I've checked that we do not wrap the get_size() vfunc. So this bug is all fixed, right? Many thanks.
But does this now need some corresponding change in gtkmm-documentation's examples?
(In reply to comment #33) > But does this now need some corresponding change in gtkmm-documentation's > examples? Yes, that's done by the patch in comment 21, minus the change of autogen.sh that you've already committed (comment 26). When the rest of the patch in comment 21 has been committed, this bug is really fixed. (The addition of *~ in .gitignore is optional, you can omit it if you don't like it.)
Thanks. I have pushed that now.