Windows pen end: read it on WM_POINTERENTER and keep a contact's end past the leave (#2285)

WM_POINTERENTER is sent, not posted, so only WH_CALLWNDPROC sees it; it
now reads the pen end like an update, so a pen held still after entering
range on its eraser end selects the Eraser.

A contact's end is kept past WM_POINTERLEAVE until the UI reads it once
(take_pen_end): an eraser-end tap and the pen leaving range, all handled
before the next frame, now erase instead of using the previous tool.

apps/photocraft/src/tablet.rs

@@ -96,7 +96,7 @@ pub fn right_button_service() -> Option<photocraft_ui_egui::RightButtonDownFn> {
 /// Eraser (#2285). Elsewhere the end arrives with the samples in the stylus feed.
 #[cfg(target_os = "windows")]
 pub fn pen_end_service() -> Option<photocraft_ui_egui::PenEndFn> {
-    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");
     }
 }

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<bool>) -> 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<bool>,
     /// The pointer id of the pen last seen, so another pointer leaving does not forget it.
     pub pen_id: Option<u32>,
+    /// 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<bool>,
 }
 
 /// 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<u32>) -> 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<u32>) {
     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<u32> {
@@ -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<bool> {
     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<bool> {
+    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]