diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 435f365..aea1f8a 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/anvil/src/glium_drawer.rs b/anvil/src/glium_drawer.rs index 25a03d3..49b7301 100644 --- a/anvil/src/glium_drawer.rs +++ b/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 GliumDrawer { self.draw_surface_tree(frame, surface, (x, y), token, screen_dimensions); } } + +pub fn schedule_initial_render( + renderer: Rc>, + evt_handle: &LoopHandle, +) { + 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), + } + } +} diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 008c44a..6f54a68 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -25,20 +25,27 @@ 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}, - drm::control::{ - connector::{Info as ConnectorInfo, State as ConnectorState}, - crtc, - encoder::Info as EncoderInfo, + calloop::{ + 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, @@ -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 { id: S::Id, + restart_id: S::Id, event_source: Source>, - surfaces: Rc>>>, + surfaces: Rc>>>>, } struct UdevHandlerImpl { @@ -270,7 +278,7 @@ impl UdevHandlerImpl { device: &mut RenderDevice, egl_buffer_reader: Rc>>, logger: &::slog::Logger, - ) -> HashMap> { + ) -> HashMap>> { // Get a set of all modesetting resource handles (excluding planes): let res_handles = device.resource_handles().unwrap(); @@ -304,7 +312,7 @@ impl UdevHandlerImpl { logger.clone(), ); - entry.insert(renderer); + entry.insert(Rc::new(renderer)); break; } } @@ -318,7 +326,7 @@ impl UdevHandlerImpl { pub fn scan_connectors( device: &mut RenderDevice, logger: &::slog::Logger, - ) -> HashMap> { + ) -> HashMap>> { // Get a set of all modesetting resource handles (excluding planes): let res_handles = device.resource_handles().unwrap(); @@ -347,7 +355,7 @@ impl UdevHandlerImpl { 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 UdevHandlerImpl { |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 UdevHandlerImpl { .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 UdevHandlerImpl { // 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 UdevHandlerImpl { 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 UdevHandlerImpl { .unwrap(); // render first frame - { - let mut frame = renderer.draw(); - frame.clear_color(0.8, 0.8, 0.9, 1.0); - frame.finish().unwrap(); - } + schedule_initial_render(renderer.clone(), &self.loop_handle); } self.backends.insert( dev_id, BackendData { id: device_session_id, + restart_id, event_source, surfaces: backends, }, @@ -465,6 +478,7 @@ impl UdevHandlerImpl { 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 UdevHandlerImpl { .unwrap(); // render first frame - { - let mut frame = renderer.draw(); - frame.clear_color(0.8, 0.8, 0.9, 1.0); - frame.finish().unwrap(); - } + schedule_initial_render(renderer.clone(), &loop_handle); } }); } @@ -514,14 +524,48 @@ impl UdevHandlerImpl { } self.notifier.unregister(backend_data.id); + self.notifier.unregister(backend_data.restart_id); debug!(self.logger, "Dropping device"); } } } -pub struct DrmHandlerImpl { +pub struct DrmHandlerImpl { + renderer: Rc, + loop_handle: LoopHandle, +} + +impl DeviceHandler for DrmHandlerImpl { + 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: ::Error) { + error!(self.renderer.logger, "{:?}", error); + } +} + +pub struct DrmRendererSessionListener { + renderer: Rc, + loop_handle: LoopHandle, +} + +impl SessionObserver for DrmRendererSessionListener { + fn pause(&mut self, _device: Option<(u32, u32)>) {} + fn activate(&mut self, _device: Option<(u32, u32, Option)>) { + // 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, - backends: Rc>>>, + backends: Rc>>>>, window_map: Rc>, pointer_location: Rc>, cursor_status: Arc>, @@ -529,10 +573,18 @@ pub struct DrmHandlerImpl { logger: ::slog::Logger, } -impl DeviceHandler for DrmHandlerImpl { - type Device = RenderDevice; - - fn vblank(&mut self, crtc: crtc::Handle) { +impl DrmRenderer { + fn render_all(self: Rc, evt_handle: Option<&LoopHandle>) { + for crtc in self.backends.borrow().keys() { + self.clone().render(*crtc, None, evt_handle); + } + } + fn render( + self: Rc, + crtc: crtc::Handle, + timer: Option, crtc::Handle)>>, + evt_handle: Option<&LoopHandle>, + ) { 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() { - error!(self.logger, "Error during rendering: {:?}", err); + let result = frame.finish(); + if result.is_ok() { + // Send frame events so that client start drawing their next frame + self.window_map.borrow().send_frames(SCOUNTER.next_serial()); } - // 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::() { + 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>::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: ::Error) { - error!(self.logger, "{:?}", error); - } } diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index 769e653..fa0911b 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -389,29 +389,21 @@ impl Device for AtomicDrmDevice { 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() - .get(&event.crtc) - .iter() - .flat_map(|x| x.upgrade()) - .next() - .is_some() - { - trace!(self.logger, "Handling event for backend {:?}", event.crtc); - if let Some(handler) = self.handler.as_ref() { - handler.borrow_mut().vblank(event.crtc); - } - } else { - self.backends.borrow_mut().remove(&event.crtc); + if self + .backends + .borrow() + .get(&event.crtc) + .iter() + .flat_map(|x| x.upgrade()) + .next() + .is_some() + { + trace!(self.logger, "Handling event for backend {:?}", event.crtc); + if let Some(handler) = self.handler.as_ref() { + handler.borrow_mut().vblank(event.crtc); } } else { - debug!( - self.logger, - "Device ({:?}) not active. Ignoring PageFlip", - self.dev_path() - ); + self.backends.borrow_mut().remove(&event.crtc); } } else { trace!( diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index f5f2b03..8f15c13 100644 --- a/src/backend/drm/atomic/session.rs +++ b/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 AtomicDrmDeviceObserver { // 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); } } } diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 9e467ce..347a0ea 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/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>, - hotspot: Cell<(u32, u32)>, - framebuffer: Cell>, + pub position: Cell>, + pub hotspot: Cell<(u32, u32)>, + pub framebuffer: Cell>, } #[derive(Debug, PartialEq, Eq, Clone)] @@ -66,6 +66,11 @@ impl AtomicDrmSurfaceInternal { connectors: &[connector::Handle], logger: ::slog::Logger, ) -> Result { + 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 RawSurface for AtomicDrmSurfaceInternal { } 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 RawSurface for AtomicDrmSurfaceInternal { 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(); - - self.build_request( - &mut [].iter(), - &mut [].iter(), - &self.planes, - Some(framebuffer), - Some(current.mode), - Some(current.blob), - )? + return Err(err); } 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,22 +530,27 @@ impl RawSurface for AtomicDrmSurfaceInternal { }; debug!(self.logger, "Setting screen: {:?}", req); - self.atomic_commit( - &[ - AtomicCommitFlags::PageFlipEvent, - AtomicCommitFlags::AllowModeset, - AtomicCommitFlags::Nonblock, - ], - req, - ) - .compat() - .map_err(|source| Error::Access { - errmsg: "Error setting crtc", - dev: self.dev_path(), - source, - })?; + let result = self + .atomic_commit( + &[ + AtomicCommitFlags::PageFlipEvent, + AtomicCommitFlags::AllowModeset, + AtomicCommitFlags::Nonblock, + ], + req, + ) + .compat() + .map_err(|source| Error::Access { + 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 RawSurface for AtomicDrmSurfaceInternal { None, )?; - trace!(self.logger, "Queueing page flip: {:#?}", req); + trace!(self.logger, "Queueing page flip: {:?}", req); self.atomic_commit( &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock], req, @@ -610,23 +609,17 @@ impl CursorBackend for AtomicDrmSurfaceInternal { 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( - |source| Error::Access { + self.add_planar_framebuffer(buffer, &[0; 4], 0) + .compat() + .map_err(|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", - dev: self.dev_path(), - source, - })?, + })?, )); self.cursor.hotspot.set(hotspot); @@ -803,58 +796,70 @@ impl AtomicDrmSurfaceInternal { let cursor_fb = self.cursor.framebuffer.get(); if let (Some(pos), Some(fb)) = (cursor_pos, cursor_fb) { - let hotspot = self.cursor.hotspot.get(); + 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( - planes.cursor, - self.plane_prop_handle(planes.cursor, "CRTC_ID")?, - property::Value::CRTC(Some(self.crtc)), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "SRC_X")?, - property::Value::UnsignedRange(0), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "SRC_Y")?, - property::Value::UnsignedRange(0), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "SRC_W")?, - property::Value::UnsignedRange((fb.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), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "CRTC_X")?, - property::Value::SignedRange(pos.0 as i64 - (hotspot.0 as i64)), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "CRTC_Y")?, - property::Value::SignedRange(pos.1 as i64 - (hotspot.1 as i64)), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "CRTC_W")?, - property::Value::UnsignedRange(fb.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), - ); - req.add_property( - planes.cursor, - self.plane_prop_handle(planes.cursor, "FB_ID")?, - property::Value::Framebuffer(Some(fb.handle())), - ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "CRTC_ID")?, + property::Value::CRTC(Some(self.crtc)), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "SRC_X")?, + property::Value::UnsignedRange(0), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "SRC_Y")?, + property::Value::UnsignedRange(0), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "SRC_W")?, + 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((cursor_info.size().1 as u64) << 16), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "CRTC_X")?, + property::Value::SignedRange(pos.0 as i64 - (hotspot.0 as i64)), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "CRTC_Y")?, + property::Value::SignedRange(pos.1 as i64 - (hotspot.1 as i64)), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "CRTC_W")?, + 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(cursor_info.size().1 as u64), + ); + req.add_property( + planes.cursor, + self.plane_prop_handle(planes.cursor, "FB_ID")?, + property::Value::Framebuffer(Some(fb)), + ); + } + Err(err) => { + warn!(self.logger, "Cursor FB invalid: {}. Skipping.", err); + self.cursor.framebuffer.set(None); + } + } } Ok(req) diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index e5d666e..6a47b31 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -191,7 +191,7 @@ impl FallbackDevice, 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, diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index c851e40..3ee2723 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -76,18 +76,20 @@ impl Into for Error { fn into(self) -> SwapBuffersError { match self { x @ Error::DeviceInactive => SwapBuffersError::TemporaryFailure(Box::new(x)), - Error::Access { source, .. } - if match source.get_ref() { - drm::SystemError::Unknown { - errno: nix::errno::Errno::EBUSY, - } => true, - drm::SystemError::Unknown { - errno: nix::errno::Errno::EINTR, - } => true, - _ => false, - } => + Error::Access { + errmsg, dev, source, .. + } if match source.get_ref() { + drm::SystemError::PermissionDenied => true, + drm::SystemError::Unknown { + errno: nix::errno::Errno::EBUSY, + } => true, + drm::SystemError::Unknown { + errno: nix::errno::Errno::EINTR, + } => true, + _ => false, + } => { - SwapBuffersError::TemporaryFailure(Box::new(source)) + SwapBuffersError::TemporaryFailure(Box::new(Error::Access { errmsg, dev, source })) } x => SwapBuffersError::ContextLost(Box::new(x)), } diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 61614a1..e17ead8 100644 --- a/src/backend/drm/egl/mod.rs +++ b/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); +type BackendRef = Weak::Surface>>; /// Representation of an egl device to create egl rendering surfaces pub struct EglDevice @@ -58,6 +62,7 @@ where logger: ::slog::Logger, default_attributes: GlAttributes, default_requirements: PixelFormatRequirements, + backends: Rc>>>, } impl AsRawFd for EglDevice @@ -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) { diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs index ff12afe..6a6f2e0 100644 --- a/src/backend/drm/egl/session.rs +++ b/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 { +pub struct EglDeviceObserver { observer: S, + backends: Weak>>>>, } -impl AsSessionObserver> for EglDevice +impl AsSessionObserver::Surface>> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, @@ -31,19 +38,32 @@ where + 'static, ::Surface: NativeSurface, { - fn observer(&mut self) -> EglDeviceObserver { + fn observer(&mut self) -> EglDeviceObserver::Surface> { EglDeviceObserver { observer: self.dev.borrow_mut().observer(), + backends: Rc::downgrade(&self.backends), } } } -impl SessionObserver for EglDeviceObserver { +impl SessionObserver for EglDeviceObserver { fn pause(&mut self, devnum: Option<(u32, u32)>) { self.observer.pause(devnum); } fn activate(&mut self, devnum: Option<(u32, u32, Option)>) { 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 _); + } + } + } + } + } } } diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs index 7bb01c0..a6592a3 100644 --- a/src/backend/drm/egl/surface.rs +++ b/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 +pub struct EglSurface(pub(super) Rc>); + +pub(super) struct EglSurfaceInternal where N: native::NativeSurface + Surface, { @@ -29,41 +33,45 @@ where type Error = Error<::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 = ::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 ::Error: Into + '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 - .make_current_with_surface(&self.surface) + self.0 + .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() } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index d4cc66a..2cd9523 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -274,7 +274,7 @@ where _ => false, } => { - SwapBuffersError::TemporaryFailure(Box::new(x)) + SwapBuffersError::TemporaryFailure(Box::new(Error::::FramebufferCreationFailed(x))) } Error::Underlying(x) => x.into(), x => SwapBuffersError::ContextLost(Box::new(x)), diff --git a/src/backend/drm/gbm/session.rs b/src/backend/drm/gbm/session.rs index 0fb6a28..4ae20c8 100644 --- a/src/backend/drm/gbm/session.rs +++ b/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 - 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 - } - } + backend.clear_framebuffers(); // reset cursor { diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 1668d17..2076905 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -74,18 +74,31 @@ impl GbmSurfaceInternal { }; self.next_buffer.set(Some(next_bo)); - if self.recreated.get() { - debug!(self.logger, "Commiting new state"); - self.crtc.commit(fb).map_err(Error::Underlying)?; - self.recreated.set(false); - } else { - trace!(self.logger, "Queueing Page flip"); - RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying)?; + if cfg!(debug_assertions) { + if let Err(err) = self.crtc.get_framebuffer(fb) { + error!(self.logger, "Cached framebuffer invalid: {:?}: {}", fb, err); + } } - 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) + }; - Ok(()) + 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<<::Surface as Surface>::Error>> { @@ -106,6 +119,17 @@ impl GbmSurfaceInternal { .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 GbmSurfaceInternal { ); } } - - // Drop the old surface after cleanup - *self.surface.borrow_mut() = surface; - - self.recreated.set(true); - - Ok(()) } } @@ -228,27 +245,7 @@ impl Drop for GbmSurfaceInternal { 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)) = { - 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); - } + self.clear_framebuffers(); } } diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 000b7c6..d99294f 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -306,25 +306,21 @@ impl Device for LegacyDrmDevice { Ok(events) => { for event in events { if let Event::PageFlip(event) = event { - if self.active.load(Ordering::SeqCst) { - if self - .backends - .borrow() - .get(&event.crtc) - .iter() - .flat_map(|x| x.upgrade()) - .next() - .is_some() - { - trace!(self.logger, "Handling event for backend {:?}", event.crtc); - if let Some(handler) = self.handler.as_ref() { - handler.borrow_mut().vblank(event.crtc); - } - } else { - self.backends.borrow_mut().remove(&event.crtc); + if self + .backends + .borrow() + .get(&event.crtc) + .iter() + .flat_map(|x| x.upgrade()) + .next() + .is_some() + { + trace!(self.logger, "Handling event for backend {:?}", event.crtc); + if let Some(handler) = self.handler.as_ref() { + handler.borrow_mut().vblank(event.crtc); } } else { - debug!(self.logger, "Device not active. Ignoring PageFlip"); + self.backends.borrow_mut().remove(&event.crtc); } } else { trace!(self.logger, "Unrelated event"); diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index de931da..be2f6f9 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -311,6 +311,11 @@ impl LegacyDrmSurfaceInternal { connectors: &[connector::Handle], logger: ::slog::Logger, ) -> Result, 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", diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index baf716b..4c8462c 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -150,7 +150,7 @@ impl> EGLDisplay { }; 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 { 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; diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 4e175d2..3ba5f32 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -12,12 +12,13 @@ use std::{ /// EGL surface of a given EGL context for rendering pub struct EGLSurface { - display: Arc, + pub(crate) display: Arc, native: N, pub(crate) surface: Cell, config_id: ffi::egl::types::EGLConfig, pixel_format: PixelFormat, surface_attributes: Vec, + logger: ::slog::Logger, } impl Deref for EGLSurface { @@ -72,6 +73,10 @@ impl EGLSurface { 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 EGLSurface { config_id: config, pixel_format, surface_attributes, + logger: log, }) } @@ -91,7 +97,7 @@ impl EGLSurface { .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,9 +123,14 @@ impl EGLSurface { }) .map_err(SwapBuffersError::EGLCreateWindowSurface)? }); - } - result + 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. diff --git a/src/backend/session/dbus/logind.rs b/src/backend/session/dbus/logind.rs index b049e2a..6473c25 100644 --- a/src/backend/session/dbus/logind.rs +++ b/src/backend/session/dbus/logind.rs @@ -278,21 +278,36 @@ impl LogindSessionImpl { let (major, minor, pause_type) = message.get3::(); 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 ); - for signal in &mut *self.signals.borrow_mut() { - if let Some(ref mut signal) = signal { - signal.pause(Some((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",