From 167475e422dad0b9ace6ebe750d654e227e5009a Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Fri, 4 Jul 2008 10:44:18 +0000 Subject: [PATCH] Bug 1015 - Cloning unparented actors with FBOs doesn't work with new layout code * clutter/clutter-texture.c (clutter_texture_new_from_actor): Now parents the source actor if it doesn't already have a parent so that it will get an allocation during layout. * tests/test-fbo.c: One of the tests tries to ensure that the ClutterTexture clone keeps the source actor alive by derefing it. However as actors have a floating reference then test-fbo doesn't have its own reference once the source is parented so unrefing just steals the parent's reference and causes badness. The test now claims the floating reference before cloning the source so that it can safely be unref'd later. --- ChangeLog | 17 ++++++++++++++ clutter/clutter-texture.c | 48 +++++++++++++++++++++++++++++++++++---- tests/test-fbo.c | 2 ++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6a8ed7f56..14bbcafbf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,20 @@ +2008-07-04 Neil Roberts + + Bug 1015 - Cloning unparented actors with FBOs doesn't work with + new layout code + + * clutter/clutter-texture.c (clutter_texture_new_from_actor): Now + parents the source actor if it doesn't already have a parent so + that it will get an allocation during layout. + + * tests/test-fbo.c: One of the tests tries to ensure that the + ClutterTexture clone keeps the source actor alive by derefing + it. However as actors have a floating reference then test-fbo + doesn't have its own reference once the source is parented so + unrefing just steals the parent's reference and causes + badness. The test now claims the floating reference before cloning + the source so that it can safely be unref'd later. + 2008-07-03 Emmanuele Bassi * clutter/x11/Makefile.am: Fix distchecking by adding the diff --git a/clutter/clutter-texture.c b/clutter/clutter-texture.c index a84e170cb..f8d2b884e 100644 --- a/clutter/clutter-texture.c +++ b/clutter/clutter-texture.c @@ -413,6 +413,23 @@ clutter_texture_get_preferred_height (ClutterActor *self, } } +static void +clutter_texture_allocate (ClutterActor *self, + const ClutterActorBox *box, + gboolean origin_changed) +{ + ClutterTexturePrivate *priv = CLUTTER_TEXTURE (self)->priv; + + /* chain up to set actor->allocation */ + CLUTTER_ACTOR_CLASS (clutter_texture_parent_class)->allocate (self, box, + origin_changed); + + /* If we adopted the source fbo then allocate that at its preferred + size */ + if (priv->fbo_source && clutter_actor_get_parent (priv->fbo_source) == self) + clutter_actor_allocate_preferred_size (priv->fbo_source, origin_changed); +} + static void clutter_texture_paint (ClutterActor *self) { @@ -651,8 +668,9 @@ clutter_texture_class_init (ClutterTextureClass *klass) actor_class->realize = clutter_texture_realize; actor_class->unrealize = clutter_texture_unrealize; - actor_class->get_preferred_width = clutter_texture_get_preferred_width; + actor_class->get_preferred_width = clutter_texture_get_preferred_width; actor_class->get_preferred_height = clutter_texture_get_preferred_height; + actor_class->allocate = clutter_texture_allocate; gobject_class->dispose = clutter_texture_dispose; gobject_class->set_property = clutter_texture_set_property; @@ -1644,8 +1662,19 @@ on_fbo_parent_change (ClutterActor *actor, * * * The source actor must be made visible (i.e by calling - * #clutter_actor_show). The source actor does not however have to - * have a parent. + * #clutter_actor_show). + * + * + * The source actor must have a parent in order for it to be + * allocated a size from the layouting mechanism. If the source + * actor does not have a parent when this function is called then + * the ClutterTexture will adopt it and allocate it at its + * preferred size. Using this you can clone an actor that is + * otherwise not displayed. Because of this feature if you do + * intend to display the source actor then you must make sure that + * the actor is parented before calling + * clutter_texture_new_from_actor() or that you unparent it before + * adding it to a container. * * * Avoid reparenting the source with the created texture. @@ -1709,7 +1738,12 @@ clutter_texture_new_from_actor (ClutterActor *actor) priv = texture->priv; - priv->fbo_source = g_object_ref(actor); + priv->fbo_source = g_object_ref_sink (actor); + + /* If the actor doesn't have a parent then claim it so that it will + get a size allocation during layout */ + if (clutter_actor_get_parent (actor) == NULL) + clutter_actor_set_parent (actor, CLUTTER_ACTOR (texture)); /* Connect up any signals which could change our underlying size */ g_signal_connect (actor, @@ -1766,6 +1800,12 @@ texture_fbo_free_resources (ClutterTexture *texture) if (priv->fbo_source != NULL) { + /* If we parented the texture then unparent it again so that it + will lose the reference */ + if (clutter_actor_get_parent (priv->fbo_source) + == CLUTTER_ACTOR (texture)) + clutter_actor_unparent (priv->fbo_source); + g_signal_handlers_disconnect_by_func (priv->fbo_source, G_CALLBACK(on_fbo_parent_change), diff --git a/tests/test-fbo.c b/tests/test-fbo.c index 47588a25c..5b32428fc 100644 --- a/tests/test-fbo.c +++ b/tests/test-fbo.c @@ -166,6 +166,8 @@ main (gint argc, /* non visual breaks */ foo_source = make_source(); + g_object_ref_sink (foo_source); + clutter_actor_show_all (foo_source); if ((fbo = clutter_texture_new_from_actor (foo_source)) == NULL) g_error("foo fbo creation failed");