egl/display: ensure provider after version check

[janKilika]

Issue addressed: EGL's Display::new making a Khr display (those are only valid for 1.5) when EGL version for the platform is actually 1.4, which causes errors when 1.5 surface creation is attempted afterwards. Caused by libglvnd "aliasing" eglGetPlatformDisplay and eglGetPlatformDisplayEXT (as in both functions call the same vendor function internally), which makes the former succeed and create a display that is only valid as an Ext display despite what otherwise would be expected according to specification.

Solution: Check for the version after performing an eglInitialize call, as it returns the actual version for the platform's implementation of EGL and therefore enables us to detect whether the aliasing issue occurred and downgrade the display from Khr to Ext if it did.

Changes: The only actual change is that it is checked in Display::initialize_display with the Display::sanitize_libglvnd_aliasing function whether (a) EglDisplay is of variant Khr and (b) version is equal to 1.4, and if both conditions are true, EglDisplay is downgraded from Khr to Ext. This seems to be safe, as eglGetPlatformDisplay and eglGetPlatformDisplayEXT in practice are made to be aliases of each other by libglvnd, and if any of the attributes passed are invalid for the underlying function, it is presumed that it would return with an error.

See issue #1689 for further details.
Also see issue #251 on libglvnd's repository.

[MarijnS95]

We can probably fit a more descriptive title than "aliasing issue" without going over any character limit.

[kchibisov]

Also need a changelog entry for this.

[janKilika]

Also need a changelog entry for this.

Done!

[MarijnS95]

https://registry.khronos.org/EGL/sdk/docs/man/html/eglQueryString.xhtml

It's unfortunate that we cannot use eglQueryString with NO_DISPLAY to "supposedly" (?) read the EGL version before calling eglInitialize(), until version 1.5? It's unclear to me if that version requirement applies to EGL_EXTENSIONS because that's the only query listed to support NO_DISPLAY - but it would mean our call is invalid on your given libglvnd too?

[MarijnS95]

Reading somewhat further, we don't ever check if https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_client_extensions.txt works and following that, https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_platform_base.txt is available before calling eglGetPlatformDisplayEXT()?

[janKilika]

https://registry.khronos.org/EGL/sdk/docs/man/html/eglQueryString.xhtml

It's unfortunate that we cannot use eglQueryString with NO_DISPLAY to "supposedly" (?) read the EGL version before calling eglInitialize(), until version 1.5? It's unclear to me if that version requirement applies to EGL_EXTENSIONS because that's the only query listed to support NO_DISPLAY - but it would mean our call is invalid on your given libglvnd too?

If by call you mean querying the client extensions, then, as far I can see, libglvnd provides querying for client extensions through eglQueryString (the function exists in libglvnd and specifically checks for NO_DISPLAY) in accord with both the 1.5 specification and EGL_EXT_client_extensions, so the call is technically valid.

[MarijnS95]

Yes I mean us calling eglQueryString for NO_DISPLAY. According to https://registry.khronos.org/EGL/sdk/docs/man/html/eglQueryString.xhtml that's only supported from 1.5 onwards but EGL_EXT_client_extensions (which requires 1.4) specifies that it's possible as long you handle the NULL+BAD_DISPLAY case gracefully.

I was wondering if we could use something like that to detect and prevent the failure case from happening (by never calling eglGetPlatformDisplay()) but it seems like we have a chicken-egg problem here anyway by not knowing if we have a 1.5 display before calling functions that "rely on" knowing the version and/or extensions 😅

---

Also, reading back the PR description, can you reword it to provide a summary of both the problem and proposed solution, rather than linking through to an 18-comment-dense issue thread "for further details"?

[kchibisov]

Could also link to upstream issue I've just opened.

[janKilika]

Also, reading back the PR description, can you reword it to provide a summary of both the problem and proposed solution, rather than linking through to an 18-comment-dense issue thread "for further details"?

Sure, relevant information provided.

[MarijnS95]

Apologies in advance for being so messy @janKilika - it looks like we're in a situation where we're deciding whether and how the current workaround is sensible or could be written in a different way (while also reviewing the existing implementation) while I'm also suggesting light changes on the current workaround and comments (which could be superseded by the former) 😅

[kchibisov]

Review based on upstream interactions.

[kchibisov]

I hope that this is the last one.