[mutter] clutter/actor: Emit the queue-redraw signal right away

[Marge Bot <marge-bot src gnome org>]

commit ce4c297cea62bd7d6aebfdd90be93a6690b72a07
Author: Jonas Dreßler <verdre v0yd nl>
Date:   Mon Oct 19 16:48:53 2020 +0200

    clutter/actor: Emit the queue-redraw signal right away
    
    Since we now decoupled the "queue-redraw" signal from creating the stage
    clip, we can move signal emission into
    _clutter_actor_queue_redraw_full() and emit the signal right away when
    queueing a redraw on an actor. With that we now no longer have to
    accommodate for the stage pending_queue_redraws list changing while
    iterating over it.
    
    To ensure we don't emit the signal too often when multiple redraws are
    queued on one actor, use the propagated_one_redraw flag to limit the
    number of emissions to a single one for every update cycle.
    
    Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1511>

 clutter/clutter/clutter-actor.c | 70 ++++++++-----------------------
 clutter/clutter/clutter-stage.c | 93 +++++++++++++++++------------------------
 2 files changed, 55 insertions(+), 108 deletions(-)
---
diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c
index 114d52a9be..678a3e6deb 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -8023,8 +8023,6 @@ _clutter_actor_finish_queue_redraw (ClutterActor *self)
      later)
   */
   priv->queue_redraw_entry = NULL;
-
-  _clutter_actor_propagate_queue_redraw (self, self);
 }
 
 void
@@ -8041,63 +8039,26 @@ _clutter_actor_queue_redraw_full (ClutterActor             *self,
    * wrapper for this function. Additionally, an effect can queue a
    * redraw by wrapping this function in clutter_effect_queue_repaint().
    *
+   * This function will emit the "queue-redraw" signal for each actor
+   * up the actor-tree, allowing to track redraws queued by children
+   * or to queue a redraw of a different actor (like a clone) in
+   * response to this one.
+   *
    * This functions queues an entry in a list associated with the
    * stage which is a list of actors that queued a redraw while
    * updating the timelines, performing layouting and processing other
    * mainloop sources before the next paint starts.
    *
-   * We aim to minimize the processing done at this point because
-   * there is a good chance other events will happen while updating
-   * the scenegraph that would invalidate any expensive work we might
-   * otherwise try to do here. For example we don't try and resolve
-   * the screen space bounding box of an actor at this stage so as to
-   * minimize how much of the screen redraw because it's possible
-   * something else will happen which will force a full redraw anyway.
-   *
    * When all updates are complete and we come to paint the stage then
-   * we iterate this list and actually emit the "queue-redraw" signals
-   * for each of the listed actors which will bubble up to the stage
-   * for each actor and at that point we will transform the actors
-   * paint volume into screen coordinates to determine the clip region
-   * for what needs to be redrawn in the next paint.
-   *
-   * Besides minimizing redundant work another reason for this
-   * deferred design is that it's more likely we will be able to
-   * determine the paint volume of an actor once we've finished
-   * updating the scenegraph because its allocation should be up to
-   * date. NB: If we can't determine an actors paint volume then we
-   * can't automatically queue a clipped redraw which can make a big
-   * difference to performance.
-   *
-   * So the control flow goes like this:
-   * One of clutter_actor_queue_redraw(),
-   *     or clutter_effect_queue_repaint()
-   *
-   * then control moves to:
-   *   _clutter_stage_queue_actor_redraw()
-   *
-   * later during _clutter_stage_do_update(), once relayouting is done
-   * and the scenegraph has been updated we will call:
-   * clutter_stage_maybe_finish_queue_redraws().
-   *
-   * clutter_stage_maybe_finish_queue_redraws() will call
-   * _clutter_actor_finish_queue_redraw() for each listed actor.
-   *
-   * Note: actors *are* allowed to queue further redraws during this
-   * process (considering clone actors or texture_new_from_actor which
-   * respond to their source queueing a redraw by queuing a redraw
-   * themselves). We repeat the process until the list is empty.
-   *
-   * This will result in the "queue-redraw" signal being fired for
-   * each actor which will pass control to the default signal handler:
-   * clutter_actor_real_queue_redraw()
-   *
-   * This will bubble up to the stages handler:
-   * clutter_stage_real_queue_redraw()
-   *
-   * clutter_stage_real_queue_redraw() will transform the actors paint
-   * volume into screen space and add it as a clip region for the next
-   * paint.
+   * we iterate this list and build the redraw clip of the stage by
+   * either using the clip that was supplied to
+   * _clutter_actor_queue_redraw_full() or by asking the actor for its
+   * redraw clip using clutter_actor_get_redraw_clip().
+   *
+   * Doing this later during the stage update instead of now is an
+   * important optimization, because later it's more likely we will be
+   * able to determine the paint volume of an actor (its allocation
+   * should be up to date).
    */
 
   /* ignore queueing a redraw for actors being destroyed */
@@ -8179,6 +8140,9 @@ _clutter_actor_queue_redraw_full (ClutterActor             *self,
     }
 
   priv->is_dirty = TRUE;
+
+  if (!priv->propagated_one_redraw)
+    _clutter_actor_propagate_queue_redraw (self, self);
 }
 
 /**
diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c
index aa64c0277a..6afc40ea5c 100644
--- a/clutter/clutter/clutter-stage.c
+++ b/clutter/clutter/clutter-stage.c
@@ -2882,6 +2882,7 @@ void
 clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
 {
   ClutterStagePrivate *priv = stage->priv;
+  GList *l;
 
   COGL_TRACE_BEGIN_SCOPED (ClutterStageFinishQueueRedraws, "FinishQueueRedraws");
 
@@ -2890,68 +2891,50 @@ clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
 
   priv->pending_finish_queue_redraws = FALSE;
 
-  /* Note: we have to repeat until the pending_queue_redraws list is
-   * empty because actors are allowed to queue redraws in response to
-   * the queue-redraw signal. For example Clone actors or
-   * texture_new_from_actor actors will have to queue a redraw if
-   * their source queues a redraw.
-   */
-  while (stage->priv->pending_queue_redraws)
+  for (l = priv->pending_queue_redraws; l; l = l->next)
     {
-      GList *l;
-      /* XXX: we need to allow stage->priv->pending_queue_redraws to
-       * be updated while we process the current entries in the list
-       * so we steal the list pointer and then reset it to an empty
-       * list before processing... */
-      GList *stolen_list = stage->priv->pending_queue_redraws;
-      stage->priv->pending_queue_redraws = NULL;
-
-      for (l = stolen_list; l; l = l->next)
-        {
-          ClutterStageQueueRedrawEntry *entry = l->data;
-
-          /* NB: Entries may be invalidated if the actor gets destroyed */
-          if (G_LIKELY (entry->actor != NULL))
-            {
-              ClutterPaintVolume old_actor_pv, new_actor_pv;
+      ClutterStageQueueRedrawEntry *entry = l->data;
 
-              _clutter_actor_finish_queue_redraw (entry->actor);
+      /* NB: Entries may be invalidated if the actor gets destroyed */
+      if (G_LIKELY (entry->actor != NULL))
+        {
+          ClutterPaintVolume old_actor_pv, new_actor_pv;
 
-              _clutter_paint_volume_init_static (&old_actor_pv, NULL);
-              _clutter_paint_volume_init_static (&new_actor_pv, NULL);
-
-              if (entry->has_clip)
-                {
-                  add_to_stage_clip (stage, &entry->clip);
-                }
-              else if (clutter_actor_get_redraw_clip (entry->actor,
-                                                      &old_actor_pv,
-                                                      &new_actor_pv))
-                {
-                  /* Add both the old paint volume of the actor (which is
-                   * currently visible on the screen) and the new paint volume
-                   * (which will be visible on the screen after this redraw)
-                   * to the redraw clip.
-                   * The former we do to ensure the old texture on the screen
-                   * will be fully painted over in case the actor was moved.
-                   */
-                  add_to_stage_clip (stage, &old_actor_pv);
-                  add_to_stage_clip (stage, &new_actor_pv);
-                }
-              else
-                {
-                  /* If there's no clip we can use, we have to trigger an
-                   * unclipped full stage redraw.
-                   */
-                  add_to_stage_clip (stage, NULL);
-                }
+          _clutter_paint_volume_init_static (&old_actor_pv, NULL);
+          _clutter_paint_volume_init_static (&new_actor_pv, NULL);
 
+          if (entry->has_clip)
+            {
+              add_to_stage_clip (stage, &entry->clip);
+            }
+          else if (clutter_actor_get_redraw_clip (entry->actor,
+                                                  &old_actor_pv,
+                                                  &new_actor_pv))
+            {
+              /* Add both the old paint volume of the actor (which is
+               * currently visible on the screen) and the new paint volume
+               * (which will be visible on the screen after this redraw)
+               * to the redraw clip.
+               * The former we do to ensure the old texture on the screen
+               * will be fully painted over in case the actor was moved.
+               */
+              add_to_stage_clip (stage, &old_actor_pv);
+              add_to_stage_clip (stage, &new_actor_pv);
+            }
+          else
+            {
+              /* If there's no clip we can use, we have to trigger an
+               * unclipped full stage redraw.
+               */
+              add_to_stage_clip (stage, NULL);
             }
-
-          free_queue_redraw_entry (entry);
         }
-      g_list_free (stolen_list);
+
+      free_queue_redraw_entry (entry);
     }
+
+  g_list_free (priv->pending_queue_redraws);
+  priv->pending_queue_redraws = NULL;
 }
 
 /**