egl: NativeSurface: replace recreate with create

src/backend/drm/egl/mod.rs

@@ -52,11 +52,10 @@ type BackendRef<D> = Weak<EglSurfaceInternal<<D as Device>::Surface>>;
 /// Representation of an egl device to create egl rendering surfaces
 pub struct EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     dev: EGLDisplay<B, D>,
     logger: ::slog::Logger,
@@ -67,11 +66,10 @@ where
 
 impl<B, D> AsRawFd for EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     fn as_raw_fd(&self) -> RawFd {
         self.dev.borrow().as_raw_fd()
@@ -80,11 +78,10 @@ where
 
 impl<B, D> EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     /// Try to create a new [`EglDevice`] from an open device.
     ///
@@ -138,22 +135,20 @@ where
 
 struct InternalDeviceHandler<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     handler: Box<dyn DeviceHandler<Device = EglDevice<B, D>> + 'static>,
 }
 
 impl<B, D> DeviceHandler for InternalDeviceHandler<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     type Device = D;
 
@@ -167,11 +162,10 @@ where
 
 impl<B, D> Device for EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     type Surface = EglSurface<<D as Device>::Surface>;
 
@@ -250,11 +244,10 @@ where
 #[cfg(feature = "use_system_lib")]
 impl<B, D> EGLGraphicsBackend for EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     fn bind_wl_display(&self, display: &Display) -> Result<EGLBufferReader, EGLError> {
         self.dev.bind_wl_display(display)
@@ -263,11 +256,10 @@ where
 
 impl<B, D> Drop for EglDevice<B, D>
 where
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<B, Arguments = Arguments, Error = <<D as Device>::Surface as Surface>::Error>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device + NativeDisplay<B, Arguments = Arguments> + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     fn drop(&mut self) {
         self.clear_handler();

src/backend/drm/egl/session.rs

@@ -28,15 +28,13 @@ pub struct EglDeviceObserver<S: SessionObserver + 'static, N: NativeSurface + Su
 impl<S, B, D> AsSessionObserver<EglDeviceObserver<S, <D as Device>::Surface>> for EglDevice<B, D>
 where
     S: SessionObserver + 'static,
-    B: Backend<Surface = <D as Device>::Surface> + 'static,
+    B: Backend<Surface = <D as Device>::Surface, Error = <<D as Device>::Surface as Surface>::Error>
-    D: Device
-        + NativeDisplay<
-            B,
-            Arguments = (crtc::Handle, Mode, Vec<connector::Handle>),
-            Error = <<D as Device>::Surface as Surface>::Error,
-        > + AsSessionObserver<S>
         + 'static,
-    <D as Device>::Surface: NativeSurface,
+    D: Device
+        + NativeDisplay<B, Arguments = (crtc::Handle, Mode, Vec<connector::Handle>)>
+        + AsSessionObserver<S>
+        + 'static,
+    <D as Device>::Surface: NativeSurface<Error = <<D as Device>::Surface as Surface>::Error>,
 {
     fn observer(&mut self) -> EglDeviceObserver<S, <D as Device>::Surface> {
         EglDeviceObserver {

src/backend/drm/gbm/egl.rs

@@ -7,7 +7,7 @@
 use crate::backend::drm::{Device, RawDevice, Surface};
 use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface};
 use crate::backend::egl::{display::EGLDisplayHandle, ffi};
-use crate::backend::egl::{wrap_egl_call, EGLError, Error as EglBackendError};
+use crate::backend::egl::{wrap_egl_call, EGLError, Error as EglBackendError, SurfaceCreationError};
 
 use super::{Error, GbmDevice, GbmSurface};
 
@@ -15,7 +15,6 @@ use drm::control::{connector, crtc, Device as ControlDevice, Mode};
 use gbm::AsRaw;
 use nix::libc::{c_int, c_void};
 use std::marker::PhantomData;
-use std::ptr;
 
 /// Egl Gbm backend type
 ///
@@ -26,6 +25,7 @@ pub struct Gbm<D: RawDevice + 'static> {
 
 impl<D: RawDevice + 'static> Backend for Gbm<D> {
     type Surface = GbmSurface<D>;
+    type Error = Error<<<D as Device>::Surface as Surface>::Error>;
 
     unsafe fn get_display<F>(
         display: ffi::NativeDisplayType,
@@ -66,7 +66,6 @@ impl<D: RawDevice + 'static> Backend for Gbm<D> {
 
 unsafe impl<D: RawDevice + ControlDevice + 'static> NativeDisplay<Gbm<D>> for GbmDevice<D> {
     type Arguments = (crtc::Handle, Mode, Vec<connector::Handle>);
-    type Error = Error<<<D as Device>::Surface as Surface>::Error>;
 
     fn is_backend(&self) -> bool {
         true
@@ -76,34 +75,40 @@ unsafe impl<D: RawDevice + ControlDevice + 'static> NativeDisplay<Gbm<D>> for Gb
         Ok(self.dev.borrow().as_raw() as *const _)
     }
 
-    fn create_surface(&mut self, args: Self::Arguments) -> Result<GbmSurface<D>, Self::Error> {
+    fn create_surface(
+        &mut self,
+        args: Self::Arguments,
+    ) -> Result<GbmSurface<D>, Error<<<D as Device>::Surface as Surface>::Error>> {
         Device::create_surface(self, args.0, args.1, &args.2)
     }
 }
 
 unsafe impl<D: RawDevice + 'static> NativeSurface for GbmSurface<D> {
+    type Error = Error<<<D as Device>::Surface as Surface>::Error>;
+
     unsafe fn create(
         &self,
         display: &EGLDisplayHandle,
         config_id: ffi::egl::types::EGLConfig,
         surface_attributes: &[c_int],
-    ) -> *const c_void {
+    ) -> Result<*const c_void, SurfaceCreationError<Self::Error>> {
+        GbmSurface::recreate(self).map_err(SurfaceCreationError::NativeSurfaceCreationFailed)?;
+
+        wrap_egl_call(|| {
             ffi::egl::CreateWindowSurface(
                 display.handle,
                 config_id,
                 self.0.surface.borrow().as_raw() as *const _,
                 surface_attributes.as_ptr(),
             )
+        })
+        .map_err(SurfaceCreationError::EGLSurfaceCreationFailed)
     }
 
     fn needs_recreation(&self) -> bool {
         self.needs_recreation()
     }
 
-    fn recreate(&self) -> Result<(), Self::Error> {
-        GbmSurface::recreate(self)
-    }
-
     fn swap_buffers(&self) -> Result<(), Self::Error> {
         // this is safe since `eglSwapBuffers` will have been called exactly once
         // if this is used by our egl module, which is why this trait is unsafe.

src/backend/egl/display.rs

@@ -379,7 +379,7 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLDisplay<B, N> {
         double_buffer: Option<bool>,
         config: ffi::egl::types::EGLConfig,
         args: N::Arguments,
-    ) -> Result<EGLSurface<B::Surface>, SurfaceCreationError<N::Error>> {
+    ) -> Result<EGLSurface<B::Surface>, SurfaceCreationError<B::Error>> {
         trace!(self.logger, "Creating EGL window surface.");
         let surface = self
             .native
@@ -399,7 +399,6 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLDisplay<B, N> {
             debug!(self.logger, "EGL surface successfully created");
             x
         })
-        .map_err(SurfaceCreationError::EGLSurfaceCreationFailed)
     }
 
     /// Returns the runtime egl version of this display

src/backend/egl/native.rs

@@ -13,7 +13,9 @@ use winit::window::Window as WinitWindow;
 /// Trait for typed backend variants (X11/Wayland/GBM)
 pub trait Backend {
     /// Surface type created by this backend
-    type Surface: NativeSurface;
+    type Surface: NativeSurface<Error = Self::Error>;
+    /// Error type thrown by the surface creation in case of failure.
+    type Error: ::std::error::Error + Send + 'static;
 
     /// Return an [`EGLDisplay`](ffi::egl::types::EGLDisplay) based on this backend
     ///
@@ -35,6 +37,7 @@ pub enum Wayland {}
 #[cfg(feature = "backend_winit")]
 impl Backend for Wayland {
     type Surface = wegl::WlEglSurface;
+    type Error = Error;
 
     unsafe fn get_display<F>(
         display: ffi::NativeDisplayType,
@@ -81,6 +84,7 @@ pub enum X11 {}
 #[cfg(feature = "backend_winit")]
 impl Backend for X11 {
     type Surface = XlibWindow;
+    type Error = Error;
 
     unsafe fn get_display<F>(
         display: ffi::NativeDisplayType,
@@ -121,8 +125,6 @@ impl Backend for X11 {
 pub unsafe trait NativeDisplay<B: Backend> {
     /// Arguments used to surface creation.
     type Arguments;
-    /// Error type thrown by the surface creation in case of failure.
-    type Error: ::std::error::Error + Send + 'static;
     /// Because one type might implement multiple [`Backend`]s this function must be called to check
     /// if the expected [`Backend`] is used at runtime.
     fn is_backend(&self) -> bool;
@@ -139,13 +141,12 @@ pub unsafe trait NativeDisplay<B: Backend> {
         ffi::egl::WINDOW_BIT as ffi::EGLint
     }
     /// Create a surface
-    fn create_surface(&mut self, args: Self::Arguments) -> Result<B::Surface, Self::Error>;
+    fn create_surface(&mut self, args: Self::Arguments) -> Result<B::Surface, B::Error>;
 }
 
 #[cfg(feature = "backend_winit")]
 unsafe impl NativeDisplay<X11> for WinitWindow {
     type Arguments = ();
-    type Error = Error;
 
     fn is_backend(&self) -> bool {
         self.xlib_display().is_some()
@@ -167,7 +168,6 @@ unsafe impl NativeDisplay<X11> for WinitWindow {
 #[cfg(feature = "backend_winit")]
 unsafe impl NativeDisplay<Wayland> for WinitWindow {
     type Arguments = ();
-    type Error = Error;
 
     fn is_backend(&self) -> bool {
         self.wayland_display().is_some()
@@ -198,10 +198,15 @@ unsafe impl NativeDisplay<Wayland> for WinitWindow {
 /// The returned [`NativeWindowType`](ffi::NativeWindowType) must be valid for EGL
 /// and there is no way to test that.
 pub unsafe trait NativeSurface {
-    /// Error of the underlying surface
+    /// Error type thrown by the surface creation in case of failure.
-    type Error: std::error::Error;
+    type Error: ::std::error::Error + Send + 'static;
-
+    /// Create an EGLSurface from the internal native type.
-    /// Create an EGLSurface from the internal native type
+    ///
+    /// Must be able to deal with re-creation of existing resources,
+    /// if `needs_recreation` can return `true`.
+    ///
+    /// # Safety
+    /// This is usually an unsafe operation returning a raw pointer.
     unsafe fn create(
         &self,
         display: &EGLDisplayHandle,
@@ -210,24 +215,15 @@ pub unsafe trait NativeSurface {
     ) -> Result<*const c_void, SurfaceCreationError<Self::Error>>;
 
     /// Will be called to check if any internal resources will need
-    /// to be recreated. Old resources must be used until `recreate`
+    /// to be recreated. Old resources must be used until `create`
-    /// was called.
+    /// was called again and a new surface was optained.
     ///
-    /// Only needs to be recreated, if this shall sometimes return true.
+    /// Only needs to be recreated, if this may return true.
     /// The default implementation always returns false.
     fn needs_recreation(&self) -> bool {
         false
     }
 
-    /// Instructs the surface to recreate internal resources
-    ///
-    /// Must only be implemented if `needs_recreation` can return `true`.
-    /// Returns true on success.
-    /// If this call was successful `ptr()` *should* return something different.
-    fn recreate(&self) -> Result<(), Self::Error> {
-        Ok(())
-    }
-
     /// Adds additional semantics when calling
     /// [EGLSurface::swap_buffers](::backend::egl::surface::EGLSurface::swap_buffers)
     ///
@@ -237,28 +233,16 @@ pub unsafe trait NativeSurface {
     }
 }
 
-/// Hack until ! gets stablized
-#[derive(Debug)]
-pub enum Never {}
-impl std::fmt::Display for Never {
-    fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        unreachable!()
-    }
-}
-impl std::error::Error for Never {}
-
 #[cfg(feature = "backend_winit")]
 unsafe impl NativeSurface for XlibWindow {
-    // this would really be a case for this:
+    type Error = Error;
-    // type Error = !; (https://github.com/rust-lang/rust/issues/35121)
-    type Error = Never;
 
     unsafe fn create(
         &self,
         display: &EGLDisplayHandle,
         config_id: ffi::egl::types::EGLConfig,
         surface_attributes: &[c_int],
-    ) -> Result<*const c_void, SurfaceCreationError<Never>> {
+    ) -> Result<*const c_void, SurfaceCreationError<Error>> {
         wrap_egl_call(|| {
             ffi::egl::CreateWindowSurface(
                 display.handle,
@@ -273,15 +257,14 @@ unsafe impl NativeSurface for XlibWindow {
 
 #[cfg(feature = "backend_winit")]
 unsafe impl NativeSurface for wegl::WlEglSurface {
-    // type Error = !;
+    type Error = Error;
-    type Error = Never;
 
     unsafe fn create(
         &self,
         display: &EGLDisplayHandle,
         config_id: ffi::egl::types::EGLConfig,
         surface_attributes: &[c_int],
-    ) -> Result<*const c_void, SurfaceCreationError<Never>> {
+    ) -> Result<*const c_void, SurfaceCreationError<Error>> {
         wrap_egl_call(|| {
             ffi::egl::CreateWindowSurface(
                 display.handle,

src/backend/egl/surface.rs

@@ -1,6 +1,6 @@
 //! EGL surface related structs
 
-use super::{ffi, native, wrap_egl_call, EGLError, SwapBuffersError};
+use super::{ffi, native, wrap_egl_call, EGLError, SurfaceCreationError, SwapBuffersError};
 use crate::backend::egl::display::EGLDisplayHandle;
 use crate::backend::graphics::PixelFormat;
 use nix::libc::c_int;
@@ -42,7 +42,7 @@ impl<N: native::NativeSurface> EGLSurface<N> {
         config: ffi::egl::types::EGLConfig,
         native: N,
         log: L,
-    ) -> Result<EGLSurface<N>, EGLError>
+    ) -> Result<EGLSurface<N>, SurfaceCreationError<N::Error>>
     where
         L: Into<Option<::slog::Logger>>,
     {
@@ -72,7 +72,9 @@ impl<N: native::NativeSurface> EGLSurface<N> {
         let surface = unsafe { native.create(&*display, config, &surface_attributes)? };
 
         if surface == ffi::egl::NO_SURFACE {
-            return Err(EGLError::BadSurface);
+            return Err(SurfaceCreationError::EGLSurfaceCreationFailed(
+                EGLError::BadSurface,
+            ));
         }
 
         Ok(EGLSurface {
@@ -106,16 +108,20 @@ impl<N: native::NativeSurface> EGLSurface<N> {
         };
 
         if self.native.needs_recreation() || surface.is_null() || is_bad_surface {
-            self.native.recreate().map_err(SwapBuffersError::Underlying)?;
             if !surface.is_null() {
                 let _ = unsafe { ffi::egl::DestroySurface(**self.display, surface as *const _) };
             }
-            //TODO remove recreate
-            self.native.recreate();
             self.surface.set(unsafe {
                 self.native
                     .create(&*self.display, self.config_id, &self.surface_attributes)
-                    .map_err(SwapBuffersError::EGLCreateWindowSurface)?
+                    .map_err(|err| match err {
+                        SurfaceCreationError::EGLSurfaceCreationFailed(err) => {
+                            SwapBuffersError::EGLCreateWindowSurface(err)
+                        }
+                        SurfaceCreationError::NativeSurfaceCreationFailed(err) => {
+                            SwapBuffersError::Underlying(err)
+                        }
+                    })?
             });
 
             result.map_err(|err| {