linebender · maan2003 · Jul 10, 2021 · Jul 9, 2021 · Jul 9, 2021 · Jul 9, 2021
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 
@@ -746,6 +747,7 @@ Last release without a changelog :(
 [#1851]: https://github.com/linebender/druid/pull/1851
 [#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

diff --git a/druid-shell/src/backend/x11/application.rs b/druid-shell/src/backend/x11/application.rs
@@ -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.
@@ -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>,
@@ -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
@@ -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,
@@ -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,
@@ -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() {

diff --git a/druid-shell/src/backend/x11/clipboard.rs b/druid-shell/src/backend/x11/clipboard.rs
@@ -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};
 
@@ -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> {
@@ -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>,
@@ -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> {

diff --git a/druid-shell/src/backend/x11/window.rs b/druid-shell/src/backend/x11/window.rs
@@ -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};
@@ -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,
@@ -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),
@@ -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(
@@ -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)
@@ -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,
@@ -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
@@ -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
@@ -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).
@@ -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(),
         ));
     }
@@ -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());
             }
         }