From 8f18dab06108592b04bda260be30b9b912a67d43 Mon Sep 17 00:00:00 2001 From: Francesca Sunshine <2096580+francesca64@users.noreply.github.com> Date: Fri, 15 Dec 2017 14:37:09 -0500 Subject: [PATCH] x11: Send window maximization hints correctly (#363) Previously, the maximization hints were being sent as two separate client messages: one for horizontal, and one for vertical. That resulted in the window only being maximized horizontally (at least with my WM). The corrected client message sets both of these hints at once. In the process of implementing that, the relevant components were refactored to use the util module, as we gradually move towards a hopeful future of a more readable X11 backend. --- src/platform/linux/x11/dnd.rs | 16 +++-- src/platform/linux/x11/mod.rs | 12 ++-- src/platform/linux/x11/util.rs | 20 ++++-- src/platform/linux/x11/window.rs | 117 ++++++++++++++++++------------- 4 files changed, 105 insertions(+), 60 deletions(-) diff --git a/src/platform/linux/x11/dnd.rs b/src/platform/linux/x11/dnd.rs index 59278873..b35c1b18 100644 --- a/src/platform/linux/x11/dnd.rs +++ b/src/platform/linux/x11/dnd.rs @@ -55,7 +55,9 @@ impl DndAtoms { ); } xconn.check_errors()?; - unsafe { atoms.set_len(DND_ATOMS_LEN); } + unsafe { + atoms.set_len(DND_ATOMS_LEN); + } Ok(DndAtoms { aware: atoms[0], enter: atoms[1], @@ -137,7 +139,7 @@ impl Dnd { this_window: c_ulong, target_window: c_ulong, state: DndState, - ) { + ) -> Result<(), XError> { let (accepted, action) = match state { DndState::Accepted => (1, self.atoms.action_private as c_long), DndState::Rejected => (0, self.atoms.none as c_long), @@ -145,9 +147,11 @@ impl Dnd { util::send_client_msg( &self.xconn, target_window, + target_window, self.atoms.status, + None, (this_window as c_long, accepted, 0, 0, action), - ); + ) } pub unsafe fn send_finished( @@ -155,7 +159,7 @@ impl Dnd { this_window: c_ulong, target_window: c_ulong, state: DndState, - ) { + ) -> Result<(), XError> { let (accepted, action) = match state { DndState::Accepted => (1, self.atoms.action_private as c_long), DndState::Rejected => (0, self.atoms.none as c_long), @@ -163,9 +167,11 @@ impl Dnd { util::send_client_msg( &self.xconn, target_window, + target_window, self.atoms.finished, + None, (this_window as c_long, accepted, action, 0, 0), - ); + ) } pub unsafe fn get_type_list( diff --git a/src/platform/linux/x11/mod.rs b/src/platform/linux/x11/mod.rs index 2d73add2..80833432 100644 --- a/src/platform/linux/x11/mod.rs +++ b/src/platform/linux/x11/mod.rs @@ -276,12 +276,15 @@ impl EventsLoop { // This results in the SelectionNotify event below self.dnd.convert_selection(xwindow, time); } - self.dnd.send_status(xwindow, source_window, DndState::Accepted); + self.dnd.send_status(xwindow, source_window, DndState::Accepted) + .expect("Failed to send XDnD status message."); } } else { unsafe { - self.dnd.send_status(xwindow, source_window, DndState::Rejected); - self.dnd.send_finished(xwindow, source_window, DndState::Rejected); + self.dnd.send_status(xwindow, source_window, DndState::Rejected) + .expect("Failed to send XDnD status message."); + self.dnd.send_finished(xwindow, source_window, DndState::Rejected) + .expect("Failed to send XDnD finished message."); } self.dnd.reset(); } @@ -296,7 +299,8 @@ impl EventsLoop { } } unsafe { - self.dnd.send_finished(xwindow, source_window, DndState::Accepted); + self.dnd.send_finished(xwindow, source_window, DndState::Accepted) + .expect("Failed to send XDnD finished message."); } } self.dnd.reset(); diff --git a/src/platform/linux/x11/util.rs b/src/platform/linux/x11/util.rs index 0969b3e5..45dbf255 100644 --- a/src/platform/linux/x11/util.rs +++ b/src/platform/linux/x11/util.rs @@ -6,16 +6,24 @@ use libc::{c_char, c_int, c_long, c_short, c_uchar, c_ulong}; use super::{ffi, XConnection, XError}; +pub unsafe fn get_atom(xconn: &Arc, name: &[u8]) -> Result { + 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, - target_window: c_ulong, + 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, 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 = target_window; + event.window = window; event.message_type = message_type; event.format = 32; event.data = ffi::ClientMessageData::new(); @@ -25,13 +33,17 @@ pub unsafe fn send_client_msg( 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, - ffi::NoEventMask, + event_mask, &mut event.into(), ); + + xconn.check_errors().map(|_| ()) } #[derive(Debug)] diff --git a/src/platform/linux/x11/window.rs b/src/platform/linux/x11/window.rs index 1caa6d4d..fbe13cb7 100644 --- a/src/platform/linux/x11/window.rs +++ b/src/platform/linux/x11/window.rs @@ -19,8 +19,7 @@ use window::MonitorId as RootMonitorId; use platform::x11::monitor::get_available_monitors; -use super::{ffi}; -use super::{XConnection, WindowId, EventsLoop}; +use super::{ffi, util, XConnection, WindowId, EventsLoop}; // TODO: remove me fn with_c_str(s: &str, f: F) -> T where F: FnOnce(*const libc::c_char) -> T { @@ -29,6 +28,24 @@ fn with_c_str(s: &str, f: F) -> T where F: FnOnce(*const libc::c_char) -> f(c_str.as_ptr()) } +#[derive(Debug)] +enum StateOperation { + Remove = 0, // _NET_WM_STATE_REMOVE + Add = 1, // _NET_WM_STATE_ADD + #[allow(dead_code)] + Toggle = 2, // _NET_WM_STATE_TOGGLE +} + +impl From for StateOperation { + fn from(b: bool) -> Self { + if b { + StateOperation::Add + } else { + StateOperation::Remove + } + } +} + pub struct XWindow { display: Arc, window: ffi::Window, @@ -145,8 +162,8 @@ impl Window2 { // Enable drag and drop unsafe { - let atom_name: *const libc::c_char = b"XdndAware\0".as_ptr() as _; - let atom = (display.xlib.XInternAtom)(display.display, atom_name, ffi::False); + let atom = util::get_atom(display, b"XdndAware\0") + .expect("Failed to call XInternAtom (XdndAware)"); let version = &5; // Latest version; hasn't changed since 2002 (display.xlib.XChangeProperty)( display.display, @@ -275,49 +292,32 @@ impl Window2 { Ok(window) } - fn set_netwm(display: &Arc, window: ffi::Window, root: ffi::Window, property: &str, val: bool) { - let state_atom = unsafe { - with_c_str("_NET_WM_STATE", |state| - (display.xlib.XInternAtom)(display.display, state, 0) - ) - }; - display.check_errors().expect("Failed to call XInternAtom"); - let atom = unsafe { - with_c_str(property, |state| - (display.xlib.XInternAtom)(display.display, state, 0) - ) - }; - display.check_errors().expect("Failed to call XInternAtom"); - - let client_message_event = ffi::XClientMessageEvent { - type_: ffi::ClientMessage, - serial: 0, - send_event: 1, // true because we are sending this through `XSendEvent` - display: display.display, - window: window, - message_type: state_atom, // the _NET_WM_STATE atom is sent to change the state of a window - format: 32, // view `data` as `c_long`s - data: { - let mut data = ffi::ClientMessageData::new(); - // This first `long` is the action; `1` means add/set following property. - data.set_long(0, val as c_long); - // This second `long` is the property to set (fullscreen) - data.set_long(1, atom as c_long); - data - } - }; - let mut x_event = ffi::XEvent::from(client_message_event); + fn set_netwm( + xconn: &Arc, + window: ffi::Window, + root: ffi::Window, + properties: (c_long, c_long, c_long, c_long), + operation: StateOperation + ) { + let state_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE\0") } + .expect("Failed to call XInternAtom (_NET_WM_STATE)"); unsafe { - (display.xlib.XSendEvent)( - display.display, + util::send_client_msg( + xconn, + window, root, - 0, - ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask, - &mut x_event as *mut _ - ); - display.check_errors().expect("Failed to call XSendEvent"); - } + state_atom, + Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask), + ( + operation as c_long, + properties.0, + properties.1, + properties.2, + properties.3, + ) + ) + }.expect("Failed to send NET_WM hint."); } pub fn set_fullscreen(&self, monitor: Option) { @@ -374,12 +374,35 @@ impl Window2 { } pub fn set_maximized(&self, maximized: bool) { - Window2::set_netwm(&self.x.display, self.x.window, self.x.root, "_NET_WM_STATE_MAXIMIZED_HORZ", maximized); - Window2::set_netwm(&self.x.display, self.x.window, self.x.root, "_NET_WM_STATE_MAXIMIZED_VERT", maximized); + let xconn = &self.x.display; + + let horz_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_MAXIMIZED_HORZ\0") } + .expect("Failed to call XInternAtom (_NET_WM_STATE_MAXIMIZED_HORZ)"); + let vert_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_MAXIMIZED_VERT\0") } + .expect("Failed to call XInternAtom (_NET_WM_STATE_MAXIMIZED_VERT)"); + + Window2::set_netwm( + xconn, + self.x.window, + self.x.root, + (horz_atom as c_long, vert_atom as c_long, 0, 0), + maximized.into() + ); } fn set_fullscreen_hint(&self, fullscreen: bool) { - Window2::set_netwm(&self.x.display, self.x.window, self.x.root, "_NET_WM_STATE_FULLSCREEN", fullscreen); + let xconn = &self.x.display; + + let fullscreen_atom = unsafe { util::get_atom(xconn, b"_NET_WM_STATE_FULLSCREEN\0") } + .expect("Failed to call XInternAtom (_NET_WM_STATE_FULLSCREEN)"); + + Window2::set_netwm( + xconn, + self.x.window, + self.x.root, + (fullscreen_atom as c_long, 0, 0, 0), + fullscreen.into() + ); } pub fn set_title(&self, title: &str) {