From 4b618bd6a6c27bb4198182a45ba84ecc095503d1 Mon Sep 17 00:00:00 2001 From: Benjamin Saunders Date: Thu, 9 Jan 2020 20:19:50 -0800 Subject: [PATCH 1/6] Don't discard high-precision cursor position data (#1375) * Don't discard high-precision cursor position data Most platforms (X11, wayland, macos, stdweb, ...) provide physical positions in f64 units, which can contain meaningful fractional data. For example, this can be empirically observed on modern X11 using a typical laptop touchpad. This is useful for e.g. content creation tools, where cursor motion might map to brush strokes on a canvas with higher-than-screen resolution, or positioning of an object in a vector space. * Update CHANGELOG.md Co-Authored-By: Murarth Co-authored-by: Murarth --- CHANGELOG.md | 1 + src/event.rs | 2 +- src/platform_impl/linux/x11/event_processor.rs | 9 +++------ src/platform_impl/web/stdweb/canvas.rs | 2 +- src/platform_impl/web/web_sys/canvas.rs | 2 +- src/platform_impl/windows/event_loop.rs | 4 ++-- 6 files changed, 9 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2be5dfad5..9df5e0376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop. - On X11, fix deadlock on window state when handling certain window events. - `WindowBuilder` now implements `Default`. +- **Breaking:** `WindowEvent::CursorMoved` changed to `f64` units, preserving high-precision data supplied by most backends # 0.20.0 (2020-01-05) diff --git a/src/event.rs b/src/event.rs index 9f30e8811..10ab62170 100644 --- a/src/event.rs +++ b/src/event.rs @@ -242,7 +242,7 @@ pub enum WindowEvent<'a> { /// (x,y) coords in pixels relative to the top-left corner of the window. Because the range of this data is /// limited by the display area and it may have been transformed by the OS to implement effects such as cursor /// acceleration, it should not be used to implement non-cursor-like interactions such as 3D camera control. - position: PhysicalPosition, + position: PhysicalPosition, #[deprecated = "Deprecated in favor of DeviceEvent::ModifiersChanged"] modifiers: ModifiersState, }, diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index 14fc1b1ff..424bb89c3 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -723,8 +723,7 @@ impl EventProcessor { util::maybe_change(&mut shared_state_lock.cursor_pos, new_cursor_pos) }); if cursor_moved == Some(true) { - let position = - PhysicalPosition::new(xev.event_x as i32, xev.event_y as i32); + let position = PhysicalPosition::new(xev.event_x, xev.event_y); callback(Event::WindowEvent { window_id, @@ -830,8 +829,7 @@ impl EventProcessor { event: CursorEntered { device_id }, }); - let position = - PhysicalPosition::new(xev.event_x as i32, xev.event_y as i32); + let position = PhysicalPosition::new(xev.event_x, xev.event_y); // The mods field on this event isn't actually populated, so query the // pointer device. In the future, we can likely remove this round-trip by @@ -899,8 +897,7 @@ impl EventProcessor { .map(|device| device.attachment) .unwrap_or(2); - let position = - PhysicalPosition::new(xev.event_x as i32, xev.event_y as i32); + let position = PhysicalPosition::new(xev.event_x, xev.event_y); callback(Event::WindowEvent { window_id, diff --git a/src/platform_impl/web/stdweb/canvas.rs b/src/platform_impl/web/stdweb/canvas.rs index 94586d048..35bc8044f 100644 --- a/src/platform_impl/web/stdweb/canvas.rs +++ b/src/platform_impl/web/stdweb/canvas.rs @@ -207,7 +207,7 @@ impl Canvas { pub fn on_cursor_move(&mut self, mut handler: F) where - F: 'static + FnMut(i32, PhysicalPosition, ModifiersState), + F: 'static + FnMut(i32, PhysicalPosition, ModifiersState), { // todo self.on_cursor_move = Some(self.add_event(move |event: PointerMoveEvent| { diff --git a/src/platform_impl/web/web_sys/canvas.rs b/src/platform_impl/web/web_sys/canvas.rs index 70b0ecc91..1d7e47463 100644 --- a/src/platform_impl/web/web_sys/canvas.rs +++ b/src/platform_impl/web/web_sys/canvas.rs @@ -216,7 +216,7 @@ impl Canvas { pub fn on_cursor_move(&mut self, mut handler: F) where - F: 'static + FnMut(i32, PhysicalPosition, ModifiersState), + F: 'static + FnMut(i32, PhysicalPosition, ModifiersState), { self.on_cursor_move = Some(self.add_event("pointermove", move |event: PointerEvent| { handler( diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index 8d90e2476..beb3cef50 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -855,8 +855,8 @@ unsafe extern "system" fn public_window_callback( }); } - let x = windowsx::GET_X_LPARAM(lparam) as i32; - let y = windowsx::GET_Y_LPARAM(lparam) as i32; + let x = windowsx::GET_X_LPARAM(lparam) as f64; + let y = windowsx::GET_Y_LPARAM(lparam) as f64; let position = PhysicalPosition::new(x, y); subclass_input.send_event(Event::WindowEvent { From 9e3844ddd9b31a083d1a2089c70be17aa4ed5787 Mon Sep 17 00:00:00 2001 From: Murarth Date: Thu, 9 Jan 2020 22:29:31 -0700 Subject: [PATCH 2/6] Fix warnings on all platforms (#1383) * Fix warnings on all platforms * Also fixed a trait impl bound that I noticed along the way --- src/event_loop.rs | 10 +++------- src/icon.rs | 17 ++++++----------- src/lib.rs | 1 + src/platform_impl/linux/x11/xdisplay.rs | 18 +++++++----------- src/platform_impl/macos/event.rs | 1 + src/platform_impl/macos/view.rs | 3 +++ .../web/event_loop/window_target.rs | 2 ++ src/platform_impl/windows/event_loop.rs | 5 +++++ 8 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/event_loop.rs b/src/event_loop.rs index e205c2316..8a05e3135 100644 --- a/src/event_loop.rs +++ b/src/event_loop.rs @@ -215,14 +215,10 @@ impl fmt::Debug for EventLoopProxy { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub struct EventLoopClosed(pub T); -impl fmt::Display for EventLoopClosed { +impl fmt::Display for EventLoopClosed { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", error::Error::description(self)) + f.write_str("Tried to wake up a closed `EventLoop`") } } -impl error::Error for EventLoopClosed { - fn description(&self) -> &str { - "Tried to wake up a closed `EventLoop`" - } -} +impl error::Error for EventLoopClosed {} diff --git a/src/icon.rs b/src/icon.rs index bbbb53f98..d512df1ad 100644 --- a/src/icon.rs +++ b/src/icon.rs @@ -29,31 +29,26 @@ pub enum BadIcon { impl fmt::Display for BadIcon { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let msg = match self { - &BadIcon::ByteCountNotDivisibleBy4 { byte_count } => format!( + match self { + BadIcon::ByteCountNotDivisibleBy4 { byte_count } => write!(f, "The length of the `rgba` argument ({:?}) isn't divisible by 4, making it impossible to interpret as 32bpp RGBA pixels.", byte_count, ), - &BadIcon::DimensionsVsPixelCount { + BadIcon::DimensionsVsPixelCount { width, height, width_x_height, pixel_count, - } => format!( + } => write!(f, "The specified dimensions ({:?}x{:?}) don't match the number of pixels supplied by the `rgba` argument ({:?}). For those dimensions, the expected pixel count is {:?}.", width, height, pixel_count, width_x_height, ), - }; - write!(f, "{}", msg) + } } } impl Error for BadIcon { - fn description(&self) -> &str { - "A valid icon cannot be created from these arguments" - } - - fn cause(&self) -> Option<&dyn Error> { + fn source(&self) -> Option<&(dyn Error + 'static)> { Some(self) } } diff --git a/src/lib.rs b/src/lib.rs index 9a9fc02c9..6011203e0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -123,6 +123,7 @@ #[allow(unused_imports)] #[macro_use] extern crate lazy_static; +#[allow(unused_imports)] #[macro_use] extern crate log; #[cfg(feature = "serde")] diff --git a/src/platform_impl/linux/x11/xdisplay.rs b/src/platform_impl/linux/x11/xdisplay.rs index 176323ec9..25065f045 100644 --- a/src/platform_impl/linux/x11/xdisplay.rs +++ b/src/platform_impl/linux/x11/xdisplay.rs @@ -111,12 +111,7 @@ pub struct XError { pub minor_code: u8, } -impl Error for XError { - #[inline] - fn description(&self) -> &str { - &self.description - } -} +impl Error for XError {} impl fmt::Display for XError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { @@ -144,17 +139,18 @@ impl From for XNotSupported { } } -impl Error for XNotSupported { - #[inline] - fn description(&self) -> &str { - match *self { +impl XNotSupported { + fn description(&self) -> &'static str { + match self { XNotSupported::LibraryOpenError(_) => "Failed to load one of xlib's shared libraries", XNotSupported::XOpenDisplayFailed => "Failed to open connection to X server", } } +} +impl Error for XNotSupported { #[inline] - fn cause(&self) -> Option<&dyn Error> { + fn source(&self) -> Option<&(dyn Error + 'static)> { match *self { XNotSupported::LibraryOpenError(ref err) => Some(err), _ => None, diff --git a/src/platform_impl/macos/event.rs b/src/platform_impl/macos/event.rs index 07a7bf0cd..6e231940f 100644 --- a/src/platform_impl/macos/event.rs +++ b/src/platform_impl/macos/event.rs @@ -287,6 +287,7 @@ pub unsafe fn modifier_event( let scancode = get_scancode(ns_event); let virtual_keycode = scancode_to_keycode(scancode); + #[allow(deprecated)] Some(WindowEvent::KeyboardInput { device_id: DEVICE_ID, input: KeyboardInput { diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index 2914c1926..4bc8bcc76 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -631,6 +631,7 @@ extern "C" fn key_down(this: &Object, _sel: Sel, event: id) { let is_repeat = msg_send![event, isARepeat]; + #[allow(deprecated)] let window_event = Event::WindowEvent { window_id, event: WindowEvent::KeyboardInput { @@ -683,6 +684,7 @@ extern "C" fn key_up(this: &Object, _sel: Sel, event: id) { let scancode = get_scancode(event) as u32; let virtual_keycode = retrieve_keycode(event); + #[allow(deprecated)] let window_event = Event::WindowEvent { window_id: WindowId(get_window_id(state.ns_window)), event: WindowEvent::KeyboardInput { @@ -797,6 +799,7 @@ extern "C" fn cancel_operation(this: &Object, _sel: Sel, _sender: id) { let event: id = msg_send![NSApp(), currentEvent]; + #[allow(deprecated)] let window_event = Event::WindowEvent { window_id: WindowId(get_window_id(state.ns_window)), event: WindowEvent::KeyboardInput { diff --git a/src/platform_impl/web/event_loop/window_target.rs b/src/platform_impl/web/event_loop/window_target.rs index 6596cebee..2395f3467 100644 --- a/src/platform_impl/web/event_loop/window_target.rs +++ b/src/platform_impl/web/event_loop/window_target.rs @@ -57,6 +57,7 @@ impl WindowTarget { let runner = self.runner.clone(); canvas.on_keyboard_press(move |scancode, virtual_keycode, modifiers| { + #[allow(deprecated)] runner.send_event(Event::WindowEvent { window_id: WindowId(id), event: WindowEvent::KeyboardInput { @@ -74,6 +75,7 @@ impl WindowTarget { let runner = self.runner.clone(); canvas.on_keyboard_release(move |scancode, virtual_keycode, modifiers| { + #[allow(deprecated)] runner.send_event(Event::WindowEvent { window_id: WindowId(id), event: WindowEvent::KeyboardInput { diff --git a/src/platform_impl/windows/event_loop.rs b/src/platform_impl/windows/event_loop.rs index beb3cef50..0d2b1497b 100644 --- a/src/platform_impl/windows/event_loop.rs +++ b/src/platform_impl/windows/event_loop.rs @@ -936,6 +936,7 @@ unsafe extern "system" fn public_window_callback( commctrl::DefSubclassProc(window, msg, wparam, lparam) } else { if let Some((scancode, vkey)) = process_key_params(wparam, lparam) { + #[allow(deprecated)] subclass_input.send_event(Event::WindowEvent { window_id: RootWindowId(WindowId(window)), event: WindowEvent::KeyboardInput { @@ -965,6 +966,7 @@ unsafe extern "system" fn public_window_callback( winuser::WM_KEYUP | winuser::WM_SYSKEYUP => { use crate::event::ElementState::Released; if let Some((scancode, vkey)) = process_key_params(wparam, lparam) { + #[allow(deprecated)] subclass_input.send_event(Event::WindowEvent { window_id: RootWindowId(WindowId(window)), event: WindowEvent::KeyboardInput { @@ -1330,6 +1332,7 @@ unsafe extern "system" fn public_window_callback( winuser::MapVirtualKeyA(windows_keycode as _, winuser::MAPVK_VK_TO_VSC); let virtual_keycode = event::vkey_to_winit_vkey(windows_keycode); + #[allow(deprecated)] subclass_input.send_event(Event::WindowEvent { window_id: RootWindowId(WindowId(window)), event: WindowEvent::KeyboardInput { @@ -1360,6 +1363,7 @@ unsafe extern "system" fn public_window_callback( winuser::MapVirtualKeyA(windows_keycode as _, winuser::MAPVK_VK_TO_VSC); let virtual_keycode = event::vkey_to_winit_vkey(windows_keycode); + #[allow(deprecated)] subclass_input.send_event(Event::WindowEvent { window_id: RootWindowId(WindowId(window)), event: WindowEvent::KeyboardInput { @@ -1913,6 +1917,7 @@ unsafe extern "system" fn thread_event_target_callback( }); } + #[allow(deprecated)] subclass_input.send_event(Event::DeviceEvent { device_id, event: Key(KeyboardInput { From 633d0deeaeee06b1e5b90e26d8c53dd39e4d755d Mon Sep 17 00:00:00 2001 From: hatoo Date: Sat, 11 Jan 2020 00:25:55 +0900 Subject: [PATCH 3/6] Fix run_return does not return on macOS unless it receives a message (#1380) * On MacOS, fix `run_return` not exit immediately * Add CHANGELOG --- CHANGELOG.md | 1 + src/platform_impl/macos/app_state.rs | 37 +++++++++++++++------------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df5e0376..ab17da071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- On macOS, fix `run_return` does not return unless it receives a message. - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop. - On X11, fix deadlock on window state when handling certain window events. - `WindowBuilder` now implements `Default`. diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs index 1cb4e5055..135a3d218 100644 --- a/src/platform_impl/macos/app_state.rs +++ b/src/platform_impl/macos/app_state.rs @@ -12,11 +12,13 @@ use std::{ }; use cocoa::{ - appkit::{NSApp, NSWindow}, - base::nil, - foundation::{NSAutoreleasePool, NSSize, NSString}, + appkit::{NSApp, NSEventType::NSApplicationDefined, NSWindow}, + base::{id, nil}, + foundation::{NSAutoreleasePool, NSPoint, NSSize}, }; +use objc::runtime::YES; + use crate::{ dpi::LogicalSize, event::{Event, StartCause, WindowEvent}, @@ -29,7 +31,6 @@ use crate::{ }, window::WindowId, }; -use objc::runtime::Object; lazy_static! { static ref HANDLER: Handler = Default::default(); @@ -339,21 +340,23 @@ impl AppState { let pool = NSAutoreleasePool::new(nil); - let windows: *const Object = msg_send![NSApp(), windows]; - let window: *const Object = msg_send![windows, objectAtIndex:0]; + let windows: id = msg_send![NSApp(), windows]; + let window: id = msg_send![windows, objectAtIndex:0]; assert_ne!(window, nil); - let title: *const Object = msg_send![window, title]; - assert_ne!(title, nil); - let postfix = NSString::alloc(nil).init_str("*"); - let some_unique_title: *const Object = - msg_send![title, stringByAppendingString: postfix]; - assert_ne!(some_unique_title, nil); - - // To stop event loop immediately, we need to send some UI event here. - let _: () = msg_send![window, setTitle: some_unique_title]; - // And restore it. - let _: () = msg_send![window, setTitle: title]; + let dummy_event: id = msg_send![class!(NSEvent), + otherEventWithType: NSApplicationDefined + location: NSPoint::new(0.0, 0.0) + modifierFlags: 0 + timestamp: 0 + windowNumber: 0 + context: nil + subtype: 0 + data1: 0 + data2: 0 + ]; + // To stop event loop immediately, we need to post some event here. + let _: () = msg_send![window, postEvent: dummy_event atStart: YES]; pool.drain(); }; From 1ddceeb0630994e4960493b1fe92bc72f8c5a54b Mon Sep 17 00:00:00 2001 From: Francesca Plebani Date: Fri, 10 Jan 2020 16:02:42 -0800 Subject: [PATCH 4/6] macOS: Unbundled window activation hack (#1318) * `ns_string_id_ref` convenience fn * Unbundled app activation hack * Greatly improved solution * Shortened names * Remove cruft I left here a year ago * Doc improvements * Use `AtomicBool` for `activationHackFlag` * Add CHANGELOG entry * Doc improvements * Fix `did_finish_launching` return type + delay more * More reliable activation checking * Fix merge goof * ...fix other merge goof Co-authored-by: Freya Gentz Co-authored-by: Bogaevsky --- CHANGELOG.md | 3 +- src/platform_impl/macos/activation_hack.rs | 208 +++++++++++++++++++++ src/platform_impl/macos/app.rs | 27 ++- src/platform_impl/macos/app_delegate.rs | 113 +++++------ src/platform_impl/macos/mod.rs | 1 + src/platform_impl/macos/monitor.rs | 7 +- src/platform_impl/macos/util/mod.rs | 18 +- src/platform_impl/macos/window.rs | 6 +- 8 files changed, 299 insertions(+), 84 deletions(-) create mode 100644 src/platform_impl/macos/activation_hack.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ab17da071..dec9f1552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- On macOS, fix issue where unbundled applications would sometimes open without being focused. - On macOS, fix `run_return` does not return unless it receives a message. - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop. - On X11, fix deadlock on window state when handling certain window events. @@ -26,7 +27,7 @@ - On all platforms except mobile and WASM, implement `Window::set_minimized`. - On X11, fix `CursorEntered` event being generated for non-winit windows. - On macOS, fix crash when starting maximized without decorations. -- On macOS, fix application not to terminate on `run_return`. +- On macOS, fix application not terminating on `run_return`. - On Wayland, fix cursor icon updates on window borders when using CSD. - On Wayland, under mutter(GNOME Wayland), fix CSD being behind the status bar, when starting window in maximized mode. - On Windows, theme the title bar according to whether the system theme is "Light" or "Dark". diff --git a/src/platform_impl/macos/activation_hack.rs b/src/platform_impl/macos/activation_hack.rs new file mode 100644 index 000000000..6cf1960cf --- /dev/null +++ b/src/platform_impl/macos/activation_hack.rs @@ -0,0 +1,208 @@ +// Normally when you run or distribute a macOS app, it's bundled: it's in one +// of those fun little folders that you have to right click "Show Package +// Contents" on, and usually contains myriad delights including, but not +// limited to, plists, icons, and of course, your beloved executable. However, +// when you use `cargo run`, your app is unbundled - it's just a lonely, bare +// executable. +// +// Apple isn't especially fond of unbundled apps, which is to say, they seem to +// barely be supported. If you move the mouse while opening a winit window from +// an unbundled app, the window will fail to activate and be in a grayed-out +// uninteractable state. Switching to another app and back is the only way to +// get the winit window into a normal state. None of this happens if the app is +// bundled, i.e. when running via Xcode. +// +// To workaround this, we just switch focus to the Dock and then switch back to +// our app. We only do this for unbundled apps, and only when they fail to +// become active on their own. +// +// This solution was derived from this Godot PR: +// https://github.com/godotengine/godot/pull/17187 +// (which appears to be based on https://stackoverflow.com/a/7602677) +// The curious specialness of mouse motions is touched upon here: +// https://github.com/godotengine/godot/issues/8653#issuecomment-358130512 +// +// We omit the 2nd step of the solution used in Godot, since it appears to have +// no effect - I speculate that it's just technical debt picked up from the SO +// answer; the API used is fairly exotic, and was historically used for very +// old versions of macOS that didn't support `activateIgnoringOtherApps`, i.e. +// in previous versions of SDL: +// https://hg.libsdl.org/SDL/file/c0bcc39a3491/src/video/cocoa/SDL_cocoaevents.m#l322 +// +// The `performSelector` delays in the Godot solution are used for sequencing, +// since refocusing the app will fail if the call is made before it finishes +// unfocusing. The delays used there are much smaller than the ones in the +// original SO answer, presumably because they found the fastest delay that +// works reliably through trial and error. Instead of using delays, we just +// handle `applicationDidResignActive`; despite the app not activating reliably, +// that still triggers when we switch focus to the Dock. +// +// The Godot solution doesn't appear to skip the hack when an unbundled app +// activates normally. Checking for this is difficult, since if you call +// `isActive` too early, it will always be `NO`. Even though we receive +// `applicationDidResignActive` when switching focus to the Dock, we never +// receive a preceding `applicationDidBecomeActive` if the app fails to +// activate normally. I wasn't able to find a proper point in time to perform +// the `isActive` check, so we instead check for the cause of the quirk: if +// any mouse motion occurs prior to us receiving `applicationDidResignActive`, +// we assume the app failed to become active. +// +// Fun fact: this issue is still present in GLFW +// (https://github.com/glfw/glfw/issues/1515) +// +// A similar issue was found in SDL, but the resolution doesn't seem to work +// for us: https://bugzilla.libsdl.org/show_bug.cgi?id=3051 + +use super::util; +use cocoa::{ + appkit::{NSApp, NSApplicationActivateIgnoringOtherApps}, + base::id, + foundation::NSUInteger, +}; +use objc::runtime::{Object, Sel, BOOL, NO, YES}; +use std::{ + os::raw::c_void, + sync::atomic::{AtomicBool, Ordering}, +}; + +#[derive(Debug, Default)] +pub struct State { + // Indicates that the hack has either completed or been skipped. + activated: AtomicBool, + // Indicates that the mouse has moved at some point in time. + mouse_moved: AtomicBool, + // Indicates that the hack is in progress, and that we should refocus when + // the app resigns active. + needs_refocus: AtomicBool, +} + +impl State { + pub fn name() -> &'static str { + "activationHackState" + } + + pub fn new() -> *mut c_void { + let this = Box::new(Self::default()); + Box::into_raw(this) as *mut c_void + } + + pub unsafe fn free(this: *mut Self) { + Box::from_raw(this); + } + + pub unsafe fn get_ptr(obj: &Object) -> *mut Self { + let this: *mut c_void = *(*obj).get_ivar(Self::name()); + assert!(!this.is_null(), "`activationHackState` pointer was null"); + this as *mut Self + } + + pub unsafe fn set_activated(obj: &Object, value: bool) { + let this = Self::get_ptr(obj); + (*this).activated.store(value, Ordering::Release); + } + + unsafe fn get_activated(obj: &Object) -> bool { + let this = Self::get_ptr(obj); + (*this).activated.load(Ordering::Acquire) + } + + pub unsafe fn set_mouse_moved(obj: &Object, value: bool) { + let this = Self::get_ptr(obj); + (*this).mouse_moved.store(value, Ordering::Release); + } + + pub unsafe fn get_mouse_moved(obj: &Object) -> bool { + let this = Self::get_ptr(obj); + (*this).mouse_moved.load(Ordering::Acquire) + } + + pub unsafe fn set_needs_refocus(obj: &Object, value: bool) { + let this = Self::get_ptr(obj); + (*this).needs_refocus.store(value, Ordering::Release); + } + + unsafe fn get_needs_refocus(obj: &Object) -> bool { + let this = Self::get_ptr(obj); + (*this).needs_refocus.load(Ordering::Acquire) + } +} + +// This is the entry point for the hack - if the app is unbundled and a mouse +// movement occurs before the app activates, it will trigger the hack. Because +// mouse movements prior to activation are the cause of this quirk, they should +// be a reliable way to determine if the hack needs to be performed. +pub extern "C" fn mouse_moved(this: &Object, _: Sel, _: id) { + trace!("Triggered `activationHackMouseMoved`"); + unsafe { + if !State::get_activated(this) { + // We check if `CFBundleName` is undefined to determine if the + // app is unbundled. + if let None = util::app_name() { + info!("App detected as unbundled"); + unfocus(this); + } else { + info!("App detected as bundled"); + } + } + } + trace!("Completed `activationHackMouseMoved`"); +} + +// Switch focus to the dock. +unsafe fn unfocus(this: &Object) { + // We only perform the hack if the app failed to activate, since otherwise, + // there'd be a gross (but fast) flicker as it unfocused and then refocused. + // However, we only enter this function if we detect mouse movement prior + // to activation, so this should always be `NO`. + // + // Note that this check isn't necessarily reliable in detecting a violation + // of the invariant above, since it's not guaranteed that activation will + // resolve before this point. In other words, it can spuriously return `NO`. + // This is also why the mouse motion approach was chosen, since it's not + // obvious how to sequence this check - if someone knows how to, then that + // would almost surely be a cleaner approach. + let active: BOOL = msg_send![NSApp(), isActive]; + if active == YES { + error!("Unbundled app activation hack triggered on an app that's already active; this shouldn't happen!"); + } else { + info!("Performing unbundled app activation hack"); + let dock_bundle_id = util::ns_string_id_ref("com.apple.dock"); + let dock_array: id = msg_send![ + class!(NSRunningApplication), + runningApplicationsWithBundleIdentifier: *dock_bundle_id + ]; + let dock_array_len: NSUInteger = msg_send![dock_array, count]; + if dock_array_len == 0 { + error!("The Dock doesn't seem to be running, so switching focus to it is impossible"); + } else { + State::set_needs_refocus(this, true); + let dock: id = msg_send![dock_array, objectAtIndex: 0]; + // This will trigger `applicationDidResignActive`, which will in + // turn call `refocus`. + let status: BOOL = msg_send![ + dock, + activateWithOptions: NSApplicationActivateIgnoringOtherApps + ]; + if status == NO { + error!("Failed to switch focus to Dock"); + } + } + } +} + +// Switch focus back to our app, causing the user to rejoice! +pub unsafe fn refocus(this: &Object) { + if State::get_needs_refocus(this) { + State::set_needs_refocus(this, false); + let app: id = msg_send![class!(NSRunningApplication), currentApplication]; + // Simply calling `NSApp activateIgnoringOtherApps` doesn't work. The + // nuanced difference isn't clear to me, but hey, I tried. + let success: BOOL = msg_send![ + app, + activateWithOptions: NSApplicationActivateIgnoringOtherApps + ]; + if success == NO { + error!("Failed to refocus app"); + } + } +} diff --git a/src/platform_impl/macos/app.rs b/src/platform_impl/macos/app.rs index fba48ae5c..4cec95124 100644 --- a/src/platform_impl/macos/app.rs +++ b/src/platform_impl/macos/app.rs @@ -2,17 +2,15 @@ use std::collections::VecDeque; use cocoa::{ appkit::{self, NSEvent}, - base::id, + base::{id, nil}, }; use objc::{ declare::ClassDecl, runtime::{Class, Object, Sel}, }; -use crate::{ - event::{DeviceEvent, ElementState, Event}, - platform_impl::platform::{app_state::AppState, event::EventWrapper, util, DEVICE_ID}, -}; +use super::{activation_hack, app_state::AppState, event::EventWrapper, util, DEVICE_ID}; +use crate::event::{DeviceEvent, ElementState, Event}; pub struct AppClass(pub *const Class); unsafe impl Send for AppClass {} @@ -51,14 +49,14 @@ extern "C" fn send_event(this: &Object, _sel: Sel, event: id) { let key_window: id = msg_send![this, keyWindow]; let _: () = msg_send![key_window, sendEvent: event]; } else { - maybe_dispatch_device_event(event); + maybe_dispatch_device_event(this, event); let superclass = util::superclass(this); let _: () = msg_send![super(this, superclass), sendEvent: event]; } } } -unsafe fn maybe_dispatch_device_event(event: id) { +unsafe fn maybe_dispatch_device_event(this: &Object, event: id) { let event_type = event.eventType(); match event_type { appkit::NSMouseMoved @@ -100,6 +98,21 @@ unsafe fn maybe_dispatch_device_event(event: id) { } AppState::queue_events(events); + + // Notify the delegate when the first mouse move occurs. This is + // used for the unbundled app activation hack, which needs to know + // if any mouse motions occurred prior to the app activating. + let delegate: id = msg_send![this, delegate]; + assert_ne!(delegate, nil); + if !activation_hack::State::get_mouse_moved(&*delegate) { + activation_hack::State::set_mouse_moved(&*delegate, true); + let () = msg_send![ + delegate, + performSelector: sel!(activationHackMouseMoved:) + withObject: nil + afterDelay: 0.0 + ]; + } } appkit::NSLeftMouseDown | appkit::NSRightMouseDown | appkit::NSOtherMouseDown => { let mut events = VecDeque::with_capacity(1); diff --git a/src/platform_impl/macos/app_delegate.rs b/src/platform_impl/macos/app_delegate.rs index c0c3a121d..9263cc121 100644 --- a/src/platform_impl/macos/app_delegate.rs +++ b/src/platform_impl/macos/app_delegate.rs @@ -1,10 +1,10 @@ +use super::{activation_hack, app_state::AppState}; use cocoa::base::id; use objc::{ declare::ClassDecl, - runtime::{Class, Object, Sel, BOOL, YES}, + runtime::{Class, Object, Sel}, }; - -use crate::platform_impl::platform::app_state::AppState; +use std::os::raw::c_void; pub struct AppDelegateClass(pub *const Class); unsafe impl Send for AppDelegateClass {} @@ -15,90 +15,67 @@ lazy_static! { let superclass = class!(NSResponder); let mut decl = ClassDecl::new("WinitAppDelegate", superclass).unwrap(); + decl.add_class_method(sel!(new), new as extern "C" fn(&Class, Sel) -> id); + decl.add_method(sel!(dealloc), dealloc as extern "C" fn(&Object, Sel)); decl.add_method( sel!(applicationDidFinishLaunching:), - did_finish_launching as extern "C" fn(&Object, Sel, id) -> BOOL, + did_finish_launching as extern "C" fn(&Object, Sel, id), ); decl.add_method( sel!(applicationDidBecomeActive:), did_become_active as extern "C" fn(&Object, Sel, id), ); decl.add_method( - sel!(applicationWillResignActive:), - will_resign_active as extern "C" fn(&Object, Sel, id), + sel!(applicationDidResignActive:), + did_resign_active as extern "C" fn(&Object, Sel, id), ); + + decl.add_ivar::<*mut c_void>(activation_hack::State::name()); decl.add_method( - sel!(applicationWillEnterForeground:), - will_enter_foreground as extern "C" fn(&Object, Sel, id), - ); - decl.add_method( - sel!(applicationDidEnterBackground:), - did_enter_background as extern "C" fn(&Object, Sel, id), - ); - decl.add_method( - sel!(applicationWillTerminate:), - will_terminate as extern "C" fn(&Object, Sel, id), + sel!(activationHackMouseMoved:), + activation_hack::mouse_moved as extern "C" fn(&Object, Sel, id), ); AppDelegateClass(decl.register()) }; } -extern "C" fn did_finish_launching(_: &Object, _: Sel, _: id) -> BOOL { - trace!("Triggered `didFinishLaunching`"); +extern "C" fn new(class: &Class, _: Sel) -> id { + unsafe { + let this: id = msg_send![class, alloc]; + let this: id = msg_send![this, init]; + (*this).set_ivar( + activation_hack::State::name(), + activation_hack::State::new(), + ); + this + } +} + +extern "C" fn dealloc(this: &Object, _: Sel) { + unsafe { + activation_hack::State::free(activation_hack::State::get_ptr(this)); + } +} + +extern "C" fn did_finish_launching(_: &Object, _: Sel, _: id) { + trace!("Triggered `applicationDidFinishLaunching`"); AppState::launched(); - trace!("Completed `didFinishLaunching`"); - YES + trace!("Completed `applicationDidFinishLaunching`"); } -extern "C" fn did_become_active(_: &Object, _: Sel, _: id) { - trace!("Triggered `didBecomeActive`"); - /*unsafe { - HANDLER.lock().unwrap().handle_nonuser_event(Event::Resumed) - }*/ - trace!("Completed `didBecomeActive`"); +extern "C" fn did_become_active(this: &Object, _: Sel, _: id) { + trace!("Triggered `applicationDidBecomeActive`"); + unsafe { + activation_hack::State::set_activated(this, true); + } + trace!("Completed `applicationDidBecomeActive`"); } -extern "C" fn will_resign_active(_: &Object, _: Sel, _: id) { - trace!("Triggered `willResignActive`"); - /*unsafe { - HANDLER.lock().unwrap().handle_nonuser_event(Event::Suspended) - }*/ - trace!("Completed `willResignActive`"); -} - -extern "C" fn will_enter_foreground(_: &Object, _: Sel, _: id) { - trace!("Triggered `willEnterForeground`"); - trace!("Completed `willEnterForeground`"); -} - -extern "C" fn did_enter_background(_: &Object, _: Sel, _: id) { - trace!("Triggered `didEnterBackground`"); - trace!("Completed `didEnterBackground`"); -} - -extern "C" fn will_terminate(_: &Object, _: Sel, _: id) { - trace!("Triggered `willTerminate`"); - /*unsafe { - let app: id = msg_send![class!(UIApplication), sharedApplication]; - let windows: id = msg_send![app, windows]; - let windows_enum: id = msg_send![windows, objectEnumerator]; - let mut events = Vec::new(); - loop { - let window: id = msg_send![windows_enum, nextObject]; - if window == nil { - break - } - let is_winit_window: BOOL = msg_send![window, isKindOfClass:class!(WinitUIWindow)]; - if is_winit_window == YES { - events.push(Event::WindowEvent { - window_id: RootWindowId(window.into()), - event: WindowEvent::Destroyed, - }); - } - } - HANDLER.lock().unwrap().handle_nonuser_events(events); - HANDLER.lock().unwrap().terminated(); - }*/ - trace!("Completed `willTerminate`"); +extern "C" fn did_resign_active(this: &Object, _: Sel, _: id) { + trace!("Triggered `applicationDidResignActive`"); + unsafe { + activation_hack::refocus(this); + } + trace!("Completed `applicationDidResignActive`"); } diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index c26385da8..8b5ee6a59 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -1,5 +1,6 @@ #![cfg(target_os = "macos")] +mod activation_hack; mod app; mod app_delegate; mod app_state; diff --git a/src/platform_impl/macos/monitor.rs b/src/platform_impl/macos/monitor.rs index d9f0baa77..817d38ee5 100644 --- a/src/platform_impl/macos/monitor.rs +++ b/src/platform_impl/macos/monitor.rs @@ -1,15 +1,14 @@ use std::{collections::VecDeque, fmt}; -use super::ffi; +use super::{ffi, util}; use crate::{ dpi::{PhysicalPosition, PhysicalSize}, monitor::{MonitorHandle as RootMonitorHandle, VideoMode as RootVideoMode}, - platform_impl::platform::util::IdRef, }; use cocoa::{ appkit::NSScreen, base::{id, nil}, - foundation::{NSString, NSUInteger}, + foundation::NSUInteger, }; use core_foundation::{ array::{CFArrayGetCount, CFArrayGetValueAtIndex}, @@ -303,7 +302,7 @@ impl MonitorHandle { let uuid = ffi::CGDisplayCreateUUIDFromDisplayID(self.0); let screens = NSScreen::screens(nil); let count: NSUInteger = msg_send![screens, count]; - let key = IdRef::new(NSString::alloc(nil).init_str("NSScreenNumber")); + let key = util::ns_string_id_ref("NSScreenNumber"); for i in 0..count { let screen = msg_send![screens, objectAtIndex: i as NSUInteger]; let device_description = NSScreen::deviceDescription(screen); diff --git a/src/platform_impl/macos/util/mod.rs b/src/platform_impl/macos/util/mod.rs index 3a454cbba..39a97c983 100644 --- a/src/platform_impl/macos/util/mod.rs +++ b/src/platform_impl/macos/util/mod.rs @@ -8,7 +8,7 @@ use std::ops::{BitAnd, Deref}; use cocoa::{ appkit::{NSApp, NSWindowStyleMask}, base::{id, nil}, - foundation::{NSAutoreleasePool, NSRect, NSUInteger}, + foundation::{NSAutoreleasePool, NSRect, NSString, NSUInteger}, }; use core_graphics::display::CGDisplay; use objc::runtime::{Class, Object, Sel, BOOL, YES}; @@ -91,6 +91,22 @@ pub fn bottom_left_to_top_left(rect: NSRect) -> f64 { CGDisplay::main().pixels_high() as f64 - (rect.origin.y + rect.size.height) } +pub unsafe fn ns_string_id_ref(s: &str) -> IdRef { + IdRef::new(NSString::alloc(nil).init_str(s)) +} + +pub unsafe fn app_name() -> Option { + let bundle: id = msg_send![class!(NSBundle), mainBundle]; + let dict: id = msg_send![bundle, infoDictionary]; + let key = ns_string_id_ref("CFBundleName"); + let app_name: id = msg_send![dict, objectForKey:*key]; + if app_name != nil { + Some(app_name) + } else { + None + } +} + pub unsafe fn superclass<'a>(this: &'a Object) -> &'a Class { let superclass: id = msg_send![this, superclass]; &*(superclass as *const _) diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index d32da6580..b77fc9077 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -36,7 +36,7 @@ use cocoa::{ NSWindow, NSWindowButton, NSWindowStyleMask, }, base::{id, nil}, - foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize, NSString}, + foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize}, }; use core_graphics::display::{CGDisplay, CGDisplayMode}; use objc::{ @@ -178,7 +178,7 @@ fn create_window( NO, )); let res = ns_window.non_nil().map(|ns_window| { - let title = IdRef::new(NSString::alloc(nil).init_str(&attrs.title)); + let title = util::ns_string_id_ref(&attrs.title); ns_window.setReleasedWhenClosed_(NO); ns_window.setTitle_(*title); ns_window.setAcceptsMouseMovedEvents_(YES); @@ -946,7 +946,7 @@ impl UnownedWindow { unsafe { let screen: id = msg_send![*self.ns_window, screen]; let desc = NSScreen::deviceDescription(screen); - let key = IdRef::new(NSString::alloc(nil).init_str("NSScreenNumber")); + let key = util::ns_string_id_ref("NSScreenNumber"); let value = NSDictionary::valueForKey_(desc, *key); let display_id = msg_send![value, unsignedIntegerValue]; RootMonitorHandle { From a6d180cefbc1b5046e324e033c93f529f416fdda Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Sat, 11 Jan 2020 11:45:52 +0300 Subject: [PATCH 5/6] On Wayland, fix coordinates in mouse events when scale factor isn't 1 (#1385) * On Wayland, fix coordinates in mouse events when scale factor isn't 1 * Refactor mouse_focus to be a surface instead of WindowId --- CHANGELOG.md | 1 + src/platform_impl/linux/wayland/pointer.rs | 28 +++++++++++++++------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dec9f1552..029bf1217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - On X11, fix deadlock on window state when handling certain window events. - `WindowBuilder` now implements `Default`. - **Breaking:** `WindowEvent::CursorMoved` changed to `f64` units, preserving high-precision data supplied by most backends +- On Wayland, fix coordinates in mouse events when scale factor isn't 1 # 0.20.0 (2020-01-05) diff --git a/src/platform_impl/linux/wayland/pointer.rs b/src/platform_impl/linux/wayland/pointer.rs index f84264c38..2ba6c934d 100644 --- a/src/platform_impl/linux/wayland/pointer.rs +++ b/src/platform_impl/linux/wayland/pointer.rs @@ -7,10 +7,13 @@ use crate::event::{ use super::{ event_loop::{CursorManager, EventsSink}, + make_wid, window::WindowStore, DeviceId, }; +use smithay_client_toolkit::surface; + use smithay_client_toolkit::reexports::client::protocol::{ wl_pointer::{self, Event as PtrEvent, WlPointer}, wl_seat, @@ -36,6 +39,7 @@ pub fn implement_pointer( cursor_manager: Arc>, ) -> WlPointer { seat.get_pointer(|pointer| { + // Currently focused winit surface let mut mouse_focus = None; let mut axis_buffer = None; let mut axis_discrete_buffer = None; @@ -53,8 +57,10 @@ pub fn implement_pointer( .. } => { let wid = store.find_wid(&surface); + if let Some(wid) = wid { - mouse_focus = Some(wid); + let scale_factor = surface::get_dpi_factor(&surface) as f64; + mouse_focus = Some(surface); // Reload cursor style only when we enter winit's surface. Calling // this function every time on `PtrEvent::Enter` could interfere with @@ -75,7 +81,8 @@ pub fn implement_pointer( device_id: crate::event::DeviceId( crate::platform_impl::DeviceId::Wayland(DeviceId), ), - position: (surface_x, surface_y).into(), + position: (surface_x * scale_factor, surface_y * scale_factor) + .into(), modifiers: modifiers_tracker.lock().unwrap().clone(), }, wid, @@ -101,13 +108,16 @@ pub fn implement_pointer( surface_y, .. } => { - if let Some(wid) = mouse_focus { + if let Some(surface) = mouse_focus.as_ref() { + let scale_factor = surface::get_dpi_factor(&surface) as f64; + let wid = make_wid(surface); sink.send_window_event( WindowEvent::CursorMoved { device_id: crate::event::DeviceId( crate::platform_impl::DeviceId::Wayland(DeviceId), ), - position: (surface_x, surface_y).into(), + position: (surface_x * scale_factor, surface_y * scale_factor) + .into(), modifiers: modifiers_tracker.lock().unwrap().clone(), }, wid, @@ -115,7 +125,7 @@ pub fn implement_pointer( } } PtrEvent::Button { button, state, .. } => { - if let Some(wid) = mouse_focus { + if let Some(surface) = mouse_focus.as_ref() { let state = match state { wl_pointer::ButtonState::Pressed => ElementState::Pressed, wl_pointer::ButtonState::Released => ElementState::Released, @@ -137,12 +147,13 @@ pub fn implement_pointer( button, modifiers: modifiers_tracker.lock().unwrap().clone(), }, - wid, + make_wid(surface), ); } } PtrEvent::Axis { axis, value, .. } => { - if let Some(wid) = mouse_focus { + if let Some(surface) = mouse_focus.as_ref() { + let wid = make_wid(surface); if pointer.as_ref().version() < 5 { let (mut x, mut y) = (0.0, 0.0); // old seat compatibility @@ -184,7 +195,8 @@ pub fn implement_pointer( PtrEvent::Frame => { let axis_buffer = axis_buffer.take(); let axis_discrete_buffer = axis_discrete_buffer.take(); - if let Some(wid) = mouse_focus { + if let Some(surface) = mouse_focus.as_ref() { + let wid = make_wid(surface); if let Some((x, y)) = axis_discrete_buffer { sink.send_window_event( WindowEvent::MouseWheel { From dc302b0db44a347dc5ccb06489d5262b1bc70ab9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ram=C3=B3n?= Date: Sun, 12 Jan 2020 18:50:34 +0100 Subject: [PATCH 6/6] Return physical position in `CursorMoved` on macOS (#1378) --- CHANGELOG.md | 1 + src/platform_impl/macos/view.rs | 31 ++++++++++--------------------- 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 029bf1217..a971693f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased +- On macOS, fix `CursorMoved` event reporting the cursor position using logical coordinates. - On macOS, fix issue where unbundled applications would sometimes open without being focused. - On macOS, fix `run_return` does not return unless it receives a message. - On Windows, fix bug where `RedrawRequested` would only get emitted every other iteration of the event loop. diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index 4bc8bcc76..c0b2adc54 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -17,6 +17,7 @@ use objc::{ }; use crate::{ + dpi::LogicalPosition, event::{ DeviceEvent, ElementState, Event, KeyboardInput, ModifiersState, MouseButton, MouseScrollDelta, TouchPhase, VirtualKeyCode, WindowEvent, @@ -59,6 +60,12 @@ struct ViewState { tracking_rect: Option, } +impl ViewState { + fn get_scale_factor(&self) -> f64 { + (unsafe { NSWindow::backingScaleFactor(self.ns_window) }) as f64 + } +} + pub fn new_view(ns_window: id) -> (IdRef, Weak>) { let cursor_state = Default::default(); let cursor_access = Arc::downgrade(&cursor_state); @@ -885,12 +892,13 @@ fn mouse_motion(this: &Object, event: id) { let x = view_point.x as f64; let y = view_rect.size.height as f64 - view_point.y as f64; + let logical_position = LogicalPosition::new(x, y); let window_event = Event::WindowEvent { window_id: WindowId(get_window_id(state.ns_window)), event: WindowEvent::CursorMoved { device_id: DEVICE_ID, - position: (x, y).into(), + position: logical_position.to_physical(state.get_scale_factor()), modifiers: event_mods(event), }, }; @@ -928,27 +936,8 @@ extern "C" fn mouse_entered(this: &Object, _sel: Sel, event: id) { }, }; - let move_event = { - let window_point = event.locationInWindow(); - let view_point: NSPoint = msg_send![this, - convertPoint:window_point - fromView:nil // convert from window coordinates - ]; - let view_rect: NSRect = msg_send![this, frame]; - let x = view_point.x as f64; - let y = (view_rect.size.height - view_point.y) as f64; - Event::WindowEvent { - window_id: WindowId(get_window_id(state.ns_window)), - event: WindowEvent::CursorMoved { - device_id: DEVICE_ID, - position: (x, y).into(), - modifiers: event_mods(event), - }, - } - }; - AppState::queue_event(EventWrapper::StaticEvent(enter_event)); - AppState::queue_event(EventWrapper::StaticEvent(move_event)); + mouse_motion(this, event); } trace!("Completed `mouseEntered`"); }