<br><br><div class="gmail_quote">2012/12/4 Jon TURNEY <span dir="ltr"><<a href="mailto:jon.turney@dronecode.org.uk" target="_blank">jon.turney@dronecode.org.uk</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 04/12/2012 15:28, Tomasz Lis wrote:<br>
> These are not all the changes, sorry for that.<br>
><br>
> It seems I don't know how to correctly re-send the patch.<br>
><br>
</div>> 2012/12/4 <<a href="mailto:listom-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org">listom-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org</a>><br>
><br>
>> From: Tomasz Lis <<a href="mailto:tomasz.lis-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org">tomasz.lis-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org</a>><br>
<div class="im">>><br>
>> Changes to correctly initialize the sRGB capability attribute and<br>
>> transfer it between XServer and the client. Modifications include<br>
>> extension string, transferring visual config attribs and fbconfig<br>
>> attribs. Also, attribute is initialized in the modules which do not<br>
>> really use it (xquartz and xwin).<br>
>> This version advertises both ARB and EXT strings, and initializes<br>
>> the capability to default value of FALSE.<br>
<br>
</div>Your updated patch doesn't seem to address idr's comment that you must not<br>
send the GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT token to clients that don't<br>
understand it.<br>
<br>
I am not sure of the mechanism for a client to indicate this understanding.<br>
<br></blockquote><div>This is true, the issue isn't closed yet.<br>Please refer to the discussion over previous version of the patch. It ended with me asking:<br><br>Do you still think we need a change here? Do you think we should filter whole configs, or only attributes within configs?<br>
Do you have a concept on how to implement it?<br>
This seem to be a big issue, maybe we could commit the changes as they are, and then think about a solution?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>><br>
>> Signed-off-by: Tomasz Lis <<a href="mailto:tomasz.lis-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org">tomasz.lis-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org</a>><br>
<div class="im">>> ---<br>
>> glx/extension_string.c | 2 ++<br>
>> glx/extension_string.h | 2 ++<br>
>> glx/glxcmds.c | 7 +++++--<br>
>> glx/glxdri2.c | 7 +++++++<br>
>> glx/glxdricommon.c | 4 +++-<br>
>> glx/glxscreens.c | 2 ++<br>
>> glx/glxscreens.h | 3 +++<br>
>> hw/xquartz/GL/visualConfigs.c | 3 +++<br>
>> hw/xwin/glx/indirect.c | 2 ++<br>
>> 9 files changed, 29 insertions(+), 3 deletions(-)<br>
>><br>
>> diff --git a/glx/extension_string.c b/glx/extension_string.c<br>
>> index 544ca1f..70dc915 100644<br>
>> --- a/glx/extension_string.c<br>
>> +++ b/glx/extension_string.c<br>
>> @@ -71,9 +71,11 @@ static const struct extension_info<br>
>> known_glx_extensions[] = {<br>
>> { GLX(ARB_create_context), VER(0,0), N, },<br>
>> { GLX(ARB_create_context_profile), VER(0,0), N, },<br>
>> { GLX(ARB_create_context_robustness), VER(0,0), N, },<br>
>> + { GLX(ARB_framebuffer_sRGB), VER(1,1), N, },<br>
>> { GLX(ARB_multisample), VER(1,4), Y, },<br>
>><br>
>> { GLX(EXT_create_context_es2_profile), VER(0,0), N, },<br>
>> + { GLX(EXT_framebuffer_sRGB), VER(1,1), N, },<br>
<br>
</div>According to the comments above, writing VER(1,1) here means that this<br>
extension is required in GLX 1.1 (although this data is not used in the X<br>
server). I don't think that's true, and you should be writing VER(0,0).<br>
<br>
I'm not entirely sure that this should be reported as a GLX extension at all.<br>
<div class="im"><br></div></blockquote><div><br>The ARB_framebuffer_sRGB spec says it is GLX extension.<br>But you are completely right with the versions - OGL 1.3 has no relation to this extension, so GLX specs up to 1.4 definitely don't require it.<br>
Agreed to change required GLX version to (0,0).<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
>> --- a/glx/glxscreens.c<br>
>> +++ b/glx/glxscreens.c<br>
>> @@ -164,7 +164,9 @@ static char GLXServerVendorName[] = "SGI";<br>
>> unsigned glxMajorVersion = SERVER_GLX_MAJOR_VERSION;<br>
>> unsigned glxMinorVersion = SERVER_GLX_MINOR_VERSION;<br>
>> static char GLXServerExtensions[] =<br>
>> + "GLX_ARB_framebuffer_sRGB "<br>
>> "GLX_ARB_multisample "<br>
>> + "GLX_EXT_framebuffer_sRGB "<br>
>> "GLX_EXT_visual_info "<br>
>> "GLX_EXT_visual_rating "<br>
>> "GLX_EXT_import_context "<br>
<br>
</div>This is the static list of extensions reported for swrast. I don't think this<br>
extension belongs here.<br></blockquote><div><br>You're right. Agreed to remove.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
_______________________________________________<br>
<a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development<br>
Archives: <a href="http://lists.x.org/archives/xorg-devel" target="_blank">http://lists.x.org/archives/xorg-devel</a><br>
Info: <a href="http://lists.x.org/mailman/listinfo/xorg-devel" target="_blank">http://lists.x.org/mailman/listinfo/xorg-devel</a><br>
</div></div></blockquote></div><br>