<div dir="ltr">Hi Brian, <div class="gmail_extra"> <div class="gmail_quote">On Mon, May 23, 2016 at 9:51 PM, Brian Paul <<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>> wrote: <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">Hi Plamena, Some style nitpicks below.Â Feel free to take 'em or leave 'em. I'm not too involved in EGL.<div><div class="h5"> On 05/23/2016 10:27 AM, Plamena Manolova wrote: <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"> 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 <<a href="mailto:plamena.manolova@intel.com" target="_blank">plamena.manolova@intel.com</a>> --- Â 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; </blockquote> > +Â Â Â Â Â break; </div></div> I'd probably do: Â return !!(dpy->ClientAPIs & EGL_OPENGL_BIT); and similar for these cases to make it a little more concise. </blockquote><div> </div><div>This would make the switch much tidier, I'll go ahead and make that change.</div><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"> <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"> +Â Â Â 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; </blockquote> 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.</blockquote><div> </div><div>Yes a EGL_BAD_PARAMETER error is generated if for whatever reason this function returns falseÂ </div><div>for all displays associated with the current thread. Actually it would make more sense for theÂ </div><div>default case to be evaluated in the caller. I'll go ahead and move it.</div><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">Â </blockquote><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"> Whitespace note: we normally indent 'case' the same amount as the 'switch'.<div class=""><div class="h5"> <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"> +Â Â } + +Â Â 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 */ </blockquote> </div></div></blockquote></div> </div></div>