[Mesa-dev] [PATCH 3/4] configure: Fix egl compilation without x11 headers

[Chad Versace]

On 03/05/2012 12:00 PM, Benjamin Franzke wrote:
> 2012/3/1 Chad Versace <chad.versace at linux.intel.com>:
>> On 03/01/2012 11:39 AM, Benjamin Franzke wrote:
>>> 2012/3/1 Chad Versace <chad.versace at 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.