egl: use EGL_KHR_display_reference

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/context.rs

@@ -55,7 +55,7 @@ impl Display {
         };
 
         let is_one_five = self.inner.version >= Version::new(1, 5);
-        if is_one_five || self.inner.client_extensions.contains("EGL_KHR_create_context") {
+        if is_one_five || self.inner.display_extensions.contains("EGL_KHR_create_context") {
             let mut flags = 0;
 
             // Add profile for the OpenGL Api.

glutin/src/api/egl/device.rs

@@ -9,7 +9,7 @@ use glutin_egl_sys::egl::types::EGLDeviceEXT;
 
 use crate::error::{ErrorKind, Result};
 
-use super::display::{extensions_from_ptr, get_extensions, NO_DISPLAY_EXTENSIONS};
+use super::display::{extensions_from_ptr, get_extensions, CLIENT_EXTENSIONS};
 use super::{Egl, EGL};
 
 /// Wrapper for `EGLDevice`.
@@ -34,7 +34,7 @@ impl Device {
         };
 
         let no_display_extensions =
-            NO_DISPLAY_EXTENSIONS.get_or_init(|| get_extensions(egl, egl::NO_DISPLAY));
+            CLIENT_EXTENSIONS.get_or_init(|| get_extensions(egl, egl::NO_DISPLAY));
 
         // Querying devices requires EGL_EXT_device_enumeration and
         // EGL_EXT_device_query.

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;
@@ -30,7 +31,7 @@ use super::surface::Surface;
 use super::{Egl, EGL};
 
 /// Extensions that don't require any display.
-pub(crate) static NO_DISPLAY_EXTENSIONS: OnceCell<HashSet<&'static str>> = OnceCell::new();
+pub(crate) static CLIENT_EXTENSIONS: OnceCell<HashSet<&'static str>> = OnceCell::new();
 
 /// A wrapper for the `EGLDisplay` and its supported extensions.
 #[derive(Debug, Clone)]
@@ -54,7 +55,7 @@ impl Display {
             None => return Err(ErrorKind::NotFound.into()),
         };
 
-        NO_DISPLAY_EXTENSIONS.get_or_init(|| get_extensions(egl, egl::NO_DISPLAY));
+        CLIENT_EXTENSIONS.get_or_init(|| get_extensions(egl, egl::NO_DISPLAY));
 
         // Create a EGL display by chaining all display creation functions aborting on
         // `EGL_BAD_ATTRIBUTE`.
@@ -113,7 +114,7 @@ impl Display {
         // Okay to unwrap here because the client extensions must have been enumerated
         // while querying the available devices or the device was gotten from an
         // existing display.
-        let extensions = NO_DISPLAY_EXTENSIONS.get().unwrap();
+        let extensions = CLIENT_EXTENSIONS.get().unwrap();
 
         if !extensions.contains("EGL_EXT_platform_base")
             && !extensions.contains("EGL_EXT_platform_device")
@@ -124,33 +125,59 @@ impl Display {
             .into());
         }
 
-        let mut attrs = Vec::<EGLint>::with_capacity(2);
+        let mut attrs = Vec::<EGLint>::with_capacity(3);
 
         // 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
         // mean there would need to be an unsafe equivalent to this function.
 
+        // Push at the end so we can pop it on failure
+        let mut has_display_reference = extensions.contains("EGL_KHR_display_reference");
+        if has_display_reference {
+            attrs.push(egl::TRACK_REFERENCES_KHR as _);
+            attrs.push(egl::TRUE as _);
+        }
+
         // Push `egl::NONE` to terminate the list.
         attrs.push(egl::NONE as EGLint);
 
-        let display = Self::check_display_error(unsafe {
-            egl.GetPlatformDisplayEXT(
-                egl::PLATFORM_DEVICE_EXT,
-                device.raw_device() as *mut _,
-                attrs.as_ptr(),
-            )
-        })
+        // NOTE: This fallback is needed because libglvnd advertises client extensions
+        // if at least one vendor library supports them. This leads to creation
+        // failures for the vendor libraries not supporting
+        // EGL_KHR_display_reference. Also according to the spec creation is allowed
+        // to fail with EGL_KHR_display_reference set to EGL_TRUE even if
+        // EGL_KHR_display_reference is advertised in the client extension
+        // string, so just always try creation without EGL_KHR_display_reference
+        // if it failed using it.
+        let platform_display = loop {
+            match Self::check_display_error(unsafe {
+                egl.GetPlatformDisplayEXT(
+                    egl::PLATFORM_DEVICE_EXT,
+                    device.raw_device() as *mut _,
+                    attrs.as_ptr(),
+                )
+            }) {
+                Err(_) if has_display_reference => {
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.push(egl::NONE as EGLint);
+                    has_display_reference = false;
+                },
+                platform_display => break platform_display,
+            }
+        }
         .map(EglDisplay::Ext)?;
 
-        Self::initialize_display(egl, display, None)
+        Self::initialize_display(egl, platform_display, None)
     }
 
     /// Get the [`Device`] the display is using.
     ///
     /// This function returns [`Err`] if the `EGL_EXT_device_query` or
     /// `EGL_EXT_device_base` extensions are not available.
     pub fn device(&self) -> Result<Device> {
-        let no_display_extensions = NO_DISPLAY_EXTENSIONS.get().unwrap();
+        let no_display_extensions = CLIENT_EXTENSIONS.get().unwrap();
 
         // Querying the device of a display only requires EGL_EXT_device_query, but we
         // also check if EGL_EXT_device_base is available since
@@ -185,14 +212,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 = NO_DISPLAY_EXTENSIONS.get().unwrap();
+        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)
@@ -224,23 +270,50 @@ impl Display {
             display = egl::DEFAULT_DISPLAY as *mut _;
         }
 
+        // Push at the end so we can pop it on failure
+        let mut has_display_reference = extensions.contains("EGL_KHR_display_reference");
+        if has_display_reference {
+            attrs.push(egl::TRACK_REFERENCES_KHR as _);
+            attrs.push(egl::TRUE as _);
+        }
+
         // Push `egl::NONE` to terminate the list.
         attrs.push(egl::NONE as EGLAttrib);
 
-        let display =
-            unsafe { egl.GetPlatformDisplay(platform, display as *mut _, attrs.as_ptr()) };
+        // NOTE: This fallback is needed because libglvnd advertises client extensions
+        // if at least one vendor library supports them. This leads to creation
+        // failures for the vendor libraries not supporting
+        // EGL_KHR_display_reference. Also according to the spec creation is allowed
+        // to fail with EGL_KHR_display_reference set to EGL_TRUE even if
+        // EGL_KHR_display_reference is advertised in the client extension
+        // string, so just always try creation without EGL_KHR_display_reference
+        // if it failed using it.
+        let platform_display = loop {
+            match Self::check_display_error(unsafe {
+                egl.GetPlatformDisplay(platform, display as *mut _, attrs.as_ptr())
+            }) {
+                Err(_) if has_display_reference => {
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.push(egl::NONE as EGLAttrib);
+                    has_display_reference = false;
+                },
+                platform_display => break platform_display,
+            }
+        };
 
-        Self::check_display_error(display).map(EglDisplay::Khr)
+        platform_display.map(EglDisplay::Khr)
     }
 
     fn get_platform_display_ext(egl: &Egl, display: RawDisplayHandle) -> Result<EglDisplay> {
         if !egl.GetPlatformDisplayEXT.is_loaded() {
             return Err(ErrorKind::NotSupported("eglGetPlatformDisplayEXT is not supported").into());
         }
 
-        let extensions = NO_DISPLAY_EXTENSIONS.get().unwrap();
+        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)]
@@ -284,13 +357,40 @@ impl Display {
             display = egl::DEFAULT_DISPLAY as *mut _;
         }
 
+        // Push at the end so we can pop it on failure
+        let mut has_display_reference = extensions.contains("EGL_KHR_display_reference");
+        if has_display_reference {
+            attrs.push(egl::TRACK_REFERENCES_KHR as _);
+            attrs.push(egl::TRUE as _);
+        }
+
         // Push `egl::NONE` to terminate the list.
         attrs.push(egl::NONE as EGLint);
 
-        let display =
-            unsafe { egl.GetPlatformDisplayEXT(platform, display as *mut _, attrs.as_ptr()) };
+        // NOTE: This fallback is needed because libglvnd advertises client extensions
+        // if at least one vendor library supports them. This leads to creation
+        // failures for the vendor libraries not supporting
+        // EGL_KHR_display_reference. Also according to the spec creation is allowed
+        // to fail with EGL_KHR_display_reference set to EGL_TRUE even if
+        // EGL_KHR_display_reference is advertised in the client extension
+        // string, so just always try creation without EGL_KHR_display_reference
+        // if it failed using it.
+        let platform_display = loop {
+            match Self::check_display_error(unsafe {
+                egl.GetPlatformDisplayEXT(platform, display as *mut _, attrs.as_ptr())
+            }) {
+                Err(_) if has_display_reference => {
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.pop();
+                    attrs.push(egl::NONE as EGLint);
+                    has_display_reference = false;
+                },
+                platform_display => break platform_display,
+            }
+        };
 
-        Self::check_display_error(display).map(|display| {
+        platform_display.map(|display| {
             if legacy {
                 // NOTE: For angle we use the Legacy code path, as that uses CreateWindowSurface
                 // instead of CreatePlatformWindowSurface*. The latter somehow
@@ -382,15 +482,15 @@ impl Display {
         };
 
         // Load extensions.
-        let client_extensions = get_extensions(egl, *display);
-        let features = Self::extract_display_features(&client_extensions, version);
+        let display_extensions = get_extensions(egl, *display);
+        let features = Self::extract_display_features(&display_extensions, version);
 
         let inner = Arc::new(DisplayInner {
             egl,
             raw: display,
             _native_display: raw_display_handle.map(NativeDisplay),
             version,
-            client_extensions,
+            display_extensions,
             features,
         });
         Ok(Self { inner })
@@ -458,7 +558,7 @@ impl GlDisplay for Display {
 
 impl GetDisplayExtensions for Display {
     fn extensions(&self) -> &HashSet<&'static str> {
-        &self.inner.client_extensions
+        &self.inner.display_extensions
     }
 }
 
@@ -480,8 +580,8 @@ pub(crate) struct DisplayInner {
     /// The version of the egl library.
     pub(crate) version: Version,
 
-    /// Client EGL extensions.
-    pub(crate) client_extensions: HashSet<&'static str>,
+    /// Display EGL extensions.
+    pub(crate) display_extensions: HashSet<&'static str>,
 
     /// The features supported by the display.
     pub(crate) features: DisplayFeatures,
@@ -490,19 +590,56 @@ 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();
+        (match self.raw {
+            EglDisplay::Khr(khr) => unsafe {
+                self.egl.QueryDisplayAttribKHR(
+                    khr,
+                    egl::TRACK_REFERENCES_KHR as _,
+                    track_references.as_mut_ptr(),
+                )
+            },
+            EglDisplay::Ext(ext) => unsafe {
+                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")
             .field("raw", &self.raw)
             .field("version", &self.version)
             .field("features", &self.features)
-            .field("extensions", &self.client_extensions)
+            .field("extensions", &self.display_extensions)
             .finish()
     }
 }
 
 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 +682,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) };
     }
 }