[Mesa-dev] [PATCH 3/8] egl/gbm: Implement EGL_EXT_buffer_age

[Eric Anholt]

Kristian Høgsberg <krh at bitplanet.net> writes:

> On Sat, Dec 15, 2012 at 2:44 AM, Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>> On Fri, Dec 14, 2012 at 04:54:32PM -0800, Eric Anholt wrote:
>>> Kristian Høgsberg <krh at bitplanet.net> writes:
>>>
>>> > ---
>>> >  src/egl/drivers/dri2/egl_dri2.h     |    1 +
>>> >  src/egl/drivers/dri2/platform_drm.c |   23 +++++++++++++++++++++++
>>> >  2 files changed, 24 insertions(+)
>>> >
>>> > diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
>>> > index be359d3..fa2a9f3 100644
>>> > --- a/src/egl/drivers/dri2/egl_dri2.h
>>> > +++ b/src/egl/drivers/dri2/egl_dri2.h
>>> > @@ -189,6 +189,7 @@ struct dri2_egl_surface
>>> >     struct {
>>> >        struct gbm_bo       *bo;
>>> >        int                  locked;
>>> > +      int                  age;
>>> >     } color_buffers[3], *back, *current;
>>> >  #ifndef HAVE_WAYLAND_PLATFORM
>>> >     __DRIbuffer           *dri_buffers[__DRI_BUFFER_COUNT];
>>> > diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
>>> > index 3e04a6c..1923033 100644
>>> > --- a/src/egl/drivers/dri2/platform_drm.c
>>> > +++ b/src/egl/drivers/dri2/platform_drm.c
>>> > @@ -324,11 +324,16 @@ dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *draw)
>>> >  {
>>> >     struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
>>> >     struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
>>> > +   int i;
>>> >
>>> >     if (dri2_surf->base.Type == EGL_WINDOW_BIT) {
>>> >        if (dri2_surf->current)
>>> >      _eglError(EGL_BAD_SURFACE, "dri2_swap_buffers");
>>> > +      for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++)
>>> > +         if (dri2_surf->color_buffers[i].bo)
>>> > +            dri2_surf->color_buffers[i].age++;
>>> >        dri2_surf->current = dri2_surf->back;
>>> > +      dri2_surf->current->age = 1;
>>> >        dri2_surf->back = NULL;
>>>
>>> OK, if I'm following this right, the ages of my buffers with this code are:
>>
>> You're missing the if (dri2_surf->color_buffers[i].bo) condition on
>> incrementing age.  We start out with bo = NULL for color_buffer[0..2]
>> and allocate the bos on demand...
>>
>>> entering my first swap
>>> buffers[0] = 0 <- front
>>> buffers[1] = 0 <- back
>>>
>>> after first swap:
>>> buffers[0] = 1 <- back
>>> buffers[1] = 1 <- front
>>
>> and so here back age will be 0 because we pick a new color_buffer from
>> the pool that doesn't yet have a bo and whose age was never incremented.
>>
>>> after second swap:
>>> buffers[0] = 1 <- front
>>> buffers[1] = 2 <- back
>>>
>>> But reading the spec, I think it's supposed to be:
>>>
>>> entering my first swap
>>> buffers[0] = 0 <- front
>>> buffers[1] = 0 <- back
>>>
>>> after first swap:
>>> buffers[0] = 0 <- back
>>> buffers[1] = 1 <- front
>>>
>>> after second swap:
>>> buffers[0] = 1 <- front
>>> buffers[1] = 2 <- back
>>>
>>> Note how after the first swap, my backbuffer should have an age of 0, for
>>> "unknown junk".
>>
>> Yup, and that's what this patch gets.
>
> Does the explanation above make sense?

Sort of -- it's all predicated on there being no way for buffers to be
allocated other than a swapbuffers -- so no MESA_copy_sub_buffers or its
cousins, and nothing for binding the front buffer as an egl image (do we
actually not support any of those?).  It seems more future-proof to just
implement the "if (buffer->age != 0)" check from the spec, along with or
in place of the "if (buffer->bo != NULL)"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121219/e1179b9b/attachment.pgp>