[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