isa/mutter
1
0
Fork 0
mirror of https://github.com/brl/mutter.git synced 2024-12-23 19:42:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

clutter/stage: Don't pass QueueRedrawEntries to actors

Browse Source 
We currently pass actors a reference to their associated
ClutterStageQueueRedrawEntry when queueing a redraw. This "splitting" of
the ownership of the entry has introduced quite a few bugs in the past
and is hard to follow.

So give up the "splitting" of the ownership and exclusively handle those
entries inside ClutterStage. To still allow removing the entry when an
actor gets unrealized introduce clutter_stage_dequeue_actor_redraw()
similar to what we already have for relayouts.

To be able to efficiently find entries when actors queue redraws, make
pending_queue_redraws a GHashTable, which fits quite nicely and also
allows removing the QueueRedrawEntries actor pointer in favour of the
key of the hashtable.

Since the struct is now private to ClutterStage, we can also rename it
to QueueRedrawEntry.

While at it, also sneak in the removal of the leading underscore from
clutter_stage_queue_actor_redraw().

Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1511>
This commit is contained in:
Jonas Dreßler 2020-10-17 22:29:05 +02:00 committed by Marge Bot
parent 1cd386551d
commit 906124b09f
3 changed files with 75 additions and 152 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
clutter/clutter/clutter-actor.c
View File

@ -775,8 +775,6 @@ struct _ClutterActorPrivate
*/ */
ClutterPaintVolume last_paint_volume; ClutterPaintVolume last_paint_volume;
ClutterStageQueueRedrawEntry *queue_redraw_entry;
ClutterColor bg_color; ClutterColor bg_color;
#ifdef CLUTTER_ENABLE_DEBUG #ifdef CLUTTER_ENABLE_DEBUG
@ -2117,6 +2115,9 @@ unrealize_actor_after_children_cb (ClutterActor *self,
priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT) priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT)
clutter_stage_dequeue_actor_relayout (CLUTTER_STAGE (stage), self); clutter_stage_dequeue_actor_relayout (CLUTTER_STAGE (stage), self);
if (stage != NULL)
clutter_stage_dequeue_actor_redraw (CLUTTER_STAGE (stage), self);
if (priv->unmapped_paint_branch_counter == 0) if (priv->unmapped_paint_branch_counter == 0)
priv->allocation = (ClutterActorBox) CLUTTER_ACTOR_BOX_UNINITIALIZED; priv->allocation = (ClutterActorBox) CLUTTER_ACTOR_BOX_UNINITIALIZED;
@ -4059,22 +4060,6 @@ _clutter_actor_stop_transitions (ClutterActor *self)
} }
} }
static ClutterActorTraverseVisitFlags
invalidate_queue_redraw_entry (ClutterActor *self,
int depth,
gpointer user_data)
{
ClutterActorPrivate *priv = self->priv;
if (priv->queue_redraw_entry != NULL)
{
_clutter_stage_queue_redraw_entry_invalidate (priv->queue_redraw_entry);
priv->queue_redraw_entry = NULL;
}
return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE;
}
static inline void static inline void
remove_child (ClutterActor *self, remove_child (ClutterActor *self,
ClutterActor *child) ClutterActor *child)
@ -4107,10 +4092,9 @@ typedef enum
REMOVE_CHILD_EMIT_PARENT_SET = 1 << 1, REMOVE_CHILD_EMIT_PARENT_SET = 1 << 1,
REMOVE_CHILD_EMIT_ACTOR_REMOVED = 1 << 2, REMOVE_CHILD_EMIT_ACTOR_REMOVED = 1 << 2,
REMOVE_CHILD_CHECK_STATE = 1 << 3, REMOVE_CHILD_CHECK_STATE = 1 << 3,
REMOVE_CHILD_FLUSH_QUEUE = 1 << 4, REMOVE_CHILD_NOTIFY_FIRST_LAST = 1 << 4,
REMOVE_CHILD_NOTIFY_FIRST_LAST = 1 << 5, REMOVE_CHILD_STOP_TRANSITIONS = 1 << 5,
REMOVE_CHILD_STOP_TRANSITIONS = 1 << 6, REMOVE_CHILD_CLEAR_STAGE_VIEWS = 1 << 6,
REMOVE_CHILD_CLEAR_STAGE_VIEWS = 1 << 7,
/* default flags for public API */ /* default flags for public API */
REMOVE_CHILD_DEFAULT_FLAGS = REMOVE_CHILD_STOP_TRANSITIONS | REMOVE_CHILD_DEFAULT_FLAGS = REMOVE_CHILD_STOP_TRANSITIONS |
@ -4118,7 +4102,6 @@ typedef enum
REMOVE_CHILD_EMIT_PARENT_SET | REMOVE_CHILD_EMIT_PARENT_SET |
REMOVE_CHILD_EMIT_ACTOR_REMOVED | REMOVE_CHILD_EMIT_ACTOR_REMOVED |
REMOVE_CHILD_CHECK_STATE | REMOVE_CHILD_CHECK_STATE |
REMOVE_CHILD_FLUSH_QUEUE |
REMOVE_CHILD_NOTIFY_FIRST_LAST | REMOVE_CHILD_NOTIFY_FIRST_LAST |
REMOVE_CHILD_CLEAR_STAGE_VIEWS, REMOVE_CHILD_CLEAR_STAGE_VIEWS,
} ClutterActorRemoveChildFlags; } ClutterActorRemoveChildFlags;
@ -4138,7 +4121,6 @@ clutter_actor_remove_child_internal (ClutterActor *self,
{ {
ClutterActor *old_first, *old_last; ClutterActor *old_first, *old_last;
gboolean destroy_meta, emit_parent_set, emit_actor_removed, check_state; gboolean destroy_meta, emit_parent_set, emit_actor_removed, check_state;
gboolean flush_queue;
gboolean notify_first_last; gboolean notify_first_last;
gboolean stop_transitions; gboolean stop_transitions;
gboolean clear_stage_views; gboolean clear_stage_views;
@ -4155,7 +4137,6 @@ clutter_actor_remove_child_internal (ClutterActor *self,
emit_parent_set = (flags & REMOVE_CHILD_EMIT_PARENT_SET) != 0; emit_parent_set = (flags & REMOVE_CHILD_EMIT_PARENT_SET) != 0;
emit_actor_removed = (flags & REMOVE_CHILD_EMIT_ACTOR_REMOVED) != 0; emit_actor_removed = (flags & REMOVE_CHILD_EMIT_ACTOR_REMOVED) != 0;
check_state = (flags & REMOVE_CHILD_CHECK_STATE) != 0; check_state = (flags & REMOVE_CHILD_CHECK_STATE) != 0;
flush_queue = (flags & REMOVE_CHILD_FLUSH_QUEUE) != 0;
notify_first_last = (flags & REMOVE_CHILD_NOTIFY_FIRST_LAST) != 0; notify_first_last = (flags & REMOVE_CHILD_NOTIFY_FIRST_LAST) != 0;
stop_transitions = (flags & REMOVE_CHILD_STOP_TRANSITIONS) != 0; stop_transitions = (flags & REMOVE_CHILD_STOP_TRANSITIONS) != 0;
clear_stage_views = (flags & REMOVE_CHILD_CLEAR_STAGE_VIEWS) != 0; clear_stage_views = (flags & REMOVE_CHILD_CLEAR_STAGE_VIEWS) != 0;
@ -4181,28 +4162,6 @@ clutter_actor_remove_child_internal (ClutterActor *self,
clutter_actor_update_map_state (child, MAP_STATE_MAKE_UNREALIZED); clutter_actor_update_map_state (child, MAP_STATE_MAKE_UNREALIZED);
} }
if (flush_queue)
{
/* We take this opportunity to invalidate any queue redraw entry
* associated with the actor and descendants since we won't be able to
* determine the appropriate stage after this.
*
* we do this after we updated the mapped state because actors might
* end up queueing redraws inside their mapped/unmapped virtual
* functions, and if we invalidate the redraw entry we could end up
* with an inconsistent state and weird memory corruption. see
* bugs:
*
* http://bugzilla.clutter-project.org/show_bug.cgi?id=2621
* https://bugzilla.gnome.org/show_bug.cgi?id=652036
*/
_clutter_actor_traverse (child,
0,
invalidate_queue_redraw_entry,
NULL,
NULL);
}
old_first = self->priv->first_child; old_first = self->priv->first_child;
old_last = self->priv->last_child; old_last = self->priv->last_child;
@ -7901,20 +7860,6 @@ clutter_actor_destroy (ClutterActor *self)
g_object_unref (self); g_object_unref (self);
} }
void
_clutter_actor_finish_queue_redraw (ClutterActor *self)
{
ClutterActorPrivate *priv = self->priv;
/* Remove queue entry early in the process, otherwise a new
queue_redraw() during signal handling could put back this
object in the stage redraw list (but the entry is freed as
soon as we return from this function, causing a segfault
later)
*/
priv->queue_redraw_entry = NULL;
}
void void
_clutter_actor_queue_redraw_full (ClutterActor *self, _clutter_actor_queue_redraw_full (ClutterActor *self,
const ClutterPaintVolume *volume, const ClutterPaintVolume *volume,
@ -7980,11 +7925,9 @@ _clutter_actor_queue_redraw_full (ClutterActor *self,
if (CLUTTER_ACTOR_IN_DESTRUCTION (stage)) if (CLUTTER_ACTOR_IN_DESTRUCTION (stage))
return; return;
self->priv->queue_redraw_entry = clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage),
_clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage), self,
priv->queue_redraw_entry, volume);
self,
volume);
/* If this is the first redraw queued then we can directly use the /* If this is the first redraw queued then we can directly use the
effect parameter */ effect parameter */

13
clutter/clutter/clutter-stage-private.h
View File

@ -31,8 +31,6 @@
G_BEGIN_DECLS G_BEGIN_DECLS
typedef struct _ClutterStageQueueRedrawEntry ClutterStageQueueRedrawEntry;
/* stage */ /* stage */
ClutterStageWindow *_clutter_stage_get_default_window (void); ClutterStageWindow *_clutter_stage_get_default_window (void);
@ -89,11 +87,12 @@ ClutterActor *_clutter_stage_do_pick (ClutterStage *stage,
ClutterPaintVolume *_clutter_stage_paint_volume_stack_allocate (ClutterStage *stage); ClutterPaintVolume *_clutter_stage_paint_volume_stack_allocate (ClutterStage *stage);
void _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage); void _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage);
ClutterStageQueueRedrawEntry *_clutter_stage_queue_actor_redraw (ClutterStage *stage, void clutter_stage_queue_actor_redraw (ClutterStage *stage,
ClutterStageQueueRedrawEntry *entry, ClutterActor *actor,
ClutterActor *actor, const ClutterPaintVolume *clip);
const ClutterPaintVolume *clip);
void _clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry *entry); void clutter_stage_dequeue_actor_redraw (ClutterStage *stage,
ClutterActor *actor);
void _clutter_stage_add_pointer_drag_actor (ClutterStage *stage, void _clutter_stage_add_pointer_drag_actor (ClutterStage *stage,
ClutterInputDevice *device, ClutterInputDevice *device,

139
clutter/clutter/clutter-stage.c
View File

@ -79,12 +79,11 @@
#define MAX_FRUSTA 64 #define MAX_FRUSTA 64
struct _ClutterStageQueueRedrawEntry typedef struct _QueueRedrawEntry
{ {
ClutterActor *actor;
gboolean has_clip; gboolean has_clip;
ClutterPaintVolume clip; ClutterPaintVolume clip;
}; } QueueRedrawEntry;
typedef struct _PickRecord typedef struct _PickRecord
{ {
@ -118,7 +117,7 @@ struct _ClutterStagePrivate
GArray *paint_volume_stack; GArray *paint_volume_stack;
GSList *pending_relayouts; GSList *pending_relayouts;
GList *pending_queue_redraws; GHashTable *pending_queue_redraws;
gint sync_delay; gint sync_delay;
@ -173,7 +172,7 @@ static guint stage_signals[LAST_SIGNAL] = { 0, };
static const ClutterColor default_stage_color = { 255, 255, 255, 255 }; static const ClutterColor default_stage_color = { 255, 255, 255, 255 };
static void free_queue_redraw_entry (ClutterStageQueueRedrawEntry *entry); static void free_queue_redraw_entry (QueueRedrawEntry *entry);
static void capture_view_into (ClutterStage *stage, static void capture_view_into (ClutterStage *stage,
gboolean paint, gboolean paint,
ClutterStageView *view, ClutterStageView *view,
@ -1309,9 +1308,7 @@ clutter_stage_dispose (GObject *object)
clutter_actor_destroy_all_children (CLUTTER_ACTOR (object)); clutter_actor_destroy_all_children (CLUTTER_ACTOR (object));
g_list_free_full (priv->pending_queue_redraws, g_hash_table_remove_all (priv->pending_queue_redraws);
(GDestroyNotify) free_queue_redraw_entry);
priv->pending_queue_redraws = NULL;
g_slist_free_full (priv->pending_relayouts, g_slist_free_full (priv->pending_relayouts,
(GDestroyNotify) g_object_unref); (GDestroyNotify) g_object_unref);
@ -1649,6 +1646,11 @@ clutter_stage_init (ClutterStage *self)
clutter_stage_set_viewport (self, geom.width, geom.height); clutter_stage_set_viewport (self, geom.width, geom.height);
priv->pending_queue_redraws =
g_hash_table_new_full (NULL, NULL,
g_object_unref,
(GDestroyNotify) free_queue_redraw_entry);
priv->paint_volume_stack = priv->paint_volume_stack =
g_array_new (FALSE, FALSE, sizeof (ClutterPaintVolume)); g_array_new (FALSE, FALSE, sizeof (ClutterPaintVolume));
@ -2714,13 +2716,13 @@ _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage)
* paint volume so we can clip the redraw request even if the user * paint volume so we can clip the redraw request even if the user
* didn't explicitly do so. * didn't explicitly do so.
*/ */
ClutterStageQueueRedrawEntry * void
_clutter_stage_queue_actor_redraw (ClutterStage *stage, clutter_stage_queue_actor_redraw (ClutterStage *stage,
ClutterStageQueueRedrawEntry *entry, ClutterActor *actor,
ClutterActor *actor, const ClutterPaintVolume *clip)
const ClutterPaintVolume *clip)
{ {
ClutterStagePrivate *priv = stage->priv; ClutterStagePrivate *priv = stage->priv;
QueueRedrawEntry *entry = NULL;
CLUTTER_NOTE (CLIPPING, "stage_queue_actor_redraw (actor=%s, clip=%p): ", CLUTTER_NOTE (CLIPPING, "stage_queue_actor_redraw (actor=%s, clip=%p): ",
_clutter_actor_get_debug_name (actor), clip); _clutter_actor_get_debug_name (actor), clip);
@ -2745,6 +2747,8 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
priv->pending_finish_queue_redraws = TRUE; priv->pending_finish_queue_redraws = TRUE;
} }
entry = g_hash_table_lookup (priv->pending_queue_redraws, actor);
if (entry) if (entry)
{ {
/* Ignore all requests to queue a redraw for an actor if a full /* Ignore all requests to queue a redraw for an actor if a full
@ -2754,7 +2758,7 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
CLUTTER_NOTE (CLIPPING, "Bail from stage_queue_actor_redraw (%s): " CLUTTER_NOTE (CLIPPING, "Bail from stage_queue_actor_redraw (%s): "
"Unclipped redraw of actor already queued", "Unclipped redraw of actor already queued",
_clutter_actor_get_debug_name (actor)); _clutter_actor_get_debug_name (actor));
return entry; return;
} }
/* If queuing a clipped redraw and a clipped redraw has /* If queuing a clipped redraw and a clipped redraw has
@ -2767,12 +2771,10 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
clutter_paint_volume_free (&entry->clip); clutter_paint_volume_free (&entry->clip);
entry->has_clip = FALSE; entry->has_clip = FALSE;
} }
return entry;
} }
else else
{ {
entry = g_slice_new (ClutterStageQueueRedrawEntry); entry = g_slice_new (QueueRedrawEntry);
entry->actor = g_object_ref (actor);
if (clip) if (clip)
{ {
@ -2783,40 +2785,24 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
else else
entry->has_clip = FALSE; entry->has_clip = FALSE;
stage->priv->pending_queue_redraws = g_hash_table_insert (priv->pending_queue_redraws,
g_list_prepend (stage->priv->pending_queue_redraws, entry); g_object_ref (actor), entry);
return entry;
} }
} }
static void static void
free_queue_redraw_entry (ClutterStageQueueRedrawEntry *entry) free_queue_redraw_entry (QueueRedrawEntry *entry)
{ {
if (entry->actor)
g_object_unref (entry->actor);
if (entry->has_clip) if (entry->has_clip)
clutter_paint_volume_free (&entry->clip); clutter_paint_volume_free (&entry->clip);
g_slice_free (ClutterStageQueueRedrawEntry, entry); g_slice_free (QueueRedrawEntry, entry);
} }
void void
_clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry *entry) clutter_stage_dequeue_actor_redraw (ClutterStage *self,
ClutterActor *actor)
{ {
if (entry == NULL) g_hash_table_remove (self->priv->pending_queue_redraws, actor);
return;
if (entry->actor != NULL)
{
g_object_unref (entry->actor);
entry->actor = NULL;
}
if (entry->has_clip)
{
clutter_paint_volume_free (&entry->clip);
entry->has_clip = FALSE;
}
} }
static void static void
@ -2882,7 +2868,8 @@ void
clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage) clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
{ {
ClutterStagePrivate *priv = stage->priv; ClutterStagePrivate *priv = stage->priv;
GList *l; GHashTableIter iter;
gpointer key, value;
COGL_TRACE_BEGIN_SCOPED (ClutterStageFinishQueueRedraws, "FinishQueueRedraws"); COGL_TRACE_BEGIN_SCOPED (ClutterStageFinishQueueRedraws, "FinishQueueRedraws");
@ -2891,50 +2878,44 @@ clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
priv->pending_finish_queue_redraws = FALSE; priv->pending_finish_queue_redraws = FALSE;
for (l = priv->pending_queue_redraws; l; l = l->next) g_hash_table_iter_init (&iter, priv->pending_queue_redraws);
while (g_hash_table_iter_next (&iter, &key, &value))
{ {
ClutterStageQueueRedrawEntry *entry = l->data; ClutterActor *redraw_actor = key;
QueueRedrawEntry *entry = value;
ClutterPaintVolume old_actor_pv, new_actor_pv;
/* NB: Entries may be invalidated if the actor gets destroyed */ _clutter_paint_volume_init_static (&old_actor_pv, NULL);
if (G_LIKELY (entry->actor != NULL)) _clutter_paint_volume_init_static (&new_actor_pv, NULL);
if (entry->has_clip)
{ {
ClutterPaintVolume old_actor_pv, new_actor_pv; add_to_stage_clip (stage, &entry->clip);
}
_clutter_paint_volume_init_static (&old_actor_pv, NULL); else if (clutter_actor_get_redraw_clip (redraw_actor,
_clutter_paint_volume_init_static (&new_actor_pv, NULL); &old_actor_pv,
&new_actor_pv))
if (entry->has_clip) {
{ /* Add both the old paint volume of the actor (which is
add_to_stage_clip (stage, &entry->clip); * currently visible on the screen) and the new paint volume
} * (which will be visible on the screen after this redraw)
else if (clutter_actor_get_redraw_clip (entry->actor, * to the redraw clip.
&old_actor_pv, * The former we do to ensure the old texture on the screen
&new_actor_pv)) * will be fully painted over in case the actor was moved.
{ */
/* Add both the old paint volume of the actor (which is add_to_stage_clip (stage, &old_actor_pv);
* currently visible on the screen) and the new paint volume add_to_stage_clip (stage, &new_actor_pv);
* (which will be visible on the screen after this redraw) }
* to the redraw clip. else
* The former we do to ensure the old texture on the screen {
* will be fully painted over in case the actor was moved. /* If there's no clip we can use, we have to trigger an
*/ * unclipped full stage redraw.
add_to_stage_clip (stage, &old_actor_pv); */
add_to_stage_clip (stage, &new_actor_pv); add_to_stage_clip (stage, NULL);
}
else
{
/* If there's no clip we can use, we have to trigger an
* unclipped full stage redraw.
*/
add_to_stage_clip (stage, NULL);
}
} }
free_queue_redraw_entry (entry); g_hash_table_iter_remove (&iter);
} }
g_list_free (priv->pending_queue_redraws);
priv->pending_queue_redraws = NULL;
} }
/** /**