[Piglit] [PATCH 13/14] dispatch: Use dlsym to lookup core symbols for EGL

[Marek Olšák]

Hi Chad,

The patch breaks GetProcAddress for libGL, because libGL doesn't
export a lot of GL functions and glXGetProcAddress is the only way to
get them. Any idea for a fix?

Marek

On Fri, Apr 24, 2015 at 9:36 PM, Chad Versace <chad.versace at intel.com> wrote:
> On Mon 13 Apr 2015, Marek Olšák wrote:
>> From: Daniel Kurtz <djkurtz at chromium.org>
>>
>> eglGetProcAddress() only supports extension functions.
>> Therefore, we must use dlsym() directly on the GL client library to look
>> up core functions.
>>
>> The implementation here is a much simplified version of the one in
>> libepoxy.  Most of the simplification is because piglit dispatch already
>> knows exactly for which GL API it is looking up a symbol.
>>
>> Signed-off-by: Daniel Kurtz <djkurtz at chromium.org>
>> ---
>>  tests/util/piglit-dispatch-init.c | 57 +++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 55 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/util/piglit-dispatch-init.c b/tests/util/piglit-dispatch-init.c
>> index cc8b684..b0e1649 100644
>> --- a/tests/util/piglit-dispatch-init.c
>> +++ b/tests/util/piglit-dispatch-init.c
>> @@ -33,6 +33,8 @@
>>
>>  #else /* Linux */
>>
>> +#include <dlfcn.h>
>> +
>>  #if defined(PIGLIT_HAS_GLX)
>>  #    include "glxew.h"
>>  #elif defined(PIGLIT_HAS_EGL)
>> @@ -154,6 +156,42 @@ get_core_proc_address(const char *function_name, int gl_10x_version)
>>
>>  #else /* Linux */
>>
>> +#if defined(PIGLIT_HAS_EGL)
>> +#define GLX_LIB "libGL.so.1"
>> +#define GLES1_LIB "libGLESv1_CM.so.1"
>> +#define GLES2_LIB "libGLESv2.so.2"
>> +
>> +/** dlopen() return value for libGL.so.1 */
>> +static void *glx_handle;
>> +
>> +/** dlopen() return value for libGLESv1_CM.so.1 */
>> +static void *gles1_handle;
>> +
>> +/** dlopen() return value for libGLESv2.so.2 */
>> +static void *gles2_handle;
>> +
>> +static void *
>> +do_dlsym(void **handle, const char *lib_name, const char *function_name)
>> +{
>> +    void *result;
>> +
>> +    if (!*handle)
>> +        *handle = dlopen(lib_name, RTLD_LAZY | RTLD_LOCAL);
>> +
>> +    if (!*handle) {
>> +        fprintf(stderr, "Could not open %s: %s\n", lib_name, dlerror());
>> +        return NULL;
>> +    }
>> +
>> +    result = dlsym(*handle, function_name);
>> +    if (!result)
>> +        fprintf(stderr, "%s() not found in %s: %s\n", function_name, lib_name,
>> +                dlerror());
>> +
>> +    return result;
>> +}
>> +#endif
>> +
>>  /**
>>   * This function is used to retrieve the address of all GL functions
>>   * on Linux.
>
> Everyting up to here looks good.
>
>> @@ -174,16 +212,31 @@ get_ext_proc_address(const char *function_name)
>>  /**
>>   * This function is used to retrieve the address of core GL functions
>>   * on Linux.
>> + *
>> + * Since eglGetProcAddress() only supports extension functions, we must use
>> + * dlsym() directly on the GL client library to lookup core functions.
>
> This comment isn't true. Sometimes eglGetProcAddress does supports core
> functions. It should instead say:
>
>   """
>   eglGetProcAddress supports querying core functions only if EGL >= 1.5
>   or if EGL_KHR_get_all_proc_addresses or
>   EGL_KHR_client_get_all_proc_addresses is supported. Rather than worry
>   about such details, when using EGL we consistently use dlsym() on the
>   client library to lookup core functions.
>   """
>
> This works for libGLESv1_CM.so and libGLESv2.so. But libGL.so wasn't designed
> with EGL in mind.  The antiquated OpenGL ABI for Linux [1] states:
>
>   3.4. [The OpenGL library] must export all OpenGL 1.2, [...] GLX 1.3, and
>        ARB_multitexture entry points statically.  [...] Applications should not
>        expect to link statically against any entry points not specified here.
>
> The specification of eglGetProcAddress and the official ABI specification for
> libGL are mutually incompatible. I think it's best to comply with
> eglGetProcAddress's restrictions rather than complying with a 15 year old
> standard that no one follows anymore. In other words, I think this patch takes
> the right approach.
>
>>   */
>>  static piglit_dispatch_function_ptr
>>  get_core_proc_address(const char *function_name, int gl_10x_version)
>>  {
>> -     /* We don't need to worry about the GL version, since on Apple
>> +#if defined(PIGLIT_HAS_EGL)
>> +     switch (gl_10x_version) {
>> +     case 11:
>> +             return do_dlsym(&gles1_handle, GLES1_LIB, function_name);
>> +     case 20:
>> +             return do_dlsym(&gles2_handle, GLES2_LIB, function_name);
>> +     case 10:
>> +     default:
>> +             /* GL does not have its own library, so use GLX */
>> +             return do_dlsym(&glx_handle, GLX_LIB, function_name);
>> +     }
>> +#else
>> +     /* We don't need to worry about the GL version, since when using GLX
>>        * we retrieve all proc addresses in the same way.
>>        */
>>       (void) gl_10x_version;
>> -
>>       return get_ext_proc_address(function_name);
>> +#endif
>>  }
>
> I was initially certain this patch would break piglit-dispatch for certain core
> GL functions. But I was wrong. This patch's approach looks good to me. (Its
> correctness relies on piglit-dispatch's subtle interpretation of the
> gl_10x_version parameter, a subtlety that's easy to forget).
>
> The default case may cause problems on some GL implementations, because of the
> specification conflicts I explained above. According to Linux's official OpenGL
> ABI, one cannot dlsym(libGL.so.1) for core functions that appeared in GL
> versions after 1.2. But, I think the risk is minor. If we later discover that
> the default case does break piglit-dispatch on some GL implementation, then we
> can easily fix it by changing it to use get_ext_proc_address() instead of dlsym().
>
> Reviewed-by: Chad Versace <chad.versace at intel.com>
>
> Thanks for upstreaming all of Daniel's fixes.