clutter/actor: Update all last_paint_volumes before painting

Updating the last_paint_volume while painting has proven itself to be
quite prone to issues: First we had to make sure actors painted by
offscreen effects get their last_paint_volumes updated correctly (see
0320649a1c), and now a new issue turned up
where we don't update the paint volumes while a fullscreen unredirect is
happening.

To stop those issues from happening and to lay the foundation for using
the last_paint_volume for other things, update the last_paint_volume in
a separate step before painting instead of doing it in
clutter_actor_paint().

To save some resources, avoid introducing another traversal of the
scenegraph and add that step into the existing step of updating the
stage_views lists of actors. To properly update the paint volumes, we
need to do that after finishing the queued redraws, which is why we move
clutter_stage_maybe_finish_queue_redraws() to happen before the new
clutter_stage_finish_layout().

Fixes https://gitlab.gnome.org/GNOME/mutter/-/issues/1699

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1773>

clutter/clutter/clutter-actor-private.h

@@ -260,8 +260,8 @@ float                           clutter_actor_get_real_resource_scale
 ClutterPaintNode *              clutter_actor_create_texture_paint_node                 (ClutterActor *self,
                                                                                          CoglTexture  *texture);
 
-void clutter_actor_update_stage_views (ClutterActor *self,
+void clutter_actor_finish_layout (ClutterActor *self,
-                                       int           phase);
+                                  int           phase);
 
 void clutter_actor_queue_immediate_relayout (ClutterActor *self);
 

clutter/clutter/clutter-actor.c

@@ -844,7 +844,6 @@ struct _ClutterActorPrivate
   guint needs_paint_volume_update   : 1;
   guint had_effects_on_last_paint_volume_update : 1;
   guint needs_update_stage_views    : 1;
-  guint children_painted            : 1;
 };
 
 enum
@@ -1495,7 +1494,7 @@ queue_update_stage_views (ClutterActor *actor)
       /* We don't really need to update the stage-views of the actors up the
        * hierarchy, we set the flag anyway though so we can avoid traversing
        * the whole scenegraph when looking for actors which need an update
-       * in clutter_actor_update_stage_views().
+       * in clutter_actor_finish_layout().
        */
       actor = actor->priv->parent;
     }
@@ -3589,30 +3588,6 @@ clutter_actor_paint_node (ClutterActor        *actor,
   return TRUE;
 }
 
-static void
-ensure_last_paint_volumes_updated (ClutterActor *self)
-{
-  ClutterActorPrivate *priv = self->priv;
-  ClutterActor *child;
-
-  /* Same entry checks as in clutter_actor_paint() */
-  if (CLUTTER_ACTOR_IN_DESTRUCTION (self))
-    return;
-
-  if (!CLUTTER_ACTOR_IS_TOPLEVEL (self) &&
-      ((priv->opacity_override >= 0) ?
-       priv->opacity_override : priv->opacity) == 0)
-    return;
-
-  if (!CLUTTER_ACTOR_IS_MAPPED (self))
-    return;
-
-  _clutter_actor_update_last_paint_volume (self);
-
-  for (child = priv->first_child; child; child = child->priv->next_sibling)
-    ensure_last_paint_volumes_updated (child);
-}
-
 /**
  * clutter_actor_paint:
  * @self: A #ClutterActor
@@ -3639,11 +3614,6 @@ clutter_actor_paint (ClutterActor        *self,
   g_autoptr (ClutterPaintNode) root_node = NULL;
   ClutterActorPrivate *priv;
   ClutterActorBox clip;
-  gboolean should_cull_out = (clutter_paint_debug_flags &
-                              (CLUTTER_DEBUG_DISABLE_CULLING |
-                               CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)) !=
-                             (CLUTTER_DEBUG_DISABLE_CULLING |
-                              CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS);
   gboolean culling_inhibited;
   gboolean clip_set = FALSE;
 
@@ -3795,14 +3765,18 @@ clutter_actor_paint (ClutterActor        *self,
   if (!culling_inhibited && !in_clone_paint ())
     {
       gboolean success;
+      gboolean should_cull_out = (clutter_paint_debug_flags &
+                                  (CLUTTER_DEBUG_DISABLE_CULLING |
+                                   CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)) !=
+                                 (CLUTTER_DEBUG_DISABLE_CULLING |
+                                  CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS);
       /* annoyingly gcc warns if uninitialized even though
        * the initialization is redundant :-( */
       ClutterCullResult result = CLUTTER_CULL_RESULT_IN;
 
-      if (should_cull_out)
+      success = should_cull_out
-        _clutter_actor_update_last_paint_volume (self);
+        ? cull_actor (self, paint_context, &result)
-
+        : FALSE;
-      success = cull_actor (self, paint_context, &result);
 
       if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_REDRAWS))
         _clutter_actor_paint_cull_result (self, success, result, actor_node);
@@ -3819,22 +3793,8 @@ clutter_actor_paint (ClutterActor        *self,
   if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_PAINT_VOLUMES))
     _clutter_actor_draw_paint_volume (self, actor_node);
 
-  priv->children_painted = FALSE;
-
   clutter_paint_node_paint (root_node, paint_context);
 
-  /* If an effect choose to not call clutter_actor_continue_paint()
-   * (for example offscreen effects might just paint their cached
-   * texture instead), the last_paint_volumes of the whole subtree
-   * still need to be updated to adjust for any changes to their
-   * eye-coordinates transformation matrices.
-   */
-  if (!priv->children_painted)
-    {
-      if (!culling_inhibited && !in_clone_paint () && should_cull_out)
-        ensure_last_paint_volumes_updated (self);
-    }
-
   /* If we make it here then the actor has run through a complete
      paint run including all the effects so it's no longer dirty */
   priv->is_dirty = FALSE;
@@ -3877,8 +3837,6 @@ clutter_actor_continue_paint (ClutterActor        *self,
       CoglFramebuffer *framebuffer;
       ClutterPaintNode *dummy;
 
-      priv->children_painted = TRUE;
-
       /* XXX - this will go away in 2.0, when we can get rid of this
        * stuff and switch to a pure retained render tree of PaintNodes
        * for the entire frame, starting from the Stage; the paint()
@@ -3954,6 +3912,11 @@ clutter_actor_pick (ClutterActor       *actor,
   ClutterActorBox clip;
   gboolean transform_pushed = FALSE;
   gboolean clip_set = FALSE;
+  gboolean should_cull = (clutter_paint_debug_flags &
+                          (CLUTTER_DEBUG_DISABLE_CULLING |
+                           CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)) !=
+                         (CLUTTER_DEBUG_DISABLE_CULLING |
+                          CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS);
 
   if (CLUTTER_ACTOR_IN_DESTRUCTION (actor))
     return;
@@ -3969,7 +3932,7 @@ clutter_actor_pick (ClutterActor       *actor,
   /* mark that we are in the paint process */
   CLUTTER_SET_PRIVATE_FLAGS (actor, CLUTTER_IN_PICK);
 
-  if (priv->paint_volume_valid && priv->last_paint_volume_valid)
+  if (should_cull && priv->paint_volume_valid && priv->last_paint_volume_valid)
     {
       graphene_box_t box;
 
@@ -15871,8 +15834,8 @@ update_resource_scale (ClutterActor *self,
 }
 
 void
-clutter_actor_update_stage_views (ClutterActor *self,
+clutter_actor_finish_layout (ClutterActor *self,
-                                  gboolean      use_max_scale)
+                             gboolean      use_max_scale)
 {
   ClutterActorPrivate *priv = self->priv;
   ClutterActor *child;
@@ -15881,16 +15844,18 @@ clutter_actor_update_stage_views (ClutterActor *self,
       CLUTTER_ACTOR_IN_DESTRUCTION (self))
     return;
 
-  if (!priv->needs_update_stage_views)
+  _clutter_actor_update_last_paint_volume (self);
-    return;
 
-  update_stage_views (self);
+  if (priv->needs_update_stage_views)
-  update_resource_scale (self, use_max_scale);
+    {
+      update_stage_views (self);
+      update_resource_scale (self, use_max_scale);
 
-  priv->needs_update_stage_views = FALSE;
+      priv->needs_update_stage_views = FALSE;
+    }
 
   for (child = priv->first_child; child; child = child->priv->next_sibling)
-    clutter_actor_update_stage_views (child, use_max_scale);
+    clutter_actor_finish_layout (child, use_max_scale);
 }
 
 /**

clutter/clutter/clutter-stage-private.h

@@ -68,7 +68,7 @@ void                clutter_stage_maybe_finish_queue_redraws (ClutterStage
 GSList *            clutter_stage_find_updated_devices   (ClutterStage          *stage);
 void                clutter_stage_update_devices         (ClutterStage          *stage,
                                                           GSList                *devices);
-void                clutter_stage_update_actor_stage_views (ClutterStage        *stage);
+void                clutter_stage_finish_layout          (ClutterStage          *stage);
 
 CLUTTER_EXPORT
 void     _clutter_stage_queue_event                       (ClutterStage *stage,

clutter/clutter/clutter-stage-view.c

@@ -1171,9 +1171,10 @@ handle_frame_clock_frame (ClutterFrameClock *frame_clock,
   clutter_stage_emit_before_update (stage, view);
 
   clutter_stage_maybe_relayout (CLUTTER_ACTOR (stage));
-  clutter_stage_update_actor_stage_views (stage);
   clutter_stage_maybe_finish_queue_redraws (stage);
 
+  clutter_stage_finish_layout (stage);
+
   devices = clutter_stage_find_updated_devices (stage);
 
   frame = CLUTTER_FRAME_INIT;

clutter/clutter/clutter-stage.c

@@ -930,7 +930,7 @@ clutter_stage_find_updated_devices (ClutterStage *stage)
 }
 
 void
-clutter_stage_update_actor_stage_views (ClutterStage *stage)
+clutter_stage_finish_layout (ClutterStage *stage)
 {
   ClutterActor *actor = CLUTTER_ACTOR (stage);
   ClutterStagePrivate *priv = stage->priv;
@@ -944,20 +944,21 @@ clutter_stage_update_actor_stage_views (ClutterStage *stage)
    * the paint.
    *
    * We're doing the whole thing twice and pass the phase to
-   * clutter_actor_update_stage_views() to allow actors to detect loops:
+   * clutter_actor_finish_layout() to allow actors to detect loops:
    * If the resource scale changes again after the relayout, the new
    * allocation of an actor probably moved the actor onto another stage
    * view, so if an actor sees phase == 1, it can choose a "final" scale.
    */
   for (phase = 0; phase < 2; phase++)
     {
-      clutter_actor_update_stage_views (actor, phase);
+      clutter_actor_finish_layout (actor, phase);
 
       if (!priv->actor_needs_immediate_relayout)
         break;
 
       priv->actor_needs_immediate_relayout = FALSE;
       clutter_stage_maybe_relayout (actor);
+      clutter_stage_maybe_finish_queue_redraws (stage);
     }
 
   g_warn_if_fail (!priv->actor_needs_immediate_relayout);