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.

9350792
fix(animation): added pending cancel count to track `is_cancelled` state

84ad947
refactor(animation): remove cancel idx decreasing

804b038
refactor(animation): remove `ANIMATION_TEMPORARILY_DISABLED` global vars

f257873
fix(animation): extend cancelling system to support multiple cancel call

dfd6e98
refactor(window): reuse window rect in `animate_position` method

18522db
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

@@ -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);
             }
         }
 

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,17 +46,44 @@ 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;
         }
     }
 
     pub fn start(&mut self, hwnd: isize) {
         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);
+            }
         }
     }
 }

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]

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

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)
         }

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(())
     }