From e27a9ae078ea2d600f25a9ef8f64da5acc702bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Thu, 12 Mar 2020 12:13:53 +0100 Subject: [PATCH 1/5] clutter/actor: Cache the complete transformation matrices of actors If we want to cache the absolute transformation matrix of an actor (the matrix relative to eye coordinates), we need to ensure the actor-to-parent transformation matrices get properly invalidated. This is currently only the case if the actor subclass does not override the apply_transform() vfunc. So provide a new API that allows invalidating the transformation matrix for actors implementing custom transformations, too. This in turn allows us to cache the whole matrix and not only the part that's put together by Clutter. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1299 --- clutter/clutter/clutter-actor.c | 86 +++++++++++++++++++++------------ clutter/clutter/clutter-actor.h | 8 ++- clutter/clutter/clutter-clone.c | 1 + clutter/clutter/clutter-stage.c | 2 + 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 86ffe2f1f06..ddd16805933 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2538,6 +2538,12 @@ absolute_allocation_changed_cb (ClutterActor *actor, return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; } +static void +transform_changed (ClutterActor *actor) +{ + actor->priv->transform_valid = FALSE; +} + /*< private > * clutter_actor_set_allocation_internal: * @self: a #ClutterActor @@ -2597,7 +2603,7 @@ clutter_actor_set_allocation_internal (ClutterActor *self, CLUTTER_NOTE (LAYOUT, "Allocation for '%s' changed", _clutter_actor_get_debug_name (self)); - priv->transform_valid = FALSE; + transform_changed (self); g_object_notify_by_pspec (obj, obj_props[PROP_ALLOCATION]); @@ -3034,14 +3040,9 @@ clutter_actor_real_apply_transform (ClutterActor *self, ClutterMatrix *matrix) { ClutterActorPrivate *priv = self->priv; - CoglMatrix *transform = &priv->transform; const ClutterTransformInfo *info; float pivot_x = 0.f, pivot_y = 0.f; - /* we already have a cached transformation */ - if (priv->transform_valid) - goto multiply_and_return; - info = _clutter_actor_get_transform_info_or_defaults (self); /* compute the pivot point given the allocated size */ @@ -3067,10 +3068,10 @@ clutter_actor_real_apply_transform (ClutterActor *self, const ClutterTransformInfo *parent_info; parent_info = _clutter_actor_get_transform_info_or_defaults (priv->parent); - clutter_matrix_init_from_matrix (transform, &(parent_info->child_transform)); + clutter_matrix_init_from_matrix (matrix, &(parent_info->child_transform)); } else - clutter_matrix_init_identity (transform); + clutter_matrix_init_identity (matrix); /* if we have an overriding transformation, we use that, and get out */ if (info->transform_set) @@ -3079,11 +3080,11 @@ clutter_actor_real_apply_transform (ClutterActor *self, * translations, since :transform is relative to the actor's coordinate * space, and to the pivot point */ - cogl_matrix_translate (transform, + cogl_matrix_translate (matrix, priv->allocation.x1 + pivot_x, priv->allocation.y1 + pivot_y, info->pivot_z); - cogl_matrix_multiply (transform, transform, &info->transform); + cogl_matrix_multiply (matrix, matrix, &info->transform); goto roll_back_pivot; } @@ -3091,7 +3092,7 @@ clutter_actor_real_apply_transform (ClutterActor *self, * of decomposing the pivot and translation info separate operations, * we just compose everything into a single translation */ - cogl_matrix_translate (transform, + cogl_matrix_translate (matrix, priv->allocation.x1 + pivot_x + info->translation.x, priv->allocation.y1 + pivot_y + info->translation.y, info->z_position + info->pivot_z + info->translation.z); @@ -3108,27 +3109,21 @@ clutter_actor_real_apply_transform (ClutterActor *self, * code we use when interpolating transformations */ if (info->scale_x != 1.0 || info->scale_y != 1.0 || info->scale_z != 1.0) - cogl_matrix_scale (transform, info->scale_x, info->scale_y, info->scale_z); + cogl_matrix_scale (matrix, info->scale_x, info->scale_y, info->scale_z); if (info->rz_angle) - cogl_matrix_rotate (transform, info->rz_angle, 0, 0, 1.0); + cogl_matrix_rotate (matrix, info->rz_angle, 0, 0, 1.0); if (info->ry_angle) - cogl_matrix_rotate (transform, info->ry_angle, 0, 1.0, 0); + cogl_matrix_rotate (matrix, info->ry_angle, 0, 1.0, 0); if (info->rx_angle) - cogl_matrix_rotate (transform, info->rx_angle, 1.0, 0, 0); + cogl_matrix_rotate (matrix, info->rx_angle, 1.0, 0, 0); roll_back_pivot: /* roll back the pivot translation */ if (pivot_x != 0.f || pivot_y != 0.f || info->pivot_z != 0.f) - cogl_matrix_translate (transform, -pivot_x, -pivot_y, -info->pivot_z); - - /* we have a valid modelview */ - priv->transform_valid = TRUE; - -multiply_and_return: - cogl_matrix_multiply (matrix, matrix, &priv->transform); + cogl_matrix_translate (matrix, -pivot_x, -pivot_y, -info->pivot_z); } /* Applies the transforms associated with this actor to the given @@ -3137,7 +3132,17 @@ void _clutter_actor_apply_modelview_transform (ClutterActor *self, ClutterMatrix *matrix) { - CLUTTER_ACTOR_GET_CLASS (self)->apply_transform (self, matrix); + ClutterActorPrivate *priv = self->priv; + + if (priv->transform_valid) + goto out; + + CLUTTER_ACTOR_GET_CLASS (self)->apply_transform (self, &priv->transform); + + priv->transform_valid = TRUE; + +out: + cogl_matrix_multiply (matrix, matrix, &priv->transform); } /* @@ -4457,7 +4462,7 @@ clutter_actor_set_pivot_point_internal (ClutterActor *self, info = _clutter_actor_get_transform_info (self); info->pivot = *pivot; - self->priv->transform_valid = FALSE; + transform_changed (self); g_object_notify_by_pspec (G_OBJECT (self), obj_props[PROP_PIVOT_POINT]); @@ -4473,7 +4478,7 @@ clutter_actor_set_pivot_point_z_internal (ClutterActor *self, info = _clutter_actor_get_transform_info (self); info->pivot_z = pivot_z; - self->priv->transform_valid = FALSE; + transform_changed (self); g_object_notify_by_pspec (G_OBJECT (self), obj_props[PROP_PIVOT_POINT_Z]); @@ -4507,7 +4512,8 @@ clutter_actor_set_translation_internal (ClutterActor *self, else g_assert_not_reached (); - self->priv->transform_valid = FALSE; + transform_changed (self); + clutter_actor_queue_redraw (self); g_object_notify_by_pspec (obj, pspec); } @@ -4639,7 +4645,7 @@ clutter_actor_set_rotation_angle_internal (ClutterActor *self, else g_assert_not_reached (); - self->priv->transform_valid = FALSE; + transform_changed (self); clutter_actor_queue_redraw (self); @@ -4764,7 +4770,8 @@ clutter_actor_set_scale_factor_internal (ClutterActor *self, else g_assert_not_reached (); - self->priv->transform_valid = FALSE; + transform_changed (self); + clutter_actor_queue_redraw (self); g_object_notify_by_pspec (obj, pspec); } @@ -11187,7 +11194,7 @@ clutter_actor_set_z_position_internal (ClutterActor *self, { info->z_position = z_position; - self->priv->transform_valid = FALSE; + transform_changed (self); clutter_actor_queue_redraw (self); @@ -14659,7 +14666,7 @@ clutter_actor_set_transform_internal (ClutterActor *self, info->transform = *transform; info->transform_set = !cogl_matrix_is_identity (&info->transform); - self->priv->transform_valid = FALSE; + transform_changed (self); clutter_actor_queue_redraw (self); @@ -19189,7 +19196,7 @@ clutter_actor_set_child_transform_internal (ClutterActor *self, /* we need to reset the transform_valid flag on each child */ clutter_actor_iter_init (&iter, self); while (clutter_actor_iter_next (&iter, &child)) - child->priv->transform_valid = FALSE; + transform_changed (self); clutter_actor_queue_redraw (self); @@ -19739,3 +19746,20 @@ clutter_actor_queue_immediate_relayout (ClutterActor *self) if (stage) clutter_stage_set_actor_needs_immediate_relayout (stage); } + +/** + * clutter_actor_invalidate_transform: + * @self: A #ClutterActor + * + * Invalidate the cached transformation matrix of @self. + * This is needed for implementations overriding the apply_transform() + * vfunc and has to be called if the matrix returned by apply_transform() + * would change. + */ +void +clutter_actor_invalidate_transform (ClutterActor *self) +{ + g_return_if_fail (CLUTTER_IS_ACTOR (self)); + + transform_changed (self); +} diff --git a/clutter/clutter/clutter-actor.h b/clutter/clutter/clutter-actor.h index 67d327b5b46..128acac1a32 100644 --- a/clutter/clutter/clutter-actor.h +++ b/clutter/clutter/clutter-actor.h @@ -178,7 +178,10 @@ struct _ClutterActor * implementation. * @apply_transform: virtual function, used when applying the transformations * to an actor before painting it or when transforming coordinates or - * the allocation; it must chain up to the parent's implementation + * the allocation; if the transformation calculated by this function may + * have changed, the cached transformation must be invalidated by calling + * clutter_actor_invalidate_transform(); it must chain up to the parent's + * implementation * @parent_set: signal class handler for the #ClutterActor::parent-set * @destroy: signal class handler for #ClutterActor::destroy. It must * chain up to the parent's implementation @@ -918,6 +921,9 @@ void clutter_actor_pick_box (ClutterActor *self, CLUTTER_EXPORT GList * clutter_actor_peek_stage_views (ClutterActor *self); +CLUTTER_EXPORT +void clutter_actor_invalidate_transform (ClutterActor *self); + G_END_DECLS #endif /* __CLUTTER_ACTOR_H__ */ diff --git a/clutter/clutter/clutter-clone.c b/clutter/clutter/clutter-clone.c index fe34359d8ec..f130a3c98ec 100644 --- a/clutter/clutter/clutter-clone.c +++ b/clutter/clutter/clutter-clone.c @@ -262,6 +262,7 @@ clutter_clone_allocate (ClutterActor *self, { priv->x_scale = x_scale; priv->y_scale = y_scale; + clutter_actor_invalidate_transform (CLUTTER_ACTOR (self)); } #if 0 diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index 8d3fc8b8009..df3a4c3c339 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -2905,6 +2905,8 @@ clutter_stage_update_view_perspective (ClutterStage *stage) z_2d, priv->viewport[2], priv->viewport[3]); + + clutter_actor_invalidate_transform (CLUTTER_ACTOR (stage)); } void -- GitLab From 18a6f65b20e7f076c134424b583f6a36b4839079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Sat, 21 Mar 2020 17:28:52 +0100 Subject: [PATCH 2/5] clutter/actor: Cache absolute modelview and projection matrix Add caching of the complete modelview and projection matrix applied to an actor (the matrix that transforms to stage window coordinates). We need this matrix very often because it's used every time we want the absolute coordinates or vertices of an actor, and that happens almost twice on every paint cycle (sometimes for calculating the resource scale and always for transforming the paint volume). So start caching this matrix for every actor and introduce the necessary invalidation logic for that. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1299 --- clutter/clutter/clutter-actor-private.h | 2 + clutter/clutter/clutter-actor.c | 111 ++++++++++++++++++------ clutter/clutter/clutter-paint-volume.c | 21 +++-- clutter/clutter/clutter-stage.c | 5 ++ 4 files changed, 105 insertions(+), 34 deletions(-) diff --git a/clutter/clutter/clutter-actor-private.h b/clutter/clutter/clutter-actor-private.h index 16ab012b6f6..82fd1a9455f 100644 --- a/clutter/clutter/clutter-actor-private.h +++ b/clutter/clutter/clutter-actor-private.h @@ -293,6 +293,8 @@ void clutter_actor_update_stage_views (ClutterActor *self, void clutter_actor_queue_immediate_relayout (ClutterActor *self); +CoglMatrix * clutter_actor_get_absolute_modelview_projection (ClutterActor *self); + G_END_DECLS #endif /* __CLUTTER_ACTOR_PRIVATE_H__ */ diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index ddd16805933..58e86642703 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -704,6 +704,9 @@ struct _ClutterActorPrivate /* the cached transformation matrix; see apply_transform() */ CoglMatrix transform; + /* the cached matrix including modelview and projection up to the stage */ + CoglMatrix absolute_modelview_projection; + float resource_scale; guint8 opacity; @@ -856,6 +859,7 @@ struct _ClutterActorPrivate guint had_effects_on_last_paint_volume_update : 1; guint absolute_origin_changed : 1; guint needs_update_stage_views : 1; + guint absolute_modelview_projection_valid : 1; }; enum @@ -2525,6 +2529,7 @@ clutter_actor_notify_if_geometry_changed (ClutterActor *self, static void absolute_allocation_changed (ClutterActor *actor) { + actor->priv->absolute_modelview_projection_valid = FALSE; queue_update_stage_views (actor); } @@ -2538,10 +2543,26 @@ absolute_allocation_changed_cb (ClutterActor *actor, return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; } +static ClutterActorTraverseVisitFlags +invalidate_absolute_modelview_projection_cb (ClutterActor *actor, + int depth, + gpointer user_data) +{ + actor->priv->absolute_modelview_projection_valid = FALSE; + + return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; +} + static void transform_changed (ClutterActor *actor) { actor->priv->transform_valid = FALSE; + + _clutter_actor_traverse (actor, + CLUTTER_ACTOR_TRAVERSE_DEPTH_FIRST, + invalidate_absolute_modelview_projection_cb, + NULL, + NULL); } /*< private > @@ -2566,7 +2587,7 @@ clutter_actor_set_allocation_internal (ClutterActor *self, { ClutterActorPrivate *priv = self->priv; GObject *obj; - gboolean x1_changed, y1_changed, x2_changed, y2_changed; + gboolean origin_changed, size_changed; ClutterActorBox old_alloc = { 0, }; g_return_if_fail (!isnan (box->x1) && !isnan (box->x2) && @@ -2578,10 +2599,10 @@ clutter_actor_set_allocation_internal (ClutterActor *self, clutter_actor_store_old_geometry (self, &old_alloc); - x1_changed = priv->allocation.x1 != box->x1; - y1_changed = priv->allocation.y1 != box->y1; - x2_changed = priv->allocation.x2 != box->x2; - y2_changed = priv->allocation.y2 != box->y2; + origin_changed = (priv->allocation.x1 != box->x1 || + priv->allocation.y1 != box->y1); + size_changed = (priv->allocation.x2 != box->x2 || + priv->allocation.y2 != box->y2); priv->allocation = *box; @@ -2590,21 +2611,32 @@ clutter_actor_set_allocation_internal (ClutterActor *self, priv->needs_height_request = FALSE; priv->needs_allocation = FALSE; - priv->absolute_origin_changed |= x1_changed || y1_changed; + priv->absolute_origin_changed |= origin_changed; + + if (origin_changed || size_changed) + { + if (priv->absolute_origin_changed) + { + /* No need to invalidate the cached absolute modelview-projection + * matrices, the propagation of priv->absolute_origin_changed + * will take care of this. + */ + priv->transform_valid = FALSE; + } + else + { + transform_changed (self); + } + } - if (priv->absolute_origin_changed || x2_changed || y2_changed) + if (priv->absolute_origin_changed || size_changed) absolute_allocation_changed (self); - if (x1_changed || - y1_changed || - x2_changed || - y2_changed) + if (origin_changed || size_changed) { CLUTTER_NOTE (LAYOUT, "Allocation for '%s' changed", _clutter_actor_get_debug_name (self)); - transform_changed (self); - g_object_notify_by_pspec (obj, obj_props[PROP_ALLOCATION]); /* if the allocation changes, so does the content box */ @@ -2859,7 +2891,7 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, int n_vertices) { ClutterActor *stage; - CoglMatrix modelview; + CoglMatrix *modelview_projection; CoglMatrix projection; float viewport[4]; @@ -2872,20 +2904,17 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, if (stage == NULL) return FALSE; - /* Note: we pass NULL as the ancestor because we don't just want the modelview - * that gets us to stage coordinates, we want to go all the way to eye - * coordinates */ - _clutter_actor_get_relative_transformation_matrix (self, NULL, &modelview); + modelview_projection = clutter_actor_get_absolute_modelview_projection (self); /* Fetch the projection and viewport */ - _clutter_stage_get_projection_matrix (CLUTTER_STAGE (stage), &projection); + clutter_matrix_init_identity (&projection); _clutter_stage_get_viewport (CLUTTER_STAGE (stage), &viewport[0], &viewport[1], &viewport[2], &viewport[3]); - _clutter_util_fully_transform_vertices (&modelview, + _clutter_util_fully_transform_vertices (modelview_projection, &projection, viewport, vertices_in, @@ -2944,8 +2973,6 @@ clutter_actor_apply_transform_to_point (ClutterActor *self, * instead. * */ -/* XXX: We should consider caching the stage relative modelview along with - * the actor itself */ static void _clutter_actor_get_relative_transformation_matrix (ClutterActor *self, ClutterActor *ancestor, @@ -3192,6 +3219,41 @@ _clutter_actor_apply_relative_transformation_matrix (ClutterActor *self, _clutter_actor_apply_modelview_transform (self, matrix); } +CoglMatrix * +clutter_actor_get_absolute_modelview_projection (ClutterActor *self) +{ + ClutterActorPrivate *priv = self->priv; + + if (priv->absolute_modelview_projection_valid) + return &priv->absolute_modelview_projection; + + cogl_matrix_init_identity (&priv->absolute_modelview_projection); + + if (priv->parent == NULL) + { + /* No parents, this must be the stage... */ + ClutterStage *stage = CLUTTER_STAGE (self); + CoglMatrix projection; + + _clutter_stage_get_projection_matrix (stage, &projection); + cogl_matrix_multiply (&priv->absolute_modelview_projection, + &priv->absolute_modelview_projection, + &projection); + } + else + { + cogl_matrix_multiply (&priv->absolute_modelview_projection, + &priv->absolute_modelview_projection, + clutter_actor_get_absolute_modelview_projection (priv->parent)); + } + + _clutter_actor_apply_modelview_transform (self, + &priv->absolute_modelview_projection); + + priv->absolute_modelview_projection_valid = TRUE; + return &priv->absolute_modelview_projection; +} + static void _clutter_actor_draw_paint_volume_full (ClutterActor *self, ClutterPaintVolume *pv, @@ -3836,10 +3898,6 @@ clutter_actor_paint (ClutterActor *self, * box represents the location of the source actor on the * screen. * - * XXX: We are starting to do a lot of vertex transforms on - * the CPU in a typical paint, so at some point we should - * audit these and consider caching some things. - * * NB: We don't perform culling while picking at this point because * clutter-stage.c doesn't setup the clipping planes appropriately. * @@ -8021,6 +8079,7 @@ clutter_actor_init (ClutterActor *self) priv->last_paint_volume_valid = TRUE; priv->transform_valid = FALSE; + priv->absolute_modelview_projection_valid = FALSE; /* the default is to stretch the content, to match the * current behaviour of basically all actors. also, it's diff --git a/clutter/clutter/clutter-paint-volume.c b/clutter/clutter/clutter-paint-volume.c index 09edeb6f0cb..a462b869c33 100644 --- a/clutter/clutter/clutter-paint-volume.c +++ b/clutter/clutter/clutter-paint-volume.c @@ -1130,20 +1130,25 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, ClutterActorBox *box) { ClutterPaintVolume projected_pv; - CoglMatrix modelview; - CoglMatrix projection; + CoglMatrix *modelview_projection; + CoglMatrix stage_projection, projection; float viewport[4]; _clutter_paint_volume_copy_static (pv, &projected_pv); - cogl_matrix_init_identity (&modelview); - /* If the paint volume isn't already in eye coordinates... */ if (pv->actor) - _clutter_actor_apply_relative_transformation_matrix (pv->actor, NULL, - &modelview); + { + modelview_projection = + clutter_actor_get_absolute_modelview_projection (pv->actor); + } + else + { + _clutter_stage_get_projection_matrix (stage, &stage_projection); + modelview_projection = &stage_projection; + } - _clutter_stage_get_projection_matrix (stage, &projection); + cogl_matrix_init_identity (&projection); _clutter_stage_get_viewport (stage, &viewport[0], &viewport[1], @@ -1151,7 +1156,7 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, &viewport[3]); _clutter_paint_volume_project (&projected_pv, - &modelview, + modelview_projection, &projection, viewport); diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index df3a4c3c339..a7c33394274 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -2220,6 +2220,11 @@ _clutter_stage_dirty_projection (ClutterStage *stage) clutter_stage_view_invalidate_projection (view); } + + /* The above didn't really invalidate our cached transformation matrix + * but the cached absolute-transform matrices of all actors. + */ + clutter_actor_invalidate_transform (CLUTTER_ACTOR (stage)); } /* -- GitLab From 2e753b8eb7bdf1bcaa78f0c95b5c39400c34c34f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Fri, 22 May 2020 18:10:40 +0200 Subject: [PATCH 3/5] clutter/actor: Invalidate stage-views on transformation changes Since we now have the neccessary infrastructure to get notified about changes to the absolute transformation matrix, we can also invalidate the stage-views list on updates to this matrix. So simply rename absolute_allocation_changed() to absolute_position_or_size_changed() to make it clear this function is not only about allocations, and call that function every time the absolute position or size of an actor changes. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1299 --- clutter/clutter/clutter-actor.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 58e86642703..e6d9f958ce6 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2527,28 +2527,18 @@ clutter_actor_notify_if_geometry_changed (ClutterActor *self, } static void -absolute_allocation_changed (ClutterActor *actor) +absolute_position_or_size_changed (ClutterActor *actor) { actor->priv->absolute_modelview_projection_valid = FALSE; queue_update_stage_views (actor); } static ClutterActorTraverseVisitFlags -absolute_allocation_changed_cb (ClutterActor *actor, - int depth, - gpointer user_data) +absolute_position_or_size_changed_cb (ClutterActor *actor, + int depth, + gpointer user_data) { - absolute_allocation_changed (actor); - - return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; -} - -static ClutterActorTraverseVisitFlags -invalidate_absolute_modelview_projection_cb (ClutterActor *actor, - int depth, - gpointer user_data) -{ - actor->priv->absolute_modelview_projection_valid = FALSE; + absolute_position_or_size_changed (actor); return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; } @@ -2560,7 +2550,7 @@ transform_changed (ClutterActor *actor) _clutter_actor_traverse (actor, CLUTTER_ACTOR_TRAVERSE_DEPTH_FIRST, - invalidate_absolute_modelview_projection_cb, + absolute_position_or_size_changed_cb, NULL, NULL); } @@ -2630,7 +2620,7 @@ clutter_actor_set_allocation_internal (ClutterActor *self, } if (priv->absolute_origin_changed || size_changed) - absolute_allocation_changed (self); + absolute_position_or_size_changed (self); if (origin_changed || size_changed) { @@ -9574,7 +9564,7 @@ clutter_actor_allocate (ClutterActor *self, { _clutter_actor_traverse (self, CLUTTER_ACTOR_TRAVERSE_DEPTH_FIRST, - absolute_allocation_changed_cb, + absolute_position_or_size_changed_cb, NULL, NULL); } @@ -9632,7 +9622,7 @@ clutter_actor_allocate (ClutterActor *self, { _clutter_actor_traverse (self, CLUTTER_ACTOR_TRAVERSE_DEPTH_FIRST, - absolute_allocation_changed_cb, + absolute_position_or_size_changed_cb, NULL, NULL); } -- GitLab From bf732bda2060915804196474862739847a59733e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Sat, 21 Mar 2020 17:35:49 +0100 Subject: [PATCH 4/5] clutter: Use cached matrix for _clutter_util_fully_transform_vertices Now that we cache the full matrix including modelview and projection of the stage for every actor, we can remove the projection matrix argument from _clutter_util_fully_transform_vertices() and simply pass it the cached modelview-projection matrix. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1299 --- clutter/clutter/clutter-actor.c | 3 -- clutter/clutter/clutter-offscreen-effect.c | 6 +-- .../clutter/clutter-paint-volume-private.h | 3 +- clutter/clutter/clutter-paint-volume.c | 13 ++---- clutter/clutter/clutter-private.h | 3 +- clutter/clutter/clutter-util.c | 43 ++++--------------- 6 files changed, 17 insertions(+), 54 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index e6d9f958ce6..8a12e4ed9e4 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2882,7 +2882,6 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, { ClutterActor *stage; CoglMatrix *modelview_projection; - CoglMatrix projection; float viewport[4]; g_return_val_if_fail (CLUTTER_IS_ACTOR (self), FALSE); @@ -2897,7 +2896,6 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, modelview_projection = clutter_actor_get_absolute_modelview_projection (self); /* Fetch the projection and viewport */ - clutter_matrix_init_identity (&projection); _clutter_stage_get_viewport (CLUTTER_STAGE (stage), &viewport[0], &viewport[1], @@ -2905,7 +2903,6 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, &viewport[3]); _clutter_util_fully_transform_vertices (modelview_projection, - &projection, viewport, vertices_in, vertices_out, diff --git a/clutter/clutter/clutter-offscreen-effect.c b/clutter/clutter/clutter-offscreen-effect.c index 75174ef1042..f679182074d 100644 --- a/clutter/clutter/clutter-offscreen-effect.c +++ b/clutter/clutter/clutter-offscreen-effect.c @@ -232,7 +232,7 @@ clutter_offscreen_effect_pre_paint (ClutterEffect *effect, ClutterOffscreenEffectPrivate *priv = self->priv; ClutterActorBox raw_box, box; ClutterActor *stage; - CoglMatrix projection, old_modelview, modelview; + CoglMatrix projection, old_modelview, modelview, modelview_projection; const ClutterPaintVolume *volume; CoglColor transparent; gfloat stage_width, stage_height; @@ -335,8 +335,8 @@ clutter_offscreen_effect_pre_paint (ClutterEffect *effect, * It doesn't appear anyone actually uses this yet, but get_target_rect is * documented as returning it. So we should... */ - _clutter_util_fully_transform_vertices (&old_modelview, - &projection, + cogl_matrix_multiply (&modelview_projection, &projection, &old_modelview); + _clutter_util_fully_transform_vertices (&modelview_projection, old_viewport, &local_offset, &priv->position, diff --git a/clutter/clutter/clutter-paint-volume-private.h b/clutter/clutter/clutter-paint-volume-private.h index 21e35c20207..050c403de8c 100644 --- a/clutter/clutter/clutter-paint-volume-private.h +++ b/clutter/clutter/clutter-paint-volume-private.h @@ -114,8 +114,7 @@ void _clutter_paint_volume_complete (ClutterPaintVolu void _clutter_paint_volume_transform (ClutterPaintVolume *pv, const CoglMatrix *matrix); void _clutter_paint_volume_project (ClutterPaintVolume *pv, - const CoglMatrix *modelview, - const CoglMatrix *projection, + const CoglMatrix *modelview_projection, const float *viewport); void _clutter_paint_volume_get_bounding_box (ClutterPaintVolume *pv, ClutterActorBox *box); diff --git a/clutter/clutter/clutter-paint-volume.c b/clutter/clutter/clutter-paint-volume.c index a462b869c33..581da1abc03 100644 --- a/clutter/clutter/clutter-paint-volume.c +++ b/clutter/clutter/clutter-paint-volume.c @@ -808,8 +808,7 @@ _clutter_paint_volume_get_bounding_box (ClutterPaintVolume *pv, void _clutter_paint_volume_project (ClutterPaintVolume *pv, - const CoglMatrix *modelview, - const CoglMatrix *projection, + const CoglMatrix *modelview_projection, const float *viewport) { int transform_count; @@ -817,8 +816,7 @@ _clutter_paint_volume_project (ClutterPaintVolume *pv, if (pv->is_empty) { /* Just transform the origin... */ - _clutter_util_fully_transform_vertices (modelview, - projection, + _clutter_util_fully_transform_vertices (modelview_projection, viewport, pv->vertices, pv->vertices, @@ -837,8 +835,7 @@ _clutter_paint_volume_project (ClutterPaintVolume *pv, else transform_count = 8; - _clutter_util_fully_transform_vertices (modelview, - projection, + _clutter_util_fully_transform_vertices (modelview_projection, viewport, pv->vertices, pv->vertices, @@ -1131,7 +1128,7 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, { ClutterPaintVolume projected_pv; CoglMatrix *modelview_projection; - CoglMatrix stage_projection, projection; + CoglMatrix stage_projection; float viewport[4]; _clutter_paint_volume_copy_static (pv, &projected_pv); @@ -1148,7 +1145,6 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, modelview_projection = &stage_projection; } - cogl_matrix_init_identity (&projection); _clutter_stage_get_viewport (stage, &viewport[0], &viewport[1], @@ -1157,7 +1153,6 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, _clutter_paint_volume_project (&projected_pv, modelview_projection, - &projection, viewport); _clutter_paint_volume_get_bounding_box (&projected_pv, box); diff --git a/clutter/clutter/clutter-private.h b/clutter/clutter/clutter-private.h index 196725fd950..901522d55f4 100644 --- a/clutter/clutter/clutter-private.h +++ b/clutter/clutter/clutter-private.h @@ -220,8 +220,7 @@ void _clutter_run_repaint_functions (ClutterRepaintFlags flags); GType _clutter_layout_manager_get_child_meta_type (ClutterLayoutManager *manager); -void _clutter_util_fully_transform_vertices (const CoglMatrix *modelview, - const CoglMatrix *projection, +void _clutter_util_fully_transform_vertices (const CoglMatrix *modelview_projection, const float *viewport, const graphene_point3d_t *vertices_in, graphene_point3d_t *vertices_out, diff --git a/clutter/clutter/clutter-util.c b/clutter/clutter/clutter-util.c index 6e59889a8c6..772163b5427 100644 --- a/clutter/clutter/clutter-util.c +++ b/clutter/clutter/clutter-util.c @@ -48,51 +48,24 @@ #define MTX_GL_SCALE_Z(z,w,v1,v2) (MTX_GL_SCALE_X ((z), (w), (v1), (v2))) void -_clutter_util_fully_transform_vertices (const CoglMatrix *modelview, - const CoglMatrix *projection, +_clutter_util_fully_transform_vertices (const CoglMatrix *modelview_projection, const float *viewport, const graphene_point3d_t *vertices_in, graphene_point3d_t *vertices_out, int n_vertices) { - CoglMatrix modelview_projection; ClutterVertex4 *vertices_tmp; int i; vertices_tmp = g_alloca (sizeof (ClutterVertex4) * n_vertices); - if (n_vertices >= 4) - { - /* XXX: we should find a way to cache this per actor */ - cogl_matrix_multiply (&modelview_projection, - projection, - modelview); - cogl_matrix_project_points (&modelview_projection, - 3, - sizeof (graphene_point3d_t), - vertices_in, - sizeof (ClutterVertex4), - vertices_tmp, - n_vertices); - } - else - { - cogl_matrix_transform_points (modelview, - 3, - sizeof (graphene_point3d_t), - vertices_in, - sizeof (ClutterVertex4), - vertices_tmp, - n_vertices); - - cogl_matrix_project_points (projection, - 3, - sizeof (ClutterVertex4), - vertices_tmp, - sizeof (ClutterVertex4), - vertices_tmp, - n_vertices); - } + cogl_matrix_project_points (modelview_projection, + 3, + sizeof (graphene_point3d_t), + vertices_in, + sizeof (ClutterVertex4), + vertices_tmp, + n_vertices); for (i = 0; i < n_vertices; i++) { -- GitLab From 074b8c7cd536a7a32dddbf40ca0bf51769736a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Sat, 21 Mar 2020 17:53:31 +0100 Subject: [PATCH 5/5] clutter/stage: Peek viewport instead of copying it Copying the same viewport around all the time for every step when calculating the transformation matrices of actors is quite a lot of unnecessary work. We can avoid this work by peeking instead of copying the viewport of the stage, and only copying it later if necessary. So rename _clutter_stage_get_viewport() to clutter_stage_peek_viewport() and simply return a pointer to the float array there. Also, since this is private API only used inside Clutter, we can avoid the type check here and assume the passed ClutterStage is valid. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1299 --- clutter/clutter/clutter-actor.c | 10 ++----- clutter/clutter/clutter-offscreen-effect.c | 11 +++---- clutter/clutter/clutter-paint-volume.c | 8 ++--- clutter/clutter/clutter-stage-private.h | 6 +--- clutter/clutter/clutter-stage.c | 35 +++++----------------- 5 files changed, 19 insertions(+), 51 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 8a12e4ed9e4..a32bc33fe0f 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2882,7 +2882,7 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, { ClutterActor *stage; CoglMatrix *modelview_projection; - float viewport[4]; + const float *viewport; g_return_val_if_fail (CLUTTER_IS_ACTOR (self), FALSE); @@ -2894,13 +2894,7 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self, return FALSE; modelview_projection = clutter_actor_get_absolute_modelview_projection (self); - - /* Fetch the projection and viewport */ - _clutter_stage_get_viewport (CLUTTER_STAGE (stage), - &viewport[0], - &viewport[1], - &viewport[2], - &viewport[3]); + viewport = clutter_stage_peek_viewport (CLUTTER_STAGE (stage)); _clutter_util_fully_transform_vertices (modelview_projection, viewport, diff --git a/clutter/clutter/clutter-offscreen-effect.c b/clutter/clutter/clutter-offscreen-effect.c index f679182074d..a8ec71c4834 100644 --- a/clutter/clutter/clutter-offscreen-effect.c +++ b/clutter/clutter/clutter-offscreen-effect.c @@ -241,6 +241,7 @@ clutter_offscreen_effect_pre_paint (ClutterEffect *effect, float resource_scale; float ceiled_resource_scale; graphene_point3d_t local_offset; + const float *viewport; gfloat old_viewport[4]; local_offset = GRAPHENE_POINT3D_INIT (0.0f, 0.0f, 0.0f); @@ -312,11 +313,11 @@ clutter_offscreen_effect_pre_paint (ClutterEffect *effect, cogl_framebuffer_set_modelview_matrix (priv->offscreen, &modelview); /* Save the original viewport for calculating priv->position */ - _clutter_stage_get_viewport (CLUTTER_STAGE (priv->stage), - &old_viewport[0], - &old_viewport[1], - &old_viewport[2], - &old_viewport[3]); + viewport = clutter_stage_peek_viewport (CLUTTER_STAGE (priv->stage)); + old_viewport[0] = viewport[0]; + old_viewport[1] = viewport[1]; + old_viewport[2] = viewport[2]; + old_viewport[3] = viewport[3]; /* Set up the viewport so that it has the same size as the stage (avoid * distortion), but translated to account for the FBO offset... diff --git a/clutter/clutter/clutter-paint-volume.c b/clutter/clutter/clutter-paint-volume.c index 581da1abc03..6cfed02e259 100644 --- a/clutter/clutter/clutter-paint-volume.c +++ b/clutter/clutter/clutter-paint-volume.c @@ -1129,7 +1129,7 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, ClutterPaintVolume projected_pv; CoglMatrix *modelview_projection; CoglMatrix stage_projection; - float viewport[4]; + const float *viewport; _clutter_paint_volume_copy_static (pv, &projected_pv); @@ -1145,11 +1145,7 @@ _clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv, modelview_projection = &stage_projection; } - _clutter_stage_get_viewport (stage, - &viewport[0], - &viewport[1], - &viewport[2], - &viewport[3]); + viewport = clutter_stage_peek_viewport (stage); _clutter_paint_volume_project (&projected_pv, modelview_projection, diff --git a/clutter/clutter/clutter-stage-private.h b/clutter/clutter/clutter-stage-private.h index 2ce0cd05b59..70eeae32bf4 100644 --- a/clutter/clutter/clutter-stage-private.h +++ b/clutter/clutter/clutter-stage-private.h @@ -50,11 +50,7 @@ ClutterStageWindow *_clutter_stage_get_window (ClutterStage void _clutter_stage_get_projection_matrix (ClutterStage *stage, CoglMatrix *projection); void _clutter_stage_dirty_projection (ClutterStage *stage); -void _clutter_stage_get_viewport (ClutterStage *stage, - float *x, - float *y, - float *width, - float *height); +const float * clutter_stage_peek_viewport (ClutterStage *stage); void _clutter_stage_dirty_viewport (ClutterStage *stage); void _clutter_stage_maybe_setup_viewport (ClutterStage *stage, ClutterStageView *view); diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index a7c33394274..7ded0928d3e 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -2319,41 +2319,22 @@ _clutter_stage_dirty_viewport (ClutterStage *stage) } /* - * clutter_stage_get_viewport: + * clutter_stage_peek_viewport: (skip) * @stage: A #ClutterStage - * @x: A location for the X position where the stage is rendered, - * in window coordinates. - * @y: A location for the Y position where the stage is rendered, - * in window coordinates. - * @width: A location for the width the stage is rendered at, - * in window coordinates. - * @height: A location for the height the stage is rendered at, - * in window coordinates. * * Returns the viewport offset and size set using * clutter_stage_set_viewport() or if the "viewport-mimics-window" property - * is TRUE then @x and @y will be set to 0 and @width and @height will equal + * is TRUE then x and y will be set to 0 and width and height will equal * the width if the stage window. * - * Since: 1.6 + * Returns: A pointer to a float array with length 4, including the + * x and y coordinates and the width and height of the viewport (in + * that order). */ -void -_clutter_stage_get_viewport (ClutterStage *stage, - float *x, - float *y, - float *width, - float *height) +const float * +clutter_stage_peek_viewport (ClutterStage *stage) { - ClutterStagePrivate *priv; - - g_return_if_fail (CLUTTER_IS_STAGE (stage)); - - priv = stage->priv; - - *x = priv->viewport[0]; - *y = priv->viewport[1]; - *width = priv->viewport[2]; - *height = priv->viewport[3]; + return stage->priv->viewport; } /** -- GitLab