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

[Brian Paul]

Hi Plamena,

Some style nitpicks below.  Feel free to take 'em or leave 'em.
I'm not too involved in EGL.


On 05/23/2016 10:27 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.
>
> Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
> ---
>   src/egl/main/eglapi.c     | 70 +++++++++++++++++++++++++++++++++++++++++++++++
>   src/egl/main/eglcurrent.h | 11 +-------
>   src/egl/main/egldisplay.c |  5 ++++
>   src/egl/main/egldisplay.h |  1 +
>   4 files changed, 77 insertions(+), 10 deletions(-)
>
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index be2c90f..9dcee79 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -1196,6 +1196,66 @@ eglGetError(void)
>   }
>
>
> +static bool
> +_eglDisplaySupportsApi(_EGLDisplay *dpy, EGLenum api)
> +{
> +   if (!dpy->Initialized) {
> +      return false;
> +   }
> +
> +   switch (api) {
> +      case EGL_OPENGL_API:
> +         if (dpy->ClientAPIs & EGL_OPENGL_BIT)
> +            return true;
 > +         break;

I'd probably do:
   return !!(dpy->ClientAPIs & EGL_OPENGL_BIT);

and similar for these cases to make it a little more concise.


> +      case EGL_OPENGL_ES_API:
> +         if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT ||
> +             dpy->ClientAPIs & EGL_OPENGL_ES2_BIT ||
> +             dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR) {
> +               return true;
> +         }
> +         break;
> +      case EGL_OPENVG_API:
> +         if (dpy->ClientAPIs & EGL_OPENVG_BIT)
> +            return true;
> +         break;
> +      default:
> +         return false;
> +         break;

break not needed.  Should the default case raise an invalid enum error 
or something?  I don't recall what the EGL spec says about that stuff.

Whitespace note: we normally indent 'case' the same amount as the 'switch'.


> +   }
> +
> +   return false;
> +}
> +
> +
> +/**
> + * Return true if a client API enum is recognized.
> + */
> +static bool
> +_eglIsApiValid(EGLenum api)
> +{
> +   _EGLDisplay *dpy = _eglGlobal.DisplayList;
> +   _EGLThreadInfo *current_thread = _eglGetCurrentThread();
> +
> +   while (dpy) {
> +      _EGLThreadInfo *thread = dpy->ThreadList;
> +
> +      while (thread) {
> +         if (thread == current_thread) {
> +            if (_eglDisplaySupportsApi(dpy, api))
> +               return true;
> +         }
> +
> +         thread = thread->Next;
> +      }
> +
> +      dpy = dpy->Next;
> +   }
> +
> +   return false;
> +}
> +
> +
>   /**
>    ** EGL 1.2
>    **/
> @@ -1211,6 +1271,16 @@ eglGetError(void)
>    *  eglWaitNative()
>    * See section 3.7 "Rendering Context" in the EGL specification for details.
>    */
> +
> + /**
> +  * Section 3.7 (Rendering Contexts) of the EGL 1.5 spec says:
> +  *
> +  * "api must specify one of the supported client APIs, either
> +  * EGL_OPENGL_API, EGL_OPENGL_ES_API, or EGL_OPENVG_API... If api
> +  * is not one of the values specified above, or if the client API
> +  * specified by api is not supported by the implementation, an
> +  * EGL_BAD_PARAMETER error is generated."
> +  */
>   EGLBoolean EGLAPIENTRY
>   eglBindAPI(EGLenum api)
>   {
> diff --git a/src/egl/main/eglcurrent.h b/src/egl/main/eglcurrent.h
> index 1e386ac..6c203be 100644
> --- a/src/egl/main/eglcurrent.h
> +++ b/src/egl/main/eglcurrent.h
> @@ -56,6 +56,7 @@ extern "C" {
>    */
>   struct _egl_thread_info
>   {
> +   _EGLThreadInfo *Next; /* used to link threads */
>      EGLint LastError;
>      _EGLContext *CurrentContexts[_EGL_API_NUM_APIS];
>      /* use index for fast access to current context */
> @@ -64,16 +65,6 @@ struct _egl_thread_info
>
>
>   /**
> - * Return true if a client API enum is recognized.
> - */
> -static inline EGLBoolean
> -_eglIsApiValid(EGLenum api)
> -{
> -   return (api >= _EGL_API_FIRST_API && api <= _EGL_API_LAST_API);
> -}
> -
> -
> -/**
>    * Convert a client API enum to an index, for use by thread info.
>    * The client API enum is assumed to be valid.
>    */
> diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
> index f6db03a..907a607 100644
> --- a/src/egl/main/egldisplay.c
> +++ b/src/egl/main/egldisplay.c
> @@ -240,6 +240,7 @@ _EGLDisplay *
>   _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
>   {
>      _EGLDisplay *dpy;
> +   _EGLThreadInfo *thread = _eglGetCurrentThread();
>
>      if (plat == _EGL_INVALID_PLATFORM)
>         return NULL;
> @@ -265,9 +266,13 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
>            /* add to the display list */
>            dpy->Next = _eglGlobal.DisplayList;
>            _eglGlobal.DisplayList = dpy;
> +         dpy->ThreadList = NULL;
>         }
>      }
>
> +   thread->Next = dpy->ThreadList;
> +   dpy->ThreadList = thread;
> +
>      mtx_unlock(_eglGlobal.Mutex);
>
>      return dpy;
> diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
> index 6bfc858..8a730ed 100644
> --- a/src/egl/main/egldisplay.h
> +++ b/src/egl/main/egldisplay.h
> @@ -140,6 +140,7 @@ struct _egl_display
>      _EGLPlatformType Platform; /**< The type of the platform display */
>      void *PlatformDisplay;     /**< A pointer to the platform display */
>
> +   _EGLThreadInfo *ThreadList;/**< A pointer to the thread the display was created form */
>      _EGLDriver *Driver;        /**< Matched driver of the display */
>      EGLBoolean Initialized;    /**< True if the display is initialized */
>
>