From a86b49c4ffa9db992a77b1f04dbfa369afb1e71d Mon Sep 17 00:00:00 2001 From: Ottatop Date: Wed, 14 Aug 2024 15:45:03 -0500 Subject: [PATCH] Clean up winit rendering and cursor logic --- src/backend.rs | 23 +++++++++------------ src/backend/udev.rs | 32 +++-------------------------- src/backend/winit.rs | 39 +++++++++-------------------------- src/cursor.rs | 48 ++++++++++++++++++-------------------------- src/state.rs | 1 + 5 files changed, 42 insertions(+), 101 deletions(-) diff --git a/src/backend.rs b/src/backend.rs index cb4606137..83e741cd8 100644 --- a/src/backend.rs +++ b/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::>() - { - 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::>() + { + udev.render_if_scheduled(pinnacle, &output); } - #[cfg(feature = "testing")] - Backend::Dummy(_) => (), } } diff --git a/src/backend/udev.rs b/src/backend/udev.rs index c9d88aeb3..7f022b58e 100644 --- a/src/backend/udev.rs +++ b/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)); diff --git a/src/backend/winit.rs b/src/backend/winit.rs index b8bf3d014..33732ff83 100644 --- a/src/backend/winit.rs +++ b/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), 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::>(); @@ -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() { diff --git a/src/cursor.rs b/src/cursor.rs index 770c94042..d7279091b 100644 --- a/src/cursor.rs +++ b/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>>, } @@ -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 { + 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; diff --git a/src/state.rs b/src/state.rs index b9f1ab047..9693f575f 100644 --- a/src/state.rs +++ b/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);