X11: General cleanup (#491)

* X11: General cleanup

This is almost entirely internal changes, and as usual, doesn't actually
fix any problems people have complained about.

- `XSetInputFocus` can't be called before the window is visible. This
was previously handled by looping (with a sleep) and querying for the
window's state until it was visible. Now we use `XIfEvent`, which blocks
until we receive `VisibilityNotify`. Note that this can't be replaced
with an `XSync` (I tried).
- We now call `XSync` at the end of window creation and check for
errors, assuring that broken windows are never returned. When creating
invisible windows, this is the only time the output buffer is flushed
during the entire window creation process (AFAIK). For visible windows,
`XIfEvent` will generally flush, but window creation has overall been
reduced to the minimum number of flushes.
- `check_errors().expect()` has been a common pattern throughout the
backend, but it seems that people (myself included) didn't make a
distinction between using it after synchronous requests and asynchronous
requests. Now we only use it after async requests if we flush first,
though this still isn't correct (since the request likely hasn't been
processed yet). The only real solution (besides forcing a sync *every
time*) is to handle asynchronous errors *asynchronously*. For future
work, I plan on adding logging, though I don't plan on actually
*handling* those errors; that's more of something to hope for in the
hypothetical async/await XCB paradise.
- We now flush whenever it makes sense to. `util::Flusher` was added to
force contributors to be aware of the output buffer.
- `Window::get_position`, `Window::get_inner_position`,
`Window::get_inner_size`, and `Window::get_outer_size` previously all
required *several* round-trips. On my machine, it took an average of
around 80µs. They've now been reduced to one round-trip each, which
reduces my measurement to 16µs. This was accomplished simply by caching
the frame extents, which are expensive to calculate (due to various
queries and heuristics), but change infrequently and predictably. I
still recommend that application developers use these methods sparingly
and generally prefer storing the values from `Resized`/`Moved`, as
that's zero overhead.
- The above change enabled me to change the `Moved` event to supply
window positions, rather than client area positions. Additionally, we no
longer generate `Moved` for real (as in, not synthetic)
`ConfigureNotify` events. Real `ConfigureNotify` events contain
positions relative to the parent window, which are typically constant
and useless. Since that position would be completely different from the
root-relative positions supplied by synthetic `ConfigureNotify` events
(which are the vast majority of them), that meant real `ConfigureNotify`
events would *always* be detected as the position having changed, so the
resultant `Moved` was multiple levels of misleading. In practice, this
meant a garbage `Moved` would be sent every time the window was resized;
now a resize has to actually change the window's position to be
accompanied by `Moved`.
- Every time we processed an `XI_Enter` event, we would leak 4 bytes via
`util::query_pointer` (`XIQueryPointer`). `XIButtonState` contains a
dynamically-allocated mask field which we weren't freeing. As this event
occurs with fairly high frequency, long-running applications could
easily accumulate substantial leaks. `util::PointerState::drop` now
takes care of this.
- The `util` module has been split up into several sub-modules, as it
was getting rather lengthy. This accounts for a significant part of this
diff, unfortunately.
- Atoms are now cached. Xlib caches them too, so `XInternAtom` wouldn't
typically be a round-trip anyway, but the added complexity is
negligible.
- Switched from `std::sync::Mutex` to `parking_lot::Mutex` (within this
backend). There appears to be no downside to this, but if anyone finds
one, this would be easy to revert.
- The WM name and supported hints are now global to the application, and
are updated upon `ReparentNotify`, which should detect when the WM was
replaced (assuming a reparenting WM was involved, that is). Previously,
these values were per-window and would never update, meaning replacing
the WM could potentially lead to (admittedly very minor) problems.
- The result of `Window2::create_empty_cursor` will now only be used if
it actually succeeds.
- `Window2::load_cursor` no longer re-allocates the cursor name.
- `util::lookup_utf8` previously allocated a 16-byte buffer on the heap.
Now it allocates a 1024-byte buffer on the stack, and falls back to
dynamic allocation if the buffer is too small. This base buffer size is
admittedly gratuitous, but less so if you're using IME.
- `with_c_str` was finally removed.
- Added `util::Format` enum to help prevent goofs when dealing with
format arguments.
- `util::get_property`, something I added way back in my first winit PR,
only calculated offsets correctly for `util::Format::Char`. This was
concealed by the accomodating buffer size, as it would be very rare for
the offset to be needed; however, testing with a buffer size of 1,
`util::Format::Long` would read from the same offset multiple times, and
`util::Format::Short` would miss data. This function now works correctly
for all formats, relying on the simple fact that the offset increases by
the buffer size on each iteration. We also account for the extra byte
that `XGetWindowProperty` allocates at the end of the buffer, and copy
data from the buffer instead of moving it and taking ownership of the
pointer.
- Drag and drop now reliably works in release mode. This is presumably
related to the `util::get_property` changes.
- `util::change_property` now exists, which should make it easier to add
features in the future.
- The `EventsLoop` device map is no longer in a mutex.
- `XConnection` now implements `Debug`.
- Valgrind no longer complains about anything related to winit (with
either the system allocator or jemalloc, though "not having valgrind
complain about jemalloc" isn't something to strive for).

* X11: Add better diagnostics when initialization fails

* X11: Handle XIQueryDevice failure

* X11: Use correct types in error handler

CHANGELOG.md

@@ -7,9 +7,13 @@
 - Corrected `get_position` on Windows to be relative to the screen rather than to the taskbar.
 - Corrected `Moved` event on Windows to use position values equivalent to those returned by `get_position`. It previously supplied client area positions instead of window positions, and would additionally interpret negative values as being very large (around `u16::MAX`).
 - Implemented `Moved` event on macOS.
+- On X11, the `Moved` event correctly use window positions rather than client area positions. Additionally, a stray `Moved` that unconditionally accompanied `Resized` with the client area position relative to the parent has been eliminated; `Moved` is still received alongside `Resized`, but now only once and always correctly.
 - On Windows, implemented all variants of `DeviceEvent` other than `Text`. Mouse `DeviceEvent`s are now received even if the window isn't in the foreground.
 - `DeviceId` on Windows is no longer a unit struct, and now contains a `u32`. For `WindowEvent`s, this will always be 0, but on `DeviceEvent`s it will be the handle to that device. `DeviceIdExt::get_persistent_identifier` can be used to acquire a unique identifier for that device that persists across replugs/reboots/etc.
 - Corrected `run_forever` on X11 to stop discarding `Awakened` events.
+- Various safety and correctness improvements to the X11 backend internals.
+- Fixed memory leak on X11 every time the mouse entered the window.
+- On X11, drag and drop now works reliably in release mode.
 
 # Version 0.13.1 (2018-04-26)
 

Cargo.toml

@@ -49,4 +49,5 @@ wayland-protocols = { version = "0.12.0", features = ["unstable_protocols"] }
 wayland-kbd = "0.13.0"
 wayland-window = "0.13.0"
 x11-dl = "2.17.5"
+parking_lot = "0.5"
 percent-encoding = "1.0"

src/lib.rs

@@ -101,6 +101,8 @@ extern crate core_graphics;
 #[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))]
 extern crate x11_dl;
 #[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))]
+extern crate parking_lot;
+#[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))]
 extern crate percent_encoding;
 #[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "dragonfly", target_os = "openbsd"))]
 #[macro_use]

src/platform/linux/mod.rs

@@ -1,18 +1,19 @@
 #![cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd", target_os = "openbsd"))]
 
 use std::collections::VecDeque;
+use std::{env, mem};
+use std::ffi::CStr;
+use std::os::raw::*;
 use std::sync::Arc;
-use std::env;
 
-use {CreationError, CursorState, EventsLoopClosed, MouseCursor, ControlFlow};
 use libc;
 
+use {CreationError, CursorState, EventsLoopClosed, MouseCursor, ControlFlow};
+use window::MonitorId as RootMonitorId;
 use self::x11::XConnection;
 use self::x11::XError;
 use self::x11::ffi::XVisualInfo;
-
 pub use self::x11::XNotSupported;
-use window::MonitorId as RootMonitorId;
 
 mod dlopen;
 pub mod wayland;
@@ -301,15 +302,19 @@ impl Window {
     }
 }
 
-unsafe extern "C" fn x_error_callback(dpy: *mut x11::ffi::Display, event: *mut x11::ffi::XErrorEvent)
-                                      -> libc::c_int
-{
-    use std::ffi::CStr;
-
-    if let Ok(ref x) = *X11_BACKEND {
-        let mut buff: Vec<u8> = Vec::with_capacity(1024);
-        (x.xlib.XGetErrorText)(dpy, (*event).error_code as i32, buff.as_mut_ptr() as *mut libc::c_char, buff.capacity() as i32);
-        let description = CStr::from_ptr(buff.as_mut_ptr() as *const libc::c_char).to_string_lossy();
+unsafe extern "C" fn x_error_callback(
+    display: *mut x11::ffi::Display,
+    event: *mut x11::ffi::XErrorEvent,
+) -> c_int {
+    if let Ok(ref xconn) = *X11_BACKEND {
+        let mut buf: [c_char; 1024] = mem::uninitialized();
+        (xconn.xlib.XGetErrorText)(
+            display,
+            (*event).error_code as c_int,
+            buf.as_mut_ptr(),
+            buf.len() as c_int,
+        );
+        let description = CStr::from_ptr(buf.as_ptr()).to_string_lossy();
 
         let error = XError {
             description: description.into_owned(),
@@ -318,9 +323,9 @@ unsafe extern "C" fn x_error_callback(dpy: *mut x11::ffi::Display, event: *mut x
             minor_code: (*event).minor_code,
         };
 
-        *x.latest_error.lock().unwrap() = Some(error);
+        *xconn.latest_error.lock() = Some(error);
     }
-
+    // Fun fact: this return value is completely ignored.
     0
 }
 
@@ -340,28 +345,39 @@ impl EventsLoop {
         if let Ok(env_var) = env::var(BACKEND_PREFERENCE_ENV_VAR) {
             match env_var.as_str() {
                 "x11" => {
-                    return EventsLoop::new_x11().unwrap();      // TODO: propagate
+                    // TODO: propagate
+                    return EventsLoop::new_x11().expect("Failed to initialize X11 backend");
                 },
                 "wayland" => {
-                    match EventsLoop::new_wayland() {
-                        Ok(e) => return e,
-                        Err(_) => panic!()      // TODO: propagate
-                    }
+                    return EventsLoop::new_wayland()
+                        .expect("Failed to initialize Wayland backend");
                 },
-                _ => panic!("Unknown environment variable value for {}, try one of `x11`,`wayland`",
-                            BACKEND_PREFERENCE_ENV_VAR),
+                _ => panic!(
+                    "Unknown environment variable value for {}, try one of `x11`,`wayland`",
+                    BACKEND_PREFERENCE_ENV_VAR,
+                ),
             }
         }
 
-        if let Ok(el) = EventsLoop::new_wayland() {
-            return el;
-        }
+        let wayland_err = match EventsLoop::new_wayland() {
+            Ok(event_loop) => return event_loop,
+            Err(err) => err,
+        };
 
-        if let Ok(el) = EventsLoop::new_x11() {
-            return el;
-        }
+        let x11_err = match EventsLoop::new_x11() {
+            Ok(event_loop) => return event_loop,
+            Err(err) => err,
+        };
 
-        panic!("No backend is available")
+        let err_string = format!(
+r#"Failed to initialize any backend!
+    Wayland status: {:#?}
+    X11 status: {:#?}
+"#,
+            wayland_err,
+            x11_err,
+        );
+        panic!(err_string);
     }
 
     pub fn new_wayland() -> Result<EventsLoop, ()> {

src/platform/linux/x11/dnd.rs

@@ -2,14 +2,12 @@ use std::io;
 use std::sync::Arc;
 use std::path::{Path, PathBuf};
 use std::str::Utf8Error;
+use std::os::raw::*;
 
-use libc::{c_char, c_int, c_long, c_uchar, c_ulong};
 use percent_encoding::percent_decode;
 
 use super::{ffi, util, XConnection, XError};
 
-const DND_ATOMS_LEN: usize = 12;
-
 #[derive(Debug)]
 pub struct DndAtoms {
     pub aware: ffi::Atom,
@@ -28,36 +26,21 @@ pub struct DndAtoms {
 
 impl DndAtoms {
     pub fn new(xconn: &Arc<XConnection>) -> Result<Self, XError> {
-        let mut atoms = Vec::with_capacity(DND_ATOMS_LEN);
-
-        let mut names = [
-            b"XdndAware\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndEnter\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndLeave\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndDrop\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndPosition\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndStatus\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndActionPrivate\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndSelection\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndFinished\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"XdndTypeList\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"text/uri-list\0".to_owned().as_mut_ptr() as *mut c_char,
-            b"None\0".to_owned().as_mut_ptr() as *mut c_char,
+        let names = [
+            b"XdndAware\0".as_ptr() as *mut c_char,
+            b"XdndEnter\0".as_ptr() as *mut c_char,
+            b"XdndLeave\0".as_ptr() as *mut c_char,
+            b"XdndDrop\0".as_ptr() as *mut c_char,
+            b"XdndPosition\0".as_ptr() as *mut c_char,
+            b"XdndStatus\0".as_ptr() as *mut c_char,
+            b"XdndActionPrivate\0".as_ptr() as *mut c_char,
+            b"XdndSelection\0".as_ptr() as *mut c_char,
+            b"XdndFinished\0".as_ptr() as *mut c_char,
+            b"XdndTypeList\0".as_ptr() as *mut c_char,
+            b"text/uri-list\0".as_ptr() as *mut c_char,
+            b"None\0".as_ptr() as *mut c_char,
         ];
-
-        unsafe {
-            (xconn.xlib.XInternAtoms)(
-                xconn.display,
-                names.as_mut_ptr(),
-                DND_ATOMS_LEN as c_int,
-                ffi::False,
-                atoms.as_mut_ptr(),
-            );
-        }
-        xconn.check_errors()?;
-        unsafe {
-            atoms.set_len(DND_ATOMS_LEN);
-        }
+        let atoms = unsafe { util::get_atoms(xconn, &names) }?;
         Ok(DndAtoms {
             aware: atoms[0],
             enter: atoms[1],
@@ -151,7 +134,7 @@ impl Dnd {
             self.atoms.status,
             None,
             (this_window as c_long, accepted, 0, 0, action),
-        )
+        ).flush()
     }
 
     pub unsafe fn send_finished(
@@ -171,7 +154,7 @@ impl Dnd {
             self.atoms.finished,
             None,
             (this_window as c_long, accepted, action, 0, 0),
-        )
+        ).flush()
     }
 
     pub unsafe fn get_type_list(

src/platform/linux/x11/mod.rs

@@ -12,15 +12,16 @@ use {CreationError, Event, EventsLoopClosed, WindowEvent, DeviceEvent,
 use events::ModifiersState;
 
 use std::{mem, ptr, slice};
-use std::sync::{Arc, Mutex, Weak};
+use std::sync::{Arc, Weak};
 use std::sync::atomic::{self, AtomicBool};
 use std::sync::mpsc;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::ffi::CStr;
-use std::os::raw::{c_char, c_int, c_long, c_uchar, c_uint, c_ulong};
+use std::os::raw::*;
 
 use libc::{self, setlocale, LC_CTYPE};
+use parking_lot::Mutex;
 
 mod events;
 mod monitor;
@@ -33,13 +34,6 @@ mod util;
 use self::dnd::{Dnd, DndState};
 use self::ime::{ImeReceiver, ImeSender, ImeCreationError, Ime};
 
-// API TRANSITION
-//
-// We don't use the gen_api_transistion!() macro but rather do the expansion manually:
-//
-// As this module is nested into platform/linux, its code is not _exactly_ the same as
-// the one generated by the macro.
-
 pub struct EventsLoop {
     display: Arc<XConnection>,
     wm_delete_window: ffi::Atom,
@@ -48,7 +42,9 @@ pub struct EventsLoop {
     ime_sender: ImeSender,
     ime: RefCell<Ime>,
     windows: Arc<Mutex<HashMap<WindowId, WindowData>>>,
-    devices: Mutex<HashMap<DeviceId, Device>>,
+    // Please don't laugh at this type signature
+    shared_state: RefCell<HashMap<WindowId, Weak<Mutex<window::SharedState>>>>,
+    devices: RefCell<HashMap<DeviceId, Device>>,
     xi2ext: XExtension,
     pending_wakeup: Arc<AtomicBool>,
     root: ffi::Window,
@@ -66,8 +62,8 @@ pub struct EventsLoopProxy {
 
 impl EventsLoop {
     pub fn new(display: Arc<XConnection>) -> EventsLoop {
-        let wm_delete_window = unsafe { (display.xlib.XInternAtom)(display.display, b"WM_DELETE_WINDOW\0".as_ptr() as *const c_char, 0) };
-        display.check_errors().expect("Failed to call XInternAtom");
+        let wm_delete_window = unsafe { util::get_atom(&display, b"WM_DELETE_WINDOW\0") }
+            .expect("Failed to call XInternAtom (WM_DELETE_WINDOW)");
 
         let dnd = Dnd::new(Arc::clone(&display))
             .expect("Failed to call XInternAtoms when initializing drag and drop");
@@ -105,19 +101,36 @@ impl EventsLoop {
         unsafe {
             let mut xinput_major_ver = ffi::XI_2_Major;
             let mut xinput_minor_ver = ffi::XI_2_Minor;
-
-            if (display.xinput2.XIQueryVersion)(display.display, &mut xinput_major_ver, &mut xinput_minor_ver) != ffi::Success as libc::c_int {
-                panic!("X server has XInput extension {}.{} but does not support XInput2", xinput_major_ver, xinput_minor_ver);
+            if (display.xinput2.XIQueryVersion)(
+                display.display,
+                &mut xinput_major_ver,
+                &mut xinput_minor_ver,
+            ) != ffi::Success as libc::c_int {
+                panic!(
+                    "X server has XInput extension {}.{} but does not support XInput2",
+                    xinput_major_ver,
+                    xinput_minor_ver,
+                );
             }
         }
 
         let root = unsafe { (display.xlib.XDefaultRootWindow)(display.display) };
+        util::update_cached_wm_info(&display, root);
 
         let wakeup_dummy_window = unsafe {
             let (x, y, w, h) = (10, 10, 10, 10);
             let (border_w, border_px, background_px) = (0, 0, 0);
-            (display.xlib.XCreateSimpleWindow)(display.display, root, x, y, w, h,
-                                               border_w, border_px, background_px)
+            (display.xlib.XCreateSimpleWindow)(
+                display.display,
+                root,
+                x,
+                y,
+                w,
+                h,
+                border_w,
+                border_px,
+                background_px,
+            )
         };
 
         let result = EventsLoop {
@@ -129,27 +142,24 @@ impl EventsLoop {
             ime_sender,
             ime,
             windows: Arc::new(Mutex::new(HashMap::new())),
-            devices: Mutex::new(HashMap::new()),
+            shared_state: RefCell::new(HashMap::new()),
+            devices: RefCell::new(HashMap::new()),
             xi2ext,
             root,
             wakeup_dummy_window,
         };
 
-        {
         // Register for device hotplug events
-            let mask = ffi::XI_HierarchyChangedMask;
         unsafe {
-                let mut event_mask = ffi::XIEventMask{
-                    deviceid: ffi::XIAllDevices,
-                    mask: &mask as *const _ as *mut c_uchar,
-                    mask_len: mem::size_of_val(&mask) as c_int,
-                };
-                (result.display.xinput2.XISelectEvents)(result.display.display, root,
-                                                 &mut event_mask as *mut ffi::XIEventMask, 1);
-            }
+            util::select_xinput_events(
+                &result.display,
+                root,
+                ffi::XIAllDevices,
+                ffi::XI_HierarchyChangedMask,
+            )
+        }.queue(); // The request buffer is flushed during init_device
 
         result.init_device(ffi::XIAllDevices);
-        }
 
         result
     }
@@ -230,7 +240,8 @@ impl EventsLoop {
             return;
         }
 
-        match xev.get_type() {
+        let event_type = xev.get_type();
+        match event_type {
             ffi::MappingNotify => {
                 unsafe { (xlib.XRefreshKeyboardMapping)(xev.as_mut()); }
                 self.display.check_errors().expect("Failed to call XRefreshKeyboardMapping");
@@ -375,45 +386,127 @@ impl EventsLoop {
             ffi::ConfigureNotify => {
                 let xev: &ffi::XConfigureEvent = xev.as_ref();
 
+                // So apparently...
+                // XSendEvent (synthetic ConfigureNotify) -> position relative to root
+                // XConfigureNotify (real ConfigureNotify) -> position relative to parent
+                // https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.5
+                // We don't want to send Moved when this is true, since then every Resized
+                // (whether the window moved or not) is accompanied by an extraneous Moved event
+                // that has a position relative to the parent window.
+                let is_synthetic = xev.send_event == ffi::True;
+
                 let window = xev.window;
                 let window_id = mkwid(window);
 
                 let new_size = (xev.width, xev.height);
                 let new_position = (xev.x, xev.y);
-                // Gymnastics to ensure self.windows isn't locked when we invoke callback
+
                 let (resized, moved) = {
-                    let mut windows = self.windows.lock().unwrap();
+                    let mut windows = self.windows.lock();
                     if let Some(window_data) = windows.get_mut(&WindowId(window)) {
-                        if window_data.config.is_none() {
-                            window_data.config = Some(WindowConfig::new(xev));
-                            (true, true)
-                        } else {
-                            let window_state = window_data.config.as_mut().unwrap();
-                            (if window_state.size != new_size {
-                                window_state.size = new_size;
-                                true
-                            } else { false },
-                            if window_state.position != new_position {
-                                window_state.position = new_position;
-                                true
-                            } else { false })
+                        let (mut resized, mut moved) = (false, false);
+
+                        if window_data.config.size.is_none() {
+                            window_data.config.size = Some(new_size);
+                            resized = true;
                         }
+                        if window_data.config.size.is_none() && is_synthetic {
+                            window_data.config.position = Some(new_position);
+                            moved = true;
+                        }
+
+                        if !resized {
+                            if window_data.config.size != Some(new_size) {
+                                window_data.config.size = Some(new_size);
+                                resized = true;
+                            }
+                        }
+                        if !moved && is_synthetic {
+                            if window_data.config.position != Some(new_position) {
+                                window_data.config.position = Some(new_position);
+                                moved = true;
+                            }
+                        }
+
+                        if !is_synthetic
+                        && window_data.config.inner_position != Some(new_position) {
+                            window_data.config.inner_position = Some(new_position);
+                            // This way, we get sent Moved when the decorations are toggled.
+                            window_data.config.position = None;
+                            self.shared_state.borrow().get(&WindowId(window)).map(|window_state| {
+                                if let Some(window_state) = window_state.upgrade() {
+                                    // Extra insurance against stale frame extents
+                                    (*window_state.lock()).frame_extents.take();
+                                }
+                            });
+                        }
+
+                        (resized, moved)
                     } else {
                         return;
                     }
                 };
+
                 if resized {
+                    let (width, height) = (xev.width as u32, xev.height as u32);
                     callback(Event::WindowEvent {
                         window_id,
-                        event: WindowEvent::Resized(xev.width as u32, xev.height as u32),
+                        event: WindowEvent::Resized(width, height),
                     });
                 }
+
                 if moved {
+                    // We need to convert client area position to window position.
+                    self.shared_state.borrow().get(&WindowId(window)).map(|window_state| {
+                        if let Some(window_state) = window_state.upgrade() {
+                            let (x, y) = {
+                                let (inner_x, inner_y) = (xev.x as i32, xev.y as i32);
+                                let mut window_state_lock = window_state.lock();
+                                if (*window_state_lock).frame_extents.is_some() {
+                                    (*window_state_lock).frame_extents
+                                        .as_ref()
+                                        .unwrap()
+                                        .inner_pos_to_outer(inner_x, inner_y)
+                                } else {
+                                    let extents = util::get_frame_extents_heuristic(
+                                        &self.display,
+                                        window,
+                                        self.root,
+                                    );
+                                    let outer_pos = extents.inner_pos_to_outer(inner_x, inner_y);
+                                    (*window_state_lock).frame_extents = Some(extents);
+                                    outer_pos
+                                }
+                            };
                             callback(Event::WindowEvent {
                                 window_id,
-                        event: WindowEvent::Moved(xev.x as i32, xev.y as i32),
+                                event: WindowEvent::Moved(x, y),
                             });
                         }
+                    });
+                }
+            }
+
+            ffi::ReparentNotify => {
+                let xev: &ffi::XReparentEvent = xev.as_ref();
+
+                let window = xev.window;
+
+                // This is generally a reliable way to detect when the window manager's been
+                // replaced, though this event is only fired by reparenting window managers
+                // (which is almost all of them). Failing to correctly update WM info doesn't
+                // really have much impact, since on the WMs affected (xmonad, dwm, etc.) the only
+                // effect is that we waste some time trying to query unsupported properties.
+                util::update_cached_wm_info(&self.display, self.root);
+
+                self.shared_state
+                    .borrow()
+                    .get(&WindowId(window))
+                    .map(|window_state| {
+                        if let Some(window_state) = window_state.upgrade() {
+                            (*window_state.lock()).frame_extents.take();
+                        }
+                    });
             }
 
             ffi::DestroyNotify => {
@@ -424,7 +517,7 @@ impl EventsLoop {
 
                 // In the event that the window's been destroyed without being dropped first, we
                 // cleanup again here.
-                self.windows.lock().unwrap().remove(&WindowId(window));
+                self.windows.lock().remove(&WindowId(window));
 
                 // Since all XIM stuff needs to happen from the same thread, we destroy the input
                 // context here instead of when dropping the window.
@@ -460,6 +553,14 @@ impl EventsLoop {
                 let window = xkev.window;
                 let window_id = mkwid(window);
 
+                // 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.
+                let device = 3;
+                let device_id = mkdid(device);
+
+                // When a compose sequence or IME pre-edit is finished, it ends in a KeyPress with
+                // a keycode of 0.
+                if xkev.keycode != 0 {
                     let modifiers = ModifiersState {
                         alt: xkev.state & ffi::Mod1Mask != 0,
                         shift: xkev.state & ffi::ShiftMask != 0,
@@ -476,26 +577,23 @@ impl EventsLoop {
                             &mut keysym,
                             ptr::null_mut(),
                         );
+                        self.display.check_errors().expect("Failed to lookup keysym");
                         keysym
                     };
-
                     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),
+                    callback(Event::WindowEvent {
+                        window_id,
+                        event: WindowEvent::KeyboardInput {
+                            device_id,
                             input: KeyboardInput {
                                 state,
                                 scancode: xkev.keycode - 8,
                                 virtual_keycode,
                                 modifiers,
                             },
-                    }});
+                        }
+                    });
                 }
 
                 if state == Pressed {
@@ -534,7 +632,7 @@ impl EventsLoop {
                         let window_id = mkwid(xev.event);
                         let device_id = mkdid(xev.deviceid);
                         if (xev.flags & ffi::XIPointerEmulated) != 0 {
-                            let windows = self.windows.lock().unwrap();
+                            let windows = self.windows.lock();
                             if let Some(window_data) = windows.get(&WindowId(xev.event)) {
                                 if window_data.multitouch {
                                     // Deliver multi-touch events instead of emulated mouse events.
@@ -623,7 +721,7 @@ impl EventsLoop {
 
                         // Gymnastics to ensure self.windows isn't locked when we invoke callback
                         if {
-                            let mut windows = self.windows.lock().unwrap();
+                            let mut windows = self.windows.lock();
                             let window_data = {
                                 if let Some(window_data) = windows.get_mut(&WindowId(xev.event)) {
                                     window_data
@@ -650,8 +748,11 @@ impl EventsLoop {
                         let mut events = Vec::new();
                         {
                             let mask = unsafe { slice::from_raw_parts(xev.valuators.mask, xev.valuators.mask_len as usize) };
-                            let mut devices = self.devices.lock().unwrap();
-                            let physical_device = devices.get_mut(&DeviceId(xev.sourceid)).unwrap();
+                            let mut devices = self.devices.borrow_mut();
+                            let physical_device = match devices.get_mut(&DeviceId(xev.sourceid)) {
+                                Some(device) => device,
+                                None => return,
+                            };
 
                             let mut value = xev.valuators.values;
                             for i in 0..xev.valuators.mask_len*8 {
@@ -698,11 +799,16 @@ impl EventsLoop {
                         let window_id = mkwid(xev.event);
                         let device_id = mkdid(xev.deviceid);
 
-                        let mut devices = self.devices.lock().unwrap();
-                        let physical_device = devices.get_mut(&DeviceId(xev.sourceid)).unwrap();
-                        for info in DeviceInfo::get(&self.display, ffi::XIAllDevices).iter() {
-                            if info.deviceid == xev.sourceid {
-                                physical_device.reset_scroll_position(info);
+                        let mut devices = self.devices.borrow_mut();
+                        let physical_device = match devices.get_mut(&DeviceId(xev.sourceid)) {
+                            Some(device) => device,
+                            None => return,
+                        };
+                        if let Some(all_info) = DeviceInfo::get(&self.display, ffi::XIAllDevices) {
+                            for device_info in all_info.iter() {
+                                if device_info.deviceid == xev.sourceid {
+                                    physical_device.reset_scroll_position(device_info);
+                                }
                             }
                         }
                         callback(Event::WindowEvent {
@@ -711,15 +817,17 @@ impl EventsLoop {
                         });
 
                         let new_cursor_pos = (xev.event_x, xev.event_y);
-                        // The mods field on this event isn't actually useful, so we have to
-                        // query the pointer device.
+
+                        // The mods field on this event isn't actually populated, so query the
+                        // pointer device. In the future, we can likely remove this round-trip by
+                        // relying on Xkb for modifier values.
                         let modifiers = unsafe {
                             util::query_pointer(
                                 &self.display,
                                 xev.event,
                                 xev.deviceid,
-                            ).expect("Failed to query pointer device")
-                        }.get_modifier_state();
+                            )
+                        }.expect("Failed to query pointer device").get_modifier_state();
 
                         callback(Event::WindowEvent { window_id, event: CursorMoved {
                             device_id,
@@ -734,7 +842,6 @@ impl EventsLoop {
                         // been destroyed, which the user presumably doesn't want to deal with.
                         let window_closed = self.windows
                             .lock()
-                            .unwrap()
                             .get(&WindowId(xev.event))
                             .is_none();
 
@@ -750,7 +857,7 @@ impl EventsLoop {
 
                         let window_id = mkwid(xev.event);
 
-                        if let None = self.windows.lock().unwrap().get(&WindowId(xev.event)) {
+                        if let None = self.windows.lock().get(&WindowId(xev.event)) {
                             return;
                         }
                         self.ime
@@ -762,11 +869,11 @@ impl EventsLoop {
 
                         // The deviceid for this event is for a keyboard instead of a pointer,
                         // so we have to do a little extra work.
-                        let device_info = DeviceInfo::get(&self.display, xev.deviceid);
-                        // For master devices, the attachment field contains the ID of the
-                        // paired master device; for the master keyboard, the attachment is
-                        // the master pointer, and vice versa.
-                        let pointer_id = unsafe { (*device_info.info) }.attachment;
+                        let pointer_id = self.devices
+                            .borrow()
+                            .get(&DeviceId(xev.deviceid))
+                            .map(|device| device.attachment)
+                            .unwrap_or(2);
 
                         callback(Event::WindowEvent {
                             window_id,
@@ -780,7 +887,7 @@ impl EventsLoop {
                     ffi::XI_FocusOut => {
                         let xev: &ffi::XIFocusOutEvent = unsafe { &*(xev.data as *const _) };
 
-                        if let None = self.windows.lock().unwrap().get(&WindowId(xev.event)) {
+                        if let None = self.windows.lock().get(&WindowId(xev.event)) {
                             return;
                         }
                         self.ime
@@ -868,19 +975,44 @@ impl EventsLoop {
                     }
 
                     ffi::XI_RawKeyPress | ffi::XI_RawKeyRelease => {
-                        // TODO: Use xkbcommon for keysym and text decoding
                         let xev: &ffi::XIRawEvent = unsafe { &*(xev.data as *const _) };
-                        let xkeysym = unsafe { (self.display.xlib.XKeycodeToKeysym)(self.display.display, xev.detail as ffi::KeyCode, 0) };
-                        callback(Event::DeviceEvent { device_id: mkdid(xev.deviceid), event: DeviceEvent::Key(KeyboardInput {
-                            scancode: (xev.detail - 8) as u32,
-                            virtual_keycode: events::keysym_to_element(xkeysym as libc::c_uint),
-                            state: match xev.evtype {
+
+                        let state = match xev.evtype {
                             ffi::XI_RawKeyPress => Pressed,
                             ffi::XI_RawKeyRelease => Released,
                             _ => unreachable!(),
-                            },
+                        };
+
+                        let device_id = xev.sourceid;
+                        let keycode = xev.detail;
+                        if keycode < 8 { return; }
+                        let scancode = (keycode - 8) as u32;
+
+                        let keysym = unsafe {
+                            (self.display.xlib.XKeycodeToKeysym)(
+                                self.display.display,
+                                xev.detail as ffi::KeyCode,
+                                0,
+                            )
+                        };
+                        self.display.check_errors().expect("Failed to lookup raw keysym");
+
+                        let virtual_keycode = events::keysym_to_element(keysym as c_uint);
+
+                        callback(Event::DeviceEvent {
+                            device_id: mkdid(device_id),
+                            event: DeviceEvent::Key(KeyboardInput {
+                                scancode,
+                                virtual_keycode,
+                                state,
+                                // So, in an ideal world we can use libxkbcommon to get modifiers.
+                                // However, libxkbcommon-x11 isn't as commonly installed as one
+                                // would hope. We can still use the Xkb extension to get
+                                // comprehensive keyboard state updates, but interpreting that
+                                // info manually is going to be involved.
                                 modifiers: ModifiersState::default(),
-                        })});
+                            }),
+                        });
                     }
 
                     ffi::XI_HierarchyChanged => {
@@ -891,7 +1023,7 @@ impl EventsLoop {
                                 callback(Event::DeviceEvent { device_id: mkdid(info.deviceid), event: DeviceEvent::Added });
                             } else if 0 != info.flags & (ffi::XISlaveRemoved | ffi::XIMasterRemoved) {
                                 callback(Event::DeviceEvent { device_id: mkdid(info.deviceid), event: DeviceEvent::Removed });
-                                let mut devices = self.devices.lock().unwrap();
+                                let mut devices = self.devices.borrow_mut();
                                 devices.remove(&DeviceId(info.deviceid));
                             }
                         }
@@ -899,25 +1031,26 @@ impl EventsLoop {
 
                     _ => {}
                 }
-            }
-
-            _ => {}
+            },
+            _ => (),
         }
 
         match self.ime_receiver.try_recv() {
             Ok((window_id, x, y)) => {
                 self.ime.borrow_mut().send_xim_spot(window_id, x, y);
-            }
-            Err(_) => ()
+            },
+            Err(_) => (),
         }
     }
 
     fn init_device(&self, device: c_int) {
-        let mut devices = self.devices.lock().unwrap();
-        for info in DeviceInfo::get(&self.display, device).iter() {
+        let mut devices = self.devices.borrow_mut();
+        if let Some(info) = DeviceInfo::get(&self.display, device) {
+            for info in info.iter() {
                 devices.insert(DeviceId(info.deviceid), Device::new(&self, info));
             }
         }
+    }
 }
 
 impl EventsLoopProxy {
@@ -933,28 +1066,19 @@ impl EventsLoopProxy {
 
         // Push an event on the X event queue so that methods run_forever will advance.
         //
-        // NOTE: This code (and the following `XSendEvent` code) is taken from the old
-        // `WindowProxy::wakeup` implementation. The code assumes that X11 is thread safe. Is this
-        // true?
-        let mut xev = ffi::XClientMessageEvent {
-            type_: ffi::ClientMessage,
-            window: self.wakeup_dummy_window,
-            format: 32,
-            message_type: 0,
-            serial: 0,
-            send_event: 0,
-            display: display.display,
-            data: unsafe { mem::zeroed() },
-        };
-
+        // NOTE: This design is taken from the old `WindowProxy::wakeup` implementation. It
+        // assumes that X11 is thread safe. Is this true?
+        // (WARNING: it's probably not true)
         unsafe {
-            let propagate = false as i32;
-            let event_mask = 0;
-            let xevent = &mut xev as *mut ffi::XClientMessageEvent as *mut ffi::XEvent;
-            (display.xlib.XSendEvent)(display.display, self.wakeup_dummy_window, propagate, event_mask, xevent);
-            (display.xlib.XFlush)(display.display);
-            display.check_errors().expect("Failed to call XSendEvent after wakeup");
-        }
+            util::send_client_msg(
+                &display,
+                self.wakeup_dummy_window,
+                self.wakeup_dummy_window,
+                0,
+                None,
+                (0, 0, 0, 0, 0),
+            )
+        }.flush().expect("Failed to call XSendEvent after wakeup");
 
         Ok(())
     }
@@ -967,21 +1091,30 @@ struct DeviceInfo<'a> {
 }
 
 impl<'a> DeviceInfo<'a> {
-    fn get(display: &'a XConnection, device: c_int) -> Self {
+    fn get(display: &'a XConnection, device: c_int) -> Option<Self> {
         unsafe {
             let mut count = mem::uninitialized();
             let info = (display.xinput2.XIQueryDevice)(display.display, device, &mut count);
-            DeviceInfo {
-                display: display,
-                info: info,
+            display.check_errors()
+                .ok()
+                .and_then(|_| {
+                    if info.is_null() || count == 0 {
+                        None
+                    } else {
+                        Some(DeviceInfo {
+                            display,
+                            info,
                             count: count as usize,
+                        })
                     }
+                })
         }
     }
 }
 
 impl<'a> Drop for DeviceInfo<'a> {
     fn drop(&mut self) {
+        assert!(!self.info.is_null());
         unsafe { (self.display.xinput2.XIFreeDeviceInfo)(self.info as *mut _) };
     }
 }
@@ -1020,15 +1153,19 @@ impl Window {
         window: &::WindowAttributes,
         pl_attribs: &PlatformSpecificWindowBuilderAttributes
     ) -> Result<Self, CreationError> {
-        let win = Arc::new(try!(Window2::new(&x_events_loop, window, pl_attribs)));
+        let win = Arc::new(Window2::new(&x_events_loop, window, pl_attribs)?);
+
+        x_events_loop.shared_state
+            .borrow_mut()
+            .insert(win.id(), Arc::downgrade(&win.shared_state));
 
         x_events_loop.ime
             .borrow_mut()
             .create_context(win.id().0)
             .expect("Failed to create input context");
 
-        x_events_loop.windows.lock().unwrap().insert(win.id(), WindowData {
-            config: None,
+        x_events_loop.windows.lock().insert(win.id(), WindowData {
+            config: Default::default(),
             multitouch: window.multitouch,
             cursor_pos: None,
         });
@@ -1050,7 +1187,6 @@ impl Window {
     pub fn send_xim_spot(&self, x: i16, y: i16) {
         let _ = self.ime_sender
             .lock()
-            .unwrap()
             .send((self.window.id().0, x, y));
     }
 }
@@ -1058,10 +1194,9 @@ impl Window {
 impl Drop for Window {
     fn drop(&mut self) {
         if let (Some(windows), Some(display)) = (self.windows.upgrade(), self.display.upgrade()) {
-            if let Some(_) = windows.lock().unwrap().remove(&self.window.id()) {
+            if let Some(_) = windows.lock().remove(&self.window.id()) {
                 unsafe {
                     (display.xlib.XDestroyWindow)(display.display, self.window.id().0);
-                    display.check_errors().expect("Failed to destroy window");
                 }
             }
         }
@@ -1069,8 +1204,9 @@ impl Drop for Window {
 }
 
 /// State maintained for translating window-related events
+#[derive(Debug)]
 struct WindowData {
-    config: Option<WindowConfig>,
+    config: WindowConfig,
     multitouch: bool,
     cursor_pos: Option<(f64, f64)>,
 }
@@ -1078,21 +1214,13 @@ struct WindowData {
 // Required by ffi members
 unsafe impl Send for WindowData {}
 
+#[derive(Debug, Default)]
 struct WindowConfig {
-    size: (c_int, c_int),
-    position: (c_int, c_int),
+    pub size: Option<(c_int, c_int)>,
+    pub position: Option<(c_int, c_int)>,
+    pub inner_position: Option<(c_int, c_int)>,
 }
 
-impl WindowConfig {
-    fn new(event: &ffi::XConfigureEvent) -> Self {
-        WindowConfig {
-            size: (event.width, event.height),
-            position: (event.x, event.y),
-        }
-    }
-}
-
-
 /// XEvents of type GenericEvent store their actual data in an XGenericEventCookie data structure. This is a wrapper to
 /// extract the cookie from a GenericEvent XEvent and release the cookie data once it has been processed
 struct GenericEventCookie<'a> {
@@ -1136,6 +1264,9 @@ fn mkdid(w: c_int) -> ::DeviceId { ::DeviceId(::platform::DeviceId::X(DeviceId(w
 struct Device {
     name: String,
     scroll_axes: Vec<(i32, ScrollAxis)>,
+    // For master devices, this is the paired device (pointer <-> keyboard).
+    // For slave devices, this is the master.
+    attachment: c_int,
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -1152,24 +1283,25 @@ enum ScrollOrientation {
 }
 
 impl Device {
-    fn new(el: &EventsLoop, info: &ffi::XIDeviceInfo) -> Self
-    {
+    fn new(el: &EventsLoop, info: &ffi::XIDeviceInfo) -> Self {
         let name = unsafe { CStr::from_ptr(info.name).to_string_lossy() };
         let mut scroll_axes = Vec::new();
 
         if Device::physical_device(info) {
             // Register for global raw events
             let mask = ffi::XI_RawMotionMask
-                | ffi::XI_RawButtonPressMask | ffi::XI_RawButtonReleaseMask
-                | ffi::XI_RawKeyPressMask | ffi::XI_RawKeyReleaseMask;
+                | ffi::XI_RawButtonPressMask
+                | ffi::XI_RawButtonReleaseMask
+                | ffi::XI_RawKeyPressMask
+                | ffi::XI_RawKeyReleaseMask;
             unsafe {
-                let mut event_mask = ffi::XIEventMask{
-                    deviceid: info.deviceid,
-                    mask: &mask as *const _ as *mut c_uchar,
-                    mask_len: mem::size_of_val(&mask) as c_int,
-                };
-                (el.display.xinput2.XISelectEvents)(el.display.display, el.root, &mut event_mask as *mut ffi::XIEventMask, 1);
-            }
+                util::select_xinput_events(
+                    &el.display,
+                    el.root,
+                    info.deviceid,
+                    mask,
+                )
+            }.queue(); // The request buffer is flushed when we poll for events
 
             // Identify scroll axes
             for class_ptr in Device::classes(info) {
@@ -1195,6 +1327,7 @@ impl Device {
         let mut device = Device {
             name: name.into_owned(),
             scroll_axes: scroll_axes,
+            attachment: info.attachment,
         };
         device.reset_scroll_position(info);
         device

src/platform/linux/x11/util.rs

@@ -1,365 +0,0 @@
-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};
-
-use super::{ffi, XConnection, XError};
-use events::ModifiersState;
-
-pub struct XSmartPointer<'a, T> {
-    xconn: &'a Arc<XConnection>,
-    pub ptr: *mut T,
-}
-
-impl<'a, T> XSmartPointer<'a, T> {
-    // You're responsible for only passing things to this that should be XFree'd.
-    // Returns None if ptr is null.
-    pub fn new(xconn: &'a Arc<XConnection>, ptr: *mut T) -> Option<Self> {
-        if !ptr.is_null() {
-            Some(XSmartPointer {
-                xconn,
-                ptr,
-            })
-        } else {
-            None
-        }
-    }
-}
-
-impl<'a, T> Deref for XSmartPointer<'a, T> {
-    type Target = T;
-
-    fn deref(&self) -> &T {
-        unsafe { &*self.ptr }
-    }
-}
-
-impl<'a, T> DerefMut for XSmartPointer<'a, T> {
-    fn deref_mut(&mut self) -> &mut T {
-        unsafe { &mut *self.ptr }
-    }
-}
-
-impl<'a, T> Drop for XSmartPointer<'a, T> {
-    fn drop(&mut self) {
-        unsafe {
-            (self.xconn.xlib.XFree)(self.ptr as *mut _);
-        }
-    }
-}
-
-pub unsafe fn get_atom(xconn: &Arc<XConnection>, name: &[u8]) -> Result<ffi::Atom, XError> {
-    let atom_name: *const c_char = name.as_ptr() as _;
-    let atom = (xconn.xlib.XInternAtom)(xconn.display, atom_name, ffi::False);
-    xconn.check_errors().map(|_| atom)
-}
-
-pub unsafe fn send_client_msg(
-    xconn: &Arc<XConnection>,
-    window: c_ulong,        // the window this is "about"; not necessarily this window
-    target_window: c_ulong, // the window we're sending to
-    message_type: ffi::Atom,
-    event_mask: Option<c_long>,
-    data: (c_long, c_long, c_long, c_long, c_long),
-) -> Result<(), XError> {
-    let mut event: ffi::XClientMessageEvent = mem::uninitialized();
-    event.type_ = ffi::ClientMessage;
-    event.display = xconn.display;
-    event.window = window;
-    event.message_type = message_type;
-    event.format = 32;
-    event.data = ffi::ClientMessageData::new();
-    event.data.set_long(0, data.0);
-    event.data.set_long(1, data.1);
-    event.data.set_long(2, data.2);
-    event.data.set_long(3, data.3);
-    event.data.set_long(4, data.4);
-
-    let event_mask = event_mask.unwrap_or(ffi::NoEventMask);
-
-    (xconn.xlib.XSendEvent)(
-        xconn.display,
-        target_window,
-        ffi::False,
-        event_mask,
-        &mut event.into(),
-    );
-
-    xconn.check_errors().map(|_| ())
-}
-
-#[derive(Debug, Clone)]
-pub enum GetPropertyError {
-    XError(XError),
-    TypeMismatch(ffi::Atom),
-    FormatMismatch(c_int),
-    NothingAllocated,
-}
-
-impl GetPropertyError {
-    pub fn is_actual_property_type(&self, t: ffi::Atom) -> bool {
-        if let GetPropertyError::TypeMismatch(actual_type) = *self {
-            actual_type == t
-        } else {
-            false
-        }
-    }
-}
-
-pub unsafe fn get_property<T>(
-    xconn: &Arc<XConnection>,
-    window: c_ulong,
-    property: ffi::Atom,
-    property_type: ffi::Atom,
-) -> Result<Vec<T>, GetPropertyError> {
-    let mut data = Vec::new();
-
-    let mut done = false;
-    while !done {
-        let mut actual_type: ffi::Atom = mem::uninitialized();
-        let mut actual_format: c_int = mem::uninitialized();
-        let mut byte_count: c_ulong = mem::uninitialized();
-        let mut bytes_after: c_ulong = mem::uninitialized();
-        let mut buf: *mut c_uchar = ptr::null_mut();
-        (xconn.xlib.XGetWindowProperty)(
-            xconn.display,
-            window,
-            property,
-            (data.len() / 4) as c_long,
-            1024,
-            ffi::False,
-            property_type,
-            &mut actual_type,
-            &mut actual_format,
-            &mut byte_count,
-            &mut bytes_after,
-            &mut buf,
-        );
-
-        if let Err(e) = xconn.check_errors() {
-            return Err(GetPropertyError::XError(e));
-        }
-
-        if actual_type != property_type {
-            return Err(GetPropertyError::TypeMismatch(actual_type));
-        }
-
-        // Fun fact: actual_format ISN'T the size of the type; it's more like a really bad enum
-        let format_mismatch = match actual_format as usize {
-            8 => mem::size_of::<T>() != mem::size_of::<c_char>(),
-            16 => mem::size_of::<T>() != mem::size_of::<c_short>(),
-            32 => mem::size_of::<T>() != mem::size_of::<c_long>(),
-            _ => true, // this won't actually be reached; the XError condition above is triggered
-        };
-
-        if format_mismatch {
-            return Err(GetPropertyError::FormatMismatch(actual_format));
-        }
-
-        if !buf.is_null() {
-            let mut buf =
-                Vec::from_raw_parts(buf as *mut T, byte_count as usize, byte_count as usize);
-            data.append(&mut buf);
-        } else {
-            return Err(GetPropertyError::NothingAllocated);
-        }
-
-        done = bytes_after == 0;
-    }
-
-    Ok(data)
-}
-
-impl From<ffi::XIModifierState> for ModifiersState {
-    fn from(mods: ffi::XIModifierState) -> Self {
-        let state = mods.effective as c_uint;
-        ModifiersState {
-            alt: state & ffi::Mod1Mask != 0,
-            shift: state & ffi::ShiftMask != 0,
-            ctrl: state & ffi::ControlMask != 0,
-            logo: state & ffi::Mod4Mask != 0,
-        }
-    }
-}
-
-#[derive(Debug)]
-pub struct PointerState {
-    #[allow(dead_code)]
-    root: ffi::Window,
-    #[allow(dead_code)]
-    child: ffi::Window,
-    #[allow(dead_code)]
-    root_x: c_double,
-    #[allow(dead_code)]
-    root_y: c_double,
-    #[allow(dead_code)]
-    win_x: c_double,
-    #[allow(dead_code)]
-    win_y: c_double,
-    #[allow(dead_code)]
-    buttons: ffi::XIButtonState,
-    modifiers: ffi::XIModifierState,
-    #[allow(dead_code)]
-    group: ffi::XIGroupState,
-    #[allow(dead_code)]
-    relative_to_window: bool,
-}
-
-impl PointerState {
-    pub fn get_modifier_state(&self) -> ModifiersState {
-        self.modifiers.into()
-    }
-}
-
-pub unsafe fn query_pointer(
-    xconn: &Arc<XConnection>,
-    window: ffi::Window,
-    device_id: c_int,
-) -> Result<PointerState, XError> {
-    let mut root_return = mem::uninitialized();
-    let mut child_return = mem::uninitialized();
-    let mut root_x_return = mem::uninitialized();
-    let mut root_y_return = mem::uninitialized();
-    let mut win_x_return = mem::uninitialized();
-    let mut win_y_return = mem::uninitialized();
-    let mut buttons_return = mem::uninitialized();
-    let mut modifiers_return = mem::uninitialized();
-    let mut group_return = mem::uninitialized();
-
-    let relative_to_window = (xconn.xinput2.XIQueryPointer)(
-        xconn.display,
-        device_id,
-        window,
-        &mut root_return,
-        &mut child_return,
-        &mut root_x_return,
-        &mut root_y_return,
-        &mut win_x_return,
-        &mut win_y_return,
-        &mut buttons_return,
-        &mut modifiers_return,
-        &mut group_return,
-    ) == ffi::True;
-
-    xconn.check_errors()?;
-
-    Ok(PointerState {
-        root: root_return,
-        child: child_return,
-        root_x: root_x_return,
-        root_y: root_y_return,
-        win_x: win_x_return,
-        win_y: win_y_return,
-        buttons: buttons_return,
-        modifiers: modifiers_return,
-        group: group_return,
-        relative_to_window,
-    })
-}
-
-unsafe fn lookup_utf8_inner(
-    xconn: &Arc<XConnection>,
-    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<XConnection>,
-    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<u8> = 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()
-}
-
-#[derive(Debug)]
-pub struct FrameExtents {
-    pub left: c_ulong,
-    pub right: c_ulong,
-    pub top: c_ulong,
-    pub bottom: c_ulong,
-}
-
-impl FrameExtents {
-    pub fn new(left: c_ulong, right: c_ulong, top: c_ulong, bottom: c_ulong) -> Self {
-        FrameExtents { left, right, top, bottom }
-    }
-
-    pub fn from_border(border: c_ulong) -> Self {
-        Self::new(border, border, border, border)
-    }
-}
-
-#[derive(Debug)]
-pub struct WindowGeometry {
-    pub x: c_int,
-    pub y: c_int,
-    pub width: c_uint,
-    pub height: c_uint,
-    pub frame: FrameExtents,
-}
-
-impl WindowGeometry {
-    pub fn get_position(&self) -> (i32, i32) {
-        (self.x as _, self.y as _)
-    }
-
-    pub fn get_inner_position(&self) -> (i32, i32) {
-        (
-            self.x.saturating_add(self.frame.left as c_int) as _,
-            self.y.saturating_add(self.frame.top as c_int) as _,
-        )
-    }
-
-    pub fn get_inner_size(&self) -> (u32, u32) {
-        (self.width as _, self.height as _)
-    }
-
-    pub fn get_outer_size(&self) -> (u32, u32) {
-        (
-            self.width.saturating_add(
-                self.frame.left.saturating_add(self.frame.right) as c_uint
-            ) as _,
-            self.height.saturating_add(
-                self.frame.top.saturating_add(self.frame.bottom) as c_uint
-            ) as _,
-        )
-    }
-}

src/platform/linux/x11/util/atom.rs

@@ -0,0 +1,59 @@
+use std::collections::HashMap;
+use std::ffi::{CStr, CString};
+use std::os::raw::*;
+
+use parking_lot::Mutex;
+
+use super::*;
+
+type AtomCache = HashMap<CString, ffi::Atom>;
+
+lazy_static! {
+    static ref ATOM_CACHE: Mutex<AtomCache> = Mutex::new(HashMap::with_capacity(2048));
+}
+
+pub unsafe fn get_atom(xconn: &Arc<XConnection>, name: &[u8]) -> Result<ffi::Atom, XError> {
+    let name = CStr::from_bytes_with_nul_unchecked(name); // I trust you. Don't let me down.
+    let mut atom_cache_lock = ATOM_CACHE.lock();
+    let cached_atom = (*atom_cache_lock).get(name).cloned();
+    if let Some(atom) = cached_atom {
+        Ok(atom)
+    } else {
+        let atom = (xconn.xlib.XInternAtom)(
+            xconn.display,
+            name.as_ptr() as *const c_char,
+            ffi::False,
+        );
+        /*println!(
+            "XInternAtom name:{:?} atom:{:?}",
+            name,
+            atom,
+        );*/
+        xconn.check_errors()?;
+        (*atom_cache_lock).insert(name.to_owned(), atom);
+        Ok(atom)
+    }
+}
+
+// Note: this doesn't use caching, for the sake of simplicity.
+// If you're dealing with this many atoms, you'll usually want to cache them locally anyway.
+pub unsafe fn get_atoms(
+    xconn: &Arc<XConnection>,
+    names: &[*mut c_char],
+) -> Result<Vec<ffi::Atom>, XError> {
+    let mut atoms = Vec::with_capacity(names.len());
+    (xconn.xlib.XInternAtoms)(
+        xconn.display,
+        names.as_ptr() as *mut _,
+        names.len() as c_int,
+        ffi::False,
+        atoms.as_mut_ptr(),
+    );
+    xconn.check_errors()?;
+    atoms.set_len(names.len());
+    /*println!(
+        "XInternAtoms atoms:{:?}",
+        atoms,
+    );*/
+    Ok(atoms)
+}

src/platform/linux/x11/util/geometry.rs

@@ -0,0 +1,350 @@
+use super::*;
+
+#[derive(Debug)]
+pub struct TranslatedCoords {
+    pub x_rel_root: c_int,
+    pub y_rel_root: c_int,
+    pub child: ffi::Window,
+}
+
+// This is adequate for get_inner_position
+pub unsafe fn translate_coords(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+    root: ffi::Window,
+) -> Result<TranslatedCoords, XError> {
+    let mut translated_coords: TranslatedCoords = mem::uninitialized();
+
+    (xconn.xlib.XTranslateCoordinates)(
+        xconn.display,
+        window,
+        root,
+        0,
+        0,
+        &mut translated_coords.x_rel_root,
+        &mut translated_coords.y_rel_root,
+        &mut translated_coords.child,
+    );
+
+    //println!("XTranslateCoordinates coords:{:?}", translated_coords);
+
+    xconn.check_errors().map(|_| translated_coords)
+}
+
+#[derive(Debug)]
+pub struct Geometry {
+    pub root: ffi::Window,
+    // If you want positions relative to the root window, use translate_coords.
+    // Note that the overwhelming majority of window managers are reparenting WMs, thus the window
+    // ID we get from window creation is for a nested window used as the window's client area. If
+    // you call get_geometry with that window ID, then you'll get the position of that client area
+    // window relative to the parent it's nested in (the frame), which isn't helpful if you want
+    // to know the frame position.
+    pub x_rel_parent: c_int,
+    pub y_rel_parent: c_int,
+    // In that same case, this will give you client area size.
+    pub width: c_uint,
+    pub height: c_uint,
+    // xmonad and dwm were the only WMs tested that use the border return at all.
+    // The majority of WMs seem to simply fill it with 0 unconditionally.
+    pub border: c_uint,
+    pub depth: c_uint,
+}
+
+// This is adequate for get_inner_size
+pub unsafe fn get_geometry(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+) -> Result<Geometry, XError> {
+    let mut geometry: Geometry = mem::uninitialized();
+
+    let _status = (xconn.xlib.XGetGeometry)(
+        xconn.display,
+        window,
+        &mut geometry.root,
+        &mut geometry.x_rel_parent,
+        &mut geometry.y_rel_parent,
+        &mut geometry.width,
+        &mut geometry.height,
+        &mut geometry.border,
+        &mut geometry.depth,
+    );
+
+    //println!("XGetGeometry geo:{:?}", geometry);
+
+    xconn.check_errors().map(|_| geometry)
+}
+
+#[derive(Debug, Clone)]
+pub struct FrameExtents {
+    pub left: c_ulong,
+    pub right: c_ulong,
+    pub top: c_ulong,
+    pub bottom: c_ulong,
+}
+
+impl FrameExtents {
+    pub fn new(left: c_ulong, right: c_ulong, top: c_ulong, bottom: c_ulong) -> Self {
+        FrameExtents { left, right, top, bottom }
+    }
+
+    pub fn from_border(border: c_ulong) -> Self {
+        Self::new(border, border, border, border)
+    }
+}
+
+fn get_frame_extents(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+) -> Option<self::FrameExtents> {
+    let extents_atom = unsafe { self::get_atom(xconn, b"_NET_FRAME_EXTENTS\0") }
+        .expect("Failed to call XInternAtom (_NET_FRAME_EXTENTS)");
+
+    if !self::hint_is_supported(extents_atom) {
+        return None;
+    }
+
+    // Of the WMs tested, xmonad, i3, dwm, IceWM (1.3.x and earlier), and blackbox don't
+    // support this. As this is part of EWMH (Extended Window Manager Hints), it's likely to
+    // be unsupported by many smaller WMs.
+    let extents: Option<Vec<c_ulong>> = unsafe {
+        self::get_property(
+            xconn,
+            window,
+            extents_atom,
+            ffi::XA_CARDINAL,
+        )
+    }.ok();
+
+    extents.and_then(|extents| {
+        if extents.len() >= 4 {
+            Some(self::FrameExtents {
+                left: extents[0],
+                right: extents[1],
+                top: extents[2],
+                bottom: extents[3],
+            })
+        } else {
+            None
+        }
+    })
+}
+
+pub fn is_top_level(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+    root: ffi::Window,
+) -> Option<bool> {
+    let client_list_atom = unsafe { self::get_atom(xconn, b"_NET_CLIENT_LIST\0") }
+        .expect("Failed to call XInternAtom (_NET_CLIENT_LIST)");
+
+    if !self::hint_is_supported(client_list_atom) {
+        return None;
+    }
+
+    let client_list: Option<Vec<ffi::Window>> = unsafe {
+        self::get_property(
+            xconn,
+            root,
+            client_list_atom,
+            ffi::XA_WINDOW,
+        )
+    }.ok();
+
+    client_list.map(|client_list| client_list.contains(&window))
+}
+
+unsafe fn get_parent_window(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+) -> Result<ffi::Window, XError> {
+    let mut root: ffi::Window = mem::uninitialized();
+    let mut parent: ffi::Window = mem::uninitialized();
+    let mut children: *mut ffi::Window = ptr::null_mut();
+    let mut nchildren: c_uint = mem::uninitialized();
+
+    let _status = (xconn.xlib.XQueryTree)(
+        xconn.display,
+        window,
+        &mut root,
+        &mut parent,
+        &mut children,
+        &mut nchildren,
+    );
+
+    // The list of children isn't used
+    if children != ptr::null_mut() {
+        (xconn.xlib.XFree)(children as *mut _);
+    }
+
+    xconn.check_errors().map(|_| parent)
+}
+
+fn climb_hierarchy(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+    root: ffi::Window,
+) -> Result<ffi::Window, XError> {
+    let mut outer_window = window;
+    loop {
+        let candidate = unsafe { get_parent_window(xconn, outer_window) }?;
+        if candidate == root {
+            break;
+        }
+        outer_window = candidate;
+    }
+    Ok(outer_window)
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum FrameExtentsHeuristicPath {
+    Supported,
+    UnsupportedNested,
+    UnsupportedBordered,
+}
+
+#[derive(Debug, Clone)]
+pub struct FrameExtentsHeuristic {
+    pub frame_extents: FrameExtents,
+    pub heuristic_path: FrameExtentsHeuristicPath,
+}
+
+impl FrameExtentsHeuristic {
+    pub fn inner_pos_to_outer(&self, x: i32, y: i32) -> (i32, i32) {
+        use self::FrameExtentsHeuristicPath::*;
+        if self.heuristic_path != UnsupportedBordered {
+            (x - self.frame_extents.left as i32, y - self.frame_extents.top as i32)
+        } else {
+            (x, y)
+        }
+    }
+
+    pub fn inner_size_to_outer(&self, width: u32, height: u32) -> (u32, u32) {
+        (
+            width.saturating_add(
+                self.frame_extents.left.saturating_add(self.frame_extents.right) as u32
+            ),
+            height.saturating_add(
+                self.frame_extents.top.saturating_add(self.frame_extents.bottom) as u32
+            ),
+        )
+    }
+}
+
+pub fn get_frame_extents_heuristic(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+    root: ffi::Window,
+) -> FrameExtentsHeuristic {
+    use self::FrameExtentsHeuristicPath::*;
+
+    // Position relative to root window.
+    // With rare exceptions, this is the position of a nested window. Cases where the window
+    // isn't nested are outlined in the comments throghout this function, but in addition to
+    // that, fullscreen windows often aren't nested.
+    let (inner_y_rel_root, child) = {
+        let coords = unsafe { translate_coords(xconn, window, root) }
+            .expect("Failed to translate window coordinates");
+        (
+            coords.y_rel_root,
+            coords.child,
+        )
+    };
+
+    let (width, height, border) = {
+        let inner_geometry = unsafe { get_geometry(xconn, window) }
+            .expect("Failed to get inner window geometry");
+        (
+            inner_geometry.width,
+            inner_geometry.height,
+            inner_geometry.border,
+        )
+    };
+
+    // The first condition is only false for un-nested windows, but isn't always false for
+    // un-nested windows. Mutter/Muffin/Budgie and Marco present a mysterious discrepancy:
+    // when y is on the range [0, 2] and if the window has been unfocused since being
+    // undecorated (or was undecorated upon construction), the first condition is true,
+    // requiring us to rely on the second condition.
+    let nested = !(window == child || is_top_level(xconn, child, root) == Some(true));
+
+    // Hopefully the WM supports EWMH, allowing us to get exact info on the window frames.
+    if let Some(mut frame_extents) = get_frame_extents(xconn, window) {
+        // Mutter/Muffin/Budgie and Marco preserve their decorated frame extents when
+        // decorations are disabled, but since the window becomes un-nested, it's easy to
+        // catch.
+        if !nested {
+            frame_extents = FrameExtents::new(0, 0, 0, 0);
+        }
+
+        // The difference between the nested window's position and the outermost window's
+        // position is equivalent to the frame size. In most scenarios, this is equivalent to
+        // manually climbing the hierarchy as is done in the case below. Here's a list of
+        // known discrepancies:
+        // * Mutter/Muffin/Budgie gives decorated windows a margin of 9px (only 7px on top) in
+        //   addition to a 1px semi-transparent border. The margin can be easily observed by
+        //   using a screenshot tool to get a screenshot of a selected window, and is
+        //   presumably used for drawing drop shadows. Getting window geometry information
+        //   via hierarchy-climbing results in this margin being included in both the
+        //   position and outer size, so a window positioned at (0, 0) would be reported as
+        //   having a position (-10, -8).
+        // * Compiz has a drop shadow margin just like Mutter/Muffin/Budgie, though it's 10px
+        //   on all sides, and there's no additional border.
+        // * Enlightenment otherwise gets a y position equivalent to inner_y_rel_root.
+        //   Without decorations, there's no difference. This is presumably related to
+        //   Enlightenment's fairly unique concept of window position; it interprets
+        //   positions given to XMoveWindow as a client area position rather than a position
+        //   of the overall window.
+
+        FrameExtentsHeuristic {
+            frame_extents,
+            heuristic_path: Supported,
+        }
+    } else if nested {
+        // If the position value we have is for a nested window used as the client area, we'll
+        // just climb up the hierarchy and get the geometry of the outermost window we're
+        // nested in.
+        let outer_window = climb_hierarchy(xconn, window, root)
+            .expect("Failed to climb window hierarchy");
+
+        let (outer_y, outer_width, outer_height) = {
+            let outer_geometry = unsafe { get_geometry(xconn, outer_window) }
+                .expect("Failed to get outer window geometry");
+            (
+                outer_geometry.y_rel_parent,
+                outer_geometry.width,
+                outer_geometry.height,
+            )
+        };
+
+        // Since we have the geometry of the outermost window and the geometry of the client
+        // area, we can figure out what's in between.
+        let diff_x = outer_width.saturating_sub(width);
+        let diff_y = outer_height.saturating_sub(height);
+        let offset_y = inner_y_rel_root.saturating_sub(outer_y) as c_uint;
+
+        let left = diff_x / 2;
+        let right = left;
+        let top = offset_y;
+        let bottom = diff_y.saturating_sub(offset_y);
+
+        let frame_extents = FrameExtents::new(
+            left.into(),
+            right.into(),
+            top.into(),
+            bottom.into(),
+        );
+        FrameExtentsHeuristic {
+            frame_extents,
+            heuristic_path: UnsupportedNested,
+        }
+    } else {
+        // This is the case for xmonad and dwm, AKA the only WMs tested that supplied a
+        // border value. This is convenient, since we can use it to get an accurate frame.
+        let frame_extents = FrameExtents::from_border(border.into());
+        FrameExtentsHeuristic {
+            frame_extents,
+            heuristic_path: UnsupportedBordered,
+        }
+    }
+}

src/platform/linux/x11/util/hint.rs

@@ -0,0 +1,20 @@
+use super::*;
+
+pub const MWM_HINTS_DECORATIONS: c_ulong = 2;
+
+#[derive(Debug)]
+pub enum StateOperation {
+    Remove = 0, // _NET_WM_STATE_REMOVE
+    Add = 1,    // _NET_WM_STATE_ADD
+    _Toggle = 2, // _NET_WM_STATE_TOGGLE
+}
+
+impl From<bool> for StateOperation {
+    fn from(b: bool) -> Self {
+        if b {
+            StateOperation::Add
+        } else {
+            StateOperation::Remove
+        }
+    }
+}

src/platform/linux/x11/util/input.rs

@@ -0,0 +1,187 @@
+use super::*;
+use events::ModifiersState;
+
+pub unsafe fn select_xinput_events(
+    xconn: &Arc<XConnection>,
+    window: c_ulong,
+    device_id: c_int,
+    mask: i32,
+) -> Flusher {
+    let mut event_mask = ffi::XIEventMask {
+        deviceid: device_id,
+        mask: &mask as *const _ as *mut c_uchar,
+        mask_len: mem::size_of_val(&mask) as c_int,
+    };
+    (xconn.xinput2.XISelectEvents)(
+        xconn.display,
+        window,
+        &mut event_mask as *mut ffi::XIEventMask,
+        1, // number of masks to read from pointer above
+    );
+    Flusher::new(xconn)
+}
+
+#[allow(dead_code)]
+pub unsafe fn select_xkb_events(
+    xconn: &Arc<XConnection>,
+    device_id: c_uint,
+    mask: c_ulong,
+) -> Option<Flusher> {
+    let status = (xconn.xlib.XkbSelectEvents)(
+        xconn.display,
+        device_id,
+        mask,
+        mask,
+    );
+    if status == ffi::True {
+        Some(Flusher::new(xconn))
+    } else {
+        None
+    }
+}
+
+impl From<ffi::XIModifierState> for ModifiersState {
+    fn from(mods: ffi::XIModifierState) -> Self {
+        let state = mods.effective as c_uint;
+        ModifiersState {
+            alt: state & ffi::Mod1Mask != 0,
+            shift: state & ffi::ShiftMask != 0,
+            ctrl: state & ffi::ControlMask != 0,
+            logo: state & ffi::Mod4Mask != 0,
+        }
+    }
+}
+
+pub struct PointerState<'a> {
+    xconn: &'a Arc<XConnection>,
+    _root: ffi::Window,
+    _child: ffi::Window,
+    _root_x: c_double,
+    _root_y: c_double,
+    _win_x: c_double,
+    _win_y: c_double,
+    _buttons: ffi::XIButtonState,
+    modifiers: ffi::XIModifierState,
+    _group: ffi::XIGroupState,
+    _relative_to_window: bool,
+}
+
+impl<'a> PointerState<'a> {
+    pub fn get_modifier_state(&self) -> ModifiersState {
+        self.modifiers.into()
+    }
+}
+
+impl<'a> Drop for PointerState<'a> {
+    fn drop(&mut self) {
+        unsafe {
+            // This is why you need to read the docs carefully...
+            (self.xconn.xlib.XFree)(self._buttons.mask as _);
+        }
+    }
+}
+
+pub unsafe fn query_pointer(
+    xconn: &Arc<XConnection>,
+    window: ffi::Window,
+    device_id: c_int,
+) -> Result<PointerState, XError> {
+    let mut root_return = mem::uninitialized();
+    let mut child_return = mem::uninitialized();
+    let mut root_x_return = mem::uninitialized();
+    let mut root_y_return = mem::uninitialized();
+    let mut win_x_return = mem::uninitialized();
+    let mut win_y_return = mem::uninitialized();
+    let mut buttons_return = mem::uninitialized();
+    let mut modifiers_return = mem::uninitialized();
+    let mut group_return = mem::uninitialized();
+
+    let relative_to_window = (xconn.xinput2.XIQueryPointer)(
+        xconn.display,
+        device_id,
+        window,
+        &mut root_return,
+        &mut child_return,
+        &mut root_x_return,
+        &mut root_y_return,
+        &mut win_x_return,
+        &mut win_y_return,
+        &mut buttons_return,
+        &mut modifiers_return,
+        &mut group_return,
+    ) == ffi::True;
+
+    xconn.check_errors()?;
+
+    Ok(PointerState {
+        xconn,
+        _root: root_return,
+        _child: child_return,
+        _root_x: root_x_return,
+        _root_y: root_y_return,
+        _win_x: win_x_return,
+        _win_y: win_y_return,
+        _buttons: buttons_return,
+        modifiers: modifiers_return,
+        _group: group_return,
+        _relative_to_window: relative_to_window,
+    })
+}
+
+unsafe fn lookup_utf8_inner(
+    xconn: &Arc<XConnection>,
+    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)
+}
+
+// A base buffer size of 1kB uses a negligible amount of RAM while preventing us from having to
+// re-allocate (and make another round-trip) in the *vast* majority of cases.
+// To test if lookup_utf8 works correctly, set this to 1.
+const TEXT_BUFFER_SIZE: usize = 1024;
+
+pub unsafe fn lookup_utf8(
+    xconn: &Arc<XConnection>,
+    ic: ffi::XIC,
+    key_event: &mut ffi::XKeyEvent,
+) -> String {
+    let mut buffer: [u8; TEXT_BUFFER_SIZE] = mem::uninitialized();
+    let (_, status, count) = lookup_utf8_inner(
+        xconn,
+        ic,
+        key_event,
+        &mut buffer,
+    );
+
+    // The buffer overflowed, so we'll make a new one on the heap.
+    if status == ffi::XBufferOverflow {
+        let mut buffer = Vec::with_capacity(count as usize);
+        buffer.set_len(count as usize);
+        let (_, _, new_count) = lookup_utf8_inner(
+            xconn,
+            ic,
+            key_event,
+            &mut buffer,
+        );
+        debug_assert_eq!(count, new_count);
+        str::from_utf8(&buffer[..count as usize])
+            .unwrap_or("")
+            .to_string()
+    } else {
+        str::from_utf8(&buffer[..count as usize])
+            .unwrap_or("")
+            .to_string()
+    }
+}

src/platform/linux/x11/util/mod.rs

@@ -0,0 +1,177 @@
+// Welcome to the util module, where we try to keep you from shooting yourself in the foot.
+// *results may vary
+
+mod atom;
+mod geometry;
+mod hint;
+mod input;
+mod window_property;
+mod wm;
+
+pub use self::atom::*;
+pub use self::geometry::*;
+pub use self::hint::*;
+pub use self::input::*;
+pub use self::window_property::*;
+pub use self::wm::*;
+
+use std::mem;
+use std::ptr;
+use std::str;
+use std::sync::Arc;
+use std::ops::{Deref, DerefMut};
+use std::os::raw::*;
+
+use super::{ffi, XConnection, XError};
+
+// This isn't actually the number of the bits in the format.
+// X11 does a match on this value to determine which type to call sizeof on.
+// Thus, we use 32 for c_long, since 32 maps to c_long which maps to 64.
+// ...if that sounds confusing, then you know why this enum is here.
+#[derive(Debug, Copy, Clone)]
+pub enum Format {
+    Char = 8,
+    Short = 16,
+    Long = 32,
+}
+
+impl Format {
+    pub fn from_format(format: usize) -> Option<Self> {
+        match format {
+            8 => Some(Format::Char),
+            16 => Some(Format::Short),
+            32 => Some(Format::Long),
+            _ => None,
+        }
+    }
+
+    pub fn is_same_size_as<T>(&self) -> bool {
+        mem::size_of::<T>() == self.get_actual_size()
+    }
+
+    pub fn get_actual_size(&self) -> usize {
+        match self {
+            &Format::Char => mem::size_of::<c_char>(),
+            &Format::Short => mem::size_of::<c_short>(),
+            &Format::Long => mem::size_of::<c_long>(),
+        }
+    }
+}
+
+pub struct XSmartPointer<'a, T> {
+    xconn: &'a Arc<XConnection>,
+    pub ptr: *mut T,
+}
+
+impl<'a, T> XSmartPointer<'a, T> {
+    // You're responsible for only passing things to this that should be XFree'd.
+    // Returns None if ptr is null.
+    pub fn new(xconn: &'a Arc<XConnection>, ptr: *mut T) -> Option<Self> {
+        if !ptr.is_null() {
+            Some(XSmartPointer {
+                xconn,
+                ptr,
+            })
+        } else {
+            None
+        }
+    }
+}
+
+impl<'a, T> Deref for XSmartPointer<'a, T> {
+    type Target = T;
+
+    fn deref(&self) -> &T {
+        unsafe { &*self.ptr }
+    }
+}
+
+impl<'a, T> DerefMut for XSmartPointer<'a, T> {
+    fn deref_mut(&mut self) -> &mut T {
+        unsafe { &mut *self.ptr }
+    }
+}
+
+impl<'a, T> Drop for XSmartPointer<'a, T> {
+    fn drop(&mut self) {
+        unsafe {
+            (self.xconn.xlib.XFree)(self.ptr as *mut _);
+        }
+    }
+}
+
+// This is impoartant, so pay attention!
+// Xlib has an output buffer, and tries to hide the async nature of X from you.
+// This buffer contains the requests you make, and is flushed under various circumstances:
+// 1. XPending, XNextEvent, and XWindowEvent flush "as needed"
+// 2. XFlush explicitly flushes
+// 3. XSync flushes and blocks until all requests are responded to
+// 4. Calls that have a return dependent on a response (i.e. XGetWindowProperty) sync internally.
+//    When in doubt, check the X11 source; if a function calls _XReply, it flushes and waits.
+// All util functions that abstract an async function will return a Flusher.
+pub unsafe fn flush_requests(xconn: &Arc<XConnection>) -> Result<(), XError> {
+    (xconn.xlib.XFlush)(xconn.display);
+    //println!("XFlush");
+    // This isn't necessarily a useful time to check for errors (since our request hasn't
+    // necessarily been processed yet)
+    xconn.check_errors()
+}
+
+pub unsafe fn sync_with_server(xconn: &Arc<XConnection>) -> Result<(), XError> {
+    (xconn.xlib.XSync)(xconn.display, ffi::False);
+    //println!("XSync");
+    xconn.check_errors()
+}
+
+#[must_use = "This request was made asynchronously, and is still in the output buffer. You must explicitly choose to either `.flush()` (empty the output buffer, sending the request now) or `.queue()` (wait to send the request, allowing you to continue to add more requests without additional round-trips). For more information, see the documentation for `util::flush_requests`."]
+pub struct Flusher<'a> {
+    xconn: &'a Arc<XConnection>,
+}
+
+impl<'a> Flusher<'a> {
+    pub fn new(xconn: &'a Arc<XConnection>) -> Self {
+        Flusher { xconn }
+    }
+
+    // "I want this request sent now!"
+    pub fn flush(self) -> Result<(), XError> {
+        unsafe { flush_requests(self.xconn) }
+    }
+
+    // "I'm aware that this request hasn't been sent, and I'm okay with waiting."
+    pub fn queue(self) {}
+}
+
+pub unsafe fn send_client_msg(
+    xconn: &Arc<XConnection>,
+    window: c_ulong,        // The window this is "about"; not necessarily this window
+    target_window: c_ulong, // The window we're sending to
+    message_type: ffi::Atom,
+    event_mask: Option<c_long>,
+    data: (c_long, c_long, c_long, c_long, c_long),
+) -> Flusher {
+    let mut event: ffi::XClientMessageEvent = mem::uninitialized();
+    event.type_ = ffi::ClientMessage;
+    event.display = xconn.display;
+    event.window = window;
+    event.message_type = message_type;
+    event.format = Format::Long as c_int;
+    event.data = ffi::ClientMessageData::new();
+    event.data.set_long(0, data.0);
+    event.data.set_long(1, data.1);
+    event.data.set_long(2, data.2);
+    event.data.set_long(3, data.3);
+    event.data.set_long(4, data.4);
+
+    let event_mask = event_mask.unwrap_or(ffi::NoEventMask);
+
+    (xconn.xlib.XSendEvent)(
+        xconn.display,
+        target_window,
+        ffi::False,
+        event_mask,
+        &mut event.into(),
+    );
+
+    Flusher::new(xconn)
+}

src/platform/linux/x11/util/window_property.rs

@@ -0,0 +1,160 @@
+use std;
+use std::fmt::Debug;
+
+use super::*;
+
+#[derive(Debug, Clone)]
+pub enum GetPropertyError {
+    XError(XError),
+    TypeMismatch(ffi::Atom),
+    FormatMismatch(c_int),
+    NothingAllocated,
+}
+
+impl GetPropertyError {
+    pub fn is_actual_property_type(&self, t: ffi::Atom) -> bool {
+        if let GetPropertyError::TypeMismatch(actual_type) = *self {
+            actual_type == t
+        } else {
+            false
+        }
+    }
+}
+
+// Number of 32-bit chunks to retrieve per iteration of get_property's inner loop.
+// To test if get_property works correctly, set this to 1.
+const PROPERTY_BUFFER_SIZE: c_long = 1024; // 4k of RAM ought to be enough for anyone!
+
+pub unsafe fn get_property<T: Debug + Clone>(
+    xconn: &Arc<XConnection>,
+    window: c_ulong,
+    property: ffi::Atom,
+    property_type: ffi::Atom,
+) -> Result<Vec<T>, GetPropertyError> {
+    let mut data = Vec::new();
+    let mut offset = 0;
+
+    let mut done = false;
+    while !done {
+        let mut actual_type: ffi::Atom = mem::uninitialized();
+        let mut actual_format: c_int = mem::uninitialized();
+        let mut quantity_returned: c_ulong = mem::uninitialized();
+        let mut bytes_after: c_ulong = mem::uninitialized();
+        let mut buf: *mut c_uchar = ptr::null_mut();
+        (xconn.xlib.XGetWindowProperty)(
+            xconn.display,
+            window,
+            property,
+            // This offset is in terms of 32-bit chunks.
+            offset,
+            // This is the quanity of 32-bit chunks to receive at once.
+            PROPERTY_BUFFER_SIZE,
+            ffi::False,
+            property_type,
+            &mut actual_type,
+            &mut actual_format,
+            // This is the quantity of items we retrieved in our format, NOT of 32-bit chunks!
+            &mut quantity_returned,
+            // ...and this is a quantity of bytes. So, this function deals in 3 different units.
+            &mut bytes_after,
+            &mut buf,
+        );
+
+        if let Err(e) = xconn.check_errors() {
+            return Err(GetPropertyError::XError(e));
+        }
+
+        if actual_type != property_type {
+            return Err(GetPropertyError::TypeMismatch(actual_type));
+        }
+
+        let format_mismatch = Format::from_format(actual_format as _)
+            .map(|actual_format| !actual_format.is_same_size_as::<T>())
+            // This won't actually be reached; the XError condition above is triggered first.
+            .unwrap_or(true);
+
+        if format_mismatch {
+            return Err(GetPropertyError::FormatMismatch(actual_format));
+        }
+
+        if !buf.is_null() {
+            offset += PROPERTY_BUFFER_SIZE;
+            let new_data = std::slice::from_raw_parts(
+                buf as *mut T,
+                quantity_returned as usize,
+            );
+            /*println!(
+                "XGetWindowProperty prop:{:?} fmt:{:02} len:{:02} off:{:02} out:{:02}, buf:{:?}",
+                property,
+                mem::size_of::<T>() * 8,
+                data.len(),
+                offset,
+                quantity_returned,
+                new_data,
+            );*/
+            data.extend_from_slice(&new_data);
+            // Fun fact: XGetWindowProperty allocates one extra byte at the end.
+            (xconn.xlib.XFree)(buf as _); // Don't try to access new_data after this.
+        } else {
+            return Err(GetPropertyError::NothingAllocated);
+        }
+
+        done = bytes_after == 0;
+    }
+
+    Ok(data)
+}
+
+#[derive(Debug)]
+pub enum PropMode {
+    Replace = ffi::PropModeReplace as isize,
+    _Prepend = ffi::PropModePrepend as isize,
+    _Append = ffi::PropModeAppend as isize,
+}
+
+#[derive(Debug, Clone)]
+pub struct InvalidFormat {
+    format_used: Format,
+    size_passed: usize,
+    size_expected: usize,
+}
+
+pub unsafe fn change_property<'a, T: Debug>(
+    xconn: &'a Arc<XConnection>,
+    window: c_ulong,
+    property: ffi::Atom,
+    property_type: ffi::Atom,
+    format: Format,
+    mode: PropMode,
+    new_value: &[T],
+) -> Flusher<'a> {
+    if !format.is_same_size_as::<T>() {
+        panic!(format!(
+            "[winit developer error] Incorrect usage of `util::change_property`: {:#?}",
+            InvalidFormat {
+                format_used: format,
+                size_passed: mem::size_of::<T>() * 8,
+                size_expected: format.get_actual_size() * 8,
+            },
+        ));
+    }
+
+    (xconn.xlib.XChangeProperty)(
+        xconn.display,
+        window,
+        property,
+        property_type,
+        format as c_int,
+        mode as c_int,
+        new_value.as_ptr() as *const c_uchar,
+        new_value.len() as c_int,
+    );
+
+    /*println!(
+        "XChangeProperty prop:{:?} val:{:?}",
+        property,
+        new_value,
+    );*/
+
+    Flusher::new(xconn)
+}

src/platform/linux/x11/util/wm.rs

@@ -0,0 +1,158 @@
+use parking_lot::Mutex;
+
+use super::*;
+
+// This info is global to the window manager.
+lazy_static! {
+    static ref SUPPORTED_HINTS: Mutex<Vec<ffi::Atom>> = Mutex::new(Vec::with_capacity(0));
+    static ref WM_NAME: Mutex<Option<String>> = Mutex::new(None);
+}
+
+pub fn hint_is_supported(hint: ffi::Atom) -> bool {
+    (*SUPPORTED_HINTS.lock()).contains(&hint)
+}
+
+pub fn wm_name_is_one_of(names: &[&str]) -> bool {
+    if let Some(ref name) = *WM_NAME.lock() {
+        names.contains(&name.as_str())
+    } else {
+        false
+    }
+}
+
+pub fn update_cached_wm_info(xconn: &Arc<XConnection>, root: ffi::Window) {
+    *SUPPORTED_HINTS.lock() = self::get_supported_hints(xconn, root);
+    *WM_NAME.lock() = self::get_wm_name(xconn, root);
+}
+
+fn get_supported_hints(xconn: &Arc<XConnection>, root: ffi::Window) -> Vec<ffi::Atom> {
+    let supported_atom = unsafe { self::get_atom(xconn, b"_NET_SUPPORTED\0") }
+        .expect("Failed to call XInternAtom (_NET_SUPPORTED)");
+    unsafe {
+        self::get_property(
+            xconn,
+            root,
+            supported_atom,
+            ffi::XA_ATOM,
+        )
+    }.unwrap_or_else(|_| Vec::with_capacity(0))
+}
+
+fn get_wm_name(xconn: &Arc<XConnection>, root: ffi::Window) -> Option<String> {
+    let check_atom = unsafe { self::get_atom(xconn, b"_NET_SUPPORTING_WM_CHECK\0") }
+        .expect("Failed to call XInternAtom (_NET_SUPPORTING_WM_CHECK)");
+    let wm_name_atom = unsafe { self::get_atom(xconn, b"_NET_WM_NAME\0") }
+        .expect("Failed to call XInternAtom (_NET_WM_NAME)");
+
+    // Mutter/Muffin/Budgie doesn't have _NET_SUPPORTING_WM_CHECK in its _NET_SUPPORTED, despite
+    // it working and being supported. This has been reported upstream, but due to the
+    // inavailability of time machines, we'll just try to get _NET_SUPPORTING_WM_CHECK
+    // regardless of whether or not the WM claims to support it.
+    //
+    // Blackbox 0.70 also incorrectly reports not supporting this, though that appears to be fixed
+    // in 0.72.
+    /*if !supported_hints.contains(&check_atom) {
+        return None;
+    }*/
+
+    // IceWM (1.3.x and earlier) doesn't report supporting _NET_WM_NAME, but will nonetheless
+    // provide us with a value for it. Note that the unofficial 1.4 fork of IceWM works fine.
+    /*if !supported_hints.contains(&wm_name_atom) {
+        return None;
+    }*/
+
+    // Of the WMs tested, only xmonad and dwm fail to provide a WM name.
+
+    // Querying this property on the root window will give us the ID of a child window created by
+    // the WM.
+    let root_window_wm_check = {
+        let result = unsafe {
+            self::get_property(
+                xconn,
+                root,
+                check_atom,
+                ffi::XA_WINDOW,
+            )
+        };
+
+        let wm_check = result
+            .ok()
+            .and_then(|wm_check| wm_check.get(0).cloned());
+
+        if let Some(wm_check) = wm_check {
+            wm_check
+        } else {
+            return None;
+        }
+    };
+
+    // Querying the same property on the child window we were given, we should get this child
+    // window's ID again.
+    let child_window_wm_check = {
+        let result = unsafe {
+            self::get_property(
+                xconn,
+                root_window_wm_check,
+                check_atom,
+                ffi::XA_WINDOW,
+            )
+        };
+
+        let wm_check = result
+            .ok()
+            .and_then(|wm_check| wm_check.get(0).cloned());
+
+        if let Some(wm_check) = wm_check {
+            wm_check
+        } else {
+            return None;
+        }
+    };
+
+    // These values should be the same.
+    if root_window_wm_check != child_window_wm_check {
+        return None;
+    }
+
+    // All of that work gives us a window ID that we can get the WM name from.
+    let wm_name = {
+        let utf8_string_atom = unsafe { self::get_atom(xconn, b"UTF8_STRING\0") }
+            .expect("Failed to call XInternAtom (UTF8_STRING)");
+
+        let result = unsafe {
+            self::get_property(
+                xconn,
+                root_window_wm_check,
+                wm_name_atom,
+                utf8_string_atom,
+            )
+        };
+
+        // IceWM requires this. IceWM was also the only WM tested that returns a null-terminated
+        // string. For more fun trivia, IceWM is also unique in including version and uname
+        // information in this string (this means you'll have to be careful if you want to match
+        // against it, though).
+        // The unofficial 1.4 fork of IceWM still includes the extra details, but properly
+        // returns a UTF8 string that isn't null-terminated.
+        let no_utf8 = if let Err(ref err) = result {
+            err.is_actual_property_type(ffi::XA_STRING)
+        } else {
+            false
+        };
+
+        if no_utf8 {
+            unsafe {
+                self::get_property(
+                    xconn,
+                    root_window_wm_check,
+                    wm_name_atom,
+                    ffi::XA_STRING,
+                )
+            }
+        } else {
+            result
+        }
+    }.ok();
+
+    wm_name.and_then(|wm_name| String::from_utf8(wm_name).ok())
+}

src/platform/linux/x11/xdisplay.rs

@@ -1,9 +1,9 @@
 use std::ptr;
 use std::fmt;
 use std::error::Error;
-use std::sync::Mutex;
 
 use libc;
+use parking_lot::Mutex;
 
 use super::ffi;
 
@@ -29,12 +29,12 @@ pub type XErrorHandler = Option<unsafe extern fn(*mut ffi::Display, *mut ffi::XE
 impl XConnection {
     pub fn new(error_handler: XErrorHandler) -> Result<XConnection, XNotSupported> {
         // opening the libraries
-        let xlib = try!(ffi::Xlib::open());
-        let xcursor = try!(ffi::Xcursor::open());
-        let xrandr = try!(ffi::Xrandr_2_2_0::open());
+        let xlib = ffi::Xlib::open()?;
+        let xcursor = ffi::Xcursor::open()?;
+        let xrandr = ffi::Xrandr_2_2_0::open()?;
         let xrandr_1_5 = ffi::Xrandr::open().ok();
-        let xinput2 = try!(ffi::XInput2::open());
-        let xlib_xcb = try!(ffi::Xlib_xcb::open());
+        let xinput2 = ffi::XInput2::open()?;
+        let xlib_xcb = ffi::Xlib_xcb::open()?;
 
         unsafe { (xlib.XInitThreads)() };
         unsafe { (xlib.XSetErrorHandler)(error_handler) };
@@ -49,13 +49,13 @@ impl XConnection {
         };
 
         Ok(XConnection {
-            xlib: xlib,
-            xrandr: xrandr,
-            xrandr_1_5: xrandr_1_5,
-            xcursor: xcursor,
-            xinput2: xinput2,
-            xlib_xcb: xlib_xcb,
-            display: display,
+            xlib,
+            xrandr,
+            xrandr_1_5,
+            xcursor,
+            xinput2,
+            xlib_xcb,
+            display,
             latest_error: Mutex::new(None),
         })
     }
@@ -63,8 +63,7 @@ impl XConnection {
     /// Checks whether an error has been triggered by the previous function calls.
     #[inline]
     pub fn check_errors(&self) -> Result<(), XError> {
-        let error = self.latest_error.lock().unwrap().take();
-
+        let error = self.latest_error.lock().take();
         if let Some(error) = error {
             Err(error)
         } else {
@@ -75,7 +74,13 @@ impl XConnection {
     /// Ignores any previous error.
     #[inline]
     pub fn ignore_error(&self) {
-        *self.latest_error.lock().unwrap() = None;
+        *self.latest_error.lock() = None;
+    }
+}
+
+impl fmt::Debug for XConnection {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.display.fmt(f)
     }
 }