<div dir="ltr">Thank you. <div> I will rename the function to validate_renderType_against_config and make it return bool. </div><div>I will fix the brace. </div></div><div class="gmail_extra"> <div class="gmail_quote"> 2013/7/15 Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div class="HOEnZb"><div class="h5">On 07/15/2013 07:28 AM, Tomasz Lis wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> The change is to correctly handle the value of renderType in GLX context. In case of the value being incorrect, context creation fails. --- src/glx/dri2_glx.c | 11 +++++++++++ src/glx/dri_glx.c | 6 ++++++ src/glx/drisw_glx.c | 12 ++++++++++++ src/glx/glxclient.h | 2 ++ src/glx/glxcmds.c | 25 +++++++++++++++++++++++++ src/glx/indirect_glx.c | 6 ++++++ src/mesa/drivers/x11/fakeglx.c | 5 ++++- 7 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c index d60c675..539e7f1 100644 --- a/src/glx/dri2_glx.c +++ b/src/glx/dri2_glx.c @@ -205,6 +205,12 @@ dri2_create_context(struct glx_screen *base, __GLXDRIconfigPrivate *config = (__GLXDRIconfigPrivate *) config_base; __DRIcontext *shared = NULL; + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(config_base, renderType); + if (!renderType) { + return NULL; + } + </blockquote> </div></div> Every place that uses verifyConextRenderTypeGLX should be: if (!verifyContextRenderTypeGLX(config_base, renderType)) return NULL; And see blow.<div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> if (shareList) { /* If the shareList context is not a DRI2 context, we cannot possibly * create a DRI2 context that shares it. @@ -277,6 +283,11 @@ dri2_create_context_attribs(struct glx_screen *base, error)) goto error_exit; + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(config_base, renderType); + if (!renderType) + goto error_exit; + if (shareList) { pcp_shared = (struct dri2_context *) shareList; shared = pcp_shared->driContext; diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c index cc45734..de44451 100644 --- a/src/glx/dri_glx.c +++ b/src/glx/dri_glx.c @@ -581,6 +581,12 @@ dri_create_context(struct glx_screen *base, if (!psc->base.driScreen) return NULL; + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(config_base, renderType); + if (!renderType) { + return NULL; + } + if (shareList) { /* If the shareList context is not a DRI context, we cannot possibly * create a DRI context that shares it. diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c index ef0e52b..0e45607 100644 --- a/src/glx/drisw_glx.c +++ b/src/glx/drisw_glx.c @@ -380,6 +380,12 @@ drisw_create_context(struct glx_screen *base, if (!psc->base.driScreen) return NULL; + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(config_base, renderType); + if (!renderType) { + return NULL; + } + if (shareList) { /* If the shareList context is not a DRISW context, we cannot possibly * create a DRISW context that shares it. @@ -451,6 +457,12 @@ drisw_create_context_attribs(struct glx_screen *base, error)) return NULL; + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(config_base, renderType); + if (!renderType) { + return NULL; + } + if (reset != __DRI_CTX_RESET_NO_NOTIFICATION) return NULL; diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h index fc8f31c..0e78584 100644 --- a/src/glx/glxclient.h +++ b/src/glx/glxclient.h @@ -803,6 +803,8 @@ extern int applegl_create_display(struct glx_display *display); #endif +extern int verifyContextRenderTypeGLX(const struct glx_config *config, int renderType); + extern struct glx_drawable *GetGLXDrawable(Display *dpy, GLXDrawable drawable); extern int InitGLXDrawable(Display *dpy, struct glx_drawable *glxDraw, diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c index 99b0218..e5ae7f8 100644 --- a/src/glx/glxcmds.c +++ b/src/glx/glxcmds.c @@ -224,6 +224,31 @@ ValidateGLXFBConfig(Display * dpy, GLXFBConfig fbconfig) return NULL; } +/** + * Verifies context's GLX_RENDER_TYPE value with config. + * \param config GLX FBConfig which will support the returned renderType. + * \param renderType The context render type to be verified. + * \return Gives the approved value of context renderType, or 0 if no valid value was found. + */ </blockquote> </div></div> The users of this function only care that it returns 0 or non-0. Have it return type Bool instead. I also think this function could use a better name. validate_renderType_against_config seems more descriptive.<div class="im"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> +int +verifyContextRenderTypeGLX(const struct glx_config *config, int renderType) +{ + switch (renderType) + { </blockquote> </div> Opening curly brace should be on the same line with the switch.<div class="im"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + case GLX_RGBA_TYPE: + return (config->renderType & GLX_RGBA_BIT) ? GLX_RGBA_TYPE : 0; </blockquote> </div> return (config->renderType & GLX_RGBA_BIT) != 0; etc.<div class="HOEnZb"><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + case GLX_COLOR_INDEX_TYPE: + return (config->renderType & GLX_COLOR_INDEX_BIT) ? GLX_COLOR_INDEX_TYPE : 0; + case GLX_RGBA_FLOAT_TYPE_ARB: + return (config->renderType & GLX_RGBA_FLOAT_BIT_ARB) ? GLX_RGBA_FLOAT_TYPE_ARB : 0; + case GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT: + return (config->renderType & GLX_RGBA_UNSIGNED_FLOAT_BIT_EXT) ? GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT : 0; + default: + break; + } + return 0; +} + _X_HIDDEN Bool glx_context_init(struct glx_context *gc, struct glx_screen *psc, struct glx_config *config) diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c index fc5107d..c1a81d5 100644 --- a/src/glx/indirect_glx.c +++ b/src/glx/indirect_glx.c @@ -352,6 +352,12 @@ indirect_create_context(struct glx_screen *psc, return NULL; } + /* Check the renderType value */ + renderType = verifyContextRenderTypeGLX(mode, renderType); + if (!renderType) { + return NULL; + } + /* Allocate our context record */ gc = calloc(1, sizeof *gc); if (!gc) { diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c index 7a2cfbe..c48fc40 100644 --- a/src/mesa/drivers/x11/fakeglx.c +++ b/src/mesa/drivers/x11/fakeglx.c @@ -2325,7 +2325,10 @@ Fake_glXCreateNewContext( Display *dpy, GLXFBConfig config, XMesaVisual xmvis = (XMesaVisual) config; if (!dpy || !config || - (renderType != GLX_RGBA_TYPE && renderType != GLX_COLOR_INDEX_TYPE)) + (renderType != GLX_RGBA_TYPE && + renderType != GLX_COLOR_INDEX_TYPE && + renderType != GLX_RGBA_FLOAT_TYPE_ARB && + renderType != GLX_RGBA_UNSIGNED_FLOAT_TYPE_EXT)) return 0; glxCtx = CALLOC_STRUCT(fake_glx_context); </blockquote> </div></div></blockquote></div> </div>