[Mesa-dev] [PATCH] egl_dri2: Allow both 24 and 32 bit X visuals for RGBA configs

[Pekka Paalanen]

On Thu, 06 Nov 2014 13:01:03 -0800
Ian Romanick <idr at freedesktop.org> wrote:

> I thought Eric and Chad already NAKed it in bugzilla.  The problem is
> that applications ask for an RGBA visual for GL blending.  They use the
> alpha channel to generate their images, but the final alpha values are,
> basically, random... and the composited result would be pure garbage.

Reading
https://bugs.freedesktop.org/show_bug.cgi?id=67676#c5
"We should certainly be exposing EGLConfigs that match up to the rgba
visual, though, so you can find it when you try." - Eric

To me that sounds like Eric would accept having the visuals there
in additional configs (as long as they are sorted after the otherwise
equivalent xRGB configs?). Eric, would you like to confirm your current
opinion?

> As Chad points out in comment #1, EGL just doesn't let applications do
> the thing the patch is trying to do.

True in general, not exactly for X11. Surely the native visual is
exposed in EGL configs for some reason? (Ok, yeah, not a good argument,
EGL considered.)

For the record, if a Wayland compositor uses the opaque region hint in
Wayland core protocol, we do not see this problem on Wayland at all. In
Wayland, so far the implementations always assume that if the alpha
channel exists in the buffer, it will be used for window blending,
except on regions marked as opaque. So we kind of already have a way to
make it work in Wayland, and just X11 is broken here.

Still, I won't even try to deny that an extension to
EGL_TRANSPARENT_TYPE would be hugely useful and the superior solution
over all the hacking. I do understand you would rather see that
developed than a native visual based solution.

More below...

> On 11/06/2014 05:12 AM, Emil Velikov wrote:
> > Humble ping x2
> > 
> > On 14/10/14 15:25, Emil Velikov wrote:
> >> Humble ping.
> >>
> >> On 23/09/14 01:25, Emil Velikov wrote:
> >>> From: Sjoerd Simons <sjoerd.simons at collabora.co.uk>
> >>>
> >>> When using RGBA EGLConfigs allow both RGB and RGBA X visuals, such that
> >>> application can decide whether they want to use RGBA (and have the
> >>> compositor blend their windows).
> >>>
> >>> On my system with this change EGLConfigs with a 24 bit visual comes up
> >>> first, as such applications blindly picking the first EGLConfig will
> >>> still get an RGB X visual.
> >>>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67676
> >>> ---
> >>>
> >>> Hello gents,
> >>>
> >>> This patch has been stuck in bugzilla since February this year. Bringing 
> >>> it around here to gather greater exposure and perhaps some 
> >>> comments/reviews.
> >>>
> >>> -Emil
> >>>
> >>>  src/egl/drivers/dri2/egl_dri2.c     |  5 +++++
> >>>  src/egl/drivers/dri2/platform_x11.c | 17 +++++++++--------
> >>>  2 files changed, 14 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> >>> index 20a7243..2ed90a7 100644
> >>> --- a/src/egl/drivers/dri2/egl_dri2.c
> >>> +++ b/src/egl/drivers/dri2/egl_dri2.c
> >>> @@ -110,6 +110,11 @@ EGLint dri2_to_egl_attribute_map[] = {
> >>>  static EGLBoolean
> >>>  dri2_match_config(const _EGLConfig *conf, const _EGLConfig *criteria)
> >>>  {
> >>> +
> >>> +   if (criteria->NativeVisualID != EGL_DONT_CARE &&
> >>> +        conf->NativeVisualID != criteria->NativeVisualID)
> >>> +      return EGL_FALSE;

Does this directly affect the behaviour of eglChooseConfig?

EGL 1.4 spec quite clearly says that NATIVE_VISUAL_ID is ignored if
specified as matching criterion to eglChooseConfig, which is why
apps are expected to match it manually rather than via eglChooseConfig.

Doesn't that mean that apps that want alpha blending of the window
already check the native visual? Maybe not if it worked by accident...

Now, this magical alpha-blending visual thing used to work the past
somehow for a long time, didn't it? Until someone noticed a performance
problem (fd.o #59783), not even a correctness problem, or are there
other bugs about correctness too? The patch that caused bug #59783 to
be reported was an intel-driver specific commit, and then #59783 got
fixed by a hardware agnostic change elsewhere, causing some existing
apps to produce unwanted results rather than just affect performance,
leading to fd.o #67676.

To not break most of existing apps that need alpha for rendering but
do not intend the alpha channel to go to the window system blending, we
only need to make sure the configs with argb visual get sorted after
the equivalent xrgb visual config? Reading the EGL 1.4 spec, that seems
perfectly valid.

I don't know if this patch guarantees the config ordering though.


Thanks,
pq

> >>> +
> >>>     if (_eglCompareConfigs(conf, criteria, NULL, EGL_FALSE) != 0)
> >>>        return EGL_FALSE;
> >>>  
> >>> diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
> >>> index a7a7338..3395fb7 100644
> >>> --- a/src/egl/drivers/dri2/platform_x11.c
> >>> +++ b/src/egl/drivers/dri2/platform_x11.c
> >>> @@ -672,14 +672,15 @@ dri2_x11_add_configs_for_visuals(struct dri2_egl_display *dri2_dpy,
> >>>  	    dri2_add_config(disp, dri2_dpy->driver_configs[j], id++,
> >>>  			    surface_type, config_attrs, rgba_masks);
> >>>  
> >>> -            /* Allow a 24-bit RGB visual 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) {
> >>> +            /* Allow both 24-bit RGB visual and 32 bit RGBA 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!
> >>> +             * Allowing both allows applications to make the decision whether
> >>> +             * 32 bit visuals are intended */
> >>> +            if (d.data->depth == 24 || d.data->depth == 32) {
> >>>                 rgba_masks[3] =
> >>>                    ~(rgba_masks[0] | rgba_masks[1] | rgba_masks[2]);
> >>>                 dri2_add_config(disp, dri2_dpy->driver_configs[j], id++,
> >>>