[PATCH weston v2 3/3] compositor, shell: surface transform inheritance

Kristian Høgsberg hoegsberg at gmail.com
Mon Mar 18 19:25:23 PDT 2013


On Fri, Mar 08, 2013 at 02:56:50PM +0200, Pekka Paalanen wrote:
> Implements surface transform inheritance. A 'parent' pointer is added to
> weston_surface::geometry, and is automatically used by
> weston_surface_update_transform(). When updating the transform, the
> parent transform is updated as needed, too.
> 
> shell_map_popup() is converted to use the new
> weston_surface_set_transform_parent() function. Now, if we moved the
> popup's parent surface while the popup is open, the popup surface will
> stick to the parent properly.
> 
> Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> 
> ---
> 
> v2:
> 
> Implement proper dirtying behaviour on top of
> weston_surface_geometry_dirty() patch. Need to mark all child surfaces
> dirty, too.
> 
> Introduce the surface transform tree invariant, which helps to optimize
> dirtying and update_transform.

Sounds good, committed.  I like how all the complexity from
shell_map_popup() now is part of the core surface transformation code.

Kristian

> ---
>  src/compositor.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/compositor.h | 15 +++++++++++++
>  src/shell.c      | 28 +----------------------
>  3 files changed, 82 insertions(+), 28 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 125276e..d47e078 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -303,6 +303,7 @@ weston_surface_create(struct weston_compositor *compositor)
>  	wl_list_insert(&surface->geometry.transformation_list,
>  		       &surface->transform.position.link);
>  	weston_matrix_init(&surface->transform.position.matrix);
> +	wl_list_init(&surface->geometry.child_list);
>  	pixman_region32_init(&surface->transform.boundingbox);
>  	surface->transform.dirty = 1;
>  
> @@ -625,6 +626,7 @@ weston_surface_update_transform_disable(struct weston_surface *surface)
>  static int
>  weston_surface_update_transform_enable(struct weston_surface *surface)
>  {
> +	struct weston_surface *parent = surface->geometry.parent;
>  	struct weston_matrix *matrix = &surface->transform.matrix;
>  	struct weston_matrix *inverse = &surface->transform.inverse;
>  	struct weston_transform *tform;
> @@ -640,6 +642,9 @@ weston_surface_update_transform_enable(struct weston_surface *surface)
>  	wl_list_for_each(tform, &surface->geometry.transformation_list, link)
>  		weston_matrix_multiply(matrix, &tform->matrix);
>  
> +	if (parent)
> +		weston_matrix_multiply(matrix, &parent->transform.matrix);
> +
>  	if (weston_matrix_invert(inverse, matrix) < 0) {
>  		/* Oops, bad total transformation, not invertible */
>  		weston_log("error: weston_surface %p"
> @@ -657,9 +662,14 @@ weston_surface_update_transform_enable(struct weston_surface *surface)
>  WL_EXPORT void
>  weston_surface_update_transform(struct weston_surface *surface)
>  {
> +	struct weston_surface *parent = surface->geometry.parent;
> +
>  	if (!surface->transform.dirty)
>  		return;
>  
> +	if (parent)
> +		weston_surface_update_transform(parent);
> +
>  	surface->transform.dirty = 0;
>  
>  	weston_surface_damage_below(surface);
> @@ -672,7 +682,8 @@ weston_surface_update_transform(struct weston_surface *surface)
>  	if (surface->geometry.transformation_list.next ==
>  	    &surface->transform.position.link &&
>  	    surface->geometry.transformation_list.prev ==
> -	    &surface->transform.position.link) {
> +	    &surface->transform.position.link &&
> +	    !parent) {
>  		weston_surface_update_transform_disable(surface);
>  	} else {
>  		if (weston_surface_update_transform_enable(surface) < 0)
> @@ -687,7 +698,23 @@ weston_surface_update_transform(struct weston_surface *surface)
>  WL_EXPORT void
>  weston_surface_geometry_dirty(struct weston_surface *surface)
>  {
> +	struct weston_surface *child;
> +
> +	/*
> +	 * The invariant: if surface->geometry.dirty, then all surfaces
> +	 * in surface->geometry.child_list have geometry.dirty too.
> +	 * Corollary: if not parent->geometry.dirty, then all ancestors
> +	 * are not dirty.
> +	 */
> +
> +	if (surface->transform.dirty)
> +		return;
> +
>  	surface->transform.dirty = 1;
> +
> +	wl_list_for_each(child, &surface->geometry.child_list,
> +			 geometry.parent_link)
> +		weston_surface_geometry_dirty(child);
>  }
>  
>  WL_EXPORT void
> @@ -797,6 +824,40 @@ weston_surface_set_position(struct weston_surface *surface,
>  	weston_surface_geometry_dirty(surface);
>  }
>  
> +static void
> +transform_parent_handle_parent_destroy(struct wl_listener *listener,
> +				       void *data)
> +{
> +	struct weston_surface *surface =
> +		container_of(listener, struct weston_surface,
> +			     geometry.parent_destroy_listener);
> +
> +	weston_surface_set_transform_parent(surface, NULL);
> +}
> +
> +WL_EXPORT void
> +weston_surface_set_transform_parent(struct weston_surface *surface,
> +				    struct weston_surface *parent)
> +{
> +	if (surface->geometry.parent) {
> +		wl_list_remove(&surface->geometry.parent_destroy_listener.link);
> +		wl_list_remove(&surface->geometry.parent_link);
> +	}
> +
> +	surface->geometry.parent = parent;
> +
> +	surface->geometry.parent_destroy_listener.notify =
> +		transform_parent_handle_parent_destroy;
> +	if (parent) {
> +		wl_signal_add(&parent->surface.resource.destroy_signal,
> +			      &surface->geometry.parent_destroy_listener);
> +		wl_list_insert(&parent->geometry.child_list,
> +			       &surface->geometry.parent_link);
> +	}
> +
> +	weston_surface_geometry_dirty(surface);
> +}
> +
>  WL_EXPORT int
>  weston_surface_is_mapped(struct weston_surface *surface)
>  {
> @@ -946,6 +1007,8 @@ destroy_surface(struct wl_resource *resource)
>  	struct weston_compositor *compositor = surface->compositor;
>  	struct weston_frame_callback *cb, *next;
>  
> +	assert(wl_list_empty(&surface->geometry.child_list));
> +
>  	if (weston_surface_is_mapped(surface))
>  		weston_surface_unmap(surface);
>  
> @@ -973,6 +1036,8 @@ destroy_surface(struct wl_resource *resource)
>  	wl_list_for_each_safe(cb, next, &surface->frame_callback_list, link)
>  		wl_resource_destroy(&cb->resource);
>  
> +	weston_surface_set_transform_parent(surface, NULL);
> +
>  	free(surface);
>  }
>  
> diff --git a/src/compositor.h b/src/compositor.h
> index bdd4874..e74b8e9 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -386,6 +386,11 @@ struct weston_region {
>   * If you want to apply a transformation in local coordinates, add your
>   * weston_transform to the head of the list. If you want to apply a
>   * transformation in global coordinates, add it to the tail of the list.
> + *
> + * If surface->geometry.parent is set, the total transformation of this
> + * surface will be the parent's total transformation and this transformation
> + * combined:
> + *    Mparent * Mn * ... * M2 * M1
>   */
>  
>  struct weston_surface {
> @@ -412,6 +417,12 @@ struct weston_surface {
>  
>  		/* struct weston_transform */
>  		struct wl_list transformation_list;
> +
> +		/* managed by weston_surface_set_transform_parent() */
> +		struct weston_surface *parent;
> +		struct wl_listener parent_destroy_listener;
> +		struct wl_list child_list; /* geometry.parent_link */
> +		struct wl_list parent_link;
>  	} geometry;
>  
>  	/* State derived from geometry state, read-only.
> @@ -682,6 +693,10 @@ void
>  weston_surface_set_position(struct weston_surface *surface,
>  			    float x, float y);
>  
> +void
> +weston_surface_set_transform_parent(struct weston_surface *surface,
> +				    struct weston_surface *parent);
> +
>  int
>  weston_surface_is_mapped(struct weston_surface *surface);
>  
> diff --git a/src/shell.c b/src/shell.c
> index 61a5be8..9ba0143 100644
> --- a/src/shell.c
> +++ b/src/shell.c
> @@ -190,7 +190,6 @@ struct shell_surface {
>  	struct {
>  		struct wl_pointer_grab grab;
>  		int32_t x, y;
> -		struct weston_transform parent_transform;
>  		int32_t initial_up;
>  		struct wl_seat *seat;
>  		uint32_t serial;
> @@ -1909,35 +1908,11 @@ shell_map_popup(struct shell_surface *shsurf)
>  	struct weston_surface *es = shsurf->surface;
>  	struct weston_surface *parent = shsurf->parent;
>  
> -	/* Remove the old transform. We don't want to add it twice
> -	 * otherwise Weston will go into an infinite loop when going
> -	 * through the transforms. */
> -	if (!wl_list_empty(&shsurf->popup.parent_transform.link)) {
> -		wl_list_remove(&shsurf->popup.parent_transform.link);
> -		wl_list_init(&shsurf->popup.parent_transform.link);
> -	}
> -
>  	es->output = parent->output;
>  	shsurf->popup.grab.interface = &popup_grab_interface;
>  
> -	weston_surface_update_transform(parent);
> -	if (parent->transform.enabled) {
> -		shsurf->popup.parent_transform.matrix =
> -			parent->transform.matrix;
> -	} else {
> -		/* construct x, y translation matrix */
> -		weston_matrix_init(&shsurf->popup.parent_transform.matrix);
> -		shsurf->popup.parent_transform.matrix.type =
> -			WESTON_MATRIX_TRANSFORM_TRANSLATE;
> -		shsurf->popup.parent_transform.matrix.d[12] =
> -			parent->geometry.x;
> -		shsurf->popup.parent_transform.matrix.d[13] =
> -			parent->geometry.y;
> -	}
> -	wl_list_insert(es->geometry.transformation_list.prev,
> -		       &shsurf->popup.parent_transform.link);
> -
>  	shsurf->popup.initial_up = 0;
> +	weston_surface_set_transform_parent(es, parent);
>  	weston_surface_set_position(es, shsurf->popup.x, shsurf->popup.y);
>  	weston_surface_update_transform(es);
>  
> @@ -2087,7 +2062,6 @@ create_shell_surface(void *shell, struct weston_surface *surface,
>  	weston_matrix_init(&shsurf->rotation.rotation);
>  
>  	wl_list_init(&shsurf->workspace_transform.link);
> -	wl_list_init(&shsurf->popup.parent_transform.link);
>  
>  	shsurf->type = SHELL_SURFACE_NONE;
>  	shsurf->next_type = SHELL_SURFACE_NONE;
> -- 
> 1.7.12.4
> 


More information about the wayland-devel mailing list