[Mesa-dev] [PATCH 03/16] main: store the common option in a struct of the Const part of the GL context

Martin Peres martin.peres at linux.intel.com
Fri Jun 5 07:59:02 PDT 2015


On 05/06/15 16:53, Brian Paul wrote:
> This is a big patch that touches a lot of files all over the place.  I 
> wonder if it could be broken up a bit.  In any case, comments below...

Thanks for the quick review, Brian!

I tried to keep the diff minimal but aside from the empty declarations 
and new files, I cannot take anything out. I will do that!

>
>
> On 06/05/2015 07:03 AM, Martin Peres wrote:
>> This will allow us to share common drirc options across drivers while
>> making sure some drivers are not left behind (current situation).
>>
>> This changes _mesa_initialize_context to make sure that every driver
>> sets the shared options either to the default values or the
>> drirc-provided options.
>>
>> Signed-off-by: Martin Peres <martin.peres at linux.intel.com>
>> ---
>>   src/gallium/include/state_tracker/st_api.h         |  4 ++
>>   src/gallium/state_trackers/dri/dri_screen.c        |  4 +-
>>   src/gallium/state_trackers/osmesa/osmesa.c         |  1 +
>>   src/mesa/Makefile.sources                          |  2 +
>>   src/mesa/drivers/dri/i915/intel_context.c          | 11 +++-
>>   src/mesa/drivers/dri/i965/brw_context.c            |  9 ++-
>>   src/mesa/drivers/dri/nouveau/nouveau_context.c     |  5 +-
>>   .../drivers/dri/radeon/radeon_common_context.c     |  6 ++
>>   src/mesa/drivers/dri/swrast/swrast.c               |  5 +-
>>   src/mesa/drivers/osmesa/osmesa.c                   |  5 ++
>>   src/mesa/drivers/x11/xm_api.c                      |  5 +-
>>   src/mesa/main/context.c                            | 17 ++++--
>>   src/mesa/main/context.h                            |  5 +-
>>   src/mesa/main/mtypes.h                             |  4 ++
>>   src/mesa/main/shared_options.c                     | 33 ++++++++++
>>   src/mesa/main/shared_options.h                     | 71 
>> ++++++++++++++++++++++
>>   src/mesa/main/tests/dispatch_sanity.cpp            |  6 ++
>>   src/mesa/state_tracker/st_context.c                |  4 +-
>>   src/mesa/state_tracker/st_manager.c                |  2 +-
>>   19 files changed, 182 insertions(+), 17 deletions(-)
>>   create mode 100644 src/mesa/main/shared_options.c
>>   create mode 100644 src/mesa/main/shared_options.h
>>
>> diff --git a/src/gallium/include/state_tracker/st_api.h 
>> b/src/gallium/include/state_tracker/st_api.h
>> index ecf1c07..eca3d50 100644
>> --- a/src/gallium/include/state_tracker/st_api.h
>> +++ b/src/gallium/include/state_tracker/st_api.h
>> @@ -29,6 +29,7 @@
>>
>>   #include "pipe/p_compiler.h"
>>   #include "pipe/p_format.h"
>> +#include "main/shared_options.h"
>>
>>   /**
>>    * \file API for communication between state trackers and state 
>> tracker
>> @@ -240,6 +241,9 @@ struct st_visual
>>    */
>>   struct st_config_options
>>   {
>> +   /** The screen's effective configuration options */
>> +   struct shared_options shared_options;
>> +
>
> This an interface violation.  The gallium state tracker API should 
> have no dependencies on core Mesa.
>
> Unfortunately, I think the config options for the state tracker API 
> and core Mesa will have to be in separate, though very similar, 
> structures.
>
> The various gallium state trackers will populate st_config_options and 
> pass them to the Mesa state tracker where they'd be translated into 
> the core Mesa options structure.

Unifying the dri drivers is a noble goal but I really want to unify all 
the drivers. This is why I created the two separate files in main :)

Can core Mesa depend on a gallium-exported header?

>
>
>>      boolean disable_blend_func_extended;
>>      boolean disable_glsl_line_continuations;
>>      boolean disable_shader_bit_encoding;
>> diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
>> b/src/gallium/state_trackers/dri/dri_screen.c
>> index 85393d8..08affdd 100644
>> --- a/src/gallium/state_trackers/dri/dri_screen.c
>> +++ b/src/gallium/state_trackers/dri/dri_screen.c
>> @@ -82,8 +82,10 @@ const __DRIconfigOptionsExtension 
>> gallium_config_options = {
>>
>>   static void
>>   dri_fill_st_options(struct st_config_options *options,
>> -                    const struct driOptionCache * optionCache)
>> +                    struct driOptionCache *optionCache)
>>   {
>> +   _mesa_shared_options_fill(optionCache, &options->shared_options);
>> +
>>      options->disable_blend_func_extended =
>>         driQueryOptionb(optionCache, "disable_blend_func_extended");
>>      options->disable_glsl_line_continuations =
>> diff --git a/src/gallium/state_trackers/osmesa/osmesa.c 
>> b/src/gallium/state_trackers/osmesa/osmesa.c
>> index 2d5d096..e4cc149 100644
>> --- a/src/gallium/state_trackers/osmesa/osmesa.c
>> +++ b/src/gallium/state_trackers/osmesa/osmesa.c
>> @@ -591,6 +591,7 @@ OSMesaCreateContextExt(GLenum format, GLint 
>> depthBits, GLint stencilBits,
>>      attribs.options.disable_shader_bit_encoding = FALSE;
>>      attribs.options.force_s3tc_enable = FALSE;
>>      attribs.options.force_glsl_version = 0;
>> + _mesa_shared_options_fill_defaults(&attribs.options.shared_options);
>>
>>      osmesa_init_st_visual(&attribs.visual,
>>                            PIPE_FORMAT_R8G8B8A8_UNORM,
>> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
>> index 83f500f..f3cb69c 100644
>> --- a/src/mesa/Makefile.sources
>> +++ b/src/mesa/Makefile.sources
>> @@ -179,6 +179,8 @@ MAIN_FILES = \
>>       main/shader_query.cpp \
>>       main/shared.c \
>>       main/shared.h \
>> +    main/shared_options.c \
>> +    main/shared_options.h \
>>       main/state.c \
>>       main/state.h \
>>       main/stencil.c \
>> diff --git a/src/mesa/drivers/dri/i915/intel_context.c 
>> b/src/mesa/drivers/dri/i915/intel_context.c
>> index 5618dcd..3789b7e 100644
>> --- a/src/mesa/drivers/dri/i915/intel_context.c
>> +++ b/src/mesa/drivers/dri/i915/intel_context.c
>> @@ -34,6 +34,7 @@
>>   #include "main/imports.h"
>>   #include "main/points.h"
>>   #include "main/renderbuffer.h"
>> +#include "main/shared_options.h"
>>
>>   #include "swrast/swrast.h"
>>   #include "swrast_setup/swrast_setup.h"
>> @@ -409,6 +410,7 @@ intelInitContext(struct intel_context *intel,
>>      struct gl_context *shareCtx = (struct gl_context *) 
>> sharedContextPrivate;
>>      __DRIscreen *sPriv = driContextPriv->driScreenPriv;
>>      struct intel_screen *intelScreen = sPriv->driverPrivate;
>> +   struct shared_options shared_options;
>>      int bo_reuse_mode;
>>
>>      /* Can't rely on invalidate events, fall back to glViewport hack */
>> @@ -419,8 +421,13 @@ intelInitContext(struct intel_context *intel,
>>
>>      intel->intelScreen = intelScreen;
>>
>> +   driParseConfigFiles(&intel->optionCache, &intelScreen->optionCache,
>> +                       sPriv->myNum, "i915");
>> +
>> +   _mesa_shared_options_fill(&intel->optionCache, &shared_options);
>> +
>>      if (!_mesa_initialize_context(&intel->ctx, api, mesaVis, shareCtx,
>> -                                 functions)) {
>> +                                 &shared_options, functions)) {
>>         *dri_ctx_error = __DRI_CTX_ERROR_NO_MEMORY;
>>         printf("%s: failed to init mesa context\n", __func__);
>>         return false;
>> @@ -441,8 +448,6 @@ intelInitContext(struct intel_context *intel,
>>      memset(&ctx->TextureFormatSupported,
>>         0, sizeof(ctx->TextureFormatSupported));
>>
>> -   driParseConfigFiles(&intel->optionCache, &intelScreen->optionCache,
>> -                       sPriv->myNum, "i915");
>>      intel->maxBatchSize = 4096;
>>
>>      /* Estimate the size of the mappable aperture into the GTT.  
>> There's an
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
>> b/src/mesa/drivers/dri/i965/brw_context.c
>> index 274a237..1086c9b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> @@ -41,6 +41,7 @@
>>   #include "main/version.h"
>>   #include "main/vtxfmt.h"
>>   #include "main/texobj.h"
>> +#include "main/shared_options.h"
>>
>>   #include "vbo/vbo_context.h"
>>
>> @@ -708,6 +709,7 @@ brwCreateContext(gl_api api,
>>      struct gl_context *shareCtx = (struct gl_context *) 
>> sharedContextPrivate;
>>      struct intel_screen *screen = sPriv->driverPrivate;
>>      const struct brw_device_info *devinfo = screen->devinfo;
>> +   struct shared_options shared_options;
>>      struct dd_function_table functions;
>>
>>      /* Only allow the __DRI_CTX_FLAG_ROBUST_BUFFER_ACCESS flag if 
>> the kernel
>> @@ -779,7 +781,11 @@ brwCreateContext(gl_api api,
>>
>>      struct gl_context *ctx = &brw->ctx;
>>
>> -   if (!_mesa_initialize_context(ctx, api, mesaVis, shareCtx, 
>> &functions)) {
>> +   brw_process_driconf_options(brw);
>> +   _mesa_shared_options_fill(&brw->optionCache, &shared_options);
>> +
>> +   if (!_mesa_initialize_context(ctx, api, mesaVis, shareCtx,
>> +                                 &shared_options, &functions)) {
>>         *dri_ctx_error = __DRI_CTX_ERROR_NO_MEMORY;
>>         fprintf(stderr, "%s: failed to init mesa context\n", __func__);
>>         intelDestroyContext(driContextPriv);
>> @@ -811,7 +817,6 @@ brwCreateContext(gl_api api,
>>
>>      _mesa_meta_init(ctx);
>>
>> -   brw_process_driconf_options(brw);
>>      brw_process_intel_debug_variable(brw);
>>
>>      if (brw->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS))
>> diff --git a/src/mesa/drivers/dri/nouveau/nouveau_context.c 
>> b/src/mesa/drivers/dri/nouveau/nouveau_context.c
>> index 383cb90..f7773a2 100644
>> --- a/src/mesa/drivers/dri/nouveau/nouveau_context.c
>> +++ b/src/mesa/drivers/dri/nouveau/nouveau_context.c
>> @@ -126,6 +126,7 @@ nouveau_context_init(struct gl_context *ctx, 
>> gl_api api,
>>       struct nouveau_context *nctx = to_nouveau_context(ctx);
>>       struct dd_function_table functions;
>>       driOptionCache optionCache;
>> +    struct shared_options shared_options;
>>
>>       int ret;
>>
>> @@ -142,10 +143,12 @@ nouveau_context_init(struct gl_context *ctx, 
>> gl_api api,
>>       /* parse information in __driConfigOptions */
>>       driParseOptionInfo(&optionCache,
>>                  nouveau_vieux_config_options.xml);
>> +    _mesa_shared_options_fill(&optionCache, &shared_options);
>>       driDestroyOptionInfo(&optionCache);
>>
>>       /* Initialize the mesa context. */
>> -    if (!_mesa_initialize_context(ctx, api, visual, share_ctx, 
>> &functions))
>> +    if (!_mesa_initialize_context(ctx, api, visual, share_ctx,
>> +                      &shared_options, &functions))
>>           return GL_FALSE;
>>
>>       nouveau_state_init(ctx);
>> diff --git a/src/mesa/drivers/dri/radeon/radeon_common_context.c 
>> b/src/mesa/drivers/dri/radeon/radeon_common_context.c
>> index 9699dcb..eb475b4 100644
>> --- a/src/mesa/drivers/dri/radeon/radeon_common_context.c
>> +++ b/src/mesa/drivers/dri/radeon/radeon_common_context.c
>> @@ -41,6 +41,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
>> THE SOFTWARE.
>>   #include "main/fbobject.h"
>>   #include "main/renderbuffer.h"
>>   #include "main/state.h"
>> +#include "main/shared_options.h"
>>   #include "util/simple_list.h"
>>   #include "swrast/swrast.h"
>>   #include "swrast_setup/swrast_setup.h"
>> @@ -139,6 +140,7 @@ GLboolean radeonInitContext(radeonContextPtr radeon,
>>       radeonScreenPtr screen = (radeonScreenPtr) (sPriv->driverPrivate);
>>       struct gl_context* ctx;
>>       struct gl_context* shareCtx;
>> +    struct shared_options shared_options;
>>       int fthrottle_mode;
>>
>>       /* Fill in additional standard functions. */
>> @@ -151,8 +153,12 @@ GLboolean radeonInitContext(radeonContextPtr 
>> radeon,
>>       else
>>           shareCtx = NULL;
>>
>> +    _mesa_shared_options_fill(&radeon->optionCache,
>> +                  &shared_options);
>> +
>>       if (!_mesa_initialize_context(&radeon->glCtx, api,
>>                         glVisual, shareCtx,
>> +                      &shared_options,
>>                         functions))
>>           return GL_FALSE;
>>
>> diff --git a/src/mesa/drivers/dri/swrast/swrast.c 
>> b/src/mesa/drivers/dri/swrast/swrast.c
>> index 1522636..1819445 100644
>> --- a/src/mesa/drivers/dri/swrast/swrast.c
>> +++ b/src/mesa/drivers/dri/swrast/swrast.c
>> @@ -768,6 +768,7 @@ dri_create_context(gl_api api,
>>       struct dri_context *share = (struct dri_context 
>> *)sharedContextPrivate;
>>       struct gl_context *mesaCtx = NULL;
>>       struct gl_context *sharedCtx = NULL;
>> +    struct shared_options shared_options;
>>       struct dd_function_table functions;
>>       driOptionCache optionCache;
>>
>> @@ -798,10 +799,12 @@ dri_create_context(gl_api api,
>>
>>       /* parse information in __driConfigOptions */
>>       driParseOptionInfo(&optionCache, swrast_config_options.xml);
>> +    _mesa_shared_options_fill(&optionCache, &shared_options);
>>       driDestroyOptionInfo(&optionCache);
>>
>>       /* basic context setup */
>> -    if (!_mesa_initialize_context(mesaCtx, api, visual, sharedCtx, 
>> &functions)) {
>> +    if (!_mesa_initialize_context(mesaCtx, api, visual, sharedCtx,
>> +                                  &shared_options, &functions)) {
>>       *error = __DRI_CTX_ERROR_NO_MEMORY;
>>       goto context_fail;
>>       }
>> diff --git a/src/mesa/drivers/osmesa/osmesa.c 
>> b/src/mesa/drivers/osmesa/osmesa.c
>> index 022523e..5856486 100644
>> --- a/src/mesa/drivers/osmesa/osmesa.c
>> +++ b/src/mesa/drivers/osmesa/osmesa.c
>> @@ -646,6 +646,7 @@ OSMesaCreateContextExt( GLenum format, GLint 
>> depthBits, GLint stencilBits,
>>                           GLint accumBits, OSMesaContext sharelist )
>>   {
>>      OSMesaContext osmesa;
>> +   struct shared_options shared_options;
>>      struct dd_function_table functions;
>>      GLint rind, gind, bind, aind;
>>      GLint redBits = 0, greenBits = 0, blueBits = 0, alphaBits =0;
>> @@ -735,6 +736,9 @@ OSMesaCreateContextExt( GLenum format, GLint 
>> depthBits, GLint stencilBits,
>>            return NULL;
>>         }
>>
>> +      /* Initialize the shared options with defaults values */
>> +      _mesa_shared_options_fill_defaults(&shared_options);
>> +
>>         /* Initialize device driver function table */
>>         _mesa_init_driver_functions(&functions);
>>         /* override with our functions */
>> @@ -746,6 +750,7 @@ OSMesaCreateContextExt( GLenum format, GLint 
>> depthBits, GLint stencilBits,
>>                                       osmesa->gl_visual,
>>                                       sharelist ? &sharelist->mesa
>>                                                 : (struct gl_context 
>> *) NULL,
>> +                                    &shared_options,
>>                                       &functions)) {
>>            _mesa_destroy_visual( osmesa->gl_visual );
>>            free(osmesa);
>> diff --git a/src/mesa/drivers/x11/xm_api.c 
>> b/src/mesa/drivers/x11/xm_api.c
>> index 65e7ca8..c10aadf 100644
>> --- a/src/mesa/drivers/x11/xm_api.c
>> +++ b/src/mesa/drivers/x11/xm_api.c
>> @@ -72,6 +72,7 @@
>>   #include "main/imports.h"
>>   #include "main/macros.h"
>>   #include "main/renderbuffer.h"
>> +#include "main/shared_options.h"
>>   #include "main/teximage.h"
>>   #include "main/version.h"
>>   #include "main/vtxfmt.h"
>> @@ -888,6 +889,7 @@ XMesaContext XMesaCreateContext( XMesaVisual v, 
>> XMesaContext share_list )
>>      static GLboolean firstTime = GL_TRUE;
>>      XMesaContext c;
>>      struct gl_context *mesaCtx;
>> +   struct shared_options shared_options;
>>      struct dd_function_table functions;
>>      TNLcontext *tnl;
>>
>> @@ -906,9 +908,10 @@ XMesaContext XMesaCreateContext( XMesaVisual v, 
>> XMesaContext share_list )
>>      /* initialize with default driver functions, then plug in XMesa 
>> funcs */
>>      _mesa_init_driver_functions(&functions);
>>      xmesa_init_driver_functions(v, &functions);
>> +   _mesa_shared_options_fill_defaults(&shared_options);
>>      if (!_mesa_initialize_context(mesaCtx, API_OPENGL_COMPAT, 
>> &v->mesa_visual,
>>                         share_list ? &(share_list->mesa) : (struct 
>> gl_context *) NULL,
>> -                      &functions)) {
>> +                      &shared_options, &functions)) {
>>         free(c);
>>         return NULL;
>>      }
>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>> index 8a59b5e..facc92b 100644
>> --- a/src/mesa/main/context.c
>> +++ b/src/mesa/main/context.c
>> @@ -563,11 +563,15 @@ init_program_limits(struct gl_constants 
>> *consts, gl_shader_stage stage,
>>    * some of these values (such as number of texture units).
>>    */
>>   void
>> -_mesa_init_constants(struct gl_constants *consts, gl_api api)
>> +_mesa_init_constants(struct gl_constants *consts,
>> +                     const struct shared_options *shared_options, 
>> gl_api api)
>>   {
>>      int i;
>>      assert(consts);
>>
>> +   /* Options */
>> +   consts->options = *shared_options;
>> +
>>      /* Constants, may be overriden (usually only reduced) by device 
>> drivers */
>>      consts->MaxTextureMbytes = MAX_TEXTURE_MBYTES;
>>      consts->MaxTextureLevels = MAX_TEXTURE_LEVELS;
>> @@ -797,12 +801,13 @@ check_context_limits(struct gl_context *ctx)
>>    * functions for the more complex data structures.
>>    */
>>   static GLboolean
>> -init_attrib_groups(struct gl_context *ctx)
>> +init_attrib_groups(struct gl_context *ctx,
>> +                   const struct shared_options *shared_options)
>>   {
>>      assert(ctx);
>>
>>      /* Constants */
>> -   _mesa_init_constants(&ctx->Const, ctx->API);
>> +   _mesa_init_constants(&ctx->Const, shared_options, ctx->API);
>>
>>      /* Extensions */
>>      _mesa_init_extensions(&ctx->Extensions);
>> @@ -1146,6 +1151,7 @@ _mesa_initialize_context(struct gl_context *ctx,
>>                            gl_api api,
>>                            const struct gl_config *visual,
>>                            struct gl_context *share_list,
>> +                         const struct shared_options *shared_options,
>>                            const struct dd_function_table 
>> *driverFunctions)
>>   {
>>      struct gl_shared_state *shared;
>> @@ -1194,7 +1200,7 @@ _mesa_initialize_context(struct gl_context *ctx,
>>
>>      _mesa_reference_shared_state(ctx, &ctx->Shared, shared);
>>
>> -   if (!init_attrib_groups( ctx ))
>> +   if (!init_attrib_groups( ctx, shared_options ))
>>         goto fail;
>>
>>      /* setup the API dispatch tables with all nop functions */
>> @@ -1285,6 +1291,7 @@ struct gl_context *
>>   _mesa_create_context(gl_api api,
>>                        const struct gl_config *visual,
>>                        struct gl_context *share_list,
>> +                     const struct shared_options *shared_options,
>>                        const struct dd_function_table *driverFunctions)
>>   {
>>      struct gl_context *ctx;
>> @@ -1293,7 +1300,7 @@ _mesa_create_context(gl_api api,
>>      if (!ctx)
>>         return NULL;
>>
>> -   if (_mesa_initialize_context(ctx, api, visual, share_list,
>> +   if (_mesa_initialize_context(ctx, api, visual, share_list, 
>> shared_options,
>>                                   driverFunctions)) {
>>         return ctx;
>>      }
>> diff --git a/src/mesa/main/context.h b/src/mesa/main/context.h
>> index 6f3c941..9e387ce 100644
>> --- a/src/mesa/main/context.h
>> +++ b/src/mesa/main/context.h
>> @@ -109,12 +109,14 @@ _mesa_initialize_context( struct gl_context *ctx,
>>                             gl_api api,
>>                             const struct gl_config *visual,
>>                             struct gl_context *share_list,
>> +                          const struct shared_options *shared_options,
>>                             const struct dd_function_table 
>> *driverFunctions);
>>
>>   extern struct gl_context *
>>   _mesa_create_context(gl_api api,
>>                        const struct gl_config *visual,
>>                        struct gl_context *share_list,
>> +                     const struct shared_options *shared_options,
>>                        const struct dd_function_table *driverFunctions);
>>
>>   extern void
>> @@ -144,7 +146,8 @@ _mesa_get_current_context(void);
>>   /*@}*/
>>
>>   extern void
>> -_mesa_init_constants(struct gl_constants *consts, gl_api api);
>> +_mesa_init_constants(struct gl_constants *consts,
>> +                     const struct shared_options *shared_options, 
>> gl_api api);
>>
>>   extern void
>>   _mesa_init_get_hash(struct gl_context *ctx);
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 0aa6076..dd75b1c 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -40,6 +40,7 @@
>>
>>   #include "main/glheader.h"
>>   #include "main/config.h"
>> +#include "main/shared_options.h"
>>   #include "glapi/glapi.h"
>>   #include "math/m_matrix.h"    /* GLmatrix */
>>   #include "glsl/shader_enums.h"
>> @@ -3376,6 +3377,9 @@ struct gl_constants
>>
>>      GLuint GLSLVersion;  /**< Desktop GLSL version supported (ex: 
>> 120 = 1.20) */
>>
>> +   /** Stores the drirc options of the different drivers */
>> +   struct shared_options options;
>> +
>>      /**
>>       * Changes default GLSL extension behavior from "error" to 
>> "warn".  It's out
>>       * of spec, but it can make some apps work that otherwise wouldn't.
>> diff --git a/src/mesa/main/shared_options.c 
>> b/src/mesa/main/shared_options.c
>> new file mode 100644
>> index 0000000..d3b843c
>> --- /dev/null
>> +++ b/src/mesa/main/shared_options.c
>> @@ -0,0 +1,33 @@
>> +/*
>> + * Mesa 3-D graphics library
>> + *
>> + * Copyright (C) 2015  Intel Corporation.  All Rights Reserved.
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be 
>> included
>> + * in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS
>> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
>> OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE 
>> USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include "shared_options.h"
>> +#include "drivers/dri/common/xmlconfig.h"
>> +
>> +void
>> +_mesa_shared_options_fill(struct driOptionCache *optionCache,
>> +                          struct shared_options *sharedOptions)
>> +{
>> +
>> +}
>> diff --git a/src/mesa/main/shared_options.h 
>> b/src/mesa/main/shared_options.h
>> new file mode 100644
>> index 0000000..abfb0a8
>> --- /dev/null
>> +++ b/src/mesa/main/shared_options.h
>> @@ -0,0 +1,71 @@
>> +/*
>> + * Mesa 3-D graphics library
>> + *
>> + * Copyright (C) 2015  Intel Corporation.  All Rights Reserved.
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be 
>> included
>> + * in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS
>> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
>> OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE 
>> USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/**
>> + * \file shared_options.h
>> + * Shared DRIconf options between the different drivers
>> + */
>> +
>> +#ifndef SHARED_OPTIONS_H
>> +#define SHARED_OPTIONS_H
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include <GL/glcorearb.h>
>> +
>> +struct driOptionCache;
>> +
>> +/**
>> + * List of shared options between the different mesa drivers
>> + */
>> +struct shared_options
>
> Most (all?) core Mesa structure types start with a gl_ prefix.  I 
> think a better name would be something like gl_context_options.

Although I agree the name is bad, omiting the gl_ prefix was a conscious 
decision because the options do not have to be gl-related. I may be 
looking too far in the future though :D

I will add the gl_ prefix!

>
>> +{
>> +   /* performance */
>> +
>> +   /* quality */
>> +
>> +   /* debug */
>> +
>> +   /* misc */
>> +};
>> +
>> +/** Fill the shared_options with information from the optionCache */
>> +void
>> +_mesa_shared_options_fill(struct driOptionCache *optionCache,
>> +                    struct shared_options *sharedOptions);
>
> This seems wrong.  Core Mesa should not know anything about DRI so 
> pulling the driOptionCache up from lower layers of the stack is an 
> interface violation.
>
> It seems to me that the DRI code should be responsible for populating 
> a gl_context_options object which would then be passed to core Mesa.
>
> We're looking at adding a config option system for Windows/WGL also.  
> We don't have any DRI stuff there, of course.

Can I suggest moving the driOptionCache to src/util? It would require a 
little bit of rework to get rid of the name "dri" and letting the 
drivers provide the configuration file's name, but It would then be 
acceptable to just pass around the optionCache instead of a structure, 
wouldn't it? That was my original idea but I realised I was breaking too 
many requirements.

>
> I think the patch series would be better if you started out by just 
> defining the new gl_context_options structure and plumbing it through 
> the _mesa_create/init_context() functions. 

Can we first try to find a way to have some common code filling some 
kind of structure that could be read by the common code of mesa? I 
really want to make sure all the drivers have the same behaviour when it 
comes to drirc and sharing the optionCache could allow that!


> Let see a complete structure with all the config options you have in 
> mind.

The current list is here: 
http://cgit.freedesktop.org/~mperes/mesa/tree/src/mesa/main/shared_options.h?h=mesa_extension_override2&id=69b3661e9ab7fe6fadc1ff964053795bf764e56b#n44

>
> For extension enable/disable, rather than duplicate the 
> MESA_EXTENSIONS_OVERRIDE strings, it might be nicer to have a list of 
> names of extensions to enable and a list of names of extensions to 
> disable.

I went this way first because I did not know that drirc options were 
overridable through env variables. I thus wanted to make sure the 
already-existing env vars could override drirc's option.

Your proposition makes a lot of sense though, now that I know better! I 
will rework this part!

>
> -Brian



More information about the mesa-dev mailing list