<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>