[Mesa-dev] [PATCH] egl: Add OpenGL_ES to API string regardless of GLES version

[Matt Turner]

On Tue, May 17, 2016 at 9:59 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Thu, May 12, 2016 at 06:21:38PM +0100, Plamena Manolova wrote:
>> According to the EGL specifications eglQueryString(EGL_CLIENT_APIS)
>> should return a string containing a combination of "OpenGL", "OpenGL_ES"
>> and "OpenVG", any other values would be considered invalid. Due to this
>> when the API string is constructed, the version of GLES should be
>> disregarded and "OpenGL_ES" should be attached once instead of
>> "OpenGL_ES2" and "OpenGL_ES3".
>
> Adding Matt who originally authored this...

I just move it actually. The code looks to be much older.

> I think you misstated this. It's not invalid to have any other value. It's
> invalid to not have one of the 3 values, which I suppose is technically possible
> if you say support ES2, but not ES or GL (for example)
>
>  "Returns a string describing which client rendering APIs are supported. The
>   string contains a space-separate list of API names. The list must include at
>   least one of OpenGL, OpenGL_ES, or OpenVG. These strings correspond
>   respectively to values EGL_OPENGL_API, EGL_OPENGL_ES_API, and EGL_OPENVG_API
>   of the eglBindAPI, api argument."
>
> I am concerned by this change since I genuinely have no clue how EGL clients
> might currently be depending on this, and as such could I request that you not
> change the existing behavior (spit out when ES2 or ES3). At the bottom I put an
> untested version of what i would have done.
>
> This fixes a test apparently, in the future, could you please mention that in
> the commit message?
>
>>
>> Signed-off-by: Plamena Manolova <plamena.manolova at intel.com>
>> ---
>>  src/egl/main/eglapi.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
>> index eb612c0..be2c90f 100644
>> --- a/src/egl/main/eglapi.c
>> +++ b/src/egl/main/eglapi.c
>> @@ -431,14 +431,11 @@ _eglCreateAPIsString(_EGLDisplay *dpy)
>>     if (dpy->ClientAPIs & EGL_OPENGL_BIT)
>>        strcat(dpy->ClientAPIsString, "OpenGL ");
>>
>> -   if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT)
>> +   if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT ||
>> +       dpy->ClientAPIs & EGL_OPENGL_ES2_BIT ||
>> +       dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR) {
>>        strcat(dpy->ClientAPIsString, "OpenGL_ES ");
>> -
>> -   if (dpy->ClientAPIs & EGL_OPENGL_ES2_BIT)
>> -      strcat(dpy->ClientAPIsString, "OpenGL_ES2 ");
>> -
>> -   if (dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)
>> -      strcat(dpy->ClientAPIsString, "OpenGL_ES3 ");
>> +   }
>>
>>     if (dpy->ClientAPIs & EGL_OPENVG_BIT)
>>        strcat(dpy->ClientAPIsString, "OpenVG ");
>
>
> diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> index eb612c0..8101840 100644
> --- a/src/egl/main/eglapi.c
> +++ b/src/egl/main/eglapi.c
> @@ -431,8 +431,12 @@ _eglCreateAPIsString(_EGLDisplay *dpy)
>     if (dpy->ClientAPIs & EGL_OPENGL_BIT)
>        strcat(dpy->ClientAPIsString, "OpenGL ");
>
> +   /* If any ES is supported, make sure we get that */
>     if (dpy->ClientAPIs & EGL_OPENGL_ES_BIT)
>        strcat(dpy->ClientAPIsString, "OpenGL_ES ");
> +   else if (dpy->ClientAPIs & EGL_OPENGL_ES2_BIT ||
> +            dpy->ClientAPIs & EGL_OPENGL_ES3_BIT_KHR)
> +      strcat(dpy->ClientAPIsString, "OpenGL_ES ");

This is kinda weird.

I think basically what you're suggesting is doing what Plamena
suggests -- strcat "OpenGL_ES" if *any* ES API is supported -- but
keep the code to emit OpenGL_ES{2,3}.

I'm fine with that, for however little it's worth.

I don't see any rationale for "OpenGL_ES2"/"OpenGL_ES3" though. The 8
year old commit d5078b94 added "OpenGL_ES2" and Chad cargo culted the
ES3 string when that support was added. It very well might be that
nothing actually depends on these strings...

>
>     if (dpy->ClientAPIs & EGL_OPENGL_ES2_BIT)
>        strcat(dpy->ClientAPIsString, "OpenGL_ES2 ");