[PATCH weston 2/4] compositor: add weston_view_set_mask() API and state

Derek Foreman derekf at osg.samsung.com
Tue Mar 3 14:50:47 PST 2015


Minor pedantry below.

On 02/03/15 09:15 AM, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> Add API for setting a clip ('scissor' in the code) rectangle per view,
> in surface coordinates. Ivi-shell requires this feature to be able to
> implement the IVI Layer Manager API, which includes clipping of
> surfaces.
> 
> The names weston_view_set_mask() and weston_view_set_mask_infinite()
> mirror the existing weston_layer_set_mask*() functions.
> 
> This view clipping complements the weston_layer clipping, because view
> clipping is defined in surface local coordinates, while layer
> mask/clipping is defined in global coordinates.
> 
> View clipping requires explicit support from the renderers. Therefore a
> new Weston capability bit is added: WESTON_CAP_VIEW_CLIP_MASK. Shells
> (and all users) of this new API are required to check the capability bit
> is set before using the API. Otherwise the rendering will not be what
> they expect.
> 
> View clips are inherited through the transformation inheritance
> mechanism. However, there are restrictions. The clip rectangle can be
> set only on the root view of a transformation inheritance tree. The
> additional transformations in child views must not rotate the coordinate
> axes. These restrictions avoid corner cases in clip inheritance, and
> keep the renderer implementations as simple as they are right now.
> Renderers only need to do an additional intersection with the clip
> rectangle which is always aligned to the surface coordinate system.
> 
> For more details, see the API documentation in the patch.
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Reviewed-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
> Tested-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
> ---
>  src/compositor.c | 164 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  src/compositor.h |  14 +++++
>  2 files changed, 169 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 269d485..e435577 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -452,6 +452,7 @@ weston_view_create(struct weston_surface *surface)
>  		       &view->transform.position.link);
>  	weston_matrix_init(&view->transform.position.matrix);
>  	wl_list_init(&view->geometry.child_list);
> +	pixman_region32_init(&view->geometry.scissor);
>  	pixman_region32_init(&view->transform.boundingbox);
>  	view->transform.dirty = 1;
>  
> @@ -1078,6 +1079,34 @@ weston_view_assign_output(struct weston_view *ev)
>  }
>  
>  static void
> +weston_view_to_view_map(struct weston_view *from, struct weston_view *to,
> +			int from_x, int from_y, int *to_x, int *to_y)
> +{
> +	float x, y;
> +
> +	weston_view_to_global_float(from, from_x, from_y, &x, &y);
> +	weston_view_from_global_float(to, x, y, &x, &y);
> +
> +	*to_x = round(x);
> +	*to_y = round(y);
> +}
> +
> +static void
> +weston_view_transfer_scissor(struct weston_view *from, struct weston_view *to)
> +{
> +	pixman_box32_t *a;
> +	pixman_box32_t b;
> +
> +	a = pixman_region32_extents(&from->geometry.scissor);
> +
> +	weston_view_to_view_map(from, to, a->x1, a->y1, &b.x1, &b.y1);
> +	weston_view_to_view_map(from, to, a->x2, a->y2, &b.x2, &b.y2);
> +
> +	pixman_region32_fini(&to->geometry.scissor);
> +	pixman_region32_init_with_extents(&to->geometry.scissor, &b);
> +}
> +
> +static void
>  view_compute_bbox(struct weston_view *view, const pixman_box32_t *inbox,
>  		  pixman_region32_t *bbox)
>  {
> @@ -1138,10 +1167,16 @@ weston_view_update_transform_disable(struct weston_view *view)
>  	view->transform.inverse.d[13] = -view->geometry.y;
>  
>  	pixman_region32_init_rect(&view->transform.boundingbox,
> -				  view->geometry.x,
> -				  view->geometry.y,
> +				  0, 0,
>  				  view->surface->width,
>  				  view->surface->height);
> +	if (view->geometry.scissor_enabled)
> +		pixman_region32_intersect(&view->transform.boundingbox,
> +					  &view->transform.boundingbox,
> +					  &view->geometry.scissor);
> +
> +	pixman_region32_translate(&view->transform.boundingbox,
> +				  view->geometry.x, view->geometry.y);
>  
>  	if (view->alpha == 1.0) {
>  		pixman_region32_copy(&view->transform.opaque,
> @@ -1159,7 +1194,8 @@ weston_view_update_transform_enable(struct weston_view *view)
>  	struct weston_matrix *matrix = &view->transform.matrix;
>  	struct weston_matrix *inverse = &view->transform.inverse;
>  	struct weston_transform *tform;
> -	pixman_box32_t surfbox;
> +	pixman_region32_t surfregion;
> +	const pixman_box32_t *surfbox;
>  
>  	view->transform.enabled = 1;
>  
> @@ -1182,11 +1218,15 @@ weston_view_update_transform_enable(struct weston_view *view)
>  		return -1;
>  	}
>  
> -	surfbox.x1 = 0;
> -	surfbox.y1 = 0;
> -	surfbox.x2 = view->surface->width;
> -	surfbox.y2 = view->surface->height;
> -	view_compute_bbox(view, &surfbox, &view->transform.boundingbox);
> +	pixman_region32_init_rect(&surfregion, 0, 0,
> +				  view->surface->width, view->surface->height);
> +	if (view->geometry.scissor_enabled)
> +		pixman_region32_intersect(&surfregion, &surfregion,
> +					  &view->geometry.scissor);
> +	surfbox = pixman_region32_extents(&surfregion);
> +
> +	view_compute_bbox(view, surfbox, &view->transform.boundingbox);
> +	pixman_region32_fini(&surfregion);
>  
>  	return 0;
>  }
> @@ -1242,6 +1282,15 @@ weston_view_update_transform(struct weston_view *view)
>  		pixman_region32_fini(&mask);
>  	}
>  
> +	if (parent) {
> +		if (parent->geometry.scissor_enabled) {
> +			view->geometry.scissor_enabled = true;
> +			weston_view_transfer_scissor(parent, view);
> +		} else {
> +			view->geometry.scissor_enabled = false;
> +		}
> +	}
> +
>  	weston_view_damage_below(view);
>  
>  	weston_view_assign_output(view);
> @@ -1403,11 +1452,14 @@ transform_parent_handle_parent_destroy(struct wl_listener *listener,
>  
>  WL_EXPORT void
>  weston_view_set_transform_parent(struct weston_view *view,
> -				    struct weston_view *parent)
> +				 struct weston_view *parent)
>  {
>  	if (view->geometry.parent) {
>  		wl_list_remove(&view->geometry.parent_destroy_listener.link);
>  		wl_list_remove(&view->geometry.parent_link);
> +
> +		if (!parent)
> +			view->geometry.scissor_enabled = false;
>  	}
>  
>  	view->geometry.parent = parent;
> @@ -1424,6 +1476,92 @@ weston_view_set_transform_parent(struct weston_view *view,
>  	weston_view_geometry_dirty(view);
>  }
>  
> +/** Set a clip mask rectangle on a view
> + *
> + * \param view The view to set the clip mask on.
> + * \param x Top-left corner X coordinate of the clip rectangle.
> + * \param y Top-left corner X coordinate of the clip rectangle.

Should say Y coordinate...

> + * \param width Width of the clip rectangle, non-negative.
> + * \param height Height of the clip rectangle, non-negative.
> + *
> + * A shell may set a clip mask rectangle on a view. Everything outside
> + * the rectangle is cut away for input and output purposes: it is
> + * not drawn and cannot be hit by hit-test based input like pointer
> + * motion or touch-downs. Everything inside the rectangle will behave
> + * normal. Clients are unaware of clipping.

s/normally/normally/

> + *
> + * The rectangle is set in the surface local coordinates. Setting a clip
> + * mask rectangle does not affect the view position, the view is positioned
> + * as it would be without a clip. The clip also does not change
> + * weston_surface::width,height.
> + *
> + * The clip mask rectangle is part of transformation inheritance
> + * (weston_view_set_transform_parent()). A clip set in the root of the
> + * transformation inheritance tree will affect all views in the tree.
> + * A clip can be set only on the root view. Attempting to set a clip
> + * on view that has a transformation parent will fail. Assigning a parent
> + * to a view that has a clip set will cause the clip to be forgotten.
> + *
> + * Because the clip mask is an axis-aligned rectangle, it poses restrictions
> + * on the additional transformations in the child views. These transformations
> + * may not rotate the coordinate axes, i.e., only translation and scaling
> + * are allowed. Violating this restriction causes the clipping to malfcuntion.

malfunction

> + * Furthermore, using scaling may cause rounding errors in child clipping.
> + *
> + * The clip mask rectangle is not automatically adjusted based on
> + * wl_surface.attach dx and dy arguments.
> + *
> + * A clip mask rectangle can be set only if the compositor capability
> + * WESTON_CAP_VIEW_CLIP_MASK is present.
> + *
> + * This function sets the clip mask rectangle and schedules a repaint for
> + * the view.
> + */
> +WL_EXPORT void
> +weston_view_set_mask(struct weston_view *view,
> +		     int x, int y, int width, int height)
> +{
> +	struct weston_compositor *compositor = view->surface->compositor;
> +
> +	if (!(compositor->capabilities & WESTON_CAP_VIEW_CLIP_MASK)) {
> +		weston_log("%s not allowed without capability!\n", __func__);
> +		return;
> +	}
> +
> +	if (view->geometry.parent) {
> +		weston_log("view %p has a parent, clip forbidden!\n", view);
> +		return;
> +	}
> +
> +	if (width < 0 || height < 0) {
> +		weston_log("%s: illegal args %d, %d, %d, %d\n", __func__,
> +			   x, y, width, height);
> +		return;
> +	}
> +
> +	pixman_region32_fini(&view->geometry.scissor);
> +	pixman_region32_init_rect(&view->geometry.scissor, x, y, width, height);

Should x, y, width, height be validated/clamped in some way?  If width
and height are set very large they can affect weston_compositor_pick_view()?

> +	view->geometry.scissor_enabled = true;
> +	weston_view_geometry_dirty(view);
> +	weston_view_schedule_repaint(view);
> +}
> +
> +/** Remove the clip mask from a view
> + *
> + * \param view The view to remove the clip mask from.
> + *
> + * Removed the clip mask rectangle and schedules a repaint.
> + *
> + * \sa weston_view_set_mask
> + */
> +WL_EXPORT void
> +weston_view_set_mask_infinite(struct weston_view *view)
> +{
> +	view->geometry.scissor_enabled = false;
> +	weston_view_geometry_dirty(view);
> +	weston_view_schedule_repaint(view);
> +}
> +
>  WL_EXPORT bool
>  weston_view_is_mapped(struct weston_view *view)
>  {
> @@ -1561,6 +1699,11 @@ weston_compositor_pick_view(struct weston_compositor *compositor,
>  						    view_ix, view_iy, NULL))
>  			continue;
>  
> +		if (view->geometry.scissor_enabled &&
> +		    !pixman_region32_contains_point(&view->geometry.scissor,
> +						    view_ix, view_iy, NULL))
> +			continue;
> +
>  		*vx = view_x;
>  		*vy = view_y;
>  		return view;
> @@ -1650,6 +1793,7 @@ weston_view_destroy(struct weston_view *view)
>  	weston_layer_entry_remove(&view->layer_link);
>  
>  	pixman_region32_fini(&view->clip);
> +	pixman_region32_fini(&view->geometry.scissor);
>  	pixman_region32_fini(&view->transform.boundingbox);
>  	pixman_region32_fini(&view->transform.opaque);
>  
> @@ -1847,6 +1991,8 @@ view_accumulate_damage(struct weston_view *view,
>  					  view->geometry.x, view->geometry.y);
>  	}
>  
> +	pixman_region32_intersect(&damage, &damage,
> +				  &view->transform.boundingbox);
>  	pixman_region32_subtract(&damage, &damage, opaque);
>  	pixman_region32_union(&view->plane->damage,
>  			      &view->plane->damage, &damage);
> diff --git a/src/compositor.h b/src/compositor.h
> index 099d187..f4ba7a5 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -599,6 +599,9 @@ enum weston_capability {
>  
>  	/* backend supports setting arbitrary resolutions */
>  	WESTON_CAP_ARBITRARY_MODES		= 0x0008,
> +
> +	/* renderer supports weston_view_set_mask() clipping */
> +	WESTON_CAP_VIEW_CLIP_MASK		= 0x0010,
>  };
>  
>  struct weston_compositor {
> @@ -797,6 +800,10 @@ struct weston_view {
>  		struct wl_listener parent_destroy_listener;
>  		struct wl_list child_list; /* geometry.parent_link */
>  		struct wl_list parent_link;
> +
> +		/* managed by weston_view_set_mask() */
> +		bool scissor_enabled;
> +		pixman_region32_t scissor; /* always a simple rect */
>  	} geometry;
>  
>  	/* State derived from geometry state, read-only.
> @@ -1244,6 +1251,13 @@ void
>  weston_view_set_transform_parent(struct weston_view *view,
>  				 struct weston_view *parent);
>  
> +void
> +weston_view_set_mask(struct weston_view *view,
> +		     int x, int y, int width, int height);
> +
> +void
> +weston_view_set_mask_infinite(struct weston_view *view);
> +
>  bool
>  weston_view_is_mapped(struct weston_view *view);
>  
> 



More information about the wayland-devel mailing list