From 4f470b9231275c6966bde054f1eaf5f57bdd88f8 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 19 Dec 2011 14:37:42 +0000 Subject: [PATCH] actor: Provide a proper implementation of replace_child() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The correct sequence of actions should be remove(old) → insert(new), not insert(new) → remove(old). We can implement a simple delegate insertion functions to insert the new child between the previous and next siblings of the old child. While we're at it, let's also add a unit test for replace_child(). --- clutter/clutter-actor.c | 74 +++++++++++++++++++++++++++++-- tests/conform/test-actor-graph.c | 60 +++++++++++++++++++++++++ tests/conform/test-conform-main.c | 1 + 3 files changed, 131 insertions(+), 4 deletions(-) diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c index 645aaa8b6..7f4928b1f 100644 --- a/clutter/clutter-actor.c +++ b/clutter/clutter-actor.c @@ -9542,6 +9542,27 @@ typedef void (* ClutterActorAddChildFunc) (ClutterActor *parent, ClutterActor *child, gpointer data); +/*< private > + * clutter_actor_add_child_internal: + * @self: a #ClutterActor + * @child: a #ClutterActor + * @add_func: delegate function + * @data: (closure): data to pass to @add_func + * @create_meta: whether this function should create a #ClutterChildMeta + * instance through the #ClutterContainer interface + * @emit_signal: whether this function should emit the + * #ClutterContainer::actor-added signal + * + * Adds @child to the list of children of @self. + * + * The actual insertion inside the list is delegated to @add_func: this + * function will just set up the state, perform basic checks, and emit + * signals. + * + * For backward compatibility with #ClutterContainer and the old + * #ClutterActor API, this function can optionally emit the Container + * signals and create #ClutterChildMeta instances. + */ static void clutter_actor_add_child_internal (ClutterActor *self, ClutterActor *child, @@ -9821,6 +9842,12 @@ clutter_actor_set_parent (ClutterActor *self, g_return_if_fail (self != parent); g_return_if_fail (self->priv->parent == NULL); + /* as this function will be called inside ClutterContainer::add + * implementations or when building up a composite actor, we have + * to preserve the old behaviour, and not create child meta or + * emit the ::actor-added signal, to avoid recursion or double + * emissions + */ clutter_actor_add_child_internal (parent, self, insert_child_at_depth, NULL, @@ -10009,6 +10036,36 @@ clutter_actor_remove_child (ClutterActor *self, clutter_actor_remove_child_internal (self, child, TRUE, TRUE); } +typedef struct _InsertBetweenData { + ClutterActor *prev_sibling; + ClutterActor *next_sibling; +} InsertBetweenData; + +static void +insert_child_between (ClutterActor *self, + ClutterActor *child, + gpointer data_) +{ + InsertBetweenData *data = data_; + ClutterActor *prev_sibling = data->prev_sibling; + ClutterActor *next_sibling = data->next_sibling; + + child->priv->prev_sibling = prev_sibling; + child->priv->next_sibling = next_sibling; + + if (prev_sibling != NULL) + prev_sibling->priv->next_sibling = child; + + if (next_sibling != NULL) + next_sibling->priv->prev_sibling = child; + + if (child->priv->prev_sibling == NULL) + self->priv->first_child = child; + + if (child->priv->next_sibling == NULL) + self->priv->last_child = child; +} + /** * clutter_actor_replace_child: * @self: a #ClutterActor @@ -10024,6 +10081,9 @@ clutter_actor_replace_child (ClutterActor *self, ClutterActor *old_child, ClutterActor *new_child) { + ClutterActor *prev_sibling, *next_sibling; + InsertBetweenData clos; + g_return_if_fail (CLUTTER_IS_ACTOR (self)); g_return_if_fail (CLUTTER_IS_ACTOR (old_child)); g_return_if_fail (old_child->priv->parent == self); @@ -10032,11 +10092,17 @@ clutter_actor_replace_child (ClutterActor *self, g_return_if_fail (new_child != self); g_return_if_fail (new_child->priv->parent == NULL); - clutter_actor_add_child_internal (self, new_child, - insert_child_above, - old_child, - TRUE, TRUE); + prev_sibling = old_child->priv->prev_sibling; + next_sibling = old_child->priv->next_sibling; + clutter_actor_remove_child_internal (self, old_child, TRUE, TRUE); + + clos.prev_sibling = prev_sibling; + clos.next_sibling = next_sibling; + clutter_actor_add_child_internal (self, new_child, + insert_child_between, + &clos, + TRUE, TRUE); } /** diff --git a/tests/conform/test-actor-graph.c b/tests/conform/test-actor-graph.c index bae546197..d1b4b865c 100644 --- a/tests/conform/test-actor-graph.c +++ b/tests/conform/test-actor-graph.c @@ -251,3 +251,63 @@ actor_lower_child (TestConformSimpleFixture *fixture, clutter_actor_destroy (actor); g_object_unref (actor); } + +void +actor_replace_child (TestConformSimpleFixture *fixture, + gconstpointer dummy) +{ + ClutterActor *actor = clutter_actor_new (); + ClutterActor *iter; + + g_object_ref_sink (actor); + + clutter_actor_add_child (actor, g_object_new (CLUTTER_TYPE_ACTOR, + "name", "foo", + NULL)); + clutter_actor_add_child (actor, g_object_new (CLUTTER_TYPE_ACTOR, + "name", "bar", + NULL)); + + iter = clutter_actor_get_child_at_index (actor, 0); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "foo"); + + clutter_actor_replace_child (actor, iter, + g_object_new (CLUTTER_TYPE_ACTOR, + "name", "baz", + NULL)); + + iter = clutter_actor_get_child_at_index (actor, 0); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "baz"); + + iter = clutter_actor_get_child_at_index (actor, 1); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "bar"); + + clutter_actor_replace_child (actor, iter, + g_object_new (CLUTTER_TYPE_ACTOR, + "name", "qux", + NULL)); + + iter = clutter_actor_get_child_at_index (actor, 0); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "baz"); + + iter = clutter_actor_get_child_at_index (actor, 1); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "qux"); + + clutter_actor_add_child (actor, g_object_new (CLUTTER_TYPE_ACTOR, + "name", "foo")); + + clutter_actor_replace_child (actor, iter, + g_object_new (CLUTTER_TYPE_ACTOR, + "name", "bar", + NULL)); + + iter = clutter_actor_get_last_child (actor); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "foo"); + iter = clutter_actor_get_previous_sibling (iter); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "bar"); + iter = clutter_actor_get_previous_sibling (iter); + g_assert_cmpstr (clutter_actor_get_name (iter), ==, "baz"); + + clutter_actor_destroy (actor); + g_object_unref (actor); +} diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c index 30d23e88c..9ce1e54ed 100644 --- a/tests/conform/test-conform-main.c +++ b/tests/conform/test-conform-main.c @@ -133,6 +133,7 @@ main (int argc, char **argv) TEST_CONFORM_SIMPLE ("/actor", actor_remove_child); TEST_CONFORM_SIMPLE ("/actor", actor_raise_child); TEST_CONFORM_SIMPLE ("/actor", actor_lower_child); + TEST_CONFORM_SIMPLE ("/actor", actor_replace_child); TEST_CONFORM_SIMPLE ("/actor", actor_destruction); TEST_CONFORM_SIMPLE ("/actor", actor_anchors); TEST_CONFORM_SIMPLE ("/actor", actor_picking);