[Mesa-dev] [PATCH RFC] egl: Add a 565 pbuffer-only EGL config under X11.

[Adam Jackson]

On Tue, 2018-10-30 at 16:38 -0700, Eric Anholt wrote:
> Eric Anholt <eric at anholt.net> writes:
> 
> > The CTS requires a 565-no-depth-no-stencil config for ES 3.0, but at depth
> > 24 of X11 we wouldn't do so.  We can satisfy that bad requirement using a
> > pbuffer-only visual with whatever other buffers the driver happens to have
> > given us.
> 
> Anyone?  Still concerned about getting Mesa to pass the current
> conformance suite.

I'm not thrilled with the CTS requiring R5G6B5 tbh. Do we know if
there's some rationale for that? Maybe something along the lines of,
"GLES target devices are likely to only support low depths, so even a
workstation GLES needs to implement low depths because otherwise how do
you know it works". Not that I'd endorse such reasoning.

To the patch itself:

> +   /* Add a 565 pbuffer-only config.  If X11 is depth 24, we wouldn't have 565
> +    * avialable, which the CTS demands.
           ^^
Typo.

> +    */
> +   for (int j = 0; dri2_dpy->driver_configs[j]; j++) {
> +      const __DRIconfig *config = dri2_dpy->driver_configs[j];
> +      const EGLint config_attrs[] = {
> +         EGL_NATIVE_VISUAL_ID,    0,
> +         EGL_NATIVE_VISUAL_TYPE,  EGL_NONE,
> +         EGL_NONE
> +      };

The commit message says you need no-depth and no-stencil, but you don't
enforce that here. Are we sure we don't need to?

> +      EGLint surface_type = EGL_PBUFFER_BIT;
> +      unsigned int rgba_masks[4] = {
> +         0x1f << 11,
> +         0x3f << 5,
> +         0x1f << 0,
> +         0,
> +      };
> +      if (dri2_add_config(disp, config, config_count + 1, surface_type,
> +                          config_attrs, rgba_masks)) {
> +         config_count++;
> +         break;
> +      }
> +   }

This seems like it introduces a subtle dependency on how driver_configs
is ordered. I'm not sure you'd want your lone 565 pbuffer config to be
sRGB or multisampled. In fact in the latter case you'd end up with an
EGLConfig with zero bits set for EGL_SURFACE_TYPE, which is a bug in
dri2_add_config() I suppose.

That's probably not how driver_configs is sorted in reality, and
passing CTS is a valid thing to want, so with the above addressed:

Reviewed-by: Adam Jackson <ajax at redhat.com>

But if CTS can be fixed, let's do that too.

- ajax