[Mesa-dev] [PATCH 08/12] egl: handle BAD_NATIVE_WINDOW/PIXMAP further up the stack

[Eric Engestrom]

On Thursday, 2017-08-03 19:29:34 +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> The basic (null) check is identical across all backends.
> Just move it to the top.
> 
> Note that eglCreateWindowSurface* already has a guard in the *Common
> helper.

Read the code, checked that, was going to comment then saw the commit msg ^^
Anyway, I'd do 2 commits here: move x11's !pixmap check, then drop the
redundant !window checks separately.

> 
> Cc: Eric Engestrom <eric.engestrom at imgtec.com>
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
> Eric, the X11 hunk was changed/introduced by Frank with commit
> 2900e8ca9077d20c5b29bb5a4171ac59ea9d1767

You could have Cc'ed him directly :P

> 
> If you guys had a usecase that hit the path, can you please check this
> commit does not break it.

Digging through our logs, the issue was highlighted by one (or more) of:
dEQP-EGL.functional.create_surface.pbuffer.*
dEQP-EGL.functional.image.api.*
dEQP-EGL.functional.image.create.*
dEQP-EGL.functional.image.modify.*
dEQP-EGL.functional.image.render_multiple_contexts.*
dEQP-EGL.functional.negative_api.*

If these pass for pixmaps with your patch then it should be fine :)

> ---
>  src/egl/drivers/dri2/platform_android.c | 2 +-
>  src/egl/drivers/dri2/platform_drm.c     | 5 -----
>  src/egl/drivers/dri2/platform_wayland.c | 5 -----
>  src/egl/drivers/dri2/platform_x11.c     | 7 -------
>  src/egl/main/eglapi.c                   | 3 +++
>  5 files changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index 342f57dd94c..a04e934c597 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -329,7 +329,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>     if (type == EGL_WINDOW_BIT) {
>        int format;
>  
> -      if (!window || window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) {
> +      if (window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) {
>           _eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface");
>           goto cleanup_surface;
>        }
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c
> index a952aa54560..7ea43e62010 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -115,11 +115,6 @@ dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>  
>     switch (type) {
>     case EGL_WINDOW_BIT:
> -      if (!window) {
> -         _eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface");
> -         goto cleanup_surf;
> -      }
> -
>        surf = gbm_dri_surface(window);
>        dri2_surf->gbm_surf = surf;
>        dri2_surf->base.Width =  surf->base.width;
> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> index 67be9683aca..7753f0f2c50 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -162,11 +162,6 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp,
>           dri2_surf->format = WL_SHM_FORMAT_ARGB8888;
>     }
>  
> -   if (!window) {
> -      _eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface");
> -      goto cleanup_surf;
> -   }
> -
>     dri2_surf->wl_win = window;
>     dri2_surf->wl_queue = wl_display_create_queue(dri2_dpy->wl_dpy);
>     if (!dri2_surf->wl_queue) {
> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
> index 111c478b9de..6634c9900c4 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -234,13 +234,6 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>                         dri2_surf->drawable, dri2_dpy->screen->root,
>  			dri2_surf->base.Width, dri2_surf->base.Height);
>     } else {
> -      if (!drawable) {
> -         if (type == EGL_WINDOW_BIT)
> -            _eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface");
> -         else
> -            _eglError(EGL_BAD_NATIVE_PIXMAP, "dri2_create_surface");
> -         goto cleanup_surf;
> -      }
>        dri2_surf->drawable = drawable;
>     }
>  
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index c5e3955c48c..9e836303c22 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -1000,6 +1000,9 @@ _eglCreatePixmapSurfaceCommon(_EGLDisplay *disp, EGLConfig config,
>     _EGLSurface *surf;
>     EGLSurface ret;
>  
> +   if (native_pixmap == NULL)
> +      RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_PIXMAP, EGL_NO_SURFACE);
> +
>  #if HAVE_SURFACELESS_PLATFORM
>     if (disp && disp->Platform == _EGL_PLATFORM_SURFACELESS) {
>        /* From the EGL_MESA_platform_surfaceless spec (v1):
> -- 
> 2.13.3
>