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