On 08/25/2011 11:19 PM, Chia-I Wu wrote:
> On Fri, Aug 26, 2011 at 2:05 PM, Chia-I Wu <olva...@gmail.com> wrote:
>> On Fri, Aug 26, 2011 at 12:55 PM, Chad Versace <c...@chad-versace.us> wrote:
>>> [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 <c...@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 <c...@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?
> This shorter answer may be more clear: there is no front buffer in
> Android so EGL cannot return __DRI_BUFFER_FRONT_LEFT unless we do
> magic. To make things work, we always use double-buffered DRI
> configs, and OpenGL ES guarantees the drivers can (hopefully) never do
> front buffer rendering.
>> The biggest confusion may come from the fact that on X11, you can
>> request the front, back, depth, and other aux. buffers. But on
>> Android, you can only request the back buffer. For each frame, it
>> works like this
>>
>> dequeue the next back buffer from the native window
>> render to the buffer
>> enqueue the buffer and it will be displayed on screen
>>
>> There are actually multiple back buffers for a window, but it is
>> transparent to a client. As for depth or other aux. buffers, they are
>> supposed to allocated by EGL. As you can see in
>> droid_get_buffers_with_format, there is no magic at the moment and it
>> simply returns the buffers it has to offer.
>>
>> Why this works is because, in the end of dri2_add_config,
>> EGL_WINDOW_BIT is set only when the EGLConfig has a double buffered
>> DRI config. That makes sure the contexts that will be created will
>> have GL_DRAW_BUFFER set to GL_BACK by default. Since there is no
>> glDrawBuffer(s) in OpenGL ES, it can never be set to GL_FRONT. Though
>> we have a rb at BUFFER_FRONT_LEFT, its storage is never allocated nor
>> required. As for EGL_RENDER_BUFFER, IMHO it is best we stay faithful
>> for now and return EGL_BACK_BUFFER.
>>
>> There is no pixmap in Android so it can be ignored. As for pbuffers,
>> which I plan to support as a follow-on commit, it works pretty much
>> the same as the windows except that the back buffer is also allocated
>> by EGL.
>>
>> This works because double-buffered DRI config is used. I considered
>> fix this, and the fix I had in mind was to remove the idea of single-
>> or double- buffered contexts[1]. But I haven't had the time to work
>> on that. Another approach may be to re-route color buffers of a
>> native window to different __DRI_BUFFER attachments, such as returning
>> the back buffer as __DRI_BUFFER_FAKE_FRONT_LEFT. But then you need to
>> make sure a single-buffered DRI config is used, which does not make
>> the situation better.
>>
>> [1] http://lists.freedesktop.org/archives/mesa-dev/2011-August/010400.html
>>
Ok, I understand now. The Android display manager may be using double buffering,
triple buffering, or something else entirely; that is hidden from libEGL. All
libEGL is permitted to do is dequeue a buffer from the native window (via
ANativeWindow::dequeueBuffer), render to it, then return the it to the
display manager via ANativeWindow::enqueueBuffer.
So we consider that buffer to be __DRI_BUFFER_BACK_LEFT, and
eglQuerySurface(EGL_RENDER_BUFFER) should of course return EGL_BACK_BUFFER.
And the BUFFER_FRONT_LEFT gl_renderbuffer allocated by intelCreateBuffer
never gets used.
Thank you for taking the time to explain this.
Reviewed-by: Chad Versace <c...@chad-versace.us>
>>>
>>> --
>>> Chad Versace
>>> c...@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;
>>>>>>>>> +}
>>>
>>
>>
>>
>> --
>> o...@lunarg.com
>>
>
>
>
--
Chad Versace
c...@chad-versace.us
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
