diff --git a/apps/photocraft/src/tablet.rs b/apps/photocraft/src/tablet.rs index f73eb6606f..51f7b333ef 100644 --- a/apps/photocraft/src/tablet.rs +++ b/apps/photocraft/src/tablet.rs @@ -96,7 +96,7 @@ pub fn right_button_service() -> Option { /// Eraser (#2285). Elsewhere the end arrives with the samples in the stylus feed. #[cfg(target_os = "windows")] pub fn pen_end_service() -> Option { - Some(Box::new(photocraft_tablet::windows::pen_end)) + Some(Box::new(photocraft_tablet::windows::take_pen_end)) } #[cfg(not(target_os = "windows"))] @@ -441,7 +441,9 @@ mod tests { #[cfg(target_os = "windows")] #[test] fn windows_eraser_end_switches_to_the_eraser_and_back() { - use photocraft_tablet::windows::{PEN_FLAG_ERASER, PEN_FLAG_INVERTED, WM_POINTERLEAVE, WM_POINTERUPDATE, handle_pointer_message}; + use photocraft_tablet::windows::{ + PEN_FLAG_ERASER, PEN_FLAG_INVERTED, WM_POINTERDOWN, WM_POINTERLEAVE, WM_POINTERUP, WM_POINTERUPDATE, handle_pointer_message, + }; use photocraft_ui_egui::stylus::Stylus; let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default()); app.services.pen_end = pen_end_service(); @@ -460,5 +462,13 @@ mod tests { hover(0); assert!(Stylus::sync_eraser_tool(&mut app)); assert_eq!(app.ui.tool, Tool::Brush, "the tip restores the Brush"); + // An eraser-end tap and the pen leaving, all before the next frame: the frame that + // handles the queued click still selects the Eraser. + handle_pointer_message(WM_POINTERDOWN, 1, |_| Some(PEN_FLAG_INVERTED | PEN_FLAG_ERASER)); + handle_pointer_message(WM_POINTERUP, 1, |_| None); + handle_pointer_message(WM_POINTERLEAVE, 1, |_| None); + assert!(Stylus::sync_eraser_tool(&mut app)); + assert_eq!(app.ui.tool, Tool::Eraser); + assert!(!Stylus::sync_eraser_tool(&mut app), "out of range"); } } diff --git a/crates/tablet/src/windows.rs b/crates/tablet/src/windows.rs index 50f53ced1d..f509cc5a0f 100644 --- a/crates/tablet/src/windows.rs +++ b/crates/tablet/src/windows.rs @@ -6,8 +6,8 @@ //! stylus barrel buttons. Thread-local hooks inspect `WM_POINTER*` messages on the UI thread to //! track the physical barrel button state and which end of the pen is in use: `WH_GETMESSAGE` for //! the posted ones (`WM_POINTERDOWN`, `WM_POINTERUPDATE`, `WM_POINTERUP`) and `WH_CALLWNDPROC` for -//! `WM_POINTERLEAVE`, which Windows documents as sent to the window, so the queue hook alone may -//! never see the pen leave. +//! `WM_POINTERENTER` and `WM_POINTERLEAVE`, which Windows documents as sent to the window, so the +//! queue hook alone may never see the pen arrive or leave. #![allow(unsafe_code, clippy::upper_case_acronyms)] @@ -23,6 +23,9 @@ const NO_PEN: u32 = u32::MAX; /// The pen end last reported, as [`encode_end`] stores it. static PEN_END: AtomicU8 = AtomicU8::new(0); +/// The pen end of the last contact [`take_pen_end`] has not read yet, as [`encode_end`] stores it. +static CONTACT_END: AtomicU8 = AtomicU8::new(0); + fn encode_end(end: Option) -> u8 { match end { None => 0, @@ -110,6 +113,8 @@ unsafe extern "system" { const WH_GETMESSAGE: i32 = 3; const WH_CALLWNDPROC: i32 = 4; +/// `WM_POINTERENTER` (winuser.h). +pub const WM_POINTERENTER: u32 = 0x0249; /// `WM_POINTERUPDATE` (winuser.h). pub const WM_POINTERUPDATE: u32 = 0x0245; /// `WM_POINTERDOWN` (winuser.h). @@ -162,6 +167,9 @@ pub struct PenState { pub end: Option, /// The pointer id of the pen last seen, so another pointer leaving does not forget it. pub pen_id: Option, + /// The end of the last pen contact, kept when the pen leaves: the contact and its release + /// can still be queued for the UI after the leave is handled, and must get that end's tool. + pub contact: Option, } /// Pure transition of [`PenState`] for one pointer message. `w_param` is the message's (pointer @@ -170,36 +178,41 @@ pub struct PenState { pub fn next_state(prev: PenState, message: u32, w_param: usize, pen_flags: impl FnOnce(u32) -> Option) -> PenState { let pointer_id = (w_param & 0xFFFF) as u32; match message { - WM_POINTERDOWN | WM_POINTERUPDATE => { + // A pen entering range while hovering may stay still: its first update can come late. + WM_POINTERDOWN | WM_POINTERUPDATE | WM_POINTERENTER => { let pointer_message_flags = ((w_param >> 16) & 0xFFFF) as u32; let pen_flags = pen_flags(pointer_id); + let end = is_eraser_end(pen_flags); // A mouse or a finger in between leaves the pen's end as it was. PenState { barrel: is_barrel_pressed(pen_flags, pointer_message_flags), - end: is_eraser_end(pen_flags).or(prev.end), + end: end.or(prev.end), pen_id: if pen_flags.is_some() { Some(pointer_id) } else { prev.pen_id }, + contact: if message == WM_POINTERDOWN { end.or(prev.contact) } else { prev.contact }, } } WM_POINTERUP => PenState { barrel: false, ..prev }, // A finger or a palm leaving while the pen stays in range (pen and touch screens). WM_POINTERLEAVE if prev.pen_id.is_some_and(|id| id != pointer_id) => prev, - WM_POINTERLEAVE => PenState::default(), + WM_POINTERLEAVE => PenState { contact: prev.contact, ..PenState::default() }, _ => prev, } } -/// Apply one pointer message to the state [`is_barrel_button_down`] and [`pen_end`] read. The -/// hooks call this; tests call it with a fake `pen_flags` reader. +/// Apply one pointer message to the state [`is_barrel_button_down`], [`pen_end`] and +/// [`take_pen_end`] read. The hooks call this; tests call it with a fake `pen_flags` reader. pub fn handle_pointer_message(message: u32, w_param: usize, pen_flags: impl FnOnce(u32) -> Option) { let prev = PenState { barrel: BARREL_DOWN.load(Ordering::Relaxed), end: decode_end(PEN_END.load(Ordering::Relaxed)), pen_id: Some(PEN_ID.load(Ordering::Relaxed)).filter(|&id| id != NO_PEN), + contact: decode_end(CONTACT_END.load(Ordering::Relaxed)), }; let next = next_state(prev, message, w_param, pen_flags); BARREL_DOWN.store(next.barrel, Ordering::Relaxed); PEN_END.store(encode_end(next.end), Ordering::Relaxed); PEN_ID.store(next.pen_id.unwrap_or(NO_PEN), Ordering::Relaxed); + CONTACT_END.store(encode_end(next.contact), Ordering::Relaxed); } fn read_pen_flags(pointer_id: u32) -> Option { @@ -227,15 +240,17 @@ unsafe extern "system" fn get_msg_proc(code: i32, w_param: usize, l_param: isize unsafe { CallNextHookEx(std::ptr::null_mut(), code, w_param, l_param) } } -/// `WH_CALLWNDPROC`: messages sent to the thread's windows. Only `WM_POINTERLEAVE` matters here; -/// down, update and up are posted and reach [`get_msg_proc`]. +/// `WH_CALLWNDPROC`: messages sent to the thread's windows. Only `WM_POINTERENTER` and +/// `WM_POINTERLEAVE` matter here; down, update and up are posted and reach [`get_msg_proc`]. unsafe extern "system" fn call_wnd_proc(code: i32, w_param: usize, l_param: isize) -> isize { if code >= 0 && l_param != 0 { // SAFETY: Win32 guarantees that for code >= 0 (HC_ACTION), l_param points to a valid // CWPSTRUCT describing the message being sent. let msg = unsafe { &*(l_param as *const CWPSTRUCT) }; - if msg.message == WM_POINTERLEAVE { - handle_pointer_message(msg.message, msg.w_param, |_| None); + match msg.message { + WM_POINTERENTER => handle_pointer_message(msg.message, msg.w_param, read_pen_flags), + WM_POINTERLEAVE => handle_pointer_message(msg.message, msg.w_param, |_| None), + _ => {} } } // SAFETY: as in `get_msg_proc`. @@ -295,6 +310,7 @@ impl Drop for Monitor { BARREL_DOWN.store(false, Ordering::Relaxed); PEN_END.store(encode_end(None), Ordering::Relaxed); PEN_ID.store(NO_PEN, Ordering::Relaxed); + CONTACT_END.store(encode_end(None), Ordering::Relaxed); } } @@ -309,6 +325,13 @@ pub fn pen_end() -> Option { decode_end(PEN_END.load(Ordering::Relaxed)) } +/// [`pen_end`] for the UI's once-per-frame read: the end of a contact not read yet comes first, +/// once, even when the pen has left since, so a click still queued for the UI gets that end's +/// tool. +pub fn take_pen_end() -> Option { + decode_end(CONTACT_END.swap(encode_end(None), Ordering::Relaxed)).or_else(pen_end) +} + #[cfg(test)] mod tests { use super::*; @@ -356,33 +379,43 @@ mod tests { fn pen_state_follows_pointer_messages() { let pen = |flags: u32| move |id: u32| (id == 7).then_some(flags); let s = next_state(PenState::default(), WM_POINTERUPDATE, wp(0), pen(0)); - assert_eq!(s, PenState { barrel: false, end: Some(false), pen_id: Some(7) }, "the tip hovers"); + assert_eq!(s, PenState { barrel: false, end: Some(false), pen_id: Some(7), contact: None }, "the tip hovers"); let s = next_state(s, WM_POINTERUPDATE, wp(0), pen(PEN_FLAG_INVERTED)); assert_eq!(s.end, Some(true), "the eraser end hovers"); let s = next_state(s, WM_POINTERDOWN, wp(POINTER_FLAG_INCONTACT), pen(PEN_FLAG_INVERTED | PEN_FLAG_ERASER)); - assert_eq!(s, PenState { barrel: false, end: Some(true), pen_id: Some(7) }); + assert_eq!(s, PenState { barrel: false, end: Some(true), pen_id: Some(7), contact: Some(true) }); let s = next_state(s, WM_POINTERUPDATE, wp(0), |_| None); assert_eq!((s.end, s.pen_id), (Some(true), Some(7)), "a mouse in between keeps the pen"); let s = next_state(s, WM_POINTERUPDATE, wp(0), pen(PEN_FLAG_BARREL)); - assert_eq!(s, PenState { barrel: true, end: Some(false), pen_id: Some(7) }); + assert_eq!(s, PenState { barrel: true, end: Some(false), pen_id: Some(7), contact: Some(true) }, "only a contact sets it"); let s = next_state(s, WM_POINTERUP, wp(0), |_| panic!("up does not read the pen")); - assert_eq!(s, PenState { barrel: false, end: Some(false), pen_id: Some(7) }, "lifting keeps the end"); + assert_eq!(s, PenState { barrel: false, end: Some(false), pen_id: Some(7), contact: Some(true) }, "lifting keeps the end"); let s = next_state(s, WM_POINTERLEAVE, wp(0), |_| panic!("leave does not read the pen")); - assert_eq!(s, PenState::default(), "leaving range forgets the pen"); + assert_eq!(s, PenState { contact: Some(true), ..PenState::default() }, "leaving range forgets the pen, not its contact"); let mouse_move = 0x0200; assert_eq!(next_state(s, mouse_move, wp(0), pen(PEN_FLAG_INVERTED)), s, "other messages change nothing"); } #[test] fn another_pointer_leaving_keeps_the_pen() { - let held = PenState { barrel: true, end: Some(true), pen_id: Some(7) }; + let held = PenState { barrel: true, end: Some(true), pen_id: Some(7), contact: None }; let finger_leaves = 2 | ((POINTER_FLAG_INCONTACT as usize) << 16); assert_eq!(next_state(held, WM_POINTERLEAVE, finger_leaves, |_| panic!("leave does not read the pen")), held); assert_eq!(next_state(held, WM_POINTERLEAVE, wp(0), |_| None), PenState::default(), "the pen itself leaving"); - let unknown = PenState { barrel: true, end: None, pen_id: None }; + let unknown = PenState { barrel: true, end: None, pen_id: None, contact: None }; assert_eq!(next_state(unknown, WM_POINTERLEAVE, finger_leaves, |_| None), PenState::default(), "no pen seen yet"); } + /// A pen entering range with its eraser end is known before it moves (`WM_POINTERENTER` can + /// come well before the first hover update). + #[test] + fn entering_range_reads_the_pen_end() { + let s = next_state(PenState::default(), WM_POINTERENTER, wp(0), |id| (id == 7).then_some(PEN_FLAG_INVERTED)); + assert_eq!(s, PenState { barrel: false, end: Some(true), pen_id: Some(7), contact: None }); + let s = next_state(s, WM_POINTERENTER, 9, |_| None); + assert_eq!((s.end, s.pen_id), (Some(true), Some(7)), "a mouse or finger entering keeps the pen"); + } + /// Serialises the tests that touch the global state the hooks write (tests run in parallel). static GLOBALS: std::sync::Mutex<()> = std::sync::Mutex::new(()); @@ -399,6 +432,28 @@ mod tests { assert_eq!(pen_end(), Some(false), "a finger leaving keeps the pen"); handle_pointer_message(WM_POINTERLEAVE, wp(0), |_| None); assert_eq!((pen_end(), is_barrel_button_down()), (None, false)); + assert_eq!(take_pen_end(), None, "no contact to report"); + } + + /// An eraser-end tap and the pen leaving range, all handled before the UI's next frame: the + /// frame still reads the eraser end once, so the queued click erases. + #[test] + fn a_contact_queued_past_the_leave_keeps_its_end() { + let _globals = GLOBALS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + handle_pointer_message(WM_POINTERLEAVE, wp(0), |_| None); + let _ = take_pen_end(); + handle_pointer_message(WM_POINTERDOWN, wp(POINTER_FLAG_INCONTACT), |_| Some(PEN_FLAG_INVERTED | PEN_FLAG_ERASER)); + handle_pointer_message(WM_POINTERUP, wp(0), |_| None); + handle_pointer_message(WM_POINTERLEAVE, wp(0), |_| None); + assert_eq!(pen_end(), None, "the pen is out of range"); + assert_eq!(take_pen_end(), Some(true), "the queued contact was the eraser end"); + assert_eq!(take_pen_end(), None, "reported once"); + handle_pointer_message(WM_POINTERDOWN, wp(POINTER_FLAG_INCONTACT), |_| Some(PEN_FLAG_INVERTED)); + handle_pointer_message(WM_POINTERUP, wp(0), |_| None); + handle_pointer_message(WM_POINTERUPDATE, wp(0), |_| Some(0)); + assert_eq!(take_pen_end(), Some(true), "the unread contact comes before the tip hovering since"); + assert_eq!(take_pen_end(), Some(false), "then the end in range"); + handle_pointer_message(WM_POINTERLEAVE, wp(0), |_| None); } #[test]