[PATCH] glx: do not pick sRGB config for 32-bit RGBA visual
Thomas Hellstrom
thellstrom at vmware.com
Fri Dec 15 14:40:14 UTC 2017
On 12/15/2017 02:50 PM, Tapani Pälli wrote:
> 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?
The ordering happens in the driver.
>
>> 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?
No it hasn't. When Kwin searches for the glxConfig matching the ARGB
visual, it omits GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT from the criterion,
and Mesa's GLX layer will then only report non-SRGB capable visuals
instead of all.
>
>> 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.
>
Yes it does. And the GLX ARB specification states that sRGB support
starts turned off so that it shouldn't affect existing applications that
get an sRGB fbconfig by mistake.
>> 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.
It would also break if there are no 32-bit fbconfigs left to choose from
except sRGB ones. This was how all this started. Before the swapMethod
extension got fixed, then an fbconfig with an awkward swapmethod
would've been chosen instead of sRGB. And all looked well.
>
>> 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.
Not on xserver master. There are many 32bit GLX visuals, some of them
still sRGBCapable.
>
>> 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.
So you actually get sRGB fbconfigs using glxChooseFBConfig() without
explicitly asking for them?
>
>> 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.
Yes, as mentioned, on xserver master there shouldn't be an issue
anymore. (at least not with non-Intel drivers). Even before your patch.
Please retest with xserver master without your patch. The problem is
fixed in the previously mentioned commit.
For xserver 1.19 and earlier, I'd try the attached patch. It should make
kwin actually find the correct fbconfig even if it's sRGB. According to
my previous debugging that was the root cause of the problem.
(mesa patch attached).
/Thomas
>
> 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=
>>>
>>
>>
-------------- next part --------------
diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c
index c707d0c..11b9153 100644
--- a/src/glx/glxcmds.c
+++ b/src/glx/glxcmds.c
@@ -944,6 +944,7 @@ init_fbconfig_for_chooser(struct glx_config * config,
config->fbconfigID = (GLXFBConfigID) (GLX_DONT_CARE);
config->swapMethod = GLX_DONT_CARE;
+ config->sRGBCapable = GLX_DONT_CARE;
}
#define MATCH_DONT_CARE( param ) \
@@ -1161,6 +1162,12 @@ fbconfig_compare(struct glx_config **a, struct glx_config **b)
PREFER_LARGER(maxPbufferHeight);
PREFER_LARGER(maxPbufferPixels);
+ if (((*a)->sRGBCapable != (*b)->sRGBCapable)) {
+ /* Prefer non-sRGBCapable
+ */
+ return ((*a)->sRGBCapable) ? 1 : -1;
+ }
+
return 0;
}
More information about the xorg-devel
mailing list