[Mesa-dev] [PATCH][RFC] dri3: Add support for the GLX_EXT_buffer_age extension

Ian Romanick idr at freedesktop.org
Wed Feb 19 17:59:41 PST 2014


On 02/19/2014 02:49 PM, Adel Gadllah wrote:
> Hi,
> 
> The attached patch adds support for the GLX_EXT_buffer_age extension,
> which is mostly used by compositors for efficient sub screen updates.
> 
> The extension should not be reported as supported when running DRI2 but
> it seems to show up when I try to disable it with LIBGL_DRI3_DISABLE ...
> not sure why suggestions welcome.
> 
> 
> P.S: Please CC me when replying as I am not subscribed to the list.

You'll need to fix that. :)

You didn't send this patch with git-send-email.  Whatever you used to
send it also mangled it, so it won't apply.

> From: Adel Gadllah <adel.gadllah at gmail.com>
> Date: Sun, 16 Feb 2014 13:40:42 +0100
> Subject: [PATCH] dri3: Add GLX_EXT_buffer_age support
> 
> ---
>  include/GL/glx.h              |  5 +++++
>  include/GL/glxext.h           |  5 +++++
>  src/glx/dri2_glx.c            |  1 +
>  src/glx/dri3_glx.c            | 17 +++++++++++++++++
>  src/glx/dri3_priv.h           |  2 ++
>  src/glx/glx_pbuffer.c         |  7 +++++++
>  src/glx/glxclient.h           |  1 +
>  src/glx/glxextensions.c       |  1 +
>  src/glx/glxextensions.h       |  1 +
>  src/mesa/drivers/x11/glxapi.c |  3 +++
>  10 files changed, 43 insertions(+)
> 
> diff --git a/include/GL/glx.h b/include/GL/glx.h
> index 234abc0..b8b4d75 100644
> --- a/include/GL/glx.h
> +++ b/include/GL/glx.h
> @@ -161,6 +161,11 @@ extern "C" {
>  #define GLX_SAMPLES                     0x186a1 /*100001*/
> 
> 
> +/*
> + * GLX_EXT_buffer_age
> + */
> +#define GLX_BACK_BUFFER_AGE_EXT 0x20F4
> +
> 
>  typedef struct __GLXcontextRec *GLXContext;
>  typedef XID GLXPixmap;
> diff --git a/include/GL/glxext.h b/include/GL/glxext.h
> index 8c642f3..36e92dc 100644
> --- a/include/GL/glxext.h
> +++ b/include/GL/glxext.h
> @@ -383,6 +383,11 @@ void glXReleaseTexImageEXT (Display *dpy,
> GLXDrawable drawable, int buffer);
>  #define GLX_FLIP_COMPLETE_INTEL           0x8182
>  #endif /* GLX_INTEL_swap_event */
> 
> +#ifndef GLX_EXT_buffer_age
> +#define GLX_EXT_buffer_age 1
> +#define GLX_BACK_BUFFER_AGE_EXT 0x20F4
> +#endif /* GLX_EXT_buffer_age */
> +

We get glxext.h directly from Khronos, so it should not be modified...
except to import new versions from upstream.  It looks like the upstream
glxext.h has this, so the first patch in the series should be "glx:
Update glxext.h to revision 25407."  And drop the change to glx.h.

>  #ifndef GLX_MESA_agp_offset
>  #define GLX_MESA_agp_offset 1
>  typedef unsigned int ( *PFNGLXGETAGPOFFSETMESAPROC) (const void *pointer);
> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
> index 67fe9c1..007f449 100644
> --- a/src/glx/dri2_glx.c
> +++ b/src/glx/dri2_glx.c
> @@ -1288,6 +1288,7 @@ dri2CreateScreen(int screen, struct glx_display *
> priv)
>     psp->waitForSBC = NULL;
>     psp->setSwapInterval = NULL;
>     psp->getSwapInterval = NULL;
> +   psp->queryBufferAge = NULL;
> 
>     if (pdp->driMinor >= 2) {
>        psp->getDrawableMSC = dri2DrawableGetMSC;
> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
> index 70ec057..07120e1 100644
> --- a/src/glx/dri3_glx.c
> +++ b/src/glx/dri3_glx.c
> @@ -1345,6 +1345,8 @@ dri3_swap_buffers(__GLXDRIdrawable *pdraw, int64_t
> target_msc, int64_t divisor,
>           target_msc = priv->msc + priv->swap_interval * (priv->send_sbc
> - priv->recv_sbc);
> 
>        priv->buffers[buf_id]->busy = 1;
> +      priv->buffers[buf_id]->last_swap = priv->swap_count;
> +
>        xcb_present_pixmap(c,
>                           priv->base.xDrawable,
>                           priv->buffers[buf_id]->pixmap,
> @@ -1379,11 +1381,23 @@ dri3_swap_buffers(__GLXDRIdrawable *pdraw,
> int64_t target_msc, int64_t divisor,
>        xcb_flush(c);
>        if (priv->stamp)
>           ++(*priv->stamp);
> +
> +       priv->swap_count++;
>     }
> 
>     return ret;
>  }
> 
> +static int
> +dri3_query_buffer_age(__GLXDRIdrawable *pdraw)
> +{
> +  struct dri3_drawable *priv = (struct dri3_drawable *) pdraw;
> +  int buf_id = DRI3_BACK_ID(priv->cur_back);

Blank line here.

Also maybe use dri3_back_buffer instead?

   const struct dri3_buffer *const back = dri3_back_buffer(priv);

   if (back->last_swap != 0)
      return 0;
   else
      return priv->swap_count - back->last_swap;

> +  if (!priv->buffers[buf_id]->last_swap)
> +    return 0;

And here.

> +  return priv->swap_count - priv->buffers[buf_id]->last_swap;
> +}
> +
>  /** dri3_open
>   *
>   * Wrapper around xcb_dri3_open
> @@ -1742,6 +1756,9 @@ dri3_create_screen(int screen, struct glx_display
> * priv)
>     psp->copySubBuffer = dri3_copy_sub_buffer;
>     __glXEnableDirectExtension(&psc->base, "GLX_MESA_copy_sub_buffer");
> 
> +   psp->queryBufferAge = dri3_query_buffer_age;
> +   __glXEnableDirectExtension(&psc->base, "GLX_EXT_buffer_age");
> +
>     free(driverName);
>     free(deviceName);
> 
> diff --git a/src/glx/dri3_priv.h b/src/glx/dri3_priv.h
> index 1d124f8..d00440a 100644
> --- a/src/glx/dri3_priv.h
> +++ b/src/glx/dri3_priv.h
> @@ -97,6 +97,7 @@ struct dri3_buffer {
>     uint32_t     cpp;
>     uint32_t     flags;
>     uint32_t     width, height;
> +   uint32_t     last_swap;
> 
>     enum dri3_buffer_type        buffer_type;
>  };
> @@ -184,6 +185,7 @@ struct dri3_drawable {
>     struct dri3_buffer *buffers[DRI3_NUM_BUFFERS];
>     int cur_back;
>     int num_back;
> +   uint32_t swap_count;
> 
>     uint32_t *stamp;
> 
> diff --git a/src/glx/glx_pbuffer.c b/src/glx/glx_pbuffer.c
> index 411d6e5..a87a0a4 100644
> --- a/src/glx/glx_pbuffer.c
> +++ b/src/glx/glx_pbuffer.c
> @@ -365,6 +365,13 @@ GetDrawableAttribute(Display * dpy, GLXDrawable
> drawable,
>  #if defined(GLX_DIRECT_RENDERING) && !defined(GLX_USE_APPLEGL)
>           {
>              __GLXDRIdrawable *pdraw = GetGLXDRIDrawable(dpy, drawable);
> +            struct glx_screen *psc = pdraw->psc;

Dereference pdraw...

> +
> +            if (attribute == GLX_BACK_BUFFER_AGE_EXT && pdraw != NULL &&

...then check whether it's NULL. tsk tsk. :)

I think the declaration of pdraw (with the #ifdef stuff) should get
moved up to the previous else (just before calling _XRead).  Then this
block could be 'if (pdraw != NULL) {" instead of just "{".  Then you can
delete the other 'pdraw != NULL' checks (below).  That refactor should
go in a separate patch.

> +                psc->driScreen->queryBufferAge != NULL) {
> +
> +                *value = psc->driScreen->queryBufferAge (pdraw);
                                                          ^
                                             No space here.

> +            }
> 
>              if (pdraw != NULL && !pdraw->textureTarget)
>                 pdraw->textureTarget =
> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
> index a7118af..9333bdf 100644
> --- a/src/glx/glxclient.h
> +++ b/src/glx/glxclient.h
> @@ -125,6 +125,7 @@ struct __GLXDRIscreenRec {
>               int64_t *msc, int64_t *sbc);
>     int (*setSwapInterval)(__GLXDRIdrawable *pdraw, int interval);
>     int (*getSwapInterval)(__GLXDRIdrawable *pdraw);
> +   int (*queryBufferAge)(__GLXDRIdrawable *pdraw);

All of the other query functions here are getSomething.  This should be
getBufferAge for consistency.

>  };
> 
>  struct __GLXDRIdrawableRec
> diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
> index f186c13..78c4a8d 100644
> --- a/src/glx/glxextensions.c
> +++ b/src/glx/glxextensions.c
> @@ -103,6 +103,7 @@ static const struct extension_info
> known_glx_extensions[] = {
>     { GLX(SGIX_visual_select_group),    VER(0,0), Y, Y, N, N },
>     { GLX(EXT_texture_from_pixmap),     VER(0,0), Y, N, N, N },
>     { GLX(INTEL_swap_event),            VER(0,0), Y, N, N, N },
> +   { GLX(EXT_buffer_age),              VER(1,4), Y, N, Y, Y },
                                              ^^^
                                              0,0

Or GLX 1.4 won't be advertised on drivers that don't support this extension.

>     { NULL }
>  };
> 
> diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h
> index 8436a94..37e4ccc 100644
> --- a/src/glx/glxextensions.h
> +++ b/src/glx/glxextensions.h
> @@ -66,6 +66,7 @@ enum
>     SGIX_visual_select_group_bit,
>     EXT_texture_from_pixmap_bit,
>     INTEL_swap_event_bit,
> +   EXT_buffer_age_bit,
>  };
> 
>  /* From the GLX perspective, the ARB and EXT extensions are identical. 
> Use a
> diff --git a/src/mesa/drivers/x11/glxapi.c b/src/mesa/drivers/x11/glxapi.c
> index dfa8946..af3d208 100644
> --- a/src/mesa/drivers/x11/glxapi.c
> +++ b/src/mesa/drivers/x11/glxapi.c
> @@ -1181,6 +1181,9 @@ _glxapi_get_extensions(void)
>  #ifdef GLX_INTEL_swap_event
>        "GLX_INTEL_swap_event",
>  #endif
> +#ifdef GLX_INTEL_buffer_age
> +      "GLX_INTEL_buffer_age",
> +#endif

This hunk shouldn't exist.  GLX_INTEL_swap_event should be listed here
either.  This is a fake GLX layer for a software rasterizer build of
Mesa.  These aren't the droids you're looking for...

>        NULL
>     };
>     return extensions;

I think you're going to end up with 3 paches:

1. Import new glxext.h.
2. Refactor the pdraw != NULL stuff, delete the stray NULL checks, etc.
3. Implement GLX_EXT_buffer_age



More information about the mesa-dev mailing list