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

[Tomasz Lis]

2012/12/4 Jon TURNEY <jon.turney at dronecode.org.uk>

> 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.
>
> This is true, the issue isn't closed yet.
Please refer to the discussion over previous version of the patch. It ended
with me asking:

Do you still think we need a change here? Do you think we should filter
whole configs, or only attributes within configs?
Do you have a concept on how to implement it?
This seem to be a big issue, maybe we could commit the changes as they are,
and then think about a solution?


> >>
> >> 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.
>
>
The ARB_framebuffer_sRGB spec says it is GLX extension.
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.
Agreed to change required GLX version to (0,0).


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

You're right. Agreed to remove.


>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20121205/15ec1db3/attachment-0001.html>