From 8c8f5e0d59440003557ce23f8a2b35da2cf0638d Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sat, 25 Apr 2020 14:33:16 +0200 Subject: [PATCH 01/26] atomic: disable connectors on device creation --- src/backend/drm/atomic/mod.rs | 47 ++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index 73653d6..b1fe607 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -187,9 +187,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>, { @@ -263,6 +275,35 @@ impl AtomicDrmDevice { dev.prop_mapping = mapping; debug!(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), dev_id, From 50f94d013a61ccc384d63cdda215b2daab1bed8a Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 13:48:03 +0200 Subject: [PATCH 02/26] legacy: disable connectors on device creation --- src/backend/drm/legacy/mod.rs | 61 +++++++++++++++++++++++++++++++++-- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 19597f3..66968f0 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -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,49 @@ impl LegacyDrmDevice { } } + if disable_connectors { + for conn in res_handles.connectors() { + let info = dev.get_connector(*conn).compat().map_err(|source| Error::Access { + errmsg: "Failed to get connector infos", + dev: dev.dev_path(), + source + })?; + if info.state() == connector::State::Connected { + let props = dev.get_properties(*conn).compat().map_err(|source| Error::Access { + errmsg: "Failed to get properties for connector", + dev: dev.dev_path(), + source + })?; + let (handles, _) = props.as_props_and_values(); + for handle in handles { + let info = dev.get_property(*handle).compat().map_err(|source| Error::Access { + errmsg: "Failed to get property of connector", + dev: dev.dev_path(), + source + })?; + if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { + dev.set_property(*conn, *handle, 3/*DRM_MODE_DPMS_OFF*/) + .compat().map_err(|source| Error::Access { + errmsg: "Failed to set property of connector", + dev: dev.dev_path(), + source + })?; + } + } + } + } + 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, From 77875f71c65bd895fe9caab2d9fc75298a4352a8 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 13:51:21 +0200 Subject: [PATCH 03/26] fallback: add disable_connectors to `FallbackDevice` initialization --- src/backend/drm/common/fallback.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index 69679a9..eebb38b 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -150,19 +150,35 @@ 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()) { + 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)?)) + Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(fd, disable_connectors, log)?)) } } } From a4203bd216bb6534a2e8ca35ed6e1d5edc61e24c Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 13:52:31 +0200 Subject: [PATCH 04/26] examples/anvil: add disable_connectors for device creation --- anvil/src/udev.rs | 2 +- examples/raw_atomic_drm.rs | 2 +- examples/raw_legacy_drm.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 5120dee..cc744a2 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -389,7 +389,7 @@ impl UdevHandler for 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..d3ec01c 100644 --- a/examples/raw_atomic_drm.rs +++ b/examples/raw_atomic_drm.rs @@ -56,7 +56,7 @@ 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(); diff --git a/examples/raw_legacy_drm.rs b/examples/raw_legacy_drm.rs index 6afa248..7c3c546 100644 --- a/examples/raw_legacy_drm.rs +++ b/examples/raw_legacy_drm.rs @@ -40,7 +40,7 @@ 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(); From b6087bf2d2183269f54ca63ffd779e283778766e Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 14:12:37 +0200 Subject: [PATCH 05/26] legacy: move create_surface into Surface constructor --- src/backend/drm/legacy/mod.rs | 50 +++--------------------------- src/backend/drm/legacy/surface.rs | 51 +++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 66968f0..d1733aa 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -250,52 +250,10 @@ 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); - } - } - } - } - - let state = State { mode, connectors }; - let backend = Rc::new(LegacyDrmSurfaceInternal { - dev: self.dev.clone(), - crtc, - state: RwLock::new(state.clone()), - pending: RwLock::new(state), - logger: self.logger.new(o!("crtc" => format!("{:?}", crtc))), - }); + let backend = Rc::new(LegacyDrmSurfaceInternal::new( + self.dev.clone(), crtc, + 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/surface.rs b/src/backend/drm/legacy/surface.rs index 6bdfa1a..83ea1f6 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -266,6 +266,57 @@ impl RawSurface for LegacyDrmSurfaceInternal { } impl LegacyDrmSurfaceInternal { + pub(crate) fn new(dev: Rc>, crtc: crtc::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); + } + } + } + } + + let state = State { current_mode, current_connectors }; + + let surface = LegacyDrmSurfaceInternal { + dev, + crtc, + state: RwLock::new(state), + pending: RwLock::new(state.clone()), + logger, + }; + + Ok(surface) + } + fn check_connector(&self, conn: connector::Handle, mode: &Mode) -> Result { let info = self .get_connector(conn) From d6fa2e96cfda66ef7e8a635f28b716e1d2021682 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 16:32:44 +0200 Subject: [PATCH 06/26] drm: Require all surfaces to always have a mode set --- anvil/src/udev.rs | 2 +- examples/raw_atomic_drm.rs | 4 +- examples/raw_legacy_drm.rs | 4 +- src/backend/drm/atomic/mod.rs | 10 +- src/backend/drm/atomic/surface.rs | 141 +++++++++++++++++------------ src/backend/drm/common/fallback.rs | 12 +-- src/backend/drm/common/mod.rs | 3 + src/backend/drm/egl/mod.rs | 24 +++-- src/backend/drm/egl/session.rs | 4 +- src/backend/drm/egl/surface.rs | 8 +- src/backend/drm/gbm/egl.rs | 8 +- src/backend/drm/gbm/mod.rs | 12 +-- src/backend/drm/gbm/surface.rs | 14 ++- src/backend/drm/legacy/mod.rs | 16 ++-- src/backend/drm/legacy/surface.rs | 68 ++++++++------ src/backend/drm/mod.rs | 24 +++-- 16 files changed, 201 insertions(+), 153 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index cc744a2..5af0e8a 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -319,7 +319,7 @@ 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(), ); diff --git a/examples/raw_atomic_drm.rs b/examples/raw_atomic_drm.rs index d3ec01c..47c2f39 100644 --- a/examples/raw_atomic_drm.rs +++ b/examples/raw_atomic_drm.rs @@ -96,9 +96,7 @@ 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 7c3c546..f5dc06e 100644 --- a/examples/raw_legacy_drm.rs +++ b/examples/raw_legacy_drm.rs @@ -78,10 +78,8 @@ 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 b1fe607..efab65c 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -19,7 +19,7 @@ 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}; @@ -339,7 +339,7 @@ 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)); } @@ -348,9 +348,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/surface.rs b/src/backend/drm/atomic/surface.rs index 57c6246..cc01e0f 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -1,7 +1,7 @@ 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; @@ -26,8 +26,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 +45,7 @@ pub(super) struct AtomicDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, + init_buffer: Cell>, } impl AsRawFd for AtomicDrmSurfaceInternal { @@ -57,26 +58,36 @@ 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() @@ -86,12 +97,8 @@ 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 +120,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 +128,23 @@ impl AtomicDrmSurfaceInternal { } } } + let state = State { + mode: current_mode, + blob: current_blob, + connectors: current_connectors, + }; + let pending = State { + mode, + blob, + connectors: connectors.into_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 +153,22 @@ 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, - }) + init_buffer: Cell::new(None), + }; + + Ok(surface) + } +} + +impl Drop for AtomicDrmSurfaceInternal { + fn drop(&mut self) { + if let Some((db, fb)) = self.init_buffer.take() { + let _ = self.destroy_framebuffer(fb); + let _ = self.destroy_dumb_buffer(db); + } } } @@ -159,11 +188,11 @@ 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 } @@ -180,15 +209,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(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -202,7 +231,7 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } else { - Err(Error::ModeNotSuitable(pending.mode.unwrap())) + Err(Error::ModeNotSuitable(pending.mode)) } } @@ -215,8 +244,8 @@ impl Surface for AtomicDrmSurfaceInternal { &mut [conn].iter(), &self.planes, None, - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly], @@ -244,8 +273,8 @@ impl Surface for AtomicDrmSurfaceInternal { &mut removed, &self.planes, None, - pending.mode, - pending.blob, + Some(pending.mode), + Some(pending.blob), )?; self.atomic_commit( @@ -259,30 +288,26 @@ impl Surface for AtomicDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { let mut pending = self.pending.write().unwrap(); // check if new config is supported - let new_blob = Some(match mode { - Some(mode) => self - .dev + 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, - })?, - None => property::Value::Unknown(0), - }); + })?; let req = self.build_request( &mut pending.connectors.iter(), &mut [].iter(), &self.planes, None, - mode, - new_blob, + Some(mode), + Some(new_blob), )?; if let Err(err) = self .atomic_commit( @@ -292,7 +317,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); } @@ -343,7 +368,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { info!( self.logger, "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() + pending.mode.name() ); } @@ -355,8 +380,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 +405,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(); @@ -834,11 +857,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 +877,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) } } diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index eebb38b..3051351 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -244,10 +244,10 @@ 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); @@ -297,9 +297,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 diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 83ef7c2..573d8d6 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -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 { diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index 3779cf5..b0927e4 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}; @@ -40,11 +40,13 @@ pub enum Error); + /// 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 + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -56,7 +58,7 @@ where impl AsRawFd for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { fn as_raw_fd(&self) -> RawFd { @@ -67,7 +69,7 @@ where impl EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { /// Try to create a new [`EglDevice`] from an open device. @@ -122,7 +124,7 @@ where struct InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { handler: Box> + 'static>, @@ -131,7 +133,7 @@ where impl DeviceHandler for InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { type Device = D; @@ -147,7 +149,7 @@ where impl Device for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { type Surface = EglSurface<::Surface>; @@ -169,6 +171,8 @@ where fn create_surface( &mut self, crtc: crtc::Handle, + mode: Mode, + connectors: &[connector::Handle], ) -> Result::Error> { info!(self.logger, "Initializing EglSurface"); @@ -182,7 +186,7 @@ where context.get_pixel_format(), self.default_requirements.double_buffer, context.get_config_id(), - crtc, + (crtc, mode, Vec::from(connectors)), ) .map_err(Error::EGL)?; @@ -221,7 +225,7 @@ where impl EGLGraphicsBackend for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + 'static, ::Surface: NativeSurface, { fn bind_wl_display(&self, display: &Display) -> Result { @@ -232,7 +236,7 @@ where impl Drop for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay + '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..68006e7 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -3,7 +3,7 @@ //! to an open [`Session`](::backend::session::Session). //! -use drm::control::crtc; +use drm::control::{crtc, connector, Mode}; use std::os::unix::io::RawFd; use super::EglDevice; @@ -22,7 +22,7 @@ impl AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay + AsSessionObserver + 'static, + D: Device + NativeDisplay)> + 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..0cacac8 100644 --- a/src/backend/drm/egl/surface.rs +++ b/src/backend/drm/egl/surface.rs @@ -53,15 +53,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) } } @@ -100,7 +100,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) } diff --git a/src/backend/drm/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 201d653..719642f 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -12,7 +12,7 @@ use crate::backend::graphics::SwapBuffersError; use super::{Error, GbmDevice, GbmSurface}; -use drm::control::{crtc, Device as ControlDevice}; +use drm::control::{crtc, connector, Device as ControlDevice, Mode}; use gbm::AsRaw; use std::marker::PhantomData; use std::ptr; @@ -52,7 +52,7 @@ impl Backend for Gbm { } 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 { @@ -63,8 +63,8 @@ unsafe impl NativeDisplay> for Gb 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) } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a292792..91861a9 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -11,7 +11,7 @@ use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface}; -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; @@ -33,9 +33,6 @@ pub enum Error 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)?; + 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)); + .size(); let surface = self .dev .borrow() diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index 30cbbb2..bf5ef9b 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -93,8 +93,6 @@ 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(); // Recreate the surface and the related resources to match the new @@ -167,15 +165,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) } } @@ -325,15 +323,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 d1733aa..75b293a 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; @@ -241,7 +241,7 @@ 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)); } @@ -250,8 +250,12 @@ impl Device for LegacyDrmDevice { return Err(Error::DeviceInactive); } + if connectors.is_empty() { + return Err(Error::SurfaceWithoutConnectors(crtc)); + } + let backend = Rc::new(LegacyDrmSurfaceInternal::new( - self.dev.clone(), crtc, + self.dev.clone(), crtc, mode, connectors, self.logger.new(o!("crtc" => format!("{:?}", crtc))), )?); diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 83ea1f6..adfaf28 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -5,6 +5,7 @@ use drm::control::{ }; use drm::Device as BasicDevice; +use std::cell::Cell; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; @@ -20,7 +21,7 @@ use failure::{Fail, ResultExt}; #[derive(Debug, PartialEq, Eq, Clone)] pub struct State { - pub mode: Option, + pub mode: Mode, pub connectors: HashSet, } @@ -30,6 +31,7 @@ pub(super) struct LegacyDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, + init_buffer: Cell>, } impl AsRawFd for LegacyDrmSurfaceInternal { @@ -96,18 +98,18 @@ 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> { 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); } @@ -124,7 +126,7 @@ impl Surface for LegacyDrmSurfaceInternal { 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 +137,23 @@ impl Surface for LegacyDrmSurfaceInternal { Ok(()) } - fn use_mode(&self, mode: Option) -> Result<(), Error> { + fn use_mode(&self, mode: Mode) -> Result<(), Error> { 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)); } } @@ -211,7 +211,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { info!( self.logger, "Setting new mode: {:?}", - pending.mode.as_ref().unwrap().name() + pending.mode.name() ); } } @@ -226,7 +226,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { .iter() .copied() .collect::>(), - pending.mode, + Some(pending.mode), ) .compat() .map_err(|source| Error::Access { @@ -266,7 +266,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { } impl LegacyDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, logger: ::slog::Logger) -> Result, Error> { + 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", @@ -304,14 +304,20 @@ impl LegacyDrmSurfaceInternal { } } - let state = State { current_mode, current_connectors }; + // 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.into_iter().copied().collect() }; let surface = LegacyDrmSurfaceInternal { dev, crtc, state: RwLock::new(state), - pending: RwLock::new(state.clone()), + pending: RwLock::new(pending), logger, + init_buffer: Cell::new(None), }; Ok(surface) @@ -371,6 +377,10 @@ impl Drop for LegacyDrmSurfaceInternal { fn drop(&mut self) { // ignore failure at this point let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); + if let Some((db, fb)) = self.init_buffer.take() { + let _ = self.destroy_framebuffer(fb); + let _ = self.destroy_dumb_buffer(db); + } } } @@ -419,11 +429,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() } @@ -439,7 +449,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) } } diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 1e05df0..534ab9d 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -94,16 +94,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. @@ -181,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. /// @@ -196,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 From 3d2e9aeff2c2600cefa7e5b937576be61190036a Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 16:40:32 +0200 Subject: [PATCH 07/26] legacy: disable connectors on surface drop --- src/backend/drm/legacy/mod.rs | 3 +++ src/backend/drm/legacy/surface.rs | 34 +++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 75b293a..5db13b6 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -76,6 +76,9 @@ impl Drop for Dev { error!(self.logger, "Failed to reset crtc ({:?}). Error: {}", handle, err); } } + // turn it off, so that remaining surfaces (if any) will not change + // the state on drop + self.active.store(false, Ordering::SeqCst); } if self.privileged { if let Err(err) = self.release_master_lock() { diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index adfaf28..dbc4050 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -9,7 +9,7 @@ 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::{RwLock, atomic::Ordering}; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; @@ -376,11 +376,41 @@ impl LegacyDrmSurfaceInternal { impl Drop for LegacyDrmSurfaceInternal { fn drop(&mut self) { // ignore failure at this point - let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); if let Some((db, fb)) = self.init_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; + } + + let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); + // disable connectors again + let current = self.state.read().unwrap(); + for conn in current.connectors.iter() { + if let Ok(info) = self.get_connector(*conn) { + if info.state() == connector::State::Connected { + if let Ok(props) = self.get_properties(*conn) { + let (handles, _) = props.as_props_and_values(); + for handle in handles { + if let Ok(info) = self.get_property(*handle) { + if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { + let _ = self.set_property(*conn, *handle, 3/*DRM_MODE_DPMS_OFF*/); + } + } + } + } + } + } + } + + // null commit + let _ = self.set_crtc(self.crtc, None, (0, 0), &[], None); } } From 7199640ad9391a1a7f31feb9fa869a129673404e Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 16:51:20 +0200 Subject: [PATCH 08/26] legacy: honor dev.active --- src/backend/drm/legacy/mod.rs | 3 --- src/backend/drm/legacy/surface.rs | 30 +++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 5db13b6..75b293a 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -76,9 +76,6 @@ impl Drop for Dev { error!(self.logger, "Failed to reset crtc ({:?}). Error: {}", handle, err); } } - // turn it off, so that remaining surfaces (if any) will not change - // the state on drop - self.active.store(false, Ordering::SeqCst); } if self.privileged { if let Err(err) = self.release_master_lock() { diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index dbc4050..92435a6 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -48,6 +48,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() @@ -63,6 +67,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 @@ -107,6 +115,10 @@ impl Surface for LegacyDrmSurfaceInternal { } 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)? { @@ -122,6 +134,10 @@ impl Surface for LegacyDrmSurfaceInternal { } fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let mut pending = self.pending.write().unwrap(); if connectors @@ -138,6 +154,10 @@ impl Surface for LegacyDrmSurfaceInternal { } 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 @@ -169,6 +189,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(); @@ -253,6 +277,10 @@ impl RawSurface for LegacyDrmSurfaceInternal { fn page_flip(&self, framebuffer: framebuffer::Handle) -> ::std::result::Result<(), SwapBuffersError> { trace!(self.logger, "Queueing Page flip"); + + if !self.dev.active.load(Ordering::SeqCst) { + return Err(SwapBuffersError::AlreadySwapped); + } ControlDevice::page_flip( self, @@ -388,7 +416,7 @@ impl Drop for LegacyDrmSurfaceInternal { // 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(); From 4786db633b5d483fb389c7f03cbb297f880b22b0 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 17:03:07 +0200 Subject: [PATCH 09/26] legacy: disable/enable connectors on remove/add --- src/backend/drm/legacy/mod.rs | 68 +++++++++++++++++-------------- src/backend/drm/legacy/surface.rs | 27 ++++-------- 2 files changed, 45 insertions(+), 50 deletions(-) diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 75b293a..7ecfe51 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -164,36 +164,8 @@ impl LegacyDrmDevice { } if disable_connectors { - for conn in res_handles.connectors() { - let info = dev.get_connector(*conn).compat().map_err(|source| Error::Access { - errmsg: "Failed to get connector infos", - dev: dev.dev_path(), - source - })?; - if info.state() == connector::State::Connected { - let props = dev.get_properties(*conn).compat().map_err(|source| Error::Access { - errmsg: "Failed to get properties for connector", - dev: dev.dev_path(), - source - })?; - let (handles, _) = props.as_props_and_values(); - for handle in handles { - let info = dev.get_property(*handle).compat().map_err(|source| Error::Access { - errmsg: "Failed to get property of connector", - dev: dev.dev_path(), - source - })?; - if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { - dev.set_property(*conn, *handle, 3/*DRM_MODE_DPMS_OFF*/) - .compat().map_err(|source| Error::Access { - errmsg: "Failed to set property of connector", - dev: dev.dev_path(), - source - })?; - } - } - } - } + 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) @@ -217,6 +189,42 @@ 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() diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 92435a6..301928a 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -201,7 +201,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 { @@ -211,6 +211,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 @@ -223,13 +224,14 @@ 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!( @@ -420,25 +422,10 @@ impl Drop for LegacyDrmSurfaceInternal { let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); // disable connectors again let current = self.state.read().unwrap(); - for conn in current.connectors.iter() { - if let Ok(info) = self.get_connector(*conn) { - if info.state() == connector::State::Connected { - if let Ok(props) = self.get_properties(*conn) { - let (handles, _) = props.as_props_and_values(); - for handle in handles { - if let Ok(info) = self.get_property(*handle) { - if info.name().to_str().map(|x| x == "DPMS").unwrap_or(false) { - let _ = self.set_property(*conn, *handle, 3/*DRM_MODE_DPMS_OFF*/); - } - } - } - } - } - } + if let Ok(_) = self.dev.set_connector_state(current.connectors.iter().copied(), false) { + // null commit + let _ = self.set_crtc(self.crtc, None, (0, 0), &[], None); } - - // null commit - let _ = self.set_crtc(self.crtc, None, (0, 0), &[], None); } } From c560aef666a2bc199d3886835e0b7948e2887854 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 17:06:32 +0200 Subject: [PATCH 10/26] legacy: do not allow removal of the last connector --- src/backend/drm/legacy/surface.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 301928a..36d9f6f 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -129,11 +129,21 @@ 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); } From 33149b17e2821b50a8c6c6346f3523b371d12a57 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 17:25:13 +0200 Subject: [PATCH 11/26] atomic: disable connectors on surface drop --- src/backend/drm/atomic/session.rs | 2 +- src/backend/drm/atomic/surface.rs | 41 ++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index b58dbe0..ad90e42 100644 --- a/src/backend/drm/atomic/session.rs +++ b/src/backend/drm/atomic/session.rs @@ -59,7 +59,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 ); } } diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index cc01e0f..00cfd12 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -8,7 +8,7 @@ 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::{RwLock, atomic::Ordering}; use failure::ResultExt as FailureResultExt; @@ -169,6 +169,45 @@ impl Drop for AtomicDrmSurfaceInternal { 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); + } } } From da18c3a5f347075474f1d80fd4e40cf4bffd72a6 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 17:28:35 +0200 Subject: [PATCH 12/26] atomic: honor dev.active --- src/backend/drm/atomic/surface.rs | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 00cfd12..6590ecf 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -236,6 +236,10 @@ impl Surface for AtomicDrmSurfaceInternal { } 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() @@ -275,6 +279,10 @@ impl Surface for AtomicDrmSurfaceInternal { } 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(); // check if new config is supported (should be) @@ -300,6 +308,10 @@ impl Surface for AtomicDrmSurfaceInternal { } fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(Error::DeviceInactive); + } + let current = self.state.write().unwrap(); let mut pending = self.pending.write().unwrap(); @@ -328,6 +340,10 @@ impl Surface for AtomicDrmSurfaceInternal { } 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 @@ -374,6 +390,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(); @@ -480,6 +500,10 @@ impl RawSurface for AtomicDrmSurfaceInternal { } fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> { + if !self.dev.active.load(Ordering::SeqCst) { + return Err(SwapBuffersError::AlreadySwapped); + } + let req = self .build_request( &mut [].iter(), @@ -506,6 +530,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(()) @@ -516,6 +544,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() { From c7a98cee21197a2ec50e42118fcae59406d50afd Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 17:32:16 +0200 Subject: [PATCH 13/26] atomic: do not allow removal of the last connector --- src/backend/drm/atomic/surface.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 6590ecf..842e04f 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -285,6 +285,11 @@ impl Surface for AtomicDrmSurfaceInternal { 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(), @@ -308,6 +313,10 @@ 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); } From e486f7b87ce20773496adb081c86118806dd4bfd Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 20:23:56 +0200 Subject: [PATCH 14/26] legacy: fixup rendering loop after tty switch --- src/backend/drm/legacy/session.rs | 77 ++++++++++++++++++++++++++++++- src/backend/drm/legacy/surface.rs | 7 --- 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index eb5c86e..e4bd243 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -8,13 +8,14 @@ use drm::Device as BasicDevice; 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 failure::ResultExt; -use super::{Dev, LegacyDrmDevice, LegacyDrmSurfaceInternal}; +use super::{Dev, LegacyDrmDevice, LegacyDrmSurfaceInternal, Error, DevPath}; use crate::backend::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -91,5 +92,77 @@ 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 36d9f6f..47ff30f 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -5,7 +5,6 @@ use drm::control::{ }; use drm::Device as BasicDevice; -use std::cell::Cell; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; @@ -31,7 +30,6 @@ pub(super) struct LegacyDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, - init_buffer: Cell>, } impl AsRawFd for LegacyDrmSurfaceInternal { @@ -357,7 +355,6 @@ impl LegacyDrmSurfaceInternal { state: RwLock::new(state), pending: RwLock::new(pending), logger, - init_buffer: Cell::new(None), }; Ok(surface) @@ -416,10 +413,6 @@ impl LegacyDrmSurfaceInternal { impl Drop for LegacyDrmSurfaceInternal { fn drop(&mut self) { // ignore failure at this point - if let Some((db, fb)) = self.init_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 From 378686611c2ec4646b49c329304bdaf104f8892a Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 20:47:52 +0200 Subject: [PATCH 15/26] drm: Let swap_buffers return real error --- src/backend/drm/atomic/surface.rs | 17 ++++++++++------- src/backend/drm/common/fallback.rs | 2 +- src/backend/drm/gbm/egl.rs | 9 ++++++++- src/backend/drm/gbm/mod.rs | 9 +++++++++ src/backend/drm/gbm/surface.rs | 22 ++++++++++------------ src/backend/drm/legacy/surface.rs | 15 +++++++++------ src/backend/drm/mod.rs | 4 +--- 7 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 842e04f..8d2ba57 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -15,7 +15,6 @@ 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 { @@ -508,9 +507,9 @@ impl RawSurface for AtomicDrmSurfaceInternal { Ok(()) } - fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> { + fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), Error> { if !self.dev.active.load(Ordering::SeqCst) { - return Err(SwapBuffersError::AlreadySwapped); + return Err(Error::DeviceInactive); } let req = self @@ -521,14 +520,18 @@ impl RawSurface for AtomicDrmSurfaceInternal { Some(framebuffer), None, None, - ) //current.mode) - .map_err(|_| SwapBuffersError::ContextLost)?; + )?; + 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(()) } @@ -971,7 +974,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 3051351..13c08ae 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -311,7 +311,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/gbm/egl.rs b/src/backend/drm/gbm/egl.rs index 719642f..085a1fd 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -89,6 +89,13 @@ unsafe impl NativeSurface for GbmSurface { fn swap_buffers(&self) -> ::std::result::Result<(), SwapBuffersError> { // 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() } + match unsafe { self.page_flip() } { + Ok(()) => Ok(()), + Err(Error::FrontBuffersExhausted) => Err(SwapBuffersError::AlreadySwapped), + Err(err) => { + warn!(self.0.logger, "Page flip failed: {}", err); + Err(SwapBuffersError::Unknown(0)) + } + } } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index 91861a9..a2d75b4 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -39,9 +39,18 @@ pub enum Error), /// 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, diff --git a/src/backend/drm/gbm/surface.rs b/src/backend/drm/gbm/surface.rs index bf5ef9b..e4ec54b 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -4,12 +4,12 @@ use super::Error; use drm::control::{connector, crtc, framebuffer, Device as ControlDevice, Mode}; use gbm::{self, BufferObject, BufferObjectFlags, Format as GbmFormat, SurfaceBufferHandle}; use image::{ImageBuffer, Rgba}; +use failure::ResultExt; 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)); @@ -275,7 +273,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() } diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index 47ff30f..8121733 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -12,7 +12,6 @@ use std::sync::{RwLock, atomic::Ordering}; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; -use crate::backend::graphics::SwapBuffersError; use super::Dev; @@ -285,11 +284,11 @@ 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(SwapBuffersError::AlreadySwapped); + return Err(Error::DeviceInactive); } ControlDevice::page_flip( @@ -298,8 +297,12 @@ impl RawSurface for LegacyDrmSurfaceInternal { framebuffer, &[PageFlipFlags::PageFlipEvent], None, - ) - .map_err(|_| SwapBuffersError::ContextLost) + ).compat() + .map_err(|source| Error::Access { + errmsg: "Failed to page flip", + dev: self.dev_path(), + source, + }) } } @@ -511,7 +514,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 534ab9d..1eb3e76 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -51,8 +51,6 @@ use calloop::mio::Interest; use calloop::InsertError; use calloop::{LoopHandle, Source}; -use super::graphics::SwapBuffersError; - #[cfg(feature = "backend_drm_atomic")] pub mod atomic; #[cfg(feature = "backend_drm")] @@ -233,7 +231,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` From 1a40ed607917e0d3d7f75770d5fd4fe0e11710f4 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 22:12:47 +0200 Subject: [PATCH 16/26] atomic: fixup rendering loop after tty switch --- src/backend/drm/atomic/session.rs | 62 ++++++++++++++++++++++++++++++- src/backend/drm/atomic/surface.rs | 2 +- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index ad90e42..1fb99c1 100644 --- a/src/backend/drm/atomic/session.rs +++ b/src/backend/drm/atomic/session.rs @@ -3,7 +3,7 @@ //! to an open [`Session`](::backend::session::Session). //! -use drm::control::crtc; +use drm::control::{crtc, property, Device as ControlDevice, AtomicCommitFlags, atomic::AtomicModeReq}; use drm::Device as BasicDevice; use nix::libc::dev_t; use nix::sys::stat; @@ -13,8 +13,10 @@ use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; +use failure::ResultExt; use super::{AtomicDrmDevice, AtomicDrmSurfaceInternal, Dev}; +use crate::backend::drm::{common::Error, DevPath}; use crate::backend::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -95,5 +97,63 @@ 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 8d2ba57..c13f1ed 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -44,7 +44,7 @@ pub(super) struct AtomicDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, - init_buffer: Cell>, + pub(super) init_buffer: Cell>, } impl AsRawFd for AtomicDrmSurfaceInternal { From 5e530b8263035b376577a04aaa51c75756611719 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 22:17:30 +0200 Subject: [PATCH 17/26] drm: better describe add_connector usage --- src/backend/drm/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 1eb3e76..5611b79 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -169,6 +169,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 From 5fb73a497ecc06028dfdecfb8ba77c3c36f15be9 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 22:37:42 +0200 Subject: [PATCH 18/26] gbm: cleanup session rendering loop restart --- src/backend/drm/gbm/session.rs | 49 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/backend/drm/gbm/session.rs b/src/backend/drm/gbm/session.rs index fbb93af..246e8ac 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,10 +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)) + if backend.crtc.set_cursor_representation(cursor, *hotspot) .is_err() { if let Err(err) = backend.dev.borrow().set_cursor(*crtc, Some(cursor)) { From 8a040630a0447bf9fcd0d9c0bc86c406f1fa514d Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Sun, 26 Apr 2020 22:43:55 +0200 Subject: [PATCH 19/26] fallback: Allow forcing legacy modeset via env-variable --- src/backend/drm/common/fallback.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index 13c08ae..e42d907 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -23,6 +23,7 @@ use drm::{ use nix::libc::c_void; use nix::libc::dev_t; use std::os::unix::io::{AsRawFd, RawFd}; +use std::env; #[cfg(feature = "use_system_lib")] use wayland_server::Display; @@ -173,11 +174,23 @@ impl FallbackDevice, LegacyDrmD let log = crate::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_drm_fallback")); info!(log, "Trying to initialize AtomicDrmDevice"); + if env::var("SMITHAY_USE_LEGACY") + .map(|x| + x == "1" + || x.to_lowercase() == "true" + || x.to_lowercase() == "yes" + || x.to_lowercase() == "y" + ).unwrap_or(false) + { + 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"); + info!(log, "Falling back to fallback LegacyDrmDevice"); Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(fd, disable_connectors, log)?)) } } From d1ac9c94db4d852e4359e168571fe0192bb0cdda Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Mon, 27 Apr 2020 00:37:57 +0200 Subject: [PATCH 20/26] atomic: Remove verbose property mapping from log --- src/backend/drm/atomic/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index efab65c..a1f4e14 100644 --- a/src/backend/drm/atomic/mod.rs +++ b/src/backend/drm/atomic/mod.rs @@ -273,7 +273,7 @@ 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 From b9201cd0b5f39b379e6242622c03bd728d96bd18 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Mon, 27 Apr 2020 00:41:40 +0200 Subject: [PATCH 21/26] atomic: Make screen setting less log spamming --- src/backend/drm/atomic/surface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index c13f1ed..d756fba 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -488,7 +488,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { } }; - debug!(self.logger, "Setting screen: {:#?}", req); + debug!(self.logger, "Setting screen: {:?}", req); self.atomic_commit( &[ AtomicCommitFlags::PageFlipEvent, From 91b03f1e37763375d1678f3ba90140d924fa18bb Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Mon, 27 Apr 2020 00:42:33 +0200 Subject: [PATCH 22/26] drm: Make DRM errors print the actual access error --- src/backend/drm/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 573d8d6..0c6f4eb 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -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, From 6c4a3817d3f5aa387a59dd7ac30a4801ee59f0b7 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Mon, 27 Apr 2020 19:46:24 +0200 Subject: [PATCH 23/26] atomic: fix set_connector/mode functions --- src/backend/drm/atomic/surface.rs | 37 ++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index d756fba..b35735d 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -44,7 +44,7 @@ pub(super) struct AtomicDrmSurfaceInternal { pub(super) state: RwLock, pub(super) pending: RwLock, pub(super) logger: ::slog::Logger, - pub(super) init_buffer: Cell>, + pub(super) test_buffer: Cell>, } impl AsRawFd for AtomicDrmSurfaceInternal { @@ -155,16 +155,36 @@ impl AtomicDrmSurfaceInternal { state: RwLock::new(state), pending: RwLock::new(pending), logger, - init_buffer: Cell::new(None), + 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.init_buffer.take() { + if let Some((db, fb)) = self.test_buffer.take() { let _ = self.destroy_framebuffer(fb); let _ = self.destroy_dumb_buffer(db); } @@ -257,7 +277,7 @@ impl Surface for AtomicDrmSurfaceInternal { &mut [conn].iter(), &mut [].iter(), &self.planes, - None, + Some(self.create_test_buffer(&pending.mode)?), Some(pending.mode), Some(pending.blob), )?; @@ -294,7 +314,7 @@ impl Surface for AtomicDrmSurfaceInternal { &mut [].iter(), &mut [conn].iter(), &self.planes, - None, + Some(self.create_test_buffer(&pending.mode)?), Some(pending.mode), Some(pending.blob), )?; @@ -331,7 +351,7 @@ impl Surface for AtomicDrmSurfaceInternal { &mut added, &mut removed, &self.planes, - None, + Some(self.create_test_buffer(&pending.mode)?), Some(pending.mode), Some(pending.blob), )?; @@ -363,12 +383,13 @@ impl Surface for AtomicDrmSurfaceInternal { 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, + test_fb, Some(mode), Some(new_blob), )?; From 31b6d844423a031f1da576baefba56524ba264c8 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Thu, 30 Apr 2020 00:24:35 +0200 Subject: [PATCH 24/26] WIP: Rework egl and glium errors --- anvil/src/glium_drawer.rs | 10 +-- examples/raw_legacy_drm.rs | 2 +- src/backend/drm/common/mod.rs | 20 ++++- src/backend/drm/egl/mod.rs | 28 ++++--- src/backend/drm/egl/session.rs | 4 +- src/backend/drm/egl/surface.rs | 15 +++- src/backend/drm/gbm/egl.rs | 37 ++++----- src/backend/drm/gbm/mod.rs | 21 +++++- src/backend/egl/context.rs | 63 ++++++---------- src/backend/egl/display.rs | 134 +++++++++++++-------------------- src/backend/egl/error.rs | 111 +++++++++++++++++++++++---- src/backend/egl/mod.rs | 89 ++++++++++++++++++++-- src/backend/egl/native.rs | 49 ++++++++---- src/backend/egl/surface.rs | 47 ++++++------ src/backend/graphics/glium.rs | 129 +++++++++++++++++++++++++++---- src/backend/graphics/mod.rs | 35 +++++---- src/backend/winit.rs | 14 ++-- 17 files changed, 537 insertions(+), 271 deletions(-) diff --git a/anvil/src/glium_drawer.rs b/anvil/src/glium_drawer.rs index b6e8f86..949634a 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,7 @@ use smithay::backend::egl::display::EGLBufferReader; use smithay::{ backend::{ egl::{BufferAccessError, EGLImages, Format}, - graphics::{gl::GLGraphicsBackend, glium::GliumGraphicsBackend}, + graphics::{gl::GLGraphicsBackend, glium::{GliumGraphicsBackend, Frame}}, }, reexports::wayland_server::protocol::{wl_buffer, wl_surface}, wayland::{ @@ -159,7 +159,7 @@ 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 +193,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 +235,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/examples/raw_legacy_drm.rs b/examples/raw_legacy_drm.rs index f5dc06e..2d01b65 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, diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index 0c6f4eb..efa2516 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -4,8 +4,8 @@ //! use drm::control::{connector, crtc, Mode, RawResourceHandle}; - use std::path::PathBuf; +use crate::backend::graphics::SwapBuffersError; pub mod fallback; @@ -71,3 +71,21 @@ 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 { + errmsg: _, + dev: _, + 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 b0927e4..e0f147e 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -17,7 +17,7 @@ use wayland_server::Display; use super::{Device, DeviceHandler, Surface}; use crate::backend::egl::native::{Backend, NativeDisplay, NativeSurface}; -use crate::backend::egl::Error as EGLError; +use crate::backend::egl::{Error as EGLError, EGLError as RawEGLError, SurfaceCreationError}; #[cfg(feature = "use_system_lib")] use crate::backend::egl::{display::EGLBufferReader, EGLGraphicsBackend}; @@ -33,8 +33,11 @@ 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), @@ -46,7 +49,7 @@ type Arguments = (crtc::Handle, Mode, Vec); pub struct EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay + 'static, + D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -58,7 +61,7 @@ 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 { @@ -69,7 +72,7 @@ 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. @@ -124,7 +127,7 @@ 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>, @@ -133,7 +136,7 @@ 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; @@ -149,7 +152,7 @@ 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>; @@ -188,7 +191,10 @@ where context.get_config_id(), (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 }) } @@ -225,7 +231,7 @@ 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 { @@ -236,7 +242,7 @@ 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 68006e7..4d34338 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -7,7 +7,7 @@ use drm::control::{crtc, connector, 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,7 @@ impl AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay)> + AsSessionObserver + 'static, + D: Device + NativeDisplay), 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 0cacac8..0824cba 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; @@ -21,8 +22,8 @@ where } impl Surface for EglSurface -where - N: NativeSurface + Surface, +where + N: native::NativeSurface + Surface, { type Connectors = ::Connectors; type Error = Error<::Error>; @@ -90,9 +91,15 @@ 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 { @@ -109,7 +116,7 @@ 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 085a1fd..d7043a2 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -7,8 +7,7 @@ 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::{Error as EglBackendError, EGLError, wrap_egl_call}; use super::{Error, GbmDevice, GbmSurface}; @@ -31,22 +30,22 @@ 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 _)) } } } @@ -59,7 +58,7 @@ unsafe impl NativeDisplay> for Gb true } - fn ptr(&self) -> Result { + fn ptr(&self) -> Result { Ok(self.dev.borrow().as_raw() as *const _) } @@ -69,6 +68,8 @@ unsafe impl NativeDisplay> for Gb } 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,25 +78,13 @@ 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. - match unsafe { self.page_flip() } { - Ok(()) => Ok(()), - Err(Error::FrontBuffersExhausted) => Err(SwapBuffersError::AlreadySwapped), - Err(err) => { - warn!(self.0.logger, "Page flip failed: {}", err); - Err(SwapBuffersError::Unknown(0)) - } - } + unsafe { self.page_flip() } } } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index a2d75b4..5d6d82e 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -10,6 +10,7 @@ //! use super::{Device, DeviceHandler, RawDevice, ResourceHandles, Surface}; +use crate::backend::graphics::SwapBuffersError; use drm::control::{connector, crtc, encoder, framebuffer, plane, Device as ControlDevice, Mode}; use drm::SystemError as DrmError; @@ -26,7 +27,7 @@ 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), @@ -256,3 +257,21 @@ 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)), + } + } +} \ No newline at end of file diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs index 938fece..7b43b69 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, Error, MakeCurrentError, wrap_egl_call}; 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,14 @@ 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"); @@ -129,22 +122,13 @@ impl EGLContext { pub unsafe fn make_current_with_surface( &self, surface: &EGLSurface, - ) -> ::std::result::Result<(), SwapBuffersError> + ) -> 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,9 @@ 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 +155,24 @@ 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 +253,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 +311,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..c158bcc 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -3,7 +3,7 @@ #[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, BufferAccessError, SurfaceCreationError, EGLContext, EGLImages, EGLSurface, Error, Format, EGLError, wrap_egl_call, }; use std::sync::Arc; @@ -44,6 +44,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 +94,7 @@ 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 +106,16 @@ 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 +128,18 @@ 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 +212,15 @@ 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 +228,13 @@ 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,44 +242,43 @@ 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 let desired_swap_interval = if attributes.vsync { 1 } else { 0 }; let config_ids = config_ids .into_iter() - .filter(|&config| unsafe { + .map(|config| unsafe { let mut min_swap_interval = 0; - ffi::egl::GetConfigAttrib( + 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( + 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::>(); + .collect::>, EGLError>>().map_err(Error::ConfigFailed)? + .into_iter().flat_map(|x| x).collect::>(); if config_ids.is_empty() { return Err(Error::NoAvailablePixelFormat); @@ -304,15 +291,12 @@ impl> EGLDisplay { macro_rules! attrib { ($display:expr, $config:expr, $attr:expr) => {{ let mut value = MaybeUninit::uninit(); - let res = ffi::egl::GetConfigAttrib( + wrap_egl_call(|| ffi::egl::GetConfigAttrib( **$display, $config, $attr as ffi::egl::types::EGLint, value.as_mut_ptr(), - ); - if res == 0 { - return Err(Error::ConfigFailed); - } + )).map_err(Error::ConfigFailed)?; value.assume_init() }}; }; @@ -357,12 +341,9 @@ 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(), @@ -375,7 +356,7 @@ impl> EGLDisplay { .map(|x| { debug!(self.logger, "EGL surface successfully created"); x - }) + }).map_err(SurfaceCreationError::EGLSurfaceCreationFailed) } /// Returns the runtime egl version of this display @@ -426,10 +407,7 @@ 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 +447,15 @@ 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 +467,34 @@ 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 +504,7 @@ impl EGLBufferReader { out.push(ffi::egl::NONE as i32); images.push({ - let image = unsafe { + let image = wrap_egl_call(|| unsafe { ffi::egl::CreateImageKHR( **self.display, ffi::egl::NO_CONTEXT, @@ -541,12 +512,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)?; + image }); } @@ -601,6 +568,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..0254628 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,89 @@ 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) +} \ No newline at end of file diff --git a/src/backend/egl/mod.rs b/src/backend/egl/mod.rs index 506e924..6000c92 100644 --- a/src/backend/egl/mod.rs +++ b/src/backend/egl/mod.rs @@ -19,7 +19,7 @@ //! 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::{SwapBuffersError as GraphicsSwapBuffersError, gl::{ffi as gl_ffi, GLGraphicsBackend}}; use nix::libc::c_uint; use std::fmt; #[cfg(feature = "wayland_frontend")] @@ -28,7 +28,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 +84,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 +99,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 +108,78 @@ 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 +321,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..09fa58a 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, Error, EGLError, wrap_egl_call}; #[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,20 @@ 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 +73,19 @@ 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 +106,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 +165,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 +186,33 @@ 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,7 +220,10 @@ 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 _ } -} +} \ No newline at end of file diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 1efe67c..3596ebf 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, EGLError, SwapBuffersError, wrap_egl_call}; 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,34 @@ 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 _) }; + 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 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()?; + // 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 _) }; } - }; - - if self.native.needs_recreation() || surface.is_null() { - self.native.recreate(); self.surface.set(unsafe { - ffi::egl::CreateWindowSurface( + 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..32be58a 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,7 @@ 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 +84,20 @@ 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 +113,94 @@ 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) + } +} \ No newline at end of file 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..acf7766 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,8 @@ 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 +318,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), } } From 9300e3509366f7119db314d3049c614d87c3897a Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Thu, 30 Apr 2020 19:03:02 +0200 Subject: [PATCH 25/26] lint: fmt --- anvil/src/glium_drawer.rs | 10 ++- anvil/src/udev.rs | 4 +- examples/raw_atomic_drm.rs | 9 +- examples/raw_legacy_drm.rs | 9 +- src/backend/drm/atomic/mod.rs | 39 ++++++--- src/backend/drm/atomic/session.rs | 33 +++++--- src/backend/drm/atomic/surface.rs | 128 +++++++++++++++++------------ src/backend/drm/common/fallback.rs | 39 ++++++--- src/backend/drm/common/mod.rs | 15 +++- src/backend/drm/egl/mod.rs | 34 ++++++-- src/backend/drm/egl/session.rs | 10 ++- src/backend/drm/egl/surface.rs | 10 ++- src/backend/drm/gbm/egl.rs | 16 ++-- src/backend/drm/gbm/mod.rs | 30 ++++--- src/backend/drm/gbm/session.rs | 12 ++- src/backend/drm/gbm/surface.rs | 6 +- src/backend/drm/legacy/mod.rs | 74 ++++++++++++----- src/backend/drm/legacy/session.rs | 40 +++++---- src/backend/drm/legacy/surface.rs | 44 ++++++---- src/backend/drm/mod.rs | 2 +- src/backend/egl/context.rs | 38 ++++++--- src/backend/egl/display.rs | 123 ++++++++++++++++++--------- src/backend/egl/error.rs | 8 +- src/backend/egl/mod.rs | 31 +++++-- src/backend/egl/native.rs | 30 +++++-- src/backend/egl/surface.rs | 29 ++++--- src/backend/graphics/glium.rs | 101 +++++++++++++++-------- src/backend/winit.rs | 6 +- 28 files changed, 623 insertions(+), 307 deletions(-) diff --git a/anvil/src/glium_drawer.rs b/anvil/src/glium_drawer.rs index 949634a..72306a6 100644 --- a/anvil/src/glium_drawer.rs +++ b/anvil/src/glium_drawer.rs @@ -16,7 +16,10 @@ use smithay::backend::egl::display::EGLBufferReader; use smithay::{ backend::{ egl::{BufferAccessError, EGLImages, Format}, - graphics::{gl::GLGraphicsBackend, glium::{GliumGraphicsBackend, Frame}}, + 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, smithay::backend::egl::EGLError::BadDisplay)) + Err(BufferAccessError::NotManaged( + buffer, + smithay::backend::egl::EGLError::BadDisplay, + )) }; match images { Ok(images) => { diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index 5af0e8a..a9d2f56 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -319,7 +319,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, connector_info.modes()[0], &[connector_info.handle()]).unwrap(), + device + .create_surface(crtc, connector_info.modes()[0], &[connector_info.handle()]) + .unwrap(), egl_buffer_reader.clone(), logger.clone(), ); diff --git a/examples/raw_atomic_drm.rs b/examples/raw_atomic_drm.rs index 47c2f39..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(), true, 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,7 +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, mode, &[connector_info.handle()]).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 2d01b65..f986ad1 100644 --- a/examples/raw_legacy_drm.rs +++ b/examples/raw_legacy_drm.rs @@ -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(), true, 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,7 +79,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, mode, &[connector_info.handle()]).unwrap()); + let surface = Rc::new( + device + .create_surface(crtc, mode, &[connector_info.handle()]) + .unwrap(), + ); /* * Lets create buffers and framebuffers. diff --git a/src/backend/drm/atomic/mod.rs b/src/backend/drm/atomic/mod.rs index a1f4e14..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, Mode, PropertyValueSet, ResourceHandle, ResourceHandles, + connector, crtc, encoder, framebuffer, plane, property, Mode, PropertyValueSet, ResourceHandle, + ResourceHandles, }; use drm::SystemError as DrmError; use drm::{ClientCapability, Device as BasicDevice}; @@ -199,7 +200,7 @@ impl AtomicDrmDevice { /// - `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 @@ -279,25 +280,38 @@ impl AtomicDrmDevice { // 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") + 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") + 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") + 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 { + .compat() + .map_err(|source| Error::Access { errmsg: "Failed to disable connectors", dev: dev.dev_path(), source, @@ -339,7 +353,12 @@ impl Device for AtomicDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::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)); } diff --git a/src/backend/drm/atomic/session.rs b/src/backend/drm/atomic/session.rs index 1fb99c1..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, property, Device as ControlDevice, AtomicCommitFlags, atomic::AtomicModeReq}; +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; @@ -13,7 +14,6 @@ use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use failure::ResultExt; use super::{AtomicDrmDevice, AtomicDrmSurfaceInternal, Dev}; use crate::backend::drm::{common::Error, DevPath}; @@ -114,31 +114,44 @@ impl AtomicDrmDeviceObserver { .map_err(|source| Error::Access { errmsg: "Error loading drm resources", dev: dev.dev_path(), - source + 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") + 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") + 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") + 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 { + .compat() + .map_err(|source| Error::Access { errmsg: "Failed to disable connectors", dev: dev.dev_path(), source, diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index b35735d..5e582df 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -1,14 +1,16 @@ use drm::buffer::Buffer; use drm::control::atomic::AtomicModeReq; use drm::control::Device as ControlDevice; -use drm::control::{connector, crtc, dumbbuffer::DumbBuffer, 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, atomic::Ordering}; +use std::sync::{atomic::Ordering, RwLock}; use failure::ResultExt as FailureResultExt; @@ -57,20 +59,27 @@ impl BasicDevice for AtomicDrmSurfaceInternal {} impl ControlDevice for AtomicDrmSurfaceInternal {} impl AtomicDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::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, })?; - + // 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) + Some(mode) => dev + .create_property_blob(mode) .compat() .map_err(|source| Error::Access { errmsg: "Failed to create Property Blob for mode", @@ -79,8 +88,9 @@ impl AtomicDrmSurfaceInternal { })?, None => property::Value::Unknown(0), }; - - let blob = dev.create_property_blob(mode) + + let blob = dev + .create_property_blob(mode) .compat() .map_err(|source| Error::Access { errmsg: "Failed to create Property Blob for mode", @@ -96,7 +106,6 @@ impl AtomicDrmSurfaceInternal { source, })?; - let mut current_connectors = HashSet::new(); for conn in res_handles.connectors() { let crtc_prop = dev @@ -163,16 +172,22 @@ impl AtomicDrmSurfaceInternal { 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 - })?; + 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); @@ -210,17 +225,32 @@ impl Drop for AtomicDrmSurfaceInternal { 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") + 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"); + 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)); @@ -335,7 +365,7 @@ impl Surface for AtomicDrmSurfaceInternal { if connectors.is_empty() { return Err(Error::SurfaceWithoutConnectors(self.crtc)); } - + if !self.dev.active.load(Ordering::SeqCst) { return Err(Error::DeviceInactive); } @@ -376,14 +406,14 @@ impl Surface for AtomicDrmSurfaceInternal { // check if new config is supported 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, - })?; - + .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(), @@ -453,11 +483,7 @@ impl RawSurface for AtomicDrmSurfaceInternal { } if current.mode != pending.mode { - info!( - self.logger, - "Setting new mode: {:?}", - pending.mode.name() - ); + info!(self.logger, "Setting new mode: {:?}", pending.mode.name()); } trace!(self.logger, "Testing screen config"); @@ -533,25 +559,25 @@ impl RawSurface for AtomicDrmSurfaceInternal { return Err(Error::DeviceInactive); } - let req = self - .build_request( - &mut [].iter(), - &mut [].iter(), - &self.planes, - Some(framebuffer), - None, - None, - )?; + 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, ) - .compat().map_err(|source| Error::Access { + .compat() + .map_err(|source| Error::Access { errmsg: "Page flip commit failed", dev: self.dev_path(), - source + source, })?; Ok(()) diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index e42d907..3f13380 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -22,8 +22,8 @@ use drm::{ #[cfg(feature = "renderer_gl")] use nix::libc::c_void; use nix::libc::dev_t; -use std::os::unix::io::{AsRawFd, RawFd}; use std::env; +use std::os::unix::io::{AsRawFd, RawFd}; #[cfg(feature = "use_system_lib")] use wayland_server::Display; @@ -175,15 +175,17 @@ impl FallbackDevice, LegacyDrmD info!(log, "Trying to initialize AtomicDrmDevice"); if env::var("SMITHAY_USE_LEGACY") - .map(|x| - x == "1" - || x.to_lowercase() == "true" - || x.to_lowercase() == "yes" - || x.to_lowercase() == "y" - ).unwrap_or(false) + .map(|x| { + x == "1" || x.to_lowercase() == "true" || x.to_lowercase() == "yes" || x.to_lowercase() == "y" + }) + .unwrap_or(false) { info!(log, "SMITHAY_USE_LEGACY is set. Forcing LegacyDrmDevice."); - return Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(fd, disable_connectors, log)?)); + return Ok(FallbackDevice::Fallback(LegacyDrmDevice::new( + fd, + disable_connectors, + log, + )?)); } match AtomicDrmDevice::new(fd.clone(), disable_connectors, log.clone()) { @@ -191,7 +193,11 @@ impl FallbackDevice, LegacyDrmD Err(err) => { error!(log, "Failed to initialize preferred AtomicDrmDevice: {}", err); info!(log, "Falling back to fallback LegacyDrmDevice"); - Ok(FallbackDevice::Fallback(LegacyDrmDevice::new(fd, disable_connectors, log)?)) + Ok(FallbackDevice::Fallback(LegacyDrmDevice::new( + fd, + disable_connectors, + log, + )?)) } } } @@ -257,10 +263,19 @@ where } } fallback_device_impl!(clear_handler, &mut Self); - fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::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, mode, connectors)?)), - FallbackDevice::Fallback(dev) => Ok(FallbackSurface::Fallback(dev.create_surface(crtc, mode, connectors)?)), + 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); diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index efa2516..f9dd54c 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -3,9 +3,9 @@ //! 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; -use crate::backend::graphics::SwapBuffersError; pub mod fallback; @@ -81,10 +81,17 @@ impl Into for Error { dev: _, source, } if match source.get_ref() { - drm::SystemError::Unknown { errno: nix::errno::Errno::EBUSY } => true, - drm::SystemError::Unknown { errno: nix::errno::Errno::EINTR } => true, + drm::SystemError::Unknown { + errno: nix::errno::Errno::EBUSY, + } => true, + drm::SystemError::Unknown { + errno: nix::errno::Errno::EINTR, + } => true, _ => false, - } => SwapBuffersError::TemporaryFailure(Box::new(source)), + } => + { + SwapBuffersError::TemporaryFailure(Box::new(source)) + } x => SwapBuffersError::ContextLost(Box::new(x)), } } diff --git a/src/backend/drm/egl/mod.rs b/src/backend/drm/egl/mod.rs index e0f147e..61614a1 100644 --- a/src/backend/drm/egl/mod.rs +++ b/src/backend/drm/egl/mod.rs @@ -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, EGLError as RawEGLError, SurfaceCreationError}; #[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::*; @@ -49,7 +49,9 @@ type Arguments = (crtc::Handle, Mode, Vec); pub struct EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { dev: EGLDisplay, @@ -61,7 +63,9 @@ where impl AsRawFd for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { fn as_raw_fd(&self) -> RawFd { @@ -72,7 +76,9 @@ where impl EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { /// Try to create a new [`EglDevice`] from an open device. @@ -127,7 +133,9 @@ where struct InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { handler: Box> + 'static>, @@ -136,7 +144,9 @@ where impl DeviceHandler for InternalDeviceHandler where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { type Device = D; @@ -152,7 +162,9 @@ where impl Device for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { type Surface = EglSurface<::Surface>; @@ -231,7 +243,9 @@ where impl EGLGraphicsBackend for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + 'static, + D: Device + + NativeDisplay::Surface as Surface>::Error> + + 'static, ::Surface: NativeSurface, { fn bind_wl_display(&self, display: &Display) -> Result { @@ -242,7 +256,9 @@ where impl Drop for EglDevice where B: Backend::Surface> + 'static, - D: Device + NativeDisplay::Surface as Surface>::Error> + '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 4d34338..ff12afe 100644 --- a/src/backend/drm/egl/session.rs +++ b/src/backend/drm/egl/session.rs @@ -3,7 +3,7 @@ //! to an open [`Session`](::backend::session::Session). //! -use drm::control::{crtc, connector, Mode}; +use drm::control::{connector, crtc, Mode}; use std::os::unix::io::RawFd; use super::EglDevice; @@ -22,7 +22,13 @@ impl AsSessionObserver> for EglDevice where S: SessionObserver + 'static, B: Backend::Surface> + 'static, - D: Device + NativeDisplay), Error=<::Surface as Surface>::Error> + 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 0824cba..7bb01c0 100644 --- a/src/backend/drm/egl/surface.rs +++ b/src/backend/drm/egl/surface.rs @@ -22,7 +22,7 @@ where } impl Surface for EglSurface -where +where N: native::NativeSurface + Surface, { type Connectors = ::Connectors; @@ -99,7 +99,9 @@ where Ok(x) => x, Err(x) => x.into(), }) - } else { Ok(()) } + } else { + Ok(()) + } } fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -116,7 +118,9 @@ where } unsafe fn make_current(&self) -> ::std::result::Result<(), SwapBuffersError> { - self.context.make_current_with_surface(&self.surface).map_err(Into::into) + 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 d7043a2..556230c 100644 --- a/src/backend/drm/gbm/egl.rs +++ b/src/backend/drm/gbm/egl.rs @@ -7,11 +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 EglBackendError, EGLError, wrap_egl_call}; +use crate::backend::egl::{wrap_egl_call, EGLError, Error as EglBackendError}; use super::{Error, GbmDevice, GbmSurface}; -use drm::control::{crtc, connector, Device as ControlDevice, Mode}; +use drm::control::{connector, crtc, Device as ControlDevice, Mode}; use gbm::AsRaw; use std::marker::PhantomData; use std::ptr; @@ -36,13 +36,19 @@ impl Backend for Gbm { { if has_dp_extension("EGL_KHR_platform_gbm") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_gbm"); - wrap_egl_call(|| 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"); - wrap_egl_call(|| 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"); - wrap_egl_call(|| 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"); wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index 5d6d82e..0c87bcf 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -171,13 +171,11 @@ impl Device for GbmDevice { ) -> Result, Error<<::Surface as Surface>::Error>> { info!(self.logger, "Initializing GbmSurface"); - let drm_surface = - Device::create_surface(&mut **self.dev.borrow_mut(), crtc, mode, connectors).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() - .size(); + let (w, h) = drm_surface.pending_mode().size(); let surface = self .dev .borrow() @@ -260,18 +258,26 @@ impl Drop for GbmDevice { impl Into for Error where - E: std::error::Error + Into + 'static + 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::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)), } } -} \ No newline at end of file +} diff --git a/src/backend/drm/gbm/session.rs b/src/backend/drm/gbm/session.rs index 246e8ac..0fb6a28 100644 --- a/src/backend/drm/gbm/session.rs +++ b/src/backend/drm/gbm/session.rs @@ -29,8 +29,8 @@ pub struct GbmDeviceObserver< impl< O: SessionObserver + 'static, - S: CursorBackend + RawSurface + 'static, - D: RawDevice + drm::control::Device + AsSessionObserver + 'static, + S: CursorBackend + RawSurface + 'static, + D: RawDevice + drm::control::Device + AsSessionObserver + 'static, > AsSessionObserver> for GbmDevice { fn observer(&mut self) -> GbmDeviceObserver { @@ -44,8 +44,8 @@ impl< impl< O: SessionObserver + 'static, - S: CursorBackend + RawSurface + 'static, - D: RawDevice + drm::control::Device + AsSessionObserver + 'static, + S: CursorBackend + RawSurface + 'static, + D: RawDevice + drm::control::Device + AsSessionObserver + 'static, > SessionObserver for GbmDeviceObserver { fn pause(&mut self, devnum: Option<(u32, u32)>) { @@ -81,9 +81,7 @@ impl< let &(ref cursor, ref hotspot): &(BufferObject<()>, (u32, u32)) = unsafe { &*backend.cursor.as_ptr() }; - if backend.crtc.set_cursor_representation(cursor, *hotspot) - .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 e4ec54b..1668d17 100644 --- a/src/backend/drm/gbm/surface.rs +++ b/src/backend/drm/gbm/surface.rs @@ -2,9 +2,9 @@ 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}; -use failure::ResultExt; use std::cell::{Cell, RefCell}; use std::rc::Rc; @@ -89,9 +89,7 @@ impl GbmSurfaceInternal { } pub fn recreate(&self) -> Result<(), Error<<::Surface as Surface>::Error>> { - let (w, h) = self - .pending_mode() - .size(); + let (w, h) = self.pending_mode().size(); // Recreate the surface and the related resources to match the new // resolution. diff --git a/src/backend/drm/legacy/mod.rs b/src/backend/drm/legacy/mod.rs index 7ecfe51..000b7c6 100644 --- a/src/backend/drm/legacy/mod.rs +++ b/src/backend/drm/legacy/mod.rs @@ -100,7 +100,7 @@ impl LegacyDrmDevice { /// - `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 @@ -190,33 +190,55 @@ impl LegacyDrmDevice { } impl Dev { - pub(in crate::backend::drm::legacy) fn set_connector_state(&self, connectors: impl Iterator, enabled: bool) -> Result<(), Error> { + 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 { + let info = self + .get_connector(conn) + .compat() + .map_err(|source| Error::Access { errmsg: "Failed to get connector infos", dev: self.dev_path(), - source + 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 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 - })?; + 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 - })?; + 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, + })?; } } } @@ -249,7 +271,12 @@ impl Device for LegacyDrmDevice { let _ = self.handler.take(); } - fn create_surface(&mut self, crtc: crtc::Handle, mode: Mode, connectors: &[connector::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)); } @@ -263,7 +290,10 @@ impl Device for LegacyDrmDevice { } let backend = Rc::new(LegacyDrmSurfaceInternal::new( - self.dev.clone(), crtc, mode, connectors, + self.dev.clone(), + crtc, + mode, + connectors, self.logger.new(o!("crtc" => format!("{:?}", crtc))), )?); diff --git a/src/backend/drm/legacy/session.rs b/src/backend/drm/legacy/session.rs index e4bd243..a211bce 100644 --- a/src/backend/drm/legacy/session.rs +++ b/src/backend/drm/legacy/session.rs @@ -5,6 +5,7 @@ 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; @@ -13,9 +14,8 @@ use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::{Rc, Weak}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use failure::ResultExt; -use super::{Dev, LegacyDrmDevice, LegacyDrmSurfaceInternal, Error, DevPath}; +use super::{Dev, DevPath, Error, LegacyDrmDevice, LegacyDrmSurfaceInternal}; use crate::backend::session::{AsSessionObserver, SessionObserver}; /// [`SessionObserver`](SessionObserver) @@ -112,24 +112,27 @@ impl LegacyDrmDeviceObserver { 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, - })?; + 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", @@ -143,11 +146,14 @@ impl LegacyDrmDeviceObserver { } // 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, - })?; + 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 @@ -162,7 +168,7 @@ impl LegacyDrmDeviceObserver { 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 8121733..dbd61c9 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -8,7 +8,7 @@ use drm::Device as BasicDevice; use std::collections::HashSet; use std::os::unix::io::{AsRawFd, RawFd}; use std::rc::Rc; -use std::sync::{RwLock, atomic::Ordering}; +use std::sync::{atomic::Ordering, RwLock}; use crate::backend::drm::{common::Error, DevPath, RawSurface, Surface}; use crate::backend::graphics::CursorBackend; @@ -115,7 +115,7 @@ impl Surface for LegacyDrmSurfaceInternal { 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)? { @@ -140,7 +140,7 @@ impl Surface for LegacyDrmSurfaceInternal { if connectors.is_empty() { return Err(Error::SurfaceWithoutConnectors(self.crtc)); } - + if !self.dev.active.load(Ordering::SeqCst) { return Err(Error::DeviceInactive); } @@ -241,11 +241,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { self.dev.set_connector_state(added.copied(), true)?; if current.mode != pending.mode { - info!( - self.logger, - "Setting new mode: {:?}", - pending.mode.name() - ); + info!(self.logger, "Setting new mode: {:?}", pending.mode.name()); } } @@ -286,7 +282,7 @@ impl RawSurface for LegacyDrmSurfaceInternal { 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); } @@ -297,7 +293,8 @@ impl RawSurface for LegacyDrmSurfaceInternal { framebuffer, &[PageFlipFlags::PageFlipEvent], None, - ).compat() + ) + .compat() .map_err(|source| Error::Access { errmsg: "Failed to page flip", dev: self.dev_path(), @@ -307,7 +304,13 @@ impl RawSurface for LegacyDrmSurfaceInternal { } impl LegacyDrmSurfaceInternal { - pub(crate) fn new(dev: Rc>, crtc: crtc::Handle, mode: Mode, connectors: &[connector::Handle], logger: ::slog::Logger) -> Result, Error> { + 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", @@ -349,9 +352,15 @@ impl LegacyDrmSurfaceInternal { // 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.into_iter().copied().collect() }; - + let state = State { + mode: current_mode.unwrap_or_else(|| unsafe { std::mem::zeroed() }), + connectors: current_connectors, + }; + let pending = State { + mode, + connectors: connectors.into_iter().copied().collect(), + }; + let surface = LegacyDrmSurfaceInternal { dev, crtc, @@ -424,11 +433,14 @@ impl Drop for LegacyDrmSurfaceInternal { // 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 let Ok(_) = self.dev.set_connector_state(current.connectors.iter().copied(), false) { + if let Ok(_) = self + .dev + .set_connector_state(current.connectors.iter().copied(), false) + { // null commit let _ = self.set_crtc(self.crtc, None, (0, 0), &[], None); } diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 5611b79..68e1da2 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -108,7 +108,7 @@ pub trait Device: AsRawFd + DevPath { &mut self, crtc: crtc::Handle, mode: Mode, - connectors: &[connector::Handle] + connectors: &[connector::Handle], ) -> Result::Error>; /// Processes any open events of the underlying file descriptor. diff --git a/src/backend/egl/context.rs b/src/backend/egl/context.rs index 7b43b69..a1ddf6b 100644 --- a/src/backend/egl/context.rs +++ b/src/backend/egl/context.rs @@ -1,6 +1,6 @@ //! EGL context related structs -use super::{ffi, Error, MakeCurrentError, wrap_egl_call}; +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}; @@ -100,7 +100,8 @@ impl EGLContext { ptr::null(), context_attributes.as_ptr(), ) - }).map_err(Error::CreationFailed)?; + }) + .map_err(Error::CreationFailed)?; info!(log, "EGL context created"); @@ -119,16 +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, - ) -> Result<(), MakeCurrentError> + pub unsafe fn make_current_with_surface(&self, surface: &EGLSurface) -> Result<(), MakeCurrentError> where N: NativeSurface, { let surface_ptr = surface.surface.get(); - wrap_egl_call(|| ffi::egl::MakeCurrent(**self.display, surface_ptr, surface_ptr, self.context)).map(|_| ()).map_err(Into::into) + 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. @@ -138,7 +138,16 @@ impl EGLContext { /// This function is marked unsafe, because the context cannot be made current /// 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) + 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. @@ -155,13 +164,20 @@ 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)})?; + wrap_egl_call(|| unsafe { + ffi::egl::MakeCurrent( + **self.display, + ffi::egl::NO_SURFACE, + ffi::egl::NO_SURFACE, + ffi::egl::NO_CONTEXT, + ) + })?; } Ok(()) } @@ -311,7 +327,7 @@ impl PixelFormatRequirements { if self.stereoscopy { error!(logger, "Stereoscopy is currently unsupported (sorry!)"); - return Err(()); + return Err(()); } Ok(()) diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index c158bcc..bc5d943 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, SurfaceCreationError, EGLContext, EGLImages, EGLSurface, Error, Format, EGLError, wrap_egl_call, + ffi, get_proc_address, native, wrap_egl_call, BufferAccessError, EGLContext, EGLError, EGLImages, + EGLSurface, Error, Format, SurfaceCreationError, }; use std::sync::Arc; @@ -94,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 = wrap_egl_call(|| ffi::egl::QueryString(ffi::egl::NO_DISPLAY, ffi::egl::EXTENSIONS as i32)).map_err(Error::InitFailed)?; + 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 @@ -108,14 +111,20 @@ impl> EGLDisplay { }; 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()).map_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(); - wrap_egl_call(|| unsafe { ffi::egl::Initialize(display, major.as_mut_ptr(), minor.as_mut_ptr()) }).map_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() }; @@ -128,7 +137,12 @@ 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(wrap_egl_call(|| ffi::egl::QueryString(display, ffi::egl::EXTENSIONS as i32)).map_err(Error::InitFailed)?) }; + 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 { @@ -139,7 +153,8 @@ impl> EGLDisplay { 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)))?; + 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), @@ -212,7 +227,8 @@ impl> EGLDisplay { } }; - reqs.create_attributes(&mut out, &self.logger).map_err(|()| Error::NoAvailablePixelFormat)?; + reqs.create_attributes(&mut out, &self.logger) + .map_err(|()| Error::NoAvailablePixelFormat)?; out.push(ffi::egl::NONE as c_int); out @@ -228,7 +244,8 @@ impl> EGLDisplay { 0, &mut num_configs, ) - }).map_err(Error::ConfigFailed)?; + }) + .map_err(Error::ConfigFailed)?; if num_configs == 0 { return Err(Error::NoAvailablePixelFormat); } @@ -242,8 +259,11 @@ impl> EGLDisplay { num_configs, &mut num_configs, ) - }).map_err(Error::ConfigFailed)?; - unsafe { config_ids.set_len(num_configs as usize); } + }) + .map_err(Error::ConfigFailed)?; + unsafe { + config_ids.set_len(num_configs as usize); + } // TODO: Deeper swap intervals might have some uses let desired_swap_interval = if attributes.vsync { 1 } else { 0 }; @@ -252,24 +272,28 @@ impl> EGLDisplay { .into_iter() .map(|config| unsafe { let mut min_swap_interval = 0; - wrap_egl_call(|| 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 Ok(None); } let mut max_swap_interval = 0; - wrap_egl_call(|| 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 Ok(None); @@ -277,8 +301,11 @@ impl> EGLDisplay { Ok(Some(config)) }) - .collect::>, EGLError>>().map_err(Error::ConfigFailed)? - .into_iter().flat_map(|x| x).collect::>(); + .collect::>, EGLError>>() + .map_err(Error::ConfigFailed)? + .into_iter() + .flat_map(|x| x) + .collect::>(); if config_ids.is_empty() { return Err(Error::NoAvailablePixelFormat); @@ -291,12 +318,15 @@ impl> EGLDisplay { macro_rules! attrib { ($display:expr, $config:expr, $attr:expr) => {{ let mut value = MaybeUninit::uninit(); - wrap_egl_call(|| ffi::egl::GetConfigAttrib( - **$display, - $config, - $attr as ffi::egl::types::EGLint, - value.as_mut_ptr(), - )).map_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() }}; }; @@ -343,7 +373,11 @@ impl> EGLDisplay { args: N::Arguments, ) -> Result, SurfaceCreationError> { trace!(self.logger, "Creating EGL window surface."); - let surface = self.native.borrow_mut().create_surface(args).map_err(SurfaceCreationError::NativeSurfaceCreationFailed)?; + let surface = self + .native + .borrow_mut() + .create_surface(args) + .map_err(SurfaceCreationError::NativeSurfaceCreationFailed)?; EGLSurface::new( self.display.clone(), @@ -356,7 +390,8 @@ impl> EGLDisplay { .map(|x| { debug!(self.logger, "EGL surface successfully created"); x - }).map_err(SurfaceCreationError::EGLSurfaceCreationFailed) + }) + .map_err(SurfaceCreationError::EGLSurfaceCreationFailed) } /// Returns the runtime egl version of this display @@ -407,7 +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"])); } - wrap_egl_call(|| unsafe { ffi::egl::BindWaylandDisplayWL(**self.display, display.c_ptr() as *mut _) }).map_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())) } } @@ -454,8 +492,9 @@ impl EGLBufferReader { ffi::egl::EGL_TEXTURE_FORMAT, &mut format, ) - }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; - + }) + .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, @@ -474,7 +513,8 @@ impl EGLBufferReader { ffi::egl::WIDTH as i32, &mut width, ) - }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut height: i32 = 0; wrap_egl_call(|| unsafe { @@ -484,7 +524,8 @@ impl EGLBufferReader { ffi::egl::HEIGHT as i32, &mut height, ) - }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut inverted: i32 = 0; wrap_egl_call(|| unsafe { @@ -494,7 +535,8 @@ impl EGLBufferReader { ffi::egl::WAYLAND_Y_INVERTED_WL, &mut inverted, ) - }).map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; + }) + .map_err(|source| BufferAccessError::NotManaged(buffer.clone(), source))?; let mut images = Vec::with_capacity(format.num_planes()); for i in 0..format.num_planes() { @@ -512,7 +554,8 @@ impl EGLBufferReader { buffer.as_ref().c_ptr() as *mut _, out.as_ptr(), ) - }).map_err(BufferAccessError::EGLImageCreationFailed)?; + }) + .map_err(BufferAccessError::EGLImageCreationFailed)?; image }); } diff --git a/src/backend/egl/error.rs b/src/backend/egl/error.rs index 0254628..cc4c24d 100644 --- a/src/backend/egl/error.rs +++ b/src/backend/egl/error.rs @@ -48,7 +48,9 @@ pub enum 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.")] + #[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).")] @@ -90,7 +92,7 @@ pub enum EGLError { #[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})")] + #[error("An unknown error ({0:x})")] Unknown(u32), } @@ -127,4 +129,4 @@ impl EGLError { pub(crate) fn wrap_egl_call R>(call: F) -> Result { let res = call(); EGLError::from_last_call().map(|()| res) -} \ No newline at end of file +} diff --git a/src/backend/egl/mod.rs b/src/backend/egl/mod.rs index 6000c92..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::{SwapBuffersError as GraphicsSwapBuffersError, 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")] @@ -116,7 +119,7 @@ pub enum SurfaceCreationError { NativeSurfaceCreationFailed(#[source] E), /// EGL surface creation failed #[error("EGL surface creation failed. Err: {0:}")] - EGLSurfaceCreationFailed(#[source] EGLError) + EGLSurfaceCreationFailed(#[source] EGLError), } /// Error that can happen when swapping buffers. @@ -134,14 +137,18 @@ pub enum SwapBuffersError { } impl std::convert::TryFrom> for GraphicsSwapBuffersError { - type Error=E; + 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))), + 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))), + x @ SwapBuffersError::EGLCreateWindowSurface(_) => { + Ok(GraphicsSwapBuffersError::ContextLost(Box::new(x))) + } SwapBuffersError::Underlying(e) => Err(e), } } @@ -164,16 +171,22 @@ impl From for GraphicsSwapBuffersError { 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)), + 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)), + 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)), + x @ MakeCurrentError(EGLError::BadCurrentSurface) => { + GraphicsSwapBuffersError::TemporaryFailure(Box::new(x)) + } // the rest is either never happening or are unrecoverable x => GraphicsSwapBuffersError::ContextLost(Box::new(x)), } diff --git a/src/backend/egl/native.rs b/src/backend/egl/native.rs index 09fa58a..7c4bd5d 100644 --- a/src/backend/egl/native.rs +++ b/src/backend/egl/native.rs @@ -1,6 +1,6 @@ //! Type safe native types for safe context/surface creation -use super::{ffi, Error, EGLError, wrap_egl_call}; +use super::{ffi, wrap_egl_call, EGLError, Error}; #[cfg(feature = "backend_winit")] use std::ptr; @@ -47,11 +47,19 @@ impl Backend for Wayland { { if has_dp_extension("EGL_KHR_platform_wayland") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_wayland"); - wrap_egl_call(|| 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"); - wrap_egl_call(|| 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"); wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) @@ -79,10 +87,14 @@ impl Backend for X11 { { if has_dp_extension("EGL_KHR_platform_x11") && ffi::egl::GetPlatformDisplay::is_loaded() { trace!(log, "EGL Display Initialization via EGL_KHR_platform_x11"); - wrap_egl_call(|| 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"); - wrap_egl_call(|| 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"); wrap_egl_call(|| ffi::egl::GetDisplay(display as *mut _)) @@ -203,7 +215,9 @@ pub unsafe trait NativeSurface { #[derive(Debug)] pub enum Never {} impl std::fmt::Display for Never { - fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { unreachable!() } + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + unreachable!() + } } impl std::error::Error for Never {} @@ -222,8 +236,8 @@ unsafe impl NativeSurface for XlibWindow { unsafe impl NativeSurface for wegl::WlEglSurface { // type Error = !; type Error = Never; - + fn ptr(&self) -> ffi::NativeWindowType { self.ptr() as *const _ } -} \ No newline at end of file +} diff --git a/src/backend/egl/surface.rs b/src/backend/egl/surface.rs index 3596ebf..4e175d2 100644 --- a/src/backend/egl/surface.rs +++ b/src/backend/egl/surface.rs @@ -1,6 +1,6 @@ //! EGL surface related structs -use super::{ffi, native, EGLError, SwapBuffersError, wrap_egl_call}; +use super::{ffi, native, wrap_egl_call, EGLError, SwapBuffersError}; use crate::backend::egl::display::EGLDisplayHandle; use crate::backend::graphics::PixelFormat; use nix::libc::c_int; @@ -90,23 +90,32 @@ impl EGLSurface { 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(()) }; + } else { + Ok(()) + }; // workaround for missing `PartialEq` impl - let is_bad_surface = if let Err(SwapBuffersError::EGLSwapBuffers(EGLError::BadSurface)) = result { true } else { false }; - + 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 { - wrap_egl_call(|| ffi::egl::CreateWindowSurface( - **self.display, - self.config_id, - self.native.ptr(), - self.surface_attributes.as_ptr(), - )).map_err(SwapBuffersError::EGLCreateWindowSurface)? + wrap_egl_call(|| { + ffi::egl::CreateWindowSurface( + **self.display, + self.config_id, + self.native.ptr(), + self.surface_attributes.as_ptr(), + ) + }) + .map_err(SwapBuffersError::EGLCreateWindowSurface)? }); } diff --git a/src/backend/graphics/glium.rs b/src/backend/graphics/glium.rs index 32be58a..9f619d5 100644 --- a/src/backend/graphics/glium.rs +++ b/src/backend/graphics/glium.rs @@ -47,7 +47,10 @@ impl GliumGraphicsBackend { /// Note that destroying a [`Frame`] is immediate, even if vsync is enabled. #[inline] pub fn draw(&self) -> Frame { - Frame(glium::Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()), self.error_channel.clone()) + Frame( + glium::Frame::new(self.context.clone(), self.backend.get_framebuffer_dimensions()), + self.error_channel.clone(), + ) } /// Borrow the underlying backend. @@ -89,7 +92,7 @@ unsafe impl Backend for InternalBackend { SwapBuffersError::ContextLost(err) => { self.1.set(Some(err)); GliumSwapBuffersError::ContextLost - }, + } SwapBuffersError::TemporaryFailure(err) => { self.1.set(Some(err)); GliumSwapBuffersError::AlreadySwapped @@ -97,7 +100,9 @@ unsafe impl Backend for InternalBackend { // I do not think, this may happen, but why not SwapBuffersError::AlreadySwapped => GliumSwapBuffersError::AlreadySwapped, }) - } else { Ok(()) } + } else { + Ok(()) + } } unsafe fn get_proc_address(&self, symbol: &str) -> *const c_void { @@ -121,15 +126,15 @@ unsafe impl Backend for InternalBackend { } /// 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() @@ -143,21 +148,28 @@ impl Frame { let err = self.1.take(); match (res, err) { (Ok(()), _) => Ok(()), - (Err(GliumSwapBuffersError::AlreadySwapped), Some(err)) => Err(SwapBuffersError::TemporaryFailure(err)), + (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)), + (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) - { + 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() } @@ -170,37 +182,62 @@ impl glium::Surface for Frame { 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 + 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) - { + 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_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_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 + 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) } -} \ No newline at end of file +} diff --git a/src/backend/winit.rs b/src/backend/winit.rs index acf7766..546f18f 100644 --- a/src/backend/winit.rs +++ b/src/backend/winit.rs @@ -50,7 +50,7 @@ pub enum Error { EGL(#[from] EGLError), /// Surface Creation failed #[error("Surface creation failed: {0}")] - SurfaceCreationError(#[from] SurfaceCreationError) + SurfaceCreationError(#[from] SurfaceCreationError), } enum Window { @@ -281,7 +281,9 @@ 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().map_err(|err| err.try_into().unwrap()), + 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()), } } From 26cae39598b421abd7dc7755630fe6133bf571c1 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Fri, 1 May 2020 01:52:51 +0200 Subject: [PATCH 26/26] lint: clippy --- src/backend/drm/atomic/surface.rs | 2 +- src/backend/drm/common/fallback.rs | 6 +++--- src/backend/drm/common/mod.rs | 23 ++++++++++------------- src/backend/drm/gbm/mod.rs | 4 ++-- src/backend/drm/legacy/surface.rs | 5 +++-- src/backend/egl/display.rs | 7 +++---- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/backend/drm/atomic/surface.rs b/src/backend/drm/atomic/surface.rs index 5e582df..9e467ce 100644 --- a/src/backend/drm/atomic/surface.rs +++ b/src/backend/drm/atomic/surface.rs @@ -144,7 +144,7 @@ impl AtomicDrmSurfaceInternal { let pending = State { mode, blob, - connectors: connectors.into_iter().copied().collect(), + connectors: connectors.iter().copied().collect(), }; let (primary, cursor) = diff --git a/src/backend/drm/common/fallback.rs b/src/backend/drm/common/fallback.rs index 3f13380..e5d666e 100644 --- a/src/backend/drm/common/fallback.rs +++ b/src/backend/drm/common/fallback.rs @@ -174,12 +174,12 @@ impl FallbackDevice, LegacyDrmD let log = crate::slog_or_stdlog(logger).new(o!("smithay_module" => "backend_drm_fallback")); info!(log, "Trying to initialize AtomicDrmDevice"); - if env::var("SMITHAY_USE_LEGACY") + 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) - { + .unwrap_or(false); + if force_legacy { info!(log, "SMITHAY_USE_LEGACY is set. Forcing LegacyDrmDevice."); return Ok(FallbackDevice::Fallback(LegacyDrmDevice::new( fd, diff --git a/src/backend/drm/common/mod.rs b/src/backend/drm/common/mod.rs index f9dd54c..c851e40 100644 --- a/src/backend/drm/common/mod.rs +++ b/src/backend/drm/common/mod.rs @@ -76,19 +76,16 @@ impl Into for Error { fn into(self) -> SwapBuffersError { match self { x @ Error::DeviceInactive => SwapBuffersError::TemporaryFailure(Box::new(x)), - Error::Access { - errmsg: _, - dev: _, - source, - } if match source.get_ref() { - drm::SystemError::Unknown { - errno: nix::errno::Errno::EBUSY, - } => true, - drm::SystemError::Unknown { - errno: nix::errno::Errno::EINTR, - } => true, - _ => false, - } => + Error::Access { 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)) } diff --git a/src/backend/drm/gbm/mod.rs b/src/backend/drm/gbm/mod.rs index 0c87bcf..d4cc66a 100644 --- a/src/backend/drm/gbm/mod.rs +++ b/src/backend/drm/gbm/mod.rs @@ -265,10 +265,10 @@ where Error::FrontBuffersExhausted => SwapBuffersError::AlreadySwapped, Error::FramebufferCreationFailed(x) if match x.get_ref() { - &drm::SystemError::Unknown { + drm::SystemError::Unknown { errno: nix::errno::Errno::EBUSY, } => true, - &drm::SystemError::Unknown { + drm::SystemError::Unknown { errno: nix::errno::Errno::EINTR, } => true, _ => false, diff --git a/src/backend/drm/legacy/surface.rs b/src/backend/drm/legacy/surface.rs index dbd61c9..de931da 100644 --- a/src/backend/drm/legacy/surface.rs +++ b/src/backend/drm/legacy/surface.rs @@ -358,7 +358,7 @@ impl LegacyDrmSurfaceInternal { }; let pending = State { mode, - connectors: connectors.into_iter().copied().collect(), + connectors: connectors.iter().copied().collect(), }; let surface = LegacyDrmSurfaceInternal { @@ -437,9 +437,10 @@ impl Drop for LegacyDrmSurfaceInternal { let _ = self.set_cursor(self.crtc, Option::<&DumbBuffer>::None); // disable connectors again let current = self.state.read().unwrap(); - if let Ok(_) = self + 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); diff --git a/src/backend/egl/display.rs b/src/backend/egl/display.rs index bc5d943..baf716b 100644 --- a/src/backend/egl/display.rs +++ b/src/backend/egl/display.rs @@ -304,7 +304,7 @@ impl> EGLDisplay { .collect::>, EGLError>>() .map_err(Error::ConfigFailed)? .into_iter() - .flat_map(|x| x) + .flatten() .collect::>(); if config_ids.is_empty() { @@ -546,7 +546,7 @@ impl EGLBufferReader { out.push(ffi::egl::NONE as i32); images.push({ - let image = wrap_egl_call(|| unsafe { + wrap_egl_call(|| unsafe { ffi::egl::CreateImageKHR( **self.display, ffi::egl::NO_CONTEXT, @@ -555,8 +555,7 @@ impl EGLBufferReader { out.as_ptr(), ) }) - .map_err(BufferAccessError::EGLImageCreationFailed)?; - image + .map_err(BufferAccessError::EGLImageCreationFailed)? }); }