[Mesa-dev] [RFC 1/7] eglplatform.h: introduce and use EGL_USE_PLATFORM_*_KHR

[Emil Velikov]

On 25 September 2016 at 00:17, Eric Engestrom <eric at engestrom.ch> wrote:
> On Thu, Sep 22, 2016 at 09:38:06AM +0100, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> In order to avoid the current, somewhat fragile detection in
>> eglplatform.h introduce explicit platform selection.
>>
>> The approach is based on the one used in Vulkan and allows one to
>> explicitly "request" the platform they will be using without the need of
>> local hacks.
>>
>> ---
>> XXX: Strictly speaking the default/else case would be the None/native
>> one, but since we still have the "autodetection" heuristics...
>>
>> Admittedly some of the names can be improved, plus there's a limited
>> about of Symbian users still in the wild. The latter kept for
>> compatibility reasons.
>> ---
>>  include/EGL/eglplatform.h | 56 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h
>> index b376e64..923b5f6 100644
>> --- a/include/EGL/eglplatform.h
>> +++ b/include/EGL/eglplatform.h
>> @@ -67,6 +67,62 @@
>>   * implementations.
>>   */
>>
>> +#if defined(EGL_USE_PLATFORM_ANDROID_KHR)
>> +#include <android/native_window.h>
>> +
>> +struct egl_native_pixmap_t;
>> +
>> +typedef struct ANativeWindow*           EGLNativeWindowType;
>> +typedef struct egl_native_pixmap_t*     EGLNativePixmapType;
>> +typedef void*                           EGLNativeDisplayType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_GBM_KHR) // XXX: Name GBM vs DRM vs other
>> +
>> +typedef struct gbm_device  *EGLNativeDisplayType;
>> +typedef struct gbm_bo      *EGLNativePixmapType;
>> +typedef void               *EGLNativeWindowType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_NONE_KHR) // XXX: Name NONE vs Native vs other
>> +
>> +typedef void            *EGLNativeDisplayType;
>> +typedef khronos_uintptr_t EGLNativePixmapType;
>> +typedef khronos_uintptr_t EGLNativeWindowType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_SYMBIAN_KHR)
>> +
>> +typedef int   EGLNativeDisplayType;
>> +typedef void *EGLNativeWindowType;
>> +typedef void *EGLNativePixmapType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_WAYLAND_KHR)
>> +
>> +typedef struct wl_display     *EGLNativeDisplayType;
>> +typedef struct wl_egl_pixmap  *EGLNativePixmapType;
>> +typedef struct wl_egl_window  *EGLNativeWindowType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_WIN32_KHR)
>> +
>> +#ifndef WIN32_LEAN_AND_MEAN
>> +#define WIN32_LEAN_AND_MEAN 1
>> +#endif
>
> Isn't that fragile too?  I'm not familiar with Windows and what this
> #define does, but presumably if someone where to `#include <windows.h>`
> first, the behaviour would change?
> If that's the case, maybe we could add an `#else #warning "Don't include
> <windows.h> before this file"`?
>
Yes, things look rather iffy, but since it's unlikely I'll test the
Windows side I've opted for a copy/paste of the existing code :-)

>> +#include <windows.h>
>> +
>> +typedef HDC     EGLNativeDisplayType;
>> +typedef HBITMAP EGLNativePixmapType;
>> +typedef HWND    EGLNativeWindowType;
>> +
>> +#elif defined(EGL_USE_PLATFORM_XLIB_KHR)
>> +#include <X11/Xlib.h>
>> +#include <X11/Xutil.h>
>> +
>> +typedef Display *EGLNativeDisplayType;
>> +typedef Pixmap   EGLNativePixmapType;
>> +typedef Window   EGLNativeWindowType;
>> +
>> +#else
>
> That `+#else` is missing its matching `+#endif` :)
>
> -------8<-------
> @@ -134,6 +190,7 @@ typedef khronos_uintptr_t    EGLNativeWindowType;
>  #else
>  #error "Platform not recognized"
>  #endif
> +#endif
>
>  /* EGL 1.2 types, renamed for consistency in EGL 1.3 */
>  typedef EGLNativeDisplayType NativeDisplayType;
> ------->8-------
>
> Overall, I think this series is a good idea, although #7 will probably
> have to wait for a while...
>
Yes, the last 2-3 patches might never make it afaict, but they're good
to illustrate things.

Thanks for catching the silly bug :-)
Emil