[PATCH 3/3] Make EGL/GLESv2 dependencies optional

[Vasily Khoruzhick]

On Fri, Jan 11, 2013 at 11:19 AM, David Herrmann
<dh.herrmann at googlemail.com> wrote:
> Hi
>
> On Fri, Jan 11, 2013 at 7:44 AM, Vasily Khoruzhick <anarsoul at gmail.com> wrote:
>> On Fri, Jan 11, 2013 at 1:33 AM, David Herrmann
>> <dh.herrmann at googlemail.com> wrote:
>>> Hi
>>>
>>> On Wed, Jan 9, 2013 at 10:44 AM, Vasily Khoruzhick <anarsoul at gmail.com> wrote:
>>>> On Wed, Jan 9, 2013 at 12:03 AM, Vasily Khoruzhick <anarsoul at gmail.com> wrote:
>>>>> On Wed, Jan 9, 2013 at 12:01 AM, John Kåre Alsaker
>>>>> <john.kare.alsaker at gmail.com> wrote:
>>>>>> EGLDisplay, EGLSurface, EGLNativeDisplayType and EGLNativeWindowType
>>>>>> should all be void *. With that change you can get rid of the ugly
>>>>>> typecasts.
>>>>
>>>> xcb_window_t is uint32_t (see xproto.h), and it's passed as
>>>> EGLNativeWindowType in compositor-x11.c,
>>>> so if EGLNativeWindowType is void * I'm getting this warning:
>>>>
>>>> compositor-x11.c: In function 'x11_compositor_create_output':
>>>> compositor-x11.c:887:3: warning: passing argument 2 of
>>>> 'gl_renderer_output_create' makes pointer from integer without a cast
>>>> [enabled by default]
>>>> In file included from compositor-x11.c:52:0:
>>>> gl-renderer.h:78:1: note: expected 'EGLNativeWindowType' but argument
>>>> is of type 'xcb_window_t'
>>>
>>> First of all you cannot use EGLNativeWindowType but allow
>>> --disable-egl. This doesn't work.
>>
>> Why? It already works. I can just typedef missing types (because
>> EGL/egl.h is included
>> only if ENABLE_EGL is defined)
>
> Sorry, I overlooked these typedefs. These seem ok, indeed.
>
>>> Either add a build-time dependency
>>> to EGL but don't link if --disable-egl is passed or don't use EGL
>>> types at all. I think both is ok but the latter one should probably be
>>> preferred.
>>> And use void* as type but add the appropriate casts to
>>> compositor-x11.c, that is, change the EGLNativeWindowType casts to
>>> (void*). But as far as I know the ABI uses void* as type so the
>>> uint32_t thingy might break ABI on 64bit systems. I wonder how that is
>>> supposed to work without casts.
>>
>> See EGL/eglplatform.h, EGLNativeWindowType depends on platform, for
>> X11 it's Window,
>> for Win32 it's HWND, for wayland it's struct wl_egl_window *, for some
>> platforms it's void *.
>
> I know but you have only a single entry point for any function. So if
> a function takes a EGLNativeWindowType as argument like
> eglCreateWindowSurface(), then this entry point is used for all
> enabled backends. If you compiled EGL with Wayland and X11 support,
> then it takes a void* as argument, as WL takes precedence (which is a
> 64bit argument on amd64). But if only X11 was enabled, then it takes a
> "Window", which is a uint32_t thing as you mentioned.
> So the eglCreateWindowSurface() entry-point in eglapi.c in mesa takes
> either a 64 or 32bit argument depending on how it was compiled. As we
> expect it to be compiled with Wayland support, you should always
> assume the ABI takes a pointer-sized argument so use void*.
> This changes as soon as you no longer require EGL to be compiled with
> Wayland support but still use it for X11. Because then the ABI might
> actually take only 32bit arguments. However, in this case
> EGLNativeWindowType is available so you don't care.
>
> So there is no way you can define a type for EGLNativeWindowType for
> that you don't have to do casts. However, to avoid "casting to an
> integer of smaller size" warnings, you can just use a 64bit integer or
> pointer for this type and keep all the casts. And the compiler will
> optimize this anyway.

OK, so I'll just use uint64_t for EGLNativeWindowType and EGLNativeDisplayType.

Thanks for your comments!

Regards
Vasily

>
> Regards
> David