[Mesa-dev] [PATCH 2/3] glx/glvnd: Fix dispatch function names and indices

[Emil Velikov]

On 19 October 2016 at 18:39, Adam Jackson <ajax at redhat.com> wrote:
> On Fri, 2016-09-16 at 19:07 +0100, Emil Velikov wrote:
>> On 14 September 2016 at 19:06, Adam Jackson <ajax at redhat.com> wrote:
>> > As this array was not actually sorted, FindGLXFunction's binary search
>> > would only sometimes work.
>> >
>>
>> This commit message is a bit iffy, yet again most of this and
>> g_glxglvnddispatchfuncs.c is dead code.
>>
>> Afaict the sole reason behind his file is to have the vendor driver
>> callback into libGLX in order to manage (add/remove) the relevant
>> fbconfig/drawable/context to vendor mappings. From a quick search we
>> have ~5 out of the 30+ functions that do that.
>>
>> Everyone else calls back into the vendor library to a) get the correct
>> vendor (dispatch) and using it dive via the vendor neutral library
>
> Yes.
>
>> _directly_ back into itself
>
> No. dispatch_BindTexImageEXT (for example) is a function that the app
> would be handed as the result of glXGetProcAddress. Yes it comes from
> Mesa, but the drawable might belong to another vendor. So:
>
> static void dispatch_BindTexImageEXT(Display *dpy, GLXDrawable drawable,
>                                      int buffer, const int *attrib_list)
> {
>     PFNGLXBINDTEXIMAGEEXTPROC pBindTexImageEXT;
>     __GLXvendorInfo *dd;
>
>     /* look up which vendor is responsible for this drawable */
>     dd = GetDispatchFromDrawable(dpy, drawable);
>     if (dd == NULL)
>         return;
>
>     /* fetch that vendor's implementation of BindTexImageEXT */
>     __FETCH_FUNCTION_PTR(BindTexImageEXT);
>     if (pBindTexImageEXT == NULL)
>         return;
>
>     /* and tail call it */
>     (*pBindTexImageEXT)(dpy, drawable, buffer, attrib_list);
> }
>
> The "implementation" in Mesa would be __glXBindTexImageEXT. One could
> argue that dispatch_* could be smart enough to recognize their own
> dispatch tables, skip the glvnd call to ->fetchDispatchEntry in
> __FETCH_FUNCTION_PTR, and call the Mesa implementation directly. But
> GLX function calls are not exactly hot paths, and I'm not sure the
> complexity would be worth it.
>
Personally I agree, yet the rest (past this piece) of GLVND shows that
one is quite concerned with not making those slow.
Last time I've asked for some clarification [over at github] it fell
on deaf ears.

I mean seriously we could drop a very sizeable hunk of GLVND if that's
the case ;-)
Of which dozen or so cases of "exit(-1)".

>> (see the generated g_libglglxwrapper.c file) to get the entry point
>> via getProcAddress callback - see __glXFindVendorDispatchAddress().
>
> Remember that getProcAddress from glvnd into the vendor library is
> binary like dlsym, not unary like glXGetProcAddress. It's asking for
> the implementation, not the dispatch.
>
No argument. At the same time it behaves like a strange mix of both :-)

Either way: this is something we want to look into [hopefully not
years down the line], not something we should fix now.

One nitpick:
Can we drop glXGetScreenDriver all together. Last time I've looked it
had zero users*, it has no spec and GLVND wasn't generating an entry
point/dispatch for it.

On the sorting side: we can take a look at the generator scripts. Be
that get glvnd install them + mesa to use or just copy the lot
in-tree.

Thanks
Emil