seat/pointer: simplify ThemedPointer handling

kchibisov · kchibisov · commit 8cecd26ed60d · 2023-05-05T20:14:23.000+03:00
This should simplify the handling of custom cursors in the user
applications by incapsulating all the necessary state in the
ThemedPointer.

In the future we can automatically handle the `wp_cursor_shape`
under the hood.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+#### Breaking Changes
+
+- `ThemedPointer::set_cursor` now takes only `Connection` and `&str`.
+- `SeatState:get_pointer_with_them*` now takes `Shm` and `WlSurface` for the themed cursor.
+- `ThemedPointer` now automatically releases the associated `WlPointer`.
+
 ## 0.17.0 - 2023-03-28
 
 #### Breaking Changes
diff --git a/examples/themed_window.rs b/examples/themed_window.rs
@@ -60,7 +60,6 @@ fn main() {
         .expect("Failed to create pool");
 
     let window_surface = compositor_state.create_surface(&qh);
-    let pointer_surface = compositor_state.create_surface(&qh);
 
     let window =
         xdg_shell_state.create_window(window_surface, WindowDecorations::ServerDefault, &qh);
@@ -80,7 +79,7 @@ fn main() {
         registry_state,
         seat_state,
         output_state,
-        _compositor_state: compositor_state,
+        compositor_state,
         subcompositor_state: Arc::new(subcompositor_state),
         shm_state,
         _xdg_shell_state: xdg_shell_state,
@@ -94,7 +93,6 @@ fn main() {
         buffer: None,
         window,
         window_frame: None,
-        pointer_surface,
         keyboard: None,
         keyboard_focus: false,
         themed_pointer: None,
@@ -118,7 +116,7 @@ struct SimpleWindow {
     registry_state: RegistryState,
     seat_state: SeatState,
     output_state: OutputState,
-    _compositor_state: CompositorState,
+    compositor_state: CompositorState,
     subcompositor_state: Arc<SubcompositorState>,
     shm_state: Shm,
     _xdg_shell_state: XdgShell,
@@ -132,7 +130,6 @@ struct SimpleWindow {
     buffer: Option<Buffer>,
     window: Window,
     window_frame: Option<FallbackFrame<Self>>,
-    pointer_surface: wl_surface::WlSurface,
     keyboard: Option<wl_keyboard::WlKeyboard>,
     keyboard_focus: bool,
     themed_pointer: Option<ThemedPointer>,
@@ -312,9 +309,16 @@ impl SeatHandler for SimpleWindow {
         if capability == Capability::Pointer && self.themed_pointer.is_none() {
             println!("Set pointer capability");
             println!("Creating pointer theme");
+            let surface = self.compositor_state.create_surface(qh);
             let themed_pointer = self
                 .seat_state
-                .get_pointer_with_theme(qh, &seat, ThemeSpec::default())
+                .get_pointer_with_theme(
+                    qh,
+                    &seat,
+                    self.shm_state.wl_shm(),
+                    surface,
+                    ThemeSpec::default(),
+                )
                 .expect("Failed to create pointer");
             self.themed_pointer.replace(themed_pointer);
         }
@@ -504,13 +508,7 @@ impl ShmHandler for SimpleWindow {
 impl SimpleWindow {
     pub fn draw(&mut self, conn: &Connection, qh: &QueueHandle<Self>) {
         if self.set_cursor {
-            let _ = self.themed_pointer.as_mut().unwrap().set_cursor(
-                conn,
-                &self.cursor_icon,
-                self.shm_state.wl_shm(),
-                &self.pointer_surface,
-                1,
-            );
+            let _ = self.themed_pointer.as_mut().unwrap().set_cursor(conn, &self.cursor_icon);
             self.set_cursor = false;
         }
 
diff --git a/src/seat/mod.rs b/src/seat/mod.rs
@@ -15,11 +15,14 @@ use std::{
 
 use wayland_client::{
     globals::GlobalList,
-    protocol::{wl_pointer, wl_seat, wl_touch},
+    protocol::{wl_pointer, wl_seat, wl_shm, wl_surface, wl_touch},
     Connection, Dispatch, Proxy, QueueHandle,
 };
 
-use crate::registry::{ProvidesRegistryState, RegistryHandler};
+use crate::{
+    compositor::SurfaceDataExt,
+    registry::{ProvidesRegistryState, RegistryHandler},
+};
 
 use self::{
     pointer::{PointerData, PointerDataExt, PointerHandler, ThemeSpec, ThemedPointer, Themes},
@@ -130,16 +133,29 @@ impl SeatState {
     /// ## Errors
     ///
     /// This will return [`SeatError::UnsupportedCapability`] if the seat does not support a pointer.
-    pub fn get_pointer_with_theme<D>(
+    pub fn get_pointer_with_theme<D, S>(
         &mut self,
         qh: &QueueHandle<D>,
         seat: &wl_seat::WlSeat,
+        shm: &wl_shm::WlShm,
+        surface: wl_surface::WlSurface,
         theme: ThemeSpec,
     ) -> Result<ThemedPointer<PointerData>, SeatError>
     where
-        D: Dispatch<wl_pointer::WlPointer, PointerData> + PointerHandler + 'static,
+        D: Dispatch<wl_pointer::WlPointer, PointerData>
+            + Dispatch<wl_surface::WlSurface, S>
+            + PointerHandler
+            + 'static,
+        S: SurfaceDataExt + 'static,
     {
-        self.get_pointer_with_theme_and_data(qh, seat, theme, PointerData::new(seat.clone()))
+        self.get_pointer_with_theme_and_data(
+            qh,
+            seat,
+            shm,
+            surface,
+            theme,
+            PointerData::new(seat.clone()),
+        )
     }
 
     /// Creates a pointer from a seat.
@@ -172,15 +188,21 @@ impl SeatState {
     /// ## Errors
     ///
     /// This will return [`SeatError::UnsupportedCapability`] if the seat does not support a pointer.
-    pub fn get_pointer_with_theme_and_data<D, U>(
+    pub fn get_pointer_with_theme_and_data<D, S, U>(
         &mut self,
         qh: &QueueHandle<D>,
         seat: &wl_seat::WlSeat,
+        shm: &wl_shm::WlShm,
+        surface: wl_surface::WlSurface,
         theme: ThemeSpec,
         pointer_data: U,
     ) -> Result<ThemedPointer<U>, SeatError>
     where
-        D: Dispatch<wl_pointer::WlPointer, U> + PointerHandler + 'static,
+        D: Dispatch<wl_pointer::WlPointer, U>
+            + Dispatch<wl_surface::WlSurface, S>
+            + PointerHandler
+            + 'static,
+        S: SurfaceDataExt + 'static,
         U: PointerDataExt + 'static,
     {
         let inner =
@@ -194,7 +216,10 @@ impl SeatState {
         Ok(ThemedPointer {
             themes: Arc::new(Mutex::new(Themes::new(theme))),
             pointer: wl_ptr,
+            shm: shm.clone(),
+            surface,
             _marker: std::marker::PhantomData,
+            _surface_data: std::marker::PhantomData,
         })
     }
 
diff --git a/src/seat/pointer/mod.rs b/src/seat/pointer/mod.rs
@@ -3,15 +3,23 @@ use std::{
     env, mem,
     sync::{Arc, Mutex},
 };
-use wayland_backend::{client::InvalidId, smallvec::SmallVec};
 
+use wayland_backend::{client::InvalidId, smallvec::SmallVec};
 use wayland_client::{
-    protocol::{wl_pointer, wl_seat::WlSeat, wl_shm, wl_surface},
+    protocol::{
+        wl_pointer::{self, WlPointer},
+        wl_seat::WlSeat,
+        wl_shm::WlShm,
+        wl_surface::WlSurface,
+    },
     Connection, Dispatch, Proxy, QueueHandle, WEnum,
 };
 use wayland_cursor::{Cursor, CursorTheme};
 
-use crate::error::GlobalError;
+use crate::{
+    compositor::{SurfaceData, SurfaceDataExt},
+    error::GlobalError,
+};
 
 use super::SeatState;
 
@@ -64,7 +72,7 @@ impl AxisScroll {
 /// A single pointer event.
 #[derive(Debug, Clone)]
 pub struct PointerEvent {
-    pub surface: wl_surface::WlSurface,
+    pub surface: WlSurface,
     pub position: (f64, f64),
     pub kind: PointerEventKind,
 }
@@ -109,7 +117,7 @@ pub trait PointerHandler: Sized {
         &mut self,
         conn: &Connection,
         qh: &QueueHandle<Self>,
-        pointer: &wl_pointer::WlPointer,
+        pointer: &WlPointer,
         events: &[PointerEvent],
     );
 }
@@ -169,7 +177,7 @@ macro_rules! delegate_pointer {
 #[derive(Debug, Default)]
 pub(crate) struct PointerDataInner {
     /// Surface the pointer most recently entered
-    pub(crate) surface: Option<wl_surface::WlSurface>,
+    pub(crate) surface: Option<WlSurface>,
     /// Position relative to the surface
     pub(crate) position: (f64, f64),
 
@@ -180,14 +188,14 @@ pub(crate) struct PointerDataInner {
     pub(crate) latest_enter: Option<u32>,
 }
 
-impl<D, U> Dispatch<wl_pointer::WlPointer, U, D> for SeatState
+impl<D, U> Dispatch<WlPointer, U, D> for SeatState
 where
-    D: Dispatch<wl_pointer::WlPointer, U> + PointerHandler,
+    D: Dispatch<WlPointer, U> + PointerHandler,
     U: PointerDataExt,
 {
     fn event(
         data: &mut D,
-        pointer: &wl_pointer::WlPointer,
+        pointer: &WlPointer,
         event: wl_pointer::Event,
         udata: &U,
         conn: &Connection,
@@ -372,59 +380,77 @@ where
 
 /// Pointer themeing
 #[derive(Debug)]
-pub struct ThemedPointer<U = PointerData> {
+pub struct ThemedPointer<U = PointerData, S = SurfaceData> {
     pub(super) themes: Arc<Mutex<Themes>>,
-    pub(super) pointer: wl_pointer::WlPointer,
+    /// The underlying wl_pointer.
+    pub(super) pointer: WlPointer,
+    pub(super) shm: WlShm,
+    /// The surface owned by the cursor to present the icon.
+    pub(super) surface: WlSurface,
     pub(super) _marker: std::marker::PhantomData<U>,
+    pub(super) _surface_data: std::marker::PhantomData<S>,
 }
 
-impl<U: PointerDataExt + 'static> ThemedPointer<U> {
-    pub fn set_cursor(
-        &self,
-        conn: &Connection,
-        name: &str,
-        shm: &wl_shm::WlShm,
-        surface: &wl_surface::WlSurface,
-        scale: i32,
-    ) -> Result<(), PointerThemeError> {
+impl<U: PointerDataExt + 'static, S: SurfaceDataExt + 'static> ThemedPointer<U, S> {
+    pub fn set_cursor(&self, conn: &Connection, name: &str) -> Result<(), PointerThemeError> {
         let mut themes = self.themes.lock().unwrap();
 
+        let scale = self.surface.data::<S>().unwrap().surface_data().scale_factor();
         let cursor = themes
-            .get_cursor(conn, name, scale as u32, shm)
+            .get_cursor(conn, name, scale as u32, &self.shm)
             .map_err(PointerThemeError::InvalidId)?
             .ok_or(PointerThemeError::CursorNotFound)?;
 
         let image = &cursor[0];
         let (w, h) = image.dimensions();
         let (hx, hy) = image.hotspot();
 
-        surface.set_buffer_scale(scale);
-        surface.attach(Some(image), 0, 0);
+        self.surface.set_buffer_scale(scale);
+        self.surface.attach(Some(image), 0, 0);
 
-        if surface.version() >= 4 {
-            surface.damage_buffer(0, 0, w as i32, h as i32);
+        if self.surface.version() >= 4 {
+            self.surface.damage_buffer(0, 0, w as i32, h as i32);
         } else {
-            // surface is old and does not support damage_buffer, so we damage
-            // in surface coordinates and hope it is not rescaled
-            surface.damage(0, 0, w as i32 / scale, h as i32 / scale);
+            // Fallback for the old old surface.
+            self.surface.damage(0, 0, w as i32 / scale, h as i32 / scale);
         }
 
         // Commit the surface to place the cursor image in the compositor's memory.
-        surface.commit();
+        self.surface.commit();
 
         // Set the pointer surface to change the pointer.
         let data = self.pointer.data::<U>();
         if let Some(serial) = data.and_then(|data| data.pointer_data().latest_enter_serial()) {
-            self.pointer.set_cursor(serial, Some(surface), hx as i32 / scale, hy as i32 / scale);
+            self.pointer.set_cursor(
+                serial,
+                Some(&self.surface),
+                hx as i32 / scale,
+                hy as i32 / scale,
+            );
             Ok(())
         } else {
             Err(PointerThemeError::MissingEnterSerial)
         }
     }
 
-    pub fn pointer(&self) -> &wl_pointer::WlPointer {
+    /// The [`WlPointer`] associated with this [`ThemedPointer`].
+    pub fn pointer(&self) -> &WlPointer {
         &self.pointer
     }
+
+    /// The associated [`WlSurface`] with this [`ThemedPointer`].
+    pub fn surface(&self) -> &WlSurface {
+        &self.surface
+    }
+}
+
+impl<U, S> Drop for ThemedPointer<U, S> {
+    fn drop(&mut self) {
+        if self.pointer.version() >= 3 {
+            self.pointer.release();
+        }
+        self.surface.destroy();
+    }
 }
 
 /// Specifies which cursor theme should be used by the theme manager.
@@ -509,7 +535,7 @@ impl Themes {
         conn: &Connection,
         name: &str,
         scale: u32,
-        shm: &wl_shm::WlShm,
+        shm: &WlShm,
     ) -> Result<Option<&Cursor>, InvalidId> {
         // Check if the theme has been initialized at the specified scale.
         if let Entry::Vacant(e) = self.themes.entry(scale) {
