[Mesa-dev] ANDROID: eglCreateImageKHR missing modifiers

[Tomasz Figa]

Hi Martin,

On Sat, Jul 21, 2018 at 1:01 AM Martin Fuzzey
<martin.fuzzey at flowbird.group> wrote:
>
> Hi,
>
> I am testing mesa / etnaviv / gbm_gralloc under Android 8.1 on i.MX6
>
> I discovered the screen capture function was not working (it was
> producing empty buffers).
>
> The reason for this seems to be that eglCreateImageKHR() with
> EGL_NATIVE_BUFFER_ANDROID does not import the modifier information.
>
> I fixed it with the patch below, but I don't think this is the right way
> as it is adding back a dependency on the gralloc_handle structure.

As you noticed, this adds back the dependency on gralloc handle
structure. Moreover, it actually adds a dependency on the handle
having a binary format compatible with gralloc_gbm_handle_t. This is
not something that we can do in upstream Mesa, since there are more
platforms running gralloc implementations other than gbm_gralloc.

>
>
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -43,6 +43,8 @@
>   #include "gralloc_drm.h"
>   #endif /* HAVE_DRM_GRALLOC */
>
> +#include <gralloc_handle.h>
> +
>   #define ALIGN(val, align)      (((val) + (align) - 1) & ~((align) - 1))
>
>   struct droid_yuv_format {
> @@ -801,6 +803,9 @@ droid_create_image_from_prime_fd(_EGLDisplay *disp,
> _EGLContext *ctx,
>                                    struct ANativeWindowBuffer *buf, int fd)
>   {
>      unsigned int pitch;
> +   struct gralloc_gbm_handle_t *grh;
> +   uint64_t modifier = 0;
> +   bool have_modifier = false;
>
>      if (is_yuv(buf->format)) {
>         _EGLImage *image;
> @@ -829,17 +834,34 @@ droid_create_image_from_prime_fd(_EGLDisplay
> *disp, _EGLContext *ctx,
>         return NULL;
>      }
>
> -   const EGLint attr_list[] = {
> +   grh = (struct gralloc_gbm_handle_t *)buf->handle;
> +   if (grh->magic == GRALLOC_HANDLE_MAGIC) {
> +      modifier = grh->modifier;
> +      have_modifier = true;
> +   }
> +
> +   EGLint attr_list[] = {
>         EGL_WIDTH, buf->width,
>         EGL_HEIGHT, buf->height,
>         EGL_LINUX_DRM_FOURCC_EXT, fourcc,
>         EGL_DMA_BUF_PLANE0_FD_EXT, fd,
>         EGL_DMA_BUF_PLANE0_PITCH_EXT, pitch,
>         EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
> +      EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT, modifier & 0xffffffff,
> +      EGL_DMA_BUF_PLANE0_MODIFIER_HI_EXT, modifier >> 32,
>         EGL_NONE, 0
>      };
>
> -   return dri2_create_image_dma_buf(disp, ctx, NULL, attr_list);
> +   if (!have_modifier) {
> +      for (int i=0; i < ARRAY_SIZE(attr_list); i+=2) {
> +         if (attr_list[i] == EGL_DMA_BUF_PLANE0_MODIFIER_LO_EXT) {
> +            attr_list[i] = EGL_NONE;
> +            break;
> +         }
> +      }
> +   }
> +
> +   return dri2_create_image_dma_buf(disp, ctx, NULL, (const EGLint
> *)attr_list);
>   }
>
> What is the preferred way of fixing this?

Unfortunately, I don't have a very good solution for this. In Chrome
OS we just don't support modifiers in EGL/GLES on Android. Obviously
it would be a good idea to have some way to query gralloc for the
modifier, but I don't see Android providing any generic way to do it.

The least evil I can think of as a makeshift for now could be an
optional gralloc0 perform call that returns the modifier for given
buffer.

Best regards,
Tomasz