On Wed, 20 Dec 2017 12:26:35 +0000
Daniel Stone <[email protected]> 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 <[email protected]>
> ---
>  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

Attachment: pgp6UpeuTMO4H.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to