From 75d16ecdebe777b81b31230f8e2d87f509536354 Mon Sep 17 00:00:00 2001 From: Francesca Frangipane Date: Fri, 23 Mar 2018 14:16:22 -0400 Subject: [PATCH] x11: Don't panic when using dead keys --- CHANGELOG.md | 2 + src/platform/linux/x11/mod.rs | 104 ++++++++++++++++++--------------- src/platform/linux/x11/util.rs | 51 ++++++++++++++++ 3 files changed, 110 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaaca49cb30..8c005c2284c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- Dead keys now work properly on X11, no longer resulting in a panic. + # Version 0.11.3 (2018-03-28) - Added `set_min_dimensions` and `set_max_dimensions` methods to `Window`, and implemented on Windows, X11, Wayland, and OSX. diff --git a/src/platform/linux/x11/mod.rs b/src/platform/linux/x11/mod.rs index d49b795a5f8..bc1198a196c 100644 --- a/src/platform/linux/x11/mod.rs +++ b/src/platform/linux/x11/mod.rs @@ -16,7 +16,7 @@ use std::sync::{Arc, Mutex, Weak}; use std::sync::atomic::{self, AtomicBool}; use std::collections::HashMap; use std::ffi::CStr; -use std::os::raw::{c_char, c_int, c_long, c_uchar, c_ulong}; +use std::os::raw::{c_char, c_int, c_long, c_uchar, c_uint, c_ulong}; use libc; @@ -199,9 +199,26 @@ impl EventsLoop { { let xlib = &self.display.xlib; - // Handle dead keys and other input method funtimes - if ffi::True == unsafe { (self.display.xlib.XFilterEvent)(xev, { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window }) } { - return; + // XFilterEvent tells us when an event has been discarded by the input method. + // Specifically, this involves all of the KeyPress events in compose/pre-edit sequences, + // along with an extra copy of the KeyRelease events. + let filtered = ffi::True == unsafe { (self.display.xlib.XFilterEvent)( + xev, + { let xev: &ffi::XAnyEvent = xev.as_ref(); xev.window } + ) }; + + if filtered { + // If we directly follow the recommendation of XFilterEvent, we'll be getting + // KeyRelease events without their corresponding KeyPress events. This condition + // ensures that we only ignore the duplicate KeyRelease events. + // + // While we still want to propagate these KeyPress events, we have to remember not to + // emit events for the characters received from them. The `filtered` variable is + // created for that exact purpose, allowing us to make that check in the KeyPress + // handler. + if xev.get_type() == ffi::KeyRelease { + return; + } } match xev.get_type() { @@ -414,16 +431,14 @@ impl EventsLoop { callback(Event::WindowEvent { window_id, event: WindowEvent::Refresh }); } - // FIXME: Use XInput2 + libxkbcommon for keyboard input! ffi::KeyPress | ffi::KeyRelease => { use events::ElementState::{Pressed, Released}; - let state; - if xev.get_type() == ffi::KeyPress { - state = Pressed; + let state = if xev.get_type() == ffi::KeyPress { + Pressed } else { - state = Released; - } + Released + }; let xkev: &mut ffi::XKeyEvent = xev.as_mut(); @@ -439,55 +454,50 @@ impl EventsLoop { let keysym = unsafe { let mut keysym = 0; - (self.display.xlib.XLookupString)(xkev, ptr::null_mut(), 0, &mut keysym, ptr::null_mut()); + (self.display.xlib.XLookupString)( + xkev, + ptr::null_mut(), + 0, + &mut keysym, + ptr::null_mut(), + ); keysym }; - let vkey = events::keysym_to_element(keysym as libc::c_uint); - - callback(Event::WindowEvent { window_id, event: WindowEvent::KeyboardInput { - // Typical virtual core keyboard ID. xinput2 needs to be used to get a reliable value. - device_id: mkdid(3), - input: KeyboardInput { - state: state, - scancode: xkev.keycode - 8, - virtual_keycode: vkey, - modifiers, - }, - }}); + let virtual_keycode = events::keysym_to_element(keysym as c_uint); + + // When a compose sequence or IME pre-edit is finished, it ends in a KeyPress with + // a keycode of 0. + if xkev.keycode != 0 { + callback(Event::WindowEvent { window_id, event: WindowEvent::KeyboardInput { + // Standard virtual core keyboard ID. XInput2 needs to be used to get a + // reliable value, though this should only be an issue under multiseat + // configurations. + device_id: mkdid(3), + input: KeyboardInput { + state, + scancode: xkev.keycode - 8, + virtual_keycode, + modifiers, + }, + }}); + } - if state == Pressed { - let written = unsafe { - use std::str; + if state == Pressed && !filtered { + let written = { + let windows = self.windows.lock().unwrap(); - const INIT_BUFF_SIZE: usize = 16; - let mut windows = self.windows.lock().unwrap(); let window_data = { - if let Some(window_data) = windows.get_mut(&WindowId(window)) { + if let Some(window_data) = windows.get(&WindowId(window)) { window_data } else { return; } }; - /* buffer allocated on heap instead of stack, due to the possible - * reallocation */ - let mut buffer: Vec = vec![mem::uninitialized(); INIT_BUFF_SIZE]; - let mut keysym: ffi::KeySym = 0; - let mut status: ffi::Status = 0; - let mut count = (self.display.xlib.Xutf8LookupString)(window_data.ic, xkev, - mem::transmute(buffer.as_mut_ptr()), - buffer.len() as libc::c_int, - &mut keysym, &mut status); - /* buffer overflowed, dynamically reallocate */ - if status == ffi::XBufferOverflow { - buffer = vec![mem::uninitialized(); count as usize]; - count = (self.display.xlib.Xutf8LookupString)(window_data.ic, xkev, - mem::transmute(buffer.as_mut_ptr()), - buffer.len() as libc::c_int, - &mut keysym, &mut status); - } - str::from_utf8(&buffer[..count as usize]).unwrap_or("").to_string() + unsafe { + util::lookup_utf8(&self.display, window_data.ic, xkev) + } }; for chr in written.chars() { diff --git a/src/platform/linux/x11/util.rs b/src/platform/linux/x11/util.rs index 37e07db976b..4d86a511f73 100644 --- a/src/platform/linux/x11/util.rs +++ b/src/platform/linux/x11/util.rs @@ -1,5 +1,6 @@ use std::mem; use std::ptr; +use std::str; use std::sync::Arc; use std::ops::{Deref, DerefMut}; use std::os::raw::{c_char, c_double, c_int, c_long, c_short, c_uchar, c_uint, c_ulong}; @@ -247,3 +248,53 @@ pub unsafe fn query_pointer( relative_to_window, }) } + +unsafe fn lookup_utf8_inner( + xconn: &Arc, + ic: ffi::XIC, + key_event: &mut ffi::XKeyEvent, + buffer: &mut [u8], +) -> (ffi::KeySym, ffi::Status, c_int) { + let mut keysym: ffi::KeySym = 0; + let mut status: ffi::Status = 0; + let count = (xconn.xlib.Xutf8LookupString)( + ic, + key_event, + buffer.as_mut_ptr() as *mut c_char, + buffer.len() as c_int, + &mut keysym, + &mut status, + ); + (keysym, status, count) +} + +pub unsafe fn lookup_utf8( + xconn: &Arc, + ic: ffi::XIC, + key_event: &mut ffi::XKeyEvent, +) -> String { + const INIT_BUFF_SIZE: usize = 16; + + // Buffer allocated on heap instead of stack, due to the possible reallocation + let mut buffer: Vec = vec![mem::uninitialized(); INIT_BUFF_SIZE]; + let (_, status, mut count) = lookup_utf8_inner( + xconn, + ic, + key_event, + &mut buffer, + ); + + // Buffer overflowed, dynamically reallocate + if status == ffi::XBufferOverflow { + buffer = vec![mem::uninitialized(); count as usize]; + let (_, _, new_count) = lookup_utf8_inner( + xconn, + ic, + key_event, + &mut buffer, + ); + count = new_count; + } + + str::from_utf8(&buffer[..count as usize]).unwrap_or("").to_string() +}