Revert "Add composition event on macOS (#1979)" (#2119)

This reverts commit 8afeb910bd.

Reverting because this change made Pinyin input unusable
(only latin characters showed even after selecting the
desired Chinese character)

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Unreleased
 
 - On X11, add mappings for numpad comma, numpad enter, numlock and pause.
+- On macOS, fix Pinyin IME input by reverting a change that intended to improve IME.
 
 # 0.26.0 (2021-12-01)
 

Cargo.toml

@@ -48,7 +48,6 @@ cocoa = "0.24"
 core-foundation = "0.9"
 core-graphics = "0.22"
 dispatch = "0.2.0"
-block = "0.1"
 
 [target.'cfg(target_os = "macos")'.dependencies.core-video-sys]
 version = "0.1.4"

src/platform_impl/macos/ffi.rs

@@ -118,8 +118,6 @@ pub enum NSWindowLevel {
     NSScreenSaverWindowLevel = kCGScreenSaverWindowLevelKey as _,
 }
 
-pub const NSStringEnumerationByComposedCharacterSequences: NSUInteger = 2;
-
 pub type CGDisplayFadeInterval = f32;
 pub type CGDisplayReservationInterval = f32;
 pub type CGDisplayBlendFraction = f32;

src/platform_impl/macos/util/mod.rs

@@ -3,13 +3,8 @@ mod cursor;
 
 pub use self::{cursor::*, r#async::*};
 
-use std::{
-    cell::Cell,
-    ops::{BitAnd, Deref},
-    rc::Rc,
-};
+use std::ops::{BitAnd, Deref};
 
-use block::ConcreteBlock;
 use cocoa::{
     appkit::{NSApp, NSWindowStyleMask},
     base::{id, nil},
@@ -18,12 +13,8 @@ use cocoa::{
 use core_graphics::display::CGDisplay;
 use objc::runtime::{Class, Object, Sel, BOOL, YES};
 
-use crate::{
-    dpi::LogicalPosition,
-    platform_impl::platform::ffi::{
-        self, NSRange, NSStringEnumerationByComposedCharacterSequences,
-    },
-};
+use crate::dpi::LogicalPosition;
+use crate::platform_impl::platform::ffi;
 
 // Replace with `!` once stable
 #[derive(Debug)]
@@ -113,31 +104,6 @@ pub unsafe fn ns_string_id_ref(s: &str) -> IdRef {
     IdRef::new(NSString::alloc(nil).init_str(s))
 }
 
-/// Returns the number of characters in a string.
-/// (A single character may consist of multiple UTF-32 code units.
-/// This is possible when long sequences of composing characters are present)
-///
-/// Unsafe because assumes that the `string` is an `NSString` object
-pub unsafe fn ns_string_char_count(string: id) -> usize {
-    let length: NSUInteger = msg_send![string, length];
-    let range = NSRange {
-        location: 0,
-        length,
-    };
-    let char_count = Rc::new(Cell::new(0));
-    let block = {
-        let char_count = char_count.clone();
-        ConcreteBlock::new(move || char_count.set(char_count.get() + 1)).copy()
-    };
-    let block = &*block;
-    let () = msg_send![string,
-        enumerateSubstringsInRange:range
-        options:NSStringEnumerationByComposedCharacterSequences
-        usingBlock:block
-    ];
-    char_count.get()
-}
-
 #[allow(dead_code)] // In case we want to use this function in the future
 pub unsafe fn app_name() -> Option<id> {
     let bundle: id = msg_send![class!(NSBundle), mainBundle];

src/platform_impl/macos/view.rs

@@ -29,7 +29,7 @@ use crate::{
             scancode_to_keycode, EventWrapper,
         },
         ffi::*,
-        util::{self, ns_string_char_count, IdRef},
+        util::{self, IdRef},
         window::get_window_id,
         DEVICE_ID,
     },
@@ -57,8 +57,6 @@ pub(super) struct ViewState {
     raw_characters: Option<String>,
     pub(super) modifiers: ModifiersState,
     tracking_rect: Option<NSInteger>,
-    is_ime_activated: bool,
-    marked_text: id,
 }
 
 impl ViewState {
@@ -70,8 +68,6 @@ impl ViewState {
 pub fn new_view(ns_window: id) -> (IdRef, Weak<Mutex<CursorState>>) {
     let cursor_state = Default::default();
     let cursor_access = Arc::downgrade(&cursor_state);
-    let marked_text =
-        unsafe { <id as NSMutableAttributedString>::init(NSMutableAttributedString::alloc(nil)) };
     let state = ViewState {
         ns_window,
         cursor_state,
@@ -79,8 +75,6 @@ pub fn new_view(ns_window: id) -> (IdRef, Weak<Mutex<CursorState>>) {
         raw_characters: None,
         modifiers: Default::default(),
         tracking_rect: None,
-        is_ime_activated: false,
-        marked_text,
     };
     unsafe {
         // This is free'd in `dealloc`
@@ -153,10 +147,7 @@ lazy_static! {
             sel!(setMarkedText:selectedRange:replacementRange:),
             set_marked_text as extern "C" fn(&mut Object, Sel, id, NSRange, NSRange),
         );
-        decl.add_method(
-            sel!(unmarkText),
-            unmark_text as extern "C" fn(&mut Object, Sel),
-        );
+        decl.add_method(sel!(unmarkText), unmark_text as extern "C" fn(&Object, Sel));
         decl.add_method(
             sel!(validAttributesForMarkedText),
             valid_attributes_for_marked_text as extern "C" fn(&Object, Sel) -> id,
@@ -168,7 +159,7 @@ lazy_static! {
         );
         decl.add_method(
             sel!(insertText:replacementRange:),
-            insert_text as extern "C" fn(&mut Object, Sel, id, NSRange),
+            insert_text as extern "C" fn(&Object, Sel, id, NSRange),
         );
         decl.add_method(
             sel!(characterIndexForPoint:),
@@ -267,6 +258,7 @@ lazy_static! {
             accepts_first_mouse as extern "C" fn(&Object, Sel, id) -> BOOL,
         );
         decl.add_ivar::<*mut c_void>("winitState");
+        decl.add_ivar::<id>("markedText");
         let protocol = Protocol::get("NSTextInputClient").unwrap();
         decl.add_protocol(&protocol);
         ViewClass(decl.register())
@@ -276,9 +268,9 @@ lazy_static! {
 extern "C" fn dealloc(this: &Object, _sel: Sel) {
     unsafe {
         let state: *mut c_void = *this.get_ivar("winitState");
-        let state = state as *mut ViewState;
-        let _: () = msg_send![(*state).marked_text, release];
-        Box::from_raw(state);
+        let marked_text: id = *this.get_ivar("markedText");
+        let _: () = msg_send![marked_text, release];
+        Box::from_raw(state as *mut ViewState);
     }
 }
 
@@ -287,6 +279,9 @@ extern "C" fn init_with_winit(this: &Object, _sel: Sel, state: *mut c_void) -> i
         let this: id = msg_send![this, init];
         if this != nil {
             (*this).set_ivar("winitState", state);
+            let marked_text =
+                <id as NSMutableAttributedString>::init(NSMutableAttributedString::alloc(nil));
+            (*this).set_ivar("markedText", marked_text);
             let _: () = msg_send![this, setPostsFrameChangedNotifications: YES];
 
             let notification_center: &Object =
@@ -393,20 +388,17 @@ extern "C" fn reset_cursor_rects(this: &Object, _sel: Sel) {
 extern "C" fn has_marked_text(this: &Object, _sel: Sel) -> BOOL {
     unsafe {
         trace!("Triggered `hasMarkedText`");
-        let state_ptr: *mut c_void = *this.get_ivar("winitState");
-        let state = &mut *(state_ptr as *mut ViewState);
-        let retval = (state.marked_text.length() > 0) as BOOL;
+        let marked_text: id = *this.get_ivar("markedText");
         trace!("Completed `hasMarkedText`");
-        retval
+        (marked_text.length() > 0) as BOOL
     }
 }
 
 extern "C" fn marked_range(this: &Object, _sel: Sel) -> NSRange {
     unsafe {
         trace!("Triggered `markedRange`");
-        let state_ptr: *mut c_void = *this.get_ivar("winitState");
-        let state = &mut *(state_ptr as *mut ViewState);
-        let length = state.marked_text.length();
+        let marked_text: id = *this.get_ivar("markedText");
+        let length = marked_text.length();
         trace!("Completed `markedRange`");
         if length > 0 {
             NSRange::new(0, length - 1)
@@ -431,60 +423,30 @@ extern "C" fn set_marked_text(
 ) {
     trace!("Triggered `setMarkedText`");
     unsafe {
-        let state_ptr: *mut c_void = *this.get_ivar("winitState");
-        let state = &mut *(state_ptr as *mut ViewState);
-
-        // Delete previous marked text
-        let char_count = ns_string_char_count(state.marked_text.string());
-        delete_marked_text(state, char_count);
-
-        state.is_ime_activated = true;
-
-        let _: () = msg_send![state.marked_text, release];
-        state.marked_text = NSMutableAttributedString::alloc(nil);
+        let marked_text_ref: &mut id = this.get_mut_ivar("markedText");
+        let _: () = msg_send![(*marked_text_ref), release];
+        let marked_text = NSMutableAttributedString::alloc(nil);
         let has_attr = msg_send![string, isKindOfClass: class!(NSAttributedString)];
         if has_attr {
-            state.marked_text.initWithAttributedString(string);
+            marked_text.initWithAttributedString(string);
         } else {
-            state.marked_text.initWithString(string);
+            marked_text.initWithString(string);
         };
-
-        let text_ns_str = state.marked_text.string();
-        let slice = slice::from_raw_parts(
-            text_ns_str.UTF8String() as *const c_uchar,
-            text_ns_str.len(),
-        );
-        let text_str = str::from_utf8_unchecked(slice);
-
-        for character in text_str.chars() {
-            AppState::queue_event(EventWrapper::StaticEvent(Event::WindowEvent {
-                window_id: WindowId(get_window_id(state.ns_window)),
-                event: WindowEvent::ReceivedCharacter(character),
-            }));
-        }
+        *marked_text_ref = marked_text;
     }
     trace!("Completed `setMarkedText`");
 }
 
-extern "C" fn unmark_text(this: &mut Object, _sel: Sel) {
+extern "C" fn unmark_text(this: &Object, _sel: Sel) {
     trace!("Triggered `unmarkText`");
     unsafe {
-        clear_marked_text(this);
-    }
-    trace!("Completed `unmarkText`");
-}
-
-/// Unsafe because assumes that `this` is an instance of the `WinitView` class that we declare
-/// programmatically
-unsafe fn clear_marked_text(this: &mut Object) {
-    let state_ptr: *mut c_void = *this.get_ivar("winitState");
-    let state = &mut *(state_ptr as *mut ViewState);
-
-    let _: () = msg_send![state.marked_text, release];
-    state.marked_text = NSMutableAttributedString::alloc(nil);
-
+        let marked_text: id = *this.get_ivar("markedText");
+        let mutable_string = marked_text.mutableString();
+        let _: () = msg_send![mutable_string, setString:""];
         let input_context: id = msg_send![this, inputContext];
         let _: () = msg_send![input_context, discardMarkedText];
+    }
+    trace!("Completed `unmarkText`");
 }
 
 extern "C" fn valid_attributes_for_marked_text(_this: &Object, _sel: Sel) -> id {
@@ -534,19 +496,12 @@ extern "C" fn first_rect_for_character_range(
     }
 }
 
-extern "C" fn insert_text(this: &mut Object, _sel: Sel, string: id, _replacement_range: NSRange) {
+extern "C" fn insert_text(this: &Object, _sel: Sel, string: id, _replacement_range: NSRange) {
     trace!("Triggered `insertText`");
     unsafe {
         let state_ptr: *mut c_void = *this.get_ivar("winitState");
         let state = &mut *(state_ptr as *mut ViewState);
 
-        let is_ime_activated: bool = state.is_ime_activated;
-        if is_ime_activated {
-            clear_marked_text(this);
-            state.is_ime_activated = false;
-            return;
-        }
-
         let has_attr = msg_send![string, isKindOfClass: class!(NSAttributedString)];
         let characters = if has_attr {
             // This is a *mut NSAttributedString
@@ -613,15 +568,6 @@ extern "C" fn do_command_by_selector(this: &Object, _sel: Sel, command: Sel) {
     trace!("Completed `doCommandBySelector`");
 }
 
-fn delete_marked_text(state: &mut ViewState, count: usize) {
-    for _ in 0..count {
-        AppState::queue_event(EventWrapper::StaticEvent(Event::WindowEvent {
-            window_id: WindowId(get_window_id(state.ns_window)),
-            event: WindowEvent::ReceivedCharacter('\u{7f}'), // fire DELETE
-        }));
-    }
-}
-
 fn get_characters(event: id, ignore_modifiers: bool) -> String {
     unsafe {
         let characters: id = if ignore_modifiers {