From 869c639ef8f2387e9b7fa7df68e2dbe22a9a6e2a Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 9 Jun 2020 15:21:05 -0500 Subject: [PATCH 1/5] Port X11 backend from xcb to x11rb. --- druid-shell/Cargo.toml | 6 +- druid-shell/src/platform/x11/application.rs | 182 +++++------ druid-shell/src/platform/x11/mod.rs | 5 +- druid-shell/src/platform/x11/util.rs | 55 +++- druid-shell/src/platform/x11/window.rs | 338 +++++++++++--------- 5 files changed, 310 insertions(+), 276 deletions(-) diff --git a/druid-shell/Cargo.toml b/druid-shell/Cargo.toml index 98034a3e28..49b528d904 100644 --- a/druid-shell/Cargo.toml +++ b/druid-shell/Cargo.toml @@ -14,7 +14,7 @@ rustdoc-args = ["--cfg", "docsrs"] default-target = "x86_64-pc-windows-msvc" [features] -x11 = ["xcb", "cairo-sys-rs"] +x11 = ["x11rb", "cairo-sys-rs"] [dependencies] # NOTE: When changing the piet or kurbo versions, ensure that @@ -39,7 +39,7 @@ gtk = { version = "0.8.1", optional = true } glib = { version = "0.9.3", optional = true } glib-sys = { version = "0.9.1", optional = true } gtk-sys = { version = "0.9.2", optional = true } -xcb = { version = "0.9.0", features = ["thread", "xlib_xcb", "randr"], optional = true } +x11rb = { version = "0.5.0", features = ["allow-unsafe-code", "randr"], optional = true } [target.'cfg(target_os="windows")'.dependencies] wio = "0.2.2" @@ -57,7 +57,7 @@ core-graphics = "0.19.0" foreign-types = "0.3.2" bitflags = "1.2.1" -# TODO(x11/dependencies): only use feature "xcb" if using XCB +# TODO(x11/dependencies): only use feature "xcb" if using X11 [target.'cfg(target_os="linux")'.dependencies] cairo-rs = { version = "0.8.1", default_features = false, features = ["xcb"] } gio = "0.8.1" diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index dd9ac1a919..e0a961e058 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -20,12 +20,10 @@ use std::convert::TryInto; use std::rc::Rc; use anyhow::{anyhow, Context, Error}; -use xcb::{ - ButtonPressEvent, ButtonReleaseEvent, ClientMessageEvent, Connection, DestroyNotifyEvent, - ExposeEvent, KeyPressEvent, MotionNotifyEvent, BUTTON_PRESS, BUTTON_RELEASE, CLIENT_MESSAGE, - COPY_FROM_PARENT, CW_EVENT_MASK, DESTROY_NOTIFY, EVENT_MASK_STRUCTURE_NOTIFY, EXPOSE, - KEY_PRESS, MOTION_NOTIFY, WINDOW_CLASS_INPUT_ONLY, -}; +use x11rb::connection::Connection; +use x11rb::protocol::xproto::{ConnectionExt, CreateWindowAux, EventMask, WindowClass}; +use x11rb::protocol::Event; +use x11rb::xcb_ffi::XCBConnection; use crate::application::AppHandler; @@ -40,7 +38,7 @@ pub(crate) struct Application { /// The X server might also host other displays. /// /// A display is a collection of screens. - connection: Rc, + connection: Rc, /// The default screen of the connected display. /// /// The connected display may also have additional screens. @@ -74,59 +72,59 @@ struct State { impl Application { pub fn new() -> Result { - let (conn, screen_num) = Connection::connect_with_xlib_display()?; + let (conn, screen_num) = XCBConnection::connect(None)?; let connection = Rc::new(conn); - let window_id = Application::create_event_window(&connection, screen_num)?; + let window_id = Application::create_event_window(&connection, screen_num as i32)?; let state = Rc::new(RefCell::new(State { quitting: false, windows: HashMap::new(), })); Ok(Application { connection, - screen_num, + screen_num: screen_num as i32, window_id, state, }) } - fn create_event_window(conn: &Rc, screen_num: i32) -> Result { - let id = conn.generate_id(); - let setup = conn.get_setup(); + fn create_event_window(conn: &Rc, screen_num: i32) -> Result { + let id = conn.generate_id()?; + let setup = conn.setup(); let screen = setup - .roots() - .nth(screen_num as usize) + .roots + .get(screen_num as usize) .ok_or_else(|| anyhow!("invalid screen num: {}", screen_num))?; - let cw_values = [(CW_EVENT_MASK, EVENT_MASK_STRUCTURE_NOTIFY)]; - // Create the actual window - xcb::create_window_checked( - // Connection to the X server - conn, - // Window depth - COPY_FROM_PARENT.try_into().unwrap(), - // The new window's ID - id, - // Parent window of this new window - screen.root(), - // X-coordinate of the new window - 0, - // Y-coordinate of the new window - 0, - // Width of the new window - 1, - // Height of the new window - 1, - // Border width - 0, - // Window class type - WINDOW_CLASS_INPUT_ONLY as u16, - // Visual ID - COPY_FROM_PARENT.try_into().unwrap(), - // Window properties mask - &cw_values, - ) - .request_check()?; + if let Some(err) = conn + .create_window( + // Window depth + x11rb::COPY_FROM_PARENT.try_into().unwrap(), + // The new window's ID + id, + // Parent window of this new window + screen.root, + // X-coordinate of the new window + 0, + // Y-coordinate of the new window + 0, + // Width of the new window + 1, + // Height of the new window + 1, + // Border width + 0, + // Window class type + WindowClass::InputOnly, + // Visual ID + x11rb::COPY_FROM_PARENT, + // Window properties mask + &CreateWindowAux::new().event_mask(EventMask::StructureNotify), + )? + .check()? + { + return Err(x11rb::errors::ReplyError::X11Error(err).into()); + } Ok(id) } @@ -152,7 +150,7 @@ impl Application { } #[inline] - pub(crate) fn connection(&self) -> &Rc { + pub(crate) fn connection(&self) -> &Rc { &self.connection } @@ -162,101 +160,85 @@ impl Application { } /// Returns `Ok(true)` if we want to exit the main loop. - fn handle_event(&self, ev: &xcb::GenericEvent) -> Result { - let ev_type = ev.response_type() & !0x80; - // NOTE: When adding handling for any of the following events, - // there must be a check against self.window_id - // to know if the event must be ignored. - // Otherwise there will be a "failed to get window" error. - // - // CIRCULATE_NOTIFY, CONFIGURE_NOTIFY, GRAVITY_NOTIFY - // MAP_NOTIFY, REPARENT_NOTIFY, UNMAP_NOTIFY - match ev_type { - EXPOSE => { - let expose = unsafe { xcb::cast_event::(&ev) }; - let window_id = expose.window(); + fn handle_event(&self, ev: &Event) -> Result { + match ev { + // NOTE: When adding handling for any of the following events, + // there must be a check against self.window_id + // to know if the event must be ignored. + // Otherwise there will be a "failed to get window" error. + // + // CIRCULATE_NOTIFY, CONFIGURE_NOTIFY, GRAVITY_NOTIFY + // MAP_NOTIFY, REPARENT_NOTIFY, UNMAP_NOTIFY + Event::Expose(ev) => { let w = self - .window(window_id) + .window(ev.window) .context("EXPOSE - failed to get window")?; - w.handle_expose(expose) - .context("EXPOSE - failed to handle")?; + w.handle_expose(ev).context("EXPOSE - failed to handle")?; } - KEY_PRESS => { - let key_press = unsafe { xcb::cast_event::(&ev) }; - let window_id = key_press.event(); + Event::KeyPress(ev) => { let w = self - .window(window_id) + .window(ev.event) .context("KEY_PRESS - failed to get window")?; - w.handle_key_press(key_press) + w.handle_key_press(ev) .context("KEY_PRESS - failed to handle")?; } - BUTTON_PRESS => { - let button_press = unsafe { xcb::cast_event::(&ev) }; - let window_id = button_press.event(); + Event::ButtonPress(ev) => { let w = self - .window(window_id) + .window(ev.event) .context("BUTTON_PRESS - failed to get window")?; // X doesn't have dedicated scroll events: it uses mouse buttons instead. // Buttons 4/5 are vertical; 6/7 are horizontal. - if button_press.detail() >= 4 && button_press.detail() <= 7 { - w.handle_wheel(button_press) + if ev.detail >= 4 && ev.detail <= 7 { + w.handle_wheel(ev) .context("BUTTON_PRESS - failed to handle wheel")?; } else { - w.handle_button_press(button_press) + w.handle_button_press(ev) .context("BUTTON_PRESS - failed to handle")?; } } - BUTTON_RELEASE => { - let button_release = unsafe { xcb::cast_event::(&ev) }; - let window_id = button_release.event(); + Event::ButtonRelease(ev) => { let w = self - .window(window_id) + .window(ev.event) .context("BUTTON_RELEASE - failed to get window")?; - if button_release.detail() >= 4 && button_release.detail() <= 7 { + if ev.detail >= 4 && ev.detail <= 7 { // This is the release event corresponding to a mouse wheel. // Ignore it: we already handled the press event. } else { - w.handle_button_release(button_release) + w.handle_button_release(ev) .context("BUTTON_RELEASE - failed to handle")?; } } - MOTION_NOTIFY => { - let motion_notify = unsafe { xcb::cast_event::(&ev) }; - let window_id = motion_notify.event(); + Event::MotionNotify(ev) => { let w = self - .window(window_id) + .window(ev.event) .context("MOTION_NOTIFY - failed to get window")?; - w.handle_motion_notify(motion_notify) + w.handle_motion_notify(ev) .context("MOTION_NOTIFY - failed to handle")?; } - CLIENT_MESSAGE => { - let client_message = unsafe { xcb::cast_event::(&ev) }; - let window_id = client_message.window(); + Event::ClientMessage(ev) => { let w = self - .window(window_id) + .window(ev.window) .context("CLIENT_MESSAGE - failed to get window")?; - w.handle_client_message(client_message) + w.handle_client_message(ev) .context("CLIENT_MESSAGE - failed to handle")?; } - DESTROY_NOTIFY => { - let destroy_notify = unsafe { xcb::cast_event::(&ev) }; - let window_id = destroy_notify.window(); - if window_id == self.window_id { + Event::DestroyNotify(ev) => { + if ev.window == self.window_id { // The destruction of the Application window means that // we need to quit the run loop. return Ok(true); } let w = self - .window(window_id) + .window(ev.window) .context("DESTROY_NOTIFY - failed to get window")?; - w.handle_destroy_notify(destroy_notify) + w.handle_destroy_notify(ev) .context("DESTROY_NOTIFY - failed to handle")?; // Remove our reference to the Window and allow it to be dropped let windows_left = self - .remove_window(window_id) + .remove_window(ev.window) .context("DESTROY_NOTIFY - failed to remove window")?; // Check if we need to finalize a quit request if windows_left == 0 && borrow!(self.state)?.quitting { @@ -270,7 +252,7 @@ impl Application { pub fn run(self, _handler: Option>) { loop { - if let Some(ev) = self.connection.wait_for_event() { + if let Ok(ev) = self.connection.wait_for_event() { match self.handle_event(&ev) { Ok(quit) => { if quit { @@ -298,7 +280,7 @@ impl Application { for window in state.windows.values() { window.destroy(); } - self.connection.flush(); + log_x11!(self.connection.flush()); } } } else { @@ -307,8 +289,8 @@ impl Application { } fn finalize_quit(&self) { - xcb::destroy_window(&self.connection, self.window_id); - self.connection.flush(); + log_x11!(self.connection.destroy_window(self.window_id)); + log_x11!(self.connection.flush()); } pub fn clipboard(&self) -> Clipboard { diff --git a/druid-shell/src/platform/x11/mod.rs b/druid-shell/src/platform/x11/mod.rs index 0653918b1a..47fa934fea 100644 --- a/druid-shell/src/platform/x11/mod.rs +++ b/druid-shell/src/platform/x11/mod.rs @@ -18,11 +18,12 @@ // Might be related to the "sleep scheduler" in XWindow::render()? // TODO(x11/render_improvements): double-buffering / present strategies / etc? +#[macro_use] +mod util; + pub mod application; pub mod clipboard; pub mod error; pub mod keycodes; pub mod menu; pub mod window; - -mod util; diff --git a/druid-shell/src/platform/x11/util.rs b/druid-shell/src/platform/x11/util.rs index f35441c6b4..1e5b74fb33 100644 --- a/druid-shell/src/platform/x11/util.rs +++ b/druid-shell/src/platform/x11/util.rs @@ -14,33 +14,42 @@ //! Miscellaneous utility functions for working with X11. -use xcb::{Connection, Screen, Visualtype, Window}; +use std::rc::Rc; + +use x11rb::errors::ReplyError; +use x11rb::protocol::randr::{ConnectionExt, ModeFlag}; +use x11rb::protocol::xproto::{Screen, Visualtype, Window}; +use x11rb::xcb_ffi::XCBConnection; // See: https://github.com/rtbo/rust-xcb/blob/master/examples/randr_screen_modes.rs -pub fn refresh_rate(conn: &Connection, window_id: Window) -> Option { - let cookie = xcb::randr::get_screen_resources(conn, window_id); - let reply = cookie.get_reply().unwrap(); - let mut modes = reply.modes(); +pub fn refresh_rate(conn: &Rc, window_id: Window) -> Option { + let reply = + (|| -> Result<_, ReplyError> { Ok(conn.randr_get_screen_resources(window_id)?.reply()?) })( + ); + let reply = match reply { + Ok(r) => r, + Err(_) => return None, + }; // TODO(x11/render_improvements): Figure out a more correct way of getting the screen's refresh rate. // Or maybe we don't even need this function if I figure out a better way to schedule redraws? // Assuming the first mode is the one we want to use. This is probably a bug on some setups. // Any better way to find the correct one? - let refresh_rate = modes.next().and_then(|mode_info| { - let flags = mode_info.mode_flags(); + let refresh_rate = reply.modes.first().and_then(|mode_info| { + let flags = mode_info.mode_flags; let vtotal = { - let mut val = mode_info.vtotal(); - if (flags & xcb::randr::MODE_FLAG_DOUBLE_SCAN) != 0 { + let mut val = mode_info.vtotal; + if (flags & u32::from(ModeFlag::DoubleScan)) != 0 { val *= 2; } - if (flags & xcb::randr::MODE_FLAG_INTERLACE) != 0 { + if (flags & u32::from(ModeFlag::Interlace)) != 0 { val /= 2; } val }; - if vtotal != 0 && mode_info.htotal() != 0 { - Some((mode_info.dot_clock() as f64) / (vtotal as f64 * mode_info.htotal() as f64)) + if vtotal != 0 && mode_info.htotal != 0 { + Some((mode_info.dot_clock as f64) / (vtotal as f64 * mode_info.htotal as f64)) } else { None } @@ -50,13 +59,25 @@ pub fn refresh_rate(conn: &Connection, window_id: Window) -> Option { } // Apparently you have to get the visualtype this way :| -pub fn get_visual_from_screen(screen: &Screen<'_>) -> Option { - for depth in screen.allowed_depths() { - for visual in depth.visuals() { - if visual.visual_id() == screen.root_visual() { - return Some(visual); +pub fn get_visual_from_screen(screen: &Screen) -> Option { + for depth in &screen.allowed_depths { + for visual in &depth.visuals { + if visual.visual_id == screen.root_visual { + return Some(*visual); } } } None } + +macro_rules! log_x11 { + ($val:expr) => { + if let Err(e) = $val { + // We probably don't want to include file/line numbers. This logging is done in + // a context where X11 errors probably just mean that the connection to the X server + // was lost. In particular, it doesn't represent a druid-shell bug for which we want + // more context. + log::error!("X11 error: {}", e); + } + }; +} diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index 6f911568ac..bc33dff0ce 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -21,13 +21,11 @@ use std::rc::{Rc, Weak}; use anyhow::{anyhow, Context, Error}; use cairo::{XCBConnection, XCBDrawable, XCBSurface, XCBVisualType}; -use xcb::{ - Atom, Visualtype, ATOM_ATOM, ATOM_STRING, ATOM_WM_NAME, CONFIG_WINDOW_STACK_MODE, - COPY_FROM_PARENT, CURRENT_TIME, CW_BACK_PIXEL, CW_EVENT_MASK, EVENT_MASK_BUTTON_PRESS, - EVENT_MASK_BUTTON_RELEASE, EVENT_MASK_EXPOSURE, EVENT_MASK_KEY_PRESS, EVENT_MASK_KEY_RELEASE, - EVENT_MASK_POINTER_MOTION, EVENT_MASK_STRUCTURE_NOTIFY, INPUT_FOCUS_POINTER_ROOT, - PROP_MODE_REPLACE, STACK_MODE_ABOVE, WINDOW_CLASS_INPUT_OUTPUT, +use x11rb::connection::Connection; +use x11rb::protocol::xproto::{ + self, Atom, AtomEnum, ConnectionExt, EventMask, ExposeEvent, PropMode, Visualtype, WindowClass, }; +use x11rb::wrapper::ConnectionExt as WrapperConnectionExt; use crate::dialog::{FileDialogOptions, FileInfo}; use crate::error::Error as ShellError; @@ -43,6 +41,36 @@ use super::application::Application; use super::menu::Menu; use super::util; +/// A version of XCB's `xcb_visualtype_t` struct. This was copied from the example in x11rb; it +/// is used to interoperate with cairo. +#[derive(Debug, Clone, Copy)] +#[repr(C)] +pub struct xcb_visualtype_t { + pub visual_id: u32, + pub class: u8, + pub bits_per_rgb_value: u8, + pub colormap_entries: u16, + pub red_mask: u32, + pub green_mask: u32, + pub blue_mask: u32, + pub pad0: [u8; 4], +} + +impl From for xcb_visualtype_t { + fn from(value: Visualtype) -> xcb_visualtype_t { + xcb_visualtype_t { + visual_id: value.visual_id, + class: value.class.into(), + bits_per_rgb_value: value.bits_per_rgb_value, + colormap_entries: value.colormap_entries, + red_mask: value.red_mask, + green_mask: value.green_mask, + blue_mask: value.blue_mask, + pad0: [0; 4], + } + } +} + pub(crate) struct WindowBuilder { app: Application, handler: Option>, @@ -101,10 +129,12 @@ impl WindowBuilder { // the client and window manager in which the client is willing to participate. // // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#wm_protocols_property - let wm_protocols = xcb::intern_atom(conn, false, "WM_PROTOCOLS") - .get_reply() + let wm_protocols = conn + .intern_atom(false, b"WM_PROTOCOLS") .context("failed to get WM_PROTOCOLS")? - .atom(); + .reply() + .context("failed to get WM_PROTOCOLS reply")? + .atom; // WM_DELETE_WINDOW // @@ -115,23 +145,23 @@ impl WindowBuilder { // Registering for but ignoring this event means that the window will remain open. // // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#window_deletion - let wm_delete_window = xcb::intern_atom(conn, false, "WM_DELETE_WINDOW") - .get_reply() + let wm_delete_window = conn + .intern_atom(false, b"WM_DELETE_WINDOW") .context("failed to get WM_DELETE_WINDOW")? - .atom(); + .reply() + .context("failed to get WM_DELETE_WINDOW reply")? + .atom; // Replace the window's WM_PROTOCOLS with the following. let protocols = [wm_delete_window]; // TODO(x11/errors): Check the response for errors? - xcb::change_property( - conn, - PROP_MODE_REPLACE as u8, + conn.change_property32( + PropMode::Replace, window_id, wm_protocols, - ATOM_ATOM, - 32, + AtomEnum::ATOM, &protocols, - ); + )?; Ok(WindowAtoms { wm_protocols, @@ -143,16 +173,19 @@ impl WindowBuilder { fn create_cairo_context( &self, window_id: u32, - visual_type: &mut Visualtype, + visual_type: &Visualtype, ) -> Result, Error> { let conn = self.app.connection(); let cairo_xcb_connection = unsafe { - XCBConnection::from_raw_none(conn.get_raw_conn() as *mut cairo_sys::xcb_connection_t) + XCBConnection::from_raw_none( + conn.get_raw_xcb_connection() as *mut cairo_sys::xcb_connection_t + ) }; let cairo_drawable = XCBDrawable(window_id); + let mut xcb_visual = xcb_visualtype_t::from(*visual_type); let cairo_visual_type = unsafe { XCBVisualType::from_raw_none( - &mut visual_type.base as *mut _ as *mut cairo_sys::xcb_visualtype_t, + &mut xcb_visual as *mut xcb_visualtype_t as *mut cairo_sys::xcb_visualtype_t, ) }; let cairo_surface = XCBSurface::create( @@ -171,62 +204,64 @@ impl WindowBuilder { pub fn build(self) -> Result { let conn = self.app.connection(); let screen_num = self.app.screen_num(); - let id = conn.generate_id(); - let setup = conn.get_setup(); + let id = conn.generate_id()?; + let setup = conn.setup(); // TODO(x11/errors): Don't unwrap for screen or visual_type? - let screen = setup.roots().nth(screen_num as usize).unwrap(); - let mut visual_type = util::get_visual_from_screen(&screen).unwrap(); - let visual_id = visual_type.visual_id(); - - let cw_values = [ - (CW_BACK_PIXEL, screen.white_pixel()), - ( - CW_EVENT_MASK, - EVENT_MASK_EXPOSURE - | EVENT_MASK_STRUCTURE_NOTIFY - | EVENT_MASK_KEY_PRESS - | EVENT_MASK_KEY_RELEASE - | EVENT_MASK_BUTTON_PRESS - | EVENT_MASK_BUTTON_RELEASE - | EVENT_MASK_POINTER_MOTION, - ), - ]; + let screen = setup.roots.get(screen_num as usize).unwrap(); + let visual_type = util::get_visual_from_screen(&screen).unwrap(); + let visual_id = visual_type.visual_id; + + let cw_values = xproto::CreateWindowAux::new() + .background_pixel(screen.white_pixel) + .event_mask( + EventMask::Exposure + | EventMask::StructureNotify + | EventMask::KeyPress + | EventMask::KeyRelease + | EventMask::ButtonPress + | EventMask::ButtonRelease + | EventMask::PointerMotion, + ); // Create the actual window - xcb::create_window_checked( - // Connection to the X server - conn, - // Window depth - COPY_FROM_PARENT.try_into().unwrap(), - // The new window's ID - id, - // Parent window of this new window - // TODO(#468): either `screen.root()` (no parent window) or pass parent here to attach - screen.root(), - // X-coordinate of the new window - 0, - // Y-coordinate of the new window - 0, - // Width of the new window - // TODO(x11/dpi_scaling): figure out DPI scaling - self.size.width as u16, - // Height of the new window - // TODO(x11/dpi_scaling): figure out DPI scaling - self.size.height as u16, - // Border width - 0, - // Window class type - WINDOW_CLASS_INPUT_OUTPUT as u16, - // Visual ID - visual_id, - // Window properties mask - &cw_values, - ) - .request_check()?; + // TODO: https://github.com/psychon/x11rb/pull/469 will make error handling easier with the + // next x11rb release. + if let Some(err) = conn + .create_window( + // Window depth + x11rb::COPY_FROM_PARENT.try_into().unwrap(), + // The new window's ID + id, + // Parent window of this new window + // TODO(#468): either `screen.root()` (no parent window) or pass parent here to attach + screen.root, + // X-coordinate of the new window + 0, + // Y-coordinate of the new window + 0, + // Width of the new window + // TODO(x11/dpi_scaling): figure out DPI scaling + self.size.width as u16, + // Height of the new window + // TODO(x11/dpi_scaling): figure out DPI scaling + self.size.height as u16, + // Border width + 0, + // Window class type + WindowClass::InputOutput, + // Visual ID + visual_id, + // Window properties mask + &cw_values, + )? + .check()? + { + return Err(x11rb::errors::ReplyError::X11Error(err).into()); + } // TODO(x11/errors): Should do proper cleanup (window destruction etc) in case of error let atoms = self.atoms(id)?; - let cairo_context = self.create_cairo_context(id, &mut visual_type)?; + let cairo_context = self.create_cairo_context(id, &visual_type)?; // Figure out the refresh rate of the current screen let refresh_rate = util::refresh_rate(conn, id); let state = RefCell::new(WindowState { size: self.size }); @@ -290,7 +325,7 @@ impl Window { /// /// Does not flush the connection. pub fn destroy(&self) { - xcb::destroy_window(self.app.connection(), self.id); + log_x11!(self.app.connection().destroy_window(self.id)); } fn cairo_surface(&self) -> Result { @@ -337,28 +372,28 @@ impl Window { /// Does not flush the connection. fn request_redraw(&self, rect: Rect) { // See: http://rtbo.github.io/rust-xcb/xcb/ffi/xproto/struct.xcb_expose_event_t.html - let expose_event = xcb::ExposeEvent::new( - self.id, - rect.x0 as u16, - rect.y0 as u16, - rect.width() as u16, - rect.height() as u16, - 0, - ); - xcb::send_event( - self.app.connection(), + let expose_event = ExposeEvent { + window: self.id, + x: rect.x0 as u16, + y: rect.y0 as u16, + width: rect.width() as u16, + height: rect.height() as u16, + count: 0, + response_type: x11rb::protocol::xproto::EXPOSE_EVENT, + sequence: 0, + }; + log_x11!(self.app.connection().send_event( false, self.id, - EVENT_MASK_EXPOSURE, - &expose_event, - ); + EventMask::Exposure, + expose_event + )); } fn render(&self, invalid_rect: Rect) -> Result<(), Error> { // Figure out the window's current size - let geometry_cookie = xcb::get_geometry(self.app.connection(), self.id); - let reply = geometry_cookie.get_reply()?; - let size = Size::new(reply.width() as f64, reply.height() as f64); + let reply = self.app.connection().get_geometry(self.id)?.reply()?; + let size = Size::new(reply.width as f64, reply.height as f64); self.set_size(size)?; let mut anim = false; @@ -404,18 +439,18 @@ impl Window { self.request_redraw(size.to_rect()); } - self.app.connection().flush(); + self.app.connection().flush()?; Ok(()) } fn show(&self) { - xcb::map_window(self.app.connection(), self.id); - self.app.connection().flush(); + log_x11!(self.app.connection().map_window(self.id)); + log_x11!(self.app.connection().flush()); } fn close(&self) { self.destroy(); - self.app.connection().flush(); + log_x11!(self.app.connection().flush()); } /// Set whether the window should be resizable @@ -431,18 +466,17 @@ impl Window { /// Bring this window to the front of the window stack and give it focus. fn bring_to_front_and_focus(&self) { // TODO(x11/misc): Unsure if this does exactly what the doc comment says; need a test case. - xcb::configure_window( - self.app.connection(), + let conn = self.app.connection(); + log_x11!(conn.configure_window( self.id, - &[(CONFIG_WINDOW_STACK_MODE as u16, STACK_MODE_ABOVE)], - ); - xcb::set_input_focus( - self.app.connection(), - INPUT_FOCUS_POINTER_ROOT as u8, + &xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::Above), + )); + log_x11!(conn.set_input_focus( + xproto::InputFocus::PointerRoot, self.id, - CURRENT_TIME, - ); - self.app.connection().flush(); + xproto::Time::CurrentTime, + )); + log_x11!(conn.flush()); } fn invalidate(&self) { @@ -454,20 +488,18 @@ impl Window { fn invalidate_rect(&self, rect: Rect) { self.request_redraw(rect); - self.app.connection().flush(); + log_x11!(self.app.connection().flush()); } fn set_title(&self, title: &str) { - xcb::change_property( - self.app.connection(), - PROP_MODE_REPLACE as u8, + log_x11!(self.app.connection().change_property8( + xproto::PropMode::Replace, self.id, - ATOM_WM_NAME, - ATOM_STRING, - 8, + AtomEnum::WM_NAME, + AtomEnum::STRING, title.as_bytes(), - ); - self.app.connection().flush(); + )); + log_x11!(self.app.connection().flush()); } fn set_menu(&self, _menu: Menu) { @@ -479,24 +511,24 @@ impl Window { Ok(Scale::new(1.0, 1.0)) } - pub fn handle_expose(&self, expose: &xcb::ExposeEvent) -> Result<(), Error> { + pub fn handle_expose(&self, expose: &xproto::ExposeEvent) -> Result<(), Error> { // TODO(x11/dpi_scaling): when dpi scaling is // implemented, it needs to be used here too let rect = Rect::from_origin_size( - (expose.x() as f64, expose.y() as f64), - (expose.width() as f64, expose.height() as f64), + (expose.x as f64, expose.y as f64), + (expose.width as f64, expose.height as f64), ); Ok(self.render(rect)?) } - pub fn handle_key_press(&self, key_press: &xcb::KeyPressEvent) -> Result<(), Error> { - let key: u32 = key_press.detail() as u32; + pub fn handle_key_press(&self, key_press: &xproto::KeyPressEvent) -> Result<(), Error> { + let key: u32 = key_press.detail as u32; let key_code: KeyCode = key.into(); let key_event = KeyEvent::new( key_code, // TODO: detect repeated keys false, - key_mods(key_press.state()), + key_mods(key_press.state), key_code, key_code, ); @@ -504,14 +536,17 @@ impl Window { Ok(()) } - pub fn handle_button_press(&self, button_press: &xcb::ButtonPressEvent) -> Result<(), Error> { - let button = mouse_button(button_press.detail()); + pub fn handle_button_press( + &self, + button_press: &xproto::ButtonPressEvent, + ) -> Result<(), Error> { + let button = mouse_button(button_press.detail); let mouse_event = MouseEvent { - pos: Point::new(button_press.event_x() as f64, button_press.event_y() as f64), + pos: Point::new(button_press.event_x as f64, button_press.event_y as f64), // The xcb state field doesn't include the newly pressed button, but // druid wants it to be included. - buttons: mouse_buttons(button_press.state()).with(button), - mods: key_mods(button_press.state()), + buttons: mouse_buttons(button_press.state).with(button), + mods: key_mods(button_press.state), // TODO: detect the count count: 1, focus: false, @@ -524,18 +559,15 @@ impl Window { pub fn handle_button_release( &self, - button_release: &xcb::ButtonReleaseEvent, + button_release: &xproto::ButtonReleaseEvent, ) -> Result<(), Error> { - let button = mouse_button(button_release.detail()); + let button = mouse_button(button_release.detail); let mouse_event = MouseEvent { - pos: Point::new( - button_release.event_x() as f64, - button_release.event_y() as f64, - ), + pos: Point::new(button_release.event_x as f64, button_release.event_y as f64), // The xcb state includes the newly released button, but druid // doesn't want it. - buttons: mouse_buttons(button_release.state()).without(button), - mods: key_mods(button_release.state()), + buttons: mouse_buttons(button_release.state).without(button), + mods: key_mods(button_release.state), count: 0, focus: false, button, @@ -545,9 +577,9 @@ impl Window { Ok(()) } - pub fn handle_wheel(&self, event: &xcb::ButtonPressEvent) -> Result<(), Error> { - let button = event.detail(); - let mods = key_mods(event.state()); + pub fn handle_wheel(&self, event: &xproto::ButtonPressEvent) -> Result<(), Error> { + let button = event.detail; + let mods = key_mods(event.state); // We use a delta of 120 per tick to match the behavior of Windows. let delta = match button { @@ -560,9 +592,9 @@ impl Window { _ => return Err(anyhow!("unexpected mouse wheel button: {}", button)), }; let mouse_event = MouseEvent { - pos: Point::new(event.event_x() as f64, event.event_y() as f64), - buttons: mouse_buttons(event.state()), - mods: key_mods(event.state()), + pos: Point::new(event.event_x as f64, event.event_y as f64), + buttons: mouse_buttons(event.state), + mods: key_mods(event.state), count: 0, focus: false, button: MouseButton::None, @@ -575,15 +607,12 @@ impl Window { pub fn handle_motion_notify( &self, - motion_notify: &xcb::MotionNotifyEvent, + motion_notify: &xproto::MotionNotifyEvent, ) -> Result<(), Error> { let mouse_event = MouseEvent { - pos: Point::new( - motion_notify.event_x() as f64, - motion_notify.event_y() as f64, - ), - buttons: mouse_buttons(motion_notify.state()), - mods: key_mods(motion_notify.state()), + pos: Point::new(motion_notify.event_x as f64, motion_notify.event_y as f64), + buttons: mouse_buttons(motion_notify.state), + mods: key_mods(motion_notify.state), count: 0, focus: false, button: MouseButton::None, @@ -595,12 +624,12 @@ impl Window { pub fn handle_client_message( &self, - client_message: &xcb::ClientMessageEvent, + client_message: &xproto::ClientMessageEvent, ) -> Result<(), Error> { // https://www.x.org/releases/X11R7.7/doc/libX11/libX11/libX11.html#id2745388 // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#window_deletion - if client_message.type_() == self.atoms.wm_protocols && client_message.format() == 32 { - let protocol = client_message.data().data32()[0]; + if client_message.type_ == self.atoms.wm_protocols && client_message.format == 32 { + let protocol = client_message.data.as_data32()[0]; if protocol == self.atoms.wm_delete_window { self.close(); } @@ -608,9 +637,10 @@ impl Window { Ok(()) } + #[allow(clippy::trivially_copy_pass_by_ref)] pub fn handle_destroy_notify( &self, - _destroy_notify: &xcb::DestroyNotifyEvent, + _destroy_notify: &xproto::DestroyNotifyEvent, ) -> Result<(), Error> { borrow_mut!(self.handler)?.destroy(); Ok(()) @@ -639,9 +669,9 @@ fn mouse_button(button: u8) -> MouseButton { fn mouse_buttons(mods: u16) -> MouseButtons { let mut buttons = MouseButtons::new(); let button_masks = &[ - (xcb::xproto::BUTTON_MASK_1, MouseButton::Left), - (xcb::xproto::BUTTON_MASK_2, MouseButton::Middle), - (xcb::xproto::BUTTON_MASK_3, MouseButton::Right), + (xproto::ButtonMask::M1, MouseButton::Left), + (xproto::ButtonMask::M2, MouseButton::Middle), + (xproto::ButtonMask::M3, MouseButton::Right), // TODO: determine the X1/X2 state, using our own caching if necessary. // BUTTON_MASK_4/5 do not work: they are for scroll events. ]; @@ -658,13 +688,13 @@ fn mouse_buttons(mods: u16) -> MouseButtons { fn key_mods(mods: u16) -> KeyModifiers { let mut ret = KeyModifiers::default(); let mut key_masks = [ - (xcb::xproto::MOD_MASK_SHIFT, &mut ret.shift), - (xcb::xproto::MOD_MASK_CONTROL, &mut ret.ctrl), + (xproto::ModMask::Shift, &mut ret.shift), + (xproto::ModMask::Control, &mut ret.ctrl), // X11's mod keys are configurable, but this seems // like a reasonable default for US keyboards, at least, // where the "windows" key seems to be MOD_MASK_4. - (xcb::xproto::MOD_MASK_1, &mut ret.alt), - (xcb::xproto::MOD_MASK_4, &mut ret.meta), + (xproto::ModMask::M1, &mut ret.alt), + (xproto::ModMask::M4, &mut ret.meta), ]; for (mask, key) in &mut key_masks { if mods & (*mask as u16) != 0 { From 184312cbdcb2c46bcfd791a1f4c3837dc77a6552 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 9 Jun 2020 15:35:41 -0500 Subject: [PATCH 2/5] Add changelog entry. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45f5dce197..e0f19f88bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ You can find its changes [documented below](#060---2020-06-01). ### Maintenance - Standardized web targeting terminology. ([#1013] by [@xStrom]) +- X11: Ported the X11 backend to `x11rb`. ([#1025] by [@jneem]) ### Outside News @@ -327,6 +328,7 @@ Last release without a changelog :( [#1008]: https://github.com/xi-editor/druid/pull/1008 [#1011]: https://github.com/xi-editor/druid/pull/1011 [#1013]: https://github.com/xi-editor/druid/pull/1013 +[#1025]: https://github.com/xi-editor/druid/pull/1025 [#1028]: https://github.com/xi-editor/druid/pull/1028 [#1042]: https://github.com/xi-editor/druid/pull/1042 [#1043]: https://github.com/xi-editor/druid/pull/1043 From 0e02b0866940494ebb8c29ee1ca75c316f0ae3a7 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Wed, 10 Jun 2020 17:09:32 -0500 Subject: [PATCH 3/5] Review comments. --- druid-shell/src/platform/x11/application.rs | 2 + druid-shell/src/platform/x11/util.rs | 66 +++++++++++---------- druid-shell/src/platform/x11/window.rs | 9 ++- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index e0a961e058..c3f154c986 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -123,6 +123,8 @@ impl Application { )? .check()? { + // TODO: https://github.com/psychon/x11rb/pull/469 will make error handling easier with + // the next x11rb release. return Err(x11rb::errors::ReplyError::X11Error(err).into()); } diff --git a/druid-shell/src/platform/x11/util.rs b/druid-shell/src/platform/x11/util.rs index 1e5b74fb33..f475648a87 100644 --- a/druid-shell/src/platform/x11/util.rs +++ b/druid-shell/src/platform/x11/util.rs @@ -16,46 +16,52 @@ use std::rc::Rc; -use x11rb::errors::ReplyError; +use anyhow::{anyhow, Error}; use x11rb::protocol::randr::{ConnectionExt, ModeFlag}; use x11rb::protocol::xproto::{Screen, Visualtype, Window}; use x11rb::xcb_ffi::XCBConnection; // See: https://github.com/rtbo/rust-xcb/blob/master/examples/randr_screen_modes.rs pub fn refresh_rate(conn: &Rc, window_id: Window) -> Option { - let reply = - (|| -> Result<_, ReplyError> { Ok(conn.randr_get_screen_resources(window_id)?.reply()?) })( - ); - let reply = match reply { - Ok(r) => r, - Err(_) => return None, - }; + let try_refresh_rate = || -> Result { + let reply = conn.randr_get_screen_resources(window_id)?.reply()?; - // TODO(x11/render_improvements): Figure out a more correct way of getting the screen's refresh rate. - // Or maybe we don't even need this function if I figure out a better way to schedule redraws? - // Assuming the first mode is the one we want to use. This is probably a bug on some setups. - // Any better way to find the correct one? - let refresh_rate = reply.modes.first().and_then(|mode_info| { - let flags = mode_info.mode_flags; - let vtotal = { - let mut val = mode_info.vtotal; - if (flags & u32::from(ModeFlag::DoubleScan)) != 0 { - val *= 2; - } - if (flags & u32::from(ModeFlag::Interlace)) != 0 { - val /= 2; - } - val - }; + // TODO(x11/render_improvements): Figure out a more correct way of getting the screen's refresh rate. + // Or maybe we don't even need this function if I figure out a better way to schedule redraws? + // Assuming the first mode is the one we want to use. This is probably a bug on some setups. + // Any better way to find the correct one? + reply + .modes + .first() + .ok_or_else(|| anyhow!("didn't get any modes")) + .and_then(|mode_info| { + let flags = mode_info.mode_flags; + let vtotal = { + let mut val = mode_info.vtotal; + if (flags & u32::from(ModeFlag::DoubleScan)) != 0 { + val *= 2; + } + if (flags & u32::from(ModeFlag::Interlace)) != 0 { + val /= 2; + } + val + }; - if vtotal != 0 && mode_info.htotal != 0 { - Some((mode_info.dot_clock as f64) / (vtotal as f64 * mode_info.htotal as f64)) - } else { + if vtotal != 0 && mode_info.htotal != 0 { + Ok((mode_info.dot_clock as f64) / (vtotal as f64 * mode_info.htotal as f64)) + } else { + Err(anyhow!("got nonsensical mode values")) + } + }) + }; + + match try_refresh_rate() { + Err(e) => { + log::error!("failed to find refresh rate: {}", e); None } - })?; - - Some(refresh_rate) + Ok(r) => Some(r), + } } // Apparently you have to get the visualtype this way :| diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index bc33dff0ce..aa475dd7c6 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -41,8 +41,13 @@ use super::application::Application; use super::menu::Menu; use super::util; -/// A version of XCB's `xcb_visualtype_t` struct. This was copied from the example in x11rb; it +/// A version of XCB's `xcb_visualtype_t` struct. This was copied from the [example] in x11rb; it /// is used to interoperate with cairo. +/// +/// The official upstream reference for this struct definition is [here]. +/// +/// [example]: https://github.com/psychon/x11rb/blob/master/cairo-example/src/main.rs +/// [here]: https://xcb.freedesktop.org/manual/structxcb__visualtype__t.html #[derive(Debug, Clone, Copy)] #[repr(C)] pub struct xcb_visualtype_t { @@ -386,7 +391,7 @@ impl Window { false, self.id, EventMask::Exposure, - expose_event + expose_event, )); } From 38b6248a8d22594da08c5f9f8eb99a59e108e199 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 19 Jun 2020 12:24:26 -0500 Subject: [PATCH 4/5] Update x11rb. --- druid-shell/Cargo.toml | 2 +- druid-shell/src/platform/x11/application.rs | 64 ++++----- druid-shell/src/platform/x11/window.rs | 137 +++++++++----------- 3 files changed, 96 insertions(+), 107 deletions(-) diff --git a/druid-shell/Cargo.toml b/druid-shell/Cargo.toml index 49b528d904..b827a52779 100644 --- a/druid-shell/Cargo.toml +++ b/druid-shell/Cargo.toml @@ -39,7 +39,7 @@ gtk = { version = "0.8.1", optional = true } glib = { version = "0.9.3", optional = true } glib-sys = { version = "0.9.1", optional = true } gtk-sys = { version = "0.9.2", optional = true } -x11rb = { version = "0.5.0", features = ["allow-unsafe-code", "randr"], optional = true } +x11rb = { version = "0.6.0", features = ["allow-unsafe-code", "randr"], optional = true } [target.'cfg(target_os="windows")'.dependencies] wio = "0.2.2" diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index c3f154c986..27fcc1a9e4 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -72,6 +72,14 @@ struct State { impl Application { pub fn new() -> Result { + // If we want to support OpenGL, we will need to open a connection with Xlib support (see + // https://xcb.freedesktop.org/opengl/ for background). There is some sample code for this + // in the `rust-xcb` crate (see `connect_with_xlib_display`), although it may be missing + // something: according to the link below, If you want to handle events through x11rb and + // libxcb, you should call XSetEventQueueOwner(dpy, XCBOwnsEventQueue). Otherwise, libX11 + // might randomly eat your events / move them to its own event queue. + // + // https://github.com/xi-editor/druid/pull/1025/files/76b923417183bd103f61e56b56a56474b7417cec#r442777892 let (conn, screen_num) = XCBConnection::connect(None)?; let connection = Rc::new(conn); let window_id = Application::create_event_window(&connection, screen_num as i32)?; @@ -96,37 +104,31 @@ impl Application { .ok_or_else(|| anyhow!("invalid screen num: {}", screen_num))?; // Create the actual window - if let Some(err) = conn - .create_window( - // Window depth - x11rb::COPY_FROM_PARENT.try_into().unwrap(), - // The new window's ID - id, - // Parent window of this new window - screen.root, - // X-coordinate of the new window - 0, - // Y-coordinate of the new window - 0, - // Width of the new window - 1, - // Height of the new window - 1, - // Border width - 0, - // Window class type - WindowClass::InputOnly, - // Visual ID - x11rb::COPY_FROM_PARENT, - // Window properties mask - &CreateWindowAux::new().event_mask(EventMask::StructureNotify), - )? - .check()? - { - // TODO: https://github.com/psychon/x11rb/pull/469 will make error handling easier with - // the next x11rb release. - return Err(x11rb::errors::ReplyError::X11Error(err).into()); - } + conn.create_window( + // Window depth + x11rb::COPY_FROM_PARENT.try_into().unwrap(), + // The new window's ID + id, + // Parent window of this new window + screen.root, + // X-coordinate of the new window + 0, + // Y-coordinate of the new window + 0, + // Width of the new window + 1, + // Height of the new window + 1, + // Border width + 0, + // Window class type + WindowClass::InputOnly, + // Visual ID + x11rb::COPY_FROM_PARENT, + // Window properties mask + &CreateWindowAux::new().event_mask(EventMask::StructureNotify), + )? + .check()?; Ok(id) } diff --git a/druid-shell/src/platform/x11/window.rs b/druid-shell/src/platform/x11/window.rs index aa475dd7c6..24aa67fdc7 100644 --- a/druid-shell/src/platform/x11/window.rs +++ b/druid-shell/src/platform/x11/window.rs @@ -21,9 +21,10 @@ use std::rc::{Rc, Weak}; use anyhow::{anyhow, Context, Error}; use cairo::{XCBConnection, XCBDrawable, XCBSurface, XCBVisualType}; +use x11rb::atom_manager; use x11rb::connection::Connection; use x11rb::protocol::xproto::{ - self, Atom, AtomEnum, ConnectionExt, EventMask, ExposeEvent, PropMode, Visualtype, WindowClass, + self, AtomEnum, ConnectionExt, EventMask, ExposeEvent, PropMode, Visualtype, WindowClass, }; use x11rb::wrapper::ConnectionExt as WrapperConnectionExt; @@ -127,51 +128,23 @@ impl WindowBuilder { /// Registers and returns all the atoms that the window will need. fn atoms(&self, window_id: u32) -> Result { let conn = self.app.connection(); - - // WM_PROTOCOLS - // - // List of atoms that identify the communications protocols between - // the client and window manager in which the client is willing to participate. - // - // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#wm_protocols_property - let wm_protocols = conn - .intern_atom(false, b"WM_PROTOCOLS") - .context("failed to get WM_PROTOCOLS")? - .reply() - .context("failed to get WM_PROTOCOLS reply")? - .atom; - - // WM_DELETE_WINDOW - // - // Including this atom in the WM_PROTOCOLS property on each window makes sure that - // if the window manager respects WM_DELETE_WINDOW it will send us the event. - // - // The WM_DELETE_WINDOW event is sent when there is a request to close the window. - // Registering for but ignoring this event means that the window will remain open. - // - // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#window_deletion - let wm_delete_window = conn - .intern_atom(false, b"WM_DELETE_WINDOW") - .context("failed to get WM_DELETE_WINDOW")? + let atoms = WindowAtoms::new(conn.as_ref()) + .context("failed to intern X11 atoms")? .reply() - .context("failed to get WM_DELETE_WINDOW reply")? - .atom; + .context("failed to get back X11 atoms")?; // Replace the window's WM_PROTOCOLS with the following. - let protocols = [wm_delete_window]; + let protocols = [atoms.WM_DELETE_WINDOW]; // TODO(x11/errors): Check the response for errors? conn.change_property32( PropMode::Replace, window_id, - wm_protocols, + atoms.WM_PROTOCOLS, AtomEnum::ATOM, &protocols, )?; - Ok(WindowAtoms { - wm_protocols, - wm_delete_window, - }) + Ok(atoms) } /// Create a new cairo `Context`. @@ -229,40 +202,34 @@ impl WindowBuilder { ); // Create the actual window - // TODO: https://github.com/psychon/x11rb/pull/469 will make error handling easier with the - // next x11rb release. - if let Some(err) = conn - .create_window( - // Window depth - x11rb::COPY_FROM_PARENT.try_into().unwrap(), - // The new window's ID - id, - // Parent window of this new window - // TODO(#468): either `screen.root()` (no parent window) or pass parent here to attach - screen.root, - // X-coordinate of the new window - 0, - // Y-coordinate of the new window - 0, - // Width of the new window - // TODO(x11/dpi_scaling): figure out DPI scaling - self.size.width as u16, - // Height of the new window - // TODO(x11/dpi_scaling): figure out DPI scaling - self.size.height as u16, - // Border width - 0, - // Window class type - WindowClass::InputOutput, - // Visual ID - visual_id, - // Window properties mask - &cw_values, - )? - .check()? - { - return Err(x11rb::errors::ReplyError::X11Error(err).into()); - } + conn.create_window( + // Window depth + x11rb::COPY_FROM_PARENT.try_into().unwrap(), + // The new window's ID + id, + // Parent window of this new window + // TODO(#468): either `screen.root()` (no parent window) or pass parent here to attach + screen.root, + // X-coordinate of the new window + 0, + // Y-coordinate of the new window + 0, + // Width of the new window + // TODO(x11/dpi_scaling): figure out DPI scaling + self.size.width as u16, + // Height of the new window + // TODO(x11/dpi_scaling): figure out DPI scaling + self.size.height as u16, + // Border width + 0, + // Window class type + WindowClass::InputOutput, + // Visual ID + visual_id, + // Window properties mask + &cw_values, + )? + .check()?; // TODO(x11/errors): Should do proper cleanup (window destruction etc) in case of error let atoms = self.atoms(id)?; @@ -303,10 +270,30 @@ pub(crate) struct Window { state: RefCell, } -/// All the Atom references the window needs. -struct WindowAtoms { - wm_protocols: Atom, - wm_delete_window: Atom, +// This creates a `struct WindowAtoms` containing the specified atoms as members (along with some +// convenience methods to intern and query those atoms). We use the following atoms: +// +// WM_PROTOCOLS +// +// List of atoms that identify the communications protocols between +// the client and window manager in which the client is willing to participate. +// +// https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#wm_protocols_property +// +// WM_DELETE_WINDOW +// +// Including this atom in the WM_PROTOCOLS property on each window makes sure that +// if the window manager respects WM_DELETE_WINDOW it will send us the event. +// +// The WM_DELETE_WINDOW event is sent when there is a request to close the window. +// Registering for but ignoring this event means that the window will remain open. +// +// https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#window_deletion +atom_manager! { + WindowAtoms: WindowAtomsCookie { + WM_PROTOCOLS, + WM_DELETE_WINDOW, + } } /// The mutable state of the window. @@ -633,9 +620,9 @@ impl Window { ) -> Result<(), Error> { // https://www.x.org/releases/X11R7.7/doc/libX11/libX11/libX11.html#id2745388 // https://www.x.org/releases/X11R7.6/doc/xorg-docs/specs/ICCCM/icccm.html#window_deletion - if client_message.type_ == self.atoms.wm_protocols && client_message.format == 32 { + if client_message.type_ == self.atoms.WM_PROTOCOLS && client_message.format == 32 { let protocol = client_message.data.as_data32()[0]; - if protocol == self.atoms.wm_delete_window { + if protocol == self.atoms.WM_DELETE_WINDOW { self.close(); } } From b0d3afdb79fef0b2cfcd1fd3ac233791d361edfb Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Fri, 19 Jun 2020 14:20:46 -0500 Subject: [PATCH 5/5] Fix broken link. --- druid-shell/src/platform/x11/application.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/druid-shell/src/platform/x11/application.rs b/druid-shell/src/platform/x11/application.rs index 27fcc1a9e4..a7e30d78c0 100644 --- a/druid-shell/src/platform/x11/application.rs +++ b/druid-shell/src/platform/x11/application.rs @@ -79,7 +79,7 @@ impl Application { // libxcb, you should call XSetEventQueueOwner(dpy, XCBOwnsEventQueue). Otherwise, libX11 // might randomly eat your events / move them to its own event queue. // - // https://github.com/xi-editor/druid/pull/1025/files/76b923417183bd103f61e56b56a56474b7417cec#r442777892 + // https://github.com/xi-editor/druid/pull/1025#discussion_r442777892 let (conn, screen_num) = XCBConnection::connect(None)?; let connection = Rc::new(conn); let window_id = Application::create_event_window(&connection, screen_num as i32)?;