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

Ian Romanick idr at freedesktop.org
Tue Jan 29 13:14:30 PST 2013


On 01/09/2013 05:46 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).
> This version advertises both ARB and EXT strings, and initializes
> the capability to default value of FALSE. It has corrected required
> GLX version and does not influence swrast. The sRGB capable attribute
> is attached only to those configs which do have this capability.
>
> Signed-off-by: Tomasz Lis <tomasz.lis at intel.com>

Other than the one thing below, this patch looks good.  Sorry for the 
massive lag.  I've been overwhelmed by OpenGL ES 3.0 work all year so 
far. :(

> ---
>   glx/extension_string.c        |    2 ++
>   glx/extension_string.h        |    2 ++
>   glx/glxcmds.c                 |   26 ++++++++++++++++++++++----
>   glx/glxdri2.c                 |    7 +++++++
>   glx/glxdricommon.c            |    4 +++-
>   glx/glxscreens.h              |    3 +++
>   hw/xquartz/GL/visualConfigs.c |    3 +++
>   hw/xwin/glx/indirect.c        |    2 ++
>   8 files changed, 44 insertions(+), 5 deletions(-)
>
> diff --git a/glx/extension_string.c b/glx/extension_string.c
> index 544ca1f..58f930f 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(0,0), N, },
>       { GLX(ARB_multisample),             VER(1,4), Y, },
>
>       { GLX(EXT_create_context_es2_profile), VER(0,0), N, },
> +    { GLX(EXT_framebuffer_sRGB),        VER(0,0), N, },
>       { 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, },

Do we want to expose both extensions together always?  As far as I can 
tell, these are the same other than the name.  Is that your 
understanding as well?  This is how I treat them on the client side.

> diff --git a/glx/extension_string.h b/glx/extension_string.h
> index 7a4a8b1..3ce5593 100644
> --- a/glx/extension_string.h
> +++ b/glx/extension_string.h
> @@ -39,8 +39,10 @@ enum {
>       ARB_create_context_bit = 0,
>       ARB_create_context_profile_bit,
>       ARB_create_context_robustness_bit,
> +    ARB_framebuffer_sRGB_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,

If we decide yes to the above, we can handle this the same way I did on 
the client:

#define EXT_framebuffer_sRGB_bit ARB_framebuffer_sRGB_bit

> diff --git a/glx/glxcmds.c b/glx/glxcmds.c
> index c1f4e22..5b7a628 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,8 +1005,17 @@ __glXDisp_GetVisualConfigs(__GLXclientState * cl, GLbyte * pc)
>           buf[p++] = modes->samples;
>           buf[p++] = GLX_SAMPLE_BUFFERS_SGIS;
>           buf[p++] = modes->sampleBuffers;
> -        buf[p++] = 0;           /* copy over visualSelectGroup (GLX_VISUAL_SELECT_GROUP_SGIX)? */
> -        buf[p++] = 0;
> +        /* Add attribute only if its value is not default. */
> +        if (modes->sRGBCapable != GL_FALSE) {
> +            buf[p++] = GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT;
> +            buf[p++] = modes->sRGBCapable;
> +        }
> +        /* Don't add visualSelectGroup (GLX_VISUAL_SELECT_GROUP_SGIX)?
> +         * Pad the remaining place with zeroes, so that attributes count is constant. */
> +        while (p < GLX_VIS_CONFIG_TOTAL) {
> +            buf[p++] = 0;
> +            buf[p++] = 0;
> +        }
>
>           assert(p == GLX_VIS_CONFIG_TOTAL);
>           if (client->swapped) {
> @@ -1017,7 +1026,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 +1118,15 @@ 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);
> +        /* Add attribute only if its value is not default. */
> +        if (modes->sRGBCapable != GL_FALSE) {
> +            WRITE_PAIR(GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT, modes->sRGBCapable);
> +        }
> +        /* Pad the remaining place with zeroes, so that attributes count is constant. */
> +        while (p < __GLX_FBCONFIG_ATTRIBS_LENGTH) {
> +            WRITE_PAIR(0, 0);
> +        }
> +        assert(p == __GLX_FBCONFIG_ATTRIBS_LENGTH);
>
>           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.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..5efd04f 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_FALSE;
> +
>                                           c = c->next;
>                                       }
>                                   }
> diff --git a/hw/xwin/glx/indirect.c b/hw/xwin/glx/indirect.c
> index c0069a2..e6faf19 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 = 0;
>
>           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 = 0;
>
>           n++;
>
>



More information about the xorg-devel mailing list