[PATCH v16 06/23] compositor-drm: Centralise more transform checks

[Pekka Paalanen]

On Thu,  5 Jul 2018 18:16:33 +0100
Daniel Stone <daniels at collabora.com> wrote:

> Put some more transformation checks in drm_view_transform_supported.
> 

Please mention that this patch removes the requirement of identical
scales for overlays and the reason why. It seems to have been replaced
with a less strict check that prevents scaling after the transformation
matrices are included.

I think loosening the check is more important than the refactoring, so
that might make a better patch subject.

> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Tested-by: Emre Ucan <eucan at de.adit-jv.com>
> ---
>  libweston/compositor-drm.c | 54 ++++++++++++++++++++++++--------------
>  1 file changed, 34 insertions(+), 20 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 59e5e4ec6..c8eb7ef59 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1172,11 +1172,30 @@ drm_plane_state_put_back(struct drm_plane_state *state)
>  	(void) drm_plane_state_alloc(state_output, plane);
>  }
>  
> +static int

The return type should be bool.


> +drm_view_transform_supported(struct weston_view *ev, struct weston_output *output)
> +{
> +	struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
> +
> +	/* This will incorrectly disallow cases where the combination of
> +	 * buffer and view transformations match the output transform.
> +	 * Fixing this requires a full analysis of the transformation
> +	 * chain. */
> +	if (ev->transform.enabled &&
> +	    ev->transform.matrix.type >= WESTON_MATRIX_TRANSFORM_ROTATE)
> +		return false;
> +
> +	if (viewport->buffer.transform != output->transform)
> +		return false;
> +
> +	return true;
> +}
> +
>  /**
>   * Given a weston_view, fill the drm_plane_state's co-ordinates to display on
>   * a given plane.
>   */
> -static void
> +static bool
>  drm_plane_state_coords_for_view(struct drm_plane_state *state,
>  				struct weston_view *ev)
>  {
> @@ -1186,6 +1205,9 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state,
>  	pixman_box32_t *box, tbox;
>  	float sxf1, syf1, sxf2, syf2;
>  
> +	if (!drm_view_transform_supported(ev, &output->base))
> +		return false;
> +
>  	/* Update the base weston_plane co-ordinates. */
>  	box = pixman_region32_extents(&ev->transform.boundingbox);
>  	state->plane->base.x = box->x1;
> @@ -1241,6 +1263,8 @@ drm_plane_state_coords_for_view(struct drm_plane_state *state,
>  		state->src_w = (buffer->width << 16) - state->src_x;
>  	if (state->src_h > (uint32_t) ((buffer->height << 16) - state->src_y))
>  		state->src_h = (buffer->height << 16) - state->src_y;
> +
> +	return true;
>  }
>  
>  /**
> @@ -1580,13 +1604,6 @@ drm_output_assign_state(struct drm_output_state *state,
>  	}
>  }
>  
> -static int
> -drm_view_transform_supported(struct weston_view *ev)
> -{
> -	return !ev->transform.enabled ||
> -		(ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE);
> -}
> -
>  static bool
>  drm_view_is_opaque(struct weston_view *ev)
>  {
> @@ -1650,7 +1667,7 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state,
>  		return NULL;

The buffer<->output transform check on the line just above becomes
redundant. Should it be removed?

Looks good otherwise.


Thanks,
pq


>  	if (viewport->buffer.scale != output->base.current_scale)
>  		return NULL;
> -	if (!drm_view_transform_supported(ev))
> +	if (!drm_view_transform_supported(ev, &output->base))
>  		return NULL;
>  
>  	if (ev->alpha != 1.0f)
> @@ -2702,7 +2719,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
>  {
>  	struct drm_output *output = output_state->output;
>  	struct weston_compositor *ec = output->base.compositor;
> -	struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
>  	struct drm_backend *b = to_drm_backend(ec);
>  	struct wl_resource *buffer_resource;
>  	struct drm_plane *p;
> @@ -2728,13 +2744,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
>  	if (wl_shm_buffer_get(buffer_resource))
>  		return NULL;
>  
> -	if (viewport->buffer.transform != output->base.transform)
> -		return NULL;
> -	if (viewport->buffer.scale != output->base.current_scale)
> -		return NULL;
> -	if (!drm_view_transform_supported(ev))
> -		return NULL;
> -
>  	if (ev->alpha != 1.0f)
>  		return NULL;
>  
> @@ -2758,6 +2767,14 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
>  	if (!state)
>  		return NULL;
>  
> +	state->output = output;
> +	if (!drm_plane_state_coords_for_view(state, ev))
> +		goto err;
> +
> +	if (state->src_w != state->dest_w << 16 ||
> +	    state->src_h != state->dest_h << 16)
> +		goto err;
> +
>  	if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) {
>  #ifdef HAVE_GBM_FD_IMPORT
>  		/* XXX: TODO:
> @@ -2816,9 +2833,6 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
>  
>  	drm_fb_set_buffer(state->fb, ev->surface->buffer_ref.buffer);
>  
> -	state->output = output;
> -	drm_plane_state_coords_for_view(state, ev);
> -
>  	return &p->base;
>  
>  err:

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180706/bbf07409/attachment.sig>