From 3bca29f3038d58f5f42978b2f7d3103f6119d565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Fri, 16 Oct 2020 23:29:02 +0200 Subject: [PATCH] clutter/actor: Only allocate when actor is mapped, not only visible In theory there's no big difference between only handling mapped actors vs only handling visible actors in clutter_actor_allocate(): The function is called recursively starting with an actor that is attached to a stage, so it should only be called on mapped actors anyway. The behavior of skipping hidden actors was introduced as an optimization with commit 0eab73dc. Since the last commit, we handle enable_paint_unmapped a bit better and don't do unnecessary work when mapping or unmapping, so we can now be a bit stricter enforcing our invariants and only allow mapped actors in clutter_actor_allocate(). We need to exclude toplevel actors from this check since the stage has a very different mapped state than normal actors, depending on the mappedness of the x11 window. Also we need to make an exception for clones (of course...): Those need their source actor to have an allocation, which means they might try to force-allocate it, and in that case we shouldn't bail out of clutter_actor_allocate(). Also moving the clutter_actor_queue_relayout() call from clutter_actor_real_show() to clutter_actor_real_map() seems to fix a bug where we don't queue redraws/relayouts on children when a parent gets shown. Fixes https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/2973 https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1366 --- clutter/clutter/clutter-actor.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 8fde0f550..a398986bf 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -1635,6 +1635,13 @@ clutter_actor_real_map (ClutterActor *self) priv->needs_update_stage_views = FALSE; queue_update_stage_views (self); } + + /* Avoid the early return in clutter_actor_queue_relayout() */ + priv->needs_width_request = FALSE; + priv->needs_height_request = FALSE; + priv->needs_allocation = FALSE; + + clutter_actor_queue_relayout (self); } /* notify on parent mapped before potentially mapping @@ -1742,6 +1749,10 @@ clutter_actor_real_unmap (ClutterActor *self) */ _clutter_paint_volume_init_static (&priv->last_paint_volume, NULL); priv->last_paint_volume_valid = TRUE; + + if (priv->parent && !CLUTTER_ACTOR_IN_DESTRUCTION (priv->parent) && + (!(priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT))) + clutter_actor_queue_relayout (priv->parent); } /* notify on parent mapped after potentially unmapping @@ -1799,8 +1810,6 @@ clutter_actor_queue_shallow_relayout (ClutterActor *self) static void clutter_actor_real_show (ClutterActor *self) { - ClutterActorPrivate *priv = self->priv; - if (CLUTTER_ACTOR_IS_VISIBLE (self)) return; @@ -1811,13 +1820,6 @@ clutter_actor_real_show (ClutterActor *self) * and the branch of the scene graph is in a stable state */ clutter_actor_update_map_state (self, MAP_STATE_CHECK); - - /* Avoid the early return in clutter_actor_queue_relayout() */ - priv->needs_width_request = FALSE; - priv->needs_height_request = FALSE; - priv->needs_allocation = FALSE; - - clutter_actor_queue_relayout (self); } static inline void @@ -1938,8 +1940,6 @@ clutter_actor_is_visible (ClutterActor *self) static void clutter_actor_real_hide (ClutterActor *self) { - ClutterActorPrivate *priv = self->priv; - if (!CLUTTER_ACTOR_IS_VISIBLE (self)) return; @@ -1950,13 +1950,6 @@ clutter_actor_real_hide (ClutterActor *self) * and the branch of the scene graph is in a stable state */ clutter_actor_update_map_state (self, MAP_STATE_CHECK); - - /* we queue a relayout unless the actor is inside a - * container that explicitly told us not to - */ - if (priv->parent != NULL && - (!(priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT))) - clutter_actor_queue_relayout (priv->parent); } /** @@ -9576,7 +9569,9 @@ clutter_actor_allocate (ClutterActor *self, ? priv->parent->priv->absolute_origin_changed : FALSE; - if (!CLUTTER_ACTOR_IS_VISIBLE (self)) + if (!CLUTTER_ACTOR_IS_TOPLEVEL (self) && + !CLUTTER_ACTOR_IS_MAPPED (self) && + !clutter_actor_has_mapped_clones (self)) { if (priv->absolute_origin_changed) {