[PATCH v16 07/23] compositor-drm: Use plane_state_coords_for_view for scanout

[Pekka Paalanen]

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

> Now that we have a helper to fill the plane state co-ordinates from a
> view, use this for the scanout plane.
> 
> We now explicitly check that the view fills exactly the fullscreen area
> and nothing else. We then use the new helper to fill out the plane state
> values, and do further checks against the filled-in co-ordinates, i.e.
> that we're not trying to show an offset into the buffer, or to scale the
> image.
> 
> An audit of the error paths found some places where we would leave a
> plane state hanging; this makes them all consistent.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Tested-by: Emre Ucan <eucan at de.adit-jv.com>
> ---
>  libweston/compositor-drm.c | 64 +++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index c8eb7ef59..2927a0ebf 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1635,8 +1635,8 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state,
>  	struct drm_plane *scanout_plane = output->scanout_plane;
>  	struct drm_plane_state *state;
>  	struct weston_buffer *buffer = ev->surface->buffer_ref.buffer;
> -	struct weston_buffer_viewport *viewport = &ev->surface->buffer_viewport;
>  	struct gbm_bo *bo;
> +	pixman_box32_t *extents;
>  
>  	/* Don't import buffers which span multiple outputs. */
>  	if (ev->output_mask != (1u << output->base.id))
> @@ -1651,23 +1651,13 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state,
>  	if (wl_shm_buffer_get(buffer->resource))
>  		return NULL;
>  
> -	/* Make sure our view is exactly compatible with the output. */
> -	if (ev->geometry.x != output->base.x ||
> -	    ev->geometry.y != output->base.y)
> -		return NULL;
> -	if (buffer->width != output->base.current_mode->width ||
> -	    buffer->height != output->base.current_mode->height)
> -		return NULL;
> -
> -	if (ev->transform.enabled)
> -		return NULL;
> -	if (ev->geometry.scissor_enabled)
> -		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, &output->base))
> +	/* Check the view spans exactly the output size, calculated in the
> +	 * logical co-ordinate space. */
> +	extents = pixman_region32_extents(&ev->transform.boundingbox);
> +	if (extents->x1 != output->base.x ||
> +	    extents->y1 != output->base.y ||
> +	    extents->x2 != output->base.x + output->base.width ||
> +	    extents->y2 != output->base.y + output->base.height)
>  		return NULL;

Isn't this check a sub-set of the dest and src rectangles check below?
Is this needed for something? Just an early exit?

>  
>  	if (ev->alpha != 1.0f)
> @@ -1682,44 +1672,48 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state,
>  		return NULL;
>  	}
>  
> +	state->output = output;
> +	if (!drm_plane_state_coords_for_view(state, ev))
> +		goto err;
> +
> +	/* The legacy API does not let us perform cropping or scaling. */
> +	if (state->src_x != 0 || state->src_y != 0 ||
> +	    state->src_w != state->dest_w << 16 ||
> +	    state->src_h != state->dest_h << 16 ||
> +	    state->dest_x != 0 || state->dest_y != 0 ||
> +	    state->dest_w != (unsigned) output->base.current_mode->width ||
> +	    state->dest_h != (unsigned) output->base.current_mode->height)
> +		goto err;
> +
>  	bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER,
>  			   buffer->resource, GBM_BO_USE_SCANOUT);
>  

Anyway:

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- 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/fe57ecec/attachment.sig>