Closed janKilika closed 6 days ago
Also need a changelog entry for this.
Done!
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?
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()
?
https://registry.khronos.org/EGL/sdk/docs/man/html/eglQueryString.xhtml
It's unfortunate that we cannot use
eglQueryString
withNO_DISPLAY
to "supposedly" (?) read the EGL version before callingeglInitialize()
, until version 1.5? It's unclear to me if that version requirement applies toEGL_EXTENSIONS
because that's the only query listed to supportNO_DISPLAY
- but it would mean our call is invalid on your givenlibglvnd
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.
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"?
Could also link to upstream issue I've just opened.
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.
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) 😅
Issue addressed: EGL's
Display::new
making aKhr
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 bylibglvnd
"aliasing"eglGetPlatformDisplay
andeglGetPlatformDisplayEXT
(as in both functions call the same vendor function internally), which makes the former succeed and create a display that is only valid as anExt
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 fromKhr
toExt
if it did.Changes: The only actual change is that it is checked in
Display::initialize_display
with theDisplay::sanitize_libglvnd_aliasing
function whether (a)EglDisplay
is of variantKhr
and (b) version is equal to 1.4, and if both conditions are true,EglDisplay
is downgraded fromKhr
toExt
. This seems to be safe, aseglGetPlatformDisplay
andeglGetPlatformDisplayEXT
in practice are made to be aliases of each other bylibglvnd
, 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.