Make display, surface and context creation safe

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+- **Breaking:** Bump `raw-window-handle` to v0.6.
+- **Breaking:** Make `glutin` types generic over the window/display they are wrapping.
+
 # Version 0.31.2
 
 - Fixed EGL not setting context version with EGL versions before 1.5 and missing context ext.

glutin-winit/Cargo.toml

@@ -19,8 +19,8 @@ wayland = ["glutin/wayland", "winit/wayland"]
 
 [dependencies]
 glutin = { version = "0.31.0", path = "../glutin", default-features = false }
-raw-window-handle = "0.5.2"
-winit = { version = "0.29.2", default-features = false, features = ["rwh_05"] }
+raw-window-handle = { version = "0.6.0", features = ["std"] }
+winit = { version = "0.29.2", default-features = false, features = ["rwh_06"] }
 
 [build-dependencies]
 cfg_aliases = "0.1.1"

glutin-winit/src/lib.rs

@@ -21,9 +21,9 @@ use glutin::platform::x11::X11GlConfigExt;
 use glutin::prelude::*;
 
 #[cfg(wgl_backend)]
-use raw_window_handle::HasRawWindowHandle;
+use raw_window_handle::HasWindowHandle;
+use raw_window_handle::{DisplayHandle, HasDisplayHandle, WindowHandle};
 
-use raw_window_handle::{HasRawDisplayHandle, RawWindowHandle};
 use winit::error::OsError;
 use winit::event_loop::EventLoopWindowTarget;
 use winit::window::{Window, WindowBuilder};
@@ -86,14 +86,20 @@ impl DisplayBuilder {
     /// **WGL:** - [`WindowBuilder`] **must** be passed in
     /// [`Self::with_window_builder`] if modern OpenGL(ES) is desired,
     /// otherwise only builtin functions like `glClear` will be available.
+    ///
+    /// # Safety
+    ///
+    /// The `Config` must not outlive the `EventLoop`.
     pub fn build<T, Picker>(
         mut self,
         window_target: &EventLoopWindowTarget<T>,
-        template_builder: ConfigTemplateBuilder,
+        template_builder: ConfigTemplateBuilder<WindowHandle<'_>>,
         config_picker: Picker,
-    ) -> Result<(Option<Window>, Config), Box<dyn Error>>
+    ) -> Result<(Option<Window>, Config<DisplayHandle<'static>>), Box<dyn Error>>
     where
-        Picker: FnOnce(Box<dyn Iterator<Item = Config> + '_>) -> Config,
+        Picker: FnOnce(
+            Box<dyn Iterator<Item = Config<DisplayHandle<'static>>> + '_>,
+        ) -> Config<DisplayHandle<'static>>,
     {
         // XXX with WGL backend window should be created first.
         #[cfg(wgl_backend)]
@@ -104,11 +110,13 @@ impl DisplayBuilder {
         };
 
         #[cfg(wgl_backend)]
-        let raw_window_handle = window.as_ref().map(|window| window.raw_window_handle());
+        let raw_window_handle = window.as_ref().map(|w| w.window_handle()).transpose()?;
         #[cfg(not(wgl_backend))]
         let raw_window_handle = None;
 
-        let gl_display = create_display(window_target, self.preference, raw_window_handle)?;
+        // SAFETY: Will not outlive the event loop.
+        let gl_display =
+            unsafe { create_display(window_target, self.preference, raw_window_handle)? };
 
         // XXX the native window must be passed to config picker when WGL is used
         // otherwise very limited OpenGL features will be supported.
@@ -121,7 +129,7 @@ impl DisplayBuilder {
 
         let template = template_builder.build();
 
-        let gl_config = unsafe {
+        let gl_config = {
             let configs = gl_display.find_configs(template)?;
             config_picker(configs)
         };
@@ -137,11 +145,16 @@ impl DisplayBuilder {
     }
 }
 
-fn create_display<T>(
+/// Create the actual display.
+///
+/// # Safety
+///
+/// The `Display` must not outlive the `EventLoop`.
+unsafe fn create_display<T>(
     window_target: &EventLoopWindowTarget<T>,
     _api_preference: ApiPreference,
-    _raw_window_handle: Option<RawWindowHandle>,
-) -> Result<Display, Box<dyn Error>> {
+    _raw_window_handle: Option<WindowHandle<'_>>,
+) -> Result<Display<DisplayHandle<'static>>, Box<dyn Error>> {
     #[cfg(egl_backend)]
     let _preference = DisplayApiPreference::Egl;
 
@@ -170,7 +183,10 @@ fn create_display<T>(
         ApiPreference::FallbackEgl => DisplayApiPreference::WglThenEgl(_raw_window_handle),
     };
 
-    unsafe { Ok(Display::new(window_target.raw_display_handle(), _preference)?) }
+    // SAFETY: Does not outlive the event loop.
+    let display_handle =
+        unsafe { DisplayHandle::borrow_raw(window_target.display_handle()?.as_raw()) };
+    Ok(Display::new(display_handle, _preference)?)
 }
 
 /// Finalize [`Window`] creation by applying the options from the [`Config`], be
@@ -179,10 +195,10 @@ fn create_display<T>(
 ///
 /// [`Window`]: winit::window::Window
 /// [`Config`]: glutin::config::Config
-pub fn finalize_window<T>(
+pub fn finalize_window<D: HasDisplayHandle, T>(
     window_target: &EventLoopWindowTarget<T>,
     mut builder: WindowBuilder,
-    gl_config: &Config,
+    gl_config: &Config<D>,
 ) -> Result<Window, OsError> {
     // Disable transparency if the end config doesn't support it.
     if gl_config.supports_transparency() == Some(false) {

glutin-winit/src/window.rs

@@ -1,15 +1,14 @@
-use std::num::NonZeroU32;
-
 use glutin::context::PossiblyCurrentContext;
 use glutin::surface::{
     GlSurface, ResizeableSurface, Surface, SurfaceAttributes, SurfaceAttributesBuilder,
     SurfaceTypeTrait, WindowSurface,
 };
-use raw_window_handle::HasRawWindowHandle;
+use raw_window_handle::{HasDisplayHandle, HasWindowHandle};
+use std::num::NonZeroU32;
 use winit::window::Window;
 
 /// [`Window`] extensions for working with [`glutin`] surfaces.
-pub trait GlWindow {
+pub trait GlWindow: HasWindowHandle + Sized {
     /// Build the surface attributes suitable to create a window surface.
     ///
     /// # Panics
@@ -23,9 +22,9 @@ pub trait GlWindow {
     /// let attrs = winit_window.build_surface_attributes(<_>::default());
     /// ```
     fn build_surface_attributes(
-        &self,
-        builder: SurfaceAttributesBuilder<WindowSurface>,
-    ) -> SurfaceAttributes<WindowSurface>;
+        self,
+        builder: SurfaceAttributesBuilder<WindowSurface<Self>>,
+    ) -> SurfaceAttributes<WindowSurface<Self>>;
 
     /// Resize the surface to the window inner size.
     ///
@@ -36,37 +35,46 @@ pub trait GlWindow {
     /// use glutin_winit::GlWindow;
     /// # use glutin::surface::{Surface, WindowSurface};
     /// # let winit_window: winit::window::Window = unimplemented!();
-    /// # let (gl_surface, gl_context): (Surface<WindowSurface>, _) = unimplemented!();
+    /// # let (gl_surface, gl_context): (Surface<glutin::NoDisplay, WindowSurface<glutin::NoWindow>>, _) = unimplemented!();
     ///
     /// winit_window.resize_surface(&gl_surface, &gl_context);
     /// ```
-    fn resize_surface(
+    fn resize_surface<D: HasDisplayHandle>(
         &self,
-        surface: &Surface<impl SurfaceTypeTrait + ResizeableSurface>,
-        context: &PossiblyCurrentContext,
+        surface: &Surface<D, impl SurfaceTypeTrait + ResizeableSurface>,
+        context: &PossiblyCurrentContext<D>,
     );
 }
 
-impl GlWindow for Window {
-    fn build_surface_attributes(
-        &self,
-        builder: SurfaceAttributesBuilder<WindowSurface>,
-    ) -> SurfaceAttributes<WindowSurface> {
-        let (w, h) = self.inner_size().non_zero().expect("invalid zero inner size");
-        builder.build(self.raw_window_handle(), w, h)
-    }
+macro_rules! implement_glwindow {
+    (<$($lt:lifetime)?> $ty:ty) => {
+        impl<$($lt)?> GlWindow for $ty {
+            fn build_surface_attributes(
+                self,
+                builder: SurfaceAttributesBuilder<WindowSurface<Self>>,
+            ) -> SurfaceAttributes<WindowSurface<Self>> {
+                let (w, h) = self.inner_size().non_zero().expect("invalid zero inner size");
+                builder.build(self, w, h)
+            }
 
-    fn resize_surface(
-        &self,
-        surface: &Surface<impl SurfaceTypeTrait + ResizeableSurface>,
-        context: &PossiblyCurrentContext,
-    ) {
-        if let Some((w, h)) = self.inner_size().non_zero() {
-            surface.resize(context, w, h)
+            fn resize_surface<D: HasDisplayHandle>(
+                &self,
+                surface: &Surface<D, impl SurfaceTypeTrait + ResizeableSurface>,
+                context: &PossiblyCurrentContext<D>,
+            ) {
+                if let Some((w, h)) = self.inner_size().non_zero() {
+                    surface.resize(context, w, h)
+                }
+            }
         }
-    }
+    };
 }
 
+implement_glwindow!(<> Window);
+implement_glwindow!(<'a> &'a Window);
+implement_glwindow!(<> std::rc::Rc<Window>);
+implement_glwindow!(<> std::sync::Arc<Window>);
+
 /// [`winit::dpi::PhysicalSize<u32>`] non-zero extensions.
 trait NonZeroU32PhysicalSize {
     /// Converts to non-zero `(width, height)`.

glutin/Cargo.toml

@@ -23,7 +23,7 @@ wayland = ["wayland-sys", "egl"]
 bitflags = "2.2.1"
 libloading = { version = "0.8.0", optional = true }
 once_cell = "1.13"
-raw-window-handle = "0.5.2"
+raw-window-handle = "0.6.0"
 
 [target.'cfg(windows)'.dependencies]
 glutin_egl_sys = { version = "0.6.0", path = "../glutin_egl_sys", optional = true }

glutin/src/api/cgl/config.rs

@@ -13,6 +13,7 @@ use icrate::AppKit::{
     NSOpenGLProfileVersion4_1Core, NSOpenGLProfileVersionLegacy,
 };
 use objc2::rc::Id;
+use raw_window_handle::{HasDisplayHandle, HasWindowHandle};
 
 use crate::config::{
     Api, AsRawConfig, ColorBufferType, ConfigSurfaceTypes, ConfigTemplate, GlConfig, RawConfig,
@@ -24,12 +25,12 @@ use crate::private::Sealed;
 use super::appkit::NSOpenGLPixelFormat;
 use super::display::Display;
 
-impl Display {
+impl<D: HasDisplayHandle> Display<D> {
     #[allow(deprecated)]
-    pub(crate) unsafe fn find_configs(
+    pub(crate) fn find_configs<W: HasWindowHandle>(
         &self,
-        template: ConfigTemplate,
-    ) -> Result<Box<dyn Iterator<Item = Config> + '_>> {
+        template: ConfigTemplate<W>,
+    ) -> Result<Box<dyn Iterator<Item = Config<D>> + '_>> {
         let mut attrs = Vec::<NSOpenGLPixelFormatAttribute>::with_capacity(32);
 
         // We use minimum to follow behavior of other platforms here.
@@ -129,12 +130,26 @@ impl Display {
 }
 
 /// A wrapper around NSOpenGLPixelFormat.
-#[derive(Debug, Clone, PartialEq, Eq)]
-pub struct Config {
-    pub(crate) inner: Arc<ConfigInner>,
+#[derive(Debug)]
+pub struct Config<D> {
+    pub(crate) inner: Arc<ConfigInner<D>>,
 }
 
-impl Config {
+impl<D> Clone for Config<D> {
+    fn clone(&self) -> Self {
+        Self { inner: self.inner.clone() }
+    }
+}
+
+impl<D> PartialEq for Config<D> {
+    fn eq(&self, other: &Self) -> bool {
+        self.inner == other.inner
+    }
+}
+
+impl<D> Eq for Config<D> {}
+
+impl<D: HasDisplayHandle> Config<D> {
     fn raw_attribute(&self, attrib: NSOpenGLPixelFormatAttribute) -> i32 {
         unsafe {
             let mut value = 0;
@@ -156,7 +171,7 @@ impl Config {
 }
 
 #[allow(deprecated)]
-impl GlConfig for Config {
+impl<D: HasDisplayHandle> GlConfig for Config<D> {
     fn color_buffer_type(&self) -> Option<ColorBufferType> {
         // On macos all color formats divide by 3 without reminder, except for the RGB
         // 565. So we can convert it in a hopefully reliable way. Also we should remove
@@ -209,37 +224,37 @@ impl GlConfig for Config {
     }
 }
 
-impl GetGlDisplay for Config {
-    type Target = Display;
+impl<D: HasDisplayHandle> GetGlDisplay for Config<D> {
+    type Target = Display<D>;
 
     fn display(&self) -> Self::Target {
         self.inner.display.clone()
     }
 }
 
-impl AsRawConfig for Config {
+impl<D: HasDisplayHandle> AsRawConfig for Config<D> {
     fn raw_config(&self) -> RawConfig {
         RawConfig::Cgl(Id::as_ptr(&self.inner.raw).cast())
     }
 }
 
-impl Sealed for Config {}
+impl<D: HasDisplayHandle> Sealed for Config<D> {}
 
-pub(crate) struct ConfigInner {
-    display: Display,
+pub(crate) struct ConfigInner<D> {
+    display: Display<D>,
     pub(crate) transparency: bool,
     pub(crate) raw: Id<NSOpenGLPixelFormat>,
 }
 
-impl PartialEq for ConfigInner {
+impl<D> PartialEq for ConfigInner<D> {
     fn eq(&self, other: &Self) -> bool {
         self.raw == other.raw
     }
 }
 
-impl Eq for ConfigInner {}
+impl<D> Eq for ConfigInner<D> {}
 
-impl fmt::Debug for ConfigInner {
+impl<D> fmt::Debug for ConfigInner<D> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Config").field("id", &self.raw).finish()
     }