From 8abcc145d7d7d61a7474c396d99779aa5a276f72 Mon Sep 17 00:00:00 2001
From: Victor Brekenfeld <github@drakulix.de>
Date: Fri, 23 Nov 2018 15:11:27 +0100
Subject: [PATCH] egl: EGLContext borrow native to circumvent RefCell runtime
 error

---
 build.rs                       |  2 +-
 src/backend/drm/egl/mod.rs     | 10 ++++----
 src/backend/drm/egl/session.rs |  2 +-
 src/backend/drm/egl/surface.rs |  9 ++++---
 src/backend/drm/gbm/surface.rs |  7 +++---
 src/backend/egl/context.rs     | 44 +++++++++++++++++++---------------
 src/backend/egl/surface.rs     |  3 +--
 src/backend/graphics/format.rs |  2 +-
 src/backend/graphics/gl.rs     |  2 +-
 src/backend/winit.rs           | 20 +++++++++-------
 10 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/build.rs b/build.rs
index e843e05..28ac1cf 100644
--- a/build.rs
+++ b/build.rs
@@ -51,4 +51,4 @@ fn main() {
 }
 
 #[cfg(not(any(feature = "backend_egl", feature = "renderer_gl")))]
-fn main() {}
\ No newline at end of file
+fn main() {}
diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs
index 802cf30..6f4509b 100644
--- a/src/backend/drm/egl/mod.rs
+++ b/src/backend/drm/egl/mod.rs
@@ -1,6 +1,5 @@
 use drm::control::{connector, crtc, Mode, ResourceHandles, ResourceInfo};
 use nix::libc::dev_t;
-use std::cell::RefCell;
 use std::iter::FromIterator;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::rc::Rc;
@@ -28,7 +27,7 @@ pub struct EglDevice<
 > where
     <D as Device>::Surface: NativeSurface,
 {
-    dev: Rc<RefCell<EGLContext<B, D>>>,
+    dev: Rc<EGLContext<B, D>>,
     logger: ::slog::Logger,
 }
 
@@ -82,9 +81,9 @@ where
         debug!(log, "Creating egl context from device");
         Ok(EglDevice {
             // Open the gbm device from the drm device and create a context based on that
-            dev: Rc::new(RefCell::new(
+            dev: Rc::new(
                 EGLContext::new(dev, attributes, Default::default(), log.clone()).map_err(Error::from)?,
-            )),
+            ),
             logger: log,
         })
     }
@@ -148,7 +147,6 @@ where
 
         let surface = self
             .dev
-            .borrow_mut()
             .create_surface((crtc, mode, Vec::from_iter(connectors)).into())?;
 
         Ok(EglSurface {
@@ -182,6 +180,6 @@ where
     <D as Device>::Surface: NativeSurface,
 {
     fn bind_wl_display(&self, display: &Display) -> EGLResult<EGLDisplay> {
-        self.dev.borrow().bind_wl_display(display)
+        self.dev.bind_wl_display(display)
     }
 }
diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs
index 338865d..fc18bca 100644
--- a/src/backend/drm/egl/session.rs
+++ b/src/backend/drm/egl/session.rs
@@ -20,7 +20,7 @@ where
 {
     fn observer(&mut self) -> EglDeviceObserver<S> {
         EglDeviceObserver {
-            observer: (**self.dev.borrow_mut()).observer(),
+            observer: self.dev.borrow_mut().observer(),
         }
     }
 }
diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs
index 682338c..5573abb 100644
--- a/src/backend/drm/egl/surface.rs
+++ b/src/backend/drm/egl/surface.rs
@@ -1,6 +1,5 @@
 use drm::control::{connector, crtc, Mode};
 use nix::libc::c_void;
-use std::cell::RefCell;
 use std::rc::Rc;
 
 use super::error::*;
@@ -19,7 +18,7 @@ pub struct EglSurface<
 > where
     <D as Device>::Surface: NativeSurface,
 {
-    pub(super) dev: Rc<RefCell<EGLContext<B, D>>>,
+    pub(super) dev: Rc<EGLContext<B, D>>,
     pub(super) surface: EGLSurface<B::Surface>,
 }
 
@@ -105,7 +104,7 @@ where
     }
 
     unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void {
-        self.dev.borrow().get_proc_address(symbol)
+        self.dev.get_proc_address(symbol)
     }
 
     fn get_framebuffer_dimensions(&self) -> (u32, u32) {
@@ -114,7 +113,7 @@ where
     }
 
     fn is_current(&self) -> bool {
-        self.dev.borrow().is_current() && self.surface.is_current()
+        self.dev.is_current() && self.surface.is_current()
     }
 
     unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> {
@@ -122,6 +121,6 @@ where
     }
 
     fn get_pixel_format(&self) -> PixelFormat {
-        self.dev.borrow().get_pixel_format()
+        self.dev.get_pixel_format()
     }
 }
diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs
index 2eb29b0..70167e7 100644
--- a/src/backend/drm/gbm/surface.rs
+++ b/src/backend/drm/gbm/surface.rs
@@ -73,16 +73,17 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
         };
         self.next_buffer.set(Some(next_bo));
 
-        trace!(self.logger, "Queueing Page flip");
         if self.recreated.get() {
+            debug!(self.logger, "Commiting new state");
             self.crtc
                 .commit(fb.handle())
                 .map_err(|_| SwapBuffersError::ContextLost)?;
             self.recreated.set(false);
-        } else {
-            self.crtc.page_flip(fb.handle())?;
         }
 
+        trace!(self.logger, "Queueing Page flip");
+        self.crtc.page_flip(fb.handle())?;
+
         self.current_frame_buffer.set(Some(fb));
 
         Ok(())
diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs
index 28f8c70..f0767db 100644
--- a/src/backend/egl/context.rs
+++ b/src/backend/egl/context.rs
@@ -5,17 +5,16 @@ use backend::graphics::PixelFormat;
 use nix::libc::{c_int, c_void};
 use slog;
 use std::{
+    cell::{Ref, RefCell, RefMut},
     ffi::{CStr, CString},
     marker::PhantomData,
-    mem,
-    ops::{Deref, DerefMut},
-    ptr,
+    mem, ptr,
     rc::Rc,
 };
 
 /// EGL context for rendering
 pub struct EGLContext<B: native::Backend, N: native::NativeDisplay<B>> {
-    native: N,
+    native: RefCell<N>,
     pub(crate) context: Rc<ffi::egl::types::EGLContext>,
     pub(crate) display: Rc<ffi::egl::types::EGLDisplay>,
     pub(crate) config_id: ffi::egl::types::EGLConfig,
@@ -26,19 +25,6 @@ pub struct EGLContext<B: native::Backend, N: native::NativeDisplay<B>> {
     _backend: PhantomData<B>,
 }
 
-impl<B: native::Backend, N: native::NativeDisplay<B>> Deref for EGLContext<B, N> {
-    type Target = N;
-    fn deref(&self) -> &N {
-        &self.native
-    }
-}
-
-impl<B: native::Backend, N: native::NativeDisplay<B>> DerefMut for EGLContext<B, N> {
-    fn deref_mut(&mut self) -> &mut N {
-        &mut self.native
-    }
-}
-
 impl<B: native::Backend, N: native::NativeDisplay<B>> EGLContext<B, N> {
     /// Create a new `EGLContext` from a given `NativeDisplay`
     pub fn new<L>(
@@ -56,7 +42,7 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLContext<B, N> {
             unsafe { EGLContext::<B, N>::new_internal(ptr, attributes, reqs, log.clone()) }?;
 
         Ok(EGLContext {
-            native,
+            native: RefCell::new(native),
             context,
             display,
             config_id,
@@ -429,10 +415,11 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLContext<B, N> {
     }
 
     /// Creates a surface for rendering
-    pub fn create_surface(&mut self, args: N::Arguments) -> Result<EGLSurface<B::Surface>> {
+    pub fn create_surface(&self, args: N::Arguments) -> Result<EGLSurface<B::Surface>> {
         trace!(self.logger, "Creating EGL window surface.");
         let surface = self
             .native
+            .borrow_mut()
             .create_surface(args)
             .chain_err(|| ErrorKind::SurfaceCreationFailed)?;
         EGLSurface::new(self, surface).map(|x| {
@@ -459,6 +446,25 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLContext<B, N> {
     pub fn get_pixel_format(&self) -> PixelFormat {
         self.pixel_format
     }
+
+    /// Borrow the underlying native display.
+    ///
+    /// This follows the same semantics as `std::cell:RefCell`.
+    /// Multiple read-only borrows are possible. Borrowing the
+    /// backend while there is a mutable reference will panic.
+    pub fn borrow(&self) -> Ref<N> {
+        self.native.borrow()
+    }
+
+    /// Borrow the underlying native display mutably.
+    ///
+    /// This follows the same semantics as `std::cell:RefCell`.
+    /// Holding any other borrow while trying to borrow the backend
+    /// mutably will panic. Note that EGL will borrow the display
+    /// mutably during surface creation.
+    pub fn borrow_mut(&self) -> RefMut<N> {
+        self.native.borrow_mut()
+    }
 }
 
 unsafe impl<B: native::Backend, N: native::NativeDisplay<B> + Send> Send for EGLContext<B, N> {}
diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs
index 3d6b15b..df3ac31 100644
--- a/src/backend/egl/surface.rs
+++ b/src/backend/egl/surface.rs
@@ -74,12 +74,11 @@ impl<N: native::NativeSurface> EGLSurface<N> {
                         err => return Err(SwapBuffersError::Unknown(err)),
                     };
                 } else {
-                    return Ok(());
+                    self.native.swap_buffers()?;
                 }
             } else {
                 return Err(SwapBuffersError::ContextLost);
             }
-            self.native.swap_buffers()?;
         };
 
         if self.native.needs_recreation() || surface.is_null() {
diff --git a/src/backend/graphics/format.rs b/src/backend/graphics/format.rs
index 7b3176d..96bef69 100644
--- a/src/backend/graphics/format.rs
+++ b/src/backend/graphics/format.rs
@@ -19,4 +19,4 @@ pub struct PixelFormat {
     pub multisampling: Option<u16>,
     /// is srgb enabled
     pub srgb: bool,
-}
\ No newline at end of file
+}
diff --git a/src/backend/graphics/gl.rs b/src/backend/graphics/gl.rs
index a39ce14..c811ff5 100644
--- a/src/backend/graphics/gl.rs
+++ b/src/backend/graphics/gl.rs
@@ -1,6 +1,6 @@
 use nix::libc::c_void;
 
-use super::{SwapBuffersError, PixelFormat};
+use super::{PixelFormat, SwapBuffersError};
 
 #[cfg_attr(feature = "cargo-clippy", allow(clippy))]
 #[allow(missing_docs)]
diff --git a/src/backend/winit.rs b/src/backend/winit.rs
index af2dd3f..0b94085 100644
--- a/src/backend/winit.rs
+++ b/src/backend/winit.rs
@@ -5,10 +5,7 @@ use backend::{
         context::GlAttributes, error as egl_error, error::Result as EGLResult, native, EGLContext,
         EGLDisplay, EGLGraphicsBackend, EGLSurface,
     },
-    graphics::{
-        gl::GLGraphicsBackend,
-        CursorBackend, SwapBuffersError, PixelFormat,
-    },
+    graphics::{gl::GLGraphicsBackend, CursorBackend, PixelFormat, SwapBuffersError},
     input::{
         Axis, AxisSource, Event as BackendEvent, InputBackend, InputHandler, KeyState, KeyboardKeyEvent,
         MouseButton, MouseButtonState, PointerAxisEvent, PointerButtonEvent, PointerMotionAbsoluteEvent,
@@ -17,7 +14,12 @@ use backend::{
     },
 };
 use nix::libc::c_void;
-use std::{cell::RefCell, cmp, error, fmt, rc::Rc, time::Instant};
+use std::{
+    cell::{Ref, RefCell},
+    cmp, error, fmt,
+    rc::Rc,
+    time::Instant,
+};
 use wayland_client::egl as wegl;
 use wayland_server::Display;
 use winit::{
@@ -56,10 +58,10 @@ enum Window {
 }
 
 impl Window {
-    fn window(&self) -> &WinitWindow {
+    fn window(&self) -> Ref<WinitWindow> {
         match *self {
-            Window::Wayland { ref context, .. } => &**context,
-            Window::X11 { ref context, .. } => &**context,
+            Window::Wayland { ref context, .. } => context.borrow(),
+            Window::X11 { ref context, .. } => context.borrow(),
         }
     }
 }
@@ -218,7 +220,7 @@ pub trait WinitEventsHandler {
 
 impl WinitGraphicsBackend {
     /// Get a reference to the internally used `winit::Window`
-    pub fn winit_window(&self) -> &WinitWindow {
+    pub fn winit_window(&self) -> Ref<WinitWindow> {
         self.window.window()
     }
 }