diff --git a/CHANGELOG.md b/CHANGELOG.md index 6679fdf8..d53278c7 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/platform_impl/linux/x11/ime/callbacks.rs b/src/platform_impl/linux/x11/ime/callbacks.rs index a957c23a..1f6bc23b 100644 --- a/src/platform_impl/linux/x11/ime/callbacks.rs +++ b/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.im, + style, *window, spot, - is_allowed, (*inner).event_sender.clone(), ); if result.is_err() { @@ -132,7 +132,7 @@ unsafe fn replace_im(inner: *mut ImeInner) -> Result<(), ReplaceImError> { // 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).im = Some(new_im); (*inner).contexts = new_contexts; (*inner).is_destroyed = false; (*inner).is_fallback = is_fallback; diff --git a/src/platform_impl/linux/x11/ime/context.rs b/src/platform_impl/linux/x11/ime/context.rs index a3109d57..a25c9950 100644 --- a/src/platform_impl/linux/x11/ime/context.rs +++ b/src/platform_impl/linux/x11/ime/context.rs @@ -5,6 +5,7 @@ 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}; @@ -191,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, @@ -203,9 +204,9 @@ impl ImeContext { pub unsafe fn new( xconn: &Arc, im: ffi::XIM, + style: Style, window: ffi::Window, ic_spot: Option, - is_allowed: bool, event_sender: ImeEventSender, ) -> Result { let client_data = Box::into_raw(Box::new(ImeContextClientData { @@ -215,12 +216,18 @@ impl ImeContext { cursor_pos: 0, })); - let ic = if is_allowed { - ImeContext::create_ic(xconn, im, window, client_data as ffi::XPointer) - .ok_or(ImeContextCreationError::Null)? - } else { - ImeContext::create_none_ic(xconn, 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() @@ -229,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), }; @@ -244,12 +251,13 @@ impl ImeContext { unsafe fn create_none_ic( xconn: &Arc, im: ffi::XIM, + style: XIMStyle, window: ffi::Window, ) -> Option { 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::<()>(), @@ -258,9 +266,10 @@ impl ImeContext { (!ic.is_null()).then(|| ic) } - unsafe fn create_ic( + unsafe fn create_preedit_ic( xconn: &Arc, im: ffi::XIM, + style: XIMStyle, window: ffi::Window, client_data: ffi::XPointer, ) -> Option { @@ -282,32 +291,34 @@ 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, + im: ffi::XIM, + style: XIMStyle, + window: ffi::Window, + ) -> Option { + 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) } @@ -326,13 +337,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, 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; } diff --git a/src/platform_impl/linux/x11/ime/inner.rs b/src/platform_impl/linux/x11/ime/inner.rs index 58b558bc..93a94cb0 100644 --- a/src/platform_impl/linux/x11/ime/inner.rs +++ b/src/platform_impl/linux/x11/ime/inner.rs @@ -1,8 +1,11 @@ -use std::{collections::HashMap, mem, ptr, sync::Arc}; +use std::{collections::HashMap, mem, sync::Arc}; use super::{ffi, XConnection, XError}; -use super::{context::ImeContext, input_method::PotentialInputMethods}; +use super::{ + context::ImeContext, + input_method::{InputMethod, PotentialInputMethods}, +}; use crate::platform_impl::platform::x11::ime::ImeEventSender; pub unsafe fn close_im(xconn: &Arc, im: ffi::XIM) -> Result<(), XError> { @@ -17,8 +20,7 @@ pub unsafe fn destroy_ic(xconn: &Arc, ic: ffi::XIC) -> Result<(), X pub struct ImeInner { pub xconn: Arc, - // WARNING: this is initially null! - pub im: ffi::XIM, + pub im: Option, pub potential_input_methods: PotentialInputMethods, pub contexts: HashMap>, // WARNING: this is initially zeroed! @@ -38,7 +40,7 @@ impl ImeInner { ) -> Self { ImeInner { xconn, - im: ptr::null_mut(), + im: None, potential_input_methods, contexts: HashMap::new(), destroy_callback: unsafe { mem::zeroed() }, @@ -49,8 +51,8 @@ impl ImeInner { } pub unsafe fn close_im_if_necessary(&self) -> Result { - if !self.is_destroyed { - close_im(&self.xconn, self.im).map(|_| true) + if !self.is_destroyed && self.im.is_some() { + close_im(&self.xconn, self.im.as_ref().unwrap().im).map(|_| true) } else { Ok(false) } diff --git a/src/platform_impl/linux/x11/ime/input_method.rs b/src/platform_impl/linux/x11/ime/input_method.rs index fecea81a..944d63c8 100644 --- a/src/platform_impl/linux/x11/ime/input_method.rs +++ b/src/platform_impl/linux/x11/ime/input_method.rs @@ -2,7 +2,7 @@ use std::{ env, ffi::{CStr, CString, IntoStringError}, fmt, - os::raw::c_char, + os::raw::{c_char, c_ulong, c_ushort}, ptr, sync::{Arc, Mutex}, }; @@ -40,15 +40,97 @@ unsafe fn open_im(xconn: &Arc, locale_modifiers: &CStr) -> Option Self { - InputMethod { im, _name: name } + fn new(xconn: &Arc, im: ffi::XIM, name: String) -> Option { + let mut styles: *mut XIMStyles = std::ptr::null_mut(); + + // Query the styles supported by the XIM. + unsafe { + if !(xconn.xlib.XGetIMValues)( + im, + ffi::XNQueryInputStyle_0.as_ptr() as *const _, + (&mut styles) as *mut _, + std::ptr::null_mut::<()>(), + ) + .is_null() + { + return None; + } + } + + 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; + } + + let preedit_style = preedit_style.unwrap_or_else(|| none_style.unwrap()); + let none_style = none_style.unwrap_or(preedit_style); + + Some(InputMethod { + im, + _name: name, + 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)] +struct XIMStyles { + count_styles: c_ushort, + supported_styles: *const XIMStyle, +} + +pub(crate) type XIMStyle = c_ulong; + #[derive(Debug)] pub enum InputMethodResult { /// Input method used locale modifier from `XMODIFIERS` environment variable. @@ -174,7 +256,7 @@ impl PotentialInputMethod { pub fn open_im(&mut self, xconn: &Arc) -> Option { let im = unsafe { open_im(xconn, &self.name.c_string) }; self.successful = Some(im.is_some()); - im.map(|im| InputMethod::new(im, self.name.string.clone())) + im.and_then(|im| InputMethod::new(xconn, im, self.name.string.clone())) } } diff --git a/src/platform_impl/linux/x11/ime/mod.rs b/src/platform_impl/linux/x11/ime/mod.rs index 746ba9ea..fec2423a 100644 --- a/src/platform_impl/linux/x11/ime/mod.rs +++ b/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 { @@ -87,7 +88,6 @@ impl Ime { let is_fallback = input_method.is_fallback(); if let Some(input_method) = input_method.ok() { - inner.im = input_method.im; inner.is_fallback = is_fallback; unsafe { let result = set_destroy_callback(&xconn, input_method.im, &*inner) @@ -97,6 +97,7 @@ impl Ime { } result?; } + inner.im = Some(input_method); Ok(Ime { xconn, inner }) } else { Err(ImeCreationError::OpenFailure(inner.potential_input_methods)) @@ -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 + } else { + im.none_style + }; + let context = unsafe { ImeContext::new( &self.inner.xconn, - self.inner.im, + 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, - 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; } }