Merge pull request #209 from Smithay/fix/egl_glium_errors

Rework rendering loop

.github/workflows/ci.yml

@@ -50,7 +50,7 @@ jobs:
           override: true
       
       - name: System dependencies
-        run: sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev 
+        run: sudo apt-get update; sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev
 
       - name: Test features
         if: matrix.features != 'all'
@@ -100,7 +100,7 @@ jobs:
           override: true
 
       - name: System dependencies
-        run: sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev
+        run: sudo apt-get update; sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev
 
       - name: Test features
         if: matrix.features != 'all'
@@ -128,7 +128,7 @@ jobs:
           components: rustfmt, clippy
 
       - name: System dependencies
-        run: sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev
+        run: sudo apt-get update; sudo apt-get install libudev-dev libgbm-dev libxkbcommon-dev libegl1-mesa-dev libwayland-dev libinput-dev libsystemd-dev libdbus-1-dev
 
       - name: Cargo fmt
         run: cargo fmt --all -- --check

anvil/src/glium_drawer.rs

@@ -19,9 +19,13 @@ use smithay::{
         graphics::{
             gl::GLGraphicsBackend,
             glium::{Frame, GliumGraphicsBackend},
+            SwapBuffersError,
         },
     },
-    reexports::wayland_server::protocol::{wl_buffer, wl_surface},
+    reexports::{
+        calloop::LoopHandle,
+        wayland_server::protocol::{wl_buffer, wl_surface},
+    },
     wayland::{
         compositor::{roles::Role, SubsurfaceRole, TraversalAction},
         data_device::DnDIconRole,
@@ -457,3 +461,23 @@ impl<F: GLGraphicsBackend + 'static> GliumDrawer<F> {
         self.draw_surface_tree(frame, surface, (x, y), token, screen_dimensions);
     }
 }
+
+pub fn schedule_initial_render<F: GLGraphicsBackend + 'static, Data: 'static>(
+    renderer: Rc<GliumDrawer<F>>,
+    evt_handle: &LoopHandle<Data>,
+) {
+    let mut frame = renderer.draw();
+    frame.clear_color(0.8, 0.8, 0.9, 1.0);
+    if let Err(err) = frame.set_finish() {
+        match err {
+            SwapBuffersError::AlreadySwapped => {}
+            SwapBuffersError::TemporaryFailure(err) => {
+                // TODO dont reschedule after 3(?) retries
+                warn!(renderer.log, "Failed to submit page_flip: {}", err);
+                let handle = evt_handle.clone();
+                evt_handle.insert_idle(move |_| schedule_initial_render(renderer, &handle));
+            }
+            SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err),
+        }
+    }
+}

anvil/src/udev.rs

@@ -25,21 +25,28 @@ use smithay::{
             legacy::LegacyDrmDevice,
             DevPath, Device, DeviceHandler, Surface,
         },
-        graphics::CursorBackend,
+        graphics::{CursorBackend, SwapBuffersError},
         libinput::{LibinputInputBackend, LibinputSessionInterface},
         session::{
             auto::{auto_session_bind, AutoSession},
-            notify_multiplexer, AsSessionObserver, Session, SessionNotifier,
+            notify_multiplexer, AsSessionObserver, Session, SessionNotifier, SessionObserver,
         },
         udev::{primary_gpu, UdevBackend, UdevEvent},
     },
     reexports::{
-        calloop::{generic::Generic, EventLoop, LoopHandle, Source},
+        calloop::{
-        drm::control::{
+            generic::Generic,
+            timer::{Timer, TimerHandle},
+            EventLoop, LoopHandle, Source,
+        },
+        drm::{
+            self,
+            control::{
                 connector::{Info as ConnectorInfo, State as ConnectorState},
                 crtc,
                 encoder::Info as EncoderInfo,
             },
+        },
         image::{ImageBuffer, Rgba},
         input::Libinput,
         nix::{fcntl::OFlag, sys::stat::dev_t},
@@ -57,7 +64,7 @@ use smithay::{
 };
 
 use crate::buffer_utils::BufferUtils;
-use crate::glium_drawer::GliumDrawer;
+use crate::glium_drawer::{schedule_initial_render, GliumDrawer};
 use crate::shell::{MyWindowMap, Roles};
 use crate::state::AnvilState;
 
@@ -242,8 +249,9 @@ pub fn run_udev(
 
 struct BackendData<S: SessionNotifier> {
     id: S::Id,
+    restart_id: S::Id,
     event_source: Source<Generic<RenderDevice>>,
-    surfaces: Rc<RefCell<HashMap<crtc::Handle, GliumDrawer<RenderSurface>>>>,
+    surfaces: Rc<RefCell<HashMap<crtc::Handle, Rc<GliumDrawer<RenderSurface>>>>>,
 }
 
 struct UdevHandlerImpl<S: SessionNotifier, Data: 'static> {
@@ -270,7 +278,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
         device: &mut RenderDevice,
         egl_buffer_reader: Rc<RefCell<Option<EGLBufferReader>>>,
         logger: &::slog::Logger,
-    ) -> HashMap<crtc::Handle, GliumDrawer<RenderSurface>> {
+    ) -> HashMap<crtc::Handle, Rc<GliumDrawer<RenderSurface>>> {
         // Get a set of all modesetting resource handles (excluding planes):
         let res_handles = device.resource_handles().unwrap();
 
@@ -304,7 +312,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                             logger.clone(),
                         );
 
-                        entry.insert(renderer);
+                        entry.insert(Rc::new(renderer));
                         break;
                     }
                 }
@@ -318,7 +326,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
     pub fn scan_connectors(
         device: &mut RenderDevice,
         logger: &::slog::Logger,
-    ) -> HashMap<crtc::Handle, GliumDrawer<RenderSurface>> {
+    ) -> HashMap<crtc::Handle, Rc<GliumDrawer<RenderSurface>>> {
         // Get a set of all modesetting resource handles (excluding planes):
         let res_handles = device.resource_handles().unwrap();
 
@@ -347,7 +355,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                         let renderer =
                             GliumDrawer::init(device.create_surface(crtc).unwrap(), logger.clone());
 
-                        backends.insert(crtc, renderer);
+                        backends.insert(crtc, Rc::new(renderer));
                         break;
                     }
                 }
@@ -372,7 +380,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                 |fd| match FallbackDevice::new(SessionFd(fd), true, self.logger.clone()) {
                     Ok(drm) => Some(drm),
                     Err(err) => {
-                        error!(self.logger, "Skipping drm device, because of error: {}", err);
+                        warn!(self.logger, "Skipping drm device, because of error: {}", err);
                         None
                     }
                 },
@@ -380,14 +388,14 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
             .and_then(|drm| match GbmDevice::new(drm, self.logger.clone()) {
                 Ok(gbm) => Some(gbm),
                 Err(err) => {
-                    error!(self.logger, "Skipping gbm device, because of error: {}", err);
+                    warn!(self.logger, "Skipping gbm device, because of error: {}", err);
                     None
                 }
             })
             .and_then(|gbm| match EglDevice::new(gbm, self.logger.clone()) {
                 Ok(egl) => Some(egl),
                 Err(err) => {
-                    error!(self.logger, "Skipping egl device, because of error: {}", err);
+                    warn!(self.logger, "Skipping egl device, because of error: {}", err);
                     None
                 }
             })
@@ -417,7 +425,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
             // Set the handler.
             // Note: if you replicate this (very simple) structure, it is rather easy
             // to introduce reference cycles with Rc. Be sure about your drop order
-            device.set_handler(DrmHandlerImpl {
+            let renderer = Rc::new(DrmRenderer {
                 compositor_token: self.compositor_token,
                 backends: backends.clone(),
                 window_map: self.window_map.clone(),
@@ -426,6 +434,14 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                 dnd_icon: self.dnd_icon.clone(),
                 logger: self.logger.clone(),
             });
+            let restart_id = self.notifier.register(DrmRendererSessionListener {
+                renderer: renderer.clone(),
+                loop_handle: self.loop_handle.clone(),
+            });
+            device.set_handler(DrmHandlerImpl {
+                renderer,
+                loop_handle: self.loop_handle.clone(),
+            });
 
             let device_session_id = self.notifier.register(device.observer());
             let dev_id = device.device_id();
@@ -441,17 +457,14 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                     .unwrap();
 
                 // render first frame
-                {
+                schedule_initial_render(renderer.clone(), &self.loop_handle);
-                    let mut frame = renderer.draw();
-                    frame.clear_color(0.8, 0.8, 0.9, 1.0);
-                    frame.finish().unwrap();
-                }
             }
 
             self.backends.insert(
                 dev_id,
                 BackendData {
                     id: device_session_id,
+                    restart_id,
                     event_source,
                     surfaces: backends,
                 },
@@ -465,6 +478,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
             let logger = &self.logger;
             let pointer_image = &self.pointer_image;
             let egl_buffer_reader = self.egl_buffer_reader.clone();
+            let loop_handle = self.loop_handle.clone();
             self.loop_handle
                 .with_source(&backend_data.event_source, |source| {
                     let mut backends = backend_data.surfaces.borrow_mut();
@@ -486,11 +500,7 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
                             .unwrap();
 
                         // render first frame
-                        {
+                        schedule_initial_render(renderer.clone(), &loop_handle);
-                            let mut frame = renderer.draw();
-                            frame.clear_color(0.8, 0.8, 0.9, 1.0);
-                            frame.finish().unwrap();
-                        }
                     }
                 });
         }
@@ -514,14 +524,48 @@ impl<S: SessionNotifier, Data: 'static> UdevHandlerImpl<S, Data> {
             }
 
             self.notifier.unregister(backend_data.id);
+            self.notifier.unregister(backend_data.restart_id);
             debug!(self.logger, "Dropping device");
         }
     }
 }
 
-pub struct DrmHandlerImpl {
+pub struct DrmHandlerImpl<Data: 'static> {
+    renderer: Rc<DrmRenderer>,
+    loop_handle: LoopHandle<Data>,
+}
+
+impl<Data: 'static> DeviceHandler for DrmHandlerImpl<Data> {
+    type Device = RenderDevice;
+
+    fn vblank(&mut self, crtc: crtc::Handle) {
+        self.renderer.clone().render(crtc, None, Some(&self.loop_handle))
+    }
+
+    fn error(&mut self, error: <RenderSurface as Surface>::Error) {
+        error!(self.renderer.logger, "{:?}", error);
+    }
+}
+
+pub struct DrmRendererSessionListener<Data: 'static> {
+    renderer: Rc<DrmRenderer>,
+    loop_handle: LoopHandle<Data>,
+}
+
+impl<Data: 'static> SessionObserver for DrmRendererSessionListener<Data> {
+    fn pause(&mut self, _device: Option<(u32, u32)>) {}
+    fn activate(&mut self, _device: Option<(u32, u32, Option<RawFd>)>) {
+        // we want to be called, after all session handling is done (TODO this is not so nice)
+        let renderer = self.renderer.clone();
+        let handle = self.loop_handle.clone();
+        self.loop_handle
+            .insert_idle(move |_| renderer.render_all(Some(&handle)));
+    }
+}
+
+pub struct DrmRenderer {
     compositor_token: CompositorToken<Roles>,
-    backends: Rc<RefCell<HashMap<crtc::Handle, GliumDrawer<RenderSurface>>>>,
+    backends: Rc<RefCell<HashMap<crtc::Handle, Rc<GliumDrawer<RenderSurface>>>>>,
     window_map: Rc<RefCell<MyWindowMap>>,
     pointer_location: Rc<RefCell<(f64, f64)>>,
     cursor_status: Arc<Mutex<CursorImageStatus>>,
@@ -529,10 +573,18 @@ pub struct DrmHandlerImpl {
     logger: ::slog::Logger,
 }
 
-impl DeviceHandler for DrmHandlerImpl {
+impl DrmRenderer {
-    type Device = RenderDevice;
+    fn render_all<Data: 'static>(self: Rc<Self>, evt_handle: Option<&LoopHandle<Data>>) {
-
+        for crtc in self.backends.borrow().keys() {
-    fn vblank(&mut self, crtc: crtc::Handle) {
+            self.clone().render(*crtc, None, evt_handle);
+        }
+    }
+    fn render<Data: 'static>(
+        self: Rc<Self>,
+        crtc: crtc::Handle,
+        timer: Option<TimerHandle<(std::rc::Weak<DrmRenderer>, crtc::Handle)>>,
+        evt_handle: Option<&LoopHandle<Data>>,
+    ) {
         if let Some(drawer) = self.backends.borrow().get(&crtc) {
             {
                 let (x, y) = *self.pointer_location.borrow();
@@ -577,16 +629,67 @@ impl DeviceHandler for DrmHandlerImpl {
                 }
             }
 
-            if let Err(err) = frame.finish() {
+            let result = frame.finish();
-                error!(self.logger, "Error during rendering: {:?}", err);
+            if result.is_ok() {
+                // Send frame events so that client start drawing their next frame
+                self.window_map.borrow().send_frames(SCOUNTER.next_serial());
             }
 
+            if let Err(err) = result {
+                warn!(self.logger, "Error during rendering: {:?}", err);
+                let reschedule = match err {
+                    SwapBuffersError::AlreadySwapped => false,
+                    SwapBuffersError::TemporaryFailure(err) => {
+                        match err.downcast_ref::<smithay::backend::drm::common::Error>() {
+                            Some(&smithay::backend::drm::common::Error::DeviceInactive) => false,
+                            Some(&smithay::backend::drm::common::Error::Access { ref source, .. })
+                                if match source.get_ref() {
+                                    drm::SystemError::PermissionDenied => true,
+                                    _ => false,
+                                } =>
+                            {
+                                false
+                            }
+                            _ => true,
+                        }
+                    }
+                    SwapBuffersError::ContextLost(err) => panic!("Rendering loop lost: {}", err),
+                };
+
+                if reschedule {
+                    match (timer, evt_handle) {
+                        (Some(handle), _) => {
+                            let _ = handle.add_timeout(
+                                Duration::from_millis(1000 /*a seconds*/ / 60 /*refresh rate*/),
+                                (Rc::downgrade(&self), crtc),
+                            );
+                        }
+                        (None, Some(evt_handle)) => {
+                            let timer = Timer::new().unwrap();
+                            let handle = timer.handle();
+                            let _ = handle.add_timeout(
+                                Duration::from_millis(1000 /*a seconds*/ / 60 /*refresh rate*/),
+                                (Rc::downgrade(&self), crtc),
+                            );
+                            evt_handle
+                                .insert_source(timer, |(renderer, crtc), handle, _data| {
+                                    if let Some(renderer) = renderer.upgrade() {
+                                        renderer.render(
+                                            crtc,
+                                            Some(handle.clone()),
+                                            Option::<&LoopHandle<Data>>::None,
+                                        );
+                                    }
+                                })
+                                .unwrap();
+                        }
+                        _ => unreachable!(),
+                    }
+                }
+            } else {
                 // Send frame events so that client start drawing their next frame
                 self.window_map.borrow().send_frames(SCOUNTER.next_serial());
             }
         }
-
-    fn error(&mut self, error: <RenderSurface as Surface>::Error) {
-        error!(self.logger, "{:?}", error);
     }
 }

src/backend/drm/atomic/mod.rs

@@ -389,7 +389,6 @@ impl<A: AsRawFd + 'static> Device for AtomicDrmDevice<A> {
                 for event in events {
                     if let Event::PageFlip(event) = event {
                         trace!(self.logger, "Got a page-flip event for crtc ({:?})", event.crtc);
-                        if self.active.load(Ordering::SeqCst) {
                         if self
                             .backends
                             .borrow()
@@ -406,13 +405,6 @@ impl<A: AsRawFd + 'static> Device for AtomicDrmDevice<A> {
                         } else {
                             self.backends.borrow_mut().remove(&event.crtc);
                         }
-                        } else {
-                            debug!(
-                                self.logger,
-                                "Device ({:?}) not active. Ignoring PageFlip",
-                                self.dev_path()
-                            );
-                        }
                     } else {
                         trace!(
                             self.logger,

src/backend/drm/atomic/session.rs

@@ -16,7 +16,7 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 
 use super::{AtomicDrmDevice, AtomicDrmSurfaceInternal, Dev};
-use crate::backend::drm::{common::Error, DevPath};
+use crate::backend::drm::{common::Error, DevPath, Surface};
 use crate::backend::session::{AsSessionObserver, SessionObserver};
 
 /// [`SessionObserver`](SessionObserver)
@@ -164,6 +164,18 @@ impl<A: AsRawFd + 'static> AtomicDrmDeviceObserver<A> {
                     // lets force a non matching state
                     current.connectors.clear();
                     current.mode = unsafe { std::mem::zeroed() };
+
+                    // recreate property blob
+                    let mode = {
+                        let pending = surface.pending.read().unwrap();
+                        pending.mode
+                    };
+                    surface.use_mode(mode)?;
+
+                    // drop cursor state
+                    surface.cursor.position.set(None);
+                    surface.cursor.hotspot.set((0, 0));
+                    surface.cursor.framebuffer.set(None);
                 }
             }
         }

src/backend/drm/atomic/surface.rs

@@ -20,9 +20,9 @@ use crate::backend::graphics::CursorBackend;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct CursorState {
-    position: Cell<Option<(u32, u32)>>,
+    pub position: Cell<Option<(u32, u32)>>,
-    hotspot: Cell<(u32, u32)>,
+    pub hotspot: Cell<(u32, u32)>,
-    framebuffer: Cell<Option<framebuffer::Info>>,
+    pub framebuffer: Cell<Option<framebuffer::Handle>>,
 }
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -66,6 +66,11 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
         connectors: &[connector::Handle],
         logger: ::slog::Logger,
     ) -> Result<Self, Error> {
+        info!(
+            logger,
+            "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors
+        );
+
         let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access {
             errmsg: "Error loading crtc info",
             dev: dev.dev_path(),
@@ -454,7 +459,7 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
         }
 
         let mut current = self.state.write().unwrap();
-        let mut pending = self.pending.write().unwrap();
+        let pending = self.pending.write().unwrap();
 
         debug!(
             self.logger,
@@ -510,25 +515,14 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                     self.logger,
                     "New screen configuration invalid!:\n\t{:#?}\n\t{}\n", req, err
                 );
-                info!(self.logger, "Reverting back to last know good state");
 
-                *pending = current.clone();
+                return Err(err);
-
-                self.build_request(
-                    &mut [].iter(),
-                    &mut [].iter(),
-                    &self.planes,
-                    Some(framebuffer),
-                    Some(current.mode),
-                    Some(current.blob),
-                )?
             } else {
                 if current.mode != pending.mode {
                     if let Err(err) = self.dev.destroy_property_blob(current.blob.into()) {
                         warn!(self.logger, "Failed to destory old mode property blob: {}", err);
                     }
                 }
-                *current = pending.clone();
 
                 // new config
                 req
@@ -536,7 +530,8 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
         };
 
         debug!(self.logger, "Setting screen: {:?}", req);
-        self.atomic_commit(
+        let result = self
+            .atomic_commit(
                 &[
                     AtomicCommitFlags::PageFlipEvent,
                     AtomicCommitFlags::AllowModeset,
@@ -549,9 +544,13 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                 errmsg: "Error setting crtc",
                 dev: self.dev_path(),
                 source,
-        })?;
+            });
 
-        Ok(())
+        if result.is_ok() {
+            *current = pending.clone();
+        }
+
+        result
     }
 
     fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> {
@@ -568,7 +567,7 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
             None,
         )?;
 
-        trace!(self.logger, "Queueing page flip: {:#?}", req);
+        trace!(self.logger, "Queueing page flip: {:?}", req);
         self.atomic_commit(
             &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock],
             req,
@@ -610,20 +609,14 @@ impl<A: AsRawFd + 'static> CursorBackend for AtomicDrmSurfaceInternal<A> {
         trace!(self.logger, "Setting the new imported cursor");
 
         if let Some(fb) = self.cursor.framebuffer.get().take() {
-            let _ = self.destroy_framebuffer(fb.handle());
+            let _ = self.destroy_framebuffer(fb);
         }
 
         self.cursor.framebuffer.set(Some(
-            self.get_framebuffer(self.add_planar_framebuffer(buffer, &[0; 4], 0).compat().map_err(
+            self.add_planar_framebuffer(buffer, &[0; 4], 0)
-                |source| Error::Access {
-                    errmsg: "Failed to import cursor",
-                    dev: self.dev_path(),
-                    source,
-                },
-            )?)
                 .compat()
                 .map_err(|source| Error::Access {
-                errmsg: "Failed to get framebuffer info",
+                    errmsg: "Failed to import cursor",
                     dev: self.dev_path(),
                     source,
                 })?,
@@ -803,6 +796,12 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
         let cursor_fb = self.cursor.framebuffer.get();
 
         if let (Some(pos), Some(fb)) = (cursor_pos, cursor_fb) {
+            match self.get_framebuffer(fb).compat().map_err(|source| Error::Access {
+                errmsg: "Error getting cursor fb",
+                dev: self.dev_path(),
+                source,
+            }) {
+                Ok(cursor_info) => {
                     let hotspot = self.cursor.hotspot.get();
 
                     req.add_property(
@@ -823,12 +822,12 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                     req.add_property(
                         planes.cursor,
                         self.plane_prop_handle(planes.cursor, "SRC_W")?,
-                property::Value::UnsignedRange((fb.size().0 as u64) << 16),
+                        property::Value::UnsignedRange((cursor_info.size().0 as u64) << 16),
                     );
                     req.add_property(
                         planes.cursor,
                         self.plane_prop_handle(planes.cursor, "SRC_H")?,
-                property::Value::UnsignedRange((fb.size().1 as u64) << 16),
+                        property::Value::UnsignedRange((cursor_info.size().1 as u64) << 16),
                     );
                     req.add_property(
                         planes.cursor,
@@ -843,19 +842,25 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
                     req.add_property(
                         planes.cursor,
                         self.plane_prop_handle(planes.cursor, "CRTC_W")?,
-                property::Value::UnsignedRange(fb.size().0 as u64),
+                        property::Value::UnsignedRange(cursor_info.size().0 as u64),
                     );
                     req.add_property(
                         planes.cursor,
                         self.plane_prop_handle(planes.cursor, "CRTC_H")?,
-                property::Value::UnsignedRange(fb.size().1 as u64),
+                        property::Value::UnsignedRange(cursor_info.size().1 as u64),
                     );
                     req.add_property(
                         planes.cursor,
                         self.plane_prop_handle(planes.cursor, "FB_ID")?,
-                property::Value::Framebuffer(Some(fb.handle())),
+                        property::Value::Framebuffer(Some(fb)),
                     );
                 }
+                Err(err) => {
+                    warn!(self.logger, "Cursor FB invalid: {}. Skipping.", err);
+                    self.cursor.framebuffer.set(None);
+                }
+            }
+        }
 
         Ok(req)
     }

src/backend/drm/common/fallback.rs

@@ -191,7 +191,7 @@ impl<A: AsRawFd + Clone + 'static> FallbackDevice<AtomicDrmDevice<A>, LegacyDrmD
         match AtomicDrmDevice::new(fd.clone(), disable_connectors, log.clone()) {
             Ok(dev) => Ok(FallbackDevice::Preference(dev)),
             Err(err) => {
-                error!(log, "Failed to initialize preferred AtomicDrmDevice: {}", err);
+                warn!(log, "Failed to initialize preferred AtomicDrmDevice: {}", err);
                 info!(log, "Falling back to fallback LegacyDrmDevice");
                 Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(
                     fd,

src/backend/drm/common/mod.rs

@@ -76,8 +76,10 @@ impl Into<SwapBuffersError> for Error {
     fn into(self) -> SwapBuffersError {
         match self {
             x @ Error::DeviceInactive => SwapBuffersError::TemporaryFailure(Box::new(x)),
-            Error::Access { source, .. }
+            Error::Access {
-                if match source.get_ref() {
+                errmsg, dev, source, ..
+            } if match source.get_ref() {
+                drm::SystemError::PermissionDenied => true,
                 drm::SystemError::Unknown {
                     errno: nix::errno::Errno::EBUSY,
                 } => true,
@@ -87,7 +89,7 @@ impl Into<SwapBuffersError> for Error {
                 _ => false,
             } =>
             {
-                SwapBuffersError::TemporaryFailure(Box::new(source))
+                SwapBuffersError::TemporaryFailure(Box::new(Error::Access { errmsg, dev, source }))
             }
             x => SwapBuffersError::ContextLost(Box::new(x)),
         }

src/backend/drm/egl/mod.rs

@@ -11,7 +11,10 @@
 use drm::control::{connector, crtc, encoder, framebuffer, plane, Mode, ResourceHandles};
 use drm::SystemError as DrmError;
 use nix::libc::dev_t;
+use std::cell::RefCell;
+use std::collections::HashMap;
 use std::os::unix::io::{AsRawFd, RawFd};
+use std::rc::{Rc, Weak};
 #[cfg(feature = "use_system_lib")]
 use wayland_server::Display;
 
@@ -44,6 +47,7 @@ pub enum Error<U: std::error::Error + std::fmt::Debug + std::fmt::Display + 'sta
 }
 
 type Arguments = (crtc::Handle, Mode, Vec<connector::Handle>);
+type BackendRef<D> = Weak<EglSurfaceInternal<<D as Device>::Surface>>;
 
 /// Representation of an egl device to create egl rendering surfaces
 pub struct EglDevice<B, D>
@@ -58,6 +62,7 @@ where
     logger: ::slog::Logger,
     default_attributes: GlAttributes,
     default_requirements: PixelFormatRequirements,
+    backends: Rc<RefCell<HashMap<crtc::Handle, BackendRef<D>>>>,
 }
 
 impl<B, D> AsRawFd for EglDevice<B, D>
@@ -125,6 +130,7 @@ where
             dev: EGLDisplay::new(dev, log.clone()).map_err(Error::EGL)?,
             default_attributes,
             default_requirements,
+            backends: Rc::new(RefCell::new(HashMap::new())),
             logger: log,
         })
     }
@@ -208,7 +214,9 @@ where
                 SurfaceCreationError::NativeSurfaceCreationFailed(err) => Error::Underlying(err),
             })?;
 
-        Ok(EglSurface { context, surface })
+        let backend = Rc::new(EglSurfaceInternal { context, surface });
+        self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend));
+        Ok(EglSurface(backend))
     }
 
     fn process_events(&mut self) {

src/backend/drm/egl/session.rs

@@ -4,21 +4,28 @@
 //!
 
 use drm::control::{connector, crtc, Mode};
+use std::cell::RefCell;
+use std::collections::HashMap;
 use std::os::unix::io::RawFd;
+use std::rc::{Rc, Weak};
 
-use super::EglDevice;
+use super::{EglDevice, EglSurfaceInternal};
 use crate::backend::drm::{Device, Surface};
-use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface};
+use crate::backend::egl::{
+    ffi,
+    native::{Backend, NativeDisplay, NativeSurface},
+};
 use crate::backend::session::{AsSessionObserver, SessionObserver};
 
 /// [`SessionObserver`](SessionObserver)
 /// linked to the [`EglDevice`](EglDevice) it was
 /// created from.
-pub struct EglDeviceObserver<S: SessionObserver + 'static> {
+pub struct EglDeviceObserver<S: SessionObserver + 'static, N: NativeSurface + Surface> {
     observer: S,
+    backends: Weak<RefCell<HashMap<crtc::Handle, Weak<EglSurfaceInternal<N>>>>>,
 }
 
-impl<S, B, D> AsSessionObserver<EglDeviceObserver<S>> for EglDevice<B, D>
+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,
@@ -31,19 +38,32 @@ where
         + 'static,
     <D as Device>::Surface: NativeSurface,
 {
-    fn observer(&mut self) -> EglDeviceObserver<S> {
+    fn observer(&mut self) -> EglDeviceObserver<S, <D as Device>::Surface> {
         EglDeviceObserver {
             observer: self.dev.borrow_mut().observer(),
+            backends: Rc::downgrade(&self.backends),
         }
     }
 }
 
-impl<S: SessionObserver + 'static> SessionObserver for EglDeviceObserver<S> {
+impl<S: SessionObserver + 'static, N: NativeSurface + Surface> SessionObserver for EglDeviceObserver<S, N> {
     fn pause(&mut self, devnum: Option<(u32, u32)>) {
         self.observer.pause(devnum);
     }
 
     fn activate(&mut self, devnum: Option<(u32, u32, Option<RawFd>)>) {
         self.observer.activate(devnum);
+        if let Some(backends) = self.backends.upgrade() {
+            for (_crtc, backend) in backends.borrow().iter() {
+                if let Some(backend) = backend.upgrade() {
+                    let old_surface = backend.surface.surface.replace(std::ptr::null());
+                    if !old_surface.is_null() {
+                        unsafe {
+                            ffi::egl::DestroySurface(**backend.surface.display, old_surface as *const _);
+                        }
+                    }
+                }
+            }
+        }
     }
 }

src/backend/drm/egl/surface.rs

@@ -12,8 +12,12 @@ use crate::backend::graphics::gl::GLGraphicsBackend;
 use crate::backend::graphics::PixelFormat;
 use crate::backend::graphics::{CursorBackend, SwapBuffersError};
 
+use std::rc::Rc;
+
 /// Egl surface for rendering
-pub struct EglSurface<N>
+pub struct EglSurface<N: native::NativeSurface + Surface>(pub(super) Rc<EglSurfaceInternal<N>>);
+
+pub(super) struct EglSurfaceInternal<N>
 where
     N: native::NativeSurface + Surface,
 {
@@ -29,41 +33,45 @@ where
     type Error = Error<<N as Surface>::Error>;
 
     fn crtc(&self) -> crtc::Handle {
-        (*self.surface).crtc()
+        (*self.0.surface).crtc()
     }
 
     fn current_connectors(&self) -> Self::Connectors {
-        self.surface.current_connectors()
+        self.0.surface.current_connectors()
     }
 
     fn pending_connectors(&self) -> Self::Connectors {
-        self.surface.pending_connectors()
+        self.0.surface.pending_connectors()
     }
 
     fn add_connector(&self, connector: connector::Handle) -> Result<(), Self::Error> {
-        self.surface.add_connector(connector).map_err(Error::Underlying)
+        self.0.surface.add_connector(connector).map_err(Error::Underlying)
     }
 
     fn remove_connector(&self, connector: connector::Handle) -> Result<(), Self::Error> {
-        self.surface
+        self.0
+            .surface
             .remove_connector(connector)
             .map_err(Error::Underlying)
     }
 
     fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
-        self.surface.set_connectors(connectors).map_err(Error::Underlying)
+        self.0
+            .surface
+            .set_connectors(connectors)
+            .map_err(Error::Underlying)
     }
 
     fn current_mode(&self) -> Mode {
-        self.surface.current_mode()
+        self.0.surface.current_mode()
     }
 
     fn pending_mode(&self) -> Mode {
-        self.surface.pending_mode()
+        self.0.surface.pending_mode()
     }
 
     fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> {
-        self.surface.use_mode(mode).map_err(Error::Underlying)
+        self.0.surface.use_mode(mode).map_err(Error::Underlying)
     }
 }
 
@@ -75,7 +83,7 @@ where
     type Error = <N as CursorBackend>::Error;
 
     fn set_cursor_position(&self, x: u32, y: u32) -> ::std::result::Result<(), Self::Error> {
-        self.surface.set_cursor_position(x, y)
+        self.0.surface.set_cursor_position(x, y)
     }
 
     fn set_cursor_representation(
@@ -83,7 +91,7 @@ where
         buffer: &Self::CursorFormat,
         hotspot: (u32, u32),
     ) -> ::std::result::Result<(), Self::Error> {
-        self.surface.set_cursor_representation(buffer, hotspot)
+        self.0.surface.set_cursor_representation(buffer, hotspot)
     }
 }
 
@@ -94,7 +102,7 @@ where
     <N as NativeSurface>::Error: Into<SwapBuffersError> + 'static,
 {
     fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> {
-        if let Err(err) = self.surface.swap_buffers() {
+        if let Err(err) = self.0.surface.swap_buffers() {
             Err(match err.try_into() {
                 Ok(x) => x,
                 Err(x) => x.into(),
@@ -114,16 +122,17 @@ where
     }
 
     fn is_current(&self) -> bool {
-        self.context.is_current() && self.surface.is_current()
+        self.0.context.is_current() && self.0.surface.is_current()
     }
 
     unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> {
-        self.context
+        self.0
-            .make_current_with_surface(&self.surface)
+            .context
+            .make_current_with_surface(&self.0.surface)
             .map_err(Into::into)
     }
 
     fn get_pixel_format(&self) -> PixelFormat {
-        self.surface.get_pixel_format()
+        self.0.surface.get_pixel_format()
     }
 }

src/backend/drm/gbm/mod.rs

@@ -274,7 +274,7 @@ where
                     _ => false,
                 } =>
             {
-                SwapBuffersError::TemporaryFailure(Box::new(x))
+                SwapBuffersError::TemporaryFailure(Box::new(Error::<E>::FramebufferCreationFailed(x)))
             }
             Error::Underlying(x) => x.into(),
             x => SwapBuffersError::ContextLost(Box::new(x)),

src/backend/drm/gbm/session.rs

@@ -58,22 +58,7 @@ impl<
         if let Some(backends) = self.backends.upgrade() {
             for (crtc, backend) in backends.borrow().iter() {
                 if let Some(backend) = backend.upgrade() {
-                    // restart rendering loop, if it was previously running
+                    backend.clear_framebuffers();
-                    if let Some(current_fb) = backend.current_frame_buffer.get() {
-                        let result = if backend.crtc.commit_pending() {
-                            backend.crtc.commit(current_fb)
-                        } else {
-                            RawSurface::page_flip(&backend.crtc, current_fb)
-                        };
-
-                        if let Err(err) = result {
-                            warn!(
-                                self.logger,
-                                "Failed to restart rendering loop. Re-creating resources. Error: {}", err
-                            );
-                            // TODO bubble up
-                        }
-                    }
 
                     // reset cursor
                     {

src/backend/drm/gbm/surface.rs

@@ -74,19 +74,32 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
         };
         self.next_buffer.set(Some(next_bo));
 
-        if self.recreated.get() {
+        if cfg!(debug_assertions) {
-            debug!(self.logger, "Commiting new state");
+            if let Err(err) = self.crtc.get_framebuffer(fb) {
-            self.crtc.commit(fb).map_err(Error::Underlying)?;
+                error!(self.logger, "Cached framebuffer invalid: {:?}: {}", fb, err);
-            self.recreated.set(false);
+            }
-        } else {
-            trace!(self.logger, "Queueing Page flip");
-            RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying)?;
         }
 
-        self.current_frame_buffer.set(Some(fb));
+        let result = if self.recreated.get() {
+            debug!(self.logger, "Commiting new state");
+            self.crtc.commit(fb).map_err(Error::Underlying)
+        } else {
+            trace!(self.logger, "Queueing Page flip");
+            RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying)
+        };
 
+        match result {
+            Ok(_) => {
+                self.recreated.set(false);
+                self.current_frame_buffer.set(Some(fb));
                 Ok(())
             }
+            Err(err) => {
+                self.unlock_buffer();
+                Err(err)
+            }
+        }
+    }
 
     pub fn recreate(&self) -> Result<(), Error<<<D as Device>::Surface as Surface>::Error>> {
         let (w, h) = self.pending_mode().size();
@@ -106,6 +119,17 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
             .map_err(Error::SurfaceCreationFailed)?;
 
         // Clean up buffers
+        self.clear_framebuffers();
+
+        // Drop the old surface after cleanup
+        *self.surface.borrow_mut() = surface;
+
+        self.recreated.set(true);
+
+        Ok(())
+    }
+
+    pub fn clear_framebuffers(&self) {
         if let Some(Ok(Some(fb))) = self.next_buffer.take().map(|mut bo| bo.take_userdata()) {
             if let Err(err) = self.crtc.destroy_framebuffer(fb) {
                 warn!(
@@ -123,13 +147,6 @@ impl<D: RawDevice + 'static> GbmSurfaceInternal<D> {
                 );
             }
         }
-
-        // Drop the old surface after cleanup
-        *self.surface.borrow_mut() = surface;
-
-        self.recreated.set(true);
-
-        Ok(())
     }
 }
 
@@ -228,27 +245,7 @@ impl<D: RawDevice + 'static> Drop for GbmSurfaceInternal<D> {
     fn drop(&mut self) {
         // Drop framebuffers attached to the userdata of the gbm surface buffers.
         // (They don't implement drop, as they need the device)
-        if let Ok(Some(fb)) = {
+        self.clear_framebuffers();
-            if let Some(mut next) = self.next_buffer.take() {
-                next.take_userdata()
-            } else {
-                Ok(None)
-            }
-        } {
-            // ignore failure at this point
-            let _ = self.crtc.destroy_framebuffer(fb);
-        }
-
-        if let Ok(Some(fb)) = {
-            if let Some(mut next) = self.front_buffer.take() {
-                next.take_userdata()
-            } else {
-                Ok(None)
-            }
-        } {
-            // ignore failure at this point
-            let _ = self.crtc.destroy_framebuffer(fb);
-        }
     }
 }
 

src/backend/drm/legacy/mod.rs

@@ -306,7 +306,6 @@ impl<A: AsRawFd + 'static> Device for LegacyDrmDevice<A> {
             Ok(events) => {
                 for event in events {
                     if let Event::PageFlip(event) = event {
-                        if self.active.load(Ordering::SeqCst) {
                         if self
                             .backends
                             .borrow()
@@ -323,9 +322,6 @@ impl<A: AsRawFd + 'static> Device for LegacyDrmDevice<A> {
                         } else {
                             self.backends.borrow_mut().remove(&event.crtc);
                         }
-                        } else {
-                            debug!(self.logger, "Device not active. Ignoring PageFlip");
-                        }
                     } else {
                         trace!(self.logger, "Unrelated event");
                     }

src/backend/drm/legacy/surface.rs

@@ -311,6 +311,11 @@ impl<A: AsRawFd + 'static> LegacyDrmSurfaceInternal<A> {
         connectors: &[connector::Handle],
         logger: ::slog::Logger,
     ) -> Result<LegacyDrmSurfaceInternal<A>, Error> {
+        info!(
+            logger,
+            "Initializing drm surface with mode {:?} and connectors {:?}", mode, connectors
+        );
+
         // Try to enumarate the current state to set the initial state variable correctly
         let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access {
             errmsg: "Error loading crtc info",

src/backend/egl/display.rs

@@ -150,7 +150,7 @@ impl<B: native::Backend, N: native::NativeDisplay<B>> EGLDisplay<B, N> {
         };
         info!(log, "EGL Extensions: {:?}", extensions);
 
-        if egl_version >= (1, 2) {
+        if egl_version <= (1, 2) {
             return Err(Error::OpenGlesNotSupported(None));
         }
         wrap_egl_call(|| unsafe { ffi::egl::BindAPI(ffi::egl::OPENGL_ES_API) })
@@ -485,7 +485,7 @@ impl EGLBufferReader {
         buffer: WlBuffer,
     ) -> ::std::result::Result<EGLImages, BufferAccessError> {
         let mut format: i32 = 0;
-        wrap_egl_call(|| unsafe {
+        let query = wrap_egl_call(|| unsafe {
             ffi::egl::QueryWaylandBufferWL(
                 **self.display,
                 buffer.as_ref().c_ptr() as _,
@@ -494,6 +494,9 @@ impl EGLBufferReader {
             )
         })
         .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?;
+        if query == ffi::egl::FALSE {
+            return Err(BufferAccessError::NotManaged(buffer, EGLError::BadParameter));
+        }
 
         let format = match format {
             x if x == ffi::egl::TEXTURE_RGB as i32 => Format::RGB,
@@ -502,7 +505,7 @@ impl EGLBufferReader {
             ffi::egl::TEXTURE_Y_UV_WL => Format::Y_UV,
             ffi::egl::TEXTURE_Y_U_V_WL => Format::Y_U_V,
             ffi::egl::TEXTURE_Y_XUXV_WL => Format::Y_XUXV,
-            _ => panic!("EGL returned invalid texture type"),
+            x => panic!("EGL returned invalid texture type: {}", x),
         };
 
         let mut width: i32 = 0;

src/backend/egl/surface.rs

@@ -12,12 +12,13 @@ use std::{
 
 /// EGL surface of a given EGL context for rendering
 pub struct EGLSurface<N: native::NativeSurface> {
-    display: Arc<EGLDisplayHandle>,
+    pub(crate) display: Arc<EGLDisplayHandle>,
     native: N,
     pub(crate) surface: Cell<ffi::egl::types::EGLSurface>,
     config_id: ffi::egl::types::EGLConfig,
     pixel_format: PixelFormat,
     surface_attributes: Vec<c_int>,
+    logger: ::slog::Logger,
 }
 
 impl<N: native::NativeSurface> Deref for EGLSurface<N> {
@@ -72,6 +73,10 @@ impl<N: native::NativeSurface> EGLSurface<N> {
             ffi::egl::CreateWindowSurface(**display, config, native.ptr(), surface_attributes.as_ptr())
         })?;
 
+        if surface == ffi::egl::NO_SURFACE {
+            return Err(EGLError::BadSurface);
+        }
+
         Ok(EGLSurface {
             display,
             native,
@@ -79,6 +84,7 @@ impl<N: native::NativeSurface> EGLSurface<N> {
             config_id: config,
             pixel_format,
             surface_attributes,
+            logger: log,
         })
     }
 
@@ -91,7 +97,7 @@ impl<N: native::NativeSurface> EGLSurface<N> {
                 .map_err(SwapBuffersError::EGLSwapBuffers)
                 .and_then(|_| self.native.swap_buffers().map_err(SwapBuffersError::Underlying))
         } else {
-            Ok(())
+            Err(SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface))
         };
 
         // workaround for missing `PartialEq` impl
@@ -117,10 +123,15 @@ impl<N: native::NativeSurface> EGLSurface<N> {
                 })
                 .map_err(SwapBuffersError::EGLCreateWindowSurface)?
             });
-        }
 
+            result.map_err(|err| {
+                debug!(self.logger, "Hiding page-flip error *before* recreation: {}", err);
+                SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface)
+            })
+        } else {
             result
         }
+    }
 
     /// Returns true if the OpenGL surface is the current one in the thread.
     pub fn is_current(&self) -> bool {

src/backend/session/dbus/logind.rs

@@ -278,21 +278,36 @@ impl LogindSessionImpl {
                     let (major, minor, pause_type) = message.get3::<u32, u32, String>();
                     let major = major.ok_or(Error::UnexpectedMethodReturn)?;
                     let minor = minor.ok_or(Error::UnexpectedMethodReturn)?;
+                    // From https://www.freedesktop.org/wiki/Software/systemd/logind/:
+                    //  `force` means the device got paused by logind already and this is only an
+                    //  asynchronous notification.
+                    //  `pause` means logind tries to pause the device and grants you limited amount
+                    //  of time to pause it. You must respond to this via PauseDeviceComplete().
+                    //  This synchronous pausing-mechanism is used for backwards-compatibility to VTs
+                    //  and logind is **free to not make use of it**.
+                    //  It is also free to send a forced PauseDevice if you don't respond in a timely manner
+                    //  (or for any other reason).
                     let pause_type = pause_type.ok_or(Error::UnexpectedMethodReturn)?;
                     debug!(
                         self.logger,
                         "Request of type \"{}\" to close device ({},{})", pause_type, major, minor
                     );
+
+                    // gone means the device was unplugged from the system and you will no longer get any
+                    // notifications about it.
+                    // This is handled via udev and is not part of our session api.
+                    if pause_type != "gone" {
                         for signal in &mut *self.signals.borrow_mut() {
                             if let Some(ref mut signal) = signal {
                                 signal.pause(Some((major, minor)));
                             }
                         }
+                    }
                     // the other possible types are "force" or "gone" (unplugged),
                     // both expect no acknowledgement (note even this is not *really* necessary,
                     // logind would just timeout and send a "force" event. There is no way to
                     // keep the device.)
-                    if &*pause_type == "pause" {
+                    if pause_type == "pause" {
                         LogindSessionImpl::blocking_call(
                             &*self.conn.borrow(),
                             "org.freedesktop.login1",