[PATCH 4/5] glx: Implement GLX_PRESERVED_CONTENTS drawable attribute
Ian Romanick
idr at freedesktop.org
Thu Feb 28 13:55:41 PST 2013
On 02/25/2013 02:04 PM, Adam Jackson wrote:
> We back pixmaps with pbuffers so they're never actually clobbered, so
> we're really just recording the value passed in at create time so that
> you get the same thing when you query.
Is that actually right? There are some cases where they query returns
the actual state, and there are some cases where the query returns the
apps setting. I haven't checked to see which this is. In Mesa, we've
taken to quoting the spec in cases like this.
> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
> glx/glxcmds.c | 90 +++++++++++++++++++++++++++++++++----------------------
> glx/glxdrawable.h | 1 +
> 2 files changed, 55 insertions(+), 36 deletions(-)
>
> diff --git a/glx/glxcmds.c b/glx/glxcmds.c
> index 192c73f..41313f2 100644
> --- a/glx/glxcmds.c
> +++ b/glx/glxcmds.c
> @@ -1378,13 +1378,43 @@ __glXDisp_DestroyPixmap(__GLXclientState * cl, GLbyte * pc)
> }
>
> static int
> +DoChangeDrawableAttributes(ClientPtr client, XID glxdrawable,
> + int numAttribs, CARD32 *attribs)
> +{
> + __GLXdrawable *pGlxDraw;
> + int i, err;
> +
> + if (!validGlxDrawable(client, glxdrawable, GLX_DRAWABLE_ANY,
> + DixSetAttrAccess, &pGlxDraw, &err))
> + return err;
> +
> + for (i = 0; i < numAttribs; i++) {
> + switch (attribs[i * 2]) {
> + case GLX_EVENT_MASK:
> + /* All we do is to record the event mask so we can send it
> + * back when queried. We never actually clobber the
> + * pbuffers, so we never need to send out the event. */
> + pGlxDraw->eventMask = attribs[i * 2 + 1];
> + break;
> + case GLX_PRESERVED_CONTENTS:
> + /* same deal */
> + pGlxDraw->preserved = attribs[i * 2 + 1];
> + break;
> + }
> + }
> +
> + return Success;
> +}
> +
> +static int
> DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
> - int width, int height, XID glxDrawableId)
> + int width, int height, XID glxDrawableId, int preserved)
> {
> __GLXconfig *config;
> __GLXscreen *pGlxScreen;
> PixmapPtr pPixmap;
> int err;
> + CARD32 attribs[2] = { GLX_PRESERVED_CONTENTS, preserved };
>
> LEGAL_NEW_RESOURCE(glxDrawableId, client);
>
> @@ -1406,9 +1436,13 @@ DoCreatePbuffer(ClientPtr client, int screenNum, XID fbconfigId,
> if (!AddResource(pPixmap->drawable.id, RT_PIXMAP, pPixmap))
> return BadAlloc;
>
> - return DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable,
> - glxDrawableId, glxDrawableId,
> - GLX_DRAWABLE_PBUFFER);
> + err = DoCreateGLXDrawable(client, pGlxScreen, config, &pPixmap->drawable,
> + glxDrawableId, glxDrawableId,
> + GLX_DRAWABLE_PBUFFER);
> + if (err != Success)
> + return err;
> +
> + return DoChangeDrawableAttributes(serverClient, glxDrawableId, 1, attribs);
> }
>
> int
> @@ -1417,7 +1451,7 @@ __glXDisp_CreatePbuffer(__GLXclientState * cl, GLbyte * pc)
> ClientPtr client = cl->client;
> xGLXCreatePbufferReq *req = (xGLXCreatePbufferReq *) pc;
> CARD32 *attrs;
> - int width, height, i;
> + int i, width = 0, height = 0, preserved = 1;
>
> REQUEST_AT_LEAST_SIZE(xGLXCreatePbufferReq);
> if (req->numAttribs > (UINT32_MAX >> 3)) {
> @@ -1427,8 +1461,6 @@ __glXDisp_CreatePbuffer(__GLXclientState * cl, GLbyte * pc)
> REQUEST_FIXED_SIZE(xGLXCreatePbufferReq, req->numAttribs << 3);
>
> attrs = (CARD32 *) (req + 1);
> - width = 0;
> - height = 0;
>
> for (i = 0; i < req->numAttribs; i++) {
> switch (attrs[i * 2]) {
> @@ -1438,15 +1470,17 @@ __glXDisp_CreatePbuffer(__GLXclientState * cl, GLbyte * pc)
> case GLX_PBUFFER_HEIGHT:
> height = attrs[i * 2 + 1];
> break;
> - case GLX_LARGEST_PBUFFER:
> case GLX_PRESERVED_CONTENTS:
> + preserved = attrs[i * 2 + 1];
> + break;
> + case GLX_LARGEST_PBUFFER:
> /* FIXME: huh... */
> break;
> }
> }
>
> return DoCreatePbuffer(cl->client, req->screen, req->fbconfig,
> - width, height, req->pbuffer);
> + width, height, req->pbuffer, preserved);
> }
>
> int
> @@ -1457,8 +1491,12 @@ __glXDisp_CreateGLXPbufferSGIX(__GLXclientState * cl, GLbyte * pc)
>
> REQUEST_AT_LEAST_SIZE(xGLXCreateGLXPbufferSGIXReq);
>
> + /*
> + * We should really handle attributes correctly, but this extension
> + * is so rare I have difficulty caring.
> + */
> return DoCreatePbuffer(cl->client, req->screen, req->fbconfig,
> - req->width, req->height, req->pbuffer);
> + req->width, req->height, req->pbuffer, 1);
> }
>
> int
> @@ -1483,31 +1521,6 @@ __glXDisp_DestroyGLXPbufferSGIX(__GLXclientState * cl, GLbyte * pc)
> return DoDestroyDrawable(cl, req->pbuffer, GLX_DRAWABLE_PBUFFER);
> }
>
> -static int
> -DoChangeDrawableAttributes(ClientPtr client, XID glxdrawable,
> - int numAttribs, CARD32 *attribs)
> -{
> - __GLXdrawable *pGlxDraw;
> - int i, err;
> -
> - if (!validGlxDrawable(client, glxdrawable, GLX_DRAWABLE_ANY,
> - DixSetAttrAccess, &pGlxDraw, &err))
> - return err;
> -
> - for (i = 0; i < numAttribs; i++) {
> - switch (attribs[i * 2]) {
> - case GLX_EVENT_MASK:
> - /* All we do is to record the event mask so we can send it
> - * back when queried. We never actually clobber the
> - * pbuffers, so we never need to send out the event. */
> - pGlxDraw->eventMask = attribs[i * 2 + 1];
> - break;
> - }
> - }
> -
> - return Success;
> -}
> -
> int
> __glXDisp_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc)
> {
> @@ -1874,7 +1887,7 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID drawId)
> ClientPtr client = cl->client;
> xGLXGetDrawableAttributesReply reply;
> __GLXdrawable *pGlxDraw;
> - CARD32 attributes[12];
> + CARD32 attributes[14];
> int numAttribs = 0, error;
>
> if (!validGlxDrawable(client, drawId, GLX_DRAWABLE_ANY,
> @@ -1900,6 +1913,11 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID drawId)
> attributes[10] = GLX_FBCONFIG_ID;
> attributes[11] = pGlxDraw->config->fbconfigID;
> numAttribs++;
> + if (pGlxDraw->type == GLX_DRAWABLE_PBUFFER) {
> + attributes[12] = GLX_PRESERVED_CONTENTS;
> + attributes[13] = pGlxDraw->preserved;
> + numAttribs++;
> + }
I wonder if this should be something like
attributes[i++] = GLX_FBCONFIG_ID;
attributes[i++] = pGlxDraw->config->fbconfigID;
if (pGlxDraw->type == GLX_DRAWABLE_PBUFFER) {
attributes[i++] = GLX_PRESERVED_CONTENTS;
attributes[i++] = pGlxDraw->preserved;
}
numAttribs = i / 2;
That makes it more future-proof when we add another attribute. It's not
a deal breaker for me either way.
>
> reply = (xGLXGetDrawableAttributesReply) {
> .type = X_Reply,
> diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h
> index 0076589..4cbf43d 100644
> --- a/glx/glxdrawable.h
> +++ b/glx/glxdrawable.h
> @@ -72,6 +72,7 @@ struct __GLXdrawable {
> ** Event mask
> */
> unsigned long eventMask;
> + int preserved;
> };
>
> #endif /* !__GLX_drawable_h__ */
>
More information about the xorg-devel
mailing list