[Mesa-dev] [PATCH] egl: Check if API is supported when using eglBindAPI.
Manolova, Plamena
plamena.manolova at intel.com
Mon May 23 21:48:46 UTC 2016
Hi Brian,
On Mon, May 23, 2016 at 9:51 PM, Brian Paul <brianp at vmware.com> wrote:
> 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.
>
>
This would make the switch much tidier, I'll go ahead and make that change.
>
> + 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.
Yes a EGL_BAD_PARAMETER error is generated if for whatever reason this
function returns false
for all displays associated with the current thread. Actually it would make
more sense for the
default case to be evaluated in the caller. I'll go ahead and move it.
>
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 */
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160523/9b6fd48d/attachment-0001.html>
More information about the mesa-dev
mailing list