[PATCH weston 1/4] gl-renderer: use eglGetPlatformDisplayEXT to get an EGLDisplay

[Daniel Stone]

Hi,

On 25 February 2015 at 12:42, Jonny Lamb <jonny.lamb at collabora.co.uk> wrote:
> +/** Checks whether a client extension or fallbacks are supported
> + *
> + * \param ec The weston compositor
> + * \param extension_suffix The EGL client extension suffix
> + * \return Negative if not supported, otherwise positive
> + *
> + * This function checks whether a specific platform_* extension is supported
> + * by the EGL extension, and if not, whether fallbacks are supported.
> +
> + * The extension suffix should be the suffix of the platform extension (that
> + * defines a <platform> argument as defined in EGL_EXT_platform_base). For
> + * example, passing "foo" will check whether either "EGL_KHR_platform_foo",
> + * "EGL_EXT_platform_foo", or "EGL_MESA_platform_foo" is supported.
> +
> + * The return value is negative:
> + *   - if EGL client extensions aren't supported.

This is pretty unfortunate with everyone checking for return codes <
0; in that case, they should just fall back to eglGetDisplay, rather
than refusing to use EGL at all.

> + * The return value is positive:
> + *   - if the EGL version is >= 1.5,
> + *   - if the supplied EGL client extension is supported,
> + *   - in the fallback case using eglGetDisplay.
> [...]
> +       if (natively_supported > 0) {
> +               weston_log("EGL version is >= 1.5 so assuming platform_base support.\n");
> +               return 1;
> +       }

If we call gl_renderer->supports("gbm"), and it has EGL >= 1.5, we'll
return 1, regardless of whether or not the actual platform is
supported. All new-EGL means is that we can elide the check for
platform_base, not for the actual platform.

> +       snprintf(s, sizeof s, "EGL_KHR_platform_%s", extension_suffix);
> +       if (strstr(extensions, s))
> +               return 1;
> +
> +       snprintf(s, sizeof s, "EGL_EXT_platform_%s", extension_suffix);
> +       if (strstr(extensions, s))
> +               return 1;
> +
> +       snprintf(s, sizeof s, "EGL_MESA_platform_%s", extension_suffix);
> +       if (strstr(extensions, s))
> +               return 1;
> +
> +       /* in the end fall back to eglGetDisplay and friends */
> +
> +       return 1;

But then again, so does this.

I think the correct logic is:
  - test for either EGL 1.5 or EGL_EXT_platform_base; if neither are
present, just continue on to using eglGetDisplay but don't treat it as
fatal (i.e. what we do today, to support legacy stacks)
  - assuming we now have platform_base guaranteed to be present, test
for the specific platform; if it's present, return 1, else return -1

Which I guess puts us to a trinary: -1 meaning 'definitely
unsupported' (can enumerate through platform_base but platform isn't
present in the enumeration), 0 meaning 'maybe supported' (no
platform_base so down to eglGetDisplay and guesswork), and 1 meaning
'definitely supported' (specific platform is present in the
enumeration, and can use GetPlatformDisplay).

Sorry this is a bit late on.

Cheers,
Daniel