[waffle] [PATCH 08/18] api: make waffle_get_proc_address() display aware

[Chad Versace]

On 08/04/2014 09:43 AM, Emil Velikov wrote:
> On 04/08/14 08:17, Chad Versace wrote:
>> Thunderbird believes this patch is Chinese... :(
>>
> In that case I will attach the patch (and push branch for-upstream-WGL-1.2
> shortly).

Thanks for attaching. I was able to copy-paste it into this email.

>> I need more time to look at this patch because of the API break.
>>
>> Even though eglGetProcAddress and glXGetProcAddress allow queries before the process
>> makes any other EGL/GLX calls, in practice I see no harm in requiring Waffle users
>> to postpone procAddress queries until after acquiring a display. Except for a few
>> special cases, GetProc'd function pointers aren't useful anyway without a current
>> context, which requires a display.
>>
>> So, I don't believe this should cause any real problems. But I want a little time to
>> consider all the implications here.
>>
> On possible issue that I can see is if one uses waffle_get_proc_address()
> using a pointer to the first waffle_display, while a second one is active. On
> the other hand I assume that waffle itself may explode a bit before that :)

Right. Waffle's internals will probably collapse on corner cases like that.

> And the current versioning scheme makes things even easier wrt having multiple
> waffles side by side. I like mine with chocolate :)

And I like my waffles with syrup :)

I have seriously considered naming waffle-dispatch (if it ever happens) "libsyrup".
Syrup is sticky like "glue", right?

> From 905cdede895aef7c234b54c405b93b17927b1746 Mon Sep 17 00:00:00 2001
> From: Emil Velikov <emil.l.velikov at gmail.com>
> Date: Tue, 22 Jul 2014 21:04:21 +0100
> Subject: [PATCH 08/19] api: make waffle_get_proc_address() display aware
> 
> For WGL we need a current context in order to get
> wglGetProcAddress to work. We resolve this by creating
> per display context (inside wgl_display_connect) that is
> later used if we call the above when there is no current
> context.
> 
> Wire up wgl_get_proc_address, update all the examples and
> documentation.
> 
> Chad, tests/functional/gl_basic_test.c seems to be doing something
> very nasty with a comprehensive explanation why. Is there another
> way around this ?
> 
> Note this commit breaks the API in a non-backwards compatible way.

I've made peace with the ABI break. Delete the code and comment in the last
hunk, and this patch is
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>

> 
> TODO:
>  - Bump the major version.
>  - Add a note in the release notes.
> 
> Cc: Chad Versace <chad.versace at linux.intel.com>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>


> --- a/src/waffle/api/waffle_gl_misc.c
> +++ b/src/waffle/api/waffle_gl_misc.c
> @@ -101,10 +101,16 @@ waffle_make_current(
>  }
>  
>  WAFFLE_API void*
> -waffle_get_proc_address(const char *name)
> +waffle_get_proc_address(struct waffle_display *dpy, const char *name)
>  {
> -    if (!api_check_entry(NULL, 0))
> +    struct wcore_display *wc_dpy = wcore_display(dpy);
> +
> +    const struct api_object *obj_list[] = {
> +        wc_dpy ? &wc_dpy->api : NULL,
> +    };
> +
> +    if (!api_check_entry(obj_list, 1))
>          return NULL;
>  
> -    return api_platform->vtbl->get_proc_address(api_platform, name);
> +    return api_platform->vtbl->get_proc_address(api_platform, wc_dpy, name);
>  }

There do exist some EGL functions that should be queryable without an active
display. Namely, functions that belong to EGL client extensions. 

However, Waffle's use case for waffle_get_proc_address() is to obtain "GL"
functions, not CGL/EGL/GLX/WGL functions. So this hunk looks good to me. In the
future, if we want to support queries for EGL client extension functions, then
we can easily patch the code path to permit the query.


> diff --git a/tests/functional/gl_basic_test.c b/tests/functional/gl_basic_test.c
> index 035b221..1f1ef8f 100644
> --- a/tests/functional/gl_basic_test.c
> +++ b/tests/functional/gl_basic_test.c
> @@ -242,7 +242,10 @@ gl_basic_draw__(struct gl_basic_draw_args__ args)
>      // call. Otherwise, libEGL may initialize itself with the incorrect
>      // platform. In my experiments, first calling eglGetProcAddress will
>      // produce a segfault in eglInitialize.
> +    // Emil: Chad can we find a better/less hacky solution for this ?
> +#if 0
>      waffle_get_proc_address("glClear");
> +#endif

I believe it's safe to delete this block of code and its comment. Long ago,
when the multi-platform support in Mesa EGL was buggy, this block prevented
gl_basic_test from exercising one of its more mysterious bugs. Anyone today who
runs gl_basic_test is likely no longer using that buggy Mesa, so there's little
value in preserving this block of code.