We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 1985 - [PATCH] Add support for requesting sRGB capable visuals.
Summary: [PATCH] Add support for requesting sRGB capable visuals.
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: video (show other bugs)
Version: HG 2.0
Hardware: All All
: P2 minor
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-25 08:07 UTC by David Gow
Modified: 2013-10-21 01:44 UTC (History)
1 user (show)

See Also:

Attachments
Add SDL_GL_FRAMEBUFFER_SRGB_CAPABLE and implement on Win32 and X11 (4.22 KB, patch)
2013-07-25 08:07 UTC, David Gow 		Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Gow 2013-07-25 08:07:37 UTC 
Created attachment 1251 [details]
Add SDL_GL_FRAMEBUFFER_SRGB_CAPABLE and implement on Win32 and X11

According to the GL_ARB_framebuffer_sRGB spec, you should request an sRGB capable visual before calling glEnable(GL_FRAMEBUFFER_SRGB). While all drivers I know of do not actually require this, it is a part of the spec and future drivers might not be so leniant.

The attached patch adds a GLattr SDL_GL_FRAMEBUFFER_SRGB_CAPABLE, which requests an SRGB capable framebuffer, and implements it for the x11 and windows backends using GLX_ARB_framebuffer_sRGB and WGL_ARB_framebuffer_sRGB.

I haven't tested the windows version, but the X11 version seems to work fine (though it shows no change when enabled, as no drivers actually use this hint).

— David
Comment 1 Ryan C. Gordon 2013-08-14 13:56:39 UTC 
This patch looks good. I'm going to apply it as soon as we figure out what the post-2.0.0 branching strategy is.

I'm also sad that I didn't notice this before 2.0.0 shipped, I would have totally added it.   :(

--ryan.
Comment 2 Ryan C. Gordon 2013-08-22 22:50:59 UTC 
So I'm pretty sure this wants the value passed with GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB to be 'True', but I can't confirm this from the extension docs, but it matches other values we set for glXChooseFBConfig().

--ryan.
Comment 3 Sam Lantinga 2013-08-23 07:46:17 UTC 
Maybe we should wait to apply this patch until it's needed by drivers?  Or is this something that's pretty important now?
Comment 4 David Gow 2013-08-24 04:15:49 UTC 
I'm not sure about whether or not it needs to be set to "True". Boolean attributes should, in theory, pass "True" to glXChooseFBConfig() and no parameter at all to glXChooseVisual(). The spec here is gloriously unhelpful, and doesn't say if GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB is a boolean value or not. A quick look through the Mesa source shows that they're treating it as an integer value, with nonzero values being "true".

It might also be worth pointing out that the spec really only states that we need to pass it to glXChooseVisual() but freeglut (for instance) only passes it to glXChooseFBConfig(). It's probably safest to pass the attribute to both.

I've yet to come across a system which supports sRGB framebuffers and doesn't enable them by default, but I certainly haven't tried every GL implementation ever. The Mesa guys decided to follow suit, and not offer any (colour) visuals which don't support sRGB if the hardware supports it. See https://bugs.freedesktop.org/show_bug.cgi?id=60633 for some more info.

In the end, this isn't urgent, as there are lots of existing programs out there which don't request sRGB visuals but expect to get them, so I suspect most drivers in the future will not require this step. I can't see it hurting to support this though, in the worst case, people just don't use it.

-- David
Comment 5 Ryan C. Gordon 2013-10-21 01:44:54 UTC 
I fixed up the FBConfig stuff to match what we do for other boolean values and committed this as https://hg.libsdl.org/SDL/rev/4861edda71d1

--ryan.