Re: [PATCH xserver] glx: Fix attribute handling for GLX_TEXTURE_FORMAT_EXT

[Eric Anholt]

Adam Jackson  writes:

> On Mon, 2018-04-02 at 14:52 -0400, Adam Jackson wrote:
>
>> @@ -1737,6 +1740,9 @@ __glXDisp_BindTexImageEXT(__GLXclientState * cl, 
>> GLbyte * pc)
>>DixReadAccess, , ))
>>  return error;
>>  
>> +if (pGlxDraw->format == GLX_TEXTURE_FORMAT_NONE_EXT)
>> +return BadMatch;
>> +
>>  if (!context->bindTexImage)
>>  return __glXError(GLXUnsupportedPrivateRequest);
>> 
>
> The conditional added here is correct in a specification sense, but the
> behavior change would be subtle. As it stands you can do
> glXBindTexImage on _any_ GLX pixmap, even one created without a
> texture-target attribute; due to accident of how various conditionals
> are written it looks like such pixmaps would implicitly be treated as
> RGBA. I'm not aware of any apps that do that and I don't think they
> would work on non-Mesa drivers, but we could instead set a default
> texture target derived from (the number of alpha bits of) the requested
> fbconfig and it would probably do the right thing.

Hmm, I don't see any required default value for GLX_TEXTURE_FORMAT in
the spec.  I'm also concerned that we're going to break some backwards
compat here, so your alpha bits plan sounds pretty decent.

Also, you need to bump the attributes by 2, not 1, when adding a new
attribute to output. :)


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver] glx: Fix attribute handling for GLX_TEXTURE_FORMAT_EXT

[Adam Jackson]

On Mon, 2018-04-02 at 14:52 -0400, Adam Jackson wrote:

> @@ -1737,6 +1740,9 @@ __glXDisp_BindTexImageEXT(__GLXclientState * cl, GLbyte 
> * pc)
>DixReadAccess, , ))
>  return error;
>  
> +if (pGlxDraw->format == GLX_TEXTURE_FORMAT_NONE_EXT)
> +return BadMatch;
> +
>  if (!context->bindTexImage)
>  return __glXError(GLXUnsupportedPrivateRequest);
> 

The conditional added here is correct in a specification sense, but the
behavior change would be subtle. As it stands you can do
glXBindTexImage on _any_ GLX pixmap, even one created without a
texture-target attribute; due to accident of how various conditionals
are written it looks like such pixmaps would implicitly be treated as
RGBA. I'm not aware of any apps that do that and I don't think they
would work on non-Mesa drivers, but we could instead set a default
texture target derived from (the number of alpha bits of) the requested
fbconfig and it would probably do the right thing.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver] glx: Fix attribute handling for GLX_TEXTURE_FORMAT_EXT

[Adam Jackson]

Initialize it correctly to GLX_TEXTURE_FORMAT_NONE_EXT, and report it in
GLXGetDrawableAttributes. Note that it and TEXTURE_TARGET are only
meaningful attributes for GLXPixmaps.

Signed-off-by: Adam Jackson 
---
 glx/glxcmds.c | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 6785e9db38..847271e9e3 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1173,6 +1173,9 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen * 
pGlxScreen,
 !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw))
 return BadAlloc;
 
+if (type == GLX_DRAWABLE_PIXMAP)
+pGlxDraw->format = GLX_TEXTURE_FORMAT_NONE_EXT;
+
 return Success;
 }
 
@@ -1737,6 +1740,9 @@ __glXDisp_BindTexImageEXT(__GLXclientState * cl, GLbyte * 
pc)
   DixReadAccess, , ))
 return error;
 
+if (pGlxDraw->format == GLX_TEXTURE_FORMAT_NONE_EXT)
+return BadMatch;
+
 if (!context->bindTexImage)
 return __glXError(GLXUnsupportedPrivateRequest);
 
@@ -1851,7 +1857,7 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID drawId)
 xGLXGetDrawableAttributesReply reply;
 __GLXdrawable *pGlxDraw = NULL;
 DrawablePtr pDraw;
-CARD32 attributes[18];
+CARD32 attributes[19];
 int num = 0, error;
 
 if (!validGlxDrawable(client, drawId, GLX_DRAWABLE_ANY,
@@ -1876,11 +1882,14 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID 
drawId)
 ATTRIB(GLX_HEIGHT, pDraw->height);
 ATTRIB(GLX_SCREEN, pDraw->pScreen->myNum);
 if (pGlxDraw) {
-ATTRIB(GLX_TEXTURE_TARGET_EXT,
-   pGlxDraw->target == GL_TEXTURE_2D ?
-GLX_TEXTURE_2D_EXT : GLX_TEXTURE_RECTANGLE_EXT);
 ATTRIB(GLX_EVENT_MASK, pGlxDraw->eventMask);
 ATTRIB(GLX_FBCONFIG_ID, pGlxDraw->config->fbconfigID);
+if (pGlxDraw->type == GLX_DRAWABLE_PIXMAP) {
+ATTRIB(GLX_TEXTURE_TARGET_EXT,
+   pGlxDraw->target == GL_TEXTURE_2D ?
+GLX_TEXTURE_2D_EXT : GLX_TEXTURE_RECTANGLE_EXT);
+ATTRIB(GLX_TEXTURE_FORMAT_EXT, pGlxDraw->format);
+}
 if (pGlxDraw->type == GLX_DRAWABLE_PBUFFER) {
 ATTRIB(GLX_PRESERVED_CONTENTS, GL_TRUE);
 }
-- 
2.16.2

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel