On Fri, Apr 25, 2014 at 11:40 AM, Emil Velikov <emil.l.veli...@gmail.com> wrote:
> On 25/04/14 19:02, Kristian Høgsberg wrote:
>> On Sun, Mar 16, 2014 at 6:48 AM, Emil Velikov <emil.l.veli...@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.
If you have five extension that all depend on different things you end
up with 2^5 different combinations of extension arrays. By building
the extension list dynamically, you can add each of these extensions
to the list as you check the conditions they depend on.
> 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.
Let's tone down the rhetoric a bit, there's no "looping like crazy"
here, we're building an array of five extension at runtime, there's no
performance problem here and the code isn't harder 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.veli...@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@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
