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: alacritty/alacritty#7146
Fixes: #1588

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
 

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::<EGLint>::with_capacity(2);
+        let mut attrs = Vec::<EGLint>::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,14 +191,33 @@ 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 !self.inner.uses_display_reference() {
+            unsafe {
+                self.inner.egl.Terminate(*self.inner.raw);
+            }
+        }
+    }
+
     fn get_platform_display(egl: &Egl, display: RawDisplayHandle) -> Result<EglDisplay> {
         if !egl.GetPlatformDisplay.is_loaded() {
             return Err(ErrorKind::NotSupported("eglGetPlatformDisplay is not supported").into());
         }
 
         let extensions = CLIENT_EXTENSIONS.get().unwrap();
 
-        let mut attrs = Vec::<EGLAttrib>::new();
+        let mut attrs = Vec::<EGLAttrib>::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::<EGLint>::new();
+        let mut attrs = Vec::<EGLint>::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 _;
@@ -490,6 +525,35 @@ pub(crate) struct DisplayInner {
     pub(crate) _native_display: Option<NativeDisplay>,
 }
 
+impl DisplayInner {
+    fn uses_display_reference(&self) -> bool {
+        if !CLIENT_EXTENSIONS.get().unwrap().contains("EGL_KHR_display_reference") {
+            return false;
+        }
+
+        // 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.
+        let mut track_references = MaybeUninit::<EGLAttrib>::uninit();
+        unsafe {
+            (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(_) => egl::FALSE,
+            } == egl::TRUE)
+        }
+    }
+}
+
 impl fmt::Debug for DisplayInner {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Display")
@@ -503,6 +567,12 @@ impl fmt::Debug for DisplayInner {
 
 impl Drop for DisplayInner {
     fn drop(&mut self) {
+        if self.uses_display_reference() {
+            unsafe {
+                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 +615,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) };
     }
 }
 

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",