On 03/05/2012 12:00 PM, Benjamin Franzke wrote: > 2012/3/1 Chad Versace <chad.vers...@linux.intel.com>: >> On 03/01/2012 11:39 AM, Benjamin Franzke wrote: >>> 2012/3/1 Chad Versace <chad.vers...@linux.intel.com>: >>>> On 02/29/2012 07:36 AM, Benjamin Franzke wrote: >>>>> We dont want eglplatform.h to typedef egl native types >>>>> to x11 types, when x11 headers are not available. >>>>> --- >>>>> configure.ac | 4 ++++ >>>>> 1 files changed, 4 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/configure.ac b/configure.ac >>>>> index 0caa1b1..92a0e52 100644 >>>>> --- a/configure.ac >>>>> +++ b/configure.ac >>>>> @@ -1402,6 +1402,10 @@ if test "x$enable_egl" = xyes; then >>>>> >>>>> AC_CHECK_FUNC(mincore, [DEFINES="$DEFINES -DHAVE_MINCORE"]) >>>>> >>>>> + if test "$have_x" != yes; then >>>>> + DEFINES="$DEFINES -DMESA_EGL_NO_X11_HEADERS" >>>>> + fi >>>>> + >>>>> if test "$enable_static" != yes; then >>>>> # build egl_glx when libGL is built >>>>> if test "x$enable_glx" = xyes; then >>>> >>>> After examining the where the macro is used, in eglplatform.h ... >>>> >>>> --- snip --- >>>> #elif defined(__unix__) >>>> >>>> #ifdef MESA_EGL_NO_X11_HEADERS >>>> >>>> typedef void *EGLNativeDisplayType; >>>> typedef khronos_uint32_t EGLNativePixmapType; >>>> typedef khronos_uint32_t EGLNativeWindowType; >>>> >>>> #else >>>> >>>> /* X11 (tentative) */ >>>> #include <X11/Xlib.h> >>>> #include <X11/Xutil.h> >>>> >>>> typedef Display *EGLNativeDisplayType; >>>> typedef Pixmap EGLNativePixmapType; >>>> typedef Window EGLNativeWindowType; >>>> >>>> #endif /* MESA_EGL_NO_X11_HEADERS */ >>>> --- end snip --- >>>> >>>> I can't think of a valid reason for the #else branch to ever be taken. >>>> If you're building EGL on Unix/Linux, but not for gbm (__GBM__), Android >>>> (ANDRDOID), >>>> Wayland (WL_EGL_PLATFORM), or X (the default), then for what platform >>>> are you building libEGL? (The question is not rhetorical). >>> >>> The platform headers for wayland/gbm are not included when compiling >>> src/egl/main/* or some other common code in st/egl, that is when the >>> else branch is taken. >>> So currently we include Xlib.h when building without x11 platform, >>> that fails if its not available. >>> Will add a note to the commit message. >> >> It appears that the patch may introduce a bug in the 64-bit build of libEGL, >> but I'm not certain. Here's my reasoning. >> >> 1. Suppose that >> - This patch is applied. >> - The system has no X headers installed. >> - Some Wayland application has a source file, broken.c, that >> #includes <wayland-egl.h> then <EGL/egl.h>. >> >> 2. In eglapi.c, which defines eglCreateWindowSurface, EGLNativeWindowType >> will be defined as khronos_uint32_t. >> >> 3. In broken.c, wayland-egl.h first defines WL_EGL_PLATFORM. Then >> EGLNativeWindowType will be defined as wl_egl_window*, a 64-bit type. > > Yes, thanks for pointing that out. > We should just typedef them to khronos_uintptr_t instead.
Ok, if we do that, then I like this patch. >> Without the patch, eglapi.c safely typedefs EGLNativeWindowType to XID, >> which matches >> the system's pointer size. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
