<div dir="ltr"><div><div>Thank you for your comments.<br></div><div><br>GCC Warnings:<br><br>There are no warnings in "glxcmds.c" during build. I think GCC only warns when multiple logic/bitwise operators are used in a row.<br>
<br></div><div>Setting context renderType from config renderType:<br></div><br>Invalid value of renderType in configs may be caused by DRI drivers which got used to the fact that the parameter was irrelevant.<br></div>Note that the two cases where renderType flags are empty will give identical results to what unpatched code would do.<br>
<div><div><div><div><div><div class="gmail_extra">If you still think glXCreateContext should fail if config renderType is invalid, I can do this change.<br></div><div class="gmail_extra"><br>drawableType setting in glXChooseFBConfig vs glXChooseVisual:<br>
</div><div class="gmail_extra">I will make a separate commit out of this. The justification is that visual configs are on-screen configs - so they have to support drawing on a window, by definition.<br></div><div class="gmail_extra">
<br><br><div class="gmail_quote">2013/7/15 Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="im">On 07/15/2013 07:28 AM, Tomasz Lis wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This patch sets the correct values of renderType and drawableType<br>
in glXCreateContext and init_fbconfig_for_chooser routines.<br>
---<br>
src/glx/glxcmds.c | 32 +++++++++++++++++++++++++++---<u></u>--<br>
1 file changed, 27 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c<br>
index 51b2237..967dab3 100644<br>
--- a/src/glx/glxcmds.c<br>
+++ b/src/glx/glxcmds.c<br>
@@ -373,7 +373,25 @@ glXCreateContext(Display * dpy, XVisualInfo * vis,<br>
return None;<br>
}<br>
<br>
- renderType = config->rgbMode ? GLX_RGBA_TYPE : GLX_COLOR_INDEX_TYPE;<br>
+ /* Choose the context render type based on DRI config values.<br>
+ * It is unusual to set this type from config, but we have no other choice,<br>
+ * as this old API does not provide renderType parameter. */<br>
+ if (config->renderType & GLX_RGBA_FLOAT_BIT_ARB) {<br>
</blockquote>
<br></div>
I /think/ GCC will generate a warning about this. Something like 'suggest parenthesis around ...". Could you double check that and fix the warning if it exists?<div><br>
<br> <br></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ renderType = GLX_RGBA_FLOAT_TYPE_ARB;<br>
+ } else if (config->renderType & GLX_RGBA_UNSIGNED_FLOAT_BIT_<u></u>EXT) {<br>
+ renderType = GLX_RGBA_UNSIGNED_FLOAT_TYPE_<u></u>EXT;<br>
+ } else if (config->renderType & GLX_RGBA_BIT) {<br>
+ renderType = GLX_RGBA_TYPE;<br>
+ } else if (config->renderType & GLX_COLOR_INDEX_BIT) {<br>
+ renderType = GLX_COLOR_INDEX_TYPE;<br>
+ } else if (config->rgbMode == GL_TRUE) {<br>
</blockquote>
<br></div>
} else if (config->rgbMode) {<br>
<br>
However, I'm unsure about this. It seems like if the renderType is not correctly set that something has gone seriously wrong. Shouldn't this function just fail?<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ /* If we're here, then renderType is not set correctly.<br>
+ * Let's use a safeguard - any TrueColor or DirectColor mode is RGB mode */<br>
+ renderType = GLX_RGBA_TYPE;<br>
+ } else {<br>
+ /* Safeguard - only one option left, all non-RGB modes are indexed modes */<br>
+ renderType = GLX_COLOR_INDEX_TYPE;<br>
+ }<br>
#endif<br>
<br>
return CreateContext(dpy, vis->visualid, config, shareList, allowDirect,<br>
@@ -860,12 +878,17 @@ init_fbconfig_for_chooser(<u></u>struct glx_config * config,<br>
config->visualID = (XID) GLX_DONT_CARE;<br>
config->visualType = GLX_DONT_CARE;<br>
<br>
- /* glXChooseFBConfig specifies different defaults for these two than<br>
+ /* glXChooseFBConfig specifies different defaults for these properties than<br>
* glXChooseVisual.<br>
*/<br>
if (fbconfig_style_tags) {<br>
config->rgbMode = GL_TRUE;<br>
config->doubleBufferMode = GLX_DONT_CARE;<br>
+ /* allow any kind of drawable, including those for off-screen buffers */<br>
+ config->drawableType = 0;<br>
+ } else {<br>
+ /* allow configs which support on-screen drawing */<br>
+ config->drawableType = GLX_WINDOW_BIT;<br>
}<br>
</blockquote>
<br></div>
This feels like it should be a separate commit. Is there some spec text or further justification for this? We ought to at least have a piglit test case...<div class=""><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
config->visualRating = GLX_DONT_CARE;<br>
@@ -876,9 +899,8 @@ init_fbconfig_for_chooser(<u></u>struct glx_config * config,<br>
config->transparentAlpha = GLX_DONT_CARE;<br>
config->transparentIndex = GLX_DONT_CARE;<br>
<br>
- config->drawableType = GLX_WINDOW_BIT;<br>
- config->renderType =<br>
- (config->rgbMode) ? GLX_RGBA_BIT : GLX_COLOR_INDEX_BIT;<br>
+ /* Set GLX_RENDER_TYPE property to not expect any flags by default. */<br>
+ config->renderType = 0;<br>
config->xRenderable = GLX_DONT_CARE;<br>
config->fbconfigID = (GLXFBConfigID) (GLX_DONT_CARE);<br>
<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div></div></div></div></div></div>