Skip to content

Commit

Permalink
x11: Use only a single atom manager (#1865)
Browse files Browse the repository at this point in the history 
* x11: Move atom manager to Application

Atoms are a global resource in X11, so it is unnecessary for every
window to query them again. This commit moves the atom manager from
Window to Application.

Signed-off-by: Uli Schlachter <[email protected]>

* x11: Merge clipboard atoms with app atoms

There is no point in keeping two separate atom managers around. This
commit just merges everything into one and thus avoids one round-trip to
the X11 server during startup.

Signed-off-by: Uli Schlachter <[email protected]>

* Update changelog

Signed-off-by: Uli Schlachter <[email protected]>
  • Loading branch information
@psychon
psychon authored Jul 10, 2021
1 parent f35bec8 commit 2b20c8c
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 97 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ You can find its changes [documented below](#070---2021-01-01).
- Update look and feel of controls when disabled ([#1717] by [@xarvic])
- Change the signature of `add_idle_callback` ([#1787] by [@jneem])
- Move macOS only function to Mac extension trait ([#1863] by [@Maan2003])
- x11: Only query atoms once instead of per window ([#1865] by [@psychon])

### Deprecated

Expand Down Expand Up @@ -749,6 +750,7 @@ Last release without a changelog :(
[#1860]: https://github.com/linebender/druid/pull/1860
[#1861]: https://github.com/linebender/druid/pull/1861
[#1863]: https://github.com/linebender/druid/pull/1863
[#1865]: https://github.com/linebender/druid/pull/1865

[Unreleased]: https://github.com/linebender/druid/compare/v0.7.0...master
[0.7.0]: https://github.com/linebender/druid/compare/v0.6.0...v0.7.0
Expand Down
82 changes: 81 additions & 1 deletion druid-shell/src/backend/x11/application.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,71 @@ use super::clipboard::Clipboard;
use super::util;
use super::window::Window;

// 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
//
// _NET_WM_PID
//
// A property containing the PID of the process that created the window.
//
// https://specifications.freedesktop.org/wm-spec/wm-spec-1.3.html#idm45805407915360
//
// _NET_WM_NAME
//
// A version of WM_NAME supporting UTF8 text.
//
// https://specifications.freedesktop.org/wm-spec/wm-spec-1.3.html#idm45805407982336
//
// UTF8_STRING
//
// The type of _NET_WM_NAME
//
// CLIPBOARD
//
// The name of the clipboard selection; used for implementing copy&paste
//
// TARGETS
//
// A target for getting the selection contents that answers with a list of supported targets
//
// INCR
//
// Type used for incremental selection transfers
x11rb::atom_manager! {
pub(crate) AppAtoms: AppAtomsCookie {
WM_PROTOCOLS,
WM_DELETE_WINDOW,
_NET_WM_PID,
_NET_WM_NAME,
UTF8_STRING,
_NET_WM_WINDOW_TYPE,
_NET_WM_WINDOW_TYPE_NORMAL,
_NET_WM_WINDOW_TYPE_DROPDOWN_MENU,
_NET_WM_WINDOW_TYPE_TOOLTIP,
_NET_WM_WINDOW_TYPE_DIALOG,
CLIPBOARD,
TARGETS,
INCR,
}
}

#[derive(Clone)]
pub(crate) struct Application {
/// The connection to the X server.
Expand All @@ -62,6 +127,8 @@ pub(crate) struct Application {
argb_visual_type: Option<Visualtype>,
/// Pending events that need to be handled later
pending_events: Rc<RefCell<VecDeque<Event>>>,
/// The atoms that we need
atoms: Rc<AppAtoms>,

/// The X11 resource database used to query dpi.
pub(crate) rdb: Rc<ResourceDb>,
Expand Down Expand Up @@ -204,6 +271,12 @@ impl Application {
col_resize: load_cursor("col-resize"),
};

let atoms = Rc::new(
AppAtoms::new(&*connection)?
.reply()
.context("get X11 atoms")?,
);

let screen = connection
.setup()
.roots
Expand All @@ -218,9 +291,10 @@ impl Application {
let clipboard = Clipboard::new(
Rc::clone(&connection),
screen_num,
Rc::clone(&atoms),
Rc::clone(&pending_events),
Rc::clone(&timestamp),
)?;
);

Ok(Application {
connection,
Expand All @@ -235,6 +309,7 @@ impl Application {
present_opcode,
root_visual_type,
argb_visual_type,
atoms,
pending_events: Default::default(),
marker: std::marker::PhantomData,
render_argb32_pictformat_cursor,
Expand Down Expand Up @@ -391,6 +466,11 @@ impl Application {
self.root_visual_type
}

#[inline]
pub(crate) fn atoms(&self) -> &AppAtoms {
&*self.atoms
}

/// Returns `Ok(true)` if we want to exit the main loop.
fn handle_event(&self, ev: &Event) -> Result<bool, Error> {
if ev.server_generated() {
Expand Down
27 changes: 11 additions & 16 deletions druid-shell/src/backend/x11/clipboard.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use x11rb::protocol::Event;
use x11rb::wrapper::ConnectionExt as _;
use x11rb::xcb_ffi::XCBConnection;

use super::application::AppAtoms;
use crate::clipboard::{ClipboardFormat, FormatId};
use tracing::{debug, error, warn};

Expand All @@ -45,30 +46,24 @@ const STRING_TARGETS: [&str; 5] = [
"text/plain",
];

x11rb::atom_manager! {
ClipboardAtoms: ClipboardAtomsCookie {
CLIPBOARD,
TARGETS,
INCR,
}
}

#[derive(Debug, Clone)]
pub struct Clipboard(Rc<RefCell<ClipboardState>>);

impl Clipboard {
pub(crate) fn new(
connection: Rc<XCBConnection>,
screen_num: usize,
atoms: Rc<AppAtoms>,
event_queue: Rc<RefCell<VecDeque<Event>>>,
timestamp: Rc<Cell<Timestamp>>,
) -> Result<Self, ReplyOrIdError> {
Ok(Self(Rc::new(RefCell::new(ClipboardState::new(
) -> Self {
Self(Rc::new(RefCell::new(ClipboardState::new(
connection,
screen_num,
atoms,
event_queue,
timestamp,
)?))))
))))
}

pub(crate) fn handle_clear(&self, event: SelectionClearEvent) -> Result<(), ConnectionError> {
Expand Down Expand Up @@ -125,7 +120,7 @@ impl Clipboard {
struct ClipboardState {
connection: Rc<XCBConnection>,
screen_num: usize,
atoms: ClipboardAtoms,
atoms: Rc<AppAtoms>,
event_queue: Rc<RefCell<VecDeque<Event>>>,
timestamp: Rc<Cell<Timestamp>>,
contents: Option<ClipboardContents>,
Expand All @@ -136,19 +131,19 @@ impl ClipboardState {
fn new(
connection: Rc<XCBConnection>,
screen_num: usize,
atoms: Rc<AppAtoms>,
event_queue: Rc<RefCell<VecDeque<Event>>>,
timestamp: Rc<Cell<Timestamp>>,
) -> Result<Self, ReplyOrIdError> {
let atoms = ClipboardAtoms::new(&*connection)?.reply()?;
Ok(Self {
) -> Self {
Self {
connection,
screen_num,
atoms,
event_queue,
timestamp,
contents: None,
incremental: Vec::new(),
})
}
}

fn put_formats(&mut self, formats: &[ClipboardFormat]) -> Result<(), ReplyOrIdError> {
Expand Down
100 changes: 20 additions & 80 deletions druid-shell/src/backend/x11/window.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use crate::scale::Scalable;
use anyhow::{anyhow, Context, Error};
use cairo::{XCBConnection as CairoXCBConnection, XCBDrawable, XCBSurface, XCBVisualType};
use tracing::{error, info, warn};
use x11rb::atom_manager;
use x11rb::connection::Connection;
use x11rb::errors::ReplyOrIdError;
use x11rb::properties::{WmHints, WmHintsState, WmSizeHints};
Expand Down Expand Up @@ -190,28 +189,6 @@ impl WindowBuilder {
// TODO(x11/menus): implement WindowBuilder::set_menu (currently a no-op)
}

/// Registers and returns all the atoms that the window will need.
fn atoms(&self, window_id: u32) -> Result<WindowAtoms, Error> {
let conn = self.app.connection();
let atoms = WindowAtoms::new(conn.as_ref())?
.reply()
.context("get X11 atoms")?;

// Replace the window's WM_PROTOCOLS with the following.
let protocols = [atoms.WM_DELETE_WINDOW];
conn.change_property32(
PropMode::REPLACE,
window_id,
atoms.WM_PROTOCOLS,
AtomEnum::ATOM,
&protocols,
)?
.check()
.context("set WM_PROTOCOLS")?;

Ok(atoms)
}

fn create_cairo_surface(
&self,
window_id: u32,
Expand Down Expand Up @@ -353,7 +330,6 @@ impl WindowBuilder {
.context("create graphics context")?;

// TODO(x11/errors): Should do proper cleanup (window destruction etc) in case of error
let atoms = self.atoms(id)?;
let cairo_surface = RefCell::new(self.create_cairo_surface(id, &visual_type)?);
let present_data = match self.initialize_present_data(id) {
Ok(p) => Some(p),
Expand All @@ -372,6 +348,7 @@ impl WindowBuilder {
)?);

// Initialize some properties
let atoms = self.app.atoms();
let pid = nix::unistd::Pid::this().as_raw();
if let Ok(pid) = u32::try_from(pid) {
conn.change_property32(
Expand All @@ -385,6 +362,18 @@ impl WindowBuilder {
.context("set _NET_WM_PID")?;
}

// Replace the window's WM_PROTOCOLS with the following.
let protocols = [atoms.WM_DELETE_WINDOW];
conn.change_property32(
PropMode::REPLACE,
id,
atoms.WM_PROTOCOLS,
AtomEnum::ATOM,
&protocols,
)?
.check()
.context("set WM_PROTOCOLS")?;

let min_size = self.min_size.to_px(scale);
log_x11!(size_hints(self.resizable, size_px, min_size)
.set_normal_hints(conn.as_ref(), id)
Expand Down Expand Up @@ -435,7 +424,6 @@ impl WindowBuilder {
app: self.app.clone(),
handler,
cairo_surface,
atoms,
area: Cell::new(ScaledArea::from_px(size_px, scale)),
scale: Cell::new(scale),
min_size,
Expand Down Expand Up @@ -519,7 +507,6 @@ pub(crate) struct Window {
app: Application,
handler: RefCell<Box<dyn WinHandler>>,
cairo_surface: RefCell<XCBSurface>,
atoms: WindowAtoms,
area: Cell<ScaledArea>,
scale: Cell<Scale>,
// min size in px
Expand Down Expand Up @@ -568,56 +555,6 @@ pub(crate) struct Window {
active_text_field: Cell<Option<TextFieldToken>>,
}

// 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
//
// _NET_WM_PID
//
// A property containing the PID of the process that created the window.
//
// https://specifications.freedesktop.org/wm-spec/wm-spec-1.3.html#idm45805407915360
//
// _NET_WM_NAME
//
// A version of WM_NAME supporting UTF8 text.
//
// https://specifications.freedesktop.org/wm-spec/wm-spec-1.3.html#idm45805407982336
//
// UTF8_STRING
//
// The type of _NET_WM_NAME
atom_manager! {
WindowAtoms: WindowAtomsCookie {
WM_PROTOCOLS,
WM_DELETE_WINDOW,
_NET_WM_PID,
_NET_WM_NAME,
UTF8_STRING,
_NET_WM_WINDOW_TYPE,
_NET_WM_WINDOW_TYPE_NORMAL,
_NET_WM_WINDOW_TYPE_DROPDOWN_MENU,
_NET_WM_WINDOW_TYPE_TOOLTIP,
_NET_WM_WINDOW_TYPE_DIALOG,
}
}

/// A collection of pixmaps for rendering to. This gets used in two different ways: if the present
/// extension is enabled, we render to a pixmap and then present it. If the present extension is
/// disabled, we render to a pixmap and then call `copy_area` on it (this probably isn't the best
Expand Down Expand Up @@ -995,6 +932,8 @@ impl Window {
return;
}

let atoms = self.app.atoms();

// This is technically incorrect. STRING encoding is *not* UTF8. However, I am not sure
// what it really is. WM_LOCALE_NAME might be involved. Hopefully, nothing cares about this
// as long as _NET_WM_NAME is also set (which uses UTF8).
Expand All @@ -1008,8 +947,8 @@ impl Window {
log_x11!(self.app.connection().change_property8(
xproto::PropMode::REPLACE,
self.id,
self.atoms._NET_WM_NAME,
self.atoms.UTF8_STRING,
atoms._NET_WM_NAME,
atoms.UTF8_STRING,
title.as_bytes(),
));
}
Expand Down Expand Up @@ -1185,9 +1124,10 @@ impl Window {
pub fn handle_client_message(&self, client_message: &xproto::ClientMessageEvent) {
// 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 atoms = self.app.atoms();
if client_message.type_ == atoms.WM_PROTOCOLS && client_message.format == 32 {
let protocol = client_message.data.as_data32()[0];
if protocol == self.atoms.WM_DELETE_WINDOW {
if protocol == atoms.WM_DELETE_WINDOW {
self.with_handler(|h| h.request_close());
}
}
Expand Down

0 comments on commit 2b20c8c

Please sign in to comment.