diff --git a/anvil/src/glium_drawer.rs b/anvil/src/glium_drawer.rs index b6e8f86..72306a6 100644 --- a/anvil/src/glium_drawer.rs +++ b/anvil/src/glium_drawer.rs @@ -7,7 +7,7 @@ use glium::{ self, index::PrimitiveType, texture::{MipmapsOption, Texture2d, UncompressedFloatFormat}, - Frame, GlObject, Surface, + GlObject, Surface, }; use slog::Logger; @@ -16,7 +16,10 @@ use smithay::backend::egl::display::EGLBufferReader; use smithay::{ backend::{ egl::{BufferAccessError, EGLImages, Format}, - graphics::{gl::GLGraphicsBackend, glium::GliumGraphicsBackend}, + graphics::{ + gl::GLGraphicsBackend, + glium::{Frame, GliumGraphicsBackend}, + }, }, reexports::wayland_server::protocol::{wl_buffer, wl_surface}, wayland::{ @@ -159,7 +162,10 @@ impl GliumDrawer { let images = if let Some(display) = &self.egl_buffer_reader.borrow().as_ref() { display.egl_buffer_contents(buffer) } else { - Err(BufferAccessError::NotManaged(buffer)) + Err(BufferAccessError::NotManaged( + buffer, + smithay::backend::egl::EGLError::BadDisplay, + )) }; match images { Ok(images) => { @@ -193,7 +199,7 @@ impl GliumDrawer { images: Some(images), // I guess we need to keep this alive ? }) } - Err(BufferAccessError::NotManaged(buffer)) => { + Err(BufferAccessError::NotManaged(buffer, _)) => { // this is not an EGL buffer, try SHM self.texture_from_shm_buffer(buffer) } @@ -235,7 +241,7 @@ impl GliumDrawer { pub fn render_texture( &self, - target: &mut glium::Frame, + target: &mut Frame, texture: &Texture2d, texture_kind: usize, y_inverted: bool, diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 206c909..5d021cb 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -324,7 +324,9 @@ impl UdevHandlerImpl { for crtc in res_handles.filter_crtcs(encoder_info.possible_crtcs()) { if let Entry::Vacant(entry) = backends.entry(crtc) { let renderer = GliumDrawer::init( - device.create_surface(crtc).unwrap(), + device + .create_surface(crtc, connector_info.modes()[0], &[connector_info.handle()]) + .unwrap(), egl_buffer_reader.clone(), logger.clone(), ); @@ -394,7 +396,7 @@ impl UdevHandlerImpl { ) .ok() .and_then( - |fd| match FallbackDevice::new(SessionFd(fd), self.logger.clone()) { + |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); diff --git a/examples/raw_atomic_drm.rs b/examples/raw_atomic_drm.rs index 9877376..bc1a2bb 100644 --- a/examples/raw_atomic_drm.rs +++ b/examples/raw_atomic_drm.rs @@ -56,7 +56,8 @@ fn main() { let mut options = OpenOptions::new(); options.read(true); options.write(true); - let mut device = AtomicDrmDevice::new(options.open("/dev/dri/card0").unwrap(), log.clone()).unwrap(); + let mut device = + AtomicDrmDevice::new(options.open("/dev/dri/card0").unwrap(), true, log.clone()).unwrap(); // Get a set of all modesetting resource handles (excluding planes): let res_handles = Device::resource_handles(&device).unwrap(); @@ -96,9 +97,11 @@ fn main() { let mode = connector_info.modes()[0]; // Use first mode (usually highest resoltion, but in reality you should filter and sort and check and match with other connectors, if you use more then one.) // Initialize the hardware backend - let surface = Rc::new(device.create_surface(crtc).unwrap()); - surface.set_connectors(&[connector_info.handle()]).unwrap(); - surface.use_mode(Some(mode)).unwrap(); + let surface = Rc::new( + device + .create_surface(crtc, mode, &[connector_info.handle()]) + .unwrap(), + ); for conn in surface.current_connectors().into_iter() { if conn != connector_info.handle() { diff --git a/examples/raw_legacy_drm.rs b/examples/raw_legacy_drm.rs index 6afa248..f986ad1 100644 --- a/examples/raw_legacy_drm.rs +++ b/examples/raw_legacy_drm.rs @@ -9,7 +9,7 @@ use smithay::{ common::Error, device_bind, legacy::{LegacyDrmDevice, LegacyDrmSurface}, - Device, DeviceHandler, RawSurface, Surface, + Device, DeviceHandler, RawSurface, }, reexports::{ calloop::EventLoop, @@ -40,7 +40,8 @@ fn main() { let mut options = OpenOptions::new(); options.read(true); options.write(true); - let mut device = LegacyDrmDevice::new(options.open("/dev/dri/card0").unwrap(), log.clone()).unwrap(); + let mut device = + LegacyDrmDevice::new(options.open("/dev/dri/card0").unwrap(), true, log.clone()).unwrap(); // Get a set of all modesetting resource handles (excluding planes): let res_handles = Device::resource_handles(&device).unwrap(); @@ -78,10 +79,12 @@ fn main() { let mode = connector_info.modes()[0]; // Use first mode (usually highest resoltion, but in reality you should filter and sort and check and match with other connectors, if you use more then one.) // Initialize the hardware backend - let surface = Rc::new(device.create_surface(crtc).unwrap()); + let surface = Rc::new( + device + .create_surface(crtc, mode, &[connector_info.handle()]) + .unwrap(), + ); - surface.use_mode(Some(mode)).unwrap(); - surface.set_connectors(&[connector_info.handle()]).unwrap(); /* * Lets create buffers and framebuffers. * We use drm-rs DumbBuffers, because they always work and require little to no setup. diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index 73653d6..769e653 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -19,7 +19,8 @@ use std::sync::{ use drm::control::{atomic::AtomicModeReq, AtomicCommitFlags, Device as ControlDevice, Event}; use drm::control::{ - connector, crtc, encoder, framebuffer, plane, property, PropertyValueSet, ResourceHandle, ResourceHandles, + connector, crtc, encoder, framebuffer, plane, property, Mode, PropertyValueSet, ResourceHandle, + ResourceHandles, }; use drm::SystemError as DrmError; use drm::{ClientCapability, Device as BasicDevice}; @@ -187,9 +188,21 @@ impl Dev { impl AtomicDrmDevice { /// Create a new [`AtomicDrmDevice`] from an open drm node /// - /// Returns an error if the file is no valid drm node or context creation was not - /// successful. - pub fn new(fd: A, logger: L) -> Result + /// # Arguments + /// + /// - `fd` - Open drm node + /// - `disable_connectors` - Setting this to true will initialize all connectors \ + /// as disabled on device creation. smithay enables connectors, when attached \ + /// to a surface, and disables them, when detached. Setting this to `false` \ + /// requires usage of `drm-rs` to disable unused connectors to prevent them \ + /// showing garbage, but will also prevent flickering of already turned on \ + /// connectors (assuming you won't change the resolution). + /// - `logger` - Optional [`slog::Logger`] to be used by this device. + /// + /// # Return + /// + /// Returns an error if the file is no valid drm node or the device is not accessible. + pub fn new(fd: A, disable_connectors: bool, logger: L) -> Result where L: Into>, { @@ -261,7 +274,49 @@ impl AtomicDrmDevice { dev.old_state = old_state; dev.prop_mapping = mapping; - debug!(log, "Mapping: {:#?}", dev.prop_mapping); + trace!(log, "Mapping: {:#?}", dev.prop_mapping); + + if disable_connectors { + // Disable all connectors as initial state + let mut req = AtomicModeReq::new(); + for conn in res_handles.connectors() { + let prop = dev + .prop_mapping + .0 + .get(&conn) + .expect("Unknown handle") + .get("CRTC_ID") + .expect("Unknown property CRTC_ID"); + req.add_property(*conn, *prop, property::Value::CRTC(None)); + } + // A crtc without a connector has no mode, we also need to reset that. + // Otherwise the commit will not be accepted. + for crtc in res_handles.crtcs() { + let active_prop = dev + .prop_mapping + .1 + .get(&crtc) + .expect("Unknown handle") + .get("ACTIVE") + .expect("Unknown property ACTIVE"); + let mode_prop = dev + .prop_mapping + .1 + .get(&crtc) + .expect("Unknown handle") + .get("MODE_ID") + .expect("Unknown property MODE_ID"); + req.add_property(*crtc, *mode_prop, property::Value::Unknown(0)); + req.add_property(*crtc, *active_prop, property::Value::Boolean(false)); + } + dev.atomic_commit(&[AtomicCommitFlags::AllowModeset], req) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to disable connectors", + dev: dev.dev_path(), + source, + })?; + } Ok(AtomicDrmDevice { dev: Rc::new(dev), @@ -298,7 +353,12 @@ impl Device for AtomicDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Error> { + fn create_surface( + &mut self, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + ) -> Result, Error> { if self.backends.borrow().contains_key(&crtc) { return Err(Error::CrtcAlreadyInUse(crtc)); } @@ -307,9 +367,15 @@ impl Device for AtomicDrmDevice { return Err(Error::DeviceInactive); } + if connectors.is_empty() { + return Err(Error::SurfaceWithoutConnectors(crtc)); + } + let backend = Rc::new(AtomicDrmSurfaceInternal::new( self.dev.clone(), crtc, + mode, + connectors, self.logger.new(o!("crtc" => format!("{:?}", crtc))), )?); diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index b58dbe0..f5f2b03 100644 --- a/src/backend/drm/atomic/session.rs +++ b/src/backend/drm/atomic/session.rs @@ -3,8 +3,9 @@ //! to an open [`Session`](::backend::session::Session). //! -use drm::control::crtc; +use drm::control::{atomic::AtomicModeReq, crtc, property, AtomicCommitFlags, Device as ControlDevice}; use drm::Device as BasicDevice; +use failure::ResultExt; use nix::libc::dev_t; use nix::sys::stat; use std::cell::RefCell; @@ -15,6 +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::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -59,7 +61,7 @@ impl SessionObserver for AtomicDrmDeviceObserver { if let Err(err) = surface.clear_plane(surface.planes.cursor) { warn!( self.logger, - "Failed to clear cursor on {:?}: {}", surface.crtc, err + "Failed to clear cursor on {:?}: {}", surface.planes.cursor, err ); } } @@ -95,5 +97,76 @@ impl SessionObserver for AtomicDrmDeviceObserver { } } self.active.store(true, Ordering::SeqCst); + // okay, the previous session/whatever might left the drm devices in any state... + // lets fix that + if let Err(err) = self.reset_state() { + warn!(self.logger, "Unable to reset state after tty switch: {}", err); + // TODO call drm-handler::error + } + } +} + +impl AtomicDrmDeviceObserver { + fn reset_state(&mut self) -> Result<(), Error> { + if let Some(dev) = self.dev.upgrade() { + let res_handles = ControlDevice::resource_handles(&*dev) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error loading drm resources", + dev: dev.dev_path(), + source, + })?; + + // Disable all connectors (otherwise we might run into conflicting commits when restarting the rendering loop) + let mut req = AtomicModeReq::new(); + for conn in res_handles.connectors() { + let prop = dev + .prop_mapping + .0 + .get(&conn) + .expect("Unknown handle") + .get("CRTC_ID") + .expect("Unknown property CRTC_ID"); + req.add_property(*conn, *prop, property::Value::CRTC(None)); + } + // A crtc without a connector has no mode, we also need to reset that. + // Otherwise the commit will not be accepted. + for crtc in res_handles.crtcs() { + let mode_prop = dev + .prop_mapping + .1 + .get(&crtc) + .expect("Unknown handle") + .get("MODE_ID") + .expect("Unknown property MODE_ID"); + let active_prop = dev + .prop_mapping + .1 + .get(&crtc) + .expect("Unknown handle") + .get("ACTIVE") + .expect("Unknown property ACTIVE"); + req.add_property(*crtc, *active_prop, property::Value::Boolean(false)); + req.add_property(*crtc, *mode_prop, property::Value::Unknown(0)); + } + dev.atomic_commit(&[AtomicCommitFlags::AllowModeset], req) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to disable connectors", + dev: dev.dev_path(), + source, + })?; + + if let Some(backends) = self.backends.upgrade() { + for surface in backends.borrow().values().filter_map(Weak::upgrade) { + let mut current = surface.state.write().unwrap(); + + // lets force a non matching state + current.connectors.clear(); + current.mode = unsafe { std::mem::zeroed() }; + } + } + } + Ok(()) } } diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 57c6246..9e467ce 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -1,21 +1,22 @@ use drm::buffer::Buffer; use drm::control::atomic::AtomicModeReq; use drm::control::Device as ControlDevice; -use drm::control::{connector, crtc, framebuffer, plane, property, AtomicCommitFlags, Mode, PlaneType}; +use drm::control::{ + connector, crtc, dumbbuffer::DumbBuffer, framebuffer, plane, property, AtomicCommitFlags, Mode, PlaneType, +}; use drm::Device as BasicDevice; use std::cell::Cell; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; -use std::sync::RwLock; +use std::sync::{atomic::Ordering, RwLock}; use failure::ResultExt as FailureResultExt; use super::Dev; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; -use crate::backend::graphics::SwapBuffersError; #[derive(Debug, Clone, PartialEq, Eq)] pub struct CursorState { @@ -26,8 +27,8 @@ pub struct CursorState { #[derive(Debug, PartialEq, Eq, Clone)] pub struct State { - pub mode: Option, - pub blob: Option>, + pub mode: Mode, + pub blob: property::Value<'static>, pub connectors: HashSet, } @@ -45,6 +46,7 @@ pub(super) struct AtomicDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, + pub(super) test_buffer: Cell>, } impl AsRawFd for AtomicDrmSurfaceInternal { @@ -57,27 +59,45 @@ impl BasicDevice for AtomicDrmSurfaceInternal {} impl ControlDevice for AtomicDrmSurfaceInternal {} impl AtomicDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result { + pub(crate) fn new( + dev: Rc>, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + logger: ::slog::Logger, + ) -> Result { let crtc_info = dev.get_crtc(crtc).compat().map_err(|source| Error::Access { errmsg: "Error loading crtc info", dev: dev.dev_path(), source, })?; - let mode = crtc_info.mode(); - let blob = match mode { - Some(mode) => Some( - dev.create_property_blob(mode) - .compat() - .map_err(|source| Error::Access { - errmsg: "Failed to create Property Blob for mode", - dev: dev.dev_path(), - source, - })?, - ), - None => None, + // If we have no current mode, we create a fake one, which will not match (and thus gets overriden on the commit below). + // A better fix would probably be making mode an `Option`, but that would mean + // we need to be sure, we require a mode to always be set without relying on the compiler. + // So we cheat, because it works and is easier to handle later. + let current_mode = crtc_info.mode().unwrap_or_else(|| unsafe { std::mem::zeroed() }); + let current_blob = match crtc_info.mode() { + Some(mode) => dev + .create_property_blob(mode) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create Property Blob for mode", + dev: dev.dev_path(), + source, + })?, + None => property::Value::Unknown(0), }; + let blob = dev + .create_property_blob(mode) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create Property Blob for mode", + dev: dev.dev_path(), + source, + })?; + let res_handles = ControlDevice::resource_handles(&*dev) .compat() .map_err(|source| Error::Access { @@ -86,12 +106,7 @@ impl AtomicDrmSurfaceInternal { source, })?; - let mut state = State { - mode, - blob, - connectors: HashSet::new(), - }; - + let mut current_connectors = HashSet::new(); for conn in res_handles.connectors() { let crtc_prop = dev .prop_mapping @@ -113,7 +128,7 @@ impl AtomicDrmSurfaceInternal { crtc_prop_info.value_type().convert_value(val) { if conn_crtc == crtc { - state.connectors.insert(*conn); + current_connectors.insert(*conn); } } break; @@ -121,13 +136,23 @@ impl AtomicDrmSurfaceInternal { } } } + let state = State { + mode: current_mode, + blob: current_blob, + connectors: current_connectors, + }; + let pending = State { + mode, + blob, + connectors: connectors.iter().copied().collect(), + }; let (primary, cursor) = AtomicDrmSurfaceInternal::find_planes(&dev, crtc).ok_or(Error::NoSuitablePlanes { crtc, dev: dev.dev_path(), })?; - Ok(AtomicDrmSurfaceInternal { + let surface = AtomicDrmSurfaceInternal { dev, crtc, cursor: CursorState { @@ -136,10 +161,102 @@ impl AtomicDrmSurfaceInternal { hotspot: Cell::new((0, 0)), }, planes: Planes { primary, cursor }, - state: RwLock::new(state.clone()), - pending: RwLock::new(state), + state: RwLock::new(state), + pending: RwLock::new(pending), logger, - }) + test_buffer: Cell::new(None), + }; + + Ok(surface) + } + + fn create_test_buffer(&self, mode: &Mode) -> Result { + let (w, h) = mode.size(); + let db = self + .create_dumb_buffer((w as u32, h as u32), drm::buffer::format::PixelFormat::ARGB8888) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create dumb buffer", + dev: self.dev_path(), + source, + })?; + let fb = self + .add_framebuffer(&db) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create framebuffer", + dev: self.dev_path(), + source, + })?; + if let Some((old_db, old_fb)) = self.test_buffer.replace(Some((db, fb))) { + let _ = self.destroy_framebuffer(old_fb); + let _ = self.destroy_dumb_buffer(old_db); + }; + + Ok(fb) + } +} + +impl Drop for AtomicDrmSurfaceInternal { + fn drop(&mut self) { + if let Some((db, fb)) = self.test_buffer.take() { + let _ = self.destroy_framebuffer(fb); + let _ = self.destroy_dumb_buffer(db); + } + + if !self.dev.active.load(Ordering::SeqCst) { + // the device is gone or we are on another tty + // old state has been restored, we shouldn't touch it. + // if we are on another tty the connectors will get disabled + // by the device, when switching back + return; + } + + // other ttys that use no cursor, might not clear it themselves. + // This makes sure our cursor won't stay visible. + if let Err(err) = self.clear_plane(self.planes.cursor) { + warn!( + self.logger, + "Failed to clear cursor on {:?}: {}", self.planes.cursor, err + ); + } + + // disable connectors again + let current = self.state.read().unwrap(); + let mut req = AtomicModeReq::new(); + for conn in current.connectors.iter() { + let prop = self + .dev + .prop_mapping + .0 + .get(&conn) + .expect("Unknown Handle") + .get("CRTC_ID") + .expect("Unknown property CRTC_ID"); + req.add_property(*conn, *prop, property::Value::CRTC(None)); + } + let active_prop = self + .dev + .prop_mapping + .1 + .get(&self.crtc) + .expect("Unknown Handle") + .get("ACTIVE") + .expect("Unknown property ACTIVE"); + let mode_prop = self + .dev + .prop_mapping + .1 + .get(&self.crtc) + .expect("Unknown Handle") + .get("MODE_ID") + .expect("Unknown property MODE_ID"); + + req.add_property(self.crtc, *active_prop, property::Value::Boolean(false)); + req.add_property(self.crtc, *mode_prop, property::Value::Unknown(0)); + if let Err(err) = self.atomic_commit(&[AtomicCommitFlags::AllowModeset], req) { + warn!(self.logger, "Unable to disable connectors: {}", err); + } } } @@ -159,15 +276,19 @@ impl Surface for AtomicDrmSurfaceInternal { self.pending.read().unwrap().connectors.clone() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.state.read().unwrap().mode } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.pending.read().unwrap().mode } fn add_connector(&self, conn: connector::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let info = self .get_connector(conn) .compat() @@ -180,15 +301,15 @@ impl Surface for AtomicDrmSurfaceInternal { let mut pending = self.pending.write().unwrap(); // check if the connector can handle the current mode - if info.modes().contains(pending.mode.as_ref().unwrap()) { + if info.modes().contains(&pending.mode) { // check if config is supported let req = self.build_request( &mut [conn].iter(), &mut [].iter(), &self.planes, - None, - pending.mode, - pending.blob, + Some(self.create_test_buffer(&pending.mode)?), + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -202,21 +323,30 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } else { - Err(Error::ModeNotSuitable(pending.mode.unwrap())) + Err(Error::ModeNotSuitable(pending.mode)) } } fn remove_connector(&self, conn: connector::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); + // the test would also prevent this, but the error message is far less helpful + if pending.connectors.contains(&conn) && pending.connectors.len() == 1 { + return Err(Error::SurfaceWithoutConnectors(self.crtc)); + } + // check if new config is supported (should be) let req = self.build_request( &mut [].iter(), &mut [conn].iter(), &self.planes, - None, - pending.mode, - pending.blob, + Some(self.create_test_buffer(&pending.mode)?), + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -232,6 +362,14 @@ impl Surface for AtomicDrmSurfaceInternal { } fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> { + if connectors.is_empty() { + return Err(Error::SurfaceWithoutConnectors(self.crtc)); + } + + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let current = self.state.write().unwrap(); let mut pending = self.pending.write().unwrap(); @@ -243,9 +381,9 @@ impl Surface for AtomicDrmSurfaceInternal { &mut added, &mut removed, &self.planes, - None, - pending.mode, - pending.blob, + Some(self.create_test_buffer(&pending.mode)?), + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( @@ -259,30 +397,31 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); // check if new config is supported - let new_blob = Some(match mode { - Some(mode) => self - .dev - .create_property_blob(mode) - .compat() - .map_err(|source| Error::Access { - errmsg: "Failed to create Property Blob for mode", - dev: self.dev_path(), - source, - })?, - None => property::Value::Unknown(0), - }); + let new_blob = self + .create_property_blob(mode) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to create Property Blob for mode", + dev: self.dev_path(), + source, + })?; + let test_fb = Some(self.create_test_buffer(&pending.mode)?); let req = self.build_request( &mut pending.connectors.iter(), &mut [].iter(), &self.planes, - None, - mode, - new_blob, + test_fb, + Some(mode), + Some(new_blob), )?; if let Err(err) = self .atomic_commit( @@ -292,7 +431,7 @@ impl Surface for AtomicDrmSurfaceInternal { .compat() .map_err(|_| Error::TestFailed(self.crtc)) { - let _ = self.dev.destroy_property_blob(new_blob.unwrap().into()); + let _ = self.dev.destroy_property_blob(new_blob.into()); return Err(err); } @@ -310,6 +449,10 @@ impl RawSurface for AtomicDrmSurfaceInternal { } fn commit(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut current = self.state.write().unwrap(); let mut pending = self.pending.write().unwrap(); @@ -340,11 +483,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { } if current.mode != pending.mode { - info!( - self.logger, - "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() - ); + info!(self.logger, "Setting new mode: {:?}", pending.mode.name()); } trace!(self.logger, "Testing screen config"); @@ -355,8 +494,8 @@ impl RawSurface for AtomicDrmSurfaceInternal { &mut removed, &self.planes, Some(framebuffer), - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; if let Err(err) = self @@ -380,15 +519,13 @@ impl RawSurface for AtomicDrmSurfaceInternal { &mut [].iter(), &self.planes, Some(framebuffer), - current.mode, - current.blob, + Some(current.mode), + Some(current.blob), )? } else { if current.mode != pending.mode { - if let Some(blob) = current.blob { - if let Err(err) = self.dev.destroy_property_blob(blob.into()) { - warn!(self.logger, "Failed to destory old mode property blob: {}", err); - } + 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(); @@ -398,7 +535,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { } }; - debug!(self.logger, "Setting screen: {:#?}", req); + debug!(self.logger, "Setting screen: {:?}", req); self.atomic_commit( &[ AtomicCommitFlags::PageFlipEvent, @@ -417,23 +554,31 @@ impl RawSurface for AtomicDrmSurfaceInternal { Ok(()) } - fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> { - let req = self - .build_request( - &mut [].iter(), - &mut [].iter(), - &self.planes, - Some(framebuffer), - None, - None, - ) //current.mode) - .map_err(|_| SwapBuffersError::ContextLost)?; + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + + let req = self.build_request( + &mut [].iter(), + &mut [].iter(), + &self.planes, + Some(framebuffer), + None, + None, + )?; + trace!(self.logger, "Queueing page flip: {:#?}", req); self.atomic_commit( &[AtomicCommitFlags::PageFlipEvent, AtomicCommitFlags::Nonblock], req, ) - .map_err(|_| SwapBuffersError::ContextLost)?; + .compat() + .map_err(|source| Error::Access { + errmsg: "Page flip commit failed", + dev: self.dev_path(), + source, + })?; Ok(()) } @@ -444,6 +589,10 @@ impl CursorBackend for AtomicDrmSurfaceInternal { type Error = Error; fn set_cursor_position(&self, x: u32, y: u32) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + trace!(self.logger, "New cursor position ({},{}) pending", x, y); self.cursor.position.set(Some((x, y))); Ok(()) @@ -454,6 +603,10 @@ impl CursorBackend for AtomicDrmSurfaceInternal { buffer: &Self::CursorFormat, hotspot: (u32, u32), ) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + trace!(self.logger, "Setting the new imported cursor"); if let Some(fb) = self.cursor.framebuffer.get().take() { @@ -834,11 +987,11 @@ impl Surface for AtomicDrmSurface { self.0.pending_connectors() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } @@ -854,7 +1007,7 @@ impl Surface for AtomicDrmSurface { self.0.set_connectors(connectors) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { self.0.use_mode(mode) } } @@ -868,7 +1021,7 @@ impl RawSurface for AtomicDrmSurface { self.0.commit(framebuffer) } - fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> { + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { RawSurface::page_flip(&*self.0, framebuffer) } } diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index 69679a9..e5d666e 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -22,6 +22,7 @@ use drm::{ #[cfg(feature = "renderer_gl")] use nix::libc::c_void; use nix::libc::dev_t; +use std::env; use std::os::unix::io::{AsRawFd, RawFd}; #[cfg(feature = "use_system_lib")] use wayland_server::Display; @@ -150,19 +151,53 @@ pub enum FallbackSurface { impl FallbackDevice, LegacyDrmDevice> { /// Try to initialize an [`AtomicDrmDevice`](::backend::drm:;atomic::AtomicDrmDevice) /// and fall back to a [`LegacyDrmDevice`] if atomic-modesetting is not supported. - pub fn new(fd: A, logger: L) -> Result + /// + /// # Arguments + /// + /// - `fd` - Open drm node (needs to be clonable to be passed to multiple initializers) + /// - `disable_connectors` - Setting this to true will initialize all connectors \ + /// as disabled on device creation. smithay enables connectors, when attached \ + /// to a surface, and disables them, when detached. Setting this to `false` \ + /// requires usage of `drm-rs` to disable unused connectors to prevent them \ + /// showing garbage, but will also prevent flickering of already turned on \ + /// connectors (assuming you won't change the resolution). + /// - `logger` - Optional [`slog::Logger`] to be used by the resulting device. + /// + /// # Return + /// + /// Returns an error, if both devices fail to initialize due to `fd` being no valid + /// drm node or the device being not accessible. + pub fn new(fd: A, disable_connectors: bool, logger: L) -> Result where L: Into>, { let log = crate::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_drm_fallback")); info!(log, "Trying to initialize AtomicDrmDevice"); - match AtomicDrmDevice::new(fd.clone(), log.clone()) { + let force_legacy = env::var("SMITHAY_USE_LEGACY") + .map(|x| { + x == "1" || x.to_lowercase() == "true" || x.to_lowercase() == "yes" || x.to_lowercase() == "y" + }) + .unwrap_or(false); + if force_legacy { + info!(log, "SMITHAY_USE_LEGACY is set. Forcing LegacyDrmDevice."); + return Ok(FallbackDevice::Fallback(LegacyDrmDevice::new( + fd, + disable_connectors, + log, + )?)); + } + + 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); - info!(log, "Falling back to fallback LegacyyDrmDevice"); - Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(fd, log)?)) + info!(log, "Falling back to fallback LegacyDrmDevice"); + Ok(FallbackDevice::Fallback(LegacyDrmDevice::new( + fd, + disable_connectors, + log, + )?)) } } } @@ -228,10 +263,19 @@ where } } fallback_device_impl!(clear_handler, &mut Self); - fn create_surface(&mut self, crtc: crtc::Handle) -> Result { + fn create_surface( + &mut self, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + ) -> Result { match self { - FallbackDevice::Preference(dev) => Ok(FallbackSurface::Preference(dev.create_surface(crtc)?)), - FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback(dev.create_surface(crtc)?)), + FallbackDevice::Preference(dev) => Ok(FallbackSurface::Preference( + dev.create_surface(crtc, mode, connectors)?, + )), + FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback( + dev.create_surface(crtc, mode, connectors)?, + )), } } fallback_device_impl!(process_events, &mut Self); @@ -281,9 +325,9 @@ where fallback_surface_impl!(add_connector, &Self, Result<(), E>, conn: connector::Handle); fallback_surface_impl!(remove_connector, &Self, Result<(), E>, conn: connector::Handle); fallback_surface_impl!(set_connectors, &Self, Result<(), E>, conns: &[connector::Handle]); - fallback_surface_impl!(current_mode, &Self, Option); - fallback_surface_impl!(pending_mode, &Self, Option); - fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Option); + fallback_surface_impl!(current_mode, &Self, Mode); + fallback_surface_impl!(pending_mode, &Self, Mode); + fallback_surface_impl!(use_mode, &Self, Result<(), E>, mode: Mode); } impl RawSurface for FallbackSurface @@ -295,7 +339,7 @@ where { fallback_surface_impl!(commit_pending, &Self, bool); fallback_surface_impl!(commit, &Self, Result<(), E>, fb: framebuffer::Handle); - fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> { + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), E> { match self { FallbackSurface::Preference(dev) => RawSurface::page_flip(dev, framebuffer), FallbackSurface::Fallback(dev) => RawSurface::page_flip(dev, framebuffer), diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 83ef7c2..c851e40 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -3,8 +3,8 @@ //! and [`Surface`](::backend::drm::Surface) implementations of the `backend::drm` module. //! +use crate::backend::graphics::SwapBuffersError; use drm::control::{connector, crtc, Mode, RawResourceHandle}; - use std::path::PathBuf; pub mod fallback; @@ -19,7 +19,7 @@ pub enum Error { #[error("Failed to aquire DRM master")] DrmMasterFailed, /// The `DrmDevice` encountered an access error - #[error("DRM access error: {errmsg} on device `{dev:?}`")] + #[error("DRM access error: {errmsg} on device `{dev:?}` ({source:})")] Access { /// Error message associated to the access error errmsg: &'static str, @@ -40,6 +40,9 @@ pub enum Error { /// The given crtc is already in use by another backend #[error("Crtc `{0:?}` is already in use by another backend")] CrtcAlreadyInUse(crtc::Handle), + /// This operation would result in a surface without connectors. + #[error("Surface of crtc `{0:?}` would have no connectors, which is not accepted")] + SurfaceWithoutConnectors(crtc::Handle), /// No encoder was found for a given connector on the set crtc #[error("No encoder found for the given connector '{connector:?}' on crtc `{crtc:?}`")] NoSuitableEncoder { @@ -68,3 +71,25 @@ pub enum Error { #[error("Atomic Test failed for new properties on crtc ({0:?})")] TestFailed(crtc::Handle), } + +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, + } => + { + SwapBuffersError::TemporaryFailure(Box::new(source)) + } + x => SwapBuffersError::ContextLost(Box::new(x)), + } + } +} diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 3779cf5..61614a1 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -8,7 +8,7 @@ //! Take a look at `anvil`s source code for an example of this. //! -use drm::control::{connector, crtc, encoder, framebuffer, plane, ResourceHandles}; +use drm::control::{connector, crtc, encoder, framebuffer, plane, Mode, ResourceHandles}; use drm::SystemError as DrmError; use nix::libc::dev_t; use std::os::unix::io::{AsRawFd, RawFd}; @@ -17,9 +17,9 @@ use wayland_server::Display; use super::{Device, DeviceHandler, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; -use crate::backend::egl::Error as EGLError; #[cfg(feature = "use_system_lib")] use crate::backend::egl::{display::EGLBufferReader, EGLGraphicsBackend}; +use crate::backend::egl::{EGLError as RawEGLError, Error as EGLError, SurfaceCreationError}; mod surface; pub use self::surface::*; @@ -33,18 +33,25 @@ pub mod session; #[derive(thiserror::Error, Debug)] pub enum Error { /// EGL Error - #[error("EGL error: {0:?}")] + #[error("EGL error: {0:}")] EGL(#[source] EGLError), + /// EGL Error + #[error("EGL error: {0:}")] + RawEGL(#[source] RawEGLError), /// Underlying backend error #[error("Underlying backend error: {0:?}")] Underlying(#[source] U), } +type Arguments = (crtc::Handle, Mode, Vec); + /// Representation of an egl device to create egl rendering surfaces pub struct EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -56,7 +63,9 @@ where impl AsRawFd for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { fn as_raw_fd(&self) -> RawFd { @@ -67,7 +76,9 @@ where impl EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { /// Try to create a new [`EglDevice`] from an open device. @@ -122,7 +133,9 @@ where struct InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { handler: Box> + 'static>, @@ -131,7 +144,9 @@ where impl DeviceHandler for InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { type Device = D; @@ -147,7 +162,9 @@ where impl Device for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { type Surface = EglSurface<::Surface>; @@ -169,6 +186,8 @@ where fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result::Error> { info!(self.logger, "Initializing EglSurface"); @@ -182,9 +201,12 @@ where context.get_pixel_format(), self.default_requirements.double_buffer, context.get_config_id(), - crtc, + (crtc, mode, Vec::from(connectors)), ) - .map_err(Error::EGL)?; + .map_err(|err| match err { + SurfaceCreationError::EGLSurfaceCreationFailed(err) => Error::RawEGL(err), + SurfaceCreationError::NativeSurfaceCreationFailed(err) => Error::Underlying(err), + })?; Ok(EglSurface { context, surface }) } @@ -221,7 +243,9 @@ where impl EGLGraphicsBackend for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { fn bind_wl_display(&self, display: &Display) -> Result { @@ -232,7 +256,9 @@ where impl Drop for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { fn drop(&mut self) { diff --git a/src/backend/drm/egl/session.rs b/src/backend/drm/egl/session.rs index ae404d0..ff12afe 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -3,11 +3,11 @@ //! to an open [`Session`](::backend::session::Session). //! -use drm::control::crtc; +use drm::control::{connector, crtc, Mode}; use std::os::unix::io::RawFd; use super::EglDevice; -use crate::backend::drm::Device; +use crate::backend::drm::{Device, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; use crate::backend::session::{AsSessionObserver, SessionObserver}; @@ -22,7 +22,13 @@ impl AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay + AsSessionObserver + 'static, + D: Device + + NativeDisplay< + B, + Arguments = (crtc::Handle, Mode, Vec), + Error = <::Surface as Surface>::Error, + > + AsSessionObserver + + 'static, ::Surface: NativeSurface, { fn observer(&mut self) -> EglDeviceObserver { diff --git a/src/backend/drm/egl/surface.rs b/src/backend/drm/egl/surface.rs index e9107a0..7bb01c0 100644 --- a/src/backend/drm/egl/surface.rs +++ b/src/backend/drm/egl/surface.rs @@ -1,5 +1,6 @@ use drm::control::{connector, crtc, Mode}; use nix::libc::c_void; +use std::convert::TryInto; use super::Error; use crate::backend::drm::Surface; @@ -22,7 +23,7 @@ where impl Surface for EglSurface where - N: NativeSurface + Surface, + N: native::NativeSurface + Surface, { type Connectors = ::Connectors; type Error = Error<::Error>; @@ -53,15 +54,15 @@ where self.surface.set_connectors(connectors).map_err(Error::Underlying) } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.surface.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.surface.pending_mode() } - fn use_mode(&self, mode: Option) -> Result<(), Self::Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> { self.surface.use_mode(mode).map_err(Error::Underlying) } } @@ -90,9 +91,17 @@ where impl GLGraphicsBackend for EglSurface where N: native::NativeSurface + Surface, + ::Error: Into + 'static, { fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.surface.swap_buffers() + if let Err(err) = self.surface.swap_buffers() { + Err(match err.try_into() { + Ok(x) => x, + Err(x) => x.into(), + }) + } else { + Ok(()) + } } fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -100,7 +109,7 @@ where } fn get_framebuffer_dimensions(&self) -> (u32, u32) { - let (w, h) = self.pending_mode().map(|mode| mode.size()).unwrap_or((1, 1)); + let (w, h) = self.pending_mode().size(); (w as u32, h as u32) } @@ -109,7 +118,9 @@ where } unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.context.make_current_with_surface(&self.surface) + self.context + .make_current_with_surface(&self.surface) + .map_err(Into::into) } fn get_pixel_format(&self) -> PixelFormat { diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 201d653..556230c 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -7,12 +7,11 @@ use crate::backend::drm::{Device, RawDevice, Surface}; use crate::backend::egl::ffi; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; -use crate::backend::egl::Error as EglError; -use crate::backend::graphics::SwapBuffersError; +use crate::backend::egl::{wrap_egl_call, EGLError, Error as EglBackendError}; use super::{Error, GbmDevice, GbmSurface}; -use drm::control::{crtc, Device as ControlDevice}; +use drm::control::{connector, crtc, Device as ControlDevice, Mode}; use gbm::AsRaw; use std::marker::PhantomData; use std::ptr; @@ -31,44 +30,52 @@ impl Backend for Gbm { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_gbm") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_gbm"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_KHR, display as *mut _, ptr::null()) + }) } else if has_dp_extension("EGL_MESA_platform_gbm") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_MESA_platform_gbm"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + }) } else if has_dp_extension("EGL_MESA_platform_gbm") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_MESA_platform_gbm"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_GBM_MESA, display as *mut _, ptr::null()) + }) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } unsafe impl NativeDisplay> for GbmDevice { - type Arguments = crtc::Handle; + type Arguments = (crtc::Handle, Mode, Vec); type Error = Error<<::Surface as Surface>::Error>; fn is_backend(&self) -> bool { true } - fn ptr(&self) -> Result { + fn ptr(&self) -> Result { Ok(self.dev.borrow().as_raw() as *const _) } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Self::Error> { - Device::create_surface(self, crtc) + fn create_surface(&mut self, args: Self::Arguments) -> Result, Self::Error> { + Device::create_surface(self, args.0, args.1, &args.2) } } unsafe impl NativeSurface for GbmSurface { + type Error = Error<<::Surface as Surface>::Error>; + fn ptr(&self) -> ffi::NativeWindowType { self.0.surface.borrow().as_raw() as *const _ } @@ -77,16 +84,11 @@ unsafe impl NativeSurface for GbmSurface { self.needs_recreation() } - fn recreate(&self) -> bool { - if let Err(err) = GbmSurface::recreate(self) { - error!(self.0.logger, "Failure recreating internal resources: {}", err); - false - } else { - true - } + fn recreate(&self) -> Result<(), Self::Error> { + GbmSurface::recreate(self) } - fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + fn swap_buffers(&self) -> Result<(), Self::Error> { // this is safe since `eglSwapBuffers` will have been called exactly once // if this is used by our egl module, which is why this trait is unsafe. unsafe { self.page_flip() } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a292792..d4cc66a 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -10,8 +10,9 @@ //! use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface}; +use crate::backend::graphics::SwapBuffersError; -use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice}; +use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Mode}; use drm::SystemError as DrmError; use gbm::{self, BufferObjectFlags, Format as GbmFormat}; use nix::libc::dev_t; @@ -26,25 +27,31 @@ use std::sync::Once; /// Errors thrown by the [`GbmDevice`](::backend::drm::gbm::GbmDevice) /// and [`GbmSurface`](::backend::drm::gbm::GbmSurface). #[derive(thiserror::Error, Debug)] -pub enum Error { +pub enum Error { /// Creation of GBM device failed #[error("Creation of GBM device failed")] InitFailed(#[source] io::Error), /// Creation of GBM surface failed #[error("Creation of GBM surface failed")] SurfaceCreationFailed(#[source] io::Error), - /// No mode is set, blocking the current operation - #[error("No mode is currently set")] - NoModeSet, /// Creation of GBM buffer object failed #[error("Creation of GBM buffer object failed")] BufferCreationFailed(#[source] io::Error), /// Writing to GBM buffer failed #[error("Writing to GBM buffer failed")] BufferWriteFailed(#[source] io::Error), + /// Creation of drm framebuffer failed + #[error("Creation of drm framebuffer failed")] + FramebufferCreationFailed(#[source] failure::Compat), /// Lock of GBM surface front buffer failed #[error("Lock of GBM surface font buffer failed")] FrontBufferLockFailed, + /// No additional buffers are available + #[error("No additional buffers are available. Did you swap twice?")] + FrontBuffersExhausted, + /// Internal state was modified + #[error("Internal state was modified. Did you change gbm userdata?")] + InvalidInternalState, /// The GBM device was destroyed #[error("The GBM device was destroyed")] DeviceDestroyed, @@ -159,17 +166,16 @@ impl Device for GbmDevice { fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result, Error<<::Surface as Surface>::Error>> { info!(self.logger, "Initializing GbmSurface"); - let drm_surface = - Device::create_surface(&mut **self.dev.borrow_mut(), crtc).map_err(Error::Underlying)?; + let drm_surface = Device::create_surface(&mut **self.dev.borrow_mut(), crtc, mode, connectors) + .map_err(Error::Underlying)?; // initialize the surface - let (w, h) = drm_surface - .pending_mode() - .map(|mode| mode.size()) - .unwrap_or((1, 1)); + let (w, h) = drm_surface.pending_mode().size(); let surface = self .dev .borrow() @@ -249,3 +255,29 @@ impl Drop for GbmDevice { self.clear_handler(); } } + +impl Into for Error +where + E: std::error::Error + Into + 'static, +{ + fn into(self) -> SwapBuffersError { + match self { + Error::FrontBuffersExhausted => SwapBuffersError::AlreadySwapped, + Error::FramebufferCreationFailed(x) + if match x.get_ref() { + drm::SystemError::Unknown { + errno: nix::errno::Errno::EBUSY, + } => true, + drm::SystemError::Unknown { + errno: nix::errno::Errno::EINTR, + } => true, + _ => false, + } => + { + SwapBuffersError::TemporaryFailure(Box::new(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 fbb93af..0fb6a28 100644 --- a/src/backend/drm/gbm/session.rs +++ b/src/backend/drm/gbm/session.rs @@ -12,6 +12,7 @@ use std::rc::{Rc, Weak}; use super::{GbmDevice, GbmSurfaceInternal}; use crate::backend::drm::{RawDevice, RawSurface}; +use crate::backend::graphics::CursorBackend; use crate::backend::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -27,11 +28,12 @@ pub struct GbmDeviceObserver< } impl< - S: SessionObserver + 'static, - D: RawDevice + ::drm::control::Device + AsSessionObserver + 'static, - > AsSessionObserver> for GbmDevice + O: SessionObserver + 'static, + S: CursorBackend + RawSurface + 'static, + D: RawDevice + drm::control::Device + AsSessionObserver + 'static, + > AsSessionObserver> for GbmDevice { - fn observer(&mut self) -> GbmDeviceObserver { + fn observer(&mut self) -> GbmDeviceObserver { GbmDeviceObserver { observer: (**self.dev.borrow_mut()).observer(), backends: Rc::downgrade(&self.backends), @@ -41,9 +43,10 @@ impl< } impl< - S: SessionObserver + 'static, - D: RawDevice + ::drm::control::Device + AsSessionObserver + 'static, - > SessionObserver for GbmDeviceObserver + O: SessionObserver + 'static, + S: CursorBackend + RawSurface + 'static, + D: RawDevice + drm::control::Device + AsSessionObserver + 'static, + > SessionObserver for GbmDeviceObserver { fn pause(&mut self, devnum: Option<(u32, u32)>) { self.observer.pause(devnum); @@ -56,20 +59,19 @@ impl< for (crtc, backend) in backends.borrow().iter() { if let Some(backend) = backend.upgrade() { // restart rendering loop, if it was previously running - if let Some(fb) = backend.current_frame_buffer.get() { - if backend.crtc.page_flip(fb).is_err() { - // Try more! - if let Err(err) = backend.recreate() { - error!( - self.logger, - "Failed to re-create gbm surface, is the device gone?\n\t{}", err - ); - } - if let Err(err) = unsafe { backend.page_flip() } { - warn!(self.logger, "Failed to restart rendering loop. Error: {}", err); - // TODO bubble this up the user somehow - // maybe expose a "running" state from a surface? - } + 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 } } @@ -79,12 +81,7 @@ impl< let &(ref cursor, ref hotspot): &(BufferObject<()>, (u32, u32)) = unsafe { &*backend.cursor.as_ptr() }; - if backend - .dev - .borrow() - .set_cursor2(*crtc, Some(cursor), ((*hotspot).0 as i32, (*hotspot).1 as i32)) - .is_err() - { + if backend.crtc.set_cursor_representation(cursor, *hotspot).is_err() { if let Err(err) = backend.dev.borrow().set_cursor(*crtc, Some(cursor)) { error!(self.logger, "Failed to reset cursor. Error: {}", err); } diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 30cbbb2..1668d17 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -2,6 +2,7 @@ use super::super::{Device, RawDevice, RawSurface, Surface}; use super::Error; use drm::control::{connector, crtc, framebuffer, Device as ControlDevice, Mode}; +use failure::ResultExt; use gbm::{self, BufferObject, BufferObjectFlags, Format as GbmFormat, SurfaceBufferHandle}; use image::{ImageBuffer, Rgba}; @@ -9,7 +10,6 @@ use std::cell::{Cell, RefCell}; use std::rc::Rc; use crate::backend::graphics::CursorBackend; -use crate::backend::graphics::SwapBuffersError; pub(super) struct GbmSurfaceInternal { pub(super) dev: Rc>>, @@ -32,7 +32,7 @@ impl GbmSurfaceInternal { // drop and release the old buffer } - pub unsafe fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> { + pub unsafe fn page_flip(&self) -> Result<(), Error<<::Surface as Surface>::Error>> { let res = { let nb = self.next_buffer.take(); let res = nb.is_some(); @@ -42,7 +42,7 @@ impl GbmSurfaceInternal { if res { // We cannot call lock_front_buffer anymore without releasing the previous buffer, which will happen when the page flip is done warn!(self.logger, "Tried to swap with an already queued flip"); - return Err(SwapBuffersError::AlreadySwapped); + return Err(Error::FrontBuffersExhausted); } // supporting only one buffer would cause a lot of inconvinience and @@ -53,13 +53,13 @@ impl GbmSurfaceInternal { .surface .borrow() .lock_front_buffer() - .expect("Surface only has one front buffer. Not supported by smithay"); + .map_err(|_| Error::FrontBufferLockFailed)?; // create a framebuffer if the front buffer does not have one already // (they are reused by gbm) let maybe_fb = next_bo .userdata() - .map_err(|_| SwapBuffersError::ContextLost)? + .map_err(|_| Error::InvalidInternalState)? .cloned(); let fb = if let Some(info) = maybe_fb { info @@ -67,7 +67,8 @@ impl GbmSurfaceInternal { let fb = self .crtc .add_planar_framebuffer(&*next_bo, &[0; 4], 0) - .map_err(|_| SwapBuffersError::ContextLost)?; + .compat() + .map_err(Error::FramebufferCreationFailed)?; next_bo.set_userdata(fb).unwrap(); fb }; @@ -75,14 +76,11 @@ impl GbmSurfaceInternal { if self.recreated.get() { debug!(self.logger, "Commiting new state"); - if let Err(err) = self.crtc.commit(fb) { - error!(self.logger, "Error commiting crtc: {}", err); - return Err(SwapBuffersError::ContextLost); - } + 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)?; + RawSurface::page_flip(&self.crtc, fb).map_err(Error::Underlying)?; } self.current_frame_buffer.set(Some(fb)); @@ -91,11 +89,7 @@ impl GbmSurfaceInternal { } pub fn recreate(&self) -> Result<(), Error<<::Surface as Surface>::Error>> { - let (w, h) = self - .pending_mode() - .or_else(|| self.current_mode()) - .ok_or(Error::NoModeSet)? - .size(); + let (w, h) = self.pending_mode().size(); // Recreate the surface and the related resources to match the new // resolution. @@ -167,15 +161,15 @@ impl Surface for GbmSurfaceInternal { self.crtc.set_connectors(connectors).map_err(Error::Underlying) } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.crtc.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.crtc.pending_mode() } - fn use_mode(&self, mode: Option) -> Result<(), Self::Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> { self.crtc.use_mode(mode).map_err(Error::Underlying) } } @@ -277,7 +271,7 @@ impl GbmSurface { /// /// When used in conjunction with an EGL context, this must be called exactly once /// after page-flipping the associated context. - pub unsafe fn page_flip(&self) -> ::std::result::Result<(), SwapBuffersError> { + pub unsafe fn page_flip(&self) -> ::std::result::Result<(), ::Error> { self.0.page_flip() } @@ -325,15 +319,15 @@ impl Surface for GbmSurface { self.0.set_connectors(connectors) } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } - fn use_mode(&self, mode: Option) -> Result<(), Self::Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error> { self.0.use_mode(mode) } } diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 19597f3..000b7c6 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -11,24 +11,24 @@ use super::{common::Error, DevPath, Device, DeviceHandler, RawDevice}; use drm::control::{ - connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Event, ResourceHandles, + connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Event, Mode, ResourceHandles, }; use drm::{Device as BasicDevice, SystemError as DrmError}; use nix::libc::dev_t; use nix::sys::stat::fstat; use std::cell::RefCell; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, RwLock}; +use std::sync::Arc; use failure::{Fail, ResultExt}; mod surface; pub use self::surface::LegacyDrmSurface; -use self::surface::{LegacyDrmSurfaceInternal, State}; +use self::surface::LegacyDrmSurfaceInternal; #[cfg(feature = "backend_session")] pub mod session; @@ -88,9 +88,21 @@ impl Drop for Dev { impl LegacyDrmDevice { /// Create a new [`LegacyDrmDevice`] from an open drm node /// - /// Returns an error if the file is no valid drm node or context creation was not - /// successful. - pub fn new(dev: A, logger: L) -> Result + /// # Arguments + /// + /// - `fd` - Open drm node + /// - `disable_connectors` - Setting this to true will initialize all connectors \ + /// as disabled on device creation. smithay enables connectors, when attached \ + /// to a surface, and disables them, when detached. Setting this to `false` \ + /// requires usage of `drm-rs` to disable unused connectors to prevent them \ + /// showing garbage, but will also prevent flickering of already turned on \ + /// connectors (assuming you won't change the resolution). + /// - `logger` - Optional [`slog::Logger`] to be used by this device. + /// + /// # Return + /// + /// Returns an error if the file is no valid drm node or the device is not accessible. + pub fn new(dev: A, disable_connectors: bool, logger: L) -> Result where L: Into>, { @@ -151,6 +163,21 @@ impl LegacyDrmDevice { } } + if disable_connectors { + dev.set_connector_state(res_handles.connectors().iter().copied(), false)?; + + for crtc in res_handles.crtcs() { + // null commit + dev.set_crtc(*crtc, None, (0, 0), &[], None) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error setting crtc", + dev: dev.dev_path(), + source, + })?; + } + } + Ok(LegacyDrmDevice { dev: Rc::new(dev), dev_id, @@ -162,6 +189,64 @@ impl LegacyDrmDevice { } } +impl Dev { + pub(in crate::backend::drm::legacy) fn set_connector_state( + &self, + connectors: impl Iterator, + enabled: bool, + ) -> Result<(), Error> { + for conn in connectors { + let info = self + .get_connector(conn) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to get connector infos", + dev: self.dev_path(), + source, + })?; + if info.state() == connector::State::Connected { + let props = self + .get_properties(conn) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to get properties for connector", + dev: self.dev_path(), + source, + })?; + let (handles, _) = props.as_props_and_values(); + for handle in handles { + let info = self + .get_property(*handle) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to get property of connector", + dev: self.dev_path(), + source, + })?; + if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { + self.set_property( + conn, + *handle, + if enabled { + 0 /*DRM_MODE_DPMS_ON*/ + } else { + 3 /*DRM_MODE_DPMS_OFF*/ + }, + ) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to set property of connector", + dev: self.dev_path(), + source, + })?; + } + } + } + } + Ok(()) + } +} + impl AsRawFd for LegacyDrmDevice { fn as_raw_fd(&self) -> RawFd { self.dev.as_raw_fd() @@ -186,7 +271,12 @@ impl Device for LegacyDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle) -> Result, Error> { + fn create_surface( + &mut self, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + ) -> Result, Error> { if self.backends.borrow().contains_key(&crtc) { return Err(Error::CrtcAlreadyInUse(crtc)); } @@ -195,52 +285,17 @@ impl Device for LegacyDrmDevice { return Err(Error::DeviceInactive); } - // Try to enumarate the current state to set the initial state variable correctly - - let crtc_info = self.get_crtc(crtc).compat().map_err(|source| Error::Access { - errmsg: "Error loading crtc info", - dev: self.dev_path(), - source, - })?; - - let mode = crtc_info.mode(); - - let mut connectors = HashSet::new(); - let res_handles = ControlDevice::resource_handles(self) - .compat() - .map_err(|source| Error::Access { - errmsg: "Error loading drm resources", - dev: self.dev_path(), - source, - })?; - for &con in res_handles.connectors() { - let con_info = self.get_connector(con).compat().map_err(|source| Error::Access { - errmsg: "Error loading connector info", - dev: self.dev_path(), - source, - })?; - if let Some(enc) = con_info.current_encoder() { - let enc_info = self.get_encoder(enc).compat().map_err(|source| Error::Access { - errmsg: "Error loading encoder info", - dev: self.dev_path(), - source, - })?; - if let Some(current_crtc) = enc_info.crtc() { - if crtc == current_crtc { - connectors.insert(con); - } - } - } + if connectors.is_empty() { + return Err(Error::SurfaceWithoutConnectors(crtc)); } - let state = State { mode, connectors }; - let backend = Rc::new(LegacyDrmSurfaceInternal { - dev: self.dev.clone(), + let backend = Rc::new(LegacyDrmSurfaceInternal::new( + self.dev.clone(), crtc, - state: RwLock::new(state.clone()), - pending: RwLock::new(state), - logger: self.logger.new(o!("crtc" => format!("{:?}", crtc))), - }); + mode, + connectors, + self.logger.new(o!("crtc" => format!("{:?}", crtc))), + )?); self.backends.borrow_mut().insert(crtc, Rc::downgrade(&backend)); Ok(LegacyDrmSurface(backend)) diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index eb5c86e..a211bce 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -5,16 +5,17 @@ use drm::control::{crtc, Device as ControlDevice}; use drm::Device as BasicDevice; +use failure::ResultExt; use nix::libc::dev_t; use nix::sys::stat; use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use super::{Dev, LegacyDrmDevice, LegacyDrmSurfaceInternal}; +use super::{Dev, DevPath, Error, LegacyDrmDevice, LegacyDrmSurfaceInternal}; use crate::backend::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -91,5 +92,83 @@ impl SessionObserver for LegacyDrmDeviceObserver { } } } + // okay, the previous session/whatever might left the drm devices in any state... + // lets fix that + if let Err(err) = self.reset_state() { + warn!(self.logger, "Unable to reset state after tty switch: {}", err); + // TODO call drm-handler::error + } + } +} + +impl LegacyDrmDeviceObserver { + fn reset_state(&mut self) -> Result<(), Error> { + // lets enumerate it the current state + if let Some(dev) = self.dev.upgrade() { + let res_handles = ControlDevice::resource_handles(&*dev) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error loading drm resources", + dev: dev.dev_path(), + source, + })?; + + let mut used_connectors = HashSet::new(); + if let Some(backends) = self.backends.upgrade() { + for surface in backends.borrow().values().filter_map(Weak::upgrade) { + let mut current = surface.state.write().unwrap(); + let pending = surface.pending.read().unwrap(); + + // store (soon to be) used connectors + used_connectors.extend(pending.connectors.clone()); + + // set current connectors + current.connectors.clear(); + for conn in res_handles.connectors() { + let conn_info = + dev.get_connector(*conn) + .compat() + .map_err(|source| Error::Access { + errmsg: "Could not load connector info", + dev: dev.dev_path(), + source, + })?; + if let Some(enc) = conn_info.current_encoder() { + let enc_info = dev.get_encoder(enc).compat().map_err(|source| Error::Access { + errmsg: "Could not load encoder info", + dev: dev.dev_path(), + source, + })?; + if enc_info.crtc().map(|crtc| crtc == surface.crtc).unwrap_or(false) { + current.connectors.insert(*conn); + } + } + } + + // set current mode + let crtc_info = dev + .get_crtc(surface.crtc) + .compat() + .map_err(|source| Error::Access { + errmsg: "Could not load crtc info", + dev: dev.dev_path(), + source, + })?; + + // If we have no current mode, we create a fake one, which will not match (and thus gets overriden on the commit below). + // A better fix would probably be making mode an `Option`, but that would mean + // we need to be sure, we require a mode to always be set without relying on the compiler. + // So we cheat, because it works and is easier to handle later. + current.mode = crtc_info.mode().unwrap_or_else(|| unsafe { std::mem::zeroed() }); + } + } + + // Disable unused connectors + let all_set = res_handles.connectors().iter().copied().collect::>(); + let unused = used_connectors.difference(&all_set); + dev.set_connector_state(unused.copied(), false)?; + } + + Ok(()) } } diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 6bdfa1a..de931da 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -8,11 +8,10 @@ use drm::Device as BasicDevice; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; -use std::sync::RwLock; +use std::sync::{atomic::Ordering, RwLock}; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; -use crate::backend::graphics::SwapBuffersError; use super::Dev; @@ -20,7 +19,7 @@ use failure::{Fail, ResultExt}; #[derive(Debug, PartialEq, Eq, Clone)] pub struct State { - pub mode: Option, + pub mode: Mode, pub connectors: HashSet, } @@ -46,6 +45,10 @@ impl CursorBackend for LegacyDrmSurfaceInternal { type Error = Error; fn set_cursor_position(&self, x: u32, y: u32) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + trace!(self.logger, "Move the cursor to {},{}", x, y); self.move_cursor(self.crtc, (x as i32, y as i32)) .compat() @@ -61,6 +64,10 @@ impl CursorBackend for LegacyDrmSurfaceInternal { buffer: &Self::CursorFormat, hotspot: (u32, u32), ) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + trace!(self.logger, "Setting the new imported cursor"); if self @@ -96,18 +103,22 @@ impl Surface for LegacyDrmSurfaceInternal { self.pending.read().unwrap().connectors.clone() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.state.read().unwrap().mode } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.pending.read().unwrap().mode } fn add_connector(&self, conn: connector::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); - if self.check_connector(conn, pending.mode.as_ref().unwrap())? { + if self.check_connector(conn, &pending.mode)? { pending.connectors.insert(conn); } @@ -115,16 +126,30 @@ impl Surface for LegacyDrmSurfaceInternal { } fn remove_connector(&self, connector: connector::Handle) -> Result<(), Error> { - self.pending.write().unwrap().connectors.remove(&connector); + let mut pending = self.pending.write().unwrap(); + + if pending.connectors.contains(&connector) && pending.connectors.len() == 1 { + return Err(Error::SurfaceWithoutConnectors(self.crtc)); + } + + pending.connectors.remove(&connector); Ok(()) } fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> { + if connectors.is_empty() { + return Err(Error::SurfaceWithoutConnectors(self.crtc)); + } + + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); if connectors .iter() - .map(|conn| self.check_connector(*conn, pending.mode.as_ref().unwrap())) + .map(|conn| self.check_connector(*conn, &pending.mode)) .collect::, _>>()? .iter() .all(|v| *v) @@ -135,25 +160,27 @@ impl Surface for LegacyDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); // check the connectors to see if this mode is supported - if let Some(mode) = mode { - for connector in &pending.connectors { - if !self - .get_connector(*connector) - .compat() - .map_err(|source| Error::Access { - errmsg: "Error loading connector info", - dev: self.dev_path(), - source, - })? - .modes() - .contains(&mode) - { - return Err(Error::ModeNotSuitable(mode)); - } + for connector in &pending.connectors { + if !self + .get_connector(*connector) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error loading connector info", + dev: self.dev_path(), + source, + })? + .modes() + .contains(&mode) + { + return Err(Error::ModeNotSuitable(mode)); } } @@ -169,6 +196,10 @@ impl RawSurface for LegacyDrmSurfaceInternal { } fn commit(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut current = self.state.write().unwrap(); let pending = self.pending.read().unwrap(); @@ -177,7 +208,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { let added = pending.connectors.difference(¤t.connectors); let mut conn_removed = false; - for conn in removed { + for conn in removed.clone() { if let Ok(info) = self.get_connector(*conn) { info!(self.logger, "Removing connector: {:?}", info.interface()); } else { @@ -187,6 +218,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { // the graphics pipeline will not be freed otherwise conn_removed = true; } + self.dev.set_connector_state(removed.copied(), false)?; if conn_removed { // We need to do a null commit to free graphics pipelines @@ -199,20 +231,17 @@ impl RawSurface for LegacyDrmSurfaceInternal { })?; } - for conn in added { + for conn in added.clone() { if let Ok(info) = self.get_connector(*conn) { info!(self.logger, "Adding connector: {:?}", info.interface()); } else { info!(self.logger, "Adding unknown connector"); } } + self.dev.set_connector_state(added.copied(), true)?; if current.mode != pending.mode { - info!( - self.logger, - "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() - ); + info!(self.logger, "Setting new mode: {:?}", pending.mode.name()); } } @@ -226,7 +255,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { .iter() .copied() .collect::>(), - pending.mode, + Some(pending.mode), ) .compat() .map_err(|source| Error::Access { @@ -251,9 +280,13 @@ impl RawSurface for LegacyDrmSurfaceInternal { }) } - fn page_flip(&self, framebuffer: framebuffer::Handle) -> ::std::result::Result<(), SwapBuffersError> { + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { trace!(self.logger, "Queueing Page flip"); + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + ControlDevice::page_flip( self, self.crtc, @@ -261,11 +294,84 @@ impl RawSurface for LegacyDrmSurfaceInternal { &[PageFlipFlags::PageFlipEvent], None, ) - .map_err(|_| SwapBuffersError::ContextLost) + .compat() + .map_err(|source| Error::Access { + errmsg: "Failed to page flip", + dev: self.dev_path(), + source, + }) } } impl LegacyDrmSurfaceInternal { + pub(crate) fn new( + dev: Rc>, + crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], + logger: ::slog::Logger, + ) -> Result, Error> { + // 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", + dev: dev.dev_path(), + source, + })?; + + let current_mode = crtc_info.mode(); + + let mut current_connectors = HashSet::new(); + let res_handles = ControlDevice::resource_handles(&*dev) + .compat() + .map_err(|source| Error::Access { + errmsg: "Error loading drm resources", + dev: dev.dev_path(), + source, + })?; + for &con in res_handles.connectors() { + let con_info = dev.get_connector(con).compat().map_err(|source| Error::Access { + errmsg: "Error loading connector info", + dev: dev.dev_path(), + source, + })?; + if let Some(enc) = con_info.current_encoder() { + let enc_info = dev.get_encoder(enc).compat().map_err(|source| Error::Access { + errmsg: "Error loading encoder info", + dev: dev.dev_path(), + source, + })?; + if let Some(current_crtc) = enc_info.crtc() { + if crtc == current_crtc { + current_connectors.insert(con); + } + } + } + } + + // If we have no current mode, we create a fake one, which will not match (and thus gets overriden on the commit below). + // A better fix would probably be making mode an `Option`, but that would mean + // we need to be sure, we require a mode to always be set without relying on the compiler. + // So we cheat, because it works and is easier to handle later. + let state = State { + mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }), + connectors: current_connectors, + }; + let pending = State { + mode, + connectors: connectors.iter().copied().collect(), + }; + + let surface = LegacyDrmSurfaceInternal { + dev, + crtc, + state: RwLock::new(state), + pending: RwLock::new(pending), + logger, + }; + + Ok(surface) + } + fn check_connector(&self, conn: connector::Handle, mode: &Mode) -> Result { let info = self .get_connector(conn) @@ -319,7 +425,26 @@ impl LegacyDrmSurfaceInternal { impl Drop for LegacyDrmSurfaceInternal { fn drop(&mut self) { // ignore failure at this point + + if !self.dev.active.load(Ordering::SeqCst) { + // the device is gone or we are on another tty + // old state has been restored, we shouldn't touch it. + // if we are on another tty the connectors will get disabled + // by the device, when switching back + return; + } + let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); + // disable connectors again + let current = self.state.read().unwrap(); + if self + .dev + .set_connector_state(current.connectors.iter().copied(), false) + .is_ok() + { + // null commit + let _ = self.set_crtc(self.crtc, None, (0, 0), &[], None); + } } } @@ -368,11 +493,11 @@ impl Surface for LegacyDrmSurface { self.0.pending_connectors() } - fn current_mode(&self) -> Option { + fn current_mode(&self) -> Mode { self.0.current_mode() } - fn pending_mode(&self) -> Option { + fn pending_mode(&self) -> Mode { self.0.pending_mode() } @@ -388,7 +513,7 @@ impl Surface for LegacyDrmSurface { self.0.set_connectors(connectors) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { self.0.use_mode(mode) } } @@ -402,7 +527,7 @@ impl RawSurface for LegacyDrmSurface { self.0.commit(framebuffer) } - fn page_flip(&self, framebuffer: framebuffer::Handle) -> ::std::result::Result<(), SwapBuffersError> { + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { RawSurface::page_flip(&*self.0, framebuffer) } } diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index e7eb844..eacdb9e 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -48,8 +48,6 @@ use std::path::PathBuf; use calloop::{generic::Generic, InsertError, LoopHandle, Source}; -use super::graphics::SwapBuffersError; - #[cfg(feature = "backend_drm_atomic")] pub mod atomic; #[cfg(feature = "backend_drm")] @@ -91,16 +89,23 @@ pub trait Device: AsRawFd + DevPath { /// Creates a new rendering surface. /// + /// # Arguments + /// /// Initialization of surfaces happens through the types provided by /// [`drm-rs`](drm). /// - /// [`crtc`](drm::control::crtc)s represent scanout engines - /// of the device pointer to one framebuffer. - /// Their responsibility is to read the data of the framebuffer and export it into an "Encoder". - /// The number of crtc's represent the number of independant output devices the hardware may handle. + /// - [`crtc`](drm::control::crtc)s represent scanout engines of the device pointing to one framebuffer. \ + /// Their responsibility is to read the data of the framebuffer and export it into an "Encoder". \ + /// The number of crtc's represent the number of independant output devices the hardware may handle. + /// - [`mode`](drm::control::Mode) describes the resolution and rate of images produced by the crtc and \ + /// has to be compatible with the provided `connectors`. + /// - [`connectors`] - List of connectors driven by the crtc. At least one(!) connector needs to be \ + /// attached to a crtc in smithay. fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result::Error>; /// Processes any open events of the underlying file descriptor. @@ -161,6 +166,11 @@ pub trait Surface { /// Tries to add a new [`connector`](drm::control::connector) /// to be used after the next commit. /// + /// **Warning**: You need to make sure, that the connector is not used with another surface + /// or was properly removed via `remove_connector` + `commit` before adding it to another surface. + /// Behavior if failing to do so is undefined, but might result in rendering errors or the connector + /// getting removed from the other surface without updating it's internal state. + /// /// Fails if the `connector` is not compatible with the underlying [`crtc`](drm::control::crtc) /// (e.g. no suitable [`encoder`](drm::control::encoder) may be found) /// or is not compatible with the currently pending @@ -178,11 +188,10 @@ pub trait Surface { fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error>; /// Returns the currently active [`Mode`](drm::control::Mode) /// of the underlying [`crtc`](drm::control::crtc) - /// if any. - fn current_mode(&self) -> Option; + fn current_mode(&self) -> Mode; /// Returns the currently pending [`Mode`](drm::control::Mode) - /// to be used after the next commit, if any. - fn pending_mode(&self) -> Option; + /// to be used after the next commit. + fn pending_mode(&self) -> Mode; /// Tries to set a new [`Mode`](drm::control::Mode) /// to be used after the next commit. /// @@ -193,7 +202,7 @@ pub trait Surface { /// *Note*: Only on a [`RawSurface`] you may directly trigger /// a [`commit`](RawSurface::commit). Other [`Surface`]s provide their /// own methods that *may* trigger a commit, you will need to read their docs. - fn use_mode(&self, mode: Option) -> Result<(), Self::Error>; + fn use_mode(&self, mode: Mode) -> Result<(), Self::Error>; } /// An open bare crtc without any rendering abstractions @@ -224,7 +233,7 @@ pub trait RawSurface: Surface + ControlDevice + BasicDevice { /// This operation is not blocking and will produce a `vblank` event once swapping is done. /// Make sure to [set a `DeviceHandler`](Device::set_handler) and /// [register the belonging `Device`](device_bind) before to receive the event in time. - fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError>; + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), ::Error>; } /// Trait representing open devices that *may* return a `Path` diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs index 938fece..a1ddf6b 100644 --- a/src/backend/egl/context.rs +++ b/src/backend/egl/context.rs @@ -1,10 +1,10 @@ //! EGL context related structs -use super::{ffi, Error}; +use super::{ffi, wrap_egl_call, Error, MakeCurrentError}; use crate::backend::egl::display::{EGLDisplay, EGLDisplayHandle}; use crate::backend::egl::native::NativeSurface; use crate::backend::egl::{native, EGLSurface}; -use crate::backend::graphics::{PixelFormat, SwapBuffersError}; +use crate::backend::graphics::PixelFormat; use std::os::raw::c_int; use std::ptr; use std::sync::Arc; @@ -93,21 +93,15 @@ impl EGLContext { trace!(log, "Creating EGL context..."); // TODO: Support shared contexts - let context = unsafe { + let context = wrap_egl_call(|| unsafe { ffi::egl::CreateContext( **display.display, config_id, ptr::null(), context_attributes.as_ptr(), ) - }; - - if context.is_null() { - match unsafe { ffi::egl::GetError() } as u32 { - ffi::egl::BAD_ATTRIBUTE => return Err(Error::CreationFailed), - err_no => return Err(Error::Unknown(err_no)), - } - } + }) + .map_err(Error::CreationFailed)?; info!(log, "EGL context created"); @@ -126,25 +120,15 @@ impl EGLContext { /// /// This function is marked unsafe, because the context cannot be made current /// on multiple threads. - pub unsafe fn make_current_with_surface( - &self, - surface: &EGLSurface, - ) -> ::std::result::Result<(), SwapBuffersError> + pub unsafe fn make_current_with_surface(&self, surface: &EGLSurface) -> Result<(), MakeCurrentError> where N: NativeSurface, { let surface_ptr = surface.surface.get(); - let ret = ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context); - - if ret == 0 { - match ffi::egl::GetError() as u32 { - ffi::egl::CONTEXT_LOST => Err(SwapBuffersError::ContextLost), - err => panic!("eglMakeCurrent failed (eglGetError returned 0x{:x})", err), - } - } else { - Ok(()) - } + wrap_egl_call(|| ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context)) + .map(|_| ()) + .map_err(Into::into) } /// Makes the OpenGL context the current context in the current thread with no surface bound. @@ -152,18 +136,18 @@ impl EGLContext { /// # Safety /// /// This function is marked unsafe, because the context cannot be made current - /// on multiple threads. - pub unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> { - let ret = ffi::egl::MakeCurrent(**self.display, ptr::null(), ptr::null(), self.context); - - if ret == 0 { - match ffi::egl::GetError() as u32 { - ffi::egl::CONTEXT_LOST => Err(SwapBuffersError::ContextLost), - err => panic!("eglMakeCurrent failed (eglGetError returned 0x{:x})", err), - } - } else { - Ok(()) - } + /// on multiple threads without being unbound again (see `unbind`) + pub unsafe fn make_current(&self) -> Result<(), MakeCurrentError> { + wrap_egl_call(|| { + ffi::egl::MakeCurrent( + **self.display, + ffi::egl::NO_SURFACE, + ffi::egl::NO_SURFACE, + self.context, + ) + }) + .map(|_| ()) + .map_err(Into::into) } /// Returns true if the OpenGL context is the current one in the thread. @@ -180,16 +164,31 @@ impl EGLContext { pub fn get_pixel_format(&self) -> PixelFormat { self.pixel_format } + + /// Unbinds this context from the current thread, if set. + /// + /// This does nothing if this context is not the current context + pub fn unbind(&self) -> Result<(), MakeCurrentError> { + if self.is_current() { + wrap_egl_call(|| unsafe { + ffi::egl::MakeCurrent( + **self.display, + ffi::egl::NO_SURFACE, + ffi::egl::NO_SURFACE, + ffi::egl::NO_CONTEXT, + ) + })?; + } + Ok(()) + } } impl Drop for EGLContext { fn drop(&mut self) { unsafe { // We need to ensure the context is unbound, otherwise it egl stalls the destroy call - if ffi::egl::GetCurrentContext() == self.context as *const _ { - ffi::egl::MakeCurrent(ptr::null(), ptr::null(), ptr::null(), ptr::null()); - } - + // ignore failures at this point + let _ = self.unbind(); ffi::egl::DestroyContext(**self.display, self.context); } } @@ -270,7 +269,7 @@ impl Default for PixelFormatRequirements { impl PixelFormatRequirements { /// Append the requirements to the given attribute list - pub fn create_attributes(&self, out: &mut Vec, logger: &slog::Logger) -> Result<(), Error> { + pub fn create_attributes(&self, out: &mut Vec, logger: &slog::Logger) -> Result<(), ()> { if let Some(hardware_accelerated) = self.hardware_accelerated { out.push(ffi::egl::CONFIG_CAVEAT as c_int); out.push(if hardware_accelerated { @@ -328,7 +327,7 @@ impl PixelFormatRequirements { if self.stereoscopy { error!(logger, "Stereoscopy is currently unsupported (sorry!)"); - return Err(Error::NoAvailablePixelFormat); + return Err(()); } Ok(()) diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index a098a92..baf716b 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -3,7 +3,8 @@ #[cfg(feature = "use_system_lib")] use crate::backend::egl::EGLGraphicsBackend; use crate::backend::egl::{ - ffi, get_proc_address, native, BufferAccessError, EGLContext, EGLImages, EGLSurface, Error, Format, + ffi, get_proc_address, native, wrap_egl_call, BufferAccessError, EGLContext, EGLError, EGLImages, + EGLSurface, Error, Format, SurfaceCreationError, }; use std::sync::Arc; @@ -44,6 +45,7 @@ impl Deref for EGLDisplayHandle { impl Drop for EGLDisplayHandle { fn drop(&mut self) { unsafe { + // ignore errors on drop ffi::egl::Terminate(self.handle); } } @@ -93,7 +95,9 @@ impl> EGLDisplay { // the first step is to query the list of extensions without any display, if supported let dp_extensions = unsafe { - let p = ffi::egl::QueryString(ffi::egl::NO_DISPLAY, ffi::egl::EXTENSIONS as i32); + let p = + wrap_egl_call(|| ffi::egl::QueryString(ffi::egl::NO_DISPLAY, ffi::egl::EXTENSIONS as i32)) + .map_err(Error::InitFailed)?; // this possibility is available only with EGL 1.5 or EGL_EXT_platform_base, otherwise // `eglQueryString` returns an error @@ -105,23 +109,22 @@ impl> EGLDisplay { list.split(' ').map(|e| e.to_string()).collect::>() } }; - debug!(log, "EGL No-Display Extensions: {:?}", dp_extensions); - let display = - unsafe { B::get_display(ptr, |e: &str| dp_extensions.iter().any(|s| s == e), log.clone()) }; - if display == ffi::egl::NO_DISPLAY { - error!(log, "EGL Display is not valid"); - return Err(Error::DisplayNotSupported); - } + let display = unsafe { + B::get_display(ptr, |e: &str| dp_extensions.iter().any(|s| s == e), log.clone()) + .map_err(Error::DisplayNotSupported)? + }; let egl_version = { let mut major: MaybeUninit = MaybeUninit::uninit(); let mut minor: MaybeUninit = MaybeUninit::uninit(); - if unsafe { ffi::egl::Initialize(display, major.as_mut_ptr(), minor.as_mut_ptr()) } == 0 { - return Err(Error::InitFailed); - } + wrap_egl_call(|| unsafe { + ffi::egl::Initialize(display, major.as_mut_ptr(), minor.as_mut_ptr()) + }) + .map_err(Error::InitFailed)?; + let major = unsafe { major.assume_init() }; let minor = unsafe { minor.assume_init() }; @@ -134,19 +137,24 @@ impl> EGLDisplay { // the list of extensions supported by the client once initialized is different from the // list of extensions obtained earlier let extensions = if egl_version >= (1, 2) { - let p = unsafe { CStr::from_ptr(ffi::egl::QueryString(display, ffi::egl::EXTENSIONS as i32)) }; + let p = unsafe { + CStr::from_ptr( + wrap_egl_call(|| ffi::egl::QueryString(display, ffi::egl::EXTENSIONS as i32)) + .map_err(Error::InitFailed)?, + ) + }; let list = String::from_utf8(p.to_bytes().to_vec()).unwrap_or_else(|_| String::new()); list.split(' ').map(|e| e.to_string()).collect::>() } else { vec![] }; - info!(log, "EGL Extensions: {:?}", extensions); - if egl_version >= (1, 2) && unsafe { ffi::egl::BindAPI(ffi::egl::OPENGL_ES_API) } == 0 { - error!(log, "OpenGLES not supported by the underlying EGL implementation"); - return Err(Error::OpenGlesNotSupported); + if egl_version >= (1, 2) { + return Err(Error::OpenGlesNotSupported(None)); } + wrap_egl_call(|| unsafe { ffi::egl::BindAPI(ffi::egl::OPENGL_ES_API) }) + .map_err(|source| Error::OpenGlesNotSupported(Some(source)))?; Ok(EGLDisplay { native: RefCell::new(native), @@ -219,15 +227,16 @@ impl> EGLDisplay { } }; - reqs.create_attributes(&mut out, &self.logger)?; + reqs.create_attributes(&mut out, &self.logger) + .map_err(|()| Error::NoAvailablePixelFormat)?; out.push(ffi::egl::NONE as c_int); out }; // calling `eglChooseConfig` - let mut num_configs = unsafe { std::mem::zeroed() }; - if unsafe { + let mut num_configs = 0; + wrap_egl_call(|| unsafe { ffi::egl::ChooseConfig( **self.display, descriptor.as_ptr(), @@ -235,18 +244,14 @@ impl> EGLDisplay { 0, &mut num_configs, ) - } == 0 - { - return Err(Error::ConfigFailed); - } - + }) + .map_err(Error::ConfigFailed)?; if num_configs == 0 { return Err(Error::NoAvailablePixelFormat); } - let mut config_ids = Vec::with_capacity(num_configs as usize); - config_ids.resize_with(num_configs as usize, || unsafe { std::mem::zeroed() }); - if unsafe { + let mut config_ids: Vec = Vec::with_capacity(num_configs as usize); + wrap_egl_call(|| unsafe { ffi::egl::ChooseConfig( **self.display, descriptor.as_ptr(), @@ -254,9 +259,10 @@ impl> EGLDisplay { num_configs, &mut num_configs, ) - } == 0 - { - return Err(Error::ConfigFailed); + }) + .map_err(Error::ConfigFailed)?; + unsafe { + config_ids.set_len(num_configs as usize); } // TODO: Deeper swap intervals might have some uses @@ -264,33 +270,41 @@ impl> EGLDisplay { let config_ids = config_ids .into_iter() - .filter(|&config| unsafe { + .map(|config| unsafe { let mut min_swap_interval = 0; - ffi::egl::GetConfigAttrib( - **self.display, - config, - ffi::egl::MIN_SWAP_INTERVAL as ffi::egl::types::EGLint, - &mut min_swap_interval, - ); + wrap_egl_call(|| { + ffi::egl::GetConfigAttrib( + **self.display, + config, + ffi::egl::MIN_SWAP_INTERVAL as ffi::egl::types::EGLint, + &mut min_swap_interval, + ) + })?; if desired_swap_interval < min_swap_interval { - return false; + return Ok(None); } let mut max_swap_interval = 0; - ffi::egl::GetConfigAttrib( - **self.display, - config, - ffi::egl::MAX_SWAP_INTERVAL as ffi::egl::types::EGLint, - &mut max_swap_interval, - ); + wrap_egl_call(|| { + ffi::egl::GetConfigAttrib( + **self.display, + config, + ffi::egl::MAX_SWAP_INTERVAL as ffi::egl::types::EGLint, + &mut max_swap_interval, + ) + })?; if desired_swap_interval > max_swap_interval { - return false; + return Ok(None); } - true + Ok(Some(config)) }) + .collect::>, EGLError>>() + .map_err(Error::ConfigFailed)? + .into_iter() + .flatten() .collect::>(); if config_ids.is_empty() { @@ -304,15 +318,15 @@ impl> EGLDisplay { macro_rules! attrib { ($display:expr, $config:expr, $attr:expr) => {{ let mut value = MaybeUninit::uninit(); - let res = ffi::egl::GetConfigAttrib( - **$display, - $config, - $attr as ffi::egl::types::EGLint, - value.as_mut_ptr(), - ); - if res == 0 { - return Err(Error::ConfigFailed); - } + wrap_egl_call(|| { + ffi::egl::GetConfigAttrib( + **$display, + $config, + $attr as ffi::egl::types::EGLint, + value.as_mut_ptr(), + ) + }) + .map_err(Error::ConfigFailed)?; value.assume_init() }}; }; @@ -357,12 +371,13 @@ impl> EGLDisplay { double_buffer: Option, config: ffi::egl::types::EGLConfig, args: N::Arguments, - ) -> Result, Error> { + ) -> Result, SurfaceCreationError> { trace!(self.logger, "Creating EGL window surface."); - let surface = self.native.borrow_mut().create_surface(args).map_err(|e| { - error!(self.logger, "EGL surface creation failed: {}", e); - Error::SurfaceCreationFailed - })?; + let surface = self + .native + .borrow_mut() + .create_surface(args) + .map_err(SurfaceCreationError::NativeSurfaceCreationFailed)?; EGLSurface::new( self.display.clone(), @@ -376,6 +391,7 @@ impl> EGLDisplay { debug!(self.logger, "EGL surface successfully created"); x }) + .map_err(SurfaceCreationError::EGLSurfaceCreationFailed) } /// Returns the runtime egl version of this display @@ -426,10 +442,10 @@ impl> EGLGraphicsBackend for EGL if !self.extensions.iter().any(|s| s == "EGL_WL_bind_wayland_display") { return Err(Error::EglExtensionNotSupported(&["EGL_WL_bind_wayland_display"])); } - let res = unsafe { ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _) }; - if res == 0 { - return Err(Error::OtherEGLDisplayAlreadyBound); - } + wrap_egl_call(|| unsafe { + ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _) + }) + .map_err(Error::OtherEGLDisplayAlreadyBound)?; Ok(EGLBufferReader::new(self.display.clone(), display.c_ptr())) } } @@ -469,16 +485,16 @@ impl EGLBufferReader { buffer: WlBuffer, ) -> ::std::result::Result { let mut format: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::EGL_TEXTURE_FORMAT, &mut format, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; + let format = match format { x if x == ffi::egl::TEXTURE_RGB as i32 => Format::RGB, x if x == ffi::egl::TEXTURE_RGBA as i32 => Format::RGBA, @@ -490,40 +506,37 @@ impl EGLBufferReader { }; let mut width: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::WIDTH as i32, &mut width, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut height: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::HEIGHT as i32, &mut height, - ) == 0 - } { - return Err(BufferAccessError::NotManaged(buffer)); - } + ) + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut inverted: i32 = 0; - if unsafe { + wrap_egl_call(|| unsafe { ffi::egl::QueryWaylandBufferWL( **self.display, buffer.as_ref().c_ptr() as _, ffi::egl::WAYLAND_Y_INVERTED_WL, &mut inverted, - ) != 0 - } { - inverted = 1; - } + ) + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut images = Vec::with_capacity(format.num_planes()); for i in 0..format.num_planes() { @@ -533,7 +546,7 @@ impl EGLBufferReader { out.push(ffi::egl::NONE as i32); images.push({ - let image = unsafe { + wrap_egl_call(|| unsafe { ffi::egl::CreateImageKHR( **self.display, ffi::egl::NO_CONTEXT, @@ -541,12 +554,8 @@ impl EGLBufferReader { buffer.as_ref().c_ptr() as *mut _, out.as_ptr(), ) - }; - if image == ffi::egl::NO_IMAGE_KHR { - return Err(BufferAccessError::EGLImageCreationFailed); - } else { - image - } + }) + .map_err(BufferAccessError::EGLImageCreationFailed)? }); } @@ -601,6 +610,7 @@ impl Drop for EGLBufferReader { fn drop(&mut self) { if !self.wayland.is_null() { unsafe { + // ignore errors on drop ffi::egl::UnbindWaylandDisplayWL(**self.display, self.wayland as _); } } diff --git a/src/backend/egl/error.rs b/src/backend/egl/error.rs index f0b0e7a..cc4c24d 100644 --- a/src/backend/egl/error.rs +++ b/src/backend/egl/error.rs @@ -1,3 +1,5 @@ +use super::ffi; + #[derive(thiserror::Error, Debug)] /// EGL errors pub enum Error { @@ -5,8 +7,8 @@ pub enum Error { #[error("The requested OpenGL version {0:?} is not supported")] OpenGlVersionNotSupported((u8, u8)), /// The EGL implementation does not support creating OpenGL ES contexts - #[error("The EGL implementation does not support creating OpenGL ES contexts")] - OpenGlesNotSupported, + #[error("The EGL implementation does not support creating OpenGL ES contexts. Err: {0:?}")] + OpenGlesNotSupported(#[source] Option), /// No available pixel format matched the criteria #[error("No available pixel format matched the criteria")] NoAvailablePixelFormat, @@ -14,26 +16,23 @@ pub enum Error { #[error("The expected backend '{0:?}' does not match the runtime")] NonMatchingBackend(&'static str), /// Unable to obtain a valid EGL Display - #[error("Unable to obtain a valid EGL Display")] - DisplayNotSupported, + #[error("Unable to obtain a valid EGL Display. Err: {0:}")] + DisplayNotSupported(#[source] EGLError), /// `eglInitialize` returned an error - #[error("Failed to initialize EGL")] - InitFailed, + #[error("Failed to initialize EGL. Err: {0:}")] + InitFailed(#[source] EGLError), /// Failed to configure the EGL context #[error("Failed to configure the EGL context")] - ConfigFailed, + ConfigFailed(#[source] EGLError), /// Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements - #[error("Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements")] - CreationFailed, - /// `eglCreateWindowSurface` failed - #[error("`eglCreateWindowSurface` failed")] - SurfaceCreationFailed, + #[error("Context creation failed as one or more requirements could not be met. Try removing some gl attributes or pixel format requirements. Err: {0:}")] + CreationFailed(#[source] EGLError), /// The required EGL extension is not supported by the underlying EGL implementation #[error("None of the following EGL extensions is supported by the underlying EGL implementation, at least one is required: {0:?}")] EglExtensionNotSupported(&'static [&'static str]), /// Only one EGLDisplay may be bound to a given `WlDisplay` at any time #[error("Only one EGLDisplay may be bound to a given `WlDisplay` at any time")] - OtherEGLDisplayAlreadyBound, + OtherEGLDisplayAlreadyBound(#[source] EGLError), /// No EGLDisplay is currently bound to this `WlDisplay` #[error("No EGLDisplay is currently bound to this `WlDisplay`")] NoEGLDisplayBound, @@ -43,7 +42,91 @@ pub enum Error { /// Failed to create `EGLImages` from the buffer #[error("Failed to create `EGLImages` from the buffer")] EGLImageCreationFailed, - /// The reason of failure could not be determined - #[error("Unknown error: {0}")] +} + +/// Raw EGL error +#[derive(thiserror::Error, Debug)] +pub enum EGLError { + /// EGL is not initialized, or could not be initialized, for the specified EGL display connection. + #[error( + "EGL is not initialized, or could not be initialized, for the specified EGL display connection." + )] + NotInitialized, + /// EGL cannot access a requested resource (for example a context is bound in another thread). + #[error("EGL cannot access a requested resource (for example a context is bound in another thread).")] + BadAccess, + /// EGL failed to allocate resources for the requested operation. + #[error("EGL failed to allocate resources for the requested operation.")] + BadAlloc, + /// An unrecognized attribute or attribute value was passed in the attribute list. + #[error("An unrecognized attribute or attribute value was passed in the attribute list.")] + BadAttribute, + /// An EGLContext argument does not name a valid EGL rendering context. + #[error("An EGLContext argument does not name a valid EGL rendering context.")] + BadContext, + /// An EGLConfig argument does not name a valid EGL frame buffer configuration. + #[error("An EGLConfig argument does not name a valid EGL frame buffer configuration.")] + BadConfig, + /// The current surface of the calling thread is a window, pixel buffer or pixmap that is no longer valid. + #[error("The current surface of the calling thread is a window, pixel buffer or pixmap that is no longer valid.")] + BadCurrentSurface, + /// An EGLDisplay argument does not name a valid EGL display connection. + #[error("An EGLDisplay argument does not name a valid EGL display connection.")] + BadDisplay, + /// An EGLSurface argument does not name a valid surface (window, pixel buffer or pixmap) configured for GL rendering. + #[error("An EGLSurface argument does not name a valid surface (window, pixel buffer or pixmap) configured for GL rendering.")] + BadSurface, + /// Arguments are inconsistent (for example, a valid context requires buffers not supplied by a valid surface). + #[error("Arguments are inconsistent (for example, a valid context requires buffers not supplied by a valid surface).")] + BadMatch, + /// One or more argument values are invalid. + #[error("One or more argument values are invalid.")] + BadParameter, + /// A NativePixmapType argument does not refer to a valid native pixmap. + #[error("A NativePixmapType argument does not refer to a valid native pixmap.")] + BadNativePixmap, + /// A NativeWindowType argument does not refer to a valid native window. + #[error("A NativeWindowType argument does not refer to a valid native window.")] + BadNativeWindow, + /// A power management event has occurred. The application must destroy all contexts and reinitialise OpenGL ES state and objects to continue rendering. + #[error("A power management event has occurred. The application must destroy all contexts and reinitialise OpenGL ES state and objects to continue rendering.")] + ContextLost, + /// An unknown error + #[error("An unknown error ({0:x})")] Unknown(u32), } + +impl From for EGLError { + fn from(value: u32) -> Self { + match value { + ffi::egl::NOT_INITIALIZED => EGLError::NotInitialized, + ffi::egl::BAD_ACCESS => EGLError::BadAccess, + ffi::egl::BAD_ALLOC => EGLError::BadAlloc, + ffi::egl::BAD_ATTRIBUTE => EGLError::BadAttribute, + ffi::egl::BAD_CONTEXT => EGLError::BadContext, + ffi::egl::BAD_CURRENT_SURFACE => EGLError::BadCurrentSurface, + ffi::egl::BAD_DISPLAY => EGLError::BadDisplay, + ffi::egl::BAD_SURFACE => EGLError::BadSurface, + ffi::egl::BAD_MATCH => EGLError::BadMatch, + ffi::egl::BAD_PARAMETER => EGLError::BadParameter, + ffi::egl::BAD_NATIVE_PIXMAP => EGLError::BadNativePixmap, + ffi::egl::BAD_NATIVE_WINDOW => EGLError::BadNativeWindow, + ffi::egl::CONTEXT_LOST => EGLError::ContextLost, + x => EGLError::Unknown(x), + } + } +} + +impl EGLError { + fn from_last_call() -> Result<(), EGLError> { + match unsafe { ffi::egl::GetError() as u32 } { + ffi::egl::SUCCESS => Ok(()), + x => Err(EGLError::from(x)), + } + } +} + +pub(crate) fn wrap_egl_call R>(call: F) -> Result { + let res = call(); + EGLError::from_last_call().map(|()| res) +} diff --git a/src/backend/egl/mod.rs b/src/backend/egl/mod.rs index 506e924..7942e98 100644 --- a/src/backend/egl/mod.rs +++ b/src/backend/egl/mod.rs @@ -19,7 +19,10 @@ //! of an EGL-based [`WlBuffer`](wayland_server::protocol::wl_buffer::WlBuffer) for rendering. #[cfg(feature = "renderer_gl")] -use crate::backend::graphics::gl::{ffi as gl_ffi, GLGraphicsBackend}; +use crate::backend::graphics::{ + gl::{ffi as gl_ffi, GLGraphicsBackend}, + SwapBuffersError as GraphicsSwapBuffersError, +}; use nix::libc::c_uint; use std::fmt; #[cfg(feature = "wayland_frontend")] @@ -28,7 +31,7 @@ use wayland_server::{protocol::wl_buffer::WlBuffer, Display}; pub mod context; pub use self::context::EGLContext; mod error; -pub use self::error::Error; +pub use self::error::*; use nix::libc::c_void; @@ -84,11 +87,11 @@ pub enum BufferAccessError { #[error("The corresponding context was lost")] ContextLost, /// This buffer is not managed by the EGL buffer - #[error("This buffer is not managed by EGL")] - NotManaged(WlBuffer), + #[error("This buffer is not managed by EGL. Err: {1:}")] + NotManaged(WlBuffer, #[source] EGLError), /// Failed to create `EGLImages` from the buffer - #[error("Failed to create EGLImages from the buffer")] - EGLImageCreationFailed, + #[error("Failed to create EGLImages from the buffer. Err: {0:}")] + EGLImageCreationFailed(#[source] EGLError), /// The required EGL extension is not supported by the underlying EGL implementation #[error("{0}")] EglExtensionNotSupported(#[from] EglExtensionNotSupportedError), @@ -99,8 +102,8 @@ impl fmt::Debug for BufferAccessError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> ::std::result::Result<(), fmt::Error> { match *self { BufferAccessError::ContextLost => write!(formatter, "BufferAccessError::ContextLost"), - BufferAccessError::NotManaged(_) => write!(formatter, "BufferAccessError::NotManaged"), - BufferAccessError::EGLImageCreationFailed => { + BufferAccessError::NotManaged(_, _) => write!(formatter, "BufferAccessError::NotManaged"), + BufferAccessError::EGLImageCreationFailed(_) => { write!(formatter, "BufferAccessError::EGLImageCreationFailed") } BufferAccessError::EglExtensionNotSupported(ref err) => write!(formatter, "{:?}", err), @@ -108,6 +111,88 @@ impl fmt::Debug for BufferAccessError { } } +/// Error that can occur when creating a surface. +#[derive(Debug, thiserror::Error)] +pub enum SurfaceCreationError { + /// Native Surface creation failed + #[error("Surface creation failed. Err: {0:}")] + NativeSurfaceCreationFailed(#[source] E), + /// EGL surface creation failed + #[error("EGL surface creation failed. Err: {0:}")] + EGLSurfaceCreationFailed(#[source] EGLError), +} + +/// Error that can happen when swapping buffers. +#[derive(Debug, thiserror::Error)] +pub enum SwapBuffersError { + /// Error of the underlying native surface + #[error("Underlying error: {0:?}")] + Underlying(#[source] E), + /// EGL error during `eglSwapBuffers` + #[error("{0:}")] + EGLSwapBuffers(#[source] EGLError), + /// EGL error during `eglCreateWindowSurface` + #[error("{0:}")] + EGLCreateWindowSurface(#[source] EGLError), +} + +impl std::convert::TryFrom> for GraphicsSwapBuffersError { + type Error = E; + fn try_from(value: SwapBuffersError) -> Result { + match value { + // bad surface is answered with a surface recreation in `swap_buffers` + x @ SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface) => { + Ok(GraphicsSwapBuffersError::TemporaryFailure(Box::new(x))) + } + // the rest is either never happening or are unrecoverable + x @ SwapBuffersError::EGLSwapBuffers(_) => Ok(GraphicsSwapBuffersError::ContextLost(Box::new(x))), + x @ SwapBuffersError::EGLCreateWindowSurface(_) => { + Ok(GraphicsSwapBuffersError::ContextLost(Box::new(x))) + } + SwapBuffersError::Underlying(e) => Err(e), + } + } +} + +/// Error that can happen when making a context (and surface) current on the active thread. +#[derive(thiserror::Error, Debug)] +#[error("`eglMakeCurrent` failed: {0}")] +pub struct MakeCurrentError(#[from] EGLError); + +impl From for GraphicsSwapBuffersError { + fn from(err: MakeCurrentError) -> GraphicsSwapBuffersError { + match err { + /* + From khronos docs: + If draw or read are not compatible with context, then an EGL_BAD_MATCH error is generated. + If context is current to some other thread, or if either draw or read are bound to contexts in another thread, an EGL_BAD_ACCESS error is generated. + If binding context would exceed the number of current contexts of that client API type supported by the implementation, an EGL_BAD_ACCESS error is generated. + If either draw or read are pbuffers created with eglCreatePbufferFromClientBuffer, and the underlying bound client API buffers are in use by the client API that created them, an EGL_BAD_ACCESS error is generated. + + Except for the first case all of these recoverable. This conversation is mostly used in winit & EglSurface, where compatible context and surfaces are build. + */ + x @ MakeCurrentError(EGLError::BadAccess) => { + GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)) + } + // BadSurface would result in a recreation in `eglSwapBuffers` -> recoverable + x @ MakeCurrentError(EGLError::BadSurface) => { + GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)) + } + /* + From khronos docs: + If the previous context of the calling thread has unflushed commands, and the previous surface is no longer valid, an EGL_BAD_CURRENT_SURFACE error is generated. + + This does not consern this or future `makeCurrent`-calls. + */ + x @ MakeCurrentError(EGLError::BadCurrentSurface) => { + GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)) + } + // the rest is either never happening or are unrecoverable + x => GraphicsSwapBuffersError::ContextLost(Box::new(x)), + } + } +} + /// Error that might happen when binding an `EGLImage` to a GL texture #[derive(Debug, Clone, PartialEq, thiserror::Error)] pub enum TextureCreationError { @@ -249,6 +334,7 @@ impl EGLImages { impl Drop for EGLImages { fn drop(&mut self) { for image in self.images.drain(..) { + // ignore result on drop unsafe { ffi::egl::DestroyImageKHR(**self.display, image); } diff --git a/src/backend/egl/native.rs b/src/backend/egl/native.rs index b7a6841..7c4bd5d 100644 --- a/src/backend/egl/native.rs +++ b/src/backend/egl/native.rs @@ -1,7 +1,6 @@ //! Type safe native types for safe context/surface creation -use super::{ffi, Error}; -use crate::backend::graphics::SwapBuffersError; +use super::{ffi, wrap_egl_call, EGLError, Error}; #[cfg(feature = "backend_winit")] use std::ptr; @@ -28,7 +27,7 @@ pub trait Backend { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay; + ) -> Result; } #[cfg(feature = "backend_winit")] @@ -42,20 +41,28 @@ impl Backend for Wayland { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_wayland") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_wayland"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_WAYLAND_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_WAYLAND_KHR, display as *mut _, ptr::null()) + }) } else if has_dp_extension("EGL_EXT_platform_wayland") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_EXT_platform_wayland"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_WAYLAND_EXT, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplayEXT( + ffi::egl::PLATFORM_WAYLAND_EXT, + display as *mut _, + ptr::null(), + ) + }) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } @@ -74,19 +81,23 @@ impl Backend for X11 { display: ffi::NativeDisplayType, has_dp_extension: F, log: ::slog::Logger, - ) -> ffi::egl::types::EGLDisplay + ) -> Result where F: Fn(&str) -> bool, { if has_dp_extension("EGL_KHR_platform_x11") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_x11"); - ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_X11_KHR, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplay(ffi::egl::PLATFORM_X11_KHR, display as *mut _, ptr::null()) + }) } else if has_dp_extension("EGL_EXT_platform_x11") && ffi::egl::GetPlatformDisplayEXT::is_loaded() { trace!(log, "EGL Display Initialization via EGL_EXT_platform_x11"); - ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_X11_EXT, display as *mut _, ptr::null()) + wrap_egl_call(|| { + ffi::egl::GetPlatformDisplayEXT(ffi::egl::PLATFORM_X11_EXT, display as *mut _, ptr::null()) + }) } else { trace!(log, "Default EGL Display Initialization via GetDisplay"); - ffi::egl::GetDisplay(display as *mut _) + wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) } } } @@ -107,7 +118,7 @@ pub unsafe trait NativeDisplay { /// Return a raw pointer EGL will accept for context creation. fn ptr(&self) -> Result; /// Create a surface - fn create_surface(&mut self, args: Self::Arguments) -> ::std::result::Result; + fn create_surface(&mut self, args: Self::Arguments) -> Result; } #[cfg(feature = "backend_winit")] @@ -166,6 +177,9 @@ unsafe impl NativeDisplay for WinitWindow { /// The returned [`NativeWindowType`](ffi::NativeWindowType) must be valid for EGL /// and there is no way to test that. pub unsafe trait NativeSurface { + /// Error of the underlying surface + type Error: std::error::Error; + /// Return a raw pointer egl will accept for surface creation. fn ptr(&self) -> ffi::NativeWindowType; @@ -184,21 +198,35 @@ pub unsafe trait NativeSurface { /// Must only be implemented if `needs_recreation` can return `true`. /// Returns true on success. /// If this call was successful `ptr()` *should* return something different. - fn recreate(&self) -> bool { - true + fn recreate(&self) -> Result<(), Self::Error> { + Ok(()) } /// Adds additional semantics when calling /// [EGLSurface::swap_buffers](::backend::egl::surface::EGLSurface::swap_buffers) /// /// Only implement if required by the backend. - fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + fn swap_buffers(&self) -> Result<(), Self::Error> { Ok(()) } } +/// Hack until ! gets stablized +#[derive(Debug)] +pub enum Never {} +impl std::fmt::Display for Never { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + unreachable!() + } +} +impl std::error::Error for Never {} + #[cfg(feature = "backend_winit")] unsafe impl NativeSurface for XlibWindow { + // this would really be a case for this: + // type Error = !; (https://github.com/rust-lang/rust/issues/35121) + type Error = Never; + fn ptr(&self) -> ffi::NativeWindowType { self.0 as *const _ } @@ -206,6 +234,9 @@ unsafe impl NativeSurface for XlibWindow { #[cfg(feature = "backend_winit")] unsafe impl NativeSurface for wegl::WlEglSurface { + // type Error = !; + type Error = Never; + fn ptr(&self) -> ffi::NativeWindowType { self.ptr() as *const _ } diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 1efe67c..4e175d2 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -1,8 +1,8 @@ //! EGL surface related structs -use super::{ffi, native, Error}; +use super::{ffi, native, wrap_egl_call, EGLError, SwapBuffersError}; use crate::backend::egl::display::EGLDisplayHandle; -use crate::backend::graphics::{PixelFormat, SwapBuffersError}; +use crate::backend::graphics::PixelFormat; use nix::libc::c_int; use std::sync::Arc; use std::{ @@ -41,7 +41,7 @@ impl EGLSurface { config: ffi::egl::types::EGLConfig, native: N, log: L, - ) -> Result, Error> + ) -> Result, EGLError> where L: Into>, { @@ -68,13 +68,9 @@ impl EGLSurface { out }; - let surface = unsafe { + let surface = wrap_egl_call(|| unsafe { ffi::egl::CreateWindowSurface(**display, config, native.ptr(), surface_attributes.as_ptr()) - }; - - if surface.is_null() { - return Err(Error::SurfaceCreationFailed); - } + })?; Ok(EGLSurface { display, @@ -87,35 +83,43 @@ impl EGLSurface { } /// Swaps buffers at the end of a frame. - pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { + pub fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { let surface = self.surface.get(); - if !surface.is_null() { - let ret = unsafe { ffi::egl::SwapBuffers(**self.display, surface as *const _) }; - - if ret == 0 { - match unsafe { ffi::egl::GetError() } as u32 { - ffi::egl::CONTEXT_LOST => return Err(SwapBuffersError::ContextLost), - err => return Err(SwapBuffersError::Unknown(err)), - }; - } else { - self.native.swap_buffers()?; - } + let result = if !surface.is_null() { + wrap_egl_call(|| unsafe { ffi::egl::SwapBuffers(**self.display, surface as *const _) }) + .map_err(SwapBuffersError::EGLSwapBuffers) + .and_then(|_| self.native.swap_buffers().map_err(SwapBuffersError::Underlying)) + } else { + Ok(()) }; - if self.native.needs_recreation() || surface.is_null() { - self.native.recreate(); + // workaround for missing `PartialEq` impl + let is_bad_surface = if let Err(SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface)) = result { + true + } else { + false + }; + + if self.native.needs_recreation() || surface.is_null() || is_bad_surface { + self.native.recreate().map_err(SwapBuffersError::Underlying)?; + if !surface.is_null() { + let _ = unsafe { ffi::egl::DestroySurface(**self.display, surface as *const _) }; + } self.surface.set(unsafe { - ffi::egl::CreateWindowSurface( - **self.display, - self.config_id, - self.native.ptr(), - self.surface_attributes.as_ptr(), - ) + wrap_egl_call(|| { + ffi::egl::CreateWindowSurface( + **self.display, + self.config_id, + self.native.ptr(), + self.surface_attributes.as_ptr(), + ) + }) + .map_err(SwapBuffersError::EGLCreateWindowSurface)? }); } - Ok(()) + result } /// Returns true if the OpenGL surface is the current one in the thread. diff --git a/src/backend/graphics/glium.rs b/src/backend/graphics/glium.rs index 5299683..9f619d5 100644 --- a/src/backend/graphics/glium.rs +++ b/src/backend/graphics/glium.rs @@ -4,35 +4,30 @@ use crate::backend::graphics::{gl::GLGraphicsBackend, SwapBuffersError}; use glium::{ backend::{Backend, Context, Facade}, debug::DebugCallbackBehavior, - Frame, SwapBuffersError as GliumSwapBuffersError, + SwapBuffersError as GliumSwapBuffersError, }; use std::{ - cell::{Ref, RefCell, RefMut}, + cell::{Cell, Ref, RefCell, RefMut}, os::raw::c_void, rc::Rc, }; -impl From for GliumSwapBuffersError { - fn from(error: SwapBuffersError) -> Self { - match error { - SwapBuffersError::ContextLost => GliumSwapBuffersError::ContextLost, - SwapBuffersError::AlreadySwapped => GliumSwapBuffersError::AlreadySwapped, - SwapBuffersError::Unknown(_) => GliumSwapBuffersError::ContextLost, // TODO - } - } -} - /// Wrapper to expose `Glium` compatibility pub struct GliumGraphicsBackend { context: Rc, backend: Rc>, + // at least this type is not `Send` or even `Sync`. + // while there can be multiple Frames, they cannot in parallel call `set_finish`. + // so a buffer of the last error is sufficient, if always cleared... + error_channel: Rc>>>, } -struct InternalBackend(RefCell); +struct InternalBackend(RefCell, Rc>>>); impl GliumGraphicsBackend { fn new(backend: T) -> GliumGraphicsBackend { - let internal = Rc::new(InternalBackend(RefCell::new(backend))); + let error_channel = Rc::new(Cell::new(None)); + let internal = Rc::new(InternalBackend(RefCell::new(backend), error_channel.clone())); GliumGraphicsBackend { // cannot fail @@ -40,6 +35,7 @@ impl GliumGraphicsBackend { Context::new(internal.clone(), true, DebugCallbackBehavior::default()).unwrap() }, backend: internal, + error_channel, } } @@ -51,7 +47,10 @@ impl GliumGraphicsBackend { /// Note that destroying a [`Frame`] is immediate, even if vsync is enabled. #[inline] pub fn draw(&self) -> Frame { - Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()) + Frame( + glium::Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()), + self.error_channel.clone(), + ) } /// Borrow the underlying backend. @@ -88,7 +87,22 @@ impl From for GliumGraphicsBackend { unsafe impl Backend for InternalBackend { fn swap_buffers(&self) -> Result<(), GliumSwapBuffersError> { - self.0.borrow().swap_buffers().map_err(Into::into) + if let Err(err) = self.0.borrow().swap_buffers() { + Err(match err { + SwapBuffersError::ContextLost(err) => { + self.1.set(Some(err)); + GliumSwapBuffersError::ContextLost + } + SwapBuffersError::TemporaryFailure(err) => { + self.1.set(Some(err)); + GliumSwapBuffersError::AlreadySwapped + } + // I do not think, this may happen, but why not + SwapBuffersError::AlreadySwapped => GliumSwapBuffersError::AlreadySwapped, + }) + } else { + Ok(()) + } } unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -104,6 +118,126 @@ unsafe impl Backend for InternalBackend { } unsafe fn make_current(&self) { + // TODO, if this ever blows up anvil, we should probably silently ignore this. + // But I have no idea, if that may happen or what glium does, if the context is not current... + // So lets leave this in to do some real world testing self.0.borrow().make_current().expect("Context was lost") } } + +/// Omplementation of `glium::Surface`, targeting the default framebuffer. +/// +/// The back- and front-buffers are swapped when you call `finish`. +/// +/// You **must** call either `finish` or `set_finish` or else the destructor will panic. +pub struct Frame(glium::Frame, Rc>>>); + +impl Frame { + /// Stop drawing, swap the buffers, and consume the Frame. + /// + /// See the documentation of [`SwapBuffersError`] about what is being returned. + pub fn finish(mut self) -> Result<(), SwapBuffersError> { + self.set_finish() + } + + /// Stop drawing, swap the buffers. + /// + /// The Frame can now be dropped regularly. Calling `finish()` or `set_finish()` again will cause `Err(SwapBuffersError::AlreadySwapped)` to be returned. + pub fn set_finish(&mut self) -> Result<(), SwapBuffersError> { + let res = self.0.set_finish(); + let err = self.1.take(); + match (res, err) { + (Ok(()), _) => Ok(()), + (Err(GliumSwapBuffersError::AlreadySwapped), Some(err)) => { + Err(SwapBuffersError::TemporaryFailure(err)) + } + (Err(GliumSwapBuffersError::AlreadySwapped), None) => Err(SwapBuffersError::AlreadySwapped), + (Err(GliumSwapBuffersError::ContextLost), Some(err)) => Err(SwapBuffersError::ContextLost(err)), + _ => unreachable!(), + } + } +} + +impl glium::Surface for Frame { + fn clear( + &mut self, + rect: Option<&glium::Rect>, + color: Option<(f32, f32, f32, f32)>, + color_srgb: bool, + depth: Option, + stencil: Option, + ) { + self.0.clear(rect, color, color_srgb, depth, stencil) + } + + fn get_dimensions(&self) -> (u32, u32) { + self.0.get_dimensions() + } + + fn get_depth_buffer_bits(&self) -> Option { + self.0.get_depth_buffer_bits() + } + + fn get_stencil_buffer_bits(&self) -> Option { + self.0.get_stencil_buffer_bits() + } + + fn draw<'a, 'b, V, I, U>( + &mut self, + v: V, + i: I, + program: &glium::Program, + uniforms: &U, + draw_parameters: &glium::draw_parameters::DrawParameters<'_>, + ) -> Result<(), glium::DrawError> + where + V: glium::vertex::MultiVerticesSource<'b>, + I: Into>, + U: glium::uniforms::Uniforms, + { + self.0.draw(v, i, program, uniforms, draw_parameters) + } + + fn blit_from_frame( + &self, + source_rect: &glium::Rect, + target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter, + ) { + self.0.blit_from_frame(source_rect, target_rect, filter); + } + + fn blit_from_simple_framebuffer( + &self, + source: &glium::framebuffer::SimpleFrameBuffer<'_>, + source_rect: &glium::Rect, + target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter, + ) { + self.0 + .blit_from_simple_framebuffer(source, source_rect, target_rect, filter) + } + + fn blit_from_multioutput_framebuffer( + &self, + source: &glium::framebuffer::MultiOutputFrameBuffer<'_>, + source_rect: &glium::Rect, + target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter, + ) { + self.0 + .blit_from_multioutput_framebuffer(source, source_rect, target_rect, filter) + } + + fn blit_color( + &self, + source_rect: &glium::Rect, + target: &S, + target_rect: &glium::BlitTarget, + filter: glium::uniforms::MagnifySamplerFilter, + ) where + S: glium::Surface, + { + self.0.blit_color(source_rect, target, target_rect, filter) + } +} diff --git a/src/backend/graphics/mod.rs b/src/backend/graphics/mod.rs index 0fb18bf..8848163 100644 --- a/src/backend/graphics/mod.rs +++ b/src/backend/graphics/mod.rs @@ -16,24 +16,33 @@ pub mod glium; pub mod software; /// Error that can happen when swapping buffers. -#[derive(Debug, Clone, PartialEq, thiserror::Error)] +#[derive(Debug, thiserror::Error)] pub enum SwapBuffersError { - /// The corresponding context has been lost and needs to be recreated. - /// - /// All the objects associated to it (textures, buffers, programs, etc.) - /// need to be recreated from scratch. - /// - /// Operations will have no effect. Functions that read textures, buffers, etc. - /// will return uninitialized data instead. - #[error("The context has been lost, it needs to be recreated")] - ContextLost, /// The buffers have already been swapped. /// /// This error can be returned when `swap_buffers` has been called multiple times /// without any modification in between. #[error("Buffers are already swapped, swap_buffers was called too many times")] AlreadySwapped, - /// Unknown error - #[error("Unknown error: {0:x}")] - Unknown(u32), + /// The corresponding context has been lost and needs to be recreated. + /// + /// All the objects associated to it (textures, buffers, programs, etc.) + /// need to be recreated from scratch. Underlying resources like native surfaces + /// might also need to be recreated. + /// + /// Operations will have no effect. Functions that read textures, buffers, etc. + /// will return uninitialized data instead. + #[error("The context has been lost, it needs to be recreated")] + ContextLost(Box), + /// A temporary condition caused to rendering to fail. + /// + /// Depending on the underlying error this *might* require fixing internal state of the rendering backend, + /// but failures mapped to `TemporaryFailure` are always recoverable without re-creating the entire stack, + /// as is represented by `ContextLost`. + /// + /// Proceed after investigating the source to reschedule another full rendering step or just this page_flip at a later time. + /// If the root cause cannot be discovered and subsequent renderings also fail, it is advised to fallback to + /// recreation. + #[error("A temporary condition caused the page flip to fail.")] + TemporaryFailure(Box), } diff --git a/src/backend/winit.rs b/src/backend/winit.rs index c1960bb..546f18f 100644 --- a/src/backend/winit.rs +++ b/src/backend/winit.rs @@ -3,7 +3,7 @@ use crate::backend::egl::display::EGLDisplay; use crate::backend::egl::get_proc_address; use crate::backend::{ - egl::{context::GlAttributes, native, EGLContext, EGLSurface, Error as EGLError}, + egl::{context::GlAttributes, native, EGLContext, EGLSurface, Error as EGLError, SurfaceCreationError}, graphics::{gl::GLGraphicsBackend, CursorBackend, PixelFormat, SwapBuffersError}, input::{ Axis, AxisSource, Event as BackendEvent, InputBackend, InputHandler, KeyState, KeyboardKeyEvent, @@ -16,6 +16,7 @@ use nix::libc::c_void; use std::{ cell::{Ref, RefCell}, cmp, + convert::TryInto, rc::Rc, time::Instant, }; @@ -47,6 +48,9 @@ pub enum Error { /// EGL error #[error("EGL error: {0}")] EGL(#[from] EGLError), + /// Surface Creation failed + #[error("Surface creation failed: {0}")] + SurfaceCreationError(#[from] SurfaceCreationError), } enum Window { @@ -277,8 +281,10 @@ impl GLGraphicsBackend for WinitGraphicsBackend { fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { trace!(self.logger, "Swapping buffers"); match *self.window { - Window::Wayland { ref surface, .. } => surface.swap_buffers(), - Window::X11 { ref surface, .. } => surface.swap_buffers(), + Window::Wayland { ref surface, .. } => { + surface.swap_buffers().map_err(|err| err.try_into().unwrap()) + } + Window::X11 { ref surface, .. } => surface.swap_buffers().map_err(|err| err.try_into().unwrap()), } } @@ -314,12 +320,12 @@ impl GLGraphicsBackend for WinitGraphicsBackend { ref surface, ref context, .. - } => context.make_current_with_surface(surface), + } => context.make_current_with_surface(surface).map_err(Into::into), Window::X11 { ref surface, ref context, .. - } => context.make_current_with_surface(surface), + } => context.make_current_with_surface(surface).map_err(Into::into), } }