[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? -- 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; >>>>>> +} _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev