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
>
>>
>> --
>> 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
>
--
o...@lunarg.com
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
