From d1902aa15a50c4a45b8d709384770c3c070d06b8 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Sat, 10 Feb 2024 00:24:03 +0400 Subject: [PATCH] On X11, don't require XSETTINGS We could fail to setup property watcher and fail to start, thus don't require XSETTINGS to work. Fixes: df8805c0 (On X11, reload DPI on _XSETTINGS_SETTINGS) --- src/platform_impl/linux/x11/util/randr.rs | 12 ++-- src/platform_impl/linux/x11/xdisplay.rs | 73 +++++++++++++---------- src/platform_impl/linux/x11/xsettings.rs | 15 +++-- 3 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/platform_impl/linux/x11/util/randr.rs b/src/platform_impl/linux/x11/util/randr.rs index 12297f32..e179e38b 100644 --- a/src/platform_impl/linux/x11/util/randr.rs +++ b/src/platform_impl/linux/x11/util/randr.rs @@ -39,11 +39,13 @@ impl XConnection { // Retrieve DPI from Xft.dpi property pub fn get_xft_dpi(&self) -> Option { // Try to get it from XSETTINGS first. - match self.xsettings_dpi() { - Ok(Some(dpi)) => return Some(dpi), - Ok(None) => {} - Err(err) => { - log::warn!("failed to fetch XSettings: {err}"); + if let Some(xsettings_screen) = self.xsettings_screen() { + match self.xsettings_dpi(xsettings_screen) { + Ok(Some(dpi)) => return Some(dpi), + Ok(None) => {} + Err(err) => { + log::warn!("failed to fetch XSettings: {err}"); + } } } diff --git a/src/platform_impl/linux/x11/xdisplay.rs b/src/platform_impl/linux/x11/xdisplay.rs index e18ac8a4..0fc87d4d 100644 --- a/src/platform_impl/linux/x11/xdisplay.rs +++ b/src/platform_impl/linux/x11/xdisplay.rs @@ -59,7 +59,7 @@ pub(crate) struct XConnection { randr_version: (u32, u32), /// Atom for the XSettings screen. - xsettings_screen: xproto::Atom, + xsettings_screen: Option, pub latest_error: Mutex>, pub cursor_cache: Mutex, ffi::Cursor>>, @@ -108,21 +108,6 @@ impl XConnection { // Get the default screen. let default_screen = unsafe { (xlib.XDefaultScreen)(display) } as usize; - // Fetch the _XSETTINGS_S[screen number] atom. - let xsettings_screen = xcb - .intern_atom(false, format!("_XSETTINGS_S{}", default_screen).as_bytes()) - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; - - // Fetch the other atoms. - let atoms = Atoms::new(&xcb) - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? - .reply() - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; - let xsettings_screen = xsettings_screen - .reply() - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? - .atom; - // Load the database. let database = resource_manager::new_from_default(&xcb) .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; @@ -134,23 +119,16 @@ impl XConnection { .reply() .expect("failed to query XRandR version"); - // Get PropertyNotify events from the XSETTINGS window. - // TODO: The XSETTINGS window here can change. In the future, listen for DestroyNotify on this window - // in order to accomodate for a changed window here. - let selector_window = xcb - .get_selection_owner(xsettings_screen) + let xsettings_screen = Self::new_xsettings_screen(&xcb, default_screen); + if xsettings_screen.is_none() { + log::warn!("error setting XSETTINGS; Xft options won't reload automatically") + } + + // Fetch atoms. + let atoms = Atoms::new(&xcb) .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? .reply() - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? - .owner; - xcb.change_window_attributes( - selector_window, - &xproto::ChangeWindowAttributesAux::new() - .event_mask(xproto::EventMask::PROPERTY_CHANGE), - ) - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? - .check() - .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; + .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; Ok(XConnection { xlib, @@ -170,6 +148,37 @@ impl XConnection { }) } + fn new_xsettings_screen(xcb: &XCBConnection, default_screen: usize) -> Option { + // Fetch the _XSETTINGS_S[screen number] atom. + let xsettings_screen = xcb + .intern_atom(false, format!("_XSETTINGS_S{}", default_screen).as_bytes()) + .ok()? + .reply() + .ok()? + .atom; + + // Get PropertyNotify events from the XSETTINGS window. + // TODO: The XSETTINGS window here can change. In the future, listen for DestroyNotify on this window + // in order to accomodate for a changed window here. + let selector_window = xcb + .get_selection_owner(xsettings_screen) + .ok()? + .reply() + .ok()? + .owner; + + xcb.change_window_attributes( + selector_window, + &xproto::ChangeWindowAttributesAux::new() + .event_mask(xproto::EventMask::PROPERTY_CHANGE), + ) + .ok()? + .check() + .ok()?; + + Some(xsettings_screen) + } + /// Checks whether an error has been triggered by the previous function calls. #[inline] pub fn check_errors(&self) -> Result<(), XError> { @@ -258,7 +267,7 @@ impl XConnection { /// Get the atom for Xsettings. #[inline] - pub fn xsettings_screen(&self) -> u32 { + pub fn xsettings_screen(&self) -> Option { self.xsettings_screen } } diff --git a/src/platform_impl/linux/x11/xsettings.rs b/src/platform_impl/linux/x11/xsettings.rs index 15674084..a96d85fb 100644 --- a/src/platform_impl/linux/x11/xsettings.rs +++ b/src/platform_impl/linux/x11/xsettings.rs @@ -4,13 +4,13 @@ //! //! [here]: https://github.com/derat/xsettingsd -use super::{atoms::*, XConnection}; - -use x11rb::protocol::xproto::ConnectionExt; - use std::iter; use std::num::NonZeroUsize; +use x11rb::protocol::xproto::{self, ConnectionExt}; + +use super::{atoms::*, XConnection}; + type Result = core::result::Result; const DPI_NAME: &[u8] = b"Xft/DPI"; @@ -20,13 +20,16 @@ const BIG_ENDIAN: u8 = b'B'; impl XConnection { /// Get the DPI from XSettings. - pub(crate) fn xsettings_dpi(&self) -> core::result::Result, super::X11Error> { + pub(crate) fn xsettings_dpi( + &self, + xsettings_screen: xproto::Atom, + ) -> core::result::Result, super::X11Error> { let atoms = self.atoms(); // Get the current owner of the screen's settings. let owner = self .xcb_connection() - .get_selection_owner(self.xsettings_screen())? + .get_selection_owner(xsettings_screen)? .reply()?; // Read the _XSETTINGS_SETTINGS property.