Clean up winit rendering and cursor logic

src/backend.rs

@@ -113,21 +113,16 @@ impl Backend {
     }
 
     pub fn render_scheduled_outputs(&mut self, pinnacle: &mut Pinnacle) {
-        match self {
-            Backend::Winit(winit) => winit.render_if_scheduled(pinnacle),
-            Backend::Udev(udev) => {
-                for output in pinnacle
-                    .outputs
-                    .iter()
-                    .filter(|(_, global)| global.is_some())
-                    .map(|(op, _)| op.clone())
-                    .collect::<Vec<_>>()
-                {
-                    udev.render_if_scheduled(pinnacle, &output);
-                }
+        if let Backend::Udev(udev) = self {
+            for output in pinnacle
+                .outputs
+                .iter()
+                .filter(|(_, global)| global.is_some())
+                .map(|(op, _)| op.clone())
+                .collect::<Vec<_>>()
+            {
+                udev.render_if_scheduled(pinnacle, &output);
             }
-            #[cfg(feature = "testing")]
-            Backend::Dummy(_) => (),
         }
     }
 

src/backend/udev.rs

@@ -50,7 +50,6 @@ use smithay::{
         SwapBuffersError,
     },
     desktop::utils::OutputPresentationFeedback,
-    input::pointer::CursorImageStatus,
     output::{Output, PhysicalProperties, Subpixel},
     reexports::{
         calloop::{
@@ -71,7 +70,7 @@ use smithay::{
             DisplayHandle,
         },
     },
-    utils::{DeviceFd, IsAlive, Point, Rectangle, Transform},
+    utils::{DeviceFd, Point, Rectangle, Transform},
     wayland::{
         dmabuf::{self, DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufState},
         shm::shm_format_to_fourcc,
@@ -1252,13 +1251,7 @@ impl Udev {
             }
         };
 
-        // TODO: is_animated or unfinished_animations_remain
-        if render_needed
-            || pinnacle
-                .cursor_state
-                .time_until_next_animated_cursor_frame()
-                .is_some()
-        {
+        if render_needed || pinnacle.cursor_state.is_current_cursor_animated() {
             self.schedule_render(&output);
         } else {
             pinnacle.send_frame_callbacks(&output, Some(surface.frame_callback_sequence));
@@ -1320,20 +1313,6 @@ impl Udev {
         let _ = renderer.upscale_filter(self.upscale_filter);
         let _ = renderer.downscale_filter(self.downscale_filter);
 
-        ///////////////////////////////////////////////////////////////////////////////////////////
-
-        // draw the cursor as relevant and
-        // reset the cursor if the surface is no longer alive
-        if let CursorImageStatus::Surface(surface) = &pinnacle.cursor_state.cursor_image() {
-            if !surface.alive() {
-                pinnacle
-                    .cursor_state
-                    .set_cursor_image(CursorImageStatus::default_named());
-            }
-        }
-
-        ///////////////////////////////////////////////////////////////////////////////////////////
-
         let pointer_location = pinnacle
             .seat
             .get_pointer()
@@ -1583,12 +1562,7 @@ impl Udev {
             }
         }
 
-        // TODO: is_animated or unfinished_animations_remain
-        if pinnacle
-            .cursor_state
-            .time_until_next_animated_cursor_frame()
-            .is_some()
-        {
+        if pinnacle.cursor_state.is_current_cursor_animated() {
             self.schedule_render(output);
         } else {
             pinnacle.send_frame_callbacks(output, Some(surface.frame_callback_sequence));

src/backend/winit.rs

@@ -16,7 +16,6 @@ use smithay::{
         },
         winit::{self, WinitEvent, WinitGraphicsBackend},
     },
-    input::pointer::CursorImageStatus,
     output::{Output, Scale, Subpixel},
     reexports::{
         calloop::{self, generic::Generic, Interest, LoopHandle, PostAction},
@@ -30,7 +29,7 @@ use smithay::{
             window::{Icon, WindowAttributes},
         },
     },
-    utils::{IsAlive, Point, Rectangle, Transform},
+    utils::{Point, Rectangle, Transform},
     wayland::dmabuf::{self, DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufState},
 };
 use tracing::{debug, error, trace, warn};
@@ -53,7 +52,6 @@ pub struct Winit {
     pub damage_tracker: OutputDamageTracker,
     pub dmabuf_state: (DmabufState, DmabufGlobal, Option<DmabufFeedback>),
     pub full_redraw: u8,
-    output_render_scheduled: bool,
     output: Output,
 }
 
@@ -173,7 +171,6 @@ impl Winit {
             damage_tracker: OutputDamageTracker::from_output(&output),
             dmabuf_state,
             full_redraw: 0,
-            output_render_scheduled: false,
             output,
         };
 
@@ -222,10 +219,10 @@ impl Winit {
                             state.process_input_event(input_evt);
                         }
                         WinitEvent::Redraw => {
-                            // let winit = state.backend.winit_mut();
-                            // winit.output_render_scheduled = true;
-                            // winit.render_if_scheduled(&mut state.pinnacle);
-                            state.backend.winit_mut().schedule_render();
+                            state
+                                .backend
+                                .winit_mut()
+                                .render_winit_window(&mut state.pinnacle);
                         }
                         WinitEvent::CloseRequested => {
                             state.pinnacle.shutdown();
@@ -244,28 +241,13 @@ impl Winit {
 
     /// Schedule a render on the winit window.
     pub fn schedule_render(&mut self) {
-        self.output_render_scheduled = true;
+        self.backend.window().request_redraw();
     }
 
-    /// Render the winit window if a render has been scheduled.
-    pub fn render_if_scheduled(&mut self, pinnacle: &mut Pinnacle) {
-        if self.output_render_scheduled {
-            self.render_winit_window(pinnacle);
-        }
-    }
-
-    pub(super) fn render_winit_window(&mut self, pinnacle: &mut Pinnacle) {
+    fn render_winit_window(&mut self, pinnacle: &mut Pinnacle) {
         let full_redraw = &mut self.full_redraw;
         *full_redraw = full_redraw.saturating_sub(1);
 
-        if let CursorImageStatus::Surface(surface) = pinnacle.cursor_state.cursor_image() {
-            if !surface.alive() {
-                pinnacle
-                    .cursor_state
-                    .set_cursor_image(CursorImageStatus::default_named());
-            }
-        }
-
         // The z-index of these is determined by `state.fixup_z_layering()`, which is called at the end
         // of every event loop cycle
         let windows = pinnacle.space.elements().cloned().collect::<Vec<_>>();
@@ -433,11 +415,8 @@ impl Winit {
 
         pinnacle.send_frame_callbacks(&self.output, None);
 
-        assert!(self.output_render_scheduled);
-        self.output_render_scheduled = false;
-
         // At the end cuz borrow checker
-        if render_after_transaction_finish {
+        if render_after_transaction_finish || pinnacle.cursor_state.is_current_cursor_animated() {
             self.schedule_render();
         }
     }
@@ -454,7 +433,7 @@ impl Winit {
             return;
         };
 
-        assert!(screencopy.output() == output);
+        assert_eq!(screencopy.output(), output);
 
         if screencopy.with_damage() {
             match render_output_result.damage.as_ref() {

src/cursor.rs

@@ -1,10 +1,11 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 
-use std::time::{Duration, Instant};
+use std::time::Duration;
 use std::{collections::HashMap, rc::Rc};
 
 use anyhow::Context;
 use smithay::backend::allocator::Fourcc;
+use smithay::utils::IsAlive;
 use smithay::{
     backend::renderer::element::memory::MemoryRenderBuffer,
     input::pointer::{CursorIcon, CursorImageStatus},
@@ -20,13 +21,11 @@ use crate::render::pointer::PointerElement;
 static FALLBACK_CURSOR_DATA: &[u8] = include_bytes!("../resources/cursor.rgba");
 
 pub struct CursorState {
-    start_time: Instant,
     current_cursor_image: CursorImageStatus,
     theme: CursorTheme,
     size: u32,
-    // memory buffer cache
     mem_buffer_cache: Vec<(Image, MemoryRenderBuffer)>,
-    // map of cursor icons to loaded images
+    /// A map of cursor icons to loaded images
     loaded_images: HashMap<CursorIcon, Option<Rc<XCursor>>>,
 }
 
@@ -38,7 +37,6 @@ impl CursorState {
         std::env::set_var("XCURSOR_SIZE", size.to_string());
 
         Self {
-            start_time: Instant::now(),
             current_cursor_image: CursorImageStatus::default_named(),
             theme: CursorTheme::load(&theme),
             size,
@@ -130,36 +128,28 @@ impl CursorState {
         }
     }
 
-    // TODO: update render to wait for est vblank, then you can remove this
-    /// If the current cursor is named and animated, get the time to the next frame, in milliseconds.
-    pub fn time_until_next_animated_cursor_frame(&mut self) -> Option<Duration> {
+    pub fn is_current_cursor_animated(&mut self) -> bool {
         match &self.current_cursor_image {
-            CursorImageStatus::Hidden => None,
+            CursorImageStatus::Hidden => false,
             CursorImageStatus::Named(icon) => {
                 let cursor = self
                     .get_xcursor_images(*icon)
                     .or_else(|| self.get_xcursor_images(CursorIcon::Default))
                     .unwrap();
 
-                if cursor.images.len() <= 1 {
-                    return None;
-                }
-
-                let mut millis = self.start_time.elapsed().as_millis() as u32;
-                let animation_length_ms = nearest_size_images(self.size, &cursor.images)
-                    .fold(0, |acc, image| acc + image.delay);
-                millis %= animation_length_ms;
-
-                for img in nearest_size_images(self.size, &cursor.images) {
-                    if millis < img.delay {
-                        return Some(Duration::from_millis((img.delay - millis).into()));
-                    }
-                    millis -= img.delay;
-                }
+                let is_animated = cursor.images.len() > 1;
+                is_animated
+            }
+            CursorImageStatus::Surface(_) => false,
+        }
+    }
 
-                None
+    /// Cleans up the current cursor if it is a dead WlSurface.
+    pub fn cleanup(&mut self) {
+        if let CursorImageStatus::Surface(surface) = &self.current_cursor_image {
+            if !surface.alive() {
+                self.current_cursor_image = CursorImageStatus::default_named();
             }
-            CursorImageStatus::Surface(_) => None,
         }
     }
 }
@@ -171,12 +161,14 @@ pub struct XCursor {
 impl XCursor {
     pub fn image(&self, time: Duration, size: u32) -> Image {
         let mut millis = time.as_millis() as u32;
+
         let animation_length_ms =
             nearest_size_images(size, &self.images).fold(0, |acc, image| acc + image.delay);
-        millis %= animation_length_ms;
+
+        millis = millis.checked_rem(animation_length_ms).unwrap_or_default();
 
         for img in nearest_size_images(size, &self.images) {
-            if millis < img.delay {
+            if millis <= img.delay {
                 return img.clone();
             }
             millis -= img.delay;

src/state.rs

@@ -221,6 +221,7 @@ impl State {
     pub fn on_event_loop_cycle_completion(&mut self) {
         self.pinnacle.fixup_z_layering();
         self.pinnacle.space.refresh();
+        self.pinnacle.cursor_state.cleanup();
         self.pinnacle.popup_manager.cleanup();
         self.update_pointer_focus();
         foreign_toplevel::refresh(self);