[mutter] clutter/stage: Don't pass QueueRedrawEntries to actors
- From: Marge Bot <marge-bot src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [mutter] clutter/stage: Don't pass QueueRedrawEntries to actors
- Date: Tue, 24 Nov 2020 19:20:43 +0000 (UTC)
commit 906124b09fe712a25907ac444f173f93ad3e7951
Author: Jonas Dreßler <verdre v0yd nl>
Date: Sat Oct 17 22:29:05 2020 +0200
clutter/stage: Don't pass QueueRedrawEntries to actors
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>
clutter/clutter/clutter-actor.c | 75 +++--------------
clutter/clutter/clutter-stage-private.h | 13 ++-
clutter/clutter/clutter-stage.c | 139 ++++++++++++++------------------
3 files changed, 75 insertions(+), 152 deletions(-)
---
diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c
index 03e49a2d59..c061f722f9 100644
--- a/clutter/clutter/clutter-actor.c
+++ b/clutter/clutter/clutter-actor.c
@@ -775,8 +775,6 @@ struct _ClutterActorPrivate
*/
ClutterPaintVolume last_paint_volume;
- ClutterStageQueueRedrawEntry *queue_redraw_entry;
-
ClutterColor bg_color;
#ifdef CLUTTER_ENABLE_DEBUG
@@ -2117,6 +2115,9 @@ unrealize_actor_after_children_cb (ClutterActor *self,
priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT)
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)
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
remove_child (ClutterActor *self,
ClutterActor *child)
@@ -4107,10 +4092,9 @@ typedef enum
REMOVE_CHILD_EMIT_PARENT_SET = 1 << 1,
REMOVE_CHILD_EMIT_ACTOR_REMOVED = 1 << 2,
REMOVE_CHILD_CHECK_STATE = 1 << 3,
- REMOVE_CHILD_FLUSH_QUEUE = 1 << 4,
- REMOVE_CHILD_NOTIFY_FIRST_LAST = 1 << 5,
- REMOVE_CHILD_STOP_TRANSITIONS = 1 << 6,
- REMOVE_CHILD_CLEAR_STAGE_VIEWS = 1 << 7,
+ REMOVE_CHILD_NOTIFY_FIRST_LAST = 1 << 4,
+ REMOVE_CHILD_STOP_TRANSITIONS = 1 << 5,
+ REMOVE_CHILD_CLEAR_STAGE_VIEWS = 1 << 6,
/* default flags for public API */
REMOVE_CHILD_DEFAULT_FLAGS = REMOVE_CHILD_STOP_TRANSITIONS |
@@ -4118,7 +4102,6 @@ typedef enum
REMOVE_CHILD_EMIT_PARENT_SET |
REMOVE_CHILD_EMIT_ACTOR_REMOVED |
REMOVE_CHILD_CHECK_STATE |
- REMOVE_CHILD_FLUSH_QUEUE |
REMOVE_CHILD_NOTIFY_FIRST_LAST |
REMOVE_CHILD_CLEAR_STAGE_VIEWS,
} ClutterActorRemoveChildFlags;
@@ -4138,7 +4121,6 @@ clutter_actor_remove_child_internal (ClutterActor *self,
{
ClutterActor *old_first, *old_last;
gboolean destroy_meta, emit_parent_set, emit_actor_removed, check_state;
- gboolean flush_queue;
gboolean notify_first_last;
gboolean stop_transitions;
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_actor_removed = (flags & REMOVE_CHILD_EMIT_ACTOR_REMOVED) != 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;
stop_transitions = (flags & REMOVE_CHILD_STOP_TRANSITIONS) != 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);
}
- 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_last = self->priv->last_child;
@@ -7901,20 +7860,6 @@ clutter_actor_destroy (ClutterActor *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
_clutter_actor_queue_redraw_full (ClutterActor *self,
const ClutterPaintVolume *volume,
@@ -7980,11 +7925,9 @@ _clutter_actor_queue_redraw_full (ClutterActor *self,
if (CLUTTER_ACTOR_IN_DESTRUCTION (stage))
return;
- self->priv->queue_redraw_entry =
- _clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage),
- priv->queue_redraw_entry,
- self,
- volume);
+ clutter_stage_queue_actor_redraw (CLUTTER_STAGE (stage),
+ self,
+ volume);
/* If this is the first redraw queued then we can directly use the
effect parameter */
diff --git a/clutter/clutter/clutter-stage-private.h b/clutter/clutter/clutter-stage-private.h
index 85f54e4976..6bfdeec79a 100644
--- a/clutter/clutter/clutter-stage-private.h
+++ b/clutter/clutter/clutter-stage-private.h
@@ -31,8 +31,6 @@
G_BEGIN_DECLS
-typedef struct _ClutterStageQueueRedrawEntry ClutterStageQueueRedrawEntry;
-
/* stage */
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);
void _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage);
-ClutterStageQueueRedrawEntry *_clutter_stage_queue_actor_redraw (ClutterStage
*stage,
- ClutterStageQueueRedrawEntry
*entry,
- ClutterActor
*actor,
- const ClutterPaintVolume
*clip);
-void _clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry
*entry);
+void clutter_stage_queue_actor_redraw (ClutterStage *stage,
+ ClutterActor *actor,
+ const ClutterPaintVolume *clip);
+
+void clutter_stage_dequeue_actor_redraw (ClutterStage *stage,
+ ClutterActor *actor);
void _clutter_stage_add_pointer_drag_actor (ClutterStage *stage,
ClutterInputDevice *device,
diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c
index 6afc40ea5c..d9191c0a25 100644
--- a/clutter/clutter/clutter-stage.c
+++ b/clutter/clutter/clutter-stage.c
@@ -79,12 +79,11 @@
#define MAX_FRUSTA 64
-struct _ClutterStageQueueRedrawEntry
+typedef struct _QueueRedrawEntry
{
- ClutterActor *actor;
gboolean has_clip;
ClutterPaintVolume clip;
-};
+} QueueRedrawEntry;
typedef struct _PickRecord
{
@@ -118,7 +117,7 @@ struct _ClutterStagePrivate
GArray *paint_volume_stack;
GSList *pending_relayouts;
- GList *pending_queue_redraws;
+ GHashTable *pending_queue_redraws;
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 void free_queue_redraw_entry (ClutterStageQueueRedrawEntry *entry);
+static void free_queue_redraw_entry (QueueRedrawEntry *entry);
static void capture_view_into (ClutterStage *stage,
gboolean paint,
ClutterStageView *view,
@@ -1309,9 +1308,7 @@ clutter_stage_dispose (GObject *object)
clutter_actor_destroy_all_children (CLUTTER_ACTOR (object));
- g_list_free_full (priv->pending_queue_redraws,
- (GDestroyNotify) free_queue_redraw_entry);
- priv->pending_queue_redraws = NULL;
+ g_hash_table_remove_all (priv->pending_queue_redraws);
g_slist_free_full (priv->pending_relayouts,
(GDestroyNotify) g_object_unref);
@@ -1649,6 +1646,11 @@ clutter_stage_init (ClutterStage *self)
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 =
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
* didn't explicitly do so.
*/
-ClutterStageQueueRedrawEntry *
-_clutter_stage_queue_actor_redraw (ClutterStage *stage,
- ClutterStageQueueRedrawEntry *entry,
- ClutterActor *actor,
- const ClutterPaintVolume *clip)
+void
+clutter_stage_queue_actor_redraw (ClutterStage *stage,
+ ClutterActor *actor,
+ const ClutterPaintVolume *clip)
{
ClutterStagePrivate *priv = stage->priv;
+ QueueRedrawEntry *entry = NULL;
CLUTTER_NOTE (CLIPPING, "stage_queue_actor_redraw (actor=%s, clip=%p): ",
_clutter_actor_get_debug_name (actor), clip);
@@ -2745,6 +2747,8 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
priv->pending_finish_queue_redraws = TRUE;
}
+ entry = g_hash_table_lookup (priv->pending_queue_redraws, actor);
+
if (entry)
{
/* 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): "
"Unclipped redraw of actor already queued",
_clutter_actor_get_debug_name (actor));
- return entry;
+ return;
}
/* 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);
entry->has_clip = FALSE;
}
- return entry;
}
else
{
- entry = g_slice_new (ClutterStageQueueRedrawEntry);
- entry->actor = g_object_ref (actor);
+ entry = g_slice_new (QueueRedrawEntry);
if (clip)
{
@@ -2783,40 +2785,24 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
else
entry->has_clip = FALSE;
- stage->priv->pending_queue_redraws =
- g_list_prepend (stage->priv->pending_queue_redraws, entry);
-
- return entry;
+ g_hash_table_insert (priv->pending_queue_redraws,
+ g_object_ref (actor), entry);
}
}
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)
clutter_paint_volume_free (&entry->clip);
- g_slice_free (ClutterStageQueueRedrawEntry, entry);
+ g_slice_free (QueueRedrawEntry, entry);
}
void
-_clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry *entry)
+clutter_stage_dequeue_actor_redraw (ClutterStage *self,
+ ClutterActor *actor)
{
- if (entry == NULL)
- 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;
- }
+ g_hash_table_remove (self->priv->pending_queue_redraws, actor);
}
static void
@@ -2882,7 +2868,8 @@ void
clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
{
ClutterStagePrivate *priv = stage->priv;
- GList *l;
+ GHashTableIter iter;
+ gpointer key, value;
COGL_TRACE_BEGIN_SCOPED (ClutterStageFinishQueueRedraws, "FinishQueueRedraws");
@@ -2891,50 +2878,44 @@ clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage)
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 */
- if (G_LIKELY (entry->actor != NULL))
- {
- ClutterPaintVolume old_actor_pv, new_actor_pv;
-
- _clutter_paint_volume_init_static (&old_actor_pv, NULL);
- _clutter_paint_volume_init_static (&new_actor_pv, NULL);
+ _clutter_paint_volume_init_static (&old_actor_pv, NULL);
+ _clutter_paint_volume_init_static (&new_actor_pv, NULL);
- if (entry->has_clip)
- {
- add_to_stage_clip (stage, &entry->clip);
- }
- else if (clutter_actor_get_redraw_clip (entry->actor,
- &old_actor_pv,
- &new_actor_pv))
- {
- /* Add both the old paint volume of the actor (which is
- * currently visible on the screen) and the new paint volume
- * (which will be visible on the screen after this redraw)
- * to the redraw clip.
- * The former we do to ensure the old texture on the screen
- * will be fully painted over in case the actor was moved.
- */
- add_to_stage_clip (stage, &old_actor_pv);
- add_to_stage_clip (stage, &new_actor_pv);
- }
- else
- {
- /* If there's no clip we can use, we have to trigger an
- * unclipped full stage redraw.
- */
- add_to_stage_clip (stage, NULL);
- }
+ if (entry->has_clip)
+ {
+ add_to_stage_clip (stage, &entry->clip);
+ }
+ else if (clutter_actor_get_redraw_clip (redraw_actor,
+ &old_actor_pv,
+ &new_actor_pv))
+ {
+ /* Add both the old paint volume of the actor (which is
+ * currently visible on the screen) and the new paint volume
+ * (which will be visible on the screen after this redraw)
+ * to the redraw clip.
+ * The former we do to ensure the old texture on the screen
+ * will be fully painted over in case the actor was moved.
+ */
+ add_to_stage_clip (stage, &old_actor_pv);
+ add_to_stage_clip (stage, &new_actor_pv);
+ }
+ 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;
}
/**
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
