Re: cellrenderer GET_SIZE super method
- From: Torsten Schoenfeld <kaffeetisch gmx de>
- To: gtk-perl-list gnome org
- Subject: Re: cellrenderer GET_SIZE super method
- Date: Sun, 29 Jun 2008 19:20:06 +0200
Kevin Ryde wrote:
I struck a bit of trouble with Gtk2::Ex::Datasheet::DBI not having a
GET_SIZE method in its Gtk2::Ex::Datasheet::DBI::CellRendererText
package. (RT 36996)
Nosing around the GtkCellRenderer.xs fallback GET_SIZE, I wondered if it
could look up through the classes of the given object instead of using
"caller". The first superclass which had class->get_size func not equal
to gtk2perl_cell_renderer_get_size would be the one to call, would it?
This is actually a very long-standing problem. And your suggestion of looking
for the first non-Perl class in the ancestry might very well solve most of the
issues!
The attached caller-v1.patch was my first attempt at this from four years (!)
ago. It did seem to fix most of the problems I saw, but it fails in one
important case, as muppet explained back then: you get endless loops when you
have a hierarchy where a Perl class inherits from a Perl class. Something
like this:
...
+- GtkCellRenderer
+- Foo::RendererOne
+- Foo::RendererTwo
When something calls the get_size vfunc on a Foo::RendererTwo,
Foo::RendererTwo::GET_SIZE is invoked if present. If it's not present, or if
it chains up, then Foo::RendererOne::GET_SIZE is called. If this one is not
present, or if it chains up, our fallback Gtk2::CellRenderer::GET_SIZE kicks
in. The fallback uses G_OBJECT_TYPE() on the object, and since
G_OBJECT_TYPE() always returns the bottommost type, this yields the type of
Foo::RendererTwo. The fallback then finds out that Foo::RendererOne is the
parent of Foo::RendererTwo and goes on to call Foo::RendererOne::GET_SIZE
which eventually gets us into Gtk2::CellRenderer::GET_SIZE again. Endless loop.
So, if both subclasses either don't provide GET_SIZE or if they both chain up
so that Gtk2::CellRenderer::GET_SIZE is reached, we get an endless loop with
caller-v1.patch due to G_OBJECT_TYPE() always returning the bottommost type.
This was the original reason of using caller() instead of simply G_OBJECT_TYPE().
But as far as I can tell, your suggestion of chaining up to the first parent
that is not a Perl subclass does indeed fix this. It's implemented and tested
in caller-v2.patch. The patch contains some more rationale of why I think
this works. The patch also contains a FIXME comment detailing a scenario
where this approach still results in an endless loop: a Perl class that
inherits from a C class that inherits from a Perl class.
But since this scenario is rather contrived, and since the current
caller()-using code will fail in this case just as well, and since your new
suggestion does fix another problem, I think we should commit caller-v2.patch.
Daniel, Kevin: can you verify that caller-v2.patch fixes the
Gtk2::Ex::Datasheet::DBI::CellRendererText issue even when you revert the
GET_SIZE "no-op" fix you applied already?
muppet: what do you think?
--
Bye,
-Torsten
Index: xs/GtkCellRenderer.xs
===================================================================
RCS file: /cvsroot/gtk2-perl/gtk2-perl-xs/Gtk2/xs/GtkCellRenderer.xs,v
retrieving revision 1.32
diff -u -d -p -r1.32 GtkCellRenderer.xs
--- xs/GtkCellRenderer.xs 7 Jan 2008 19:54:49 -0000 1.32
+++ xs/GtkCellRenderer.xs 29 Jun 2008 14:41:10 -0000
@@ -653,23 +653,16 @@ GET_SIZE (GtkCellRenderer * cell, ...)
Gtk2::CellRenderer::parent_start_editing = 7
PREINIT:
GtkCellRendererClass * class;
- GType thisclass, parent_class;
- SV * saveddefsv;
+ GType this, parent;
PPCODE:
- /* may i ask who's calling? */
- saveddefsv = newSVsv (DEFSV);
- eval_pv ("$_ = caller;", 0);
- thisclass = gperl_type_from_package (SvPV_nolen (DEFSV));
- SvSetSV (DEFSV, saveddefsv);
- if (!thisclass)
- thisclass = G_OBJECT_TYPE (cell);
/* look up his parent */
- parent_class = g_type_parent (thisclass);
- if (! g_type_is_a (parent_class, GTK_TYPE_CELL_RENDERER))
+ this = G_OBJECT_TYPE (cell);
+ parent = g_type_parent (this);
+ if (! g_type_is_a (parent, GTK_TYPE_CELL_RENDERER))
croak ("parent of %s is not a GtkCellRenderer",
- g_type_name (thisclass));
+ g_type_name (this));
/* that's our boy. call one of his functions. */
- class = g_type_class_peek (parent_class);
+ class = g_type_class_peek (parent);
switch (ix) {
case 4: /* deprecated parent_get_size */
case 0: /* GET_SIZE */
? t/GtkCellRendererIface-Chaining.t
Index: xs/GtkCellRenderer.xs
===================================================================
RCS file: /cvsroot/gtk2-perl/gtk2-perl-xs/Gtk2/xs/GtkCellRenderer.xs,v
retrieving revision 1.32
diff -u -d -p -r1.32 GtkCellRenderer.xs
--- xs/GtkCellRenderer.xs 7 Jan 2008 19:54:49 -0000 1.32
+++ xs/GtkCellRenderer.xs 29 Jun 2008 16:52:00 -0000
@@ -652,36 +652,63 @@ GET_SIZE (GtkCellRenderer * cell, ...)
Gtk2::CellRenderer::parent_activate = 6
Gtk2::CellRenderer::parent_start_editing = 7
PREINIT:
- GtkCellRendererClass * class;
- GType thisclass, parent_class;
- SV * saveddefsv;
+ GtkCellRendererClass *parent_class = NULL;
+ GType this, parent;
PPCODE:
- /* may i ask who's calling? */
- saveddefsv = newSVsv (DEFSV);
- eval_pv ("$_ = caller;", 0);
- thisclass = gperl_type_from_package (SvPV_nolen (DEFSV));
- SvSetSV (DEFSV, saveddefsv);
- if (!thisclass)
- thisclass = G_OBJECT_TYPE (cell);
- /* look up his parent */
- parent_class = g_type_parent (thisclass);
- if (! g_type_is_a (parent_class, GTK_TYPE_CELL_RENDERER))
- croak ("parent of %s is not a GtkCellRenderer",
- g_type_name (thisclass));
- /* that's our boy. call one of his functions. */
- class = g_type_class_peek (parent_class);
+ /* look up the parent.
+ *
+ * FIXME: this approach runs into an endless loop with a hierarchy
+ * where a Perl class inherits from a C class which inherits from a
+ * Perl class. Like this:
+ *
+ * ...
+ * +- GtkCellRenderer
+ * +- Foo::RendererOne (Perl subclass)
+ * +- FooRendererTwo (C subclass)
+ * +- Foo::RendererThree (Perl subclass)
+ *
+ * yes, this is contrived. But possible!
+ */
+ this = G_OBJECT_TYPE (cell);
+ while ((parent = g_type_parent (this))) {
+ if (! g_type_is_a (parent, GTK_TYPE_CELL_RENDERER))
+ croak ("parent of %s is not a GtkCellRenderer",
+ g_type_name (this));
+
+ parent_class = g_type_class_peek (parent);
+
+ /* check if this class isn't actually one of ours. if it is a
+ * Perl class, then we must not chain up to it: if it had a sub
+ * defined for the current vfunc, we wouldn't be in this
+ * fallback one here; so chaining up would result in the
+ * fallback being called again. this will lead to an endless
+ * loop.
+ *
+ * so, if it's not a Perl class, we're done. if it is,
+ * continue in the while loop to the next parent. */
+ if (parent_class->get_size != gtk2perl_cell_renderer_get_size) {
+ break;
+ }
+
+ this = parent;
+ }
+
+ /* the ancestry will always contain GtkCellRenderer, so parent and
+ * parent_class should never be NULL. */
+ assert (parent != NULL && parent_class != NULL);
+
switch (ix) {
case 4: /* deprecated parent_get_size */
case 0: /* GET_SIZE */
- if (class->get_size) {
+ if (parent_class->get_size) {
gint x_offset, y_offset, width, height;
- class->get_size (cell,
- SvGtkWidget (ST (1)),
- SvGdkRectangle_ornull (ST (2)),
- &x_offset,
- &y_offset,
- &width,
- &height);
+ parent_class->get_size (cell,
+ SvGtkWidget (ST (1)),
+ SvGdkRectangle_ornull (ST (2)),
+ &x_offset,
+ &y_offset,
+ &width,
+ &height);
EXTEND (SP, 4);
PUSHs (sv_2mortal (newSViv (x_offset)));
PUSHs (sv_2mortal (newSViv (y_offset)));
@@ -691,41 +718,41 @@ GET_SIZE (GtkCellRenderer * cell, ...)
break;
case 5: /* deprecated parent_render */
case 1: /* RENDER */
- if (class->render)
- class->render (cell,
- SvGdkDrawable_ornull (ST (1)), /* drawable */
- SvGtkWidget_ornull (ST (2)), /* widget */
- SvGdkRectangle_ornull (ST (3)), /* background_area */
- SvGdkRectangle_ornull (ST (4)), /* cell_area */
- SvGdkRectangle_ornull (ST (5)), /* expose_area */
- SvGtkCellRendererState (ST (6))); /* flags */
+ if (parent_class->render)
+ parent_class->render (cell,
+ SvGdkDrawable_ornull (ST (1)), /* drawable */
+ SvGtkWidget_ornull (ST (2)), /* widget */
+ SvGdkRectangle_ornull (ST (3)), /* background_area */
+ SvGdkRectangle_ornull (ST (4)), /* cell_area */
+ SvGdkRectangle_ornull (ST (5)), /* expose_area */
+ SvGtkCellRendererState (ST (6))); /* flags */
break;
case 6: /* deprecated parent_activate */
case 2: /* ACTIVATE */
- if (class->activate) {
+ if (parent_class->activate) {
gboolean ret;
- ret = class->activate (cell,
- SvGdkEvent (ST (1)),
- SvGtkWidget (ST (2)),
- SvGChar (ST (3)),
- SvGdkRectangle_ornull (ST (4)),
- SvGdkRectangle_ornull (ST (5)),
- SvGtkCellRendererState (ST (6)));
+ ret = parent_class->activate (cell,
+ SvGdkEvent (ST (1)),
+ SvGtkWidget (ST (2)),
+ SvGChar (ST (3)),
+ SvGdkRectangle_ornull (ST (4)),
+ SvGdkRectangle_ornull (ST (5)),
+ SvGtkCellRendererState (ST (6)));
EXTEND (SP, 1);
PUSHs (sv_2mortal (newSViv (ret)));
}
break;
case 7: /* deprecated parent_start_editing */
case 3: /* START_EDITING */
- if (class->start_editing) {
+ if (parent_class->start_editing) {
GtkCellEditable * editable;
- editable = class->start_editing (cell,
- SvGdkEvent_ornull (ST (1)),
- SvGtkWidget (ST (2)),
- SvGChar (ST (3)),
- SvGdkRectangle_ornull (ST (4)),
- SvGdkRectangle_ornull (ST (5)),
- SvGtkCellRendererState (ST (6)));
+ editable = parent_class->start_editing (cell,
+ SvGdkEvent_ornull (ST (1)),
+ SvGtkWidget (ST (2)),
+ SvGChar (ST (3)),
+ SvGdkRectangle_ornull (ST (4)),
+ SvGdkRectangle_ornull (ST (5)),
+ SvGtkCellRendererState (ST (6)));
EXTEND (SP, 1);
PUSHs (sv_2mortal (newSVGtkCellEditable_ornull (editable)));
}
--- /dev/null 2008-04-13 20:57:27.000000000 +0200
+++ t/GtkCellRendererIface-Chaining.t 2008-06-29 18:10:19.000000000 +0200
@@ -0,0 +1,58 @@
+#!/usr/bin/perl
+
+# $Id$
+
+use Gtk2::TestHelper tests => 2;
+
+package CellRendererFoo;
+
+use Glib::Object::Subclass
+ Gtk2::CellRendererText::,
+ ;
+
+our $hits = 0;
+
+sub GET_SIZE {
+ #warn __PACKAGE__;
+ $hits++;
+ if ($hits > 50) {
+ die 'Overflow';
+ }
+ shift->SUPER::GET_SIZE (@_);
+}
+
+package CellRendererBar;
+
+use Glib::Object::Subclass
+ CellRendererFoo::,
+ ;
+
+our $hits = 0;
+
+sub GET_SIZE {
+ #warn __PACKAGE__;
+ $hits++;
+ shift->SUPER::GET_SIZE (@_);
+}
+
+package main;
+
+my $model = Gtk2::ListStore->new ('Glib::String');
+foreach (qw/foo fluffy flurble frob frobnitz ftang fire/) {
+ my $iter = $model->append;
+ $model->set ($iter, 0 => $_);
+}
+
+my $view = Gtk2::TreeView->new ($model);
+
+my $renderer = CellRendererBar->new;
+my $column = Gtk2::TreeViewColumn->new_with_attributes ('F-Words', $renderer,
+ text => 0);
+$view->append_column ($column);
+
+my $window = Gtk2::Window->new;
+$window->add ($view);
+
+ok (eval { $window->show_all; 1; }, 'no overflow');
+ok ($CellRendererFoo::hits == $CellRendererBar::hits,
+ 'both classes were hit just as often');
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
