From b4527730a90d08fb7ee89bd0650fccc0fcaf628f Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Fri, 17 Jan 2025 12:38:42 +0100 Subject: [PATCH 1/7] Use wrapper type for CFUUID (#4032) This no longer exposes `CGDisplayCreateUUIDFromDisplayID` and instead uses `CFUUID` to avoid a leak. Monitor comparisons should also be more stable now. --- src/platform_impl/macos/monitor.rs | 53 +++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/platform_impl/macos/monitor.rs b/src/platform_impl/macos/monitor.rs index e78d84f0be..bf7ee708ea 100644 --- a/src/platform_impl/macos/monitor.rs +++ b/src/platform_impl/macos/monitor.rs @@ -6,6 +6,7 @@ use std::fmt; use core_foundation::array::{CFArrayGetCount, CFArrayGetValueAtIndex}; use core_foundation::base::{CFRelease, TCFType}; use core_foundation::string::CFString; +use core_foundation::uuid::{CFUUIDGetUUIDBytes, CFUUID}; use core_graphics::display::{ CGDirectDisplayID, CGDisplay, CGDisplayBounds, CGDisplayCopyDisplayMode, }; @@ -100,15 +101,42 @@ impl VideoModeHandle { #[derive(Clone)] pub struct MonitorHandle(CGDirectDisplayID); +type MonitorUuid = [u8; 16]; + +impl MonitorHandle { + /// Internal comparisons of [`MonitorHandle`]s are done first requesting a UUID for the handle. + fn uuid(&self) -> MonitorUuid { + let cf_uuid = unsafe { + CFUUID::wrap_under_create_rule(ffi::CGDisplayCreateUUIDFromDisplayID(self.0)) + }; + let uuid = unsafe { CFUUIDGetUUIDBytes(cf_uuid.as_concrete_TypeRef()) }; + MonitorUuid::from([ + uuid.byte0, + uuid.byte1, + uuid.byte2, + uuid.byte3, + uuid.byte4, + uuid.byte5, + uuid.byte6, + uuid.byte7, + uuid.byte8, + uuid.byte9, + uuid.byte10, + uuid.byte11, + uuid.byte12, + uuid.byte13, + uuid.byte14, + uuid.byte15, + ]) + } +} + // `CGDirectDisplayID` changes on video mode change, so we cannot rely on that // for comparisons, but we can use `CGDisplayCreateUUIDFromDisplayID` to get an // unique identifier that persists even across system reboots impl PartialEq for MonitorHandle { fn eq(&self, other: &Self) -> bool { - unsafe { - ffi::CGDisplayCreateUUIDFromDisplayID(self.0) - == ffi::CGDisplayCreateUUIDFromDisplayID(other.0) - } + self.uuid() == other.uuid() } } @@ -122,18 +150,13 @@ impl PartialOrd for MonitorHandle { impl Ord for MonitorHandle { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - unsafe { - ffi::CGDisplayCreateUUIDFromDisplayID(self.0) - .cmp(&ffi::CGDisplayCreateUUIDFromDisplayID(other.0)) - } + self.uuid().cmp(&other.uuid()) } } impl std::hash::Hash for MonitorHandle { fn hash(&self, state: &mut H) { - unsafe { - ffi::CGDisplayCreateUUIDFromDisplayID(self.0).hash(state); - } + self.uuid().hash(state); } } @@ -296,13 +319,11 @@ impl MonitorHandle { } pub(crate) fn ns_screen(&self, mtm: MainThreadMarker) -> Option> { - let uuid = unsafe { ffi::CGDisplayCreateUUIDFromDisplayID(self.0) }; + let uuid = self.uuid(); NSScreen::screens(mtm).into_iter().find(|screen| { let other_native_id = get_display_id(screen); - let other_uuid = unsafe { - ffi::CGDisplayCreateUUIDFromDisplayID(other_native_id as CGDirectDisplayID) - }; - uuid == other_uuid + let other = MonitorHandle::new(other_native_id); + uuid == other.uuid() }) } } From 04d01a813d67d80dc5f6a3280969628a945be28e Mon Sep 17 00:00:00 2001 From: Tom Churchman Date: Fri, 17 Jan 2025 17:29:10 +0100 Subject: [PATCH 2/7] wayland: clear IME preedit only when necessary When all we'll be doing is setting a new preedit, the preedit doesn't have to be explicitly cleared first. This change is perhaps debatable. The direct reason for this is to make it easier to work around quirks/bugs: in Masonry we've found IBus appears to resend the IME preedit in response to `Window::set_ime_cursor_area` (`zwp_text_input_v3::set_cursor_rectangle`). Because currently the preedit is first cleared, a new IME cursor area is sent, which again causes IBus to resend the preedit. This can loop for a while. The Wayland protocol is mechanically quite prescriptive, it says for zwp_text_input_v3:event:done. > 1. Replace existing preedit string with the cursor. > 2. Delete requested surrounding text. > 3. Insert commit string with the cursor at its end. > 4. Calculate surrounding text to send. > 5. Insert new preedit text in cursor position. > 6. Place cursor inside preedit text. Winit currently doesn't do surrounding text, so 2. and 4. can be ignored. In Winit's IME model, without a commit, sending just the `Ime::Preedit` event without explicitly clearing is arguably still equivalent to doing 1., 5., and 6. --- src/changelog/unreleased.md | 4 ++++ .../linux/wayland/seat/text_input/mod.rs | 14 +++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index f3a0f6d2c3..fbcee62140 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -39,3 +39,7 @@ The migration guide could reference other migration examples in the current changelog entry. ## Unreleased + +### Changed + +- On Wayland, no longer send an explicit clearing `Ime::Preedit` just prior to a new `Ime::Preedit`. diff --git a/src/platform_impl/linux/wayland/seat/text_input/mod.rs b/src/platform_impl/linux/wayland/seat/text_input/mod.rs index 49d9363597..db724893c5 100644 --- a/src/platform_impl/linux/wayland/seat/text_input/mod.rs +++ b/src/platform_impl/linux/wayland/seat/text_input/mod.rs @@ -121,11 +121,15 @@ impl Dispatch for TextInputState { None => return, }; - // Clear preedit at the start of `Done`. - state.events_sink.push_window_event( - WindowEvent::Ime(Ime::Preedit(String::new(), None)), - window_id, - ); + // Clear preedit, unless all we'll be doing next is sending a new preedit. + if text_input_data.pending_commit.is_some() + || text_input_data.pending_preedit.is_none() + { + state.events_sink.push_window_event( + WindowEvent::Ime(Ime::Preedit(String::new(), None)), + window_id, + ); + } // Send `Commit`. if let Some(text) = text_input_data.pending_commit.take() { From 5112d26423579641a06890b45897e0ebbb591bda Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 31 Jan 2025 18:24:33 +0100 Subject: [PATCH 3/7] Document that we require cargo +nightly fmt (#4105) --- CONTRIBUTING.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 74351b2785..dfcb93a323 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,6 +19,11 @@ All patches have to be sent on Github as [pull requests][prs]. To simplify your life during review it's recommended to check the "give contributors write access to the branch" checkbox. +We use unstable Rustfmt options across the project, so please run +`cargo +nightly fmt` before submitting your work. If you are unable to do so, +the maintainers can do it for you before merging, just state so in your pull +request description. + #### Handling review During the review process certain events could require an action from your side, From e81cc6c1455cc2f3d473b54754924f5edd887dfe Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Mon, 3 Feb 2025 20:43:43 +0300 Subject: [PATCH 4/7] x11: fix crash with uim Let's just not forward events to the IME once the user requested that it should be disabled, though, still try to change its state explicitly. Fixes #4082. --- src/changelog/unreleased.md | 4 +++ src/platform_impl/linux/x11/ime/callbacks.rs | 8 ++---- src/platform_impl/linux/x11/ime/context.rs | 25 +++++++++-------- .../linux/x11/ime/input_method.rs | 4 +-- src/platform_impl/linux/x11/ime/mod.rs | 27 +++++-------------- 5 files changed, 27 insertions(+), 41 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index fbcee62140..38c723927f 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -43,3 +43,7 @@ changelog entry. ### Changed - On Wayland, no longer send an explicit clearing `Ime::Preedit` just prior to a new `Ime::Preedit`. + +### Fixed + +- On X11, fixed crash with uim diff --git a/src/platform_impl/linux/x11/ime/callbacks.rs b/src/platform_impl/linux/x11/ime/callbacks.rs index c75724ec83..fa0fa5c6fc 100644 --- a/src/platform_impl/linux/x11/ime/callbacks.rs +++ b/src/platform_impl/linux/x11/ime/callbacks.rs @@ -123,19 +123,15 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> { let is_allowed = old_context.as_ref().map(|old_context| old_context.is_allowed()).unwrap_or_default(); - // We can't use the style from the old context here, since it may change on reload, so - // pick style from the new XIM based on the old state. - let style = if is_allowed { new_im.preedit_style } else { new_im.none_style }; - let new_context = { let result = unsafe { ImeContext::new( xconn, - new_im.im, - style, + &new_im, *window, spot, (*inner).event_sender.clone(), + is_allowed, ) }; if result.is_err() { diff --git a/src/platform_impl/linux/x11/ime/context.rs b/src/platform_impl/linux/x11/ime/context.rs index 89a241cce3..e90836a5f2 100644 --- a/src/platform_impl/linux/x11/ime/context.rs +++ b/src/platform_impl/linux/x11/ime/context.rs @@ -5,10 +5,9 @@ use std::{mem, ptr}; use x11_dl::xlib::{XIMCallback, XIMPreeditCaretCallbackStruct, XIMPreeditDrawCallbackStruct}; -use crate::platform_impl::platform::x11::ime::input_method::{Style, XIMStyle}; -use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventSender}; - use super::{ffi, util, XConnection, XError}; +use crate::platform_impl::platform::x11::ime::input_method::{InputMethod, Style, XIMStyle}; +use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventSender}; /// IME creation error. #[derive(Debug)] @@ -184,7 +183,7 @@ struct ImeContextClientData { pub struct ImeContext { pub(crate) ic: ffi::XIC, pub(crate) ic_spot: ffi::XPoint, - pub(crate) style: Style, + pub(crate) allowed: bool, // Since the data is passed shared between X11 XIM callbacks, but couldn't be directly free // from there we keep the pointer to automatically deallocate it. _client_data: Box, @@ -193,11 +192,11 @@ pub struct ImeContext { impl ImeContext { pub(crate) unsafe fn new( xconn: &Arc, - im: ffi::XIM, - style: Style, + im: &InputMethod, window: ffi::Window, ic_spot: Option, event_sender: ImeEventSender, + allowed: bool, ) -> Result { let client_data = Box::into_raw(Box::new(ImeContextClientData { window, @@ -206,20 +205,24 @@ impl ImeContext { cursor_pos: 0, })); + let style = if allowed { im.preedit_style } else { im.none_style }; + let ic = match style as _ { Style::Preedit(style) => unsafe { ImeContext::create_preedit_ic( xconn, - im, + im.im, style, window, client_data as ffi::XPointer, ) }, Style::Nothing(style) => unsafe { - ImeContext::create_nothing_ic(xconn, im, style, window) + ImeContext::create_nothing_ic(xconn, im.im, style, window) + }, + Style::None(style) => unsafe { + ImeContext::create_none_ic(xconn, im.im, style, window) }, - Style::None(style) => unsafe { ImeContext::create_none_ic(xconn, im, style, window) }, } .ok_or(ImeContextCreationError::Null)?; @@ -228,7 +231,7 @@ impl ImeContext { let mut context = ImeContext { ic, ic_spot: ffi::XPoint { x: 0, y: 0 }, - style, + allowed, _client_data: unsafe { Box::from_raw(client_data) }, }; @@ -335,7 +338,7 @@ impl ImeContext { } pub fn is_allowed(&self) -> bool { - !matches!(self.style, Style::None(_)) + self.allowed } // Set the spot for preedit text. Setting spot isn't working with libX11 when preedit callbacks diff --git a/src/platform_impl/linux/x11/ime/input_method.rs b/src/platform_impl/linux/x11/ime/input_method.rs index 7f147bf1c4..b9d3ca7101 100644 --- a/src/platform_impl/linux/x11/ime/input_method.rs +++ b/src/platform_impl/linux/x11/ime/input_method.rs @@ -81,9 +81,7 @@ impl InputMethod { } let preedit_style = preedit_style.unwrap_or_else(|| none_style.unwrap()); - // Always initialize none style even when it's not advertised, since it seems to work - // regardless... - let none_style = none_style.unwrap_or(Style::None(XIM_NONE_STYLE)); + let none_style = none_style.unwrap_or(preedit_style); Some(InputMethod { im, _name: name, preedit_style, none_style }) } diff --git a/src/platform_impl/linux/x11/ime/mod.rs b/src/platform_impl/linux/x11/ime/mod.rs index 063598a3ea..0a419c8d50 100644 --- a/src/platform_impl/linux/x11/ime/mod.rs +++ b/src/platform_impl/linux/x11/ime/mod.rs @@ -10,15 +10,13 @@ use std::sync::Arc; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -use tracing::debug; - -use super::{ffi, util, XConnection, XError}; use self::callbacks::*; use self::context::ImeContext; pub use self::context::ImeContextCreationError; use self::inner::{close_im, ImeInner}; -use self::input_method::{PotentialInputMethods, Style}; +use self::input_method::PotentialInputMethods; +use super::{ffi, util, XConnection, XError}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -114,39 +112,26 @@ impl Ime { pub fn create_context( &mut self, window: ffi::Window, - with_preedit: bool, + with_ime: bool, ) -> Result { let context = if self.is_destroyed() { // Create empty entry in map, so that when IME is rebuilt, this window has a context. None } else { let im = self.inner.im.as_ref().unwrap(); - let style = if with_preedit { im.preedit_style } else { im.none_style }; let context = unsafe { ImeContext::new( &self.inner.xconn, - im.im, - style, + im, window, None, self.inner.event_sender.clone(), + with_ime, )? }; - // Check the state on the context, since it could fail to enable or disable preedit. - let event = if matches!(style, Style::None(_)) { - if with_preedit { - debug!("failed to create IME context with preedit support.") - } - ImeEvent::Disabled - } else { - if !with_preedit { - debug!("failed to create IME context without preedit support.") - } - ImeEvent::Enabled - }; - + let event = if context.is_allowed() { ImeEvent::Enabled } else { ImeEvent::Disabled }; self.inner.event_sender.send((window, event)).expect("Failed to send enabled event"); Some(context) From c4db9ae78536ecf644a96c834352ed40c41e35a6 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Wed, 5 Feb 2025 09:04:09 +0300 Subject: [PATCH 5/7] x11: fix modifiers replay The serial was not unique, thus leading to issues and replay being triggered for normal input. Track modifiers based on they keycodes instead, since it's more unique. Links: https://github.com/alacritty/alacritty/issues/8461 --- src/changelog/unreleased.md | 3 ++- src/platform_impl/linux/x11/event_processor.rs | 13 +++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 38c723927f..afe1edb60e 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -46,4 +46,5 @@ changelog entry. ### Fixed -- On X11, fixed crash with uim +- On X11, fix crash with uim. +- On X11, fix modifiers for keys that were sent by the same X11 request. diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index 438c808b8d..79f5c114f0 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -66,7 +66,10 @@ pub struct EventProcessor { pub active_window: Option, /// Latest modifiers we've sent for the user to trigger change in event. pub modifiers: Cell, - pub xfiltered_modifiers: VecDeque, + // Track modifiers based on keycodes. NOTE: that serials generally don't work for tracking + // since they are not unique and could be duplicated in case of sequence of key events is + // delivered at near the same time. + pub xfiltered_modifiers: VecDeque, pub xmodmap: util::ModifierKeymap, pub is_composing: bool, } @@ -163,13 +166,11 @@ impl EventProcessor { let xev: &XKeyEvent = xev.as_ref(); if self.xmodmap.is_modifier(xev.keycode as u8) { // Don't grow the buffer past the `MAX_MOD_REPLAY_LEN`. This could happen - // when the modifiers are consumed entirely or serials are altered. - // - // Both cases shouldn't happen in well behaving clients. + // when the modifiers are consumed entirely. if self.xfiltered_modifiers.len() == MAX_MOD_REPLAY_LEN { self.xfiltered_modifiers.pop_back(); } - self.xfiltered_modifiers.push_front(xev.serial); + self.xfiltered_modifiers.push_front(xev.keycode as u8); } } @@ -950,7 +951,7 @@ impl EventProcessor { // itself are out of sync due to XkbState being delivered before XKeyEvent, since it's // being replayed by the XIM, thus we should replay ourselves. let replay = if let Some(position) = - self.xfiltered_modifiers.iter().rev().position(|&s| s == xev.serial) + self.xfiltered_modifiers.iter().rev().position(|&s| s == xev.keycode as u8) { // We don't have to replay modifiers pressed before the current event if some events // were not forwarded to us, since their state is irrelevant. From 0d18724349f7bdd6db9b615cce488c7f605ec096 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Thu, 6 Feb 2025 10:56:10 +0100 Subject: [PATCH 6/7] ios: fix timers Fixes #4074. Fixes #3816. --- src/changelog/unreleased.md | 1 + src/platform_impl/ios/app_state.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index afe1edb60e..460d62f69e 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -48,3 +48,4 @@ changelog entry. - On X11, fix crash with uim. - On X11, fix modifiers for keys that were sent by the same X11 request. +- On iOS, fix high CPU usage even when using `ControlFlow::Wait`. diff --git a/src/platform_impl/ios/app_state.rs b/src/platform_impl/ios/app_state.rs index 30019a0701..e34bf43c97 100644 --- a/src/platform_impl/ios/app_state.rs +++ b/src/platform_impl/ios/app_state.rs @@ -375,6 +375,7 @@ impl AppState { (ControlFlow::Wait, ControlFlow::Wait) => { let start = Instant::now(); self.set_state(AppStateImpl::Waiting { waiting_handler, start }); + self.waker.stop() }, (ControlFlow::WaitUntil(old_instant), ControlFlow::WaitUntil(new_instant)) if old_instant == new_instant => From 7ff23bbc2c466db35704157b2b43b6874493473c Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Wed, 5 Feb 2025 14:58:57 +0300 Subject: [PATCH 7/7] Winit version 0.30.9 --- .github/workflows/ci.yml | 7 ++++++- Cargo.toml | 2 +- README.md | 2 +- src/changelog/unreleased.md | 10 ---------- src/changelog/v0.30.md | 12 ++++++++++++ src/platform/android.rs | 2 +- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 955344727f..f2e85a932c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,12 @@ jobs: - name: Generate lockfile # Also updates the crates.io index - run: cargo generate-lockfile && cargo update -p ahash --precise 0.8.7 && cargo update -p bumpalo --precise 3.14.0 + run: | + cargo generate-lockfile + cargo update -p ahash --precise 0.8.7 + cargo update -p bumpalo --precise 3.14.0 + cargo update -p objc2-encode --precise 4.0.3 + cargo update -p orbclient --precise 0.3.47 - name: Install GCC Multilib if: (matrix.platform.os == 'ubuntu-latest') && contains(matrix.platform.target, 'i686') diff --git a/Cargo.toml b/Cargo.toml index d6487655a0..19e53ed603 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "winit" -version = "0.30.8" +version = "0.30.9" authors = [ "The winit contributors", "Pierre Krieger ", diff --git a/README.md b/README.md index 136e75de93..966e36e5e1 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ ```toml [dependencies] -winit = "0.30.8" +winit = "0.30.9" ``` ## [Documentation](https://docs.rs/winit) diff --git a/src/changelog/unreleased.md b/src/changelog/unreleased.md index 460d62f69e..f3a0f6d2c3 100644 --- a/src/changelog/unreleased.md +++ b/src/changelog/unreleased.md @@ -39,13 +39,3 @@ The migration guide could reference other migration examples in the current changelog entry. ## Unreleased - -### Changed - -- On Wayland, no longer send an explicit clearing `Ime::Preedit` just prior to a new `Ime::Preedit`. - -### Fixed - -- On X11, fix crash with uim. -- On X11, fix modifiers for keys that were sent by the same X11 request. -- On iOS, fix high CPU usage even when using `ControlFlow::Wait`. diff --git a/src/changelog/v0.30.md b/src/changelog/v0.30.md index 2a791ec0d0..1abc1e8af2 100644 --- a/src/changelog/v0.30.md +++ b/src/changelog/v0.30.md @@ -1,3 +1,15 @@ +## 0.30.9 + +### Changed + +- On Wayland, no longer send an explicit clearing `Ime::Preedit` just prior to a new `Ime::Preedit`. + +### Fixed + +- On X11, fix crash with uim. +- On X11, fix modifiers for keys that were sent by the same X11 request. +- On iOS, fix high CPU usage even when using `ControlFlow::Wait`. + ## 0.30.8 ### Added diff --git a/src/platform/android.rs b/src/platform/android.rs index 71c3364559..55aa05561c 100644 --- a/src/platform/android.rs +++ b/src/platform/android.rs @@ -62,7 +62,7 @@ //! If your application is currently based on `NativeActivity` via the `ndk-glue` crate and building //! with `cargo apk`, then the minimal changes would be: //! 1. Remove `ndk-glue` from your `Cargo.toml` -//! 2. Enable the `"android-native-activity"` feature for Winit: `winit = { version = "0.30.8", +//! 2. Enable the `"android-native-activity"` feature for Winit: `winit = { version = "0.30.9", //! features = [ "android-native-activity" ] }` //! 3. Add an `android_main` entrypoint (as above), instead of using the '`[ndk_glue::main]` proc //! macro from `ndk-macros` (optionally add a dependency on `android_logger` and initialize