From 5a565b42586abb0c851888b827ffabc0cce38b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Sat, 13 Mar 2021 20:43:13 +0100 Subject: [PATCH] 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 0320649a1c50993f308344e0dd296abaad0ee6d4), 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: --- clutter/clutter/clutter-actor-private.h | 4 +- clutter/clutter/clutter-actor.c | 85 ++++++++----------------- clutter/clutter/clutter-stage-private.h | 2 +- clutter/clutter/clutter-stage-view.c | 3 +- clutter/clutter/clutter-stage.c | 7 +- 5 files changed, 34 insertions(+), 67 deletions(-) diff --git a/clutter/clutter/clutter-actor-private.h b/clutter/clutter/clutter-actor-private.h index c34d772e8..46505777a 100644 --- a/clutter/clutter/clutter-actor-private.h +++ b/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, - int phase); +void clutter_actor_finish_layout (ClutterActor *self, + int phase); void clutter_actor_queue_immediate_relayout (ClutterActor *self); diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 956718dd8..bcfffc714 100644 --- a/clutter/clutter/clutter-actor.c +++ b/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) - _clutter_actor_update_last_paint_volume (self); - - success = cull_actor (self, paint_context, &result); + success = should_cull_out + ? cull_actor (self, paint_context, &result) + : FALSE; 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, - gboolean use_max_scale) +clutter_actor_finish_layout (ClutterActor *self, + 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) - return; + _clutter_actor_update_last_paint_volume (self); - update_stage_views (self); - update_resource_scale (self, use_max_scale); + if (priv->needs_update_stage_views) + { + 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); } /** diff --git a/clutter/clutter/clutter-stage-private.h b/clutter/clutter/clutter-stage-private.h index 8f7ff3135..05412ace8 100644 --- a/clutter/clutter/clutter-stage-private.h +++ b/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, diff --git a/clutter/clutter/clutter-stage-view.c b/clutter/clutter/clutter-stage-view.c index 9b325e911..b44bf8885 100644 --- a/clutter/clutter/clutter-stage-view.c +++ b/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; diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index 5b574b5e1..95e42e858 100644 --- a/clutter/clutter/clutter-stage.c +++ b/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);