diff --git a/komorebi/src/border_manager/border.rs b/komorebi/src/border_manager/border.rs index 7360ee95..dc78903e 100644 --- a/komorebi/src/border_manager/border.rs +++ b/komorebi/src/border_manager/border.rs @@ -79,8 +79,9 @@ use windows_numerics::Matrix3x2; pub const WM_UPDATE_BRUSHES: u32 = WM_USER + 1; /// Custom WM_USER message used to drive the border in lockstep with an active -/// movement animation. lparam carries a `Box` ownership transfer that the -/// receiving WndProc reclaims and applies as the new tracked rect. +/// movement animation. The receiving WndProc takes the newest rect the animation +/// has for it from `border_manager::take_animated_rect` and applies it as the +/// new tracked rect. pub const WM_ANIMATE_RECT: u32 = WM_USER + 2; pub struct RenderFactory(ID2D1Factory); @@ -446,29 +447,6 @@ impl Border { }; } - /// Drive the border to follow `rect` during a movement animation. Hands - /// ownership of a boxed `Rect` to the border's message-loop thread via - /// `WM_ANIMATE_RECT`, which mirrors the redraw path normally driven by - /// `EVENT_OBJECT_LOCATIONCHANGE` on the real source window. - pub fn animate_to(&self, rect: Rect) { - let boxed = Box::new(rect); - let ptr = Box::into_raw(boxed); - let posted = unsafe { - PostMessageW( - Option::from(self.hwnd()), - WM_ANIMATE_RECT, - WPARAM(0), - LPARAM(ptr as isize), - ) - }; - if posted.is_err() { - // Reclaim the box on failure to avoid leaking. - unsafe { - drop(Box::from_raw(ptr)); - } - } - } - pub fn set_position(&self, rect: &Rect, reference_hwnd: isize) -> color_eyre::Result<()> { let mut rect = *rect; rect.add_margin(self.width); @@ -555,8 +533,13 @@ impl Border { LRESULT(0) } WM_ANIMATE_RECT => { - // lparam carries an owned Box from the animation thread. - let rect_box = Box::from_raw(lparam.0 as *mut Rect); + // The newest rect there is, which is all there is to draw + // if the animation sent more than one since the last time. + let Some(rect) = crate::border_manager::take_animated_rect(window.0 as isize) + else { + return LRESULT(0); + }; + let border_pointer: *mut Border = GetWindowLongPtrW(window, GWLP_USERDATA) as _; if border_pointer.is_null() { @@ -567,7 +550,7 @@ impl Border { return LRESULT(0); } - apply_tracked_rect(border_pointer, *rect_box); + apply_tracked_rect(border_pointer, rect); LRESULT(0) } WM_PAINT => { @@ -696,3 +679,16 @@ impl Border { } } } + +/// Tell a border, from any thread, that the animation has a rect for it to +/// draw, which it takes with `border_manager::take_animated_rect`. +pub fn post_animated_rect(border_hwnd: isize) { + let _ = unsafe { + PostMessageW( + Option::from(HWND(border_hwnd as *mut std::ffi::c_void)), + WM_ANIMATE_RECT, + WPARAM(0), + LPARAM(0), + ) + }; +} diff --git a/komorebi/src/border_manager/mod.rs b/komorebi/src/border_manager/mod.rs index 9f527119..07d01e67 100644 --- a/komorebi/src/border_manager/mod.rs +++ b/komorebi/src/border_manager/mod.rs @@ -23,7 +23,6 @@ use parking_lot::Mutex; use serde::Deserialize; use serde::Serialize; use std::collections::HashMap; -use std::collections::HashSet; use std::collections::hash_map::Entry; use std::ops::Deref; use std::sync::Arc; @@ -109,8 +108,14 @@ fn event_rx() -> Receiver { lazy_static! { /// The windows that a ghost is standing in for, which are hidden and not - /// where the ghost is. The animation moves their borders. - static ref GHOSTED: Mutex> = Mutex::new(HashSet::new()); + /// where the ghost is, with the border of each, if it had one when the + /// ghost was made. The animation moves their borders. + static ref GHOSTED: Mutex>> = Mutex::new(HashMap::new()); + + /// The newest rect an animation has for a border, by the handle of the + /// border, for as long as the border has not got round to drawing it. + static ref ANIMATED_RECTS: Mutex> = + Mutex::new(HashMap::new()); } /// Say that a ghost is, or has stopped being, what is seen of a window: from @@ -118,14 +123,16 @@ lazy_static! { /// (see `animate_to`) and not the window, which is not where it is seen. pub fn set_ghosted(hwnd: isize, ghosted: bool) { if ghosted { - GHOSTED.lock().insert(hwnd); - } else { - GHOSTED.lock().remove(&hwnd); + // looked up once here, and not for every frame of the animation + let border = window_border(hwnd).map(|border| border.border_hwnd); + GHOSTED.lock().insert(hwnd, border); + } else if let Some(Some(border)) = GHOSTED.lock().remove(&hwnd) { + ANIMATED_RECTS.lock().remove(&border); } } pub(crate) fn is_ghosted(hwnd: isize) -> bool { - GHOSTED.lock().contains(&hwnd) + GHOSTED.lock().contains_key(&hwnd) } pub fn window_border(hwnd: isize) -> Option { @@ -136,20 +143,55 @@ pub fn window_border(hwnd: isize) -> Option { }) } +/// Say what the newest rect an animation has for a border is, and whether that +/// is the first since the border last drew one, in which case the border has to +/// be told to. +fn queue_animated_rect( + pending: &mut HashMap, + border: isize, + rect: crate::core::Rect, +) -> bool { + pending.insert(border, rect).is_none() +} + /// Drive the border that tracks `source_hwnd` to follow `rect`. No-op when no /// border is registered for the source window. Used by movement animations to /// keep the border visually in sync while the source window is cloaked. +/// +/// A border draws on a thread of its own, and an animation sends it a rect +/// for every frame, which it can take longer to draw than a frame lasts. It is +/// only told when there is a rect to draw, and draws the newest when it gets +/// there, instead of every one it was sent in turn and further and further +/// behind. pub fn animate_to(source_hwnd: isize, rect: crate::core::Rect) { - let border_id = match WINDOWS_BORDERS.lock().get(&source_hwnd).cloned() { - Some(id) => id, - None => return, + let border = { + let mut ghosted = GHOSTED.lock(); + let Some(border) = ghosted.get_mut(&source_hwnd) else { + return; + }; + + // a window that had no border when its ghost was made can get one + if border.is_none() { + *border = window_border(source_hwnd).map(|border| border.border_hwnd); + } + + match *border { + Some(border) => border, + None => return, + } }; - let state = BORDER_STATE.lock(); - if let Some(border) = state.get(&border_id) { - border.animate_to(rect); + + if queue_animated_rect(&mut ANIMATED_RECTS.lock(), border, rect) { + border::post_animated_rect(border); } } +/// The newest rect an animation has for a border, which the border takes from +/// the animation to draw +pub(crate) fn take_animated_rect(border: isize) -> Option { + ANIMATED_RECTS.lock().remove(&border) +} + pub fn send_notification(hwnd: Option) { if event_tx().try_send(Notification::Update(hwnd)).is_err() { tracing::warn!("channel is full; dropping notification") @@ -940,4 +982,32 @@ mod tests { set_ghosted(hwnd, false); assert!(!is_ghosted(hwnd)); } + + #[test] + fn a_border_is_only_told_of_a_rect_when_it_has_none_waiting() { + let mut pending = HashMap::new(); + let rect = |left| crate::core::Rect { + left, + top: 0, + right: 10, + bottom: 10, + }; + + // the first rect has to be sent, and the ones after it only replace it + assert!(queue_animated_rect(&mut pending, 7, rect(1))); + assert!(!queue_animated_rect(&mut pending, 7, rect(2))); + assert!(!queue_animated_rect(&mut pending, 7, rect(3))); + + // another border has its own + assert!(queue_animated_rect(&mut pending, 8, rect(9))); + + // and what the border draws is the newest, after which it is told again + assert_eq!(pending.remove(&7).map(|rect| rect.left), Some(3)); + assert!(queue_animated_rect(&mut pending, 7, rect(4))); + } + + #[test] + fn nothing_is_waiting_for_a_border_that_was_not_ghosted() { + assert!(take_animated_rect(0x7e57_0003).is_none()); + } }