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

[Manolova, Plamena]

Hi Daniel,
Thanks for reviewing!

On Fri, May 13, 2016 at 5:09 PM, Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
>
> On 13 May 2016 at 17:03, Plamena Manolova <plamena.manolova at intel.com>
> wrote:
> > @@ -444,6 +444,8 @@ _eglCreateAPIsString(_EGLDisplay *dpy)
> >        strcat(dpy->ClientAPIsString, "OpenVG ");
> >
> >     assert(strlen(dpy->ClientAPIsString) <
> sizeof(dpy->ClientAPIsString));
> > +
> > +   _eglGlobal.ClientAPIsString = dpy->ClientAPIsString;
>
> What happens when the display is destroyed and this is freed? Or when
> different displays have different supported APIs?
>

You're right this would cause trouble. I think we have two alternatives
here:
1: We could copy the string, but that wouldn't address the case in which
different displays have different APIs.
2: We could fetch the current context and use the ClientAPIsString of the
display associated with it. If there's no current
    context we could simply return EGL_FALSE since we'll have no way of
verifying whether the API is supported.

>
> > @@ -69,7 +70,26 @@ struct _egl_thread_info
> >  static inline EGLBoolean
> >  _eglIsApiValid(EGLenum api)
> >  {
> > -   return (api >= _EGL_API_FIRST_API && api <= _EGL_API_LAST_API);
> > +   char *api_string;
> > +   switch (api) {
> > +      case EGL_OPENGL_API:
> > +         api_string = "OpenGL";
> > +         break;
> > +      case EGL_OPENGL_ES_API:
> > +         api_string = "OpenGL_ES";
> > +         break;
> > +      case EGL_OPENVG_API:
> > +         api_string = "OpenVG";
> > +         break;
> > +      default:
> > +         return EGL_FALSE;
> > +      break;
> > +   }
> > +
> > +   if (strstr(api_string, _eglGlobal.ClientAPIsString))
> > +      return EGL_TRUE;
> > +   else
> > +      return EGL_FALSE;
>
> This is trivially broken: it returns TRUE if a display only supports
> OpenGL ES, but you request to bind OpenGL.
>
>
Thank you for catching this! Silly mistake on my part.


> Cheers,
> Daniel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160513/e8cd53f6/attachment.html>