From 9b16eff78466dab38f9fe88f5bb29047190ad8c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Mon, 19 Oct 2020 12:21:39 +0200 Subject: [PATCH] clutter: Move assembling the redraw clip out of "queue-redraw" signal Putting together the redraw clip of the stage never really fitted nicely with the "queue-redraw" signal emission, it forces us to emit the signals in a batch and we also use a weird trick to get the old paint volume that's already on-screen into the final redraw clip (we call _clutter_actor_propagate_queue_redraw() on the stage). So start breaking up this association by making the stage explicitely request the redraw clip from the actor and removing the ClutterPaintVolume argument from _clutter_actor_finish_queue_redraw(). This is done by adding a private function clutter_actor_get_redraw_clip() which returns our old (currently visible) paint volume and the new paint volume. This also allows removing the check whether a full stage redraw has been queued in clutter_actor_real_queue_redraw() and we can now just stop the signal emission if a propagation happened at least once. Part-of: --- clutter/clutter/clutter-actor-private.h | 7 +- clutter/clutter/clutter-actor.c | 92 +++++--------- clutter/clutter/clutter-actor.h | 3 +- clutter/clutter/clutter-stage.c | 161 ++++++++++++++---------- 4 files changed, 132 insertions(+), 131 deletions(-) diff --git a/clutter/clutter/clutter-actor-private.h b/clutter/clutter/clutter-actor-private.h index 01caa0296..c34d772e8 100644 --- a/clutter/clutter/clutter-actor-private.h +++ b/clutter/clutter/clutter-actor-private.h @@ -232,8 +232,7 @@ void _clutter_actor_queue_redraw_full const ClutterPaintVolume *volume, ClutterEffect *effect); -void _clutter_actor_finish_queue_redraw (ClutterActor *self, - ClutterPaintVolume *clip); +void _clutter_actor_finish_queue_redraw (ClutterActor *self); gboolean _clutter_actor_set_default_paint_volume (ClutterActor *self, GType check_gtype, @@ -268,6 +267,10 @@ void clutter_actor_queue_immediate_relayout (ClutterActor *self); gboolean clutter_actor_is_painting_unmapped (ClutterActor *self); +gboolean clutter_actor_get_redraw_clip (ClutterActor *self, + ClutterPaintVolume *dst_old_pv, + ClutterPaintVolume *dst_new_pv); + G_END_DECLS #endif /* __CLUTTER_ACTOR_PRIVATE_H__ */ diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index cabd95f8f..114d52a9b 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2602,9 +2602,8 @@ _clutter_actor_queue_redraw_on_clones (ClutterActor *self) } static void -_clutter_actor_propagate_queue_redraw (ClutterActor *self, - ClutterActor *origin, - ClutterPaintVolume *pv) +_clutter_actor_propagate_queue_redraw (ClutterActor *self, + ClutterActor *origin) { gboolean stop = FALSE; @@ -2624,11 +2623,11 @@ _clutter_actor_propagate_queue_redraw (ClutterActor *self, if (g_signal_has_handler_pending (self, actor_signals[QUEUE_REDRAW], 0, TRUE)) { - g_signal_emit (self, actor_signals[QUEUE_REDRAW], 0, origin, pv, &stop); + g_signal_emit (self, actor_signals[QUEUE_REDRAW], 0, origin, &stop); } else { - stop = CLUTTER_ACTOR_GET_CLASS (self)->queue_redraw (self, origin, pv); + stop = CLUTTER_ACTOR_GET_CLASS (self)->queue_redraw (self, origin); } if (stop) @@ -2639,9 +2638,8 @@ _clutter_actor_propagate_queue_redraw (ClutterActor *self, } static gboolean -clutter_actor_real_queue_redraw (ClutterActor *self, - ClutterActor *origin, - ClutterPaintVolume *paint_volume) +clutter_actor_real_queue_redraw (ClutterActor *self, + ClutterActor *origin) { CLUTTER_NOTE (PAINT, "Redraw queued on '%s' (from: '%s')", _clutter_actor_get_debug_name (self), @@ -2667,20 +2665,13 @@ clutter_actor_real_queue_redraw (ClutterActor *self, if (!CLUTTER_ACTOR_IS_VISIBLE (self)) return TRUE; - /* Although we could determine here that a full stage redraw - * has already been queued and immediately bail out, we actually - * guarantee that we will propagate a queue-redraw signal to our - * parent at least once so that it's possible to implement a + /* We guarantee that we will propagate a queue-redraw signal up + * the tree at least once so that it's possible to implement a * container that tracks which of its children have queued a * redraw. */ if (self->priv->propagated_one_redraw) - { - ClutterActor *stage = _clutter_actor_get_stage_internal (self); - if (stage != NULL && - _clutter_stage_has_full_redraw_queued (CLUTTER_STAGE (stage))) - return TRUE; - } + return TRUE; self->priv->propagated_one_redraw = TRUE; @@ -7414,13 +7405,12 @@ clutter_actor_class_init (ClutterActorClass *klass) G_STRUCT_OFFSET (ClutterActorClass, queue_redraw), g_signal_accumulator_true_handled, NULL, - _clutter_marshal_BOOLEAN__OBJECT_BOXED, - G_TYPE_BOOLEAN, 2, - CLUTTER_TYPE_ACTOR, - CLUTTER_TYPE_PAINT_VOLUME); + _clutter_marshal_BOOLEAN__OBJECT, + G_TYPE_BOOLEAN, 1, + CLUTTER_TYPE_ACTOR); g_signal_set_va_marshaller (actor_signals[QUEUE_REDRAW], G_TYPE_FROM_CLASS (object_class), - _clutter_marshal_BOOLEAN__OBJECT_BOXEDv); + _clutter_marshal_BOOLEAN__OBJECTv); /** * ClutterActor::queue-relayout: @@ -8022,11 +8012,9 @@ clutter_actor_destroy (ClutterActor *self) } void -_clutter_actor_finish_queue_redraw (ClutterActor *self, - ClutterPaintVolume *clip) +_clutter_actor_finish_queue_redraw (ClutterActor *self) { ClutterActorPrivate *priv = self->priv; - ClutterPaintVolume *pv = NULL; /* Remove queue entry early in the process, otherwise a new queue_redraw() during signal handling could put back this @@ -8036,39 +8024,7 @@ _clutter_actor_finish_queue_redraw (ClutterActor *self, */ priv->queue_redraw_entry = NULL; - /* If we've been explicitly passed a clip volume then there's - * nothing more to calculate, but otherwise the only thing we know - * is that the change is constrained to the given actor. - * - * The idea is that if we know the paint volume for where the actor - * was last drawn (in eye coordinates) and we also have the paint - * volume for where it will be drawn next (in actor coordinates) - * then if we queue a redraw for both these volumes that will cover - * everything that needs to be redrawn to clear the old view and - * show the latest view of the actor. - * - * Don't clip this redraw if we don't know what position we had for - * the previous redraw since we don't know where to set the clip so - * it will clear the actor as it is currently. - */ - if (clip) - { - pv = clip; - } - else if (G_LIKELY (priv->last_paint_volume_valid)) - { - pv = _clutter_actor_get_paint_volume_mutable (self); - if (pv) - { - ClutterActor *stage = _clutter_actor_get_stage_internal (self); - - /* make sure we redraw the actors old position... */ - _clutter_actor_propagate_queue_redraw (stage, stage, - &priv->last_paint_volume); - } - } - - _clutter_actor_propagate_queue_redraw (self, self, pv); + _clutter_actor_propagate_queue_redraw (self, self); } void @@ -19667,3 +19623,21 @@ clutter_actor_invalidate_transform (ClutterActor *self) transform_changed (self); } + +gboolean +clutter_actor_get_redraw_clip (ClutterActor *self, + ClutterPaintVolume *dst_old_pv, + ClutterPaintVolume *dst_new_pv) +{ + ClutterActorPrivate *priv = self->priv; + ClutterPaintVolume *paint_volume; + + paint_volume = _clutter_actor_get_paint_volume_mutable (self); + if (!paint_volume || !priv->last_paint_volume_valid) + return FALSE; + + _clutter_paint_volume_set_from_volume (dst_old_pv, &priv->last_paint_volume); + _clutter_paint_volume_set_from_volume (dst_new_pv, paint_volume); + + return TRUE; +} diff --git a/clutter/clutter/clutter-actor.h b/clutter/clutter/clutter-actor.h index bc858fb5a..fac4e47c3 100644 --- a/clutter/clutter/clutter-actor.h +++ b/clutter/clutter/clutter-actor.h @@ -240,8 +240,7 @@ struct _ClutterActorClass ClutterPickContext *pick_context); gboolean (* queue_redraw) (ClutterActor *actor, - ClutterActor *leaf_that_queued, - ClutterPaintVolume *paint_volume); + ClutterActor *leaf_that_queued); /* size negotiation */ void (* get_preferred_width) (ClutterActor *self, diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index 3808d2955..aa64c0277 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -1054,68 +1054,6 @@ is_full_stage_redraw_queued (ClutterStage *stage) return TRUE; } -static gboolean -clutter_stage_real_queue_redraw (ClutterActor *actor, - ClutterActor *leaf, - ClutterPaintVolume *redraw_clip) -{ - ClutterStage *stage = CLUTTER_STAGE (actor); - ClutterStageWindow *stage_window; - ClutterActorBox bounding_box; - ClutterActorBox intersection_box; - cairo_rectangle_int_t geom, stage_clip; - - if (CLUTTER_ACTOR_IN_DESTRUCTION (actor)) - return TRUE; - - /* If the backend can't do anything with redraw clips (e.g. it already knows - * it needs to redraw everything anyway) then don't spend time transforming - * any clip volume into stage coordinates... */ - stage_window = _clutter_stage_get_window (stage); - if (stage_window == NULL) - return TRUE; - - if (is_full_stage_redraw_queued (stage)) - return FALSE; - - if (redraw_clip == NULL) - { - clutter_stage_add_redraw_clip (stage, NULL); - return FALSE; - } - - if (redraw_clip->is_empty) - return TRUE; - - /* Convert the clip volume into stage coordinates and then into an - * axis aligned stage coordinates bounding box... */ - _clutter_paint_volume_get_stage_paint_box (redraw_clip, - stage, - &bounding_box); - - _clutter_stage_window_get_geometry (stage_window, &geom); - - intersection_box.x1 = MAX (bounding_box.x1, 0); - intersection_box.y1 = MAX (bounding_box.y1, 0); - intersection_box.x2 = MIN (bounding_box.x2, geom.width); - intersection_box.y2 = MIN (bounding_box.y2, geom.height); - - /* There is no need to track degenerate/empty redraw clips */ - if (intersection_box.x2 <= intersection_box.x1 || - intersection_box.y2 <= intersection_box.y1) - return TRUE; - - /* when converting to integer coordinates make sure we round the edges of the - * clip rectangle outwards... */ - stage_clip.x = intersection_box.x1; - stage_clip.y = intersection_box.y1; - stage_clip.width = intersection_box.x2 - stage_clip.x; - stage_clip.height = intersection_box.y2 - stage_clip.y; - - clutter_stage_add_redraw_clip (stage, &stage_clip); - return FALSE; -} - gboolean _clutter_stage_has_full_redraw_queued (ClutterStage *stage) { @@ -1437,7 +1375,6 @@ clutter_stage_class_init (ClutterStageClass *klass) actor_class->hide = clutter_stage_hide; actor_class->hide_all = clutter_stage_hide_all; actor_class->queue_relayout = clutter_stage_real_queue_relayout; - actor_class->queue_redraw = clutter_stage_real_queue_redraw; actor_class->apply_transform = clutter_stage_real_apply_transform; klass->paint_view = clutter_stage_real_paint_view; @@ -2882,6 +2819,65 @@ _clutter_stage_queue_redraw_entry_invalidate (ClutterStageQueueRedrawEntry *entr } } +static void +add_to_stage_clip (ClutterStage *stage, + ClutterPaintVolume *redraw_clip) +{ + ClutterStageWindow *stage_window; + ClutterActorBox bounding_box; + ClutterActorBox intersection_box; + cairo_rectangle_int_t geom, stage_clip; + + if (CLUTTER_ACTOR_IN_DESTRUCTION (CLUTTER_ACTOR (stage))) + return; + + /* If the backend can't do anything with redraw clips (e.g. it already knows + * it needs to redraw everything anyway) then don't spend time transforming + * any clip volume into stage coordinates... */ + stage_window = _clutter_stage_get_window (stage); + if (stage_window == NULL) + return; + + if (is_full_stage_redraw_queued (stage)) + return; + + if (redraw_clip == NULL) + { + clutter_stage_add_redraw_clip (stage, NULL); + return; + } + + if (redraw_clip->is_empty) + return; + + /* Convert the clip volume into stage coordinates and then into an + * axis aligned stage coordinates bounding box... */ + _clutter_paint_volume_get_stage_paint_box (redraw_clip, + stage, + &bounding_box); + + _clutter_stage_window_get_geometry (stage_window, &geom); + + intersection_box.x1 = MAX (bounding_box.x1, 0); + intersection_box.y1 = MAX (bounding_box.y1, 0); + intersection_box.x2 = MIN (bounding_box.x2, geom.width); + intersection_box.y2 = MIN (bounding_box.y2, geom.height); + + /* There is no need to track degenerate/empty redraw clips */ + if (intersection_box.x2 <= intersection_box.x1 || + intersection_box.y2 <= intersection_box.y1) + return; + + /* when converting to integer coordinates make sure we round the edges of the + * clip rectangle outwards... */ + stage_clip.x = intersection_box.x1; + stage_clip.y = intersection_box.y1; + stage_clip.width = intersection_box.x2 - stage_clip.x; + stage_clip.height = intersection_box.y2 - stage_clip.y; + + clutter_stage_add_redraw_clip (stage, &stage_clip); +} + void clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage) { @@ -2913,15 +2909,44 @@ clutter_stage_maybe_finish_queue_redraws (ClutterStage *stage) for (l = stolen_list; l; l = l->next) { ClutterStageQueueRedrawEntry *entry = l->data; - ClutterPaintVolume *clip; /* NB: Entries may be invalidated if the actor gets destroyed */ if (G_LIKELY (entry->actor != NULL)) - { - clip = entry->has_clip ? &entry->clip : NULL; + { + ClutterPaintVolume old_actor_pv, new_actor_pv; - _clutter_actor_finish_queue_redraw (entry->actor, clip); - } + _clutter_actor_finish_queue_redraw (entry->actor); + + _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); + } + + } free_queue_redraw_entry (entry); }