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 <[email protected]> wrote: > On Mon 13 Apr 2015, Marek Olšák wrote: >> From: Daniel Kurtz <[email protected]> >> >> 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 <[email protected]> >> --- >> 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 <[email protected]> > > Thanks for upstreaming all of Daniel's fixes. _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
