[Mesa-dev] [PATCH] mesa: do not use _glapi_new_nop_table() for DRI builds

[Ian Romanick]

On 05/26/2015 11:17 AM, Brian Paul wrote:
> On 05/26/2015 12:07 PM, Ian Romanick wrote:
>> On 05/21/2015 06:19 PM, Brian Paul wrote:
>>> Commit 4bdbb588a9d38 introduced new _glapi_new_nop_table() and
>>> _glapi_set_nop_handler() functions in the glapi dispatcher (which
>>> live in libGL.so).  The calls to those functions from context.c
>>> would be undefined (i.e. an ABI break) if the libGL used at runtime
>>> was older.
>>>
>>> For the time being, use the old single generic_nop() function for
>>> non-Windows builds to avoid this problem.  At some point in the future
>>> it should be safe to remove this work-around.  See comments for more
>>> details.
>>>
>>> v2: Incorporate feedback from Emil.  Use _WIN32 instead of
>>> GLX_DIRECT_RENDERING to control behavior, move comments.
>>>
>>> Cc: 10.6 <mesa-stable at lists.freedesktop.org>
>>> ---
>>>   src/mesa/main/context.c | 62
>>> +++++++++++++++++++++++++++++++++++++++++++++++--
>>>   1 file changed, 60 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>>> index 544cc14..02875ba 100644
>>> --- a/src/mesa/main/context.c
>>> +++ b/src/mesa/main/context.c
>>> @@ -883,6 +883,19 @@ update_default_objects(struct gl_context *ctx)
>>>   }
>>>
>>>
>>> +/* XXX this is temporary and should be removed at some point in the
>>> + * future when there's a reasonable expectation that the libGL library
>>> + * contains the _glapi_new_nop_table() and _glapi_set_nop_handler()
>>> + * functions which were added in Mesa 10.6.
>>> + */
>>
>> The other way would be to use dlsym determine whether or not the
>> function is available.  That should work on most or all ELF systems.  We
>> could also use the usual DRI extension mechanism.  That's the generally
>> accepted way to advertise new, optional libGL functionality to drivers.
> 
> OK, I can look into that someday.
> 
>>
>> Either way, this patch is
>>
>> Reviewed-and-tested-by: Ian Romanick <ian.d.romanick at intel.com>
> 
> Thanks!

Arg.  You didn't 'make check', did you? :(  I just tested the "hard"
part (using new driver with old libGL), and I didn't check the "easy" thing.

> -Brian
> 
>>
>>> +#if !defined(_WIN32)
>>> +/* Avoid libGL / driver ABI break */
>>> +#define USE_GLAPI_NOP_FEATURES 0
>>> +#else
>>> +#define USE_GLAPI_NOP_FEATURES 1
>>> +#endif
>>> +
>>> +
>>>   /**
>>>    * This function is called by the glapi no-op functions.  For each
>>> OpenGL
>>>    * function/entrypoint there's a simple no-op function.  These "no-op"
>>> @@ -898,6 +911,7 @@ update_default_objects(struct gl_context *ctx)
>>>    *
>>>    * \param name  the name of the OpenGL function
>>>    */
>>> +#if USE_GLAPI_NOP_FEATURES
>>>   static void
>>>   nop_handler(const char *name)
>>>   {
>>> @@ -914,6 +928,7 @@ nop_handler(const char *name)
>>>      }
>>>   #endif
>>>   }
>>> +#endif
>>>
>>>
>>>   /**
>>> @@ -923,7 +938,45 @@ nop_handler(const char *name)
>>>   static void GLAPIENTRY
>>>   nop_glFlush(void)
>>>   {
>>> -   /* don't record an error like we do in _mesa_generic_nop() */
>>> +   /* don't record an error like we do in nop_handler() */
>>> +}
>>> +#endif
>>> +
>>> +
>>> +#if !USE_GLAPI_NOP_FEATURES
>>> +static int
>>> +generic_nop(void)
>>> +{
>>> +   GET_CURRENT_CONTEXT(ctx);
>>> +   _mesa_error(ctx, GL_INVALID_OPERATION,
>>> +               "unsupported function called "
>>> +               "(unsupported extension or deprecated function?)");
>>> +   return 0;
>>> +}
>>> +#endif
>>> +
>>> +
>>> +/**
>>> + * Create a new API dispatch table in which all entries point to the
>>> + * generic_nop() function.  This will not work on Windows because of
>>> + * the __stdcall convention which requires the callee to clean up the
>>> + * call stack.  That's impossible with one generic no-op function.
>>> + */
>>> +#if !USE_GLAPI_NOP_FEATURES
>>> +static struct _glapi_table *
>>> +new_nop_table(unsigned numEntries)
>>> +{
>>> +   struct _glapi_table *table;
>>> +
>>> +   table = malloc(numEntries * sizeof(_glapi_proc));
>>> +   if (table) {
>>> +      _glapi_proc *entry = (_glapi_proc *) table;
>>> +      unsigned i;
>>> +      for (i = 0; i < numEntries; i++) {
>>> +         entry[i] = (_glapi_proc) generic_nop;
>>> +      }
>>> +   }
>>> +   return table;
>>>   }
>>>   #endif
>>>
>>> @@ -941,7 +994,11 @@ alloc_dispatch_table(void)
>>>       * Mesa we do this to accommodate different versions of libGL
>>> and various
>>>       * DRI drivers.
>>>       */
>>> -   GLint numEntries = MAX2(_glapi_get_dispatch_table_size(),
>>> _gloffset_COUNT);
>>> +   int numEntries = MAX2(_glapi_get_dispatch_table_size(),
>>> _gloffset_COUNT);
>>> +
>>> +#if !USE_GLAPI_NOP_FEATURES
>>> +   struct _glapi_table *table = new_nop_table(numEntries);
>>> +#else
>>>      struct _glapi_table *table = _glapi_new_nop_table(numEntries);
>>>
>>>   #if defined(_WIN32)
>>> @@ -967,6 +1024,7 @@ alloc_dispatch_table(void)
>>>   #endif
>>>
>>>      _glapi_set_nop_handler(nop_handler);
>>> +#endif
>>>
>>>      return table;
>>>   }
>>>
>>
>