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: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/3442>
This commit is contained in:
Sebastian Keller 2023-12-05 21:50:39 +01:00 committed by Marge Bot
parent 05efc92084
commit dfe4d218f1
5 changed files with 9 additions and 52 deletions

View File

@ -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);

View File

@ -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 */

View File

@ -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)

View File

@ -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);

View File

@ -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;