<div dir="ltr">Hi Daniel,<div>Thanks for reviewing!<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 13, 2016 at 5:09 PM, Daniel Stone <span dir="ltr"><<a href="mailto:daniel@fooishbar.org" target="_blank">daniel@fooishbar.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<span class=""><br>
On 13 May 2016 at 17:03, Plamena Manolova <<a href="mailto:plamena.manolova@intel.com">plamena.manolova@intel.com</a>> wrote:<br>
> @@ -444,6 +444,8 @@ _eglCreateAPIsString(_EGLDisplay *dpy)<br>
>        strcat(dpy->ClientAPIsString, "OpenVG ");<br>
><br>
>     assert(strlen(dpy->ClientAPIsString) < sizeof(dpy->ClientAPIsString));<br>
> +<br>
> +   _eglGlobal.ClientAPIsString = dpy->ClientAPIsString;<br>
<br>
</span>What happens when the display is destroyed and this is freed? Or when<br>
different displays have different supported APIs?<br></blockquote><div><br></div><div>You're right this would cause trouble. I think we have two alternatives here:</div><div>1: We could copy the string, but that wouldn't address the case in which different displays have different APIs.</div><div>2: We could fetch the current context and use the ClientAPIsString of the display associated with it. If there's no current</div><div>    context we could simply return EGL_FALSE since we'll have no way of verifying whether the API is supported.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> @@ -69,7 +70,26 @@ struct _egl_thread_info<br>
>  static inline EGLBoolean<br>
>  _eglIsApiValid(EGLenum api)<br>
>  {<br>
> -   return (api >= _EGL_API_FIRST_API && api <= _EGL_API_LAST_API);<br>
> +   char *api_string;<br>
> +   switch (api) {<br>
> +      case EGL_OPENGL_API:<br>
> +         api_string = "OpenGL";<br>
> +         break;<br>
> +      case EGL_OPENGL_ES_API:<br>
> +         api_string = "OpenGL_ES";<br>
> +         break;<br>
> +      case EGL_OPENVG_API:<br>
> +         api_string = "OpenVG";<br>
> +         break;<br>
> +      default:<br>
> +         return EGL_FALSE;<br>
> +      break;<br>
> +   }<br>
> +<br>
> +   if (strstr(api_string, _eglGlobal.ClientAPIsString))<br>
> +      return EGL_TRUE;<br>
> +   else<br>
> +      return EGL_FALSE;<br>
<br>
</span>This is trivially broken: it returns TRUE if a display only supports<br>
OpenGL ES, but you request to bind OpenGL.<br>
<br></blockquote><div> </div><div>Thank you for catching this! Silly mistake on my part.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
Daniel<br>
</blockquote></div><br></div></div></div>