From 20051d384dcf8d6516fa729adff5e64510510076 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Thu, 27 Jan 2022 18:55:06 +0100 Subject: [PATCH 1/6] drm: Use EventMetadata to pass on frame timings --- examples/raw_drm.rs | 2 +- src/backend/drm/device/mod.rs | 48 ++++++++++++++++++++++++++++++++--- src/backend/drm/mod.rs | 2 +- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/examples/raw_drm.rs b/examples/raw_drm.rs index 4c20132..12ca435 100644 --- a/examples/raw_drm.rs +++ b/examples/raw_drm.rs @@ -122,7 +122,7 @@ fn main() { let mut event_loop = EventLoop::<()>::try_new().unwrap(); event_loop .handle() - .insert_source(device, move |event, _: &mut (), _: &mut ()| match event { + .insert_source(device, move |event, _: &mut _, _: &mut ()| match event { DrmEvent::VBlank(crtc) => vblank_handler.vblank(crtc), DrmEvent::Error(e) => panic!("{}", e), }) diff --git a/src/backend/drm/device/mod.rs b/src/backend/drm/device/mod.rs index 23e9582..471fafe 100644 --- a/src/backend/drm/device/mod.rs +++ b/src/backend/drm/device/mod.rs @@ -3,10 +3,11 @@ use std::cell::RefCell; use std::os::unix::io::{AsRawFd, RawFd}; use std::path::PathBuf; use std::sync::{atomic::AtomicBool, Arc}; +use std::time::{Instant, SystemTime}; use calloop::{EventSource, Interest, Poll, PostAction, Readiness, Token, TokenFactory}; use drm::control::{connector, crtc, Device as ControlDevice, Event, Mode, ResourceHandles}; -use drm::{ClientCapability, Device as BasicDevice}; +use drm::{ClientCapability, Device as BasicDevice, DriverCapability}; use nix::libc::dev_t; use nix::sys::stat::fstat; @@ -27,6 +28,7 @@ pub struct DrmDevice { #[cfg(feature = "backend_session")] pub(super) links: RefCell>, has_universal_planes: bool, + has_monotonic_timestamps: bool, resources: ResourceHandles, pub(super) logger: ::slog::Logger, token: Token, @@ -136,6 +138,10 @@ impl DrmDevice { let has_universal_planes = dev .set_client_capability(ClientCapability::UniversalPlanes, true) .is_ok(); + let has_monotonic_timestamps = dev + .get_driver_capability(DriverCapability::MonotonicTimestamp) + .unwrap_or(0) + == 1; let resources = dev.resource_handles().map_err(|source| Error::Access { errmsg: "Error loading resource handles", dev: dev.dev_path(), @@ -154,6 +160,7 @@ impl DrmDevice { #[cfg(feature = "backend_session")] links: RefCell::new(Vec::new()), has_universal_planes, + has_monotonic_timestamps, resources, logger: log, token: Token::invalid(), @@ -311,12 +318,30 @@ pub enum DrmEvent { Error(Error), } +/// Timing metadata for page-flip events +#[derive(Debug)] +pub struct EventMetadata { + /// The time the frame flip happend + pub time: Time, + /// The sequence number of the frame + pub sequence: u32, +} + +/// Either a realtime or monotonic timestamp +#[derive(Debug)] +pub enum Time { + /// Monotonic time stamp + Monotonic(Instant), + /// Realtime time stamp + Realtime(SystemTime), +} + impl EventSource for DrmDevice where A: AsRawFd + 'static, { type Event = DrmEvent; - type Metadata = (); + type Metadata = Option; type Ret = (); fn process_events( @@ -336,7 +361,22 @@ where for event in events { if let Event::PageFlip(event) = event { trace!(self.logger, "Got a page-flip event for crtc ({:?})", event.crtc); - callback(DrmEvent::VBlank(event.crtc), &mut ()); + let metadata = EventMetadata { + time: if self.has_monotonic_timestamps { + // There is no way to create an Instant, although the underlying type on unix systems + // is just libc::timespec, which is literally what drm-rs is getting from the kernel and just converting + // into a Duration. So we cheat and initialize a Zero-Instant (because although Instant::ZERO + // exists, its private, so you cannot create abitrary Instants). What we really need is a unix-Ext + // trait for both SystemTime and Instant to convert from a libc::timespec. + // + // But this works for now, although it is quite the hack. + Time::Monotonic(unsafe { std::mem::zeroed::() } + event.duration) + } else { + Time::Realtime(SystemTime::UNIX_EPOCH + event.duration) + }, + sequence: event.frame, + }; + callback(DrmEvent::VBlank(event.crtc), &mut Some(metadata)); } else { trace!( self.logger, @@ -353,7 +393,7 @@ where dev: self.dev_path(), source, }), - &mut (), + &mut None, ); } } diff --git a/src/backend/drm/mod.rs b/src/backend/drm/mod.rs index 7a61840..8b845dd 100644 --- a/src/backend/drm/mod.rs +++ b/src/backend/drm/mod.rs @@ -76,7 +76,7 @@ pub mod node; pub(self) mod session; pub(self) mod surface; -pub use device::{DevPath, DrmDevice, DrmEvent}; +pub use device::{DevPath, DrmDevice, DrmEvent, EventMetadata as DrmEventMetadata, Time as DrmEventTime}; pub use error::Error as DrmError; pub use node::{CreateDrmNodeError, DrmNode, NodeType}; #[cfg(feature = "backend_gbm")] From c6cbce208267b239efc74fc573256e707edbd4f8 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Sat, 29 Jan 2022 14:31:42 +0100 Subject: [PATCH 2/6] Remove socket initialization from udev.rs Since `AnvilState::init` already initializes the socket, it shouldn't be necessary to duplicate this in the `udev.rs` backend. --- anvil/src/udev.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index d61c392..03c24d0 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -107,14 +107,6 @@ pub fn run_udev(log: Logger) { let mut event_loop = EventLoop::try_new().unwrap(); let display = Rc::new(RefCell::new(Display::new())); - let name = display - .borrow_mut() - .add_socket_auto() - .unwrap() - .into_string() - .unwrap(); - info!(log, "Listening on wayland socket"; "name" => name.clone()); - ::std::env::set_var("WAYLAND_DISPLAY", name); /* * Initialize session */ From 11ab4c7f92b9ae5a4335af0fcdb87520a9e561f8 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Sun, 30 Jan 2022 16:43:52 +0100 Subject: [PATCH 3/6] data_device: Pass seat to DnD events --- anvil/src/state.rs | 2 +- src/wayland/data_device/dnd_grab.rs | 4 +++- src/wayland/data_device/mod.rs | 8 +++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/anvil/src/state.rs b/anvil/src/state.rs index 6d1c123..f542f7b 100644 --- a/anvil/src/state.rs +++ b/anvil/src/state.rs @@ -167,7 +167,7 @@ impl AnvilState { DataDeviceEvent::DnDStarted { icon, .. } => { *dnd_icon2.lock().unwrap() = icon; } - DataDeviceEvent::DnDDropped => { + DataDeviceEvent::DnDDropped { .. } => { *dnd_icon2.lock().unwrap() = None; } _ => {} diff --git a/src/wayland/data_device/dnd_grab.rs b/src/wayland/data_device/dnd_grab.rs index c15daff..97a8973 100644 --- a/src/wayland/data_device/dnd_grab.rs +++ b/src/wayland/data_device/dnd_grab.rs @@ -209,7 +209,9 @@ impl PointerGrab for DnDGrab { source.cancelled(); } } - (&mut *self.callback.borrow_mut())(super::DataDeviceEvent::DnDDropped); + (&mut *self.callback.borrow_mut())(super::DataDeviceEvent::DnDDropped { + seat: self.seat.clone(), + }); self.icon = None; // in all cases abandon the drop // no more buttons are pressed, release the grab diff --git a/src/wayland/data_device/mod.rs b/src/wayland/data_device/mod.rs index d74d504..4ef0109 100644 --- a/src/wayland/data_device/mod.rs +++ b/src/wayland/data_device/mod.rs @@ -88,13 +88,18 @@ pub enum DataDeviceEvent { /// The icon the client requested to be used to be associated with the cursor icon /// during the drag'n'drop. icon: Option, + /// The seat on which the DnD operation was started + seat: Seat, }, /// The drag'n'drop action was finished by the user releasing the buttons /// /// At this point, any pointer icon should be removed. /// /// Note that this event will only be generated for client-initiated drag'n'drop session. - DnDDropped, + DnDDropped { + /// The seat on which the DnD action was finished. + seat: Seat, + }, /// A client requested to read the server-set selection SendSelection { /// the requested mime type @@ -445,6 +450,7 @@ where (&mut *callback.borrow_mut())(DataDeviceEvent::DnDStarted { source: source.clone(), icon: icon.clone(), + seat: seat.clone(), }); let start_data = pointer.grab_start_data().unwrap(); pointer.set_grab( From 8e8e1f7a945f4eb209cfe6b9a0b6c9b4a8a1c163 Mon Sep 17 00:00:00 2001 From: Christian Meissl Date: Mon, 31 Jan 2022 17:50:42 +0100 Subject: [PATCH 4/6] fix dma buffer fd leak --- src/backend/allocator/gbm.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/backend/allocator/gbm.rs b/src/backend/allocator/gbm.rs index f5bf9fd..10fc07e 100644 --- a/src/backend/allocator/gbm.rs +++ b/src/backend/allocator/gbm.rs @@ -96,14 +96,19 @@ impl AsDmabuf for GbmBuffer { return Err(GbmConvertError::UnsupportedBuffer); //TODO } - if self.fd()? == 0 { + // Make sure to only call fd once as each call will create + // a new file descriptor which has to be closed + let fd = self.fd()?; + + // gbm_bo_get_fd returns -1 if an error occurs + if fd == -1 { return Err(GbmConvertError::InvalidFD); } let mut builder = Dmabuf::builder_from_buffer(self, DmabufFlags::empty()); for idx in 0..planes { builder.add_plane( - self.fd()?, + fd, idx as u32, self.offset(idx)?, self.stride_for_plane(idx)?, From 0c8cb491295cf54d9c2c90c29224cb56185d6c3e Mon Sep 17 00:00:00 2001 From: i509VCB Date: Mon, 31 Jan 2022 13:48:13 -0600 Subject: [PATCH 5/6] drm: Replace inner Option on DrmNode and use mem::forget --- src/backend/drm/node/mod.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/backend/drm/node/mod.rs b/src/backend/drm/node/mod.rs index e53e420..59efaa0 100644 --- a/src/backend/drm/node/mod.rs +++ b/src/backend/drm/node/mod.rs @@ -7,7 +7,7 @@ use libc::dev_t; use std::{ fmt::{self, Display, Formatter}, - fs, io, + fs, io, mem, os::unix::prelude::{AsRawFd, IntoRawFd, RawFd}, path::{Path, PathBuf}, }; @@ -20,8 +20,7 @@ use nix::{ /// A node which refers to a DRM device. #[derive(Debug)] pub struct DrmNode { - // Always `Some`, None variant is used when taking ownership - fd: Option, + fd: RawFd, dev: dev_t, ty: NodeType, } @@ -56,11 +55,7 @@ impl DrmNode { _ => return Err(CreateDrmNodeError::NotDrmNode), }; - Ok(DrmNode { - fd: Some(fd), - dev, - ty, - }) + Ok(DrmNode { fd, dev, ty }) } /// Returns the type of the DRM node. @@ -120,22 +115,22 @@ impl Display for DrmNode { } impl IntoRawFd for DrmNode { - fn into_raw_fd(mut self) -> RawFd { - self.fd.take().unwrap() + fn into_raw_fd(self) -> RawFd { + let fd = self.fd; + mem::forget(self); + fd } } impl AsRawFd for DrmNode { fn as_raw_fd(&self) -> RawFd { - self.fd.unwrap() + self.fd } } impl Drop for DrmNode { fn drop(&mut self) { - if let Some(fd) = self.fd { - let _ = close(fd); - } + let _ = close(self.fd); } } From d797cdcdcbdc16378231e48eb31f3cc7a5cdca49 Mon Sep 17 00:00:00 2001 From: Victor Berger Date: Tue, 1 Feb 2022 19:23:01 +0100 Subject: [PATCH 6/6] wayland.seat: Fix pointer grab API The pointer grab mecanism had a few inconsistencies in its behavior when a grab is set or unset. THis commit uniformizes the behavior, and also introduces a synthetic motion event when the grab is set, allowing the newly set grab to change the focus or the pointer location if needed. Also adjust the DnDGrab, as well as the resize and move grabs from anvil to unset the focus while they are active, matching weston's behavior. --- anvil/src/shell.rs | 20 ++-- src/wayland/data_device/dnd_grab.rs | 5 +- src/wayland/data_device/mod.rs | 2 + src/wayland/seat/pointer.rs | 150 +++++++++++++++++----------- 4 files changed, 113 insertions(+), 64 deletions(-) diff --git a/anvil/src/shell.rs b/anvil/src/shell.rs index 627b498..0e31f3f 100644 --- a/anvil/src/shell.rs +++ b/anvil/src/shell.rs @@ -48,12 +48,15 @@ struct MoveSurfaceGrab { impl PointerGrab for MoveSurfaceGrab { fn motion( &mut self, - _handle: &mut PointerInnerHandle<'_>, + handle: &mut PointerInnerHandle<'_>, location: Point, _focus: Option<(wl_surface::WlSurface, Point)>, - _serial: Serial, - _time: u32, + serial: Serial, + time: u32, ) { + // While the grab is active, no client has pointer focus + handle.motion(location, None, serial, time); + let delta = location - self.start_data.location; let new_location = self.initial_window_location.to_f64() + delta; @@ -152,6 +155,9 @@ impl PointerGrab for ResizeSurfaceGrab { return; } + // While the grab is active, no client has pointer focus + handle.motion(location, None, serial, time); + let (mut dx, mut dy) = (location - self.start_data.location).into(); let mut new_window_width = self.initial_window_size.w; @@ -479,7 +485,7 @@ pub fn init_shell(display: Rc>, log: ::sl initial_window_location, }; - pointer.set_grab(grab, serial); + pointer.set_grab(grab, serial, 0); } XdgRequest::Resize { @@ -539,7 +545,7 @@ pub fn init_shell(display: Rc>, log: ::sl last_window_size: initial_window_size, }; - pointer.set_grab(grab, serial); + pointer.set_grab(grab, serial, 0); } XdgRequest::AckConfigure { @@ -806,7 +812,7 @@ pub fn init_shell(display: Rc>, log: ::sl initial_window_location, }; - pointer.set_grab(grab, serial); + pointer.set_grab(grab, serial, 0); } ShellRequest::Resize { @@ -866,7 +872,7 @@ pub fn init_shell(display: Rc>, log: ::sl last_window_size: initial_window_size, }; - pointer.set_grab(grab, serial); + pointer.set_grab(grab, serial, 0); } _ => (), } diff --git a/src/wayland/data_device/dnd_grab.rs b/src/wayland/data_device/dnd_grab.rs index c15daff..bc1d363 100644 --- a/src/wayland/data_device/dnd_grab.rs +++ b/src/wayland/data_device/dnd_grab.rs @@ -53,12 +53,15 @@ impl DnDGrab { impl PointerGrab for DnDGrab { fn motion( &mut self, - _handle: &mut PointerInnerHandle<'_>, + handle: &mut PointerInnerHandle<'_>, location: Point, focus: Option<(wl_surface::WlSurface, Point)>, serial: Serial, time: u32, ) { + // While the grab is active, no client has pointer focus + handle.motion(location, None, serial, time); + let seat_data = self .seat .user_data() diff --git a/src/wayland/data_device/mod.rs b/src/wayland/data_device/mod.rs index d74d504..bdb9d6e 100644 --- a/src/wayland/data_device/mod.rs +++ b/src/wayland/data_device/mod.rs @@ -356,6 +356,7 @@ pub fn start_dnd( Rc::new(RefCell::new(callback)), ), serial, + 0, ); } } @@ -457,6 +458,7 @@ where callback.clone(), ), serial, + 0, ); return; } diff --git a/src/wayland/seat/pointer.rs b/src/wayland/seat/pointer.rs index 8d604b5..2eb18a3 100644 --- a/src/wayland/seat/pointer.rs +++ b/src/wayland/seat/pointer.rs @@ -91,6 +91,76 @@ impl PointerInternal { } } + fn set_grab(&mut self, serial: Serial, grab: G, time: u32) { + self.grab = GrabStatus::Active(serial, Box::new(grab)); + // generate a move to let the grab change the focus or move the pointer as result of its initialization + let location = self.location; + let focus = self.focus.clone(); + self.motion(location, focus, serial, time); + } + + fn unset_grab(&mut self, serial: Serial, time: u32) { + self.grab = GrabStatus::None; + // restore the focus + let location = self.location; + let focus = self.pending_focus.clone(); + self.motion(location, focus, serial, time); + } + + fn motion( + &mut self, + location: Point, + focus: Option<(WlSurface, Point)>, + serial: Serial, + time: u32, + ) { + // do we leave a surface ? + let mut leave = true; + self.location = location; + if let Some((ref current_focus, _)) = self.focus { + if let Some((ref surface, _)) = focus { + if current_focus.as_ref().equals(surface.as_ref()) { + leave = false; + } + } + } + if leave { + self.with_focused_pointers(|pointer, surface| { + pointer.leave(serial.into(), surface); + if pointer.as_ref().version() >= 5 { + pointer.frame(); + } + }); + self.focus = None; + (self.image_callback)(CursorImageStatus::Default); + } + + // do we enter one ? + if let Some((surface, surface_location)) = focus { + let entered = self.focus.is_none(); + // in all cases, update the focus, the coordinates of the surface + // might have changed + self.focus = Some((surface, surface_location)); + let (x, y) = (location - surface_location.to_f64()).into(); + if entered { + self.with_focused_pointers(|pointer, surface| { + pointer.enter(serial.into(), surface, x, y); + if pointer.as_ref().version() >= 5 { + pointer.frame(); + } + }) + } else { + // we were on top of a surface and remained on it + self.with_focused_pointers(|pointer, _| { + pointer.motion(time, x, y); + if pointer.as_ref().version() >= 5 { + pointer.frame(); + } + }) + } + } + } + fn with_focused_pointers(&self, mut f: F) where F: FnMut(&WlPointer, &WlSurface), @@ -160,13 +230,13 @@ impl PointerHandle { /// Change the current grab on this pointer to the provided grab /// /// Overwrites any current grab. - pub fn set_grab(&self, grab: G, serial: Serial) { - self.inner.borrow_mut().grab = GrabStatus::Active(serial, Box::new(grab)); + pub fn set_grab(&self, grab: G, serial: Serial, time: u32) { + self.inner.borrow_mut().set_grab(serial, grab, time); } /// Remove any current grab on this pointer, resetting it to the default behavior - pub fn unset_grab(&self) { - self.inner.borrow_mut().grab = GrabStatus::None; + pub fn unset_grab(&self, serial: Serial, time: u32) { + self.inner.borrow_mut().unset_grab(serial, time); } /// Check if this pointer is currently grabbed with this serial @@ -283,6 +353,13 @@ pub struct GrabStartData { /// rather than trying to guess when the grab will end. pub trait PointerGrab { /// A motion was reported + /// + /// This method allows you attach additional behavior to a motion event, possibly altering it. + /// You generally will want to invoke `PointerInnerHandle::motion()` as part of your processing. If you + /// don't, the rest of the compositor will behave as if the motion event never occurred. + /// + /// Some grabs (such as drag'n'drop, shell resize and motion) unset the focus while they are active, + /// this is achieved by just setting the focus to `None` when invoking `PointerInnerHandle::motion()`. fn motion( &mut self, handle: &mut PointerInnerHandle<'_>, @@ -292,6 +369,10 @@ pub trait PointerGrab { time: u32, ); /// A button press was reported + /// + /// This method allows you attach additional behavior to a button event, possibly altering it. + /// You generally will want to invoke `PointerInnerHandle::button()` as part of your processing. If you + /// don't, the rest of the compositor will behave as if the button event never occurred. fn button( &mut self, handle: &mut PointerInnerHandle<'_>, @@ -301,6 +382,10 @@ pub trait PointerGrab { time: u32, ); /// An axis scroll was reported + /// + /// This method allows you attach additional behavior to an axis event, possibly altering it. + /// You generally will want to invoke `PointerInnerHandle::axis()` as part of your processing. If you + /// don't, the rest of the compositor will behave as if the axis event never occurred. fn axis(&mut self, handle: &mut PointerInnerHandle<'_>, details: AxisFrame); /// The data about the event that started the grab. fn start_data(&self) -> &GrabStartData; @@ -317,19 +402,15 @@ impl<'a> PointerInnerHandle<'a> { /// Change the current grab on this pointer to the provided grab /// /// Overwrites any current grab. - pub fn set_grab(&mut self, serial: Serial, grab: G) { - self.inner.grab = GrabStatus::Active(serial, Box::new(grab)); + pub fn set_grab(&mut self, serial: Serial, grab: G, time: u32) { + self.inner.set_grab(serial, grab, time); } /// Remove any current grab on this pointer, resetting it to the default behavior /// /// This will also restore the focus of the underlying pointer pub fn unset_grab(&mut self, serial: Serial, time: u32) { - self.inner.grab = GrabStatus::None; - // restore the focus - let location = self.current_location(); - let focus = self.inner.pending_focus.clone(); - self.motion(location, focus, serial, time); + self.inner.unset_grab(serial, time); } /// Access the current focus of this pointer @@ -368,51 +449,7 @@ impl<'a> PointerInnerHandle<'a> { serial: Serial, time: u32, ) { - // do we leave a surface ? - let mut leave = true; - self.inner.location = location; - if let Some((ref current_focus, _)) = self.inner.focus { - if let Some((ref surface, _)) = focus { - if current_focus.as_ref().equals(surface.as_ref()) { - leave = false; - } - } - } - if leave { - self.inner.with_focused_pointers(|pointer, surface| { - pointer.leave(serial.into(), surface); - if pointer.as_ref().version() >= 5 { - pointer.frame(); - } - }); - self.inner.focus = None; - (self.inner.image_callback)(CursorImageStatus::Default); - } - - // do we enter one ? - if let Some((surface, surface_location)) = focus { - let entered = self.inner.focus.is_none(); - // in all cases, update the focus, the coordinates of the surface - // might have changed - self.inner.focus = Some((surface, surface_location)); - let (x, y) = (location - surface_location.to_f64()).into(); - if entered { - self.inner.with_focused_pointers(|pointer, surface| { - pointer.enter(serial.into(), surface, x, y); - if pointer.as_ref().version() >= 5 { - pointer.frame(); - } - }) - } else { - // we were on top of a surface and remained on it - self.inner.with_focused_pointers(|pointer, _| { - pointer.motion(time, x, y); - if pointer.as_ref().version() >= 5 { - pointer.frame(); - } - }) - } - } + self.inner.motion(location, focus, serial, time); } /// Notify that a button was pressed @@ -687,6 +724,7 @@ impl PointerGrab for DefaultGrab { location: handle.current_location(), }, }, + time, ); } }