Correction: Only 2 pipeline statistics tests were broken, so all seems to be good (except those 2 tests).
Marek On Tue, Apr 28, 2015 at 3:18 PM, Marek Olšák <[email protected]> wrote: > 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
