[Mesa-dev] [PATCH 05/16] glx/dri2: rework __DRIextension handling

[Emil Velikov]

On 25/04/14 19:02, Kristian Høgsberg wrote:
> On Sun, Mar 16, 2014 at 6:48 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> Use a const arrays with the extensions, and use the correct one at
>> runtime. This removes the assumption that the dri2_screen has a
>> fixed size dri extension list and avoids the individual assignment
>> of each extension at runtime.
> 
> This change seems like unnecessary churn.  Your new mechanism doesn't
> scale to multiple optional extensions, where it turns into a
> combinatorial explosion.  The array size is of course correlated with
> the max number of extensions you might add to the array and we could
> add an assert to make sure we don't overwrite it.
> 
Not sure what explosion are you referring to here. Want to add an extra
extension - just add two lines of code.

Idea behind this is, as we already know the complete set of extensions, there
is little to no benefit in looping like crazy at runtime. The benefit may be
~0% although it makes code _a bit_ easier to follow.

-Emil

>> While we're here make sure that the DRI*Extensions report the
>> version of the interface implemented over the listed in the headers.
>> While both are currently the same, this may change in the future.
> 
> This part is good.
> 
>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>> ---
>>  src/glx/dri2_glx.c | 37 +++++++++++++++++++++----------------
>>  1 file changed, 21 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>> index acd8427..84ae875 100644
>> --- a/src/glx/dri2_glx.c
>> +++ b/src/glx/dri2_glx.c
>> @@ -74,7 +74,7 @@ struct dri2_display
>>
>>     __glxHashTable *dri2Hash;
>>
>> -   const __DRIextension *loader_extensions[4];
>> +   const __DRIextension **loader_extensions;
>>  };
>>
>>  struct dri2_context
>> @@ -968,7 +968,21 @@ static const __DRIdri2LoaderExtension dri2LoaderExtension_old = {
>>  };
>>
>>  static const __DRIuseInvalidateExtension dri2UseInvalidate = {
>> -   { __DRI_USE_INVALIDATE, __DRI_USE_INVALIDATE_VERSION }
>> +   .base = { __DRI_USE_INVALIDATE, 1 }
>> +};
>> +
>> +static const __DRIextension *loader_extensions_old[] = {
>> +   &dri2LoaderExtension_old.base,
>> +   &systemTimeExtension.base,
>> +   &dri2UseInvalidate.base,
>> +   NULL,
>> +};
>> +
>> +static const __DRIextension *loader_extensions[] = {
>> +   &dri2LoaderExtension.base,
>> +   &systemTimeExtension.base,
>> +   &dri2UseInvalidate.base,
>> +   NULL,
>>  };
>>
>>  _X_HIDDEN void
>> @@ -1240,15 +1254,13 @@ dri2CreateScreen(int screen, struct glx_display * priv)
>>     if (psc->dri2->base.version >= 4) {
>>        psc->driScreen =
>>           psc->dri2->createNewScreen2(screen, psc->fd,
>> -                                     (const __DRIextension **)
>> -                                     &pdp->loader_extensions[0],
>> +                                     pdp->loader_extensions,
>>                                       extensions,
>>                                       &driver_configs, psc);
>>     } else {
>>        psc->driScreen =
>>           psc->dri2->createNewScreen(screen, psc->fd,
>> -                                    (const __DRIextension **)
>> -                                    &pdp->loader_extensions[0],
>> +                                    pdp->loader_extensions,
>>                                      &driver_configs, psc);
>>     }
>>
>> @@ -1365,7 +1377,7 @@ _X_HIDDEN __GLXDRIdisplay *
>>  dri2CreateDisplay(Display * dpy)
>>  {
>>     struct dri2_display *pdp;
>> -   int eventBase, errorBase, i;
>> +   int eventBase, errorBase;
>>
>>     if (!DRI2QueryExtension(dpy, &eventBase, &errorBase))
>>        return NULL;
>> @@ -1386,17 +1398,10 @@ dri2CreateDisplay(Display * dpy)
>>     pdp->base.destroyDisplay = dri2DestroyDisplay;
>>     pdp->base.createScreen = dri2CreateScreen;
>>
>> -   i = 0;
>>     if (pdp->driMinor < 1)
>> -      pdp->loader_extensions[i++] = &dri2LoaderExtension_old.base;
>> +      pdp->loader_extensions = loader_extensions_old;
>>     else
>> -      pdp->loader_extensions[i++] = &dri2LoaderExtension.base;
>> -
>> -   pdp->loader_extensions[i++] = &systemTimeExtension.base;
>> -
>> -   pdp->loader_extensions[i++] = &dri2UseInvalidate.base;
>> -
>> -   pdp->loader_extensions[i++] = NULL;
>> +      pdp->loader_extensions = loader_extensions;
>>
>>     pdp->dri2Hash = __glxHashCreate();
>>     if (pdp->dri2Hash == NULL) {
>> --
>> 1.9.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev