On 18 July 2017 at 21:49, Miguel A. Vico <mvicom...@nvidia.com> wrote:
> We need wl_egl_window to be a versioned struct in order to keep track of
> ABI changes.
>
> This change makes the first member of wl_egl_window the version number.
>
> An heuristic in the wayland driver is added so that we don't break
> backwards compatibility:
>
> - If the first field (version) is an actual pointer, it is an old
> implementation of wl_egl_window, and version points to the wl_surface
> proxy.
>
> - Else, the first field is the version number, and we have
> wl_egl_window::surface pointing to the wl_surface proxy.
>
> Signed-off-by: Miguel A. Vico <mvicom...@nvidia.com>
> Reviewed-by: James Jones <jajo...@nvidia.com>
This commit will cause a break in the ABI checker. Yet again, I'm
short on ideas how to avoid that :-(
> ---
> src/egl/drivers/dri2/platform_wayland.c | 13 ++++++++++++-
> src/egl/wayland/wayland-egl/wayland-egl-priv.h | 6 +++++-
> src/egl/wayland/wayland-egl/wayland-egl.c | 6 +++++-
> 3 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_wayland.c
> b/src/egl/drivers/dri2/platform_wayland.c
> index ee68284217..0f0a12fd80 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -41,6 +41,7 @@
> #include "egl_dri2.h"
> #include "egl_dri2_fallbacks.h"
> #include "loader.h"
> +#include "eglglobals.h"
>
> #include <wayland-client.h>
> #include "wayland-drm-client-protocol.h"
> @@ -100,6 +101,16 @@ destroy_window_callback(void *data)
> dri2_surf->wl_win = NULL;
> }
>
> +static struct wl_surface *
> +get_wl_surface_proxy(struct wl_egl_window *window)
> +{
> + if (_eglPointerIsDereferencable((void *)(window->version))) {
> + /* window->version points to actual wl_surface data */
> + return wl_proxy_create_wrapper((void *)(window->version));
> + }
Please add a comment in there. I'm thinking about the following
although use whatever you prefer.
Version 3 of wl_egl_window introduced a version field, at the same
location where a pointer to wl_surface was stored.
> + return wl_proxy_create_wrapper(window->surface);
> +}
> +
> /**
> * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface().
> */
> @@ -171,7 +182,7 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
> wl_proxy_set_queue((struct wl_proxy *)dri2_surf->wl_dpy_wrapper,
> dri2_surf->wl_queue);
>
> - dri2_surf->wl_surface_wrapper = wl_proxy_create_wrapper(window->surface);
> + dri2_surf->wl_surface_wrapper = get_wl_surface_proxy(window);
> if (!dri2_surf->wl_surface_wrapper) {
> _eglError(EGL_BAD_ALLOC, "dri2_create_surface");
> goto cleanup_drm;
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> index 92c31d9454..3b59908cc1 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> +++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> @@ -41,8 +41,10 @@
> extern "C" {
> #endif
>
> +#define WL_EGL_WINDOW_VERSION 3
> +
> struct wl_egl_window {
> - struct wl_surface *surface;
> + const intptr_t version;
>
> int width;
> int height;
> @@ -55,6 +57,8 @@ struct wl_egl_window {
> void *private;
> void (*resize_callback)(struct wl_egl_window *, void *);
> void (*destroy_window_callback)(void *);
> +
> + struct wl_surface *surface;
> };
>
> #ifdef __cplusplus
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c
> b/src/egl/wayland/wayland-egl/wayland-egl.c
> index 4a4701a2de..02645549e0 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl.c
> +++ b/src/egl/wayland/wayland-egl/wayland-egl.c
> @@ -28,6 +28,7 @@
> */
>
> #include <stdlib.h>
> +#include <string.h>
>
> #include <wayland-client.h>
> #include "wayland-egl.h"
> @@ -54,6 +55,7 @@ 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)
> @@ -63,6 +65,8 @@ wl_egl_window_create(struct wl_surface *surface,
> if (!egl_window)
> return NULL;
>
> + memcpy(egl_window, &_INIT_, sizeof *egl_window);
> +
The _INIT_ and memcpy seems like an overkill. At the same time the
current malloc + init each member is not that robust.
Something like the following seems reasonable, imho.
*egl_window = (struct wl_egl_window) {
.version = WL_EGL_WINDOW_VERSION,
.surface = surface,
.width = width,
.height = height,
};
> @@ -70,7 +74,7 @@ wl_egl_window_create(struct wl_surface *surface,
> wl_egl_window_resize(egl_window, width, height, 0, 0);
> egl_window->attached_width = 0;
> egl_window->attached_height = 0;
> -
> +
Unrelated whitespace change.
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
