x11: Overhaul XIM code (#451)

Fixes #195
Fixes #277
Fixes #455

* Read `XMODIFIERS` explicitly/directly instead of calling `XSetLocaleModifiers` with an
empty string. This is useful for debugging purposes, and more clear to read and handle.
* Fallback to local input method if the one specified in `XMODIFIERS` is later closed on the
server end (i.e. if ibus/fcitx is terminated). Previously, that would cause the event loop
to freeze and usually also segfault.
* If using the fallback input method, respond to the `XMODIFIERS` input method later
becoming available. This means that the input method restarting is handled, and that even if
the program was started while ibus/fcitx/etc. was unavailable, it will start using it as
soon as it becomes available.
* Only one input method is opened for the whole event loop, with each window having its own
input context.
* IME works completely out of the box now, no longer requiring application developers to
call `setlocale` or `XSetLocaleModifiers`.
* Detailed error messages are provided if no input method could be opened. However, no
information is provided to the user if their intended `XMODIFIERS` input method failed to
open but the fallbacks (which will ostensibly always succeed) succeeded; in my opinion, this
is something that is best filled by adding a logging feature to winit.

CHANGELOG.md

@@ -3,6 +3,7 @@
 - Overhauled X11 window geometry calculations. `get_position` and `set_position` are more universally accurate across different window managers, and `get_outer_size` actually works now.
 - Fixed SIGSEGV/SIGILL crashes on macOS caused by stabilization of the `!` (never) type.
 - Implement `WindowEvent::HiDPIFactorChanged` for macOS
+- On X11, input methods now work completely out of the box, no longer requiring application developers to manually call `setlocale`. Additionally, when input methods are started, stopped, or restarted on the server end, it's correctly handled.
 
 # Version 0.12.0 (2018-04-06)
 

src/platform/linux/x11/ime/callbacks.rs

@@ -0,0 +1,185 @@
+use std::ptr;
+use std::sync::Arc;
+use std::collections::HashMap;
+use std::os::raw::c_char;
+
+use super::{ffi, XConnection, XError};
+
+use super::inner::{close_im, ImeInner};
+use super::input_method::PotentialInputMethods;
+use super::context::{ImeContextCreationError, ImeContext};
+
+pub unsafe fn xim_set_callback(
+    xconn: &Arc<XConnection>,
+    xim: ffi::XIM,
+    field: *const c_char,
+    callback: *mut ffi::XIMCallback,
+) -> Result<(), XError> {
+    // It's advisable to wrap variadic FFI functions in our own functions, as we want to minimize
+    // access that isn't type-checked.
+    (xconn.xlib.XSetIMValues)(
+        xim,
+        field,
+        callback,
+        ptr::null_mut::<()>(),
+    );
+    xconn.check_errors()
+}
+
+// Set a callback for when an input method matching the current locale modifiers becomes
+// available. Note that this has nothing to do with what input methods are open or able to be
+// opened, and simply uses the modifiers that are set when the callback is set.
+// * This is called per locale modifier, not per input method opened with that locale modifier.
+// * Trying to set this for multiple locale modifiers causes problems, i.e. one of the rebuilt
+//   input contexts would always silently fail to use the input method.
+pub unsafe fn set_instantiate_callback(
+    xconn: &Arc<XConnection>,
+    client_data: ffi::XPointer,
+) -> Result<(), XError> {
+    (xconn.xlib.XRegisterIMInstantiateCallback)(
+        xconn.display,
+        ptr::null_mut(),
+        ptr::null_mut(),
+        ptr::null_mut(),
+        Some(xim_instantiate_callback),
+        client_data,
+    );
+    xconn.check_errors()
+}
+
+pub unsafe fn unset_instantiate_callback(
+    xconn: &Arc<XConnection>,
+    client_data: ffi::XPointer,
+) -> Result<(), XError> {
+    (xconn.xlib.XUnregisterIMInstantiateCallback)(
+        xconn.display,
+        ptr::null_mut(),
+        ptr::null_mut(),
+        ptr::null_mut(),
+        Some(xim_instantiate_callback),
+        client_data,
+    );
+    xconn.check_errors()
+}
+
+pub unsafe fn set_destroy_callback(
+    xconn: &Arc<XConnection>,
+    im: ffi::XIM,
+    inner: &ImeInner,
+) -> Result<(), XError> {
+    xim_set_callback(
+        &xconn,
+        im,
+        ffi::XNDestroyCallback_0.as_ptr() as *const _,
+        &inner.destroy_callback as *const _ as *mut _,
+    )
+}
+
+#[derive(Debug)]
+enum ReplaceImError {
+    MethodOpenFailed(PotentialInputMethods),
+    ContextCreationFailed(ImeContextCreationError),
+    SetDestroyCallbackFailed(XError),
+}
+
+// Attempt to replace current IM (which may or may not be presently valid) with a new one. This
+// includes replacing all existing input contexts and free'ing resources as necessary. This only
+// modifies existing state if all operations succeed.
+unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> {
+    let xconn = &(*inner).xconn;
+
+    let (new_im, is_fallback) = {
+        let new_im = (*inner).potential_input_methods.open_im(xconn, None);
+        let is_fallback = new_im.is_fallback();
+        (
+            new_im.ok().ok_or_else(|| {
+                ReplaceImError::MethodOpenFailed((*inner).potential_input_methods.clone())
+            })?,
+            is_fallback,
+        )
+    };
+
+    // It's important to always set a destroy callback, since there's otherwise potential for us
+    // to try to use or free a resource that's already been destroyed on the server.
+    {
+        let result = set_destroy_callback(xconn, new_im.im, &*inner);
+        if result.is_err() {
+            let _ = close_im(xconn, new_im.im);
+        }
+        result
+    }.map_err(ReplaceImError::SetDestroyCallbackFailed)?;
+
+    let mut new_contexts = HashMap::new();
+    for (window, old_context) in (*inner).contexts.iter() {
+        let spot = old_context.as_ref().map(|old_context| old_context.ic_spot);
+        let new_context = {
+            let result = ImeContext::new(
+                xconn,
+                new_im.im,
+                *window,
+                spot,
+            );
+            if result.is_err() {
+                let _ = close_im(xconn, new_im.im);
+            }
+            result.map_err(ReplaceImError::ContextCreationFailed)?
+        };
+        new_contexts.insert(*window, Some(new_context));
+    }
+
+    // If we've made it this far, everything succeeded.
+    let _ = (*inner).destroy_all_contexts_if_necessary();
+    let _ = (*inner).close_im_if_necessary();
+    (*inner).im = new_im.im;
+    (*inner).contexts = new_contexts;
+    (*inner).is_destroyed = false;
+    (*inner).is_fallback = is_fallback;
+    Ok(())
+}
+
+pub unsafe extern fn xim_instantiate_callback(
+    _display: *mut ffi::Display,
+    client_data: ffi::XPointer,
+    // This field is unsupplied.
+    _call_data: ffi::XPointer,
+) {
+    let inner: *mut ImeInner = client_data as _;
+    if !inner.is_null() {
+        let xconn = &(*inner).xconn;
+        let result = replace_im(inner);
+        if result.is_ok() {
+            let _ = unset_instantiate_callback(xconn, client_data);
+            (*inner).is_fallback = false;
+        } else if result.is_err() && (*inner).is_destroyed {
+            // We have no usable input methods!
+            result.expect("Failed to reopen input method");
+        }
+    }
+}
+
+// This callback is triggered when the input method is closed on the server end. When this
+// happens, XCloseIM/XDestroyIC doesn't need to be called, as the resources have already been
+// free'd (attempting to do so causes our connection to freeze).
+pub unsafe extern fn xim_destroy_callback(
+    _xim: ffi::XIM,
+    client_data: ffi::XPointer,
+    // This field is unsupplied.
+    _call_data: ffi::XPointer,
+) {
+    let inner: *mut ImeInner = client_data as _;
+    if !inner.is_null() {
+        (*inner).is_destroyed = true;
+        let xconn = &(*inner).xconn;
+        if !(*inner).is_fallback {
+            let _ = set_instantiate_callback(xconn, client_data);
+            // Attempt to open fallback input method.
+            let result = replace_im(inner);
+            if result.is_ok() {
+                (*inner).is_fallback = true;
+            } else {
+                // We have no usable input methods!
+                result.expect("Failed to open fallback input method");
+            }
+        }
+    }
+}

src/platform/linux/x11/ime/context.rs

@@ -0,0 +1,134 @@
+use std::ptr;
+use std::sync::Arc;
+use std::os::raw::{c_short, c_void};
+
+use super::{ffi, util, XConnection, XError};
+
+#[derive(Debug)]
+pub enum ImeContextCreationError {
+    XError(XError),
+    Null,
+}
+
+unsafe fn create_pre_edit_attr<'a>(
+    xconn: &'a Arc<XConnection>,
+    ic_spot: &'a ffi::XPoint,
+) -> util::XSmartPointer<'a, c_void> {
+    util::XSmartPointer::new(
+        xconn,
+        (xconn.xlib.XVaCreateNestedList)(
+            0,
+            ffi::XNSpotLocation_0.as_ptr() as *const _,
+            ic_spot,
+            ptr::null_mut::<()>(),
+        ),
+    ).expect("XVaCreateNestedList returned NULL")
+}
+
+// WARNING: this struct doesn't destroy its XIC resource when dropped.
+// This is intentional, as it doesn't have enough information to know whether or not the context
+// still exists on the server. Since `ImeInner` has that awareness, destruction must be handled
+// through `ImeInner`.
+#[derive(Debug)]
+pub struct ImeContext {
+    pub ic: ffi::XIC,
+    pub ic_spot: ffi::XPoint,
+}
+
+impl ImeContext {
+    pub unsafe fn new(
+        xconn: &Arc<XConnection>,
+        im: ffi::XIM,
+        window: ffi::Window,
+        ic_spot: Option<ffi::XPoint>,
+    ) -> Result<Self, ImeContextCreationError> {
+        let ic = if let Some(ic_spot) = ic_spot {
+            ImeContext::create_ic_with_spot(xconn, im, window, ic_spot)
+        } else {
+            ImeContext::create_ic(xconn, im, window)
+        };
+
+        let ic = ic.ok_or(ImeContextCreationError::Null)?;
+        xconn.check_errors().map_err(ImeContextCreationError::XError)?;
+
+        Ok(ImeContext {
+            ic,
+            ic_spot: ic_spot.unwrap_or_else(|| ffi::XPoint { x: 0, y: 0 }),
+        })
+    }
+
+    unsafe fn create_ic(
+        xconn: &Arc<XConnection>,
+        im: ffi::XIM,
+        window: ffi::Window,
+    ) -> Option<ffi::XIC> {
+        let ic = (xconn.xlib.XCreateIC)(
+            im,
+            ffi::XNInputStyle_0.as_ptr() as *const _,
+            ffi::XIMPreeditNothing | ffi::XIMStatusNothing,
+            ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
+            ptr::null_mut::<()>(),
+        );
+        if ic.is_null() {
+            None
+        } else {
+            Some(ic)
+        }
+    }
+
+    unsafe fn create_ic_with_spot(
+        xconn: &Arc<XConnection>,
+        im: ffi::XIM,
+        window: ffi::Window,
+        ic_spot: ffi::XPoint,
+    ) -> Option<ffi::XIC> {
+        let pre_edit_attr = create_pre_edit_attr(xconn, &ic_spot);
+        let ic = (xconn.xlib.XCreateIC)(
+            im,
+            ffi::XNInputStyle_0.as_ptr() as *const _,
+            ffi::XIMPreeditNothing | ffi::XIMStatusNothing,
+            ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
+            ffi::XNPreeditAttributes_0.as_ptr() as *const _,
+            pre_edit_attr.ptr,
+            ptr::null_mut::<()>(),
+        );
+        if ic.is_null() {
+            None
+        } else {
+            Some(ic)
+        }
+    }
+
+    pub fn focus(&self, xconn: &Arc<XConnection>) -> Result<(), XError> {
+        unsafe {
+            (xconn.xlib.XSetICFocus)(self.ic);
+        }
+        xconn.check_errors()
+    }
+
+    pub fn unfocus(&self, xconn: &Arc<XConnection>) -> Result<(), XError> {
+        unsafe {
+            (xconn.xlib.XUnsetICFocus)(self.ic);
+        }
+        xconn.check_errors()
+    }
+
+    pub fn set_spot(&mut self, xconn: &Arc<XConnection>, x: c_short, y: c_short) {
+        if self.ic_spot.x == x && self.ic_spot.y == y {
+            return;
+        }
+        self.ic_spot = ffi::XPoint { x, y };
+
+        unsafe {
+            let pre_edit_attr = create_pre_edit_attr(xconn, &self.ic_spot);
+            (xconn.xlib.XSetICValues)(
+                self.ic,
+                ffi::XNPreeditAttributes_0.as_ptr() as *const _,
+                pre_edit_attr.ptr,
+                ptr::null_mut::<()>(),
+            );
+        }
+    }
+}