mesa3d-dev

[Mesa3d-dev] [PATCH] glx: Fix config chooser logic for 'mask' matching

[Kristian H. <kr...@bi...>]

When matching attributes using the 'mask' matching criteria, the spec
says that

  "Only GLXFBConfigs for which the set bits of attribute include all
   the bits that are set in the requested value are
   considered. (Additional bits might be set in the attribute)."

The current test returns true if the two bit masks have bits in
common, specifically it matches even if the requested value has bits
set that are not set in the fbconfig attribute.  For example, an
application asking for

  GLX_DRAWABLE_TYPE, GLX_PIXMAP_BIT | GLX_PBUFFER_BIT,

as glxpbdemo does, will match fbconfigs that don't support pbuffer
rendering, as long as they support pixmap rendering.
---
 src/glx/glxcmds.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index 7cdd42c..e256a07 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -1110,6 +1110,13 @@ init_fbconfig_for_chooser(__GLcontextModes * config,
     }                                           \
   } while ( 0 )
 
+/* Test that all bits from a are contained in b */
+#define MATCH_MASK(param)			\
+  do {						\
+    if ((a->param & ~b->param) != 0)		\
+      return False;				\
+  } while (0);
+
 /**
  * Determine if two GLXFBConfigs are compatible.
  *
@@ -1148,11 +1155,8 @@ fbconfigs_compatible(const __GLcontextModes * const a,
    MATCH_DONT_CARE(stereoMode);
    MATCH_EXACT(level);
 
-   if (((a->drawableType & b->drawableType) == 0)
-       || ((a->renderType & b->renderType) == 0)) {
-      return False;
-   }
-
+   MATCH_MASK(drawableType);
+   MATCH_MASK(renderType);
 
    /* There is a bug in a few of the XFree86 DDX drivers.  They contain
     * visuals with a "transparent type" of 0 when they really mean GLX_NONE.
-- 
1.7.0.1

Re: [Mesa3d-dev] [PATCH] glx: Fix config chooser logic for 'mask' matching

[Ian R. <id...@fr...>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Kristian Høgsberg wrote:
> When matching attributes using the 'mask' matching criteria, the spec
> says that
> 
>   "Only GLXFBConfigs for which the set bits of attribute include all
>    the bits that are set in the requested value are
>    considered. (Additional bits might be set in the attribute)."
> 
> The current test returns true if the two bit masks have bits in
> common, specifically it matches even if the requested value has bits
> set that are not set in the fbconfig attribute.  For example, an
> application asking for
> 
>   GLX_DRAWABLE_TYPE, GLX_PIXMAP_BIT | GLX_PBUFFER_BIT,
> 
> as glxpbdemo does, will match fbconfigs that don't support pbuffer
> rendering, as long as they support pixmap rendering.

Reviewed-by: Ian Romanick <ian...@in...>

This should go into master, 7.8, and mesa_7_7_branch.  Are there any
specific bugs associated with this?  If so, those should be documented
in the commit message.

> ---
>  src/glx/glxcmds.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
> index 7cdd42c..e256a07 100644
> --- a/src/glx/glxcmds.c
> +++ b/src/glx/glxcmds.c
> @@ -1110,6 +1110,13 @@ init_fbconfig_for_chooser(__GLcontextModes * config,
>      }                                           \
>    } while ( 0 )
>  
> +/* Test that all bits from a are contained in b */
> +#define MATCH_MASK(param)			\
> +  do {						\
> +    if ((a->param & ~b->param) != 0)		\
> +      return False;				\
> +  } while (0);
> +
>  /**
>   * Determine if two GLXFBConfigs are compatible.
>   *
> @@ -1148,11 +1155,8 @@ fbconfigs_compatible(const __GLcontextModes * const a,
>     MATCH_DONT_CARE(stereoMode);
>     MATCH_EXACT(level);
>  
> -   if (((a->drawableType & b->drawableType) == 0)
> -       || ((a->renderType & b->renderType) == 0)) {
> -      return False;
> -   }
> -
> +   MATCH_MASK(drawableType);
> +   MATCH_MASK(renderType);
>  
>     /* There is a bug in a few of the XFree86 DDX drivers.  They contain
>      * visuals with a "transparent type" of 0 when they really mean GLX_NONE.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAku/d2QACgkQX1gOwKyEAw/6gACfcz1EfbxKymUy90FCCQjxyDp8
OyMAn0mFF4f8OOn+jPnwzKrHz5m0capG
=pndz
-----END PGP SIGNATURE-----

Re: [Mesa3d-dev] [PATCH] glx: Fix config chooser logic for 'mask' matching

[Kristian H. <kr...@bi...>]

2010/4/9 Ian Romanick <id...@fr...>:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Kristian Høgsberg wrote:
>> When matching attributes using the 'mask' matching criteria, the spec
>> says that
>>
>>   "Only GLXFBConfigs for which the set bits of attribute include all
>>    the bits that are set in the requested value are
>>    considered. (Additional bits might be set in the attribute)."
>>
>> The current test returns true if the two bit masks have bits in
>> common, specifically it matches even if the requested value has bits
>> set that are not set in the fbconfig attribute.  For example, an
>> application asking for
>>
>>   GLX_DRAWABLE_TYPE, GLX_PIXMAP_BIT | GLX_PBUFFER_BIT,
>>
>> as glxpbdemo does, will match fbconfigs that don't support pbuffer
>> rendering, as long as they support pixmap rendering.
>
> Reviewed-by: Ian Romanick <ian...@in...>

Thanks.

> This should go into master, 7.8, and mesa_7_7_branch.

Done.

> Are there any
> specific bugs associated with this?  If so, those should be documented
> in the commit message.

Not that I know.  I just ran into it when trying to run glxpbdemo.

Kristian