From e22c76b3ac8a00e505b6c769bc391f6742a27bc7 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 18 Mar 2022 14:50:24 +0100 Subject: [PATCH] macOS `set_ime_position` fixes (#2180) * Use NSView's inputContext instead of creating our own This means that `set_ime_position` now properly invalidates the character coordinates. * Make `set_ime_position` robust against moving windows --- CHANGELOG.md | 1 + src/platform_impl/macos/util/mod.rs | 6 ----- src/platform_impl/macos/view.rs | 34 ++++++++++++++--------------- src/platform_impl/macos/window.rs | 13 +---------- 4 files changed, 19 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ad24fcf..beca7e5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre # Unreleased +- macOS: Remove the need to call `set_ime_position` after moving the window. - Added `Window::is_visible`. - Added `Window::is_resizable`. - Added `Window::is_decorated`. diff --git a/src/platform_impl/macos/util/mod.rs b/src/platform_impl/macos/util/mod.rs index b3e27439..7747a57b 100644 --- a/src/platform_impl/macos/util/mod.rs +++ b/src/platform_impl/macos/util/mod.rs @@ -147,12 +147,6 @@ pub unsafe fn superclass(this: &Object) -> &Class { &*superclass } -pub unsafe fn create_input_context(view: id) -> IdRef { - let input_context: id = msg_send![class!(NSTextInputContext), alloc]; - let input_context: id = msg_send![input_context, initWithClient: view]; - IdRef::new(input_context) -} - #[allow(dead_code)] pub unsafe fn open_emoji_picker() { let () = msg_send![NSApp(), orderFrontCharacterPalette: nil]; diff --git a/src/platform_impl/macos/view.rs b/src/platform_impl/macos/view.rs index 7046e458..2d879d58 100644 --- a/src/platform_impl/macos/view.rs +++ b/src/platform_impl/macos/view.rs @@ -53,7 +53,8 @@ impl Default for CursorState { pub(super) struct ViewState { ns_window: id, pub cursor_state: Arc>, - ime_spot: Option<(f64, f64)>, + /// The position of the candidate window. + ime_position: LogicalPosition, raw_characters: Option, pub(super) modifiers: ModifiersState, tracking_rect: Option, @@ -71,7 +72,8 @@ pub fn new_view(ns_window: id) -> (IdRef, Weak>) { let state = ViewState { ns_window, cursor_state, - ime_spot: None, + // By default, open the candidate window in the top left corner + ime_position: LogicalPosition::new(0.0, 0.0), raw_characters: None, modifiers: Default::default(), tracking_rect: None, @@ -87,14 +89,11 @@ pub fn new_view(ns_window: id) -> (IdRef, Weak>) { } } -pub unsafe fn set_ime_position(ns_view: id, input_context: id, x: f64, y: f64) { +pub unsafe fn set_ime_position(ns_view: id, position: LogicalPosition) { let state_ptr: *mut c_void = *(*ns_view).get_mut_ivar("winitState"); let state = &mut *(state_ptr as *mut ViewState); - let content_rect = - NSWindow::contentRectForFrameRect_(state.ns_window, NSWindow::frame(state.ns_window)); - let base_x = content_rect.origin.x as f64; - let base_y = (content_rect.origin.y + content_rect.size.height) as f64; - state.ime_spot = Some((base_x + x, base_y - y)); + state.ime_position = position; + let input_context: id = msg_send![ns_view, inputContext]; let _: () = msg_send![input_context, invalidateCharacterCoordinates]; } @@ -480,15 +479,16 @@ extern "C" fn first_rect_for_character_range( unsafe { let state_ptr: *mut c_void = *this.get_ivar("winitState"); let state = &mut *(state_ptr as *mut ViewState); - let (x, y) = state.ime_spot.unwrap_or_else(|| { - let content_rect = NSWindow::contentRectForFrameRect_( - state.ns_window, - NSWindow::frame(state.ns_window), - ); - let x = content_rect.origin.x; - let y = util::bottom_left_to_top_left(content_rect); - (x, y) - }); + let content_rect = + NSWindow::contentRectForFrameRect_(state.ns_window, NSWindow::frame(state.ns_window)); + let base_x = content_rect.origin.x as f64; + let base_y = (content_rect.origin.y + content_rect.size.height) as f64; + let x = base_x + state.ime_position.x; + let y = base_y - state.ime_position.y; + // This is not ideal: We _should_ return a different position based on + // the currently selected character (which varies depending on the type + // and size of the character), but in the current `winit` API there is + // no way to express this. Same goes for the `NSSize`. NSRect::new(NSPoint::new(x as _, y as _), NSSize::new(0.0, 0.0)) } } diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs index 4ac5532d..147ab971 100644 --- a/src/platform_impl/macos/window.rs +++ b/src/platform_impl/macos/window.rs @@ -363,7 +363,6 @@ impl Drop for SharedStateMutexGuard<'_> { pub struct UnownedWindow { pub ns_window: IdRef, // never changes pub ns_view: IdRef, // never changes - input_context: IdRef, // never changes shared_state: Arc>, decorations: AtomicBool, cursor_state: Weak>, @@ -398,8 +397,6 @@ impl UnownedWindow { ns_window.setInitialFirstResponder_(*ns_view); } - let input_context = unsafe { util::create_input_context(*ns_view) }; - let scale_factor = unsafe { NSWindow::backingScaleFactor(*ns_window) as f64 }; unsafe { @@ -442,7 +439,6 @@ impl UnownedWindow { let window = Arc::new(UnownedWindow { ns_view, ns_window, - input_context, shared_state: Arc::new(Mutex::new(win_attribs.into())), decorations: AtomicBool::new(decorations), cursor_state, @@ -1046,14 +1042,7 @@ impl UnownedWindow { pub fn set_ime_position(&self, spot: Position) { let scale_factor = self.scale_factor(); let logical_spot = spot.to_logical(scale_factor); - unsafe { - view::set_ime_position( - *self.ns_view, - *self.input_context, - logical_spot.x, - logical_spot.y, - ); - } + unsafe { view::set_ime_position(*self.ns_view, logical_spot) }; } #[inline]