[Mesa-dev] [PATCH 2/2] egl: Check if API is supported when using eglBindAPI.

[Ian Romanick]

On 05/17/2016 09:35 AM, Plamena Manolova wrote:
> According to the EGL specifications before binding an API
> we must check whether it's supported first. If not eglBindAPI
> should return EGL_FALSE and generate a EGL_BAD_PARAMETER error.

Can you provide a spec quotation?

> Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
> ---
>  src/egl/main/eglcurrent.h | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/src/egl/main/eglcurrent.h b/src/egl/main/eglcurrent.h
> index 1e386ac..f2e19cc 100644
> --- a/src/egl/main/eglcurrent.h
> +++ b/src/egl/main/eglcurrent.h
> @@ -32,7 +32,8 @@
>  #include "c99_compat.h"
>  
>  #include "egltypedefs.h"
> -
> +#include "eglglobals.h"
> +#include "egldisplay.h"
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -62,14 +63,40 @@ struct _egl_thread_info
>     EGLint CurrentAPIIndex;
>  };
>  
> -
> +static inline EGLBoolean
> +_eglDisplaySupportsApi(_EGLDisplay *dpy, EGLenum api)

Since this is only used internally, please use bool/true/false.  Based
on my comments at the bottom, I think this function should go directly
in eglapi.c.

> +{

This is a really complex way of doing something quite simple.  How about.

   unsigned api_bit;

   if (!dpy->Initialized)
      return false:

   switch (api) {
   case EGL_OPENGL_API:
      api_bit = EGL_OPENGL_BIT;
      break;
   case EGL_OPENGL_ES_API:
      api_bit = EGL_OPENGL_ES_BIT |
                EGL_OPENGL_ES2_BIT |
                EGL_OPENGL_ES3_BIT_KHR;
      break;
   case EGL_OPENVG_API:
      api_bit = EGL_OPENVG_BIT;
      break;
   default:
      api_bit = 0;
      break;
   }

   return (dpy->ClientAPIs & api_bit) != 0;

> +   if (!dpy->Initialized) {
> +      return EGL_FALSE;
> +   } else if (api == EGL_OPENGL_API && dpy->ClientAPIs & EGL_OPENGL_BIT) {
> +      return EGL_TRUE;
> +   } else if (api == EGL_OPENGL_ES_API &&
> +      (dpy->ClientAPIs & EGL_OPENGL_ES_BIT ||
> +       dpy->ClientAPIs & EGL_OPENGL_ES2_BIT ||
> +       dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)) {
> +      return EGL_TRUE;
> +   } else if (api == EGL_OPENVG_API && dpy->ClientAPIs & EGL_OPENVG_BIT) {
> +      return EGL_TRUE;
> +   } else {
> +      return EGL_FALSE;
> +   }
> +}
>  /**
>   * Return true if a client API enum is recognized.
>   */
>  static inline EGLBoolean
>  _eglIsApiValid(EGLenum api)

There's only one user of this function, and I suspect there will only
every be that one user.  I would do a first patch that moves this
function into eglapi.c... maybe even just move this function into the
only caller.  Then this patch would change the way it works.

>  {
> -   return (api >= _EGL_API_FIRST_API && api <= _EGL_API_LAST_API);
> +   _EGLDisplay *dpy = _eglGlobal.DisplayList;
> +
> +   while (dpy) {
> +      if (_eglDisplaySupportsApi(dpy, api) == EGL_TRUE)

Even without changing _eglDisplaySupportsApi to return bool, you can
drop the '== EGL_TRUE'.

> +         return EGL_TRUE;
> +
> +      dpy = dpy->Next;
> +   }
> +
> +   return EGL_FALSE;
>  }
>  
>