From dfe4d218f1343b46e90140020d861bf6b4864c4f Mon Sep 17 00:00:00 2001 From: Sebastian Keller Date: Tue, 5 Dec 2023 21:50:39 +0100 Subject: [PATCH] clutter/actor: Make get_transformed_paint_volume() transfer full Transfer none was achieved using a stack GArray in the stage which would get resized to 0 at the end of every frame to "free" it. In the case of direct scanout however, painting the next frame only happens after leaving fullscreen again. Until then the array just kept growing and because GArrays don't reallocate when shrunk, this memory remained allocated even after leaving fullscreen. There is no cache benefit from storing paint volumes this way, because nothing accesses them after their immediate use in the calling code. Also the reduced overhead from avoiding malloc calls seems negligible as according to heaptrack this only makes up about 2-3% of the temporary allocations. Changing this to transfer full and removing the stack array simplifies the code and fixes the "leak". Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/3191 Part-of: --- clutter/clutter/clutter-actor.c | 17 ++++-------- clutter/clutter/clutter-actor.h | 2 +- clutter/clutter/clutter-stage.c | 38 -------------------------- src/compositor/meta-window-actor-x11.c | 2 +- src/compositor/meta-window-group.c | 2 +- 5 files changed, 9 insertions(+), 52 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 271295949..414974dd6 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -1675,7 +1675,7 @@ set_show_on_set_parent (ClutterActor *self, static void clutter_actor_queue_redraw_on_parent (ClutterActor *self) { - const ClutterPaintVolume *pv; + g_autoptr (ClutterPaintVolume) pv = NULL; if (!self->priv->parent) return; @@ -5584,7 +5584,7 @@ clutter_actor_real_get_paint_volume (ClutterActor *self, child != NULL; child = child->priv->next_sibling) { - const ClutterPaintVolume *child_volume; + g_autoptr (ClutterPaintVolume) child_volume = NULL; /* we ignore unmapped children, since they won't be painted. * @@ -14517,12 +14517,10 @@ clutter_actor_get_paint_volume (ClutterActor *self) * transformed paint volume of all of its children and union them * together using clutter_paint_volume_union(). * - * Return value: (transfer none) (nullable): a pointer to a #ClutterPaintVolume, - * or %NULL if no volume could be determined. The returned pointer is - * not guaranteed to be valid across multiple frames; if you wish to - * keep it, you will have to copy it using clutter_paint_volume_copy(). + * Return value: (transfer full) (nullable): a pointer to a #ClutterPaintVolume, + * or %NULL if no volume could be determined. */ -const ClutterPaintVolume * +ClutterPaintVolume * clutter_actor_get_transformed_paint_volume (ClutterActor *self, ClutterActor *relative_to_ancestor) { @@ -14541,10 +14539,7 @@ clutter_actor_get_transformed_paint_volume (ClutterActor *self, if (volume == NULL) return NULL; - transformed_volume = - _clutter_stage_paint_volume_stack_allocate (CLUTTER_STAGE (stage)); - - _clutter_paint_volume_copy_static (volume, transformed_volume); + transformed_volume = clutter_paint_volume_copy (volume); _clutter_paint_volume_transform_relative (transformed_volume, relative_to_ancestor); diff --git a/clutter/clutter/clutter-actor.h b/clutter/clutter/clutter-actor.h index c10a7f326..ec08f6f96 100644 --- a/clutter/clutter/clutter-actor.h +++ b/clutter/clutter/clutter-actor.h @@ -589,7 +589,7 @@ void clutter_actor_get_background_color CLUTTER_EXPORT const ClutterPaintVolume * clutter_actor_get_paint_volume (ClutterActor *self); CLUTTER_EXPORT -const ClutterPaintVolume * clutter_actor_get_transformed_paint_volume (ClutterActor *self, +ClutterPaintVolume * clutter_actor_get_transformed_paint_volume (ClutterActor *self, ClutterActor *relative_to_ancestor); /* Events */ diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index cb1890369..b9099fdc0 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -126,8 +126,6 @@ struct _ClutterStagePrivate GPtrArray *cur_event_actors; GArray *cur_event_emission_chain; - GArray *paint_volume_stack; - GSList *pending_relayouts; int update_freeze_count; @@ -441,8 +439,6 @@ clutter_stage_do_paint_view (ClutterStage *stage, g_array_append_val (clip_frusta, clip_frustum); } - _clutter_stage_paint_volume_stack_free_all (stage); - paint_flags = clutter_stage_view_get_default_paint_flags (view); paint_context = clutter_paint_context_new_for_view (view, @@ -1290,8 +1286,6 @@ clutter_stage_finalize (GObject *object) g_free (priv->title); - g_array_free (priv->paint_volume_stack, TRUE); - G_OBJECT_CLASS (clutter_stage_parent_class)->finalize (object); } @@ -1694,9 +1688,6 @@ clutter_stage_init (ClutterStage *self) clutter_stage_set_title (self, g_get_prgname ()); clutter_stage_set_key_focus (self, NULL); clutter_stage_set_viewport (self, geom.width, geom.height); - - priv->paint_volume_stack = - g_array_new (FALSE, FALSE, sizeof (ClutterPaintVolume)); } static void @@ -2529,35 +2520,6 @@ clutter_stage_schedule_update (ClutterStage *stage) priv->update_scheduled = TRUE; } -ClutterPaintVolume * -_clutter_stage_paint_volume_stack_allocate (ClutterStage *stage) -{ - GArray *paint_volume_stack = stage->priv->paint_volume_stack; - - g_array_set_size (paint_volume_stack, - paint_volume_stack->len+1); - - return &g_array_index (paint_volume_stack, - ClutterPaintVolume, - paint_volume_stack->len - 1); -} - -void -_clutter_stage_paint_volume_stack_free_all (ClutterStage *stage) -{ - GArray *paint_volume_stack = stage->priv->paint_volume_stack; - int i; - - for (i = 0; i < paint_volume_stack->len; i++) - { - ClutterPaintVolume *pv = - &g_array_index (paint_volume_stack, ClutterPaintVolume, i); - clutter_paint_volume_free (pv); - } - - g_array_set_size (paint_volume_stack, 0); -} - void clutter_stage_add_to_redraw_clip (ClutterStage *stage, ClutterPaintVolume *redraw_clip) diff --git a/src/compositor/meta-window-actor-x11.c b/src/compositor/meta-window-actor-x11.c index cf208b995..8574df604 100644 --- a/src/compositor/meta-window-actor-x11.c +++ b/src/compositor/meta-window-actor-x11.c @@ -1351,7 +1351,7 @@ meta_window_actor_x11_get_paint_volume (ClutterActor *actor, if (surface) { ClutterActor *surface_actor = CLUTTER_ACTOR (surface); - const ClutterPaintVolume *child_volume; + g_autoptr (ClutterPaintVolume) child_volume = NULL; child_volume = clutter_actor_get_transformed_paint_volume (surface_actor, actor); diff --git a/src/compositor/meta-window-group.c b/src/compositor/meta-window-group.c index 46be63812..bb59264c5 100644 --- a/src/compositor/meta-window-group.c +++ b/src/compositor/meta-window-group.c @@ -146,7 +146,7 @@ meta_window_group_get_paint_volume (ClutterActor *self, clutter_actor_iter_init (&iter, self); while (clutter_actor_iter_next (&iter, &child)) { - const ClutterPaintVolume *child_volume; + g_autoptr (ClutterPaintVolume) child_volume = NULL; if (!clutter_actor_is_mapped (child)) continue;