[PATCH] glx: do not pick sRGB config for 32-bit RGBA visual

Fri Dec 15 13:50:30 UTC 2017

Hi;

On 15.12.2017 10:10, Thomas Hellstrom wrote:
> On 12/11/2017 06:53 AM, Tapani Pälli wrote:
>> Hi;
>>
>> Any comments? Without this change there will be issues with certain 
>> Linux desktops when distributions start to use Mesa 17.4.
>>
> 
> Tapani,
> Did you actually try this with latest xserver master without this patch 
> applied, or with 1.19 only?

I did not try with master branch. TBH my workflow with plasma desktop is 
a bit terrible as I'm testing things with Kubuntu so I have to apply 
Ubuntu's patches and what not to make this whole thing work. Easiest way 
simply was to modify/use sources that apt-get source gave me. Also I 
could not make plasma to work in Fedora which I normally use.

> With it you should have a number of 32 bit fbconfig both with and 
> without sRGB and typically the non-sRGB fbconfigs would be first in the 
> list so the built-in 32-bit visual would pick the non-sRGB visuals 
> anyway. So with xserver master your patch wouldn't have any effect at 
> all. At most it would reorder the 32-bit visuals?

OK. So far there has been always just one 32bit GLX visual exposed. Is 
the ordering happening only by luck or are non-sRGB visuals explicitly 
ordered before sRGB ones?

> There was a previous problem when we only had a single 32-bit visual and 
> it got assigned an sRGB visual, because the mesa GLX layer doesn't allow 
> sRGB fbconfigs in glxChooseFBConfig. I think there's where the real 

I'm not sure if I understand this issue correctly but I've done glx test 
app (attached in one of the bugs) that uses glXChooseFBConfig and it is 
able to choose config which has GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT set, 
those are in the list. Maybe this has been already fixed?

> problem is (or one of them), since at least kwin uses glxChooseFBConfig 
> to look up an fbconfig list to match the fbconfig of the 32-bit visual. 
> When that problem was fixed in xserver commit f84e59a4 it seemed all 
> drivers except Intel's worked fine again.
> 
> FWIW, Nvidia's proprietary driver only exposes sRGB fbconfigs so 
> apparently the applications work fine with them.

Does Plasma desktop work with those drivers? For example Nouveau does 
not do that and it exposes only 1 32bit GLX visual.

> Now with non-master X servers (1.19 and earlier) your patch, while 
> fixing things on some drivers, might actually break others, because 
> there are no other usable 32-bit fbconfigs left. It depends on the 
> number of fbconfigs the driver is exposing.

It would only break something if someone wants 32bit visual that has 
also GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT set and does not accept non-sRGB 
one. AFAIK none of the current compositors do this.

> So to summarize:
> 
> 1) Your patch shouldn't really have any effect on Xorg master other than 
> reordering the 32-bit visuals and if so only if sRGB fbconfigs are 
> listed first by the driver.

It has the effect that we never pick FBconfig that has sRGBCapable set 
for 32bit GLX visual.

> 2) The fix for non-master X servers  would probably be to allow sRGB 
> fbconfigs in mesa's glxChooseFBConfigs so that apps can actually find 
> the fbconfig associated with the single 32-bit visual.

AFAIK sRGB configs are already listed in glxChooseFBConfigs so this does 
not help. That was actually the main issue, the only 32bit visual 
exposed had sRGB.

> 3) If there are any remaining problems, they are probably driver related.

I agree that there may be driver issues too and I'm happy to see 
discussion on this problem. I spent quite a bit of time trying to 
understand it and finally thought the safest way would be to go back how 
it has been in the past, 32bit visual with non-srgb.

I would be interested if you have some suggestions or alternatives on 
how to approach/debug this issue.

Thanks;

> /Thomas
> 
> 
> 
> 
> 
>> On 11/28/2017 09:23 AM, Tapani Pälli wrote:
>>> This fixes blending issues seen with kwin and gnome-shell when
>>> 32bit visual has sRGB capability set.
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> Bugzilla: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D103699&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=ui-fsRhvz5hb1wtFp3g-rGtMhklZIsnn4P3UZ3D6z0s&s=IS0-fxjcgga5I5ISJ_b9nJbZiRewgoSkKXIM40JUUDQ&e= 
>>>
>>> Bugzilla: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D103646&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=ui-fsRhvz5hb1wtFp3g-rGtMhklZIsnn4P3UZ3D6z0s&s=Y_58iAZqhtAjhJ1sdV4G3nogUnJf1eI7dYmOScyKmh8&e= 
>>>
>>> Bugzilla: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D103655&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=ui-fsRhvz5hb1wtFp3g-rGtMhklZIsnn4P3UZ3D6z0s&s=_qzmVp96ZC9XYBJXLUVI4_0prbenD-Wr7zgp8EpiSeo&e= 
>>>
>>> ---
>>>   glx/glxscreens.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/glx/glxscreens.c b/glx/glxscreens.c
>>> index 73444152a..596d972e0 100644
>>> --- a/glx/glxscreens.c
>>> +++ b/glx/glxscreens.c
>>> @@ -271,6 +271,11 @@ pickFBConfig(__GLXscreen * pGlxScreen, VisualPtr 
>>> visual)
>>>           /* If it's the 32-bit RGBA visual, demand a 32-bit 
>>> fbconfig. */
>>>           if (visual->nplanes == 32 && config->rgbBits != 32)
>>>               continue;
>>> +        /* If it's the 32-bit RGBA visual, do not pick sRGB capable 
>>> config.
>>> +         * This can cause issues with compositors that are not sRGB 
>>> aware.
>>> +         */
>>> +        if (visual->nplanes == 32 && config->sRGBCapable == GL_TRUE)
>>> +            continue;
>>>           /* Can't use the same FBconfig for multiple X visuals.  I 
>>> think. */
>>>           if (config->visualID != 0)
>>>               continue;
>>>
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.x.org_archives_xorg-2Ddevel&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=ui-fsRhvz5hb1wtFp3g-rGtMhklZIsnn4P3UZ3D6z0s&s=T9cfugB_nXlqjC4UHDYOmuBXo7pn-y9cpC0piRNiGMA&e= 
>>
>> Info: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.x.org_mailman_listinfo_xorg-2Ddevel&d=DwIGaQ&c=uilaK90D4TOVoH58JNXRgQ&r=wnSlgOCqfpNS4d02vP68_E9q2BNMCwfD2OZ_6dCFVQQ&m=ui-fsRhvz5hb1wtFp3g-rGtMhklZIsnn4P3UZ3D6z0s&s=0TlRCHlNDRf3nI5I3zcZS07a9gRTk6vXVtuolKjD1wg&e= 
>>
> 
> 