Winit version 0.28.4

The new analysis suggests that we can remove mut.

CHANGELOG.md

@@ -8,6 +8,16 @@ And please only add new entries to the top of this list, right below the `# Unre
 
 # Unreleased
 
+# 0.28.4
+
+- On macOS, fix empty marked text blocking regular input.
+- On macOS, fixed potential panic when getting refresh rate.
+- On macOS, fix crash when calling `Window::set_ime_position` from another thread.
+
+# 0.28.3
+
+- Fix macOS memory leaks.
+
 # 0.28.2
 
 - Implement `HasRawDisplayHandle` for `EventLoop`.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "winit"
-version = "0.28.2"
+version = "0.28.4"
 authors = ["The winit contributors", "Pierre Krieger <pierre.krieger1708@gmail.com>"]
 description = "Cross-platform window creation library."
 edition = "2021"
@@ -68,7 +68,7 @@ ndk = "0.7.0"
 
 [target.'cfg(any(target_os = "ios", target_os = "macos"))'.dependencies]
 core-foundation = "0.9.3"
-objc2 = "=0.3.0-beta.3"
+objc2 = ">=0.3.0-beta.3, <0.3.0-beta.4" # Allow `0.3.0-beta.3.patch-leaks`
 
 [target.'cfg(target_os = "macos")'.dependencies]
 core-graphics = "0.22.3"

README.md

@@ -6,7 +6,7 @@
 
 ```toml
 [dependencies]
-winit = "0.28.2"
+winit = "0.28.4"
 ```
 
 ## [Documentation](https://docs.rs/winit)

src/platform_impl/linux/wayland/window/mod.rs

@@ -170,12 +170,12 @@ impl Window {
                     use sctk::window::{Event, State};
 
                     let winit_state = dispatch_data.get::<WinitState>().unwrap();
-                    let mut window_compositor_update = winit_state
+                    let window_compositor_update = winit_state
                         .window_compositor_updates
                         .get_mut(&window_id)
                         .unwrap();
 
-                    let mut window_user_requests = winit_state
+                    let window_user_requests = winit_state
                         .window_user_requests
                         .get_mut(&window_id)
                         .unwrap();

src/platform_impl/linux/x11/window.rs

@@ -384,7 +384,7 @@ impl UnownedWindow {
                     }
                 }
 
-                let mut shared_state = window.shared_state.get_mut().unwrap();
+                let shared_state = window.shared_state.get_mut().unwrap();
                 shared_state.min_inner_size = min_inner_size.map(Into::into);
                 shared_state.max_inner_size = max_inner_size.map(Into::into);
                 shared_state.resize_increments = window_attrs.resize_increments;

src/platform_impl/macos/monitor.rs

@@ -230,7 +230,9 @@ impl MonitorHandle {
                 return None;
             }
 
-            Some((time.time_scale as i64 / time.time_value * 1000) as u32)
+            (time.time_scale as i64)
+                .checked_div(time.time_value)
+                .map(|v| (v * 1000) as u32)
         }
     }
 

src/platform_impl/macos/util/async.rs

@@ -2,10 +2,10 @@ use std::ops::Deref;
 
 use dispatch::Queue;
 use objc2::foundation::{is_main_thread, CGFloat, NSPoint, NSSize, NSString};
-use objc2::rc::autoreleasepool;
+use objc2::rc::{autoreleasepool, Id};
 
 use crate::{
-    dpi::LogicalSize,
+    dpi::{LogicalPosition, LogicalSize},
     platform_impl::platform::{
         appkit::{NSScreen, NSWindow, NSWindowLevel, NSWindowStyleMask},
         ffi,
@@ -201,3 +201,11 @@ pub(crate) fn close_sync(window: &NSWindow) {
         });
     });
 }
+
+pub(crate) fn set_ime_position_sync(window: &WinitWindow, logical_spot: LogicalPosition<f64>) {
+    let window = MainThreadSafe(window);
+    run_on_main(move || {
+        // TODO(madsmtm): Remove the need for this
+        unsafe { Id::from_shared(window.view()) }.set_ime_position(logical_spot);
+    });
+}

src/platform_impl/macos/view.rs

@@ -7,7 +7,7 @@ use objc2::foundation::{
     NSArray, NSAttributedString, NSAttributedStringKey, NSCopying, NSMutableAttributedString,
     NSObject, NSPoint, NSRange, NSRect, NSSize, NSString, NSUInteger,
 };
-use objc2::rc::{Id, Owned, Shared};
+use objc2::rc::{Id, Owned, Shared, WeakId};
 use objc2::runtime::{Object, Sel};
 use objc2::{class, declare_class, msg_send, msg_send_id, sel, ClassType};
 
@@ -55,8 +55,9 @@ enum ImeState {
     /// The IME events are disabled, so only `ReceivedCharacter` is being sent to the user.
     Disabled,
 
-    /// The IME events are enabled.
-    Enabled,
+    /// The ground state of enabled IME input. It means that both Preedit and regular keyboard
+    /// input could be start from it.
+    Ground,
 
     /// The IME is in preedit.
     Preedit,
@@ -136,7 +137,8 @@ declare_class!(
     #[derive(Debug)]
     #[allow(non_snake_case)]
     pub(super) struct WinitView {
-        _ns_window: IvarDrop<Id<WinitWindow, Shared>>,
+        // Weak reference because the window keeps a strong reference to the view
+        _ns_window: IvarDrop<Box<WeakId<WinitWindow>>>,
         pub(super) state: IvarDrop<Box<ViewState>>,
         marked_text: IvarDrop<Id<NSMutableAttributedString, Owned>>,
         accepts_first_mouse: bool,
@@ -167,7 +169,10 @@ declare_class!(
                     forward_key_to_app: false,
                 };
 
-                Ivar::write(&mut this._ns_window, window.retain());
+                Ivar::write(
+                    &mut this._ns_window,
+                    Box::new(WeakId::new(&window.retain())),
+                );
                 Ivar::write(&mut this.state, Box::new(state));
                 Ivar::write(&mut this.marked_text, NSMutableAttributedString::new());
                 Ivar::write(&mut this.accepts_first_mouse, accepts_first_mouse);
@@ -321,7 +326,7 @@ declare_class!(
                 )
             };
 
-            // Update marked text
+            // Update marked text.
             *self.marked_text = marked_text;
 
             // Notify IME is active if application still doesn't know it.
@@ -330,10 +335,11 @@ declare_class!(
                 self.queue_event(WindowEvent::Ime(Ime::Enabled));
             }
 
-            // Don't update self.state to preedit when we've just commited a string, since the following
-            // preedit string will be None anyway.
-            if self.state.ime_state != ImeState::Commited {
+            if self.hasMarkedText() {
                 self.state.ime_state = ImeState::Preedit;
+            } else {
+                // In case the preedit was cleared, set IME into the Ground state.
+                self.state.ime_state = ImeState::Ground;
             }
 
             // Empty string basically means that there's no preedit, so indicate that by sending
@@ -359,7 +365,7 @@ declare_class!(
             self.queue_event(WindowEvent::Ime(Ime::Preedit(String::new(), None)));
             if self.is_ime_enabled() {
                 // Leave the Preedit self.state
-                self.state.ime_state = ImeState::Enabled;
+                self.state.ime_state = ImeState::Ground;
             } else {
                 warn!("Expected to have IME enabled when receiving unmarkText");
             }
@@ -447,8 +453,8 @@ declare_class!(
             self.state.forward_key_to_app = true;
 
             if self.hasMarkedText() && self.state.ime_state == ImeState::Preedit {
-                // Leave preedit so that we also report the keyup for this key
-                self.state.ime_state = ImeState::Enabled;
+                // Leave preedit so that we also report the key-up for this key.
+                self.state.ime_state = ImeState::Ground;
             }
         }
     }
@@ -463,7 +469,6 @@ declare_class!(
                 self.state.input_source = input_source;
                 self.queue_event(WindowEvent::Ime(Ime::Disabled));
             }
-            let was_in_preedit = self.state.ime_state == ImeState::Preedit;
 
             // Get the characters from the event.
             let ev_mods = event_mods(event);
@@ -484,7 +489,6 @@ declare_class!(
             // we must send the `KeyboardInput` event during IME if it triggered
             // `doCommandBySelector`. (doCommandBySelector means that the keyboard input
             // is not handled by IME and should be handled by the application)
-            let mut text_commited = false;
             if self.state.ime_allowed {
                 let new_event = if ignore_alt_characters {
                     replace_event_chars(event, &characters)
@@ -499,21 +503,25 @@ declare_class!(
                 if self.state.ime_state == ImeState::Commited {
                     // Remove any marked text, so normal input can continue.
                     *self.marked_text = NSMutableAttributedString::new();
-                    self.state.ime_state = ImeState::Enabled;
-                    text_commited = true;
                 }
             }
 
-            let now_in_preedit = self.state.ime_state == ImeState::Preedit;
-
             let scancode = event.scancode() as u32;
             let virtual_keycode = retrieve_keycode(event);
 
             self.update_potentially_stale_modifiers(event);
 
-            let ime_related = was_in_preedit || now_in_preedit || text_commited;
+            let had_ime_input = match self.state.ime_state {
+                ImeState::Commited => {
+                    // Allow normal input after the commit.
+                    self.state.ime_state = ImeState::Ground;
+                    true
+                }
+                ImeState::Preedit => true,
+                _ => false,
+            };
 
-            if !ime_related || self.state.forward_key_to_app || !self.state.ime_allowed {
+            if !had_ime_input || self.state.forward_key_to_app {
                 #[allow(deprecated)]
                 self.queue_event(WindowEvent::KeyboardInput {
                     device_id: DEVICE_ID,
@@ -540,8 +548,8 @@ declare_class!(
 
             self.update_potentially_stale_modifiers(event);
 
-            // We want to send keyboard input when we are not currently in preedit
-            if self.state.ime_state != ImeState::Preedit {
+            // We want to send keyboard input when we are currently in the ground state.
+            if self.state.ime_state == ImeState::Ground {
                 #[allow(deprecated)]
                 self.queue_event(WindowEvent::KeyboardInput {
                     device_id: DEVICE_ID,
@@ -873,11 +881,11 @@ impl WinitView {
         // (which is incompatible with `frameDidChange:`)
         //
         // unsafe { msg_send_id![self, window] }
-        (*self._ns_window).clone()
+        self._ns_window.load().expect("view to have a window")
     }
 
     fn window_id(&self) -> WindowId {
-        WindowId(self._ns_window.id())
+        WindowId(self.window().id())
     }
 
     fn queue_event(&self, event: WindowEvent<'static>) {

src/platform_impl/macos/window.rs

@@ -1165,8 +1165,7 @@ impl WinitWindow {
     pub fn set_ime_position(&self, spot: Position) {
         let scale_factor = self.scale_factor();
         let logical_spot = spot.to_logical(scale_factor);
-        // TODO(madsmtm): Remove the need for this
-        unsafe { Id::from_shared(self.view()) }.set_ime_position(logical_spot);
+        util::set_ime_position_sync(self, logical_spot);
     }
 
     #[inline]