[Mesa-dev] [PATCH 04/15] egl: import headers from Khronos EGL registry
Marek Olšák
maraeo at gmail.com
Wed May 27 15:57:35 PDT 2015
Hi Chad,
I broke up the patches and sent them as a separate patch series a few days ago.
[PATCH 1/3] egl: import egl.h from registry (v2)
[PATCH 2/3] egl: import eglext.h from registry and cleanup eglmesaext.h (v2)
[PATCH 3/3] egl: import platform headers from registry (v2)
Marek
On Wed, May 27, 2015 at 8:20 PM, Chad Versace <chad.versace at intel.com> wrote:
> On Thu 14 May 2015, Emil Velikov wrote:
>> Hi Marek,
>> On 12/05/15 22:54, Marek Olšák wrote:
>> > From: Marek Olšák <marek.olsak at amd.com>
>> >
>> > with the extension of keeping:
>> > #define KHRONOS_APICALL __attribute__((visibility("default")))
>> >
>> > And don't include mesa headers in egl.h.
>> Can we do this more gradually (like below). It will ease the conflicts
>> that this patch might cause.
>>
>> - egl.h - should have no side effects
>> - eglext.h and eglmesaext.h - will clearly illustrate the parts that
>> have been moved from the latter to the former.
>> - eglplatform.h and khrplatform.h - there are a few controversial
>> changes which might cause breakage.
>
> I want to see this patch broken up too. Currently, the patch contains
> a straightforward header update layered on top of Marek's header
> modifications. The two should be split into separate patches so it's
> clear which changes are which.
>
>> > ---
>> > include/EGL/egl.h | 562 +++++++++++++++++++++------------------------
>> > include/EGL/eglext.h | 259 +++++++++++++++++++--
>> > include/EGL/eglplatform.h | 45 +---
>> > include/KHR/khrplatform.h | 25 +-
>> > src/egl/main/egltypedefs.h | 2 +
>> > 5 files changed, 540 insertions(+), 353 deletions(-)
>> >
>>
>> > diff --git a/include/EGL/eglplatform.h b/include/EGL/eglplatform.h
>> > index 2eb6865..1284089 100644
>> > --- a/include/EGL/eglplatform.h
>> > +++ b/include/EGL/eglplatform.h
>>
>> > -#elif defined(__WINSCW__) || defined(__SYMBIAN32__) /* Symbian */
>> > +#elif defined(__APPLE__) || defined(__WINSCW__) || defined(__SYMBIAN32__) /* Symbian */
>> >
>> > typedef int EGLNativeDisplayType;
>> > typedef void *EGLNativeWindowType;
>> > typedef void *EGLNativePixmapType;
>> >
>> > -#elif defined(WL_EGL_PLATFORM)
>> > +#elif defined(__ANDROID__) || defined(ANDROID)
>> >
>> > -typedef struct wl_display *EGLNativeDisplayType;
>> > -typedef struct wl_egl_pixmap *EGLNativePixmapType;
>> > -typedef struct wl_egl_window *EGLNativeWindowType;
>> > +#include <android/native_window.h>
>> >
>> > -#elif defined(__GBM__)
>> > -
>> > -typedef struct gbm_device *EGLNativeDisplayType;
>> > -typedef struct gbm_bo *EGLNativePixmapType;
>> > -typedef void *EGLNativeWindowType;
>> > -
>> > -#elif defined(ANDROID) /* Android */
>> > -
>> > -struct ANativeWindow;
>> > struct egl_native_pixmap_t;
>> >
>> > -typedef struct ANativeWindow *EGLNativeWindowType;
>> > -typedef struct egl_native_pixmap_t *EGLNativePixmapType;
>> > -typedef void *EGLNativeDisplayType;
>> > +typedef struct ANativeWindow* EGLNativeWindowType;
>> > +typedef struct egl_native_pixmap_t* EGLNativePixmapType;
>> > +typedef void* EGLNativeDisplayType;
>> >
>> > #elif defined(__unix__)
>> >
>> > -#if defined(MESA_EGL_NO_X11_HEADERS)
>> > -
>> > -typedef void *EGLNativeDisplayType;
>> > -typedef khronos_uintptr_t EGLNativePixmapType;
>> > -typedef khronos_uintptr_t EGLNativeWindowType;
>> > -
>> > -#else
>> > -
>> > /* X11 (tentative) */
>> > #include <X11/Xlib.h>
>> > #include <X11/Xutil.h>
>> > @@ -122,18 +103,8 @@ typedef Display *EGLNativeDisplayType;
>> > typedef Pixmap EGLNativePixmapType;
>> > typedef Window EGLNativeWindowType;
>> >
>> > -#endif /* MESA_EGL_NO_X11_HEADERS */
>> > -
>> > -#elif __HAIKU__
>> > -#include <kernel/image.h>
>> > -typedef void *EGLNativeDisplayType;
>> > -typedef khronos_uintptr_t EGLNativePixmapType;
>> > -typedef khronos_uintptr_t EGLNativeWindowType;
>> > -
>> Upon closer look, one could get away with the above changes, although
>> there may be something more subtle to it.
>>
>> Kristian, Chad,
>>
>> Would you have any suggestions for/against nuking the Wayland/GBM/other
>> typedefs ? With an extra eye on the Haiku changes, what would it take to
>> get the current eglplatform.h (or equivalent) accepted with Khronos ?
>
> I'm against. Khronos intended eglplatform.h specifically for this
> purpose. To quote the Khronos API Implementer's Guide:
>
> Implementers may need to modify eglplatform.h. In particular the
> eglNativeDisplayType, eglNativeWindowType, and eglNativePixmapType
> typedefs must be defined as appropriate for the platform (typically,
> they will be typedef'ed to corresponding types in the native window
> system). Developer documentation should mention the correspondence so
> that developers know what parameters to pass to eglCreateWindowSurface,
> eglCreatePixmapSurface, and eglCopyBuffers. Documentation should also
> describe the format of the display_id parameter to eglGetDisplay, since
> this is a platform-specific identifier.
>
> [https://www.khronos.org/registry/implementers_guide.html]
>
> And the EGL 1.5 spec says this about eglplatform.h:
>
> All platform-specific types, values, and macros used in egl.h are
> partitioned into a platform header, eglplatform.h, which is
> automatically included by egl.h. [...] Implementers should need to
> modify only eglplatform.h, never egl.h 2 .
>
> Therefore eglplatform.h should keep the typedefs for Wayland, GBM,
> Haiku, and Android. The specs say the vendor should provide the
> typedefs, and other projects are already relying on them. The header
> needs to keep MESA_EGL_NO_X11_HEADERS block too, because that block was
> added specifically to fix compilation on Linux systems that lacked X11.
>
>> > diff --git a/include/KHR/khrplatform.h b/include/KHR/khrplatform.h
>> > index 4479539..faa0ed7 100644
>> > --- a/include/KHR/khrplatform.h
>> > +++ b/include/KHR/khrplatform.h
>>
>> > #if defined(_WIN32) && !defined(__SCITECH_SNAP__)
>> > -# if defined(KHRONOS_DLL_EXPORTS)
>> > -# define KHRONOS_APICALL __declspec(dllexport)
>> > -# else
>> > -# define KHRONOS_APICALL __declspec(dllimport)
>> > -# endif
>> > +# define KHRONOS_APICALL __declspec(dllimport)
>> This might cause a problem with our Windows/SCons build. On the surface
>> it seems to rely on KHRONOS_DLL_EXPORTS. Although we do have module
>> definition files, which set the correct symbols as external.
>>
>> Perhaps Jose/Brian might be able to confirm if things are good or go
>> pear-shaped ?
>>
>>
>> > diff --git a/src/egl/main/egltypedefs.h b/src/egl/main/egltypedefs.h
>> > index e90959a..2430033 100644
>> > --- a/src/egl/main/egltypedefs.h
>> > +++ b/src/egl/main/egltypedefs.h
>> > @@ -35,6 +35,8 @@
>> >
>> > #include <EGL/egl.h>
>> > #include <EGL/eglext.h>
>> > +#include <EGL/eglextchromium.h>
>> > +#include <EGL/eglmesaext.h>
>> >
>> If my memory recalls correctly there was a discussion that
>> additional/third party extension headers should be included from within
>> eglext.h. Although I'm struggling to find the quote plus was never a fan
>> of it.
>
>> This will require that we check all possible users of the remaining
>> mesa/chromium extensions and update them :\
>
> My interpretation of the Khronos API Implementer's Guide says otherwise:
>
> Functions and enumerants for unregistered implementer extensions should
> be declared and defined in an implementer's own header file.
>
> But, from a pragmatic standpoint, I don't wish to break existing user's
> builds over this small technicality. I prefer to continue including
> eglext{vendor}.h from eglext.h unless there is a practical reason not
> to.
More information about the mesa-dev
mailing list