[Mesa-dev] [PATCH 03/25] glx: move dri2_convert_glx_query_renderer_attribs into a common place

Fri Feb 28 17:39:54 PST 2014

On 28/02/14 19:27, Ian Romanick wrote:
> On 02/21/2014 07:03 PM, Emil Velikov wrote:
>> This way we can reuse it for drisw and dri3.
>> Squash a couple minor whitespace fixes while we're here.
> 
> Now I remember why I put this all in its own file... there are unit
> tests in src/glx/tests that link with these functions.  If they have to
> link with something from dri_common.c, libxcb get pulled in, and the
> link E X P L O D E S. :(  For a good time, call "make check" with this
> patch applied.
> 
Strange I seem to get a ton of duplicated symbols due to redefinition of
functions within the tests. Which is sigh...

/**
 * \name Indirect-rendering function stubs
 *
 * These are necessary so that indirect_init.c doesn't try to link with
 * indirect.c.  Linking with indirect.c would require linking with various X
 * libraries and a bunch of other stuff.  While this is ugly, it does
simplify
 * the build quite a bit.
 */

Does the tests actually call those functions ? If not, one can link with
indirect.c and leave to extra deps (X etc.) as undefined.

This will remove these stubs and should resolve our problems in a better
fashion.

> Given that, I like my "dri3: Enable GLX_MESA_query_renderer on DRI3 too"
> patch better. :)  Alternately, we could leave
> dri2_convert_glx_query_renderer_attribs in this file, but make it public
> (and rename it dri_convert_glx_query_renderer_attribs).
> 
> Thoughts / opinions?
> 
The major hit with your approach is that it additionally convolutes the
header inclusions. I have a some local patches that clears up some of
the unnecessary ones and I suspect that there are few more needed.

If you're going with "keep all the queries in dri2_query_renderer.c"
approach please move all the declarations out of dri2.h into a separate
header and keep the convert function at it's current place.

IMHO dri2_query_renderer.c sounds a bit ambiguous as it will be used by
dri2, dri3, and drisw, one might want to rename that and please use
designated initialisers for glx_screen_vtable.

I'm possibly pushing it with the last two but please take the header
business closer to heart.

Thanks
-Emil

>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>   src/glx/dri2_query_renderer.c | 34 +---------------------------------
>>   src/glx/dri_common.c          | 40
>> ++++++++++++++++++++++++++++++++++++----
>>   src/glx/dri_common.h          |  3 +++
>>   3 files changed, 40 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/glx/dri2_query_renderer.c
>> b/src/glx/dri2_query_renderer.c
>> index 3baba86..5d66064 100644
>> --- a/src/glx/dri2_query_renderer.c
>> +++ b/src/glx/dri2_query_renderer.c
>> @@ -25,41 +25,9 @@
>>   #include "xf86drm.h"
>>   #include "dri2.h"
>>   #include "dri_interface.h"
>> +#include "dri_common.h"
>>   #include "dri2_priv.h"
>>
>> -#define __RENDERER(attrib) \
>> -    { GLX_RENDERER_##attrib##_MESA, __DRI2_RENDERER_##attrib }
>> -
>> -static const struct {
>> -   unsigned int glx_attrib, dri2_attrib;
>> -} query_renderer_map[] = {
>> -  __RENDERER(VENDOR_ID),
>> -  __RENDERER(DEVICE_ID),
>> -  __RENDERER(VERSION),
>> -  __RENDERER(ACCELERATED),
>> -  __RENDERER(VIDEO_MEMORY),
>> -  __RENDERER(UNIFIED_MEMORY_ARCHITECTURE),
>> -  __RENDERER(PREFERRED_PROFILE),
>> -  __RENDERER(OPENGL_CORE_PROFILE_VERSION),
>> -  __RENDERER(OPENGL_COMPATIBILITY_PROFILE_VERSION),
>> -  __RENDERER(OPENGL_ES_PROFILE_VERSION),
>> -  __RENDERER(OPENGL_ES2_PROFILE_VERSION),
>> -};
>> -
>> -#undef __RENDERER
>> -
>> -static int
>> -dri2_convert_glx_query_renderer_attribs(int attribute)
>> -{
>> -   int i;
>> -
>> -   for (i = 0; i < ARRAY_SIZE(query_renderer_map); i++)
>> -      if (query_renderer_map[i].glx_attrib == attribute)
>> -         return query_renderer_map[i].dri2_attrib;
>> -
>> -   return -1;
>> -}
>> -
>>   _X_HIDDEN int
>>   dri2_query_renderer_integer(struct glx_screen *base, int attribute,
>>                               unsigned int *value)
>> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
>> index 012c8f4..fb70107 100644
>> --- a/src/glx/dri_common.c
>> +++ b/src/glx/dri_common.c
>> @@ -204,7 +204,7 @@ static const struct
>>   {
>>      unsigned int attrib, offset;
>>   } attribMap[] = {
>> -   __ATTRIB(__DRI_ATTRIB_BUFFER_SIZE, rgbBits),
>> +      __ATTRIB(__DRI_ATTRIB_BUFFER_SIZE, rgbBits),
>>         __ATTRIB(__DRI_ATTRIB_LEVEL, level),
>>         __ATTRIB(__DRI_ATTRIB_RED_SIZE, redBits),
>>         __ATTRIB(__DRI_ATTRIB_GREEN_SIZE, greenBits),
>> @@ -241,10 +241,9 @@ static const struct
>>   #if 0
>>         __ATTRIB(__DRI_ATTRIB_SWAP_METHOD, swapMethod),
>>   #endif
>> -__ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb),
>> +      __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb),
>>         __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGBA, bindToTextureRgba),
>> -      __ATTRIB(__DRI_ATTRIB_BIND_TO_MIPMAP_TEXTURE,
>> -                     bindToMipmapTexture),
>> +      __ATTRIB(__DRI_ATTRIB_BIND_TO_MIPMAP_TEXTURE,
>> bindToMipmapTexture),
>>         __ATTRIB(__DRI_ATTRIB_YINVERTED, yInverted),
>>         __ATTRIB(__DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE, sRGBCapable)
>>   };
>> @@ -593,4 +592,37 @@ dri2_convert_glx_attribs(unsigned num_attribs,
>> const uint32_t *attribs,
>>      return true;
>>   }
>>
>> +#define __RENDERER(attrib) \
>> +    { GLX_RENDERER_##attrib##_MESA, __DRI2_RENDERER_##attrib }
>> +
>> +static const struct {
>> +   unsigned int glx_attrib, dri2_attrib;
>> +} query_renderer_map[] = {
>> +  __RENDERER(VENDOR_ID),
>> +  __RENDERER(DEVICE_ID),
>> +  __RENDERER(VERSION),
>> +  __RENDERER(ACCELERATED),
>> +  __RENDERER(VIDEO_MEMORY),
>> +  __RENDERER(UNIFIED_MEMORY_ARCHITECTURE),
>> +  __RENDERER(PREFERRED_PROFILE),
>> +  __RENDERER(OPENGL_CORE_PROFILE_VERSION),
>> +  __RENDERER(OPENGL_COMPATIBILITY_PROFILE_VERSION),
>> +  __RENDERER(OPENGL_ES_PROFILE_VERSION),
>> +  __RENDERER(OPENGL_ES2_PROFILE_VERSION),
>> +};
>> +
>> +#undef __RENDERER
>> +
>> +_X_HIDDEN int
>> +dri2_convert_glx_query_renderer_attribs(int attribute)
>> +{
>> +   int i;
>> +
>> +   for (i = 0; i < ARRAY_SIZE(query_renderer_map); i++)
>> +      if (query_renderer_map[i].glx_attrib == attribute)
>> +         return query_renderer_map[i].dri2_attrib;
>> +
>> +   return -1;
>> +}
>> +
>>   #endif /* GLX_DIRECT_RENDERING */
>> diff --git a/src/glx/dri_common.h b/src/glx/dri_common.h
>> index 6234fd8..fc3b660 100644
>> --- a/src/glx/dri_common.h
>> +++ b/src/glx/dri_common.h
>> @@ -85,4 +85,7 @@ dri2_convert_glx_attribs(unsigned num_attribs, const
>> uint32_t *attribs,
>>                            uint32_t *render_type, uint32_t *flags,
>> unsigned *api,
>>                            int *reset, unsigned *error);
>>
>> +extern int
>> +dri2_convert_glx_query_renderer_attribs(int attribute);
>> +
>>   #endif /* _DRI_COMMON_H */
>>
> 