Use eglGetPlatformDisplay when it's available. #4826
Conversation
Added a platform parameter to egl_init_context. If the caller provides a platform other than EGL_NONE, then it will try to use eglGetPlatformDisplay or eglGetPlatformDisplayEXT instead of eglGetDisplay. If neither eglGetPlatformDisplay or eglGetPlatformDisplayEXT is supported, then it will still fall back to calling eglGetDisplay. Updated the Wayland, X11, and DRM callers to use the correct platform enum. Those are the callers that don't just pass EGL_DEFAULT_DISPLAY as the native display handle. Calling eglGetDisplay with any value other than EGL_DEFAULT_DISPLAY is inherently unreliable, because it requires the EGL implementation to guess a platform type based on a (void *) pointer. Some implementations might not identify a particular platform, or worse, might guess wrong. Fixes libretro#4790
|
Thanks for this. I was not aware of this, I have confirmed that DRM KMS mode works again on an iGPU. |
|
Android and emscripten builds are failing |
|
There are some platforms which broke down though because of this PR. This is for Android - This is for Emscripten - |
|
I am afraid that for Android, going this way would make it inherently less backwards compatible. https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_platform_android.txt So mayhaps we need to skip some portions of your PR for platforms like Android and Emscripten? I will leave this research up to you. In any case, the downtime of Android/Emscripten is an immediate pressing concern. |
|
I'm doing this workaround for now until you can come up with something better - |
|
Ah, sorry, I should have thought of that. The problem there is that the EGL header files don't have the EGL 1.5 or EGL_EXT_platform_base declarations. But, the EGL header files have macros to indicate what versions and extensions are available, so it should be enough to just add an I'll get a pull request up in a moment with that change. |
|
OK, that should probably be scalable enough. I will test this tomorrow. |
| @@ -563,7 +563,8 @@ static bool gfx_ctx_drm_egl_set_video_mode(gfx_ctx_drm_data_t *drm) | |||
| case GFX_CTX_OPENGL_ES_API: | |||
| case GFX_CTX_OPENVG_API: | |||
| #ifdef HAVE_EGL | |||
| if (!egl_init_context(&drm->egl, (EGLNativeDisplayType)g_gbm_dev, &major, | |||
| if (!egl_init_context(&drm->egl, EGL_PLATFORM_GBM_KHR, | |||
There was a problem hiding this comment.
Just a side note: linux systems with old EGL headers will fail on this (ubuntu 14.04 for example), there's no EGL_PLATFORM_GBM_KHR in < EGL/eglext.h >
libegl1-mesa 10.1.3
gfx/drivers_context/drm_ctx.c: In function ‘gfx_ctx_drm_egl_set_video_mode’:
gfx/drivers_context/drm_ctx.c:566:43: error: ‘EGL_PLATFORM_GBM_KHR’ undeclared (first use in this function)
if (!egl_init_context(&drm->egl, EGL_PLATFORM_GBM_KHR,
^
Needs to build with --disable-wayland --disable-egl
There was a problem hiding this comment.
@kbrenneman Do you have a solution for the issue pointed out by @sergiobenrocha2 as well?
There was a problem hiding this comment.
The same sort of #ifdef should work for that as well -- the EGL header file has a macro for every EGL extension that it defines.
I can put together a pull request when I get home this afternoon.
This changes the EGL code so that it will try to use eglGetPlatformDisplay or eglGetPlatformDisplayEXT instead of eglGetDisplay.
Basically, eglGetDisplay does not can cannot work reliably with any value except EGL_DEFAULT_DISPLAY. Different EGL implementations will have different heuristics to try to guess what kind of platform it's dealing with, but that guessing won't always work.
This is the root cause of #4790, where the EGL library doesn't know how to detect a GBM display. But, depending on the EGL implementation, similar or worse problems could show up with X11 or Wayland, too.
This patch will first check the EGL client version string, and if it's got at least EGL 1.5, then it'll use eglGetPlatformDisplay. Otherwise, it'll check the EGL extension string and if EGL_EXT_platform_base is supported, then it'll use eglGetPlatformDisplayEXT. If both of those fail, then it'll fall back to using eglGetDisplay.