[PATCH] [xorg-dev][xserver] Full support of sRGB capable fbconfigs.

[Jon TURNEY]

On 04/12/2012 15:28, Tomasz Lis wrote:
> These are not all the changes, sorry for that.
> 
> It seems I don't know how to correctly re-send the patch.
> 
> 2012/12/4 <listom-Re5JQEeQqe8AvxtiuMwx3w at public.gmane.org>
> 
>> From: Tomasz Lis <tomasz.lis-ral2JQCrhuEAvxtiuMwx3w at public.gmane.org>
>>
>> Changes to correctly initialize the sRGB capability attribute and
>> transfer it between XServer and the client. Modifications include
>> extension string, transferring visual config attribs and fbconfig
>> attribs. Also, attribute is initialized in the modules which do not
>> really use it (xquartz and xwin).
>> This version advertises both ARB and EXT strings, and initializes
>> the capability to default value of FALSE.

Your updated patch doesn't seem to address idr's comment that you must not
send the GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT token to clients that don't
understand it.

I am not sure of the mechanism for a client to indicate this understanding.

>>
>> Signed-off-by: Tomasz Lis <tomasz.lis-ral2JQCrhuEAvxtiuMwx3w at public.gmane.org>
>> ---
>>  glx/extension_string.c        |    2 ++
>>  glx/extension_string.h        |    2 ++
>>  glx/glxcmds.c                 |    7 +++++--
>>  glx/glxdri2.c                 |    7 +++++++
>>  glx/glxdricommon.c            |    4 +++-
>>  glx/glxscreens.c              |    2 ++
>>  glx/glxscreens.h              |    3 +++
>>  hw/xquartz/GL/visualConfigs.c |    3 +++
>>  hw/xwin/glx/indirect.c        |    2 ++
>>  9 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/glx/extension_string.c b/glx/extension_string.c
>> index 544ca1f..70dc915 100644
>> --- a/glx/extension_string.c
>> +++ b/glx/extension_string.c
>> @@ -71,9 +71,11 @@ static const struct extension_info
>> known_glx_extensions[] = {
>>      { GLX(ARB_create_context),          VER(0,0), N, },
>>      { GLX(ARB_create_context_profile),  VER(0,0), N, },
>>      { GLX(ARB_create_context_robustness), VER(0,0), N, },
>> +    { GLX(ARB_framebuffer_sRGB),        VER(1,1), N, },
>>      { GLX(ARB_multisample),             VER(1,4), Y, },
>>
>>      { GLX(EXT_create_context_es2_profile), VER(0,0), N, },
>> +    { GLX(EXT_framebuffer_sRGB),        VER(1,1), N, },

According to the comments above, writing VER(1,1) here means that this
extension is required in GLX 1.1 (although this data is not used in the X
server).  I don't think that's true, and you should be writing VER(0,0).

I'm not entirely sure that this should be reported as a GLX extension at all.

>> --- a/glx/glxscreens.c
>> +++ b/glx/glxscreens.c
>> @@ -164,7 +164,9 @@ static char GLXServerVendorName[] = "SGI";
>>  unsigned glxMajorVersion = SERVER_GLX_MAJOR_VERSION;
>>  unsigned glxMinorVersion = SERVER_GLX_MINOR_VERSION;
>>  static char GLXServerExtensions[] =
>> +    "GLX_ARB_framebuffer_sRGB "
>>      "GLX_ARB_multisample "
>> +    "GLX_EXT_framebuffer_sRGB "
>>      "GLX_EXT_visual_info "
>>      "GLX_EXT_visual_rating "
>>      "GLX_EXT_import_context "

This is the static list of extensions reported for swrast.  I don't think this
extension belongs here.