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

Ian Romanick idr at freedesktop.org
Mon Nov 26 09:41:58 PST 2012


On 11/22/2012 07:08 AM, Tomasz Lis wrote:
> From: Tomasz Lis <tomasz.lis at intel.com>
>
> 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).

Please word-wrap commit messages to 72 characters.

> Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>
> ---
>   glx/extension_string.c        |    1 +
>   glx/extension_string.h        |    1 +
>   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, 27 insertions(+), 3 deletions(-)
>
> diff --git a/glx/extension_string.c b/glx/extension_string.c
> index 544ca1f..47a9746 100644
> --- a/glx/extension_string.c
> +++ b/glx/extension_string.c
> @@ -74,6 +74,7 @@ static const struct extension_info known_glx_extensions[] = {
>       { 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, },

Also advertise the ARB string.

>       { GLX(EXT_import_context),          VER(0,0), Y, },
>       { GLX(EXT_texture_from_pixmap),     VER(0,0), Y, },
>       { GLX(EXT_visual_info),             VER(0,0), Y, },
> diff --git a/glx/extension_string.h b/glx/extension_string.h
> index 7a4a8b1..a91385f 100644
> --- a/glx/extension_string.h
> +++ b/glx/extension_string.h
> @@ -41,6 +41,7 @@ enum {
>       ARB_create_context_robustness_bit,
>       ARB_multisample_bit,
>       EXT_create_context_es2_profile_bit,
> +    EXT_framebuffer_sRGB_bit,
>       EXT_import_context_bit,
>       EXT_texture_from_pixmap_bit,
>       EXT_visual_info_bit,
> diff --git a/glx/glxcmds.c b/glx/glxcmds.c
> index c1f4e22..720a1a8 100644
> --- a/glx/glxcmds.c
> +++ b/glx/glxcmds.c
> @@ -913,7 +913,7 @@ __glXDisp_CopyContext(__GLXclientState * cl, GLbyte * pc)
>
>   enum {
>       GLX_VIS_CONFIG_UNPAIRED = 18,
> -    GLX_VIS_CONFIG_PAIRED = 20
> +    GLX_VIS_CONFIG_PAIRED = 22
>   };
>
>   enum {
> @@ -1005,6 +1005,8 @@ __glXDisp_GetVisualConfigs(__GLXclientState * cl, GLbyte * pc)
>           buf[p++] = modes->samples;
>           buf[p++] = GLX_SAMPLE_BUFFERS_SGIS;
>           buf[p++] = modes->sampleBuffers;
> +        buf[p++] = GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT;
> +        buf[p++] = modes->sRGBCapable;
>           buf[p++] = 0;           /* copy over visualSelectGroup (GLX_VISUAL_SELECT_GROUP_SGIX)? */
>           buf[p++] = 0;
>
> @@ -1017,7 +1019,7 @@ __glXDisp_GetVisualConfigs(__GLXclientState * cl, GLbyte * pc)
>       return Success;
>   }
>
> -#define __GLX_TOTAL_FBCONFIG_ATTRIBS (36)
> +#define __GLX_TOTAL_FBCONFIG_ATTRIBS (37)
>   #define __GLX_FBCONFIG_ATTRIBS_LENGTH (__GLX_TOTAL_FBCONFIG_ATTRIBS * 2)
>   /**
>    * Send the set of GLXFBConfigs to the client.  There is not currently
> @@ -1109,6 +1111,7 @@ DoGetFBConfigs(__GLXclientState * cl, unsigned screen)
>           WRITE_PAIR(GLX_BIND_TO_MIPMAP_TEXTURE_EXT, modes->bindToMipmapTexture);
>           WRITE_PAIR(GLX_BIND_TO_TEXTURE_TARGETS_EXT,
>                      modes->bindToTextureTargets);
> +        WRITE_PAIR(GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT, modes->sRGBCapable);

I believe that sRGB visuals and fbconfigs should only be sent to the 
client if the client has said that it supports one of the sRGB 
extensions.  Otherwise the client doesn't know what 
GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB means.  It will either drop the visuals 
or treat the sRGB and non-sRGB visuals the same.

Also, please use the _ARB enums instead.

>
>           if (client->swapped) {
>               __GLX_SWAP_INT_ARRAY(buf, __GLX_FBCONFIG_ATTRIBS_LENGTH);
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index bce1bfa..ef7afb4 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -882,6 +882,13 @@ initializeExtensions(__GLXDRIscreen * screen)
>                      "AIGLX: enabled GLX_SGI_swap_control and GLX_MESA_swap_control\n");
>       }
>
> +    /* enable EXT_framebuffer_sRGB extension (even if there are no sRGB capable fbconfigs) */
> +    {
> +        __glXEnableExtension(screen->glx_enable_bits,
> +                 "GLX_EXT_framebuffer_sRGB");
> +        LogMessage(X_INFO, "AIGLX: enabled GLX_EXT_framebuffer_sRGB\n");
> +    }
> +
>       for (i = 0; extensions[i]; i++) {
>   #ifdef __DRI_READ_DRAWABLE
>           if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) {
> diff --git a/glx/glxdricommon.c b/glx/glxdricommon.c
> index c90f380..b027f24 100644
> --- a/glx/glxdricommon.c
> +++ b/glx/glxdricommon.c
> @@ -105,7 +105,9 @@ __ATTRIB(__DRI_ATTRIB_BUFFER_SIZE, rgbBits),
>           __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb),
>           __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGBA, bindToTextureRgba),
>           __ATTRIB(__DRI_ATTRIB_BIND_TO_MIPMAP_TEXTURE, bindToMipmapTexture),
> -        __ATTRIB(__DRI_ATTRIB_YINVERTED, yInverted),};
> +        __ATTRIB(__DRI_ATTRIB_YINVERTED, yInverted),
> +        __ATTRIB(__DRI_ATTRIB_FRAMEBUFFER_SRGB_CAPABLE, sRGBCapable),
> +        };
>
>   static void
>   setScalar(__GLXconfig * config, unsigned int attrib, unsigned int value)
> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
> index 61d590c..e87256d 100644
> --- 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 "
> diff --git a/glx/glxscreens.h b/glx/glxscreens.h
> index b29df58..0a7b604 100644
> --- a/glx/glxscreens.h
> +++ b/glx/glxscreens.h
> @@ -102,6 +102,9 @@ struct __GLXconfig {
>       GLint bindToMipmapTexture;
>       GLint bindToTextureTargets;
>       GLint yInverted;
> +
> +    /* ARB_framebuffer_sRGB */
> +    GLint sRGBCapable;
>   };
>
>   GLint glxConvertToXVisualType(int visualType);
> diff --git a/hw/xquartz/GL/visualConfigs.c b/hw/xquartz/GL/visualConfigs.c
> index e37eefb..18518af 100644
> --- a/hw/xquartz/GL/visualConfigs.c
> +++ b/hw/xquartz/GL/visualConfigs.c
> @@ -326,6 +326,9 @@ __glXAquaCreateVisualConfigs(int *numConfigsPtr, int screenNumber)
>                                           c->bindToTextureTargets = 0;
>                                           c->yInverted = 0;
>
> +                                        /* EXT_framebuffer_sRGB */
> +                                        c->sRGBCapable = GL_DONT_CARE;

This is wrong.  sRGBCapable should be either True or False. 
GL_DONT_CARE (~0) is not a valid value.

> +
>                                           c = c->next;
>                                       }
>                                   }
> diff --git a/hw/xwin/glx/indirect.c b/hw/xwin/glx/indirect.c
> index c0069a2..5b339af 100644
> --- a/hw/xwin/glx/indirect.c
> +++ b/hw/xwin/glx/indirect.c
> @@ -2017,6 +2017,7 @@ glxWinCreateConfigs(HDC hdc, glxWinScreen * screen)
>           c->base.bindToMipmapTexture = -1;
>           c->base.bindToTextureTargets = -1;
>           c->base.yInverted = -1;
> +        c->base.sRGBCapable = -1;
>
>           n++;
>
> @@ -2411,6 +2412,7 @@ glxWinCreateConfigsExt(HDC hdc, glxWinScreen * screen)
>               GLX_TEXTURE_1D_BIT_EXT | GLX_TEXTURE_2D_BIT_EXT |
>               GLX_TEXTURE_RECTANGLE_BIT_EXT;
>           c->base.yInverted = -1;
> +        c->base.sRGBCapable = -1;
>
>           n++;
>
>



More information about the xorg-devel mailing list