[Mesa-dev] [PATCH 2/3] egl_dri2: add support for Android

Chad Versace chad at chad-versace.us
Thu Aug 25 21:55:03 PDT 2011


[CC'ing krh because I learned much of this from him, so may have some
insight to share with us.]

On 08/25/2011 08:14 PM, Chia-I Wu wrote:
> On Fri, Aug 26, 2011 at 4:09 AM, Chad Versace <chad at chad-versace.us> wrote:
>> On 08/24/2011 06:20 PM, Chia-I Wu wrote:
>>> On Thu, Aug 25, 2011 at 8:58 AM, Chad Versace <chad at chad-versace.us> wrote:
>>> Comments below.
>>>
>>> On 08/23/2011 08:10 PM, Chia-I Wu wrote:
>>>>>> Add platform_android.c that supports _EGL_PLAFORM_ANDROID.  It works
>>>>>> with drm_gralloc, where back buffers of windows are backed by GEM
>>>>>> objects.
>>>>>>
>>>>>> In Android a native window has a queue of back buffers allocated by the
>>>>>> server, through drm_gralloc.  For each frame, EGL needs to
>>>>>>
>>>>>>   dequeue the next back buffer
>>>>>>   render to the buffer
>>>>>>   enqueue the buffer
>>>>>>
>>>>>> After enqueuing, the buffer is no longer valid to EGL.  A window has no
>>>>>> depth buffer or other aux buffers.  They need to be allocated locally by
>>>>>> EGL.
>>>>>> ---
>>>>>>  src/egl/drivers/dri2/egl_dri2.c         |   89 +++++
>>>>>>  src/egl/drivers/dri2/egl_dri2.h         |   18 +
>>>>>>  src/egl/drivers/dri2/platform_android.c |  588 +++++++++++++++++++++++++++++++
>>>>>>  3 files changed, 695 insertions(+), 0 deletions(-)
>>>>>>  create mode 100644 src/egl/drivers/dri2/platform_android.c
>>>>>>
>>>>>> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
>>>>>> new file mode 100644
>>>>>> index 0000000..d0de94c
>>>>>> --- /dev/null
>>>>>> +++ b/src/egl/drivers/dri2/platform_android.c
>>>
>>> [snip]
>>>
>>>>>> +static _EGLSurface *
>>>>>> +droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>>>>>> +                 _EGLConfig *conf, EGLNativeWindowType window,
>>>>>> +                 const EGLint *attrib_list)
>>>>>> +{
>>>>>> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>>>>>> +   struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
>>>>>> +   struct dri2_egl_surface *dri2_surf;
>>>>>> +   int format;
>>>>>> +
>>>>>> +   (void) drv;
>>>>>> +
>>>>>> +   if (!window || window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) {
>>>>>> +      _eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface");
>>>>>> +      return NULL;
>>>>>> +   }
>>>>>> +   if (window->query(window, NATIVE_WINDOW_FORMAT, &format)) {
>>>>>> +      _eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface");
>>>>>> +      return NULL;
>>>>>> +   }
>>>>>> +   if (format != dri2_conf->base.NativeVisualID) {
>>>>>> +      _eglLog(_EGL_WARNING, "Native format mismatch: 0x%x != 0x%x",
>>>>>> +            format, dri2_conf->base.NativeVisualID);
>>>>>> +   }
>>>>>> +
>>>>>> +   dri2_surf = calloc(1, sizeof *dri2_surf);
>>>>>> +   if (!dri2_surf) {
>>>>>> +      _eglError(EGL_BAD_ALLOC, "droid_create_surface");
>>>>>> +      return NULL;
>>>>>> +   }
>>>>>> +
>>>>>> +   if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list))
>>>>>> +      goto cleanup_surf;
>>>
>>> It seems that droid_get_buffers_with_format() only supports single-buffered
>>> surfaces. So eglCreateWindowSurface should reject requests for which
>>> EGL_RENDER_BUFFER != EGL_SINGLE_BUFFER.
>>>
>>>      if (dri2_surf->base.RenderBuffer != EGL_SINGLE_BUFFER)
>>>         goto cleanup_surf;
>>>> [CC Benjamin]
>>>> EGL_RENDER_BUFFER is a hint here.  It is fine if the native window
>>>> does not support EGL_SINGLE_BUFFER.
>>
>>
>> You're right, specifying EGL_RENDER_BUFFER in eglCreateWindowSurface is just
>> a hint. And since the spec says this about eglQuerySurface,
>>    Querying EGL_RENDER_BUFFER returns the buffer which client API rendering
>>    is requested to use. For a window surface, this is the same attribute value specified
>>    when the surface was created.
>> we shouldn't alter the requested value of dri2_surf->base.RenderBuffer.
>>
>>>> On the other hand, ctx->WindowRenderBuffer should be set.  It is
>>>> queried by eglQueryContext.  I think it can be set in
>>>> dri2_create_context: to EGL_BACK_BUFFER when there is
>>>> dri_double_config and EGL_SINGLE_BUFFER when there is only
>>>> dri_single_config.  Idea?
>>
>> I agree that it seems safe to set ctx->WindowRenderBuffer when there is only a
>> dri_single_config. So, to ensure that the EGLConfig passed to
>> eglCreateContext does not have a dri_double_config, I think you need
>> to add this snippet in droid_add_configs_for_visuals:
>>   for (i = 0; visuals[i].format; i++) {
>>      ...
>>      for (j = 0; dri2_dpy->driver_configs[j]; j++) {
>>         ...
>>         /* Maybe this? */
>>         if (dri2_dpy->driver_configs[j]->modes.doubleBufferMode)
>>            continue;
>>         /* end suggestion */
>>
>>         dri2_conf = dri2_add_config(dpy, dri2_dpy->driver_configs[j],
>>               count + 1, visuals[i].size, EGL_WINDOW_BIT, NULL,
>>               visuals[i].rgba_masks);
>> Also, the call to dri2_dpy->dri2->createNewDrawable should be passed
>> dri2_conf->dri_single_config.
>>
>> But I am hesitant to set it to EGL_BACK_BUFFER when there is a
>> dri2_double_config. If the driver supports rendering to a single buffered
>> surface under a context that supports double-buffering, then, if we had set
>> ctx->WindowRenderBuffer = EGL_BACK_BUFFER, that would force
>> eglQueryContext(EGL_RENDER_BUFFER) to always return EGL_BACK_BUFFER, even
>> when rendering to the single buffered surface.
> A client can only see the back buffer(s) on Android.  There is no
> single-buffered surface on Android.  In that case, reporting
> EGL_SINGLE_BUFFER means we have to lie about a front buffer and make
> glFlush/glFinish imply eglSwapBuffers.  And out of nowhere, the EGL
> spec says that OpenGL ES does not support EGL_SINGLE_BUFFER for window
> surfaces...
> 
> I think this is one of the gray areas that hasn't been sorted out yet.
>  OpenGL requires a front buffer and GL_DRAW_BUFFER is a GL state that
> EGL (or GLX) cannot change.  It is not clear how
> eglQueryContext(EGL_RENDER_BUFFER) and glDrawBuffer() interact.  On
> the other hand, OpenGL ES does not require a front buffer and there is
> no GL_DRAW_BUFFER so EGL has EGL_RENDER_BUFFER to make up for it.
> Take these into considerations, this issue may be too much for this
> series to resolve.
> 

Aside from the spec confusion, which is indeed a mess, I'm confused on how
droid_create_surface and droid_get_buffers_with_format interact. If you could
clear that up for me, then maybe I would better understand your reply.

Let's walk through the creation of eglCreateWindowSurface with an Intel
driver on Android, and in the process I will explain why the behavior of
droid_get_buffers_with_format confuses me.

1. eglCreateWindowSurface resolves to droid_create_surface
2. which calls dri2_dpy->dri2->createDrawable(..., dri2_conf->dri_double_config, ...)
3. which resolves to dri2CreateNewDrawable
4. which calls driCreateNewDrawable
4. which calls psp->DriverAPI.CreateBuffer(..., &config->modes, ...)
5. which resolves to intelCreateBuffer(..., mesaVis, ...)

For non-pixamp surfaces, intelCreateBuffer always creates BUFFER_FRONT_LEFT. It
also creates BUFFER_BACK_LEFT if the mesaVis is double-buffered (which it is in
this case, because we passed in dri2_conf->dri_double_config).

Sometime before drawing to the surface, the following call sequence occurs on that
surface (which is here called the drawable).
10. intel_update_renderbuffers(..., drawable) is called
11. which calls intel_query_dri2_buffers_no_separate_stencil(..., drawable, ...)
12. which calls screen->dri2.loader->getBuffersWithFormat(drawable, ...)
13. which resolves to droid_get_buffers_with_format(driDrawable, ...)

Before continuing the walkthrough, I'll reproduce droid_get_buffers_with_format for reference.

> +static __DRIbuffer *
> +droid_get_buffers_with_format(__DRIdrawable * driDrawable,
> +			     int *width, int *height,
> +			     unsigned int *attachments, int count,
> +			     int *out_count, void *loaderPrivate)
> +{
> +   struct dri2_egl_surface *dri2_surf = loaderPrivate;
> +   struct dri2_egl_display *dri2_dpy =
> +      dri2_egl_display(dri2_surf->base.Resource.Display);
> +   int i;
> +
> +   if (!dri2_surf->buffer) {
> +      if (!droid_dequeue_buffer(dri2_surf))
> +         return NULL;
> +   }
> +
> +   /* free outdated buffers and update the surface size */
> +   if (dri2_surf->base.Width != dri2_surf->buffer->width ||
> +       dri2_surf->base.Height != dri2_surf->buffer->height) {
> +      droid_free_local_buffers(dri2_surf);
> +      dri2_surf->base.Width = dri2_surf->buffer->width;
> +      dri2_surf->base.Height = dri2_surf->buffer->height;
> +   }
> +
> +   dri2_surf->buffer_count = 0;
> +   for (i = 0; i < count * 2; i += 2) {
> +      __DRIbuffer *buf, *local;
> +
> +      assert(dri2_surf->buffer_count < ARRAY_SIZE(dri2_surf->buffers));
> +      buf = &dri2_surf->buffers[dri2_surf->buffer_count];
> +
> +      switch (attachments[i]) {
> +      case __DRI_BUFFER_BACK_LEFT:
> +         buf->attachment = attachments[i];
> +         buf->name = get_native_buffer_name(dri2_surf->buffer);
> +         buf->cpp = get_format_bpp(dri2_surf->buffer->format);
> +         buf->pitch = dri2_surf->buffer->stride * buf->cpp;
> +         buf->flags = 0;
> +
> +         dri2_surf->buffer_count++;
> +         break;
> +      case __DRI_BUFFER_DEPTH:
> +      case __DRI_BUFFER_STENCIL:
> +      case __DRI_BUFFER_ACCUM:
> +      case __DRI_BUFFER_DEPTH_STENCIL:
> +      case __DRI_BUFFER_HIZ:
> +         local = droid_alloc_local_buffer(dri2_surf,
> +               attachments[i], attachments[i + 1]);
> +
> +         if (local) {
> +            *buf = *local;
> +            dri2_surf->buffer_count++;
> +         }
> +         break;
> +      case __DRI_BUFFER_FRONT_LEFT:
> +      case __DRI_BUFFER_FRONT_RIGHT:
> +      case __DRI_BUFFER_FAKE_FRONT_LEFT:
> +      case __DRI_BUFFER_FAKE_FRONT_RIGHT:
> +      case __DRI_BUFFER_BACK_RIGHT:
> +      default:
> +         /* no front or right buffers */
> +         break;
> +      }
> +   }
> +
> +   if (width)
> +      *width = dri2_surf->base.Width;
> +   if (height)
> +      *height = dri2_surf->base.Height;
> +
> +   *out_count = dri2_surf->buffer_count;;
> +
> +   return dri2_surf->buffers;
> +}

For a surface created with a single-buffered config, there is a conflict.
droid_create_surface (via intelCreateBuffer) created a __DRI_BUFFER_FRONT_LEFT,
but no __DRI_BUFFER_BACK_LEFT, but droid_get_buffers_with_format ignores the
front-left and handles the back-left.

For a surface created with a double-buffered config, why is the front-left
ignored, even though dri2_create_surface created a front-left renderbuffer?

Is this due to some quirk of the Android window manager? Did I fail to
understand something here?

-- 
Chad Versace
chad at chad-versace.us

>>>
>>> I think the DRI2 surface type needs to be set here too.
>>>
>>>      dri2_surf->type = DRI2_WINDOW_SURFACE;
>>>> This field seems to be used by wayland only.  For the other places,
>>>> dri2_surf->Base.Type is used.
>>>>>> +
>>>>>> +   dri2_surf->dri_drawable =
>>>>>> +      (*dri2_dpy->dri2->createNewDrawable)(dri2_dpy->dri_screen,
>>>>>> +                                        dri2_conf->dri_double_config,
>>                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> I think dri2_conf->dri_single_config should be passed here.
>>
>>>>>> +                                           dri2_surf);
>>>>>> +   if (dri2_surf->dri_drawable == NULL) {
>>>>>> +      _eglError(EGL_BAD_ALLOC, "dri2->createNewDrawable");
>>>>>> +      goto cleanup_surf;
>>>>>> +   }
>>>>>> +
>>>>>> +   window->common.incRef(&window->common);
>>>>>> +   window->query(window, NATIVE_WINDOW_WIDTH, &dri2_surf->base.Width);
>>>>>> +   window->query(window, NATIVE_WINDOW_HEIGHT, &dri2_surf->base.Height);
>>>>>> +
>>>>>> +   dri2_surf->window = window;
>>>>>> +
>>>>>> +   return &dri2_surf->base;
>>>>>> +
>>>>>> +cleanup_surf:
>>>>>> +   free(dri2_surf);
>>>>>> +
>>>>>> +   return NULL;
>>>>>> +}


More information about the mesa-dev mailing list