actor: defer queue-redraw signaling

Instead of immediately, recursively emitting the "queue-redraw" signal
when clutter_actor_queue_redraw is called we now defer this process
until all stage updates are complete. This allows us to aggregate
repeated _queue_redraw requests for the same actor avoiding redundant
paint volume transformations. By deferring we also increase the
likelihood that the actor will have a valid paint volume since it will
have an up to date allocation; this in turn means we will more often be
able to automatically queue clipped redraws which can have a big impact
on performance.

Here's an outline of the actor queue redraw mechanism:

The process starts in clutter_actor_queue_redraw or
_clutter_actor_queue_redraw_with_clip.

These functions queue 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 (see
_clutter_stage_do_update) 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.

Note: actors are allowed to queue a redraw in reseponse to a
queue-redraw signal so we repeat the processing of the list until it
remains empty. An example of when this happens is for Clone actors or
clutter_texture_new_from_actor actors which need to queue a redraw if
their source queues a redraw.

clutter/clutter-actor.c

@@ -472,6 +472,8 @@ struct _ClutterActorPrivate
   ClutterActorBox last_paint_box;
 
   gboolean paint_volume_disabled;
+
+  ClutterStageQueueRedrawEntry *queue_redraw_entry;
 };
 
 enum
@@ -1773,8 +1775,8 @@ clutter_actor_real_allocate (ClutterActor           *self,
 }
 
 static void
-clutter_actor_queue_redraw_with_origin (ClutterActor *self,
+_clutter_actor_signal_queue_redraw (ClutterActor *self,
-                                        ClutterActor *origin)
+                                    ClutterActor *origin)
 {
   /* no point in queuing a redraw on a destroyed actor */
   if (CLUTTER_ACTOR_IN_DESTRUCTION (self))
@@ -1842,7 +1844,7 @@ clutter_actor_real_queue_redraw (ClutterActor *self,
   if (parent != NULL)
     {
       /* this will go up recursively */
-      clutter_actor_queue_redraw_with_origin (parent, origin);
+      _clutter_actor_signal_queue_redraw (parent, origin);
     }
 }
 
@@ -4903,6 +4905,80 @@ clutter_actor_destroy (ClutterActor *self)
   g_object_unref (self);
 }
 
+void
+_clutter_actor_finish_queue_redraw (ClutterActor *self,
+                                    ClutterPaintVolume *clip)
+{
+  ClutterActorPrivate *priv = self->priv;
+  const ClutterPaintVolume *pv;
+  gboolean clipped;
+
+  /* The idea is that if we know the paint box for where the actor was
+   * last drawn and we also have the paint volume for where it will be
+   * drawn next then if we queue a redraw for both these regions that
+   * will cover everything that needs to be redrawn to clear the old
+   * view and show the latest view of the actor.
+   *
+   * Don't clip this redraw if we don't know what position we had for
+   * the previous redraw since we don't know where to set the clip so
+   * it will clear the actor as it is currently.
+   */
+  if (G_LIKELY (priv->last_paint_box_valid))
+    {
+      pv = clutter_actor_get_paint_volume (self);
+      if (pv)
+        {
+          ClutterActor *stage = _clutter_actor_get_stage_internal (self);
+          ClutterPaintVolume stage_pv;
+          ClutterActorBox *box = &priv->last_paint_box;
+          ClutterVertex origin;
+
+          _clutter_paint_volume_init_static (stage, &stage_pv);
+
+          origin.x = box->x1;
+          origin.y = box->y1;
+          origin.z = 0;
+          clutter_paint_volume_set_origin (&stage_pv, &origin);
+          clutter_paint_volume_set_width (&stage_pv, box->x2 - box->x1);
+          clutter_paint_volume_set_height (&stage_pv, box->y2 - box->y1);
+
+          /* make sure we redraw the actors old position... */
+          _clutter_actor_set_queue_redraw_clip (stage, &stage_pv);
+          _clutter_actor_signal_queue_redraw (stage, stage);
+          _clutter_actor_set_queue_redraw_clip (stage, NULL);
+
+          clutter_paint_volume_free (&stage_pv);
+
+          /* XXX: Ideally the redraw signal would take a clip volume
+           * argument, but that would be an ABI break. Until we can
+           * break the ABI we pass the argument out-of-band via an
+           * actor->priv member...
+           */
+
+          /* setup the clip for the actors new position... */
+          _clutter_actor_set_queue_redraw_clip (self, pv);
+          clipped = TRUE;
+        }
+      else
+        clipped = FALSE;
+    }
+  else
+    clipped = FALSE;
+
+  _clutter_actor_signal_queue_redraw (self, self);
+
+  /* Just in case anyone is manually firing redraw signals without
+   * using the public queue_redraw() API we are careful to ensure that
+   * our out-of-band clip member is cleared before returning...
+   *
+   * Note: A NULL clip denotes a full-stage, un-clipped redraw
+   */
+  if (G_LIKELY (clipped))
+    _clutter_actor_set_queue_redraw_clip (self, NULL);
+
+  priv->queue_redraw_entry = NULL;
+}
+
 /**
  * clutter_actor_queue_redraw:
  * @self: A #ClutterActor
@@ -4927,69 +5003,83 @@ clutter_actor_destroy (ClutterActor *self)
 void
 clutter_actor_queue_redraw (ClutterActor *self)
 {
-  ClutterActorPrivate *priv;
-  const ClutterPaintVolume *pv;
-  gboolean clipped;
   ClutterActor *stage;
 
-  g_return_if_fail (CLUTTER_IS_ACTOR (self));
+  /* Here's an outline of the actor queue redraw mechanism:
+   *
+   * The process starts either here or in
+   * _clutter_actor_queue_redraw_with_clip.
+   *
+   * These functions queue 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:
+   * clutter_actor_queue_redraw and
+   * _clutter_actor_queue_redraw_with_clip
+   *
+   * 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_finish_queue_redraws
+   *
+   * _clutter_stage_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.
+   */
 
-  priv = self->priv;
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
 
   /* Ignore queuing a redraw for actors not descended from a stage */
   stage = _clutter_actor_get_stage_internal (self);
   if (!stage)
     return;
 
-  /* Don't clip this redraw if we don't know what position we had for
+  self->priv->queue_redraw_entry =
-   * the previous redraw since we don't know where to set the clip so
+    _clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage),
-   * it will clear the actor as it is currently. */
+                                       self->priv->queue_redraw_entry,
-  if (G_LIKELY (priv->last_paint_box_valid))
+                                       self,
-    {
+                                       NULL);
-      pv = clutter_actor_get_paint_volume (self);
-      if (pv)
-        {
-          ClutterPaintVolume stage_pv;
-          _clutter_paint_volume_init_static (stage, &stage_pv);
-          ClutterActorBox *box = &priv->last_paint_box;
-          ClutterVertex origin;
-
-          origin.x = box->x1;
-          origin.y = box->y1;
-          origin.z = 0;
-          clutter_paint_volume_set_origin (&stage_pv, &origin);
-          clutter_paint_volume_set_width (&stage_pv, box->x2 - box->x1);
-          clutter_paint_volume_set_height (&stage_pv, box->y2 - box->y1);
-
-          /* The idea is that if we know the paint volume for where
-           * the actor was last drawn and we also have the paint
-           * volume for where it will be drawn next then if we queue
-           * a redraw for both these regions that will cover
-           * everything that needs to be redrawn to clear the old
-           * view and show the latest view of the actor.
-           */
-          _clutter_actor_queue_redraw_with_clip (stage, 0, &stage_pv);
-
-          clutter_paint_volume_free (&stage_pv);
-          _clutter_actor_set_queue_redraw_clip (self, pv);
-          clipped = TRUE;
-        }
-      else
-        clipped = FALSE;
-    }
-  else
-    clipped = FALSE;
-
-  clutter_actor_queue_redraw_with_origin (self, self);
-
-  /* Just in case anyone is manually firing redraw signals without
-   * using the public queue_redraw() API we are careful to ensure that
-   * our out-of-band clip member is cleared before returning...
-   *
-   * Note: A NULL clip denotes a full-stage, un-clipped redraw
-   */
-  if (G_LIKELY (clipped))
-    _clutter_actor_set_queue_redraw_clip (self, NULL);
 }
 
 static void
@@ -5063,6 +5153,9 @@ _clutter_actor_queue_redraw_with_clip (ClutterActor       *self,
   ClutterPaintVolume allocation_pv;
   ClutterPaintVolume *pv;
   gboolean should_free_pv;
+  ClutterActor *stage;
+
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
 
   if (flags & CLUTTER_REDRAW_CLIPPED_TO_ALLOCATION)
     {
@@ -5076,7 +5169,7 @@ _clutter_actor_queue_redraw_with_clip (ClutterActor       *self,
           /* NB: NULL denotes an undefined clip which will result in a
            * full redraw... */
           _clutter_actor_set_queue_redraw_clip (self, NULL);
-          clutter_actor_queue_redraw_with_origin (self, self);
+          _clutter_actor_signal_queue_redraw (self, self);
           return;
         }
 
@@ -5102,25 +5195,15 @@ _clutter_actor_queue_redraw_with_clip (ClutterActor       *self,
       should_free_pv = FALSE;
     }
 
-  /* XXX: Ideally the redraw signal would take a clip volume
+  /* Ignore queuing a redraw for actors not descended from a stage */
-   * argument, but that would be an ABI break. Until we can break the
+  stage = _clutter_actor_get_stage_internal (self);
-   * ABI we pass the argument out-of-band via an actor->priv member...
+  if (!stage)
-   */
+    return;
 
-  _clutter_actor_set_queue_redraw_clip (self, pv);
+  _clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage),
-
+                                     self->priv->queue_redraw_entry,
-  clutter_actor_queue_redraw_with_origin (self, self);
+                                     self,
-
+                                     pv);
-  /* Just in case anyone is manually firing redraw signals without
-   * using the public queue_redraw() API we are careful to ensure that
-   * our out-of-band clip member is cleared before returning...
-   *
-   * Note: A NULL clip denotes a full-stage, un-clipped redraw
-   */
-  _clutter_actor_set_queue_redraw_clip (self, NULL);
-
-  if (should_free_pv)
-    clutter_paint_volume_free (pv);
 }
 
 static void
@@ -7587,6 +7670,12 @@ clutter_actor_unparent (ClutterActor *self)
   if (priv->parent_actor == NULL)
     return;
 
+  /* We take this opportunity to invalidate any queue redraw entry
+   * associated with the actor since we won't be able to determine the
+   * appropriate stage after this. */
+  if (priv->queue_redraw_entry)
+    _clutter_stage_queue_redraw_entry_invalidate (priv->queue_redraw_entry);
+
   was_mapped = CLUTTER_ACTOR_IS_MAPPED (self);
 
   /* we need to unrealize *before* we set parent_actor to NULL,

clutter/clutter-private.h

@@ -275,6 +275,8 @@ struct _ClutterPaintVolume
    */
 };
 
+typedef struct _ClutterStageQueueRedrawEntry ClutterStageQueueRedrawEntry;
+
 #define CLUTTER_CONTEXT()	(_clutter_context_get_default ())
 ClutterMainContext *_clutter_context_get_default (void);
 gboolean            _clutter_context_is_initialized (void);
@@ -380,6 +382,13 @@ void                _clutter_stage_paint_volume_stack_free_all (ClutterStage *st
 
 const ClutterGeometry *_clutter_stage_get_clip (ClutterStage *stage);
 
+ClutterStageQueueRedrawEntry *_clutter_stage_queue_actor_redraw           (ClutterStage *stage,
+                                                                           ClutterStageQueueRedrawEntry *entry,
+                                                                           ClutterActor *actor,
+                                                                           ClutterPaintVolume *clip);
+
+void                          _clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry *entry);
+
 
 /* vfuncs implemented by backend */
 GType         _clutter_backend_impl_get_type  (void);
@@ -467,12 +476,14 @@ void _clutter_actor_set_enable_paint_unmapped (ClutterActor *self,
 void _clutter_actor_set_has_pointer (ClutterActor *self,
                                      gboolean      has_pointer);
 
-void _clutter_actor_queue_redraw_with_clip (ClutterActor          *self,
+void _clutter_actor_queue_redraw_with_clip   (ClutterActor              *self,
-                                            ClutterRedrawFlags     flags,
+                                              ClutterRedrawFlags         flags,
-                                            ClutterPaintVolume    *clip_volume);
+                                              ClutterPaintVolume        *clip_volume);
 const ClutterPaintVolume *_clutter_actor_get_queue_redraw_clip (ClutterActor *self);
-void _clutter_actor_set_queue_redraw_clip (ClutterActor *self,
+void _clutter_actor_set_queue_redraw_clip     (ClutterActor             *self,
-                                           const ClutterPaintVolume *clip_volume);
+                                               const ClutterPaintVolume *clip_volume);
+void _clutter_actor_finish_queue_redraw       (ClutterActor             *self,
+                                               ClutterPaintVolume       *clip);
 
 void _clutter_run_repaint_functions (void);
 

clutter/clutter-stage.c

@@ -98,6 +98,13 @@ typedef enum { /*< prefix=CLUTTER_STAGE >*/
 
 #define STAGE_NO_CLEAR_ON_PAINT(s)      ((((ClutterStage *) (s))->priv->stage_hints & CLUTTER_STAGE_NO_CLEAR_ON_PAINT) != 0)
 
+struct _ClutterStageQueueRedrawEntry
+{
+  ClutterActor *actor;
+  gboolean has_clip;
+  ClutterPaintVolume clip;
+};
+
 struct _ClutterStagePrivate
 {
   /* the stage implementation */
@@ -122,6 +129,8 @@ struct _ClutterStagePrivate
 
   const ClutterGeometry *current_paint_clip;
 
+  GList              *pending_queue_redraws;
+
   guint relayout_pending       : 1;
   guint redraw_pending         : 1;
   guint is_fullscreen          : 1;
@@ -169,6 +178,11 @@ static guint stage_signals[LAST_SIGNAL] = { 0, };
 
 static const ClutterColor default_stage_color = { 255, 255, 255, 255 };
 
+static void _clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage);
+
+static void
+_clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage);
+
 static void
 clutter_stage_get_preferred_width (ClutterActor *self,
                                    gfloat        for_height,
@@ -782,6 +796,8 @@ _clutter_stage_do_update (ClutterStage *stage)
   if (!priv->redraw_pending)
     return FALSE;
 
+  _clutter_stage_maybe_finish_queue_redraws (stage);
+
   CLUTTER_NOTE (PAINT, "redrawing via idle for stage[%p]", stage);
   _clutter_do_redraw (stage);
 
@@ -826,31 +842,6 @@ clutter_stage_real_queue_redraw (ClutterActor *actor,
   CoglMatrix modelview;
   ClutterActorBox bounding_box;
 
-  CLUTTER_NOTE (PAINT, "Redraw request number %lu",
-                CLUTTER_CONTEXT ()->redraw_count + 1);
-
-  if (!priv->redraw_pending)
-    {
-      ClutterMasterClock *master_clock;
-
-      priv->redraw_pending = TRUE;
-
-      master_clock = _clutter_master_clock_get_default ();
-      _clutter_master_clock_start_running (master_clock);
-    }
-  else
-    CLUTTER_CONTEXT ()->redraw_count += 1;
-
-  /* We have an optimization in _clutter_do_pick to detect when the
-   * scene is static so we can cache a full, un-clipped pick buffer to
-   * avoid continuous pick renders.
-   *
-   * Currently the assumption is that actors queue a redraw when some
-   * state changes that affects painting *or* picking so we can use
-   * this point to invalidate any currently cached pick buffer.
-   */
-  _clutter_stage_set_pick_buffer_valid (stage, FALSE);
-
   /* If the backend can't do anything with redraw clips (e.g. it already knows
    * it needs to redraw everything anyway) then don't spend time transforming
    * any clip volume into stage coordinates... */
@@ -3140,3 +3131,135 @@ _clutter_stage_get_clip (ClutterStage *stage)
   return stage->priv->current_paint_clip;
 }
 
+/* When an actor queues a redraw we add it to a list on the stage that
+ * gets processed once all updates to the stage have been finished.
+ *
+ * This deferred approach to processing queue_redraw requests means
+ * that we can avoid redundant transformations of clip volumes if
+ * something later triggers a full stage redraw anyway. It also means
+ * we can be more sure that all the referenced actors will have valid
+ * allocations improving the chance that we can determine the actors
+ * paint volume so we can clip the redraw request even if the user
+ * didn't explicitly do so.
+ */
+ClutterStageQueueRedrawEntry *
+_clutter_stage_queue_actor_redraw (ClutterStage *stage,
+                                   ClutterStageQueueRedrawEntry *entry,
+                                   ClutterActor *actor,
+                                   ClutterPaintVolume *clip)
+{
+  ClutterStagePrivate *priv = stage->priv;
+
+  CLUTTER_NOTE (PAINT, "Redraw request number %lu",
+                CLUTTER_CONTEXT ()->redraw_count + 1);
+
+  if (!priv->redraw_pending)
+    {
+      ClutterMasterClock *master_clock;
+
+      priv->redraw_pending = TRUE;
+
+      master_clock = _clutter_master_clock_get_default ();
+      _clutter_master_clock_start_running (master_clock);
+    }
+  else
+    CLUTTER_CONTEXT ()->redraw_count += 1;
+
+  /* We have an optimization in _clutter_do_pick to detect when the
+   * scene is static so we can cache a full, un-clipped pick buffer to
+   * avoid continuous pick renders.
+   *
+   * Currently the assumption is that actors queue a redraw when some
+   * state changes that affects painting *or* picking so we can use
+   * this point to invalidate any currently cached pick buffer.
+   */
+  _clutter_stage_set_pick_buffer_valid (stage, FALSE);
+
+  if (entry)
+    {
+      /* Ignore all requests to queue a redraw for an actor if a full
+       * (non-clipped) redraw of the actor has already been queued. */
+      if (!entry->has_clip)
+        return entry;
+
+      /* If queuing a clipped redraw and a clipped redraw has
+       * previously been queued for this actor then combine the latest
+       * clip together with the existing clip */
+      if (clip)
+        clutter_paint_volume_union (&entry->clip, clip);
+      return entry;
+    }
+  else
+    {
+      entry = g_slice_new (ClutterStageQueueRedrawEntry);
+      entry->actor = g_object_ref (actor);
+
+      if (clip)
+        {
+          entry->has_clip = TRUE;
+          _clutter_paint_volume_init_static (actor, &entry->clip);
+          _clutter_paint_volume_set_from_volume (&entry->clip, clip);
+        }
+
+      stage->priv->pending_queue_redraws =
+        g_list_prepend (stage->priv->pending_queue_redraws, entry);
+
+      return entry;
+    }
+}
+
+static void
+free_queue_redraw_entry (ClutterStageQueueRedrawEntry *entry)
+{
+  g_object_unref (entry->actor);
+  if (entry->has_clip)
+    clutter_paint_volume_free (&entry->clip);
+  g_slice_free (ClutterStageQueueRedrawEntry, entry);
+}
+
+void
+_clutter_stage_queue_redraw_entry_invalidate (
+                                          ClutterStageQueueRedrawEntry *entry)
+{
+  g_object_unref (entry->actor);
+  entry->actor = NULL;
+  if (entry->has_clip)
+    clutter_paint_volume_free (&entry->clip);
+}
+
+static void
+_clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
+{
+  /* 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)
+    {
+      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_UNLIKELY (entry->actor == NULL))
+            continue;
+
+          ClutterPaintVolume *clip = entry->has_clip ? &entry->clip : NULL;
+
+          _clutter_actor_finish_queue_redraw (entry->actor, clip);
+          free_queue_redraw_entry (entry);
+        }
+      g_list_free (stolen_list);
+    }
+}
+