diff --git a/desktop/src/components/app.rs b/desktop/src/components/app.rs index 12368586..593803ee 100644 --- a/desktop/src/components/app.rs +++ b/desktop/src/components/app.rs @@ -1,5 +1,5 @@ mod drag_drop_overlay; -mod drag_handlers; +pub(crate) mod drag_handlers; mod drop_handlers; mod keybinding_engine; mod listeners; @@ -44,7 +44,45 @@ use shortcut_overlay::{ }; /// Left mouse button ID for DeviceEvent::Button (platform-dependent raw value) +/// +/// tao does not normalize this value across platforms: +/// - macOS reports `NSEvent::buttonNumber()`, which is 0-based (left = 0). +/// - Windows derives it from the raw-input button index as `index + 1` for +/// consistency with X11, so left = 1. +/// +/// Getting this wrong is silent and severe: the release event never matches, so +/// `handle_drag_mouse_release` never runs and an active tab drag never ends. The +/// detached preview window then keeps following the cursor forever, and the +/// global drag state is never cleared. +#[cfg(target_os = "macos")] const MOUSE_BUTTON_LEFT: u32 = 0; +#[cfg(not(target_os = "macos"))] +const MOUSE_BUTTON_LEFT: u32 = 1; + +/// Apply tao's device event filter to match whether a tab drag is in progress. +/// +/// `DeviceEventFilter::Unfocused` (tao's default) drops device events for +/// unfocused windows, which is the right trade-off while idle but fatal during a +/// drag - see the call site. Only the transitions are applied, so this is cheap +/// to call on every event. +#[cfg(target_os = "windows")] +fn sync_device_event_filter( + target: &dioxus::desktop::tao::event_loop::EventLoopWindowTarget, + dragging: bool, +) { + use dioxus::desktop::tao::event_loop::DeviceEventFilter; + use std::sync::atomic::{AtomicBool, Ordering}; + + static RELAXED: AtomicBool = AtomicBool::new(false); + + if RELAXED.swap(dragging, Ordering::Relaxed) != dragging { + target.set_device_event_filter(if dragging { + DeviceEventFilter::Never + } else { + DeviceEventFilter::Unfocused + }); + } +} #[component] pub fn App( @@ -160,64 +198,81 @@ pub fn App( }); // Handle window events - use_wry_event_handler(move |event, _| match event { - TaoEvent::WindowEvent { - event: WindowEvent::Resized(size), - window_id, - .. - } => { - let window = window(); - if window_id == &window.id() { - sync_window_metrics( - state, - None, - Some(size.to_logical::(window.scale_factor())), - ); + use_wry_event_handler(move |event, target| { + // A tab drag is tracked exclusively by its SOURCE window, but tao drops + // DeviceEvents for unfocused windows by default. Detaching a tab creates + // and focuses a preview window, which unfocuses the source - so tracking + // died mid-drag: motion stopped and the release that ends the drag never + // arrived, leaving a drag that follows the cursor forever. Relax the + // filter while dragging and restore it after, so idle windows do not pay + // for device events they do not need. + // + // Windows-only by nature: tao ignores this filter everywhere else, which + // is exactly why the drag architecture works on macOS as written. + #[cfg(target_os = "windows")] + sync_device_event_filter(target, drag::is_active_drag()); + #[cfg(not(target_os = "windows"))] + let _ = target; + + match event { + TaoEvent::WindowEvent { + event: WindowEvent::Resized(size), + window_id, + .. + } => { + let window = window(); + if window_id == &window.id() { + sync_window_metrics( + state, + None, + Some(size.to_logical::(window.scale_factor())), + ); + } } - } - TaoEvent::WindowEvent { - event: WindowEvent::Moved(position), - window_id, - .. - } => { - let window = window(); - if window_id == &window.id() { - sync_window_metrics( - state, - Some(position.to_logical::(window.scale_factor())), - None, - ); + TaoEvent::WindowEvent { + event: WindowEvent::Moved(position), + window_id, + .. + } => { + let window = window(); + if window_id == &window.id() { + sync_window_metrics( + state, + Some(position.to_logical::(window.scale_factor())), + None, + ); + } } - } - // DeviceEvent: Global mouse tracking for tab drag - // These events are delivered regardless of window focus, enabling cross-window drag - TaoEvent::DeviceEvent { - event: DeviceEvent::MouseMotion { .. }, - .. - } => { - // Only process if we're the source window of an active drag - if let Some(dragged) = drag::get_dragged_tab() { - if dragged.source_window_id == window().id() && drag::is_active_drag() { - handle_drag_mouse_motion(state); + // DeviceEvent: Global mouse tracking for tab drag + // These events are delivered regardless of window focus, enabling cross-window drag + TaoEvent::DeviceEvent { + event: DeviceEvent::MouseMotion { .. }, + .. + } => { + // Only process if we're the source window of an active drag + if let Some(dragged) = drag::get_dragged_tab() { + if dragged.source_window_id == window().id() && drag::is_active_drag() { + handle_drag_mouse_motion(state); + } } } - } - TaoEvent::DeviceEvent { - event: - DeviceEvent::Button { - state: ElementState::Released, - button, - .. - }, - .. - } if *button == MOUSE_BUTTON_LEFT => { - if let Some(dragged) = drag::get_dragged_tab() { - if dragged.source_window_id == window().id() && drag::is_active_drag() { - handle_drag_mouse_release(state); + TaoEvent::DeviceEvent { + event: + DeviceEvent::Button { + state: ElementState::Released, + button, + .. + }, + .. + } if *button == MOUSE_BUTTON_LEFT => { + if let Some(dragged) = drag::get_dragged_tab() { + if dragged.source_window_id == window().id() && drag::is_active_drag() { + handle_drag_mouse_release(state); + } } } + _ => {} } - _ => {} }); // Listen for cross-window file/directory open events (from sidebar context menu) diff --git a/desktop/src/components/app/drag_handlers.rs b/desktop/src/components/app/drag_handlers.rs index d4860ff2..3c3ab01f 100644 --- a/desktop/src/components/app/drag_handlers.rs +++ b/desktop/src/components/app/drag_handlers.rs @@ -1,7 +1,6 @@ use dioxus::desktop::tao::dpi::LogicalPosition; use dioxus::desktop::window; use dioxus::prelude::*; -use mouse_position::mouse_position::Mouse; use crate::drag; use crate::events::{ActiveDragUpdate, ACTIVE_DRAG_UPDATE}; @@ -18,13 +17,14 @@ pub(super) const DETACH_DEBOUNCE_MS: u64 = 50; pub(super) fn handle_drag_mouse_motion(state: AppState) { use crate::window; - // Get current mouse position - let (screen_x, screen_y) = match Mouse::get_mouse_position() { - Mouse::Position { x, y } => (x as f64, y as f64), - Mouse::Error => { - tracing::debug!("Failed to get mouse position during active drag"); - return; - } + // Get current mouse position, normalized to logical coordinates. + // The raw value is physical pixels on Windows - see cursor_position_to_logical. + let scale_factor = dioxus::desktop::window().scale_factor(); + let Some((screen_x, screen_y)) = + crate::utils::screen::get_cursor_logical_position(scale_factor) + else { + tracing::debug!("Failed to get mouse position during active drag"); + return; }; let Some(active) = drag::get_active_drag() else { @@ -118,7 +118,7 @@ pub(super) fn handle_drag_mouse_motion(state: AppState) { /// - `DetachState::Pending` -> Drag cancelled during debounce, restore tab /// - `DetachState::Creating` -> Window creation in progress, cancel and restore /// - `DetachState::Detached` -> Preview visible, commit as new window -pub(super) fn handle_drag_mouse_release(mut state: AppState) { +pub(crate) fn handle_drag_mouse_release(mut state: AppState) { use crate::drag::DetachState; let Some(active) = drag::get_active_drag() else { diff --git a/desktop/src/components/main_app.rs b/desktop/src/components/main_app.rs index 21936e5d..f5c4d631 100644 --- a/desktop/src/components/main_app.rs +++ b/desktop/src/components/main_app.rs @@ -3,7 +3,9 @@ use crate::state::Tab; use crate::window::settings; #[cfg(not(target_os = "windows"))] use dioxus::desktop::use_muda_event_handler; -use dioxus::desktop::{window, WindowCloseBehaviour}; +use dioxus::desktop::window; +#[cfg(target_os = "macos")] +use dioxus::desktop::WindowCloseBehaviour; use dioxus::prelude::*; // ============================================================================ @@ -22,9 +24,19 @@ use dioxus::prelude::*; #[component] pub fn MainApp() -> Element { // Configure WindowCloseBehaviour::WindowHides for first window + // + // macOS only: hiding the last window models the dock lifecycle, where an app + // outlives its windows and is reopened from the dock. Windows and Linux have + // no such concept, so it just leaves a headless process behind - and the + // single-instance IPC then swallows every subsequent launch into it, making + // the app look dead. The Dioxus default (WindowCloses, exiting on last window + // close) is the correct behaviour there. use_hook(|| { - tracing::debug!("Configuring main window with WindowHides behavior"); - window().set_close_behavior(WindowCloseBehaviour::WindowHides); + #[cfg(target_os = "macos")] + { + tracing::debug!("Configuring main window with WindowHides behavior"); + window().set_close_behavior(WindowCloseBehaviour::WindowHides); + } // Set chrome inset (window frame offset) - only first call takes effect let win = &window().window; diff --git a/desktop/src/components/tab/tab_bar.rs b/desktop/src/components/tab/tab_bar.rs index c4842c7a..6fb1e93c 100644 --- a/desktop/src/components/tab/tab_bar.rs +++ b/desktop/src/components/tab/tab_bar.rs @@ -332,11 +332,22 @@ pub fn TabBar() -> Element { // Pointer up handler for local pending state only // Active drag release is handled by DeviceEvent in App component let handle_pointerup = move |_evt: Event| { - // Only cancel local pending state - // Active drag is handled by DeviceEvent if matches!(*local_drag_state.read(), LocalDragState::Pending(_)) { local_drag_state.set(LocalDragState::Idle); } + + // Also end an active drag. DeviceEvent is the primary release path since + // it works across windows, but it is the ONLY one, so whenever it fails + // to arrive the drag never ends and the overlay swallows every click - + // an unrecoverable UI. A pointerup that reaches this window is proof the + // button came up, so honour it. Releasing over another window still + // relies on DeviceEvent; handle_drag_mouse_release is idempotent, so a + // duplicate from both paths is harmless. + if drag::is_active_drag() + && drag::get_dragged_tab().is_some_and(|d| d.source_window_id == current_window_id) + { + crate::components::app::drag_handlers::handle_drag_mouse_release(state); + } }; // Get current global drag info for rendering (from signal for reactivity) @@ -381,6 +392,16 @@ pub fn TabBar() -> Element { on_drag_start: move |pending: PendingDrag| { local_drag_state.set(LocalDragState::Pending(pending)); }, + on_grab_offset_refined: move |offset: crate::window::Offset| { + // Only applies while the drag has not started yet. Once it + // is active the offset is baked into the floating tab's + // position, and moving it would make the ghost jump. + let current = local_drag_state.read().clone(); + if let LocalDragState::Pending(mut pending) = current { + pending.grab_offset = offset; + local_drag_state.set(LocalDragState::Pending(pending)); + } + }, } } diff --git a/desktop/src/components/tab/tab_item.rs b/desktop/src/components/tab/tab_item.rs index 9776ccd4..103ca2fb 100644 --- a/desktop/src/components/tab/tab_item.rs +++ b/desktop/src/components/tab/tab_item.rs @@ -16,6 +16,9 @@ pub fn TabItem( is_active: bool, shift_class: Option<&'static str>, on_drag_start: EventHandler, + /// Delivers the precise grab offset once the tab's client rect resolves. + /// See `handle_pointerdown` for why this cannot be awaited inline. + on_grab_offset_refined: EventHandler, ) -> Element { let mut state = use_context::(); let tab_name = tab.display_name(); @@ -36,8 +39,19 @@ pub fn TabItem( // Handle pointer down for drag initiation // Uses PointerData for setPointerCapture compatibility (window-external drag) - // Uses async to get accurate grab_offset via getBoundingClientRect - let handle_pointerdown = move |evt: Event| async move { + // + // The pending drag MUST be registered synchronously. `get_client_rect()` is an + // async round trip through the webview IPC bridge, and awaiting it before + // registering meant every `pointermove` that arrived in the meantime was + // ignored. On Windows that round trip is slow enough to outlast the whole + // gesture: the drag was registered from a stale position only after the user + // had already released the button, so it started on mouse-up and then never + // ended (the release that would have ended it was already gone). macOS hides + // this because the round trip resolves fast enough to win the race. + // + // So: register immediately with the offset within the event target, then + // refine it once the exact element rect arrives. + let handle_pointerdown = move |evt: Event| { // Only start drag on left button if evt.data().trigger_button() != Some(dioxus::html::input_data::MouseButton::Primary) { return; @@ -45,36 +59,35 @@ pub fn TabItem( let pointer_id = evt.data().pointer_id(); let client_coords = evt.client_coordinates(); - - // Calculate grab_offset using getBoundingClientRect for accuracy - // Clone signal data before await to avoid holding GenerationalRef across await point - let mounted_data = tab_element.read().clone(); - let (grab_x, grab_y) = if let Some(ref mounted) = mounted_data { - if let Ok(rect) = mounted.get_client_rect().await { - calculate_grab_offset( - client_coords.x, - client_coords.y, - rect.origin.x, - rect.origin.y, - ) - } else { - // Fallback to element_coordinates - let element_coords = evt.element_coordinates(); - (element_coords.x, element_coords.y) - } - } else { - // Fallback to element_coordinates - let element_coords = evt.element_coordinates(); - (element_coords.x, element_coords.y) - }; + let element_coords = evt.element_coordinates(); on_drag_start.call(PendingDrag { index, start_x: client_coords.x, start_y: client_coords.y, - grab_offset: crate::window::Offset::new(grab_x, grab_y), + grab_offset: crate::window::Offset::new(element_coords.x, element_coords.y), pointer_id, }); + + // `element_coordinates()` is relative to whatever child node the pointer + // landed on (label, icon, close button), so refine it against the tab + // element itself as soon as the rect is available. + let mounted_data = tab_element.read().clone(); + spawn(async move { + let Some(mounted) = mounted_data else { + return; + }; + let Ok(rect) = mounted.get_client_rect().await else { + return; + }; + let (grab_x, grab_y) = calculate_grab_offset( + client_coords.x, + client_coords.y, + rect.origin.x, + rect.origin.y, + ); + on_grab_offset_refined.call(crate::window::Offset::new(grab_x, grab_y)); + }); }; // Handle right-click to show context menu diff --git a/desktop/src/utils/screen.rs b/desktop/src/utils/screen.rs index 4524d5e9..26814a38 100644 --- a/desktop/src/utils/screen.rs +++ b/desktop/src/utils/screen.rs @@ -96,6 +96,53 @@ pub fn get_primary_display() -> Option { .or_else(|| displays.first().cloned()) } +/// Normalize a raw cursor position to logical coordinates. +/// +/// `Mouse::get_mouse_position()` does not report the same coordinate space on +/// every platform: +/// - macOS (core-graphics) reports logical points already. +/// - Windows (`GetCursorPos`) reports physical pixels, because tao marks the +/// process `DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2`. +/// +/// Everything the drag code compares against is logical: DOM +/// `screen_coordinates()`, the tab bar bounds measured from the DOM, and the +/// rects `drag::is_point_in_window` derives as `physical / scale`. Feeding a +/// physical value into that world silently breaks it — at 150% scaling the +/// cursor reads 1.5x too far, so every window hit test misses, the drag detaches +/// immediately, and the preview window is positioned far off target. +/// +/// `scale_factor` is the scale factor used to convert raw coordinates into +/// logical coordinates. Ideally this comes from the display currently under the +/// cursor; some call sites (e.g. drag tracking) may use the source window's scale +/// factor, which can be inaccurate across mixed-DPI monitors. +pub fn cursor_position_to_logical(x: f64, y: f64, scale_factor: f64) -> (f64, f64) { + #[cfg(target_os = "windows")] + { + if scale_factor > 0.0 { + return (x / scale_factor, y / scale_factor); + } + (x, y) + } + #[cfg(not(target_os = "windows"))] + { + let _ = scale_factor; + (x, y) + } +} + +/// Get the current cursor position in logical coordinates. +/// +/// See [`cursor_position_to_logical`] for why normalization is needed. +/// Returns `None` if the cursor position cannot be determined. +pub fn get_cursor_logical_position(scale_factor: f64) -> Option<(f64, f64)> { + match Mouse::get_mouse_position() { + Mouse::Position { x, y } => { + Some(cursor_position_to_logical(x as f64, y as f64, scale_factor)) + } + Mouse::Error => None, + } +} + /// Get the display where the cursor is currently located. /// /// # Returns @@ -295,4 +342,34 @@ mod tests { assert_eq!(position.x, -40); assert_eq!(position.y, 20); } + + #[cfg(target_os = "windows")] + #[test] + fn test_cursor_position_to_logical_divides_by_scale_on_windows() { + // GetCursorPos reports physical pixels, so a cursor at logical (400, 300) + // on a 150% display arrives as (600, 450) and must be scaled back down. + assert_eq!( + cursor_position_to_logical(600.0, 450.0, 1.5), + (400.0, 300.0) + ); + } + + #[cfg(not(target_os = "windows"))] + #[test] + fn test_cursor_position_to_logical_is_identity_off_windows() { + // core-graphics (macOS) already reports logical points. + assert_eq!( + cursor_position_to_logical(600.0, 450.0, 1.5), + (600.0, 450.0) + ); + } + + #[test] + fn test_cursor_position_to_logical_handles_zero_scale() { + // A bogus scale factor must not produce NaN/inf coordinates. + assert_eq!( + cursor_position_to_logical(600.0, 450.0, 0.0), + (600.0, 450.0) + ); + } } diff --git a/desktop/src/window/settings.rs b/desktop/src/window/settings.rs index 46d50edc..5ece6ad0 100644 --- a/desktop/src/window/settings.rs +++ b/desktop/src/window/settings.rs @@ -1,6 +1,5 @@ use dioxus::desktop::tao::dpi::{LogicalPosition, LogicalSize}; use dioxus::prelude::*; -use mouse_position::mouse_position::Mouse; use std::path::PathBuf; use crate::components::right_sidebar::RightSidebarTab; @@ -150,11 +149,12 @@ fn resolve_window_position( fn resolve_window_position_from_cursor( window_size: LogicalSize, ) -> Option> { - let (x, y) = match Mouse::get_mouse_position() { - Mouse::Position { x, y } => (x as f64, y as f64), - Mouse::Error => return None, - }; let display = get_cursor_display().or_else(get_primary_display)?; + // The cursor is compared against logical display bounds below, so it has to + // be normalized first - the raw value is physical pixels on Windows. Scale by + // the display under the cursor rather than any window's factor, since that is + // the one the bounds are derived from. + let (x, y) = crate::utils::screen::get_cursor_logical_position(display.scale_factor as f64)?; let (display_origin, display_size) = display_info_logical_bounds(&display)?; let display_x = display_origin.x as f64; let display_y = display_origin.y as f64;