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

[David Herrmann]

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.

Regards
David