From 3c035287502ca419b7ba56a7d8c37a19cd0bfd92 Mon Sep 17 00:00:00 2001 From: thearturca Date: Tue, 23 Jul 2024 12:20:56 +0300 Subject: [PATCH] fix(animation): enable cross-monitor animations This commit is a squashed combination of the following commits from #920 by @thearturca. Thanks to both @thearturca for @amnweb for their work in fixing and thoroughly testing these changes respectively. 935079281a79bacdb8777c0a4606d221329dccc6 fix(animation): added pending cancel count to track `is_cancelled` state 84ad947e1fb773bf23d770608a72aa26abdbb0b7 refactor(animation): remove cancel idx decreasing 804b0380f709da94d8ec4ae9918dac7db0fd3590 refactor(animation): remove `ANIMATION_TEMPORARILY_DISABLED` global vars f25787393c6ecd7f48fade369f58fb671e1205b3 fix(animation): extend cancelling system to support multiple cancel call dfd6e98e9c0d077b871aaff8175d80aa024eca3c refactor(window): reuse window rect in `animate_position` method 18522db9023f33b3ed67f004f60d44e342f4bed9 fix(animations): change check for existings animation to `pending_cancel_count` field Before it was checking `cancel_idx_counter` which is `id` counter. It never gonna equals `0` and doesn't represent all animations that running for that window. So it doesn't delete entry from hashmap. That leads to bug when border and stackbar doesn't get notified after animation ends. --- komorebi/src/animation.rs | 30 +++++++++++++++------ komorebi/src/animation_manager.rs | 43 ++++++++++++++++++++++++++----- komorebi/src/lib.rs | 1 - komorebi/src/process_event.rs | 5 ---- komorebi/src/window.rs | 21 +++++++-------- komorebi/src/window_manager.rs | 18 ------------- 6 files changed, 67 insertions(+), 51 deletions(-) diff --git a/komorebi/src/animation.rs b/komorebi/src/animation.rs index 3ebdd00e..c0461d8b 100644 --- a/komorebi/src/animation.rs +++ b/komorebi/src/animation.rs @@ -416,12 +416,15 @@ impl Animation { pub fn new(hwnd: isize) -> Self { Self { hwnd } } - pub fn cancel(&mut self) { + + /// Returns true if the animation needs to continue + pub fn cancel(&mut self) -> bool { if !ANIMATION_MANAGER.lock().in_progress(self.hwnd) { - return; + return true; } - ANIMATION_MANAGER.lock().cancel(self.hwnd); + // should be more than 0 + let cancel_idx = ANIMATION_MANAGER.lock().init_cancel(self.hwnd); let max_duration = Duration::from_secs(1); let spent_duration = Instant::now(); @@ -434,6 +437,12 @@ impl Animation { ANIMATION_DURATION.load(Ordering::SeqCst) / 2, )); } + + let latest_cancel_idx = ANIMATION_MANAGER.lock().latest_cancel_idx(self.hwnd); + + ANIMATION_MANAGER.lock().end_cancel(self.hwnd); + + latest_cancel_idx == cancel_idx } #[allow(clippy::cast_possible_truncation)] @@ -460,7 +469,11 @@ impl Animation { mut render_callback: impl FnMut(f64) -> Result<()>, ) -> Result<()> { if ANIMATION_MANAGER.lock().in_progress(self.hwnd) { - self.cancel(); + let should_animate = self.cancel(); + + if !should_animate { + return Ok(()); + } } ANIMATION_MANAGER.lock().start(self.hwnd); @@ -474,8 +487,7 @@ impl Animation { // check if animation is cancelled if ANIMATION_MANAGER.lock().is_cancelled(self.hwnd) { // cancel animation - // set all flags - ANIMATION_MANAGER.lock().end(self.hwnd); + ANIMATION_MANAGER.lock().cancel(self.hwnd); return Ok(()); } @@ -485,8 +497,10 @@ impl Animation { render_callback(progress).ok(); // sleep until next frame - if frame_start.elapsed() < target_frame_time { - std::thread::sleep(target_frame_time - frame_start.elapsed()); + let frame_time_elapsed = frame_start.elapsed(); + + if frame_time_elapsed < target_frame_time { + std::thread::sleep(target_frame_time - frame_time_elapsed); } } diff --git a/komorebi/src/animation_manager.rs b/komorebi/src/animation_manager.rs index 2fd3f746..ce0308cd 100644 --- a/komorebi/src/animation_manager.rs +++ b/komorebi/src/animation_manager.rs @@ -8,7 +8,8 @@ pub static ANIMATIONS_IN_PROGRESS: AtomicUsize = AtomicUsize::new(0); #[derive(Debug, Clone, Copy)] struct AnimationState { pub in_progress: bool, - pub is_cancelled: bool, + pub cancel_idx_counter: usize, + pub pending_cancel_count: usize, } #[derive(Debug)] @@ -31,7 +32,7 @@ impl AnimationManager { pub fn is_cancelled(&self, hwnd: isize) -> bool { if let Some(animation_state) = self.animations.get(&hwnd) { - animation_state.is_cancelled + animation_state.pending_cancel_count > 0 } else { false } @@ -45,9 +46,35 @@ impl AnimationManager { } } + pub fn init_cancel(&mut self, hwnd: isize) -> usize { + if let Some(animation_state) = self.animations.get_mut(&hwnd) { + animation_state.pending_cancel_count += 1; + animation_state.cancel_idx_counter += 1; + + // return cancel idx + animation_state.cancel_idx_counter + } else { + 0 + } + } + + pub fn latest_cancel_idx(&mut self, hwnd: isize) -> usize { + if let Some(animation_state) = self.animations.get_mut(&hwnd) { + animation_state.cancel_idx_counter + } else { + 0 + } + } + + pub fn end_cancel(&mut self, hwnd: isize) { + if let Some(animation_state) = self.animations.get_mut(&hwnd) { + animation_state.pending_cancel_count -= 1; + } + } + pub fn cancel(&mut self, hwnd: isize) { if let Some(animation_state) = self.animations.get_mut(&hwnd) { - animation_state.is_cancelled = true; + animation_state.in_progress = false; } } @@ -55,7 +82,8 @@ impl AnimationManager { if let Entry::Vacant(e) = self.animations.entry(hwnd) { e.insert(AnimationState { in_progress: true, - is_cancelled: false, + cancel_idx_counter: 0, + pending_cancel_count: 0, }); ANIMATIONS_IN_PROGRESS.store(self.animations.len(), Ordering::Release); @@ -70,10 +98,11 @@ impl AnimationManager { pub fn end(&mut self, hwnd: isize) { if let Some(animation_state) = self.animations.get_mut(&hwnd) { animation_state.in_progress = false; - animation_state.is_cancelled = false; - self.animations.remove(&hwnd); - ANIMATIONS_IN_PROGRESS.store(self.animations.len(), Ordering::Release); + if animation_state.pending_cancel_count == 0 { + self.animations.remove(&hwnd); + ANIMATIONS_IN_PROGRESS.store(self.animations.len(), Ordering::Release); + } } } } diff --git a/komorebi/src/lib.rs b/komorebi/src/lib.rs index e22bd084..5000fd81 100644 --- a/komorebi/src/lib.rs +++ b/komorebi/src/lib.rs @@ -225,7 +225,6 @@ pub static SESSION_ID: AtomicU32 = AtomicU32::new(0); pub static REMOVE_TITLEBARS: AtomicBool = AtomicBool::new(false); pub static ANIMATION_ENABLED: AtomicBool = AtomicBool::new(false); -pub static ANIMATION_TEMPORARILY_DISABLED: AtomicBool = AtomicBool::new(false); pub static ANIMATION_DURATION: AtomicU64 = AtomicU64::new(250); #[must_use] diff --git a/komorebi/src/process_event.rs b/komorebi/src/process_event.rs index c0479f98..9750d01e 100644 --- a/komorebi/src/process_event.rs +++ b/komorebi/src/process_event.rs @@ -32,7 +32,6 @@ use crate::workspace_reconciliator::ALT_TAB_HWND; use crate::workspace_reconciliator::ALT_TAB_HWND_INSTANT; use crate::Notification; use crate::NotificationEvent; -use crate::ANIMATION_TEMPORARILY_DISABLED; use crate::DATA_DIR; use crate::HIDDEN_HWNDS; use crate::REGEX_IDENTIFIERS; @@ -478,8 +477,6 @@ impl WindowManager { origin_container_idx, )) = pending { - ANIMATION_TEMPORARILY_DISABLED.store(true, Ordering::SeqCst); - let target_workspace_idx = self .monitors() .get(target_monitor_idx) @@ -521,8 +518,6 @@ impl WindowManager { self.focus_monitor(target_monitor_idx)?; self.focus_workspace(target_workspace_idx)?; self.update_focused_workspace(false, false)?; - - ANIMATION_TEMPORARILY_DISABLED.store(false, Ordering::SeqCst); } // Here we handle a simple move on the same monitor which is treated as // a container swap diff --git a/komorebi/src/window.rs b/komorebi/src/window.rs index 9432e7a4..4441f991 100644 --- a/komorebi/src/window.rs +++ b/komorebi/src/window.rs @@ -5,7 +5,6 @@ use crate::stackbar_manager; use crate::ANIMATIONS_IN_PROGRESS; use crate::ANIMATION_DURATION; use crate::ANIMATION_ENABLED; -use crate::ANIMATION_TEMPORARILY_DISABLED; use std::collections::HashMap; use std::convert::TryFrom; use std::fmt::Display; @@ -172,11 +171,10 @@ impl Window { ) } - pub fn animate_position(&self, layout: &Rect, top: bool) -> Result<()> { + pub fn animate_position(&self, start_rect: &Rect, target_rect: &Rect, top: bool) -> Result<()> { let hwnd = self.hwnd(); - let curr_rect = WindowsApi::window_rect(hwnd).unwrap(); - - let target_rect = *layout; + let start_rect = *start_rect; + let target_rect = *target_rect; let duration = Duration::from_millis(ANIMATION_DURATION.load(Ordering::SeqCst)); let mut animation = self.animation; @@ -188,7 +186,7 @@ impl Window { std::thread::spawn(move || { animation.animate(duration, |progress: f64| { - let new_rect = Animation::lerp_rect(&curr_rect, &target_rect, progress); + let new_rect = Animation::lerp_rect(&start_rect, &target_rect, progress); if progress == 1.0 { WindowsApi::position_window(hwnd, &new_rect, top)?; @@ -209,7 +207,6 @@ impl Window { // using MoveWindow because it runs faster than SetWindowPos // so animation have more fps and feel smoother WindowsApi::move_window(hwnd, &new_rect, false)?; - // WindowsApi::position_window(hwnd, &new_rect, top)?; WindowsApi::invalidate_rect(hwnd, None, false); } @@ -221,14 +218,14 @@ impl Window { } pub fn set_position(&self, layout: &Rect, top: bool) -> Result<()> { - if WindowsApi::window_rect(self.hwnd())?.eq(layout) { + let window_rect = WindowsApi::window_rect(self.hwnd())?; + + if window_rect.eq(layout) { return Ok(()); } - if ANIMATION_ENABLED.load(Ordering::SeqCst) - && !ANIMATION_TEMPORARILY_DISABLED.load(Ordering::SeqCst) - { - self.animate_position(layout, top) + if ANIMATION_ENABLED.load(Ordering::SeqCst) { + self.animate_position(&window_rect, layout, top) } else { WindowsApi::position_window(self.hwnd(), layout, top) } diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index c42a8b3f..d05d3f59 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -67,7 +67,6 @@ use crate::BorderColours; use crate::Colour; use crate::Rgb; use crate::WorkspaceRule; -use crate::ANIMATION_TEMPORARILY_DISABLED; use crate::CUSTOM_FFM; use crate::DATA_DIR; use crate::DISPLAY_INDEX_PREFERENCES; @@ -1113,7 +1112,6 @@ impl WindowManager { follow: bool, ) -> Result<()> { self.handle_unmanaged_window_behaviour()?; - ANIMATION_TEMPORARILY_DISABLED.store(true, Ordering::SeqCst); tracing::info!("moving container"); @@ -1185,15 +1183,12 @@ impl WindowManager { self.update_focused_workspace(self.mouse_follows_focus, true)?; - ANIMATION_TEMPORARILY_DISABLED.store(false, Ordering::SeqCst); - Ok(()) } #[tracing::instrument(skip(self))] pub fn move_container_to_workspace(&mut self, idx: usize, follow: bool) -> Result<()> { self.handle_unmanaged_window_behaviour()?; - ANIMATION_TEMPORARILY_DISABLED.store(true, Ordering::SeqCst); tracing::info!("moving container"); @@ -1207,8 +1202,6 @@ impl WindowManager { self.update_focused_workspace(mouse_follows_focus, true)?; - ANIMATION_TEMPORARILY_DISABLED.store(false, Ordering::SeqCst); - Ok(()) } @@ -1312,13 +1305,6 @@ impl WindowManager { let origin_monitor_idx = self.focused_monitor_idx(); let target_container_idx = workspace.new_idx_for_direction(direction); - let animation_temporarily_disabled = if target_container_idx.is_none() { - ANIMATION_TEMPORARILY_DISABLED.store(true, Ordering::SeqCst); - true - } else { - false - }; - match target_container_idx { // If there is nowhere to move on the current workspace, try to move it onto the monitor // in that direction if there is one @@ -1445,10 +1431,6 @@ impl WindowManager { self.update_focused_workspace(self.mouse_follows_focus, true)?; - if animation_temporarily_disabled { - ANIMATION_TEMPORARILY_DISABLED.store(false, Ordering::SeqCst); - } - Ok(()) }