Thanks for the review. > I'd probably put the version checking logic into a helper function, but not a big deal.
Good idea. I'll do that in a follow on patch. > It looks like we're not doing as extensive version checking in the glx code. It's actually the same amount of checking. But on GLX we somehow need to return a different error for ES profiles to make piglit happy. Jose ________________________________________ From: Brian Paul <[email protected]> Sent: 14 November 2014 18:20 To: Jose Fonseca; [email protected] Subject: Re: [PATCH 2/2] st/wgl: Implement WGL_EXT_create_context_es/es2_profile. Just one minor comment below. Otherwise looks fine. Reviewed-by: Brian Paul <[email protected]> On 11/14/2014 10:20 AM, [email protected] wrote: > From: José Fonseca <[email protected]> > > Derived from st/glx's GLX_EXT_create_context_es/es2_profile implementation. > > Tested with an OpenGL ES 2.0 ApiTrace. > --- > src/gallium/state_trackers/wgl/stw_context.c | 74 > +++++++++++++--------- > src/gallium/state_trackers/wgl/stw_ext_context.c | 65 +++++++++---------- > .../state_trackers/wgl/stw_ext_extensionsstring.c | 2 + > 3 files changed, 78 insertions(+), 63 deletions(-) > > diff --git a/src/gallium/state_trackers/wgl/stw_context.c > b/src/gallium/state_trackers/wgl/stw_context.c > index 99debfd..2ed6c2bf 100644 > --- a/src/gallium/state_trackers/wgl/stw_context.c > +++ b/src/gallium/state_trackers/wgl/stw_context.c > @@ -201,35 +201,51 @@ stw_create_context_attribs(HDC hdc, INT iLayerPlane, > DHGLRC hShareContext, > if (contextFlags & WGL_CONTEXT_DEBUG_BIT_ARB) > attribs.flags |= ST_CONTEXT_FLAG_DEBUG; > > - /* There are no profiles before OpenGL 3.2. The > - * WGL_ARB_create_context_profile spec says: > - * > - * "If the requested OpenGL version is less than 3.2, > - * WGL_CONTEXT_PROFILE_MASK_ARB is ignored and the functionality of > the > - * context is determined solely by the requested version." > - * > - * The spec also says: > - * > - * "The default value for WGL_CONTEXT_PROFILE_MASK_ARB is > - * WGL_CONTEXT_CORE_PROFILE_BIT_ARB." > - * > - * The spec also says: > - * > - * "If version 3.1 is requested, the context returned may implement > - * any of the following versions: > - * > - * * Version 3.1. The GL_ARB_compatibility extension may or may not > - * be implemented, as determined by the implementation. > - * * The core profile of version 3.2 or greater." > - * > - * and because Mesa doesn't support GL_ARB_compatibility, the only chance > to > - * honour a 3.1 context is through core profile. > - */ > - attribs.profile = ST_PROFILE_DEFAULT; > - if (((majorVersion > 3 || (majorVersion == 3 && minorVersion >= 2)) > - && ((profileMask & WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB) == 0)) > || > - (majorVersion == 3 && minorVersion == 1)) > - attribs.profile = ST_PROFILE_OPENGL_CORE; > + switch (profileMask) { > + case WGL_CONTEXT_CORE_PROFILE_BIT_ARB: > + /* There are no profiles before OpenGL 3.2. The > + * WGL_ARB_create_context_profile spec says: > + * > + * "If the requested OpenGL version is less than 3.2, > + * WGL_CONTEXT_PROFILE_MASK_ARB is ignored and the functionality > + * of the context is determined solely by the requested version." > + */ > + if (majorVersion > 3 || (majorVersion == 3 && minorVersion >= 2)) { > + attribs.profile = ST_PROFILE_OPENGL_CORE; > + break; > + } > + /* fall-through */ > + case WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB: > + /* > + * The spec also says: > + * > + * "If version 3.1 is requested, the context returned may implement > + * any of the following versions: > + * > + * * Version 3.1. The GL_ARB_compatibility extension may or may > not > + * be implemented, as determined by the implementation. > + * * The core profile of version 3.2 or greater." > + * > + * and because Mesa doesn't support GL_ARB_compatibility, the only > chance to > + * honour a 3.1 context is through core profile. > + */ > + if (majorVersion == 3 && minorVersion == 1) { > + attribs.profile = ST_PROFILE_OPENGL_CORE; > + } else { > + attribs.profile = ST_PROFILE_DEFAULT; > + } > + break; > + case WGL_CONTEXT_ES_PROFILE_BIT_EXT: > + if (majorVersion >= 2) { > + attribs.profile = ST_PROFILE_OPENGL_ES2; > + } else { > + attribs.profile = ST_PROFILE_OPENGL_ES1; > + } > + break; > + default: > + assert(0); > + goto no_st_ctx; > + } > > ctx->st = stw_dev->stapi->create_context(stw_dev->stapi, > stw_dev->smapi, &attribs, &ctx_err, shareCtx ? shareCtx->st : > NULL); > diff --git a/src/gallium/state_trackers/wgl/stw_ext_context.c > b/src/gallium/state_trackers/wgl/stw_ext_context.c > index 451f330..8a96cac 100644 > --- a/src/gallium/state_trackers/wgl/stw_ext_context.c > +++ b/src/gallium/state_trackers/wgl/stw_ext_context.c > @@ -62,6 +62,8 @@ wglCreateContextAttribsARB(HDC hDC, HGLRC hShareContext, > const int *attribList) > int profileMask = WGL_CONTEXT_CORE_PROFILE_BIT_ARB; > int i; > BOOL done = FALSE; > + const int contextFlagsAll = (WGL_CONTEXT_DEBUG_BIT_ARB | > + WGL_CONTEXT_FORWARD_COMPATIBLE_BIT_ARB); > > /* parse attrib_list */ > if (attribList) { > @@ -94,34 +96,36 @@ wglCreateContextAttribsARB(HDC hDC, HGLRC hShareContext, > const int *attribList) > } > } > > + /* check contextFlags */ > + if (contextFlags & ~contextFlagsAll) { > + SetLastError(ERROR_INVALID_PARAMETER); > + return NULL; > + } > + > + /* check profileMask */ > + if (profileMask != WGL_CONTEXT_CORE_PROFILE_BIT_ARB && > + profileMask != WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB && > + profileMask != WGL_CONTEXT_ES_PROFILE_BIT_EXT) { > + SetLastError(ERROR_INVALID_PROFILE_ARB); > + return NULL; > + } > + > /* check version (generate ERROR_INVALID_VERSION_ARB if bad) */ > - switch (majorVersion) { > - case 1: > - if (minorVersion < 0 || minorVersion > 5) { > - SetLastError(ERROR_INVALID_VERSION_ARB); > - return 0; > - } > - break; > - case 2: > - if (minorVersion < 0 || minorVersion > 1) { > - SetLastError(ERROR_INVALID_VERSION_ARB); > - return 0; > - } > - break; > - case 3: > - if (minorVersion < 0 || minorVersion > 3) { > - SetLastError(ERROR_INVALID_VERSION_ARB); > - return 0; > - } > - break; > - case 4: > - if (minorVersion < 0 || minorVersion > 2) { > - SetLastError(ERROR_INVALID_VERSION_ARB); > - return 0; > - } > - break; > - default: > - return 0; > + if (majorVersion <= 0 || > + minorVersion < 0 || > + (profileMask != WGL_CONTEXT_ES_PROFILE_BIT_EXT && > + ((majorVersion == 1 && minorVersion > 5) || > + (majorVersion == 2 && minorVersion > 1) || > + (majorVersion == 3 && minorVersion > 3) || > + (majorVersion == 4 && minorVersion > 5) || > + majorVersion > 4)) || > + (profileMask == WGL_CONTEXT_ES_PROFILE_BIT_EXT && > + ((majorVersion == 1 && minorVersion > 1) || > + (majorVersion == 2 && minorVersion > 0) || > + (majorVersion == 3 && minorVersion > 1) || > + majorVersion > 3))) { I'd probably put the version checking logic into a helper function, but not a big deal. It looks like we're not doing as extensive version checking in the glx code. > + SetLastError(ERROR_INVALID_VERSION_ARB); > + return NULL; > } > > if ((contextFlags & WGL_CONTEXT_FORWARD_COMPATIBLE_BIT_ARB) && > @@ -130,13 +134,6 @@ wglCreateContextAttribsARB(HDC hDC, HGLRC hShareContext, > const int *attribList) > return 0; > } > > - /* check profileMask */ > - if (profileMask != WGL_CONTEXT_CORE_PROFILE_BIT_ARB && > - profileMask != WGL_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB) { > - SetLastError(ERROR_INVALID_PROFILE_ARB); > - return 0; > - } > - > /* Get pointer to OPENGL32.DLL's wglCreate/DeleteContext() functions */ > if (opengl_lib == 0) { > /* Open the OPENGL32.DLL library */ > diff --git a/src/gallium/state_trackers/wgl/stw_ext_extensionsstring.c > b/src/gallium/state_trackers/wgl/stw_ext_extensionsstring.c > index 06a152b..a8c085a 100644 > --- a/src/gallium/state_trackers/wgl/stw_ext_extensionsstring.c > +++ b/src/gallium/state_trackers/wgl/stw_ext_extensionsstring.c > @@ -41,6 +41,8 @@ static const char *stw_extension_string = > "WGL_ARB_multisample " > "WGL_ARB_pbuffer " > "WGL_ARB_pixel_format " > + "WGL_EXT_create_context_es_profile " > + "WGL_EXT_create_context_es2_profile " > /* "WGL_EXT_swap_interval " */ > "WGL_EXT_extensions_string"; > > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
