[Mesa-dev] EGL/android: pbuffer implementation.

[Emil Velikov]

On 14 November 2016 at 11:49, Liu Zhiquan <zhiquan.liu at intel.com> wrote:
> mesa android path didn't support pbuffer, so add pbuffer support to
> fix most deqp and cts pbuffer test cases fail;
> add support of front buffer and single buffer config.
>
> Test status: android CTS EGL pbuffer test can run without native crash.
> test:[DEQP,EGL]all deqp pbuffer case passed.
>
> V2: remove duplicate codes.
>
For a second I thought you did not address everything. Information
about the bugs fixed is great, but you want to give a brief summary
about the implementation.
A couple of things that come to mind (although feel free to change/add
your favourite)
 - before double-buffered - now no longer the case.
 - pbuffer is implemented similar to surfaceless/x11 where the [dri]
driver expects a front buffer thus we have to "fake it" in the loader.

> Signed-off-by: Liu Zhiquan <zhiquan.liu at intel.com>
> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.h         |   3 +-
>  src/egl/drivers/dri2/platform_android.c | 109 ++++++++++++++++++++++++--------
>  2 files changed, 86 insertions(+), 26 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index 3add32e..f3d09dc 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -290,7 +290,8 @@ struct dri2_egl_surface
>  #ifdef HAVE_ANDROID_PLATFORM
>     struct ANativeWindow *window;
>     struct ANativeWindowBuffer *buffer;
> -   __DRIimage *dri_image;
> +   __DRIimage *dri_image_back;
> +   __DRIimage *dri_image_front;
>
>     /* EGL-owned buffers */
>     __DRIbuffer           *local_buffers[__DRI_BUFFER_COUNT];
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index ec52a02..760a45c 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -43,6 +43,20 @@
>
>  #define ALIGN(val, align)      (((val) + (align) - 1) & ~((align) - 1))
>
> +static __DRIimage*
> +alloc_image(struct dri2_egl_display *dri2_dpy,
> +                  struct dri2_egl_surface *dri2_surf,
> +                  uint32_t format)
> +{
> +   return dri2_dpy->image->createImage(
> +            dri2_dpy->dri_screen,
> +            dri2_surf->base.Width,
> +            dri2_surf->base.Height,
> +            format,
Well done with propagating the requested format ! Seems that
platform_surfaceless/wayland/other? have a bug here :-\
Can I interest you in fixing those as follow-up patches ? Note this
request is _not_ a requirement/blocker for this work to land.

Nitpick: can you please inline the function into its only caller or if
you prefer to keep it separate (as-is) rewrap to use the full line.


>  static int
> -get_back_bo(struct dri2_egl_surface *dri2_surf)
> +get_back_bo(struct dri2_egl_display *dri2_dpy, struct dri2_egl_surface *dri2_surf)
>  {
> -   struct dri2_egl_display *dri2_dpy =
> -      dri2_egl_display(dri2_surf->base.Resource.Display);


> +      if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
> +         get_back_bo(dri2_dpy, dri2_surf);
> +      }
> +
> +      if (!dri2_surf->dri_image_back) {
Nitpick: please keep get_back_bo() as-is (no extra argument, check
return value) and update it as a follow-up.

With the trivial suggestions, the patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

Thanks
Emil