[Mesa-dev] [PATCH 09/10] egl/x11: Match depth 30 RGB visuals to 32-bit RGBA EGLConfigs.

[Mario Kleiner]

On 09/07/2017 04:38 PM, Emil Velikov wrote:
> On 7 September 2017 at 01:51, Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
>> On 09/06/2017 03:18 PM, Eric Engestrom wrote:
>>>
>>> On Tuesday, 2017-09-05 07:01:13 +0200, Mario Kleiner wrote:
>>>>
>>>> Similar to the matching of 24 bit RGB visuals to 32-bit
>>>> RGBA EGLConfigs.
>>>>
>>>> Fixes failure of piglit egl tests to select ARGB2101010
>>>> visuals via eglChooseConfig() with EGL_ALPHA_BITS 2 on
>>>> a depth 30 X-Screen.
>>>>
>>>> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
>>>> ---
>>>>    src/egl/drivers/dri2/platform_x11.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/egl/drivers/dri2/platform_x11.c
>>>> b/src/egl/drivers/dri2/platform_x11.c
>>>> index 062c8a4..df768ab 100644
>>>> --- a/src/egl/drivers/dri2/platform_x11.c
>>>> +++ b/src/egl/drivers/dri2/platform_x11.c
>>>> @@ -781,13 +781,14 @@ dri2_x11_add_configs_for_visuals(struct
>>>> dri2_egl_display *dri2_dpy,
>>>>                      config_count++;
>>>>                  /* Allow a 24-bit RGB visual to match a 32-bit RGBA
>>>> EGLConfig.
>>>> +             * Ditto for 30-bit RGB visuals to match a 32-bit RGBA
>>>> EGLConfig.
>>>>                 * Otherwise it will only match a 32-bit RGBA visual.  On a
>>>>                 * composited window manager on X11, this will make all of
>>>> the
>>>>                 * EGLConfigs with destination alpha get blended by the
>>>>                 * compositor.  This is probably not what the application
>>>>                 * wants... especially on drivers that only have 32-bit
>>>> RGBA
>>>>                 * EGLConfigs! */
>>>> -            if (d.data->depth == 24) {
>>>> +            if (d.data->depth == 24 || d.data->depth == 30) {
>>>>                   rgba_masks[3] =
>>>>                      ~(rgba_masks[0] | rgba_masks[1] | rgba_masks[2]);
>>>>                   dri2_conf = dri2_add_config(disp, config, config_count +
>>>> 1,
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> Haven't looked into it in details, but I feel like the two switches in
>>> swrastCreateDrawable() and dri2_create_image_khr_pixmap() would need
>>> updating as well, don't they? (probably as a separate patch though)
>>>
>>
>> Thanks for the feedback. Will check this and do so in some add-on patches.

I added some handling to the swrastCreateDrawable, which seems to work 
fine. I also think most egl platforms (surfaceless, drm/gbm, wayland 
confirmed via testing) should be fine. Android would still need to be 
extended if it already has some HAL formats with 10 bits? Not sure if i 
should do this, as atm. it only exposes 8 bpc formats to clients, which 
it handles.

For the x11/egl backend, the dri2_create_image_khr_pixmap() and 
dri3_create_image_khr_pixmap() are a bit of a problem. They derive the 
format from the drawables depth, which i think is ambiguous for depth 32 
(argb8888 or argb2101010). This is used for compositing under x11/egl, 
e.g., by KDE Plasma's optional EGL+OpenGL compositing method (GLX+OpenGL 
is the default under X11). I added a case 30: --> XRGB2101010, and 
testing, also with photometer, shows clients get properly composited 
with 30 bit depth. Depth 32 drawables - still mapping to argb8888, 
magically also work without visual artifacts, although i don't know why?

The currently used xcb requests to the server don't carry info about the 
true color format of pixmaps.

> 
> Can you please check that workloads as mentioned in commit
> 11a955aef42730ab009490f03c03c54ed07db666 still work.
> 

These work. I used apitrace quite a bit during testing, so the retracers 
work fine.

> Thanks
> Emil
>