On X11 query for XIM styles before creating IME

Fixes rust-windowing#2448.

CHANGELOG.md

@@ -26,6 +26,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 - On Wayland, fixed `Ime::Preedit` not being sent on IME reset.
 - Fixed unbound version specified for `raw-window-handle` leading to compilation failures.
 - Empty `Ime::Preedit` event will be sent before `Ime::Commit` to help clearing preedit.
+- On X11, fixed IME context picking by querying for supported styles beforehand.
 
 # 0.27.2 (2022-8-12)
 

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

@@ -108,17 +108,17 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> {
     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 is_allowed = old_context
+        let style = old_context
             .as_ref()
-            .map(|old_context| old_context.is_allowed)
+            .map(|old_context| old_context.style)
             .unwrap_or_default();
         let new_context = {
             let result = ImeContext::new(
                 xconn,
-                &new_im,
+                new_im.im,
+                style,
                 *window,
                 spot,
-                is_allowed,
                 (*inner).event_sender.clone(),
             );
             if result.is_err() {

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

@@ -5,8 +5,8 @@ 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 crate::platform_impl::platform::x11::ime::input_method::InputMethod;
 
 use super::{ffi, util, XConnection, XError};
 
@@ -192,9 +192,9 @@ struct ImeContextClientData {
 // still exists on the server. Since `ImeInner` has that awareness, destruction must be handled
 // through `ImeInner`.
 pub struct ImeContext {
-    pub(super) ic: ffi::XIC,
-    pub(super) ic_spot: ffi::XPoint,
-    pub(super) is_allowed: bool,
+    pub(crate) ic: ffi::XIC,
+    pub(crate) ic_spot: ffi::XPoint,
+    pub(crate) style: Style,
     // Since the data is passed shared between X11 XIM callbacks, but couldn't be direclty free from
     // there we keep the pointer to automatically deallocate it.
     _client_data: Box<ImeContextClientData>,
@@ -203,10 +203,10 @@ pub struct ImeContext {
 impl ImeContext {
     pub unsafe fn new(
         xconn: &Arc<XConnection>,
-        im: &InputMethod,
+        im: ffi::XIM,
+        style: Style,
         window: ffi::Window,
         ic_spot: Option<ffi::XPoint>,
-        is_allowed: bool,
         event_sender: ImeEventSender,
     ) -> Result<Self, ImeContextCreationError> {
         let client_data = Box::into_raw(Box::new(ImeContextClientData {
@@ -216,12 +216,18 @@ impl ImeContext {
             cursor_pos: 0,
         }));
 
-        let ic = if is_allowed {
-            ImeContext::create_ic(xconn, im.im, window, client_data as ffi::XPointer)
-                .ok_or(ImeContextCreationError::Null)?
-        } else {
-            ImeContext::create_none_ic(xconn, im.im, window).ok_or(ImeContextCreationError::Null)?
-        };
+        let ic = match style as _ {
+            Style::Preedit(style) => ImeContext::create_preedit_ic(
+                xconn,
+                im,
+                style,
+                window,
+                client_data as ffi::XPointer,
+            ),
+            Style::Nothing(style) => ImeContext::create_nothing_ic(xconn, im, style, window),
+            Style::None(style) => ImeContext::create_none_ic(xconn, im, style, window),
+        }
+        .ok_or(ImeContextCreationError::Null)?;
 
         xconn
             .check_errors()
@@ -230,7 +236,7 @@ impl ImeContext {
         let mut context = ImeContext {
             ic,
             ic_spot: ffi::XPoint { x: 0, y: 0 },
-            is_allowed,
+            style,
             _client_data: Box::from_raw(client_data),
         };
 
@@ -245,12 +251,13 @@ impl ImeContext {
     unsafe fn create_none_ic(
         xconn: &Arc<XConnection>,
         im: ffi::XIM,
+        style: XIMStyle,
         window: ffi::Window,
     ) -> Option<ffi::XIC> {
         let ic = (xconn.xlib.XCreateIC)(
             im,
             ffi::XNInputStyle_0.as_ptr() as *const _,
-            ffi::XIMPreeditNone | ffi::XIMStatusNone,
+            style,
             ffi::XNClientWindow_0.as_ptr() as *const _,
             window,
             ptr::null_mut::<()>(),
@@ -259,9 +266,10 @@ impl ImeContext {
         (!ic.is_null()).then(|| ic)
     }
 
-    unsafe fn create_ic(
+    unsafe fn create_preedit_ic(
         xconn: &Arc<XConnection>,
         im: ffi::XIM,
+        style: XIMStyle,
         window: ffi::Window,
         client_data: ffi::XPointer,
     ) -> Option<ffi::XIC> {
@@ -283,32 +291,35 @@ impl ImeContext {
         )
         .expect("XVaCreateNestedList returned NULL");
 
-        let ic = {
-            let ic = (xconn.xlib.XCreateIC)(
-                im,
-                ffi::XNInputStyle_0.as_ptr() as *const _,
-                ffi::XIMPreeditCallbacks | ffi::XIMStatusNothing,
-                ffi::XNClientWindow_0.as_ptr() as *const _,
-                window,
-                ffi::XNPreeditAttributes_0.as_ptr() as *const _,
-                preedit_attr.ptr,
-                ptr::null_mut::<()>(),
-            );
+        let ic = (xconn.xlib.XCreateIC)(
+            im,
+            ffi::XNInputStyle_0.as_ptr() as *const _,
+            style,
+            ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
+            ffi::XNPreeditAttributes_0.as_ptr() as *const _,
+            preedit_attr.ptr,
+            ptr::null_mut::<()>(),
+        );
 
-            // If we've failed to create IC with preedit callbacks fallback to normal one.
-            if ic.is_null() {
-                (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::<()>(),
-                )
-            } else {
-                ic
-            }
-        };
+        (!ic.is_null()).then(|| ic)
+    }
+
+    unsafe fn create_nothing_ic(
+        xconn: &Arc<XConnection>,
+        im: ffi::XIM,
+        style: XIMStyle,
+        window: ffi::Window,
+    ) -> Option<ffi::XIC> {
+        // If we've failed to create IC with preedit callbacks fallback to normal one.
+        let ic = (xconn.xlib.XCreateIC)(
+            im,
+            ffi::XNInputStyle_0.as_ptr() as *const _,
+            style,
+            ffi::XNClientWindow_0.as_ptr() as *const _,
+            window,
+            ptr::null_mut::<()>(),
+        );
 
         (!ic.is_null()).then(|| ic)
     }
@@ -327,13 +338,17 @@ impl ImeContext {
         xconn.check_errors()
     }
 
+    pub fn is_allowed(&self) -> bool {
+        !matches!(self.style, Style::None(_))
+    }
+
     // Set the spot for preedit text. Setting spot isn't working with libX11 when preedit callbacks
     // are being used. Certain IMEs do show selection window, but it's placed in bottom left of the
     // window and couldn't be changed.
     //
     // For me see: https://bugs.freedesktop.org/show_bug.cgi?id=1580.
     pub fn set_spot(&mut self, xconn: &Arc<XConnection>, x: c_short, y: c_short) {
-        if !self.is_allowed || self.ic_spot.x == x && self.ic_spot.y == y {
+        if !self.is_allowed() || self.ic_spot.x == x && self.ic_spot.y == y {
             return;
         }
 

src/platform_impl/linux/x11/ime/input_method.rs

@@ -40,7 +40,8 @@ unsafe fn open_im(xconn: &Arc<XConnection>, locale_modifiers: &CStr) -> Option<f
 #[derive(Debug)]
 pub struct InputMethod {
     pub im: ffi::XIM,
-    pub xim_styles: Vec<XIMStyle>,
+    pub preedit_style: Option<Style>,
+    pub none_style: Option<Style>,
     _name: String,
 }
 
@@ -62,31 +63,70 @@ impl InputMethod {
             }
         }
 
-        let xim_styles = unsafe {
-            std::slice::from_raw_parts((*styles).supported_styles, (*styles).count_styles as _)
-                .to_vec()
-        };
+        let mut preedit_style = None;
+        let mut none_style = None;
 
         unsafe {
+            std::slice::from_raw_parts((*styles).supported_styles, (*styles).count_styles as _)
+                .iter()
+                .for_each(|style| match *style {
+                    XIM_PREEDIT_STYLE => {
+                        preedit_style = Some(Style::Preedit(*style));
+                    }
+                    XIM_NOTHING_STYLE if preedit_style.is_none() => {
+                        preedit_style = Some(Style::Nothing(*style))
+                    }
+                    XIM_NONE_STYLE => none_style = Some(Style::None(*style)),
+                    _ => (),
+                });
+
             (xconn.xlib.XFree)(styles.cast());
+        };
+
+        if preedit_style.is_none() && none_style.is_none() {
+            return None;
         }
 
         Some(InputMethod {
             im,
             _name: name,
-            xim_styles,
+            preedit_style,
+            none_style,
         })
     }
 }
 
+const XIM_PREEDIT_STYLE: XIMStyle = (ffi::XIMPreeditCallbacks | ffi::XIMStatusNothing) as XIMStyle;
+const XIM_NOTHING_STYLE: XIMStyle = (ffi::XIMPreeditNothing | ffi::XIMStatusNothing) as XIMStyle;
+const XIM_NONE_STYLE: XIMStyle = (ffi::XIMPreeditNone | ffi::XIMStatusNone) as XIMStyle;
+
+/// Style of the IME context.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Style {
+    /// Preedit callbacks.
+    Preedit(XIMStyle),
+
+    /// Nothing.
+    Nothing(XIMStyle),
+
+    /// No IME.
+    None(XIMStyle),
+}
+
+impl Default for Style {
+    fn default() -> Self {
+        Style::None(XIM_NONE_STYLE)
+    }
+}
+
 #[repr(C)]
 #[derive(Debug)]
-pub struct XIMStyles {
+struct XIMStyles {
     count_styles: c_ushort,
     supported_styles: *const XIMStyle,
 }
 
-pub type XIMStyle = c_ulong;
+pub(crate) type XIMStyle = c_ulong;
 
 #[derive(Debug)]
 pub enum InputMethodResult {

src/platform_impl/linux/x11/ime/mod.rs

@@ -17,8 +17,9 @@ use self::{
     callbacks::*,
     context::ImeContext,
     inner::{close_im, ImeInner},
-    input_method::PotentialInputMethods,
+    input_method::{PotentialInputMethods, Style},
 };
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 pub enum ImeEvent {
@@ -120,37 +121,35 @@ impl Ime {
             // 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.unwrap_or_else(|| im.none_style.unwrap())
+            } else {
+                im.none_style.unwrap_or_else(|| im.preedit_style.unwrap())
+            };
+
             let context = unsafe {
                 ImeContext::new(
                     &self.inner.xconn,
-                    self.inner.im.as_ref().unwrap(),
+                    im.im,
+                    style,
                     window,
                     None,
-                    with_preedit,
                     self.inner.event_sender.clone(),
-                )
-                .or_else(|_| {
-                    debug!(
-                        "failed to create an IME context {} preedit support",
-                        if with_preedit { "with" } else { "without" }
-                    );
-                    ImeContext::new(
-                        &self.inner.xconn,
-                        self.inner.im.as_ref().unwrap(),
-                        window,
-                        None,
-                        !with_preedit,
-                        self.inner.event_sender.clone(),
-                    )
-                })
-            }?;
+                )?
+            };
 
             // Check the state on the context, since it could fail to enable or disable preedit.
-            let event = if context.is_allowed {
-                ImeEvent::Enabled
-            } else {
-                // There's no IME without 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
             };
 
             self.inner
@@ -160,6 +159,7 @@ impl Ime {
 
             Some(context)
         };
+
         self.inner.contexts.insert(window, context);
         Ok(!self.is_destroyed())
     }
@@ -223,7 +223,7 @@ impl Ime {
         }
 
         if let Some(&mut Some(ref mut context)) = self.inner.contexts.get_mut(&window) {
-            if allowed == context.is_allowed {
+            if allowed == context.is_allowed() {
                 return;
             }
         }