[Mesa-dev] [PATCH 09/10] egl: enhance KHR_gl_image extensions checks

[Eric Engestrom]

On Friday, 2017-06-30 12:15:19 +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Drop the (duplicate) top-level check in dri2_create_image_khr() and add
> the respective checks in dri2_create_image_khr_{texture,renderbuffer}
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  src/egl/drivers/dri2/egl_dri2.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index a641d774523..78a6d5f2219 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1682,6 +1682,11 @@ dri2_create_image_khr_renderbuffer(_EGLDisplay *disp, _EGLContext *ctx,
>        return EGL_NO_IMAGE_KHR;
>     }
>  
> +   if (!disp->Extensions.KHR_gl_renderbuffer_image) {
> +      _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> +      return EGL_NO_IMAGE_KHR;
> +   }
> +
>     dri_image =
>        dri2_dpy->image->createImageFromRenderbuffer(dri2_ctx->dri_context,
>                                                     renderbuffer, NULL);
> @@ -1820,30 +1825,38 @@ dri2_create_image_khr_texture(_EGLDisplay *disp, _EGLContext *ctx,
>  
>     switch (target) {
>     case EGL_GL_TEXTURE_2D_KHR:
> +      if (!disp->Extensions.KHR_gl_texture_2D_image) {
> +         _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> +         return EGL_NO_IMAGE_KHR;
> +      }
>        depth = 0;
>        gl_target = GL_TEXTURE_2D;
>        break;
>     case EGL_GL_TEXTURE_3D_KHR:
> -      if (disp->Extensions.KHR_gl_texture_3D_image) {
> -         depth = attrs.GLTextureZOffset;
> -         gl_target = GL_TEXTURE_3D;
> -         break;
> -      }
> -      else {
> +      if (!disp->Extensions.KHR_gl_texture_3D_image) {
>           _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
>           return EGL_NO_IMAGE_KHR;
>        }
> +
> +      depth = attrs.GLTextureZOffset;
> +      gl_target = GL_TEXTURE_3D;
> +      break;
>     case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_X_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_X_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_Y_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_Z_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_KHR:
> +      if (!disp->Extensions.KHR_gl_texture_cubemap_image) {
> +         _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> +         return EGL_NO_IMAGE_KHR;
> +      }
> +
>        depth = target - EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_X_KHR;
>        gl_target = GL_TEXTURE_CUBE_MAP;
>        break;
>     default:
> -      _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> +      assert(!"Unexpected target in dri2_create_image_khr_texture()");

I think the _eglError() should stay.

My comment on patch 1/10 clearly shows that I didn't read the whole
series before replying: you can ignore it :]

Series is:
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

Btw, I really like the solution in patch 3/10, I had this split on my
todo-list for a while, but I hadn't thought of a good way to avoid an
`if`-nesting-chain; your solution looks better than what I had in mind :)

Cheers,
  Eric

>        return EGL_NO_IMAGE_KHR;
>     }
>  
> @@ -2447,15 +2460,8 @@ dri2_create_image_khr(_EGLDriver *drv, _EGLDisplay *disp,
>     case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Y_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_POSITIVE_Z_KHR:
>     case EGL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Z_KHR:
> -      return dri2_create_image_khr_texture(disp, ctx, target, buffer, attr_list);
>     case EGL_GL_TEXTURE_3D_KHR:
> -      if (disp->Extensions.KHR_gl_texture_3D_image) {
> -         return dri2_create_image_khr_texture(disp, ctx, target, buffer, attr_list);
> -      }
> -      else {
> -         _eglError(EGL_BAD_PARAMETER, "dri2_create_image_khr");
> -         return EGL_NO_IMAGE_KHR;
> -      }
> +      return dri2_create_image_khr_texture(disp, ctx, target, buffer, attr_list);
>     case EGL_GL_RENDERBUFFER_KHR:
>        return dri2_create_image_khr_renderbuffer(disp, ctx, buffer, attr_list);
>  #ifdef HAVE_LIBDRM
> -- 
> 2.13.0
>