<div dir="ltr">Hi Ian,<div>Thanks for reviewing!<br><div class="gmail_extra"><br style="font-size:12.8px"><div class="gmail_quote" style="font-size:12.8px"><span class="im">On Wed, May 18, 2016 at 4:33 PM, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">On 05/17/2016 09:35 AM, Plamena Manolova wrote:<br>> According to the EGL specifications before binding an API<br>> we must check whether it's supported first. If not eglBindAPI<br>> should return EGL_FALSE and generate a EGL_BAD_PARAMETER error.<br><br>Can you provide a spec quotation?<br></blockquote><div><br></div></span><div><a href="https://www.khronos.org/registry/egl/sdk/docs/man/html/eglBindAPI.xhtml" target="_blank">https://www.khronos.org/registry/egl/sdk/docs/man/html/eglBindAPI.xhtml</a></div><div><br></div><div>"EGL_BAD_PARAMETER is generated if api is not one of the accepted tokens, or if the </div><div>specified client API is not supported by the EGL implementation."</div><div><div class="adm"><div id="q_154c53c535f06da9_3" class="h4"><div class="" style=""></div></div></div><div class="im"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">> Signed-off-by: Plamena Manolova <<a href="mailto:plamena.manolova@intel.com" target="_blank">plamena.manolova@intel.com</a>><br>> ---<br>>  src/egl/main/eglcurrent.h | 33 ++++++++++++++++++++++++++++++---<br>>  1 file changed, 30 insertions(+), 3 deletions(-)<br>><br>> diff --git a/src/egl/main/eglcurrent.h b/src/egl/main/eglcurrent.h<br>> index 1e386ac..f2e19cc 100644<br>> --- a/src/egl/main/eglcurrent.h<br>> +++ b/src/egl/main/eglcurrent.h<br>> @@ -32,7 +32,8 @@<br>>  #include "c99_compat.h"<br>><br>>  #include "egltypedefs.h"<br>> -<br>> +#include "eglglobals.h"<br>> +#include "egldisplay.h"<br>><br>>  #ifdef __cplusplus<br>>  extern "C" {<br>> @@ -62,14 +63,40 @@ struct _egl_thread_info<br>>     EGLint CurrentAPIIndex;<br>>  };<br>><br>> -<br>> +static inline EGLBoolean<br>> +_eglDisplaySupportsApi(_EGLDisplay *dpy, EGLenum api)<br><br>Since this is only used internally, please use bool/true/false.  Based<br>on my comments at the bottom, I think this function should go directly<br>in eglapi.c.<br><br>> +{<br><br>This is a really complex way of doing something quite simple.  How about.<br><br>   unsigned api_bit;<br><br>   if (!dpy->Initialized)<br>      return false:<br><br>   switch (api) {<br>   case EGL_OPENGL_API:<br>      api_bit = EGL_OPENGL_BIT;<br>      break;<br>   case EGL_OPENGL_ES_API:<br>      api_bit = EGL_OPENGL_ES_BIT |<br>                EGL_OPENGL_ES2_BIT |<br>                EGL_OPENGL_ES3_BIT_KHR;<br>      break;<br>   case EGL_OPENVG_API:<br>      api_bit = EGL_OPENVG_BIT;<br>      break;<br>   default:<br>      api_bit = 0;<br>      break;<br>   }<br><br>   return (dpy->ClientAPIs & api_bit) != 0;</blockquote><div><br></div></div></div><div>I'll make those changes.</div><span class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>> +   if (!dpy->Initialized) {<br>> +      return EGL_FALSE;<br>> +   } else if (api == EGL_OPENGL_API && dpy->ClientAPIs & EGL_OPENGL_BIT) {<br>> +      return EGL_TRUE;<br>> +   } else if (api == EGL_OPENGL_ES_API &&<br>> +      (dpy->ClientAPIs & EGL_OPENGL_ES_BIT ||<br>> +       dpy->ClientAPIs & EGL_OPENGL_ES2_BIT ||<br>> +       dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)) {<br>> +      return EGL_TRUE;<br>> +   } else if (api == EGL_OPENVG_API && dpy->ClientAPIs & EGL_OPENVG_BIT) {<br>> +      return EGL_TRUE;<br>> +   } else {<br>> +      return EGL_FALSE;<br>> +   }<br>> +}<br>>  /**<br>>   * Return true if a client API enum is recognized.<br>>   */<br>>  static inline EGLBoolean<br>>  _eglIsApiValid(EGLenum api)<br><br>There's only one user of this function, and I suspect there will only<br>every be that one user.  I would do a first patch that moves this<br>function into eglapi.c... maybe even just move this function into the<br>only caller.  Then this patch would change the way it works.<br><br></blockquote><div><br></div></span><div>Makes sense I'll go ahead and move it to the actual caller.</div><div class=""><div id=":rc" class="" tabindex="0"><img class="" src="https://ssl.gstatic.com/ui/v1/icons/mail/images/cleardot.gif" style=""></div></div><div class=""><span class="im"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">>  {<br>> -   return (api >= _EGL_API_FIRST_API && api <= _EGL_API_LAST_API);<br>> +   _EGLDisplay *dpy = _eglGlobal.DisplayList;<br>> +<br>> +   while (dpy) {<br>> +      if (_eglDisplaySupportsApi(dpy, api) == EGL_TRUE)<br><br>Even without changing _eglDisplaySupportsApi to return bool, you can<br>drop the '== EGL_TRUE'.<br><div><br>> +         return EGL_TRUE;<br>> +<br>> +      dpy = dpy->Next;<br>> +   }<br>> +<br>> +   return EGL_FALSE;<br>>  }<br>><br>><br><br></div></blockquote></span></div></div></div></div></div>