Kristian Høgsberg <k...@bitplanet.net> writes:
> On Sat, Dec 15, 2012 at 2:44 AM, Kristian Høgsberg <hoegsb...@gmail.com>
> wrote:
>> On Fri, Dec 14, 2012 at 04:54:32PM -0800, Eric Anholt wrote:
>>> Kristian Høgsberg <k...@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)"
pgpapy8ZQWcrD.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
