From ecf0428c244788f2ded076967195282d9bb9bfa7 Mon Sep 17 00:00:00 2001 From: Robin Ebert Date: Mon, 9 Oct 2023 16:52:49 +0200 Subject: [PATCH] egl: use EGL_KHR_display_reference This resolves a past issue where glutin would not be able to terminate a display due to risk of two displays being created from the same native display. Besides not terminating EGL raises issue with nvidia on Wayland, thus the `api::egl::Display::terminate` is added to help clients call this function when calling `eglTerminate` is generally unsafe to do automatically when dropping the `Display`. Links: https://github.com/alacritty/alacritty/issues/7146 Fixes: #1588 --- CHANGELOG.md | 2 + glutin/src/api/egl/display.rs | 77 +++++++++++++++++++++++++++++------ glutin_egl_sys/build.rs | 1 + 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 182f7e7e22..4e33eb778d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ - **Breaking:** `bitflags` which is used as a part of public API was updated to `2.0`. - **Breaking:** `.*SurfaceAccessor` traits got removed; their methods now on respective `.*GlContext` traits instead. - **Breaking:** `GlContext` trait is now a part of the `prelude`. +- Automatically cleanup the `EGLDisplay` when `EGL_KHR_display_reference` is present. +- Add `api::egl::Display::terminate` to terminate the display when glutin doesn't manage it. # Version 0.30.10 diff --git a/glutin/src/api/egl/display.rs b/glutin/src/api/egl/display.rs index a752bb077b..540ddaf9bd 100644 --- a/glutin/src/api/egl/display.rs +++ b/glutin/src/api/egl/display.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use std::ffi::{self, CStr}; +use std::mem::MaybeUninit; use std::ops::Deref; use std::os::raw::c_char; use std::sync::Arc; @@ -124,7 +125,12 @@ impl Display { .into()); } - let mut attrs = Vec::::with_capacity(2); + let mut attrs = Vec::::with_capacity(3); + + if extensions.contains("EGL_KHR_display_reference") { + attrs.push(egl::TRACK_REFERENCES_KHR as _); + attrs.push(egl::TRUE as _); + } // TODO: Some extensions exist like EGL_EXT_device_drm which allow specifying // which DRM master fd to use under the hood by the implementation. This would @@ -185,6 +191,25 @@ impl Display { Device::from_ptr(self.inner.egl, device) } + /// Terminate the EGL display. + /// + /// When the display is managed by glutin with the + /// `EGL_KHR_display_reference` this function does nothing and + /// `eglTerminate` will be automatically invoked during display destruction. + /// + /// # Safety + /// + /// This function will destroy the global EGL state, even the one created + /// and managed by other libraries. Use this function only when you're + /// bringing everything down. + pub unsafe fn terminate(self) { + if !CLIENT_EXTENSIONS.get().unwrap().contains("EGL_KHR_display_reference") { + unsafe { + self.inner.egl.Terminate(*self.inner.raw); + } + } + } + fn get_platform_display(egl: &Egl, display: RawDisplayHandle) -> Result { if !egl.GetPlatformDisplay.is_loaded() { return Err(ErrorKind::NotSupported("eglGetPlatformDisplay is not supported").into()); @@ -192,7 +217,7 @@ impl Display { let extensions = CLIENT_EXTENSIONS.get().unwrap(); - let mut attrs = Vec::::new(); + let mut attrs = Vec::::with_capacity(5); let (platform, mut display) = match display { #[cfg(wayland_platform)] RawDisplayHandle::Wayland(handle) @@ -219,6 +244,11 @@ impl Display { }, }; + if extensions.contains("EGL_KHR_display_reference") { + attrs.push(egl::TRACK_REFERENCES_KHR as _); + attrs.push(egl::TRUE as _); + } + // Be explicit here. if display.is_null() { display = egl::DEFAULT_DISPLAY as *mut _; @@ -240,7 +270,7 @@ impl Display { let extensions = CLIENT_EXTENSIONS.get().unwrap(); - let mut attrs = Vec::::new(); + let mut attrs = Vec::::with_capacity(5); let mut legacy = false; let (platform, mut display) = match display { #[cfg(wayland_platform)] @@ -279,6 +309,11 @@ impl Display { }, }; + if extensions.contains("EGL_KHR_display_reference") { + attrs.push(egl::TRACK_REFERENCES_KHR as _); + attrs.push(egl::TRUE as _); + } + // Be explicit here. if display.is_null() { display = egl::DEFAULT_DISPLAY as *mut _; @@ -503,6 +538,32 @@ impl fmt::Debug for DisplayInner { impl Drop for DisplayInner { fn drop(&mut self) { + // If the EGL_TRACK_REFERENCES_KHR attribute is true, then EGL will internally + // reference count the display. If that is the case, glutin can + // terminate the display without worry for the instance being + // reused elsewhere. + if CLIENT_EXTENSIONS.get().unwrap().contains("EGL_KHR_display_reference") { + let mut track_references = MaybeUninit::::uninit(); + unsafe { + if match self.raw { + EglDisplay::Khr(khr) => self.egl.QueryDisplayAttribKHR( + khr, + egl::TRACK_REFERENCES_KHR as _, + track_references.as_mut_ptr(), + ), + EglDisplay::Ext(ext) => self.egl.QueryDisplayAttribEXT( + ext, + egl::TRACK_REFERENCES_KHR as _, + track_references.as_mut_ptr(), + ), + EglDisplay::Legacy(_) => return, + } == egl::TRUE + { + self.egl.Terminate(*self.raw); + } + } + } + // We cannot call safely call `eglTerminate`. // // This may sound confusing, but this is a result of how EGL works: @@ -545,16 +606,6 @@ impl Drop for DisplayInner { // of not dropping the display is negligible because the display will // probably be destroyed on app termination and we can let the // operating system deal with tearing down EGL instead. - // - // # Possible future work: - // - // For platform displays, we could track the use of individual raw - // window handles and display attributes (recall the "with the - // same parameters" line) and use that to determine if it is safe to - // terminate the display, but that increases maintenance burden and is - // possibly flaky to implement. - - // unsafe { self.egl.Terminate(self.raw) }; } } diff --git a/glutin_egl_sys/build.rs b/glutin_egl_sys/build.rs index ccef77db20..337458bd61 100644 --- a/glutin_egl_sys/build.rs +++ b/glutin_egl_sys/build.rs @@ -37,6 +37,7 @@ fn main() { "EGL_EXT_swap_buffers_with_damage", "EGL_KHR_create_context", "EGL_KHR_create_context_no_error", + "EGL_KHR_display_reference", "EGL_KHR_fence_sync", "EGL_KHR_platform_android", "EGL_KHR_platform_gbm",