[PATCH v14 18/41] compositor-drm: Make alpha-to-opaque handling common

[Pekka Paalanen]

On Wed, 20 Dec 2017 12:26:35 +0000
Daniel Stone <daniels at collabora.com> wrote:

> Rather than a hardcoded ARGB8888 -> XRGB8888 translation inside a
> GBM-specific helper, just determine whether or not the view is opaque,
> and use the generic helpers to implement the format translation.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  libweston/compositor-drm.c | 111 +++++++++++++++++----------------------------
>  1 file changed, 41 insertions(+), 70 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index d5955c9ca..d61be4f1b 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -946,7 +946,7 @@ drm_fb_ref(struct drm_fb *fb)
>  
>  static struct drm_fb *
>  drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend,
> -		   uint32_t format, enum drm_fb_type type)
> +		   bool is_opaque, enum drm_fb_type type)
>  {

> @@ -1665,7 +1662,7 @@ drm_output_render_gl(struct drm_output_state *state, pixman_region32_t *damage)
>  		return NULL;
>  	}
>  
> -	ret = drm_fb_get_from_bo(bo, b, output->gbm_format, BUFFER_GBM_SURFACE);
> +	ret = drm_fb_get_from_bo(bo, b, false, BUFFER_GBM_SURFACE);

is_opaque=false is a little counter-intuitive here, because our
renderers always produce opaque images. I suppose the intention here is
to use the bo format as is. Would it warrant a comment on why we want
that?

>  	if (!ret) {
>  		weston_log("failed to get drm_fb for bo\n");
>  		gbm_surface_release_buffer(output->gbm_surface, bo);

> @@ -2763,12 +2731,16 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
>  	if (!bo)
>  		goto err;
>  
> -	format = drm_output_check_plane_format(p, ev, bo);
> -	if (format == 0)
> +	state->fb = drm_fb_get_from_bo(bo, b, drm_view_is_opaque(ev),
> +				       BUFFER_CLIENT);
> +	if (!state->fb)
>  		goto err;
>  
> -	state->fb = drm_fb_get_from_bo(bo, b, format, BUFFER_CLIENT);
> -	if (!state->fb)
> +	/* Check whether the format is supported */
> +	for (i = 0; i < p->count_formats; i++)
> +		if (p->formats[i] == state->fb->format->format)
> +			break;
> +	if (i == p->count_formats)
>  		goto err;

The error path calls gbm_bo_destroy(), it probably shouldn't.


>  
>  	drm_fb_set_buffer(state->fb, ev->surface->buffer_ref.buffer);
> @@ -3992,8 +3964,7 @@ drm_output_init_cursor_egl(struct drm_output *output, struct drm_backend *b)
>  			goto err;
>  
>  		output->gbm_cursor_fb[i] =
> -			drm_fb_get_from_bo(bo, b, GBM_FORMAT_ARGB8888,
> -					   BUFFER_CURSOR);
> +			drm_fb_get_from_bo(bo, b, false, BUFFER_CURSOR);
>  		if (!output->gbm_cursor_fb[i]) {
>  			gbm_bo_destroy(bo);
>  			goto err;

Other than those, looks good to me.


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/20180123/3f75a1c7/attachment-0001.sig>