[PATCH weston 10/68] compositor-drm: Make scanout view preparation more stringent

[Derek Foreman]

On 09/12/16 01:57 PM, Daniel Stone wrote:
> Don't import buffers which span multiple outputs, short-cut any attempt
> to import SHM buffers, and ignore buffers with a global alpha set.
>
> I'm not convinced all of these conditions entirely make sense, but this
> at least makes them equally nonsensical.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
>
> Differential Revision: https://phabricator.freedesktop.org/D1414
> ---
>  libweston/compositor-drm.c | 35 ++++++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8cd9d5a..e22d792 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -462,6 +462,13 @@ drm_output_release_fb(struct drm_output *output, struct drm_fb *fb)
>  	}
>  }
>
> +static int
> +drm_view_transform_supported(struct weston_view *ev)
> +{
> +	return !ev->transform.enabled ||
> +		(ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE);
> +}
> +
>  static uint32_t
>  drm_output_check_scanout_format(struct drm_output *output,

Not sure this function name is really ideal anymore?  Maybe it never 
was, lots of stuff here I'm not sure one would all "format".

>  				struct weston_surface *es, struct gbm_bo *bo)
> @@ -502,27 +509,40 @@ drm_output_prepare_scanout_view(struct drm_output *output,
>  	struct gbm_bo *bo;
>  	uint32_t format;
>
> +	/* Don't import buffers which span multiple outputs. */
> +	if (ev->output_mask != (1u << output->base.id))
> +		return NULL;
> +

I suppose technically this is overly stringent, but I'd rather it be too 
careful than broken.

>  	/* We use GBM to import buffers. */
>  	if (b->gbm == NULL)
>  		return NULL;
>
>  	if (buffer == NULL)
>  		return NULL;
> +	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;
> +

Was there a reason to move this block?

I really don't have any concerns that would warrant another revision, 
this looks good to me.  (Also looks like it could be trivially landed 
without dependency on any other patch in this series, with no functional 
issues.)

Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

>  	if (ev->transform.enabled)
>  		return NULL;
>  	if (ev->geometry.scissor_enabled)
>  		return NULL;
> -
> -	if (buffer->width != output->base.current_mode->width ||
> -	    buffer->height != output->base.current_mode->height)
> -		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;
>
>  	bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER,
>  			   buffer->resource, GBM_BO_USE_SCANOUT);
> @@ -936,13 +956,6 @@ drm_output_check_sprite_format(struct drm_sprite *s,
>  	return 0;
>  }
>
> -static int
> -drm_view_transform_supported(struct weston_view *ev)
> -{
> -	return !ev->transform.enabled ||
> -		(ev->transform.matrix.type < WESTON_MATRIX_TRANSFORM_ROTATE);
> -}
> -
>  static struct weston_plane *
>  drm_output_prepare_overlay_view(struct drm_output *output,
>  				struct weston_view *ev)
>