From 0b019282c32a6749f1baf4cd56822de5edde916d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 20 Mar 2024 19:22:47 -0700 Subject: [PATCH] Wayland: double click (#9608) This PR builds off of an earlier version of https://github.com/zed-industries/zed/pull/9595, rearranges some of the logic, and removes an unused platform API. Release Notes: - N/A --------- Co-authored-by: apricotbucket28 --- crates/gpui/src/app.rs | 5 - crates/gpui/src/platform.rs | 1 - crates/gpui/src/platform/linux/platform.rs | 13 +- crates/gpui/src/platform/linux/util.rs | 35 +++++- .../gpui/src/platform/linux/wayland/client.rs | 119 ++++++++++-------- crates/gpui/src/platform/mac/platform.rs | 8 -- crates/gpui/src/platform/test/platform.rs | 5 - crates/gpui/src/platform/windows/platform.rs | 5 - crates/gpui/src/platform/windows/window.rs | 2 +- 9 files changed, 108 insertions(+), 85 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 85db09785ffcd903e71b7b155aa22aafe1de05db..af4bedabef292a430d61a4c1bbb5e7dac29b6996 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -584,11 +584,6 @@ impl AppContext { self.platform.path_for_auxiliary_executable(name) } - /// Returns the maximum duration in which a second mouse click must occur for an event to be a double-click event. - pub fn double_click_interval(&self) -> Duration { - self.platform.double_click_interval() - } - /// Displays a platform modal for selecting paths. /// When one or more paths are selected, they'll be relayed asynchronously via the returned oneshot channel. /// If cancelled, a `None` will be relayed instead. diff --git a/crates/gpui/src/platform.rs b/crates/gpui/src/platform.rs index 5244ca65cfb0f8900e7df3b51246b16d5a87a901..098836723da4250e2854756c1e2e21cf2344a9e0 100644 --- a/crates/gpui/src/platform.rs +++ b/crates/gpui/src/platform.rs @@ -129,7 +129,6 @@ pub(crate) trait Platform: 'static { fn app_version(&self) -> Result; fn app_path(&self) -> Result; fn local_timezone(&self) -> UtcOffset; - fn double_click_interval(&self) -> Duration; fn path_for_auxiliary_executable(&self, name: &str) -> Result; fn set_cursor_style(&self, style: CursorStyle); diff --git a/crates/gpui/src/platform/linux/platform.rs b/crates/gpui/src/platform/linux/platform.rs index 0f5dc8b18a4055d8bea06980e6fdc3239785b77c..fd266c57ecef166d918fa54c29fd6218e8286b46 100644 --- a/crates/gpui/src/platform/linux/platform.rs +++ b/crates/gpui/src/platform/linux/platform.rs @@ -22,8 +22,8 @@ use wayland_client::Connection; use crate::platform::linux::client::Client; use crate::platform::linux::wayland::WaylandClient; use crate::{ - Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, - ForegroundExecutor, Keymap, LinuxDispatcher, LinuxTextSystem, Menu, PathPromptOptions, + px, Action, AnyWindowHandle, BackgroundExecutor, ClipboardItem, CursorStyle, DisplayId, + ForegroundExecutor, Keymap, LinuxDispatcher, LinuxTextSystem, Menu, PathPromptOptions, Pixels, Platform, PlatformDisplay, PlatformInput, PlatformTextSystem, PlatformWindow, Result, SemanticVersion, Task, WindowOptions, WindowParams, }; @@ -32,6 +32,11 @@ use super::x11::X11Client; pub(super) const SCROLL_LINES: f64 = 3.0; +// Values match the defaults on GTK. +// Taken from https://github.com/GNOME/gtk/blob/main/gtk/gtksettings.c#L320 +pub(super) const DOUBLE_CLICK_INTERVAL: Duration = Duration::from_millis(400); +pub(super) const DOUBLE_CLICK_DISTANCE: Pixels = px(5.0); + #[derive(Default)] pub(crate) struct Callbacks { open_urls: Option)>>, @@ -310,10 +315,6 @@ impl Platform for LinuxPlatform { "Linux" } - fn double_click_interval(&self) -> Duration { - Duration::default() - } - fn os_version(&self) -> Result { Ok(SemanticVersion { major: 1, diff --git a/crates/gpui/src/platform/linux/util.rs b/crates/gpui/src/platform/linux/util.rs index 543ebbeae358a7d6cc3af0402a36dd34cf793837..a485fb4b175836c080f9c9f6994de13663cbaca9 100644 --- a/crates/gpui/src/platform/linux/util.rs +++ b/crates/gpui/src/platform/linux/util.rs @@ -1,6 +1,7 @@ use xkbcommon::xkb::{self, Keycode, Keysym, State}; -use crate::{Keystroke, Modifiers}; +use super::DOUBLE_CLICK_DISTANCE; +use crate::{Keystroke, Modifiers, Pixels, Point}; impl Keystroke { pub(super) fn from_xkb(state: &State, modifiers: Modifiers, keycode: Keycode) -> Self { @@ -93,3 +94,35 @@ impl Keystroke { } } } + +pub(super) fn is_within_click_distance(a: Point, b: Point) -> bool { + let diff = a - b; + diff.x.abs() <= DOUBLE_CLICK_DISTANCE && diff.y.abs() <= DOUBLE_CLICK_DISTANCE +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{px, Point}; + + #[test] + fn test_is_within_click_distance() { + let zero = Point::new(px(0.0), px(0.0)); + assert_eq!( + is_within_click_distance(zero, Point::new(px(5.0), px(5.0))), + true + ); + assert_eq!( + is_within_click_distance(zero, Point::new(px(-4.9), px(5.0))), + true + ); + assert_eq!( + is_within_click_distance(Point::new(px(3.0), px(2.0)), Point::new(px(-2.0), px(-2.0))), + true + ); + assert_eq!( + is_within_click_distance(zero, Point::new(px(5.0), px(5.1))), + false + ); + } +} diff --git a/crates/gpui/src/platform/linux/wayland/client.rs b/crates/gpui/src/platform/linux/wayland/client.rs index a3a57f0491a195ba95cdb0d6c870148131ce1046..eb1b01360a167b3a7a49bd8c1f5d6a8103103f36 100644 --- a/crates/gpui/src/platform/linux/wayland/client.rs +++ b/crates/gpui/src/platform/linux/wayland/client.rs @@ -1,8 +1,9 @@ use std::cell::RefCell; +use std::mem; use std::num::NonZeroU32; use std::rc::Rc; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; use calloop::timer::{TimeoutAction, Timer}; use calloop::LoopHandle; @@ -35,7 +36,9 @@ use wayland_protocols::xdg::shell::client::{xdg_surface, xdg_toplevel, xdg_wm_ba use xkbcommon::xkb::ffi::XKB_KEYMAP_FORMAT_TEXT_V1; use xkbcommon::xkb::{self, Keycode, KEYMAP_COMPILE_NO_FLAGS}; +use super::super::DOUBLE_CLICK_INTERVAL; use crate::platform::linux::client::Client; +use crate::platform::linux::util::is_within_click_distance; use crate::platform::linux::wayland::cursor::Cursor; use crate::platform::linux::wayland::window::{WaylandDecorationState, WaylandWindow}; use crate::platform::{LinuxPlatformInner, PlatformWindow}; @@ -46,6 +49,7 @@ use crate::{ MouseDownEvent, MouseMoveEvent, MouseUpEvent, NavigationDirection, Pixels, PlatformDisplay, PlatformInput, Point, ScrollDelta, ScrollWheelEvent, TouchPhase, }; +use crate::{point, px}; /// Used to convert evdev scancode to xkb scancode const MIN_KEYCODE: u32 = 8; @@ -61,11 +65,12 @@ pub(crate) struct WaylandClientStateInner { outputs: Vec<(wl_output::WlOutput, Rc>)>, platform_inner: Rc, keymap_state: Option, + click_state: ClickState, repeat: KeyRepeat, modifiers: Modifiers, scroll_direction: f64, axis_source: AxisSource, - mouse_location: Option>, + mouse_location: Point, button_pressed: Option, mouse_focused_window: Option>, keyboard_focused_window: Option>, @@ -85,6 +90,12 @@ pub(crate) struct WaylandClientState { primary: Rc>, } +struct ClickState { + last_click: Instant, + last_location: Point, + current_count: usize, +} + pub(crate) struct KeyRepeat { characters_per_second: u32, delay: Duration, @@ -163,6 +174,11 @@ impl WaylandClient { windows: Vec::new(), platform_inner: Rc::clone(&linux_platform_inner), keymap_state: None, + click_state: ClickState { + last_click: Instant::now(), + last_location: Point::new(px(0.0), px(0.0)), + current_count: 0, + }, repeat: KeyRepeat { characters_per_second: 16, delay: Duration::from_millis(500), @@ -178,7 +194,7 @@ impl WaylandClient { }, scroll_direction: -1.0, axis_source: AxisSource::Wheel, - mouse_location: None, + mouse_location: point(px(0.0), px(0.0)), button_pressed: None, mouse_focused_window: None, keyboard_focused_window: None, @@ -439,7 +455,7 @@ impl Dispatch for WaylandClientState { } wl_surface::Event::Leave { output } => { // We use `PreferredBufferScale` instead to set the scale if it's available - if surface.version() >= 6 { + if surface.version() >= wl_surface::EVT_PREFERRED_BUFFER_SCALE_SINCE { return; } @@ -842,27 +858,21 @@ impl Dispatch for WaylandClientState { surface_y, .. } => { - let mut mouse_focused_window = None; - for window in &state.windows { + let windows = mem::take(&mut state.windows); + for window in windows.iter() { if window.1.surface.id() == surface.id() { window.1.set_focused(true); - mouse_focused_window = Some(Rc::clone(&window.1)); - } - } - if mouse_focused_window.is_some() { - state.mouse_focused_window = mouse_focused_window; - let mut cursor_state = self_state.cursor_state.borrow_mut(); - let cursor_icon_name = cursor_state.cursor_icon_name.clone(); - if let Some(mut cursor) = cursor_state.cursor.as_mut() { - cursor.set_serial_id(serial); - cursor.set_icon(&wl_pointer, cursor_icon_name); + state.mouse_focused_window = Some(window.1.clone()); + let mut cursor_state = self_state.cursor_state.borrow_mut(); + let cursor_icon_name = cursor_state.cursor_icon_name.clone(); + if let Some(mut cursor) = cursor_state.cursor.as_mut() { + cursor.set_serial_id(serial); + cursor.set_icon(&wl_pointer, cursor_icon_name); + } } } - - state.mouse_location = Some(Point { - x: Pixels::from(surface_x), - y: Pixels::from(surface_y), - }); + state.windows = windows; + state.mouse_location = point(px(surface_x as f32), px(surface_y as f32)); } wl_pointer::Event::Motion { time, @@ -873,13 +883,11 @@ impl Dispatch for WaylandClientState { if state.mouse_focused_window.is_none() { return; } - state.mouse_location = Some(Point { - x: Pixels::from(surface_x), - y: Pixels::from(surface_y), - }); + state.mouse_location = point(px(surface_x as f32), px(surface_y as f32)); + state.mouse_focused_window.as_ref().unwrap().handle_input( PlatformInput::MouseMove(MouseMoveEvent { - position: state.mouse_location.unwrap(), + position: state.mouse_location, pressed_button: state.button_pressed, modifiers: state.modifiers, }), @@ -897,18 +905,34 @@ impl Dispatch for WaylandClientState { } => { let button = linux_button_to_gpui(button); let Some(button) = button else { return }; - if state.mouse_focused_window.is_none() || state.mouse_location.is_none() { + if state.mouse_focused_window.is_none() { return; } match button_state { wl_pointer::ButtonState::Pressed => { + let click_elapsed = state.click_state.last_click.elapsed(); + + if click_elapsed < DOUBLE_CLICK_INTERVAL + && is_within_click_distance( + state.click_state.last_location, + state.mouse_location, + ) + { + state.click_state.current_count += 1; + } else { + state.click_state.current_count = 1; + } + + state.click_state.last_click = Instant::now(); + state.click_state.last_location = state.mouse_location; + state.button_pressed = Some(button); state.mouse_focused_window.as_ref().unwrap().handle_input( PlatformInput::MouseDown(MouseDownEvent { button, - position: state.mouse_location.unwrap(), + position: state.mouse_location, modifiers: state.modifiers, - click_count: 1, + click_count: state.click_state.current_count, }), ); } @@ -917,9 +941,9 @@ impl Dispatch for WaylandClientState { state.mouse_focused_window.as_ref().unwrap().handle_input( PlatformInput::MouseUp(MouseUpEvent { button, - position: state.mouse_location.unwrap(), - modifiers: Modifiers::default(), - click_count: 1, + position: state.mouse_location, + modifiers: state.modifiers, + click_count: state.click_state.current_count, }), ); } @@ -945,20 +969,16 @@ impl Dispatch for WaylandClientState { axis: WEnum::Value(axis), value120, } => { - let focused_window = &state.mouse_focused_window; - let mouse_location = &state.mouse_location; - if let (Some(focused_window), Some(mouse_location)) = - (focused_window, mouse_location) - { + if let Some(focused_window) = &state.mouse_focused_window { let value = value120 as f64 * state.scroll_direction; focused_window.handle_input(PlatformInput::ScrollWheel(ScrollWheelEvent { - position: *mouse_location, + position: state.mouse_location, delta: match axis { wl_pointer::Axis::VerticalScroll => { - ScrollDelta::Pixels(Point::new(Pixels(0.0), Pixels(value as f32))) + ScrollDelta::Pixels(point(px(0.0), px(value as f32))) } wl_pointer::Axis::HorizontalScroll => { - ScrollDelta::Pixels(Point::new(Pixels(value as f32), Pixels(0.0))) + ScrollDelta::Pixels(point(px(value as f32), px(0.0))) } _ => unimplemented!(), }, @@ -979,21 +999,16 @@ impl Dispatch for WaylandClientState { { return; } - - let focused_window = &state.mouse_focused_window; - let mouse_location = &state.mouse_location; - if let (Some(focused_window), Some(mouse_location)) = - (focused_window, mouse_location) - { + if let Some(focused_window) = &state.mouse_focused_window { let value = value * state.scroll_direction; focused_window.handle_input(PlatformInput::ScrollWheel(ScrollWheelEvent { - position: *mouse_location, + position: state.mouse_location, delta: match axis { wl_pointer::Axis::VerticalScroll => { - ScrollDelta::Pixels(Point::new(Pixels(0.0), Pixels(value as f32))) + ScrollDelta::Pixels(point(px(0.0), px(value as f32))) } wl_pointer::Axis::HorizontalScroll => { - ScrollDelta::Pixels(Point::new(Pixels(value as f32), Pixels(0.0))) + ScrollDelta::Pixels(point(px(value as f32), px(0.0))) } _ => unimplemented!(), }, @@ -1003,17 +1018,15 @@ impl Dispatch for WaylandClientState { } } wl_pointer::Event::Leave { surface, .. } => { - let focused_window = &state.mouse_focused_window; - if let Some(focused_window) = focused_window { + if let Some(focused_window) = &state.mouse_focused_window { focused_window.handle_input(PlatformInput::MouseMove(MouseMoveEvent { - position: Point::::default(), + position: Point::default(), pressed_button: None, modifiers: Modifiers::default(), })); focused_window.set_focused(false); } state.mouse_focused_window = None; - state.mouse_location = None; } _ => {} } diff --git a/crates/gpui/src/platform/mac/platform.rs b/crates/gpui/src/platform/mac/platform.rs index 7d76b1d27a26c6d46edb4d355c92bcbf737e5680..0227a1061ac97a18eed2f13f913479dbebe65285 100644 --- a/crates/gpui/src/platform/mac/platform.rs +++ b/crates/gpui/src/platform/mac/platform.rs @@ -48,7 +48,6 @@ use std::{ rc::Rc, slice, str, sync::Arc, - time::Duration, }; use time::UtcOffset; @@ -715,13 +714,6 @@ impl Platform for MacPlatform { "macOS" } - fn double_click_interval(&self) -> Duration { - unsafe { - let double_click_interval: f64 = msg_send![class!(NSEvent), doubleClickInterval]; - Duration::from_secs_f64(double_click_interval) - } - } - fn os_version(&self) -> Result { unsafe { let process_info = NSProcessInfo::processInfo(nil); diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index 5e0c2146050f5d013d0faaa4617560fc6048e31e..649b74f7dd841cb230d749259798cf0864351958 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -12,7 +12,6 @@ use std::{ path::PathBuf, rc::{Rc, Weak}, sync::Arc, - time::Duration, }; /// TestPlatform implements the Platform trait for use in tests. @@ -303,10 +302,6 @@ impl Platform for TestPlatform { Task::ready(Ok(())) } - fn double_click_interval(&self) -> std::time::Duration { - Duration::from_millis(500) - } - fn register_url_scheme(&self, _: &str) -> Task> { unimplemented!() } diff --git a/crates/gpui/src/platform/windows/platform.rs b/crates/gpui/src/platform/windows/platform.rs index 472bfee12c853384d9f8207642dfcca5850d097b..ba3db5e8e729616e4c3156d54175d3db858a413b 100644 --- a/crates/gpui/src/platform/windows/platform.rs +++ b/crates/gpui/src/platform/windows/platform.rs @@ -638,11 +638,6 @@ impl Platform for WindowsPlatform { UtcOffset::from_hms(hours as _, minutes as _, 0).unwrap() } - fn double_click_interval(&self) -> Duration { - let millis = unsafe { GetDoubleClickTime() }; - Duration::from_millis(millis as _) - } - // todo(windows) fn path_for_auxiliary_executable(&self, name: &str) -> Result { Err(anyhow!("not yet implemented")) diff --git a/crates/gpui/src/platform/windows/window.rs b/crates/gpui/src/platform/windows/window.rs index 63ddad1617d19512e5dbb40706e4ae2a357648a7..11f3795cef3427ee386ef769c96d3b2389e08d2f 100644 --- a/crates/gpui/src/platform/windows/window.rs +++ b/crates/gpui/src/platform/windows/window.rs @@ -858,7 +858,7 @@ impl WindowsWindowInner { let width = rect.right - rect.left; let height = rect.bottom - rect.top; // this will emit `WM_SIZE` and `WM_MOVE` right here - // even before this funtion returns + // even before this function returns // the new size is handled in `WM_SIZE` unsafe { SetWindowPos(