Skip to content

Commit

Permalink
On X11 query for XIM styles before creating IME
Browse files Browse the repository at this point in the history
Fixes #2448.
  • Loading branch information
kchibisov committed Sep 11, 2022
1 parent f1470d1 commit 66319c5
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 79 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,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)

Expand Down
8 changes: 4 additions & 4 deletions src/platform_impl/linux/x11/ime/callbacks.rs
Expand Up @@ -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() {
Expand All @@ -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;
Expand Down
93 changes: 54 additions & 39 deletions src/platform_impl/linux/x11/ime/context.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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<ImeContextClientData>,
Expand All @@ -203,9 +204,9 @@ impl ImeContext {
pub unsafe fn new(
xconn: &Arc<XConnection>,
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 {
Expand All @@ -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()
Expand All @@ -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),
};

Expand All @@ -244,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::<()>(),
Expand All @@ -258,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> {
Expand All @@ -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<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 _,
style,
ffi::XNClientWindow_0.as_ptr() as *const _,
window,
ptr::null_mut::<()>(),
);

(!ic.is_null()).then(|| ic)
}
Expand All @@ -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<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;
}

Expand Down
16 changes: 9 additions & 7 deletions 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<XConnection>, im: ffi::XIM) -> Result<(), XError> {
Expand All @@ -17,8 +20,7 @@ pub unsafe fn destroy_ic(xconn: &Arc<XConnection>, ic: ffi::XIC) -> Result<(), X

pub struct ImeInner {
pub xconn: Arc<XConnection>,
// WARNING: this is initially null!
pub im: ffi::XIM,
pub im: Option<InputMethod>,
pub potential_input_methods: PotentialInputMethods,
pub contexts: HashMap<ffi::Window, Option<ImeContext>>,
// WARNING: this is initially zeroed!
Expand All @@ -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() },
Expand All @@ -49,8 +51,8 @@ impl ImeInner {
}

pub unsafe fn close_im_if_necessary(&self) -> Result<bool, XError> {
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)
}
Expand Down
90 changes: 86 additions & 4 deletions src/platform_impl/linux/x11/ime/input_method.rs
Expand Up @@ -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,
};
Expand Down Expand Up @@ -41,15 +41,97 @@ unsafe fn open_im(xconn: &Arc<XConnection>, locale_modifiers: &CStr) -> Option<f
#[derive(Debug)]
pub struct InputMethod {
pub im: ffi::XIM,
pub preedit_style: Style,
pub none_style: Style,
_name: String,
}

impl InputMethod {
fn new(im: ffi::XIM, name: String) -> Self {
InputMethod { im, _name: name }
fn new(xconn: &Arc<XConnection>, im: ffi::XIM, name: String) -> Option<Self> {
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.
Expand Down Expand Up @@ -175,7 +257,7 @@ impl PotentialInputMethod {
pub fn open_im(&mut self, xconn: &Arc<XConnection>) -> Option<InputMethod> {
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()))
}
}

Expand Down

0 comments on commit 66319c5

Please sign in to comment.