[RFC wayland 1/9] wayland-egl: import libwayland-egl.so frontend library from Mesa

[Daniel Stone]

Hi Emil,

On 15 September 2017 at 11:29, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> Currently we're in a bit of a pickle - both from vendor and user POV.

This commit message is a bit long-winded. Could you please try to
compress it down to something like:

Currently the client-facing libwayland-egl API is defined by a header
file shipped by Wayland, but the implementation is left to each
vendor. This can cause collisions when multiple implementations are
installed on the same system. Importing the implementation into
Wayland with a stable and versioned driver-facing ABI allows multiple
drivers etc etc

Apart from the below, this series, when squashed into a single patch
for import, is:
Reviewed-by: Daniel Stone <daniels at collabora.com>

I realise this is a straight copy/import, so sorry to pick on these, but:

> +#include <stddef.h> // offsetof
> +#include <stdio.h>  // printf
> +
> +#include "wayland-egl-priv.h" // Current struct wl_egl_window implementation

C++ comments :(

> +/* GCC visibility */
> +#if defined(__GNUC__)
> +#define WL_EGL_EXPORT __attribute__ ((visibility("default")))
> +#else
> +#define WL_EGL_EXPORT
> +#endif

This bit can be deleted.

wayland-egl-priv.h could be renamed wayland-egl-backend.h, since it's
by definition no longer private. I'm also not a fan of importing dead
code and only wiring it up later.

> +#ifdef  __cplusplus
> +extern "C" {
> +#endif
> +
> +#define WL_EGL_WINDOW_VERSION 3
> +
> +struct wl_egl_window {
> +       const intptr_t version;
> +
> +       int width;
> +       int height;
> +       int dx;
> +       int dy;
> +
> +       int attached_width;
> +       int attached_height;
> +
> +       void *private;
> +       void (*resize_callback)(struct wl_egl_window *, void *);
> +       void (*destroy_window_callback)(void *);
> +
> +       struct wl_surface *surface;
> +};

I'd like to see this and the definitions of wayland-egl-abi-check.c
not be a complete copy of each other; perhaps in a separate private
header?

wayland-egl-priv.h could be renamed wayland-egl-backend.h, since it's
by definition no longer private. I'm also not a fan of importing dead
code and only wiring it up later.

> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
> new file mode 100755
> index 0000000..e7105ea
> --- /dev/null
> +++ b/egl/wayland-egl-symbols-check
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +FUNCS=$(nm -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o "T .*" | cut -c 3- | while read func; do

Please take the input library name from the command line, so
build-system details are confined to just the build system.

> +WL_EGL_EXPORT void
> +wl_egl_window_resize(struct wl_egl_window *egl_window,

Reuse WL_EXPORT.

> +WL_EGL_EXPORT struct wl_egl_window *
> +wl_egl_window_create(struct wl_surface *surface,
> +                    int width, int height)
> +{
> +       struct wl_egl_window _INIT_ = { .version = WL_EGL_WINDOW_VERSION };
> +       struct wl_egl_window *egl_window;
> +
> +       if (width <= 0 || height <= 0)
> +               return NULL;
> +
> +       egl_window = malloc(sizeof *egl_window);
> +       if (!egl_window)
> +               return NULL;
> +
> +       memcpy(egl_window, &_INIT_, sizeof *egl_window);

The initialiser is weird; it should be static const if it remains, but
why not just use calloc and set it like the rest of the members below?

Cheers,
Daniel