From 581c750ae9432f5e945648240d7935c4aee3de38 Mon Sep 17 00:00:00 2001 From: Christian Meissl Date: Sat, 11 May 2024 20:40:17 +0200 Subject: [PATCH] use Cow for focus handling this reduces unnecessary clones significantly --- anvil/src/focus.rs | 14 +++--- anvil/src/input_handler.rs | 6 +-- anvil/src/shell/element.rs | 6 +-- anvil/src/shell/mod.rs | 6 +-- anvil/src/shell/xdg.rs | 10 ++-- anvil/src/state.rs | 11 ++-- src/desktop/space/wayland/window.rs | 12 ++--- src/desktop/space/wayland/x11.rs | 6 ++- src/desktop/wayland/layer.rs | 5 +- src/desktop/wayland/popup/grab.rs | 2 +- src/desktop/wayland/popup/manager.rs | 5 +- src/desktop/wayland/window.rs | 50 ++++++++++--------- src/wayland/cursor_shape.rs | 2 +- src/wayland/seat/mod.rs | 8 +-- src/wayland/selection/data_device/dnd_grab.rs | 4 +- .../selection/data_device/server_dnd_grab.rs | 4 +- 16 files changed, 82 insertions(+), 69 deletions(-) diff --git a/anvil/src/focus.rs b/anvil/src/focus.rs index 0a77e31584..2190277779 100644 --- a/anvil/src/focus.rs +++ b/anvil/src/focus.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + #[cfg(feature = "xwayland")] use smithay::xwayland::X11Surface; pub use smithay::{ @@ -69,7 +71,7 @@ impl IsAlive for PointerFocusTarget { impl From for WlSurface { #[inline] fn from(target: PointerFocusTarget) -> Self { - target.wl_surface().unwrap() + target.wl_surface().unwrap().into_owned() } } @@ -450,11 +452,11 @@ impl TouchTarget> for PointerFocus impl WaylandFocus for PointerFocusTarget { #[inline] - fn wl_surface(&self) -> Option { + fn wl_surface(&self) -> Option> { match self { PointerFocusTarget::WlSurface(w) => w.wl_surface(), #[cfg(feature = "xwayland")] - PointerFocusTarget::X11Surface(w) => w.wl_surface(), + PointerFocusTarget::X11Surface(w) => w.wl_surface().map(Cow::Owned), PointerFocusTarget::SSD(_) => None, } } @@ -474,11 +476,11 @@ impl WaylandFocus for PointerFocusTarget { impl WaylandFocus for KeyboardFocusTarget { #[inline] - fn wl_surface(&self) -> Option { + fn wl_surface(&self) -> Option> { match self { KeyboardFocusTarget::Window(w) => w.wl_surface(), - KeyboardFocusTarget::LayerSurface(l) => Some(l.wl_surface().clone()), - KeyboardFocusTarget::Popup(p) => Some(p.wl_surface().clone()), + KeyboardFocusTarget::LayerSurface(l) => Some(Cow::Borrowed(l.wl_surface())), + KeyboardFocusTarget::Popup(p) => Some(Cow::Borrowed(p.wl_surface())), } } } diff --git a/anvil/src/input_handler.rs b/anvil/src/input_handler.rs index 986a8a5b29..c5ab5c29b6 100644 --- a/anvil/src/input_handler.rs +++ b/anvil/src/input_handler.rs @@ -878,7 +878,7 @@ impl AnvilState { // If pointer is now in a constraint region, activate it // TODO Anywhere else pointer is moved needs to do this if let Some((under, surface_location)) = - new_under.and_then(|(target, loc)| Some((target.wl_surface()?, loc))) + new_under.and_then(|(target, loc)| Some((target.wl_surface()?.into_owned(), loc))) { with_pointer_constraint(&under, &pointer, |constraint| match constraint { Some(constraint) if !constraint.is_active() => { @@ -981,7 +981,7 @@ impl AnvilState { tool.motion( pointer_location, - under.and_then(|(f, loc)| f.wl_surface().map(|s| (s, loc))), + under.and_then(|(f, loc)| f.wl_surface().map(|s| (s.into_owned(), loc))), &tablet, SCOUNTER.next_serial(), evt.time_msec(), @@ -1028,7 +1028,7 @@ impl AnvilState { pointer.frame(self); if let (Some(under), Some(tablet), Some(tool)) = ( - under.and_then(|(f, loc)| f.wl_surface().map(|s| (s, loc))), + under.and_then(|(f, loc)| f.wl_surface().map(|s| (s.into_owned(), loc))), tablet, tool, ) { diff --git a/anvil/src/shell/element.rs b/anvil/src/shell/element.rs index d855e00210..8ac0ab804f 100644 --- a/anvil/src/shell/element.rs +++ b/anvil/src/shell/element.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::{borrow::Cow, time::Duration}; use smithay::{ backend::renderer::{ @@ -123,7 +123,7 @@ impl WindowElement { } #[inline] - pub fn wl_surface(&self) -> Option { + pub fn wl_surface(&self) -> Option> { self.0.wl_surface() } @@ -152,7 +152,7 @@ impl IsAlive for SSD { impl WaylandFocus for SSD { #[inline] - fn wl_surface(&self) -> Option { + fn wl_surface(&self) -> Option> { self.0.wl_surface() } } diff --git a/anvil/src/shell/mod.rs b/anvil/src/shell/mod.rs index 181cceed3c..6d03d396d5 100644 --- a/anvil/src/shell/mod.rs +++ b/anvil/src/shell/mod.rs @@ -63,7 +63,7 @@ fn fullscreen_output_geometry( .or_else(|| { let w = space .elements() - .find(|window| window.wl_surface().map(|s| s == *wl_surface).unwrap_or(false)); + .find(|window| window.wl_surface().map(|s| &*s == wl_surface).unwrap_or(false)); w.and_then(|w| space.outputs_for_element(w).first().cloned()) }) .as_ref() @@ -200,7 +200,7 @@ impl AnvilState { pub fn window_for_surface(&self, surface: &WlSurface) -> Option { self.space .elements() - .find(|window| window.wl_surface().map(|s| s == *surface).unwrap_or(false)) + .find(|window| window.wl_surface().map(|s| &*s == surface).unwrap_or(false)) .cloned() } } @@ -226,7 +226,7 @@ fn ensure_initial_configure(surface: &WlSurface, space: &Space, p if let Some(window) = space .elements() - .find(|window| window.wl_surface().map(|s| s == *surface).unwrap_or(false)) + .find(|window| window.wl_surface().map(|s| &*s == surface).unwrap_or(false)) .cloned() { // send the initial configure if relevant diff --git a/anvil/src/shell/xdg.rs b/anvil/src/shell/xdg.rs index 872b00c685..41b4ab2661 100644 --- a/anvil/src/shell/xdg.rs +++ b/anvil/src/shell/xdg.rs @@ -250,7 +250,7 @@ impl XdgShellHandler for AnvilState { let window = self .space .elements() - .find(|element| element.wl_surface().as_ref() == Some(&surface)); + .find(|element| element.wl_surface().as_deref() == Some(&surface)); if let Some(window) = window { use xdg_decoration::zv1::server::zxdg_toplevel_decoration_v1::Mode; let is_ssd = configure @@ -291,7 +291,7 @@ impl XdgShellHandler for AnvilState { let window = self .space .elements() - .find(|window| window.wl_surface().map(|s| s == *wl_surface).unwrap_or(false)) + .find(|window| window.wl_surface().map(|s| &*s == wl_surface).unwrap_or(false)) .unwrap(); surface.with_pending_state(|state| { @@ -392,7 +392,7 @@ impl XdgShellHandler for AnvilState { if let Some(root) = find_popup_root_surface(&kind).ok().and_then(|root| { self.space .elements() - .find(|w| w.wl_surface().map(|s| s == root).unwrap_or(false)) + .find(|w| w.wl_surface().map(|s| *s == root).unwrap_or(false)) .cloned() .map(KeyboardFocusTarget::from) .or_else(|| { @@ -598,13 +598,13 @@ impl AnvilState { fn handle_toplevel_commit(space: &mut Space, surface: &WlSurface) -> Option<()> { let window = space .elements() - .find(|w| w.wl_surface().as_ref() == Some(surface)) + .find(|w| w.wl_surface().as_deref() == Some(surface)) .cloned()?; let mut window_loc = space.element_location(&window)?; let geometry = window.geometry(); - let new_loc: Point, Logical> = with_states(&window.wl_surface()?, |states| { + let new_loc: Point, Logical> = with_states(window.wl_surface().as_deref()?, |states| { let data = states.data_map.get::>()?.borrow_mut(); if let ResizeState::Resizing(resize_data) = data.resize_state { diff --git a/anvil/src/state.rs b/anvil/src/state.rs index 4fb3ea7155..444b9cdda5 100644 --- a/anvil/src/state.rs +++ b/anvil/src/state.rs @@ -305,7 +305,7 @@ impl InputMethodHandler for AnvilState { fn parent_geometry(&self, parent: &WlSurface) -> Rectangle { self.space .elements() - .find_map(|window| (window.wl_surface().as_ref() == Some(parent)).then(|| window.geometry())) + .find_map(|window| (window.wl_surface().as_deref() == Some(parent)).then(|| window.geometry())) .unwrap_or_default() } } @@ -334,7 +334,10 @@ delegate_relative_pointer!(@ AnvilState PointerConstraintsHandler for AnvilState { fn new_constraint(&mut self, surface: &WlSurface, pointer: &PointerHandle) { // XXX region - if pointer.current_focus().and_then(|x| x.wl_surface()).as_ref() == Some(surface) { + let Some(current_focus) = pointer.current_focus() else { + return; + }; + if current_focus.wl_surface().as_deref() == Some(surface) { with_pointer_constraint(surface, pointer, |constraint| { constraint.unwrap().activate(); }); @@ -374,7 +377,7 @@ impl XdgActivationHandler for AnvilState { let w = self .space .elements() - .find(|window| window.wl_surface().map(|s| s == surface).unwrap_or(false)) + .find(|window| window.wl_surface().map(|s| *s == surface).unwrap_or(false)) .cloned(); if let Some(window) = w { self.space.raise_element(&window, true); @@ -515,7 +518,7 @@ impl XWaylandKeyboardGrabHandler for AnvilState< let elem = self .space .elements() - .find(|elem| elem.wl_surface().as_ref() == Some(surface))?; + .find(|elem| elem.wl_surface().as_deref() == Some(surface))?; Some(KeyboardFocusTarget::Window(elem.0.clone())) } } diff --git a/src/desktop/space/wayland/window.rs b/src/desktop/space/wayland/window.rs index 97c7e309a8..0390aaa588 100644 --- a/src/desktop/space/wayland/window.rs +++ b/src/desktop/space/wayland/window.rs @@ -56,9 +56,9 @@ impl SpaceElement for Window { state.borrow_mut().output_overlap.retain(|weak, _| weak != output); } - if let Some(surface) = &self.wl_surface() { - output_update(output, None, surface); - for (popup, _) in PopupManager::popups_for_surface(surface) { + if let Some(surface) = self.wl_surface() { + output_update(output, None, &surface); + for (popup, _) in PopupManager::popups_for_surface(&surface) { output_update(output, None, popup.wl_surface()); } } @@ -69,11 +69,11 @@ impl SpaceElement for Window { self.user_data().insert_if_missing(WindowOutputUserData::default); let state = self.user_data().get::().unwrap().borrow(); - if let Some(surface) = &self.wl_surface() { + if let Some(surface) = self.wl_surface() { for (weak, overlap) in state.output_overlap.iter() { if let Some(output) = weak.upgrade() { - output_update(&output, Some(*overlap), surface); - for (popup, location) in PopupManager::popups_for_surface(surface) { + output_update(&output, Some(*overlap), &surface); + for (popup, location) in PopupManager::popups_for_surface(&surface) { let mut overlap = *overlap; overlap.loc -= location; output_update(&output, Some(overlap), popup.wl_surface()); diff --git a/src/desktop/space/wayland/x11.rs b/src/desktop/space/wayland/x11.rs index effbdc9e0f..2f1708a9df 100644 --- a/src/desktop/space/wayland/x11.rs +++ b/src/desktop/space/wayland/x11.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use wayland_server::protocol::wl_surface::WlSurface; use crate::{ @@ -18,8 +20,8 @@ use super::{output_update, WindowOutputUserData}; impl WaylandFocus for X11Surface { #[inline] - fn wl_surface(&self) -> Option { - self.state.lock().unwrap().wl_surface.clone() + fn wl_surface(&self) -> Option> { + self.state.lock().unwrap().wl_surface.clone().map(Cow::Owned) } } diff --git a/src/desktop/wayland/layer.rs b/src/desktop/wayland/layer.rs index 6c8d517e98..5c04d650d1 100644 --- a/src/desktop/wayland/layer.rs +++ b/src/desktop/wayland/layer.rs @@ -18,6 +18,7 @@ use wayland_protocols::wp::presentation_time::server::wp_presentation_feedback; use wayland_server::protocol::wl_surface::{self, WlSurface}; use std::{ + borrow::Cow, cell::{RefCell, RefMut}, hash::{Hash, Hasher}, sync::Arc, @@ -711,7 +712,7 @@ impl LayerSurface { impl WaylandFocus for LayerSurface { #[inline] - fn wl_surface(&self) -> Option { - Some(self.0.surface.wl_surface().clone()) + fn wl_surface(&self) -> Option> { + Some(Cow::Borrowed(self.0.surface.wl_surface())) } } diff --git a/src/desktop/wayland/popup/grab.rs b/src/desktop/wayland/popup/grab.rs index edd19dfc3e..e11c9a0f08 100644 --- a/src/desktop/wayland/popup/grab.rs +++ b/src/desktop/wayland/popup/grab.rs @@ -355,7 +355,7 @@ where let root_surface = self.root.wl_surface()?; self.toplevel_grab .ungrab(&root_surface, strategy) - .or(Some(root_surface)) + .or(Some(root_surface.into_owned())) } /// Convenience method for getting a [`KeyboardGrabStartData`] for this grab. diff --git a/src/desktop/wayland/popup/manager.rs b/src/desktop/wayland/popup/manager.rs index 75b46251fb..ef62a90822 100644 --- a/src/desktop/wayland/popup/manager.rs +++ b/src/desktop/wayland/popup/manager.rs @@ -68,7 +68,10 @@ impl PopupManager { ::PointerFocus: From<::KeyboardFocus> + WaylandFocus, { let surface = popup.wl_surface(); - assert_eq!(root.wl_surface(), Some(find_popup_root_surface(&popup)?)); + assert_eq!( + root.wl_surface().as_deref(), + Some(&find_popup_root_surface(&popup)?) + ); match popup { PopupKind::Xdg(ref xdg) => { diff --git a/src/desktop/wayland/window.rs b/src/desktop/wayland/window.rs index dfa3883f50..25752676fe 100644 --- a/src/desktop/wayland/window.rs +++ b/src/desktop/wayland/window.rs @@ -12,6 +12,7 @@ use crate::{ }, }; use std::{ + borrow::Cow, hash::{Hash, Hasher}, sync::{ atomic::{AtomicU8, Ordering}, @@ -152,9 +153,10 @@ impl Window { /// Returns the geometry of this window. pub fn geometry(&self) -> Rectangle { let bbox = self.bbox(); - if let Some(surface) = &self.wl_surface() { + + if let Some(surface) = self.wl_surface() { // It's the set geometry clamped to the bounding box with the full bounding box as the fallback. - with_states(surface, |states| { + with_states(&surface, |states| { states .cached_state .current::() @@ -182,8 +184,8 @@ impl Window { /// will not include the popups. pub fn bbox_with_popups(&self) -> Rectangle { let mut bounding_box = self.bbox(); - if let Some(surface) = &self.wl_surface() { - for (popup, location) in PopupManager::popups_for_surface(surface) { + if let Some(surface) = self.wl_surface() { + for (popup, location) in PopupManager::popups_for_surface(&surface) { let surface = popup.wl_surface(); let offset = self.geometry().loc + location - popup.geometry().loc; bounding_box = bounding_box.merge(bbox_from_surface_tree(surface, offset)); @@ -229,9 +231,9 @@ impl Window { F: FnMut(&wl_surface::WlSurface, &SurfaceData) -> Option + Copy, { let time = time.into(); - if let Some(surface) = &self.wl_surface() { - send_frames_surface_tree(surface, output, time, throttle, primary_scan_out_output); - for (popup, _) in PopupManager::popups_for_surface(surface) { + if let Some(surface) = self.wl_surface() { + send_frames_surface_tree(&surface, output, time, throttle, primary_scan_out_output); + for (popup, _) in PopupManager::popups_for_surface(&surface) { let surface = popup.wl_surface(); send_frames_surface_tree(surface, output, time, throttle, primary_scan_out_output); } @@ -250,14 +252,14 @@ impl Window { P: FnMut(&wl_surface::WlSurface, &SurfaceData) -> Option + Copy, F: Fn(&wl_surface::WlSurface, &SurfaceData) -> &'a DmabufFeedback + Copy, { - if let Some(surface) = &self.wl_surface() { + if let Some(surface) = self.wl_surface() { send_dmabuf_feedback_surface_tree( - surface, + &surface, output, primary_scan_out_output, select_dmabuf_feedback, ); - for (popup, _) in PopupManager::popups_for_surface(surface) { + for (popup, _) in PopupManager::popups_for_surface(&surface) { let surface = popup.wl_surface(); send_dmabuf_feedback_surface_tree( surface, @@ -281,14 +283,14 @@ impl Window { F1: FnMut(&wl_surface::WlSurface, &SurfaceData) -> Option + Copy, F2: FnMut(&wl_surface::WlSurface, &SurfaceData) -> wp_presentation_feedback::Kind + Copy, { - if let Some(surface) = &self.wl_surface() { + if let Some(surface) = self.wl_surface() { take_presentation_feedback_surface_tree( - surface, + &surface, output_feedback, primary_scan_out_output, presentation_feedback_flags, ); - for (popup, _) in PopupManager::popups_for_surface(surface) { + for (popup, _) in PopupManager::popups_for_surface(&surface) { let surface = popup.wl_surface(); take_presentation_feedback_surface_tree( surface, @@ -305,9 +307,9 @@ impl Window { where F: FnMut(&wl_surface::WlSurface, &SurfaceData), { - if let Some(surface) = &self.wl_surface() { - with_surfaces_surface_tree(surface, &mut processor); - for (popup, _) in PopupManager::popups_for_surface(surface) { + if let Some(surface) = self.wl_surface() { + with_surfaces_surface_tree(&surface, &mut processor); + for (popup, _) in PopupManager::popups_for_surface(&surface) { let surface = popup.wl_surface(); with_surfaces_surface_tree(surface, &mut processor); } @@ -319,8 +321,8 @@ impl Window { /// Needs to be called whenever the toplevel surface or any unsynchronized subsurfaces of this window are updated /// to correctly update the bounding box of this window. pub fn on_commit(&self) { - if let Some(surface) = &self.wl_surface() { - *self.0.bbox.lock().unwrap() = bbox_from_surface_tree(surface, (0, 0)); + if let Some(surface) = self.wl_surface() { + *self.0.bbox.lock().unwrap() = bbox_from_surface_tree(&surface, (0, 0)); } } @@ -336,9 +338,9 @@ impl Window { surface_type: WindowSurfaceType, ) -> Option<(wl_surface::WlSurface, Point)> { let point = point.into(); - if let Some(surface) = &self.wl_surface() { + if let Some(surface) = self.wl_surface() { if surface_type.contains(WindowSurfaceType::POPUP) { - for (popup, location) in PopupManager::popups_for_surface(surface) { + for (popup, location) in PopupManager::popups_for_surface(&surface) { let offset = self.geometry().loc + location - popup.geometry().loc; if let Some(result) = under_from_surface_tree(popup.wl_surface(), point, offset, surface_type) @@ -349,7 +351,7 @@ impl Window { } if surface_type.contains(WindowSurfaceType::TOPLEVEL) { - return under_from_surface_tree(surface, point, (0, 0), surface_type); + return under_from_surface_tree(&surface, point, (0, 0), surface_type); } } @@ -394,11 +396,11 @@ impl Window { impl WaylandFocus for Window { #[inline] - fn wl_surface(&self) -> Option { + fn wl_surface(&self) -> Option> { match &self.0.surface { - WindowSurface::Wayland(s) => Some(s.wl_surface().clone()), + WindowSurface::Wayland(s) => Some(Cow::Borrowed(s.wl_surface())), #[cfg(feature = "xwayland")] - WindowSurface::X11(s) => s.wl_surface(), + WindowSurface::X11(s) => s.wl_surface().map(Cow::Owned), } } } diff --git a/src/wayland/cursor_shape.rs b/src/wayland/cursor_shape.rs index 55fea45f63..52da9b9a00 100644 --- a/src/wayland/cursor_shape.rs +++ b/src/wayland/cursor_shape.rs @@ -33,7 +33,7 @@ //! # fn alive(&self) -> bool { true } //! # } //! # impl WaylandFocus for Target { -//! # fn wl_surface(&self) -> Option { +//! # fn wl_surface(&self) -> Option> { //! # None //! # } //! # } diff --git a/src/wayland/seat/mod.rs b/src/wayland/seat/mod.rs index d35afcb01b..799bc6d8ba 100644 --- a/src/wayland/seat/mod.rs +++ b/src/wayland/seat/mod.rs @@ -60,7 +60,7 @@ pub(crate) mod keyboard; pub(crate) mod pointer; mod touch; -use std::{fmt, sync::Arc}; +use std::{borrow::Cow, fmt, sync::Arc}; use crate::input::{Inner, Seat, SeatHandler, SeatRc, SeatState}; @@ -88,7 +88,7 @@ pub trait WaylandFocus { /// /// *Note*: This has to return `Some`, if `same_client_as` can return true /// for any provided `ObjectId` - fn wl_surface(&self) -> Option; + fn wl_surface(&self) -> Option>; /// Returns true, if the underlying wayland object originates from /// the same client connection as the provided `ObjectId`. /// @@ -102,8 +102,8 @@ pub trait WaylandFocus { impl WaylandFocus for wl_surface::WlSurface { #[inline] - fn wl_surface(&self) -> Option { - Some(self.clone()) + fn wl_surface(&self) -> Option> { + Some(Cow::Borrowed(self)) } } diff --git a/src/wayland/selection/data_device/dnd_grab.rs b/src/wayland/selection/data_device/dnd_grab.rs index bf2d51c968..bab387d56d 100644 --- a/src/wayland/selection/data_device/dnd_grab.rs +++ b/src/wayland/selection/data_device/dnd_grab.rs @@ -110,7 +110,7 @@ where .get::>>() .unwrap() .borrow_mut(); - if focus.as_ref().and_then(|(s, _)| s.wl_surface()) != self.current_focus.clone() { + if focus.as_ref().and_then(|(s, _)| s.wl_surface()).as_deref() != self.current_focus.as_ref() { // focus changed, we need to make a leave if appropriate if let Some(surface) = self.current_focus.take() { // only leave if there is a data source or we are on the original client @@ -191,7 +191,7 @@ where } } } - self.current_focus = Some(surface); + self.current_focus = Some(surface.into_owned()); } else { // make a move if self.data_source.is_some() || self.origin.id().same_client_as(&surface.id()) { diff --git a/src/wayland/selection/data_device/server_dnd_grab.rs b/src/wayland/selection/data_device/server_dnd_grab.rs index f3cec23976..7ae88caa80 100644 --- a/src/wayland/selection/data_device/server_dnd_grab.rs +++ b/src/wayland/selection/data_device/server_dnd_grab.rs @@ -101,7 +101,7 @@ where .get::>>() .unwrap() .borrow_mut(); - if focus.as_ref().and_then(|(s, _)| s.wl_surface()) != self.current_focus.clone() { + if focus.as_ref().and_then(|(s, _)| s.wl_surface()).as_deref() != self.current_focus.as_ref() { // focus changed, we need to make a leave if appropriate if let Some(surface) = self.current_focus.take() { for device in seat_data.known_data_devices() { @@ -168,7 +168,7 @@ where self.pending_offers.push(offer); } self.offer_data = Some(offer_data); - self.current_focus = Some(surface); + self.current_focus = Some(surface.into_owned()); } else { // make a move for device in seat_data.known_data_devices() {