On X11, reload DPI on _XSETTINGS_SETTINGS

This also fixes the deadlock when such reload may happen.

Fixes: #3383
Signed-off-by: John Nunley <dev@notgull.net>
Signed-off-by: Kirill Chibisov <contact@kchibisov.com>

CHANGELOG.md

@@ -43,6 +43,8 @@ Unreleased` header.
 - **Breaking:** Changed the signature of `EventLoop::with_user_event` to return a builder.
 - **Breaking:** Removed `EventLoopBuilder::with_user_event`, the functionality is now available in `EventLoop::with_user_event`.
 - Add `Window::builder`, which is intended to replace the (now deprecated) `WindowBuilder::new`.
+- On X11, reload dpi on `_XSETTINGS_SETTINGS` update.
+- On X11, fix deadlock when adjusting DPI and resizing at the same time.
 
 # 0.29.10
 

src/platform_impl/linux/x11/atoms.rs

@@ -100,7 +100,8 @@ atom_manager! {
     _NET_FRAME_EXTENTS,
     _NET_SUPPORTED,
     _NET_SUPPORTING_WM_CHECK,
-    _XEMBED
+    _XEMBED,
+    _XSETTINGS_SETTINGS
 }
 
 impl Index<AtomName> for Atoms {

src/platform_impl/linux/x11/event_processor.rs

@@ -485,24 +485,34 @@ impl EventProcessor {
                         }
                     }
 
-                    let mut shared_state_lock = window.shared_state_lock();
+                    // NOTE: Ensure that the lock is dropped before handling the resized and
-                    let hittest = shared_state_lock.cursor_hittest;
+                    // sending the event back to user.
+                    let hittest = {
+                        let mut shared_state_lock = window.shared_state_lock();
+                        let hittest = shared_state_lock.cursor_hittest;
 
-                    // This is a hack to ensure that the DPI adjusted resize is actually applied on all WMs. KWin
+                        // This is a hack to ensure that the DPI adjusted resize is actually
-                    // doesn't need this, but Xfwm does. The hack should not be run on other WMs, since tiling
+                        // applied on all WMs. KWin doesn't need this, but Xfwm does. The hack
-                    // WMs constrain the window size, making the resize fail. This would cause an endless stream of
+                        // should not be run on other WMs, since tiling WMs constrain the window
-                    // XResizeWindow requests, making Xorg, the winit client, and the WM consume 100% of CPU.
+                        // size, making the resize fail. This would cause an endless stream of
-                    if let Some(adjusted_size) = shared_state_lock.dpi_adjusted {
+                        // XResizeWindow requests, making Xorg, the winit client, and the WM
-                        if new_inner_size == adjusted_size || !util::wm_name_is_one_of(&["Xfwm4"]) {
+                        // consume 100% of CPU.
-                            // When this finally happens, the event will not be synthetic.
+                        if let Some(adjusted_size) = shared_state_lock.dpi_adjusted {
-                            shared_state_lock.dpi_adjusted = None;
+                            if new_inner_size == adjusted_size
-                        } else {
+                                || !util::wm_name_is_one_of(&["Xfwm4"])
-                            window.request_inner_size_physical(adjusted_size.0, adjusted_size.1);
+                            {
+                                // When this finally happens, the event will not be synthetic.
+                                shared_state_lock.dpi_adjusted = None;
+                            } else {
+                                // Unlock shared state to prevent deadlock in callback below
+                                drop(shared_state_lock);
+                                window
+                                    .request_inner_size_physical(adjusted_size.0, adjusted_size.1);
+                            }
                         }
-                    }
 
-                    // Unlock shared state to prevent deadlock in callback below
+                        hittest
-                    drop(shared_state_lock);
+                    };
 
                     // Reload hittest.
                     if hittest.unwrap_or(false) {
@@ -576,7 +586,9 @@ impl EventProcessor {
                 let xev: &ffi::XPropertyEvent = xev.as_ref();
                 let atom = xev.atom as xproto::Atom;
 
-                if atom == xproto::Atom::from(xproto::AtomEnum::RESOURCE_MANAGER) {
+                if atom == xproto::Atom::from(xproto::AtomEnum::RESOURCE_MANAGER)
+                    || atom == atoms[_XSETTINGS_SETTINGS]
+                {
                     self.process_dpi_change(&mut callback);
                 }
             }

src/platform_impl/linux/x11/mod.rs

@@ -10,6 +10,7 @@ mod monitor;
 pub mod util;
 mod window;
 mod xdisplay;
+mod xsettings;
 
 pub(crate) use self::{
     monitor::{MonitorHandle, VideoModeHandle},
@@ -889,6 +890,9 @@ pub enum X11Error {
 
     /// Could not find a matching X11 visual for this visualid
     NoSuchVisual(xproto::Visualid),
+
+    /// Unable to parse xsettings.
+    XsettingsParse(xsettings::ParserError),
 }
 
 impl fmt::Display for X11Error {
@@ -913,6 +917,9 @@ impl fmt::Display for X11Error {
                     visualid
                 )
             }
+            X11Error::XsettingsParse(err) => {
+                write!(f, "Failed to parse xsettings: {:?}", err)
+            }
         }
     }
 }
@@ -981,6 +988,12 @@ impl From<ReplyOrIdError> for X11Error {
     }
 }
 
+impl From<xsettings::ParserError> for X11Error {
+    fn from(value: xsettings::ParserError) -> Self {
+        Self::XsettingsParse(value)
+    }
+}
+
 /// The underlying x11rb connection that we are using.
 type X11rbConnection = x11rb::xcb_ffi::XCBConnection;
 

src/platform_impl/linux/x11/util/randr.rs

@@ -38,6 +38,15 @@ pub fn calc_dpi_factor(
 impl XConnection {
     // Retrieve DPI from Xft.dpi property
     pub fn get_xft_dpi(&self) -> Option<f64> {
+        // 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}");
+            }
+        }
+
         self.database()
             .get_string("Xft.dpi", "")
             .and_then(|s| f64::from_str(s).ok())

src/platform_impl/linux/x11/xdisplay.rs

@@ -13,7 +13,10 @@ use crate::window::CursorIcon;
 use super::{atoms::Atoms, ffi, monitor::MonitorHandle};
 use x11rb::{
     connection::Connection,
-    protocol::{randr::ConnectionExt as _, xproto},
+    protocol::{
+        randr::ConnectionExt as _,
+        xproto::{self, ConnectionExt},
+    },
     resource_manager,
     xcb_ffi::XCBConnection,
 };
@@ -55,6 +58,9 @@ pub(crate) struct XConnection {
     /// RandR version.
     randr_version: (u32, u32),
 
+    /// Atom for the XSettings screen.
+    xsettings_screen: xproto::Atom,
+
     pub latest_error: Mutex<Option<XError>>,
     pub cursor_cache: Mutex<HashMap<Option<CursorIcon>, ffi::Cursor>>,
 }
@@ -102,11 +108,20 @@ impl XConnection {
         // Get the default screen.
         let default_screen = unsafe { (xlib.XDefaultScreen)(display) } as usize;
 
-        // Fetch the atoms.
+        // 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)
@@ -119,6 +134,24 @@ 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)
+            .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)))?;
+
         Ok(XConnection {
             xlib,
             xcursor,
@@ -133,6 +166,7 @@ impl XConnection {
             database: RwLock::new(database),
             cursor_cache: Default::default(),
             randr_version: (randr_version.major_version, randr_version.minor_version),
+            xsettings_screen,
         })
     }
 
@@ -221,6 +255,12 @@ impl XConnection {
             }
         }
     }
+
+    /// Get the atom for Xsettings.
+    #[inline]
+    pub fn xsettings_screen(&self) -> u32 {
+        self.xsettings_screen
+    }
 }
 
 impl fmt::Debug for XConnection {

src/platform_impl/linux/x11/xsettings.rs

@@ -0,0 +1,342 @@
+//! Parser for the xsettings data format.
+//!
+//! Some of this code is referenced from [here].
+//!
+//! [here]: https://github.com/derat/xsettingsd
+
+use super::{atoms::*, XConnection};
+
+use x11rb::protocol::xproto::ConnectionExt;
+
+use std::iter;
+use std::num::NonZeroUsize;
+
+type Result<T> = core::result::Result<T, ParserError>;
+
+const DPI_NAME: &[u8] = b"Xft/DPI";
+const DPI_MULTIPLIER: f64 = 1024.0;
+const LITTLE_ENDIAN: u8 = b'l';
+const BIG_ENDIAN: u8 = b'B';
+
+impl XConnection {
+    /// Get the DPI from XSettings.
+    pub(crate) fn xsettings_dpi(&self) -> core::result::Result<Option<f64>, 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())?
+            .reply()?;
+
+        // Read the _XSETTINGS_SETTINGS property.
+        let data: Vec<u8> = self
+            .get_property(
+                owner.owner,
+                atoms[_XSETTINGS_SETTINGS],
+                atoms[_XSETTINGS_SETTINGS],
+            )
+            .unwrap();
+
+        // Parse the property.
+        let dpi_setting = read_settings(&data)?
+            .find(|res| res.as_ref().map_or(true, |s| s.name == DPI_NAME))
+            .transpose()?;
+        if let Some(dpi_setting) = dpi_setting {
+            let base_dpi = match dpi_setting.data {
+                SettingData::Integer(dpi) => dpi as f64,
+                SettingData::String(_) => {
+                    return Err(ParserError::BadType(SettingType::String).into())
+                }
+                SettingData::Color(_) => {
+                    return Err(ParserError::BadType(SettingType::Color).into())
+                }
+            };
+
+            Ok(Some(base_dpi / DPI_MULTIPLIER))
+        } else {
+            Ok(None)
+        }
+    }
+}
+
+/// Read over the settings in the block of data.
+fn read_settings(data: &[u8]) -> Result<impl Iterator<Item = Result<Setting<'_>>> + '_> {
+    // Create a parser. This automatically parses the first 8 bytes for metadata.
+    let mut parser = Parser::new(data)?;
+
+    // Read the total number of settings.
+    let total_settings = parser.i32()?;
+
+    // Iterate over the settings.
+    let iter = iter::repeat_with(move || Setting::parse(&mut parser)).take(total_settings as usize);
+    Ok(iter)
+}
+
+/// A setting in the settings list.
+struct Setting<'a> {
+    /// The name of the setting.
+    name: &'a [u8],
+
+    /// The data contained in the setting.
+    data: SettingData<'a>,
+}
+
+/// The data contained in a setting.
+enum SettingData<'a> {
+    Integer(i32),
+    String(#[allow(dead_code)] &'a [u8]),
+    Color(#[allow(dead_code)] [i16; 4]),
+}
+
+impl<'a> Setting<'a> {
+    /// Parse a new `SettingData`.
+    fn parse(parser: &mut Parser<'a>) -> Result<Self> {
+        // Read the type.
+        let ty: SettingType = parser.i8()?.try_into()?;
+
+        // Read another byte of padding.
+        parser.advance(1)?;
+
+        // Read the name of the setting.
+        let name_len = parser.i16()?;
+        let name = parser.advance(name_len as usize)?;
+        parser.pad(name.len(), 4)?;
+
+        // Ignore the serial number.
+        parser.advance(4)?;
+
+        let data = match ty {
+            SettingType::Integer => {
+                // Read a 32-bit integer.
+                SettingData::Integer(parser.i32()?)
+            }
+
+            SettingType::String => {
+                // Read the data.
+                let data_len = parser.i32()?;
+                let data = parser.advance(data_len as usize)?;
+                parser.pad(data.len(), 4)?;
+
+                SettingData::String(data)
+            }
+
+            SettingType::Color => {
+                // Read i16's of color.
+                let (red, blue, green, alpha) =
+                    (parser.i16()?, parser.i16()?, parser.i16()?, parser.i16()?);
+
+                SettingData::Color([red, blue, green, alpha])
+            }
+        };
+
+        Ok(Setting { name, data })
+    }
+}
+
+#[derive(Debug)]
+pub enum SettingType {
+    Integer = 0,
+    String = 1,
+    Color = 2,
+}
+
+impl TryFrom<i8> for SettingType {
+    type Error = ParserError;
+
+    fn try_from(value: i8) -> Result<Self> {
+        Ok(match value {
+            0 => Self::Integer,
+            1 => Self::String,
+            2 => Self::Color,
+            x => return Err(ParserError::InvalidType(x)),
+        })
+    }
+}
+
+/// Parser for the incoming byte stream.
+struct Parser<'a> {
+    bytes: &'a [u8],
+    endianness: Endianness,
+}
+
+impl<'a> Parser<'a> {
+    /// Create a new parser.
+    fn new(bytes: &'a [u8]) -> Result<Self> {
+        let (endianness, bytes) = bytes
+            .split_first()
+            .ok_or_else(|| ParserError::ran_out(1, 0))?;
+        let endianness = match *endianness {
+            BIG_ENDIAN => Endianness::Big,
+            LITTLE_ENDIAN => Endianness::Little,
+            _ => Endianness::native(),
+        };
+
+        Ok(Self {
+            // Ignore three bytes of padding and the four-byte serial.
+            bytes: bytes
+                .get(7..)
+                .ok_or_else(|| ParserError::ran_out(7, bytes.len()))?,
+            endianness,
+        })
+    }
+
+    /// Get a slice of bytes.
+    fn advance(&mut self, n: usize) -> Result<&'a [u8]> {
+        if n == 0 {
+            return Ok(&[]);
+        }
+
+        if n > self.bytes.len() {
+            Err(ParserError::ran_out(n, self.bytes.len()))
+        } else {
+            let (part, rem) = self.bytes.split_at(n);
+            self.bytes = rem;
+            Ok(part)
+        }
+    }
+
+    /// Skip some padding.
+    fn pad(&mut self, size: usize, pad: usize) -> Result<()> {
+        let advance = (pad - (size % pad)) % pad;
+        self.advance(advance)?;
+        Ok(())
+    }
+
+    /// Get a single byte.
+    fn i8(&mut self) -> Result<i8> {
+        self.advance(1).map(|s| s[0] as i8)
+    }
+
+    /// Get two bytes.
+    fn i16(&mut self) -> Result<i16> {
+        self.advance(2).map(|s| {
+            let bytes: &[u8; 2] = s.try_into().unwrap();
+            match self.endianness {
+                Endianness::Big => i16::from_be_bytes(*bytes),
+                Endianness::Little => i16::from_le_bytes(*bytes),
+            }
+        })
+    }
+
+    /// Get four bytes.
+    fn i32(&mut self) -> Result<i32> {
+        self.advance(4).map(|s| {
+            let bytes: &[u8; 4] = s.try_into().unwrap();
+            match self.endianness {
+                Endianness::Big => i32::from_be_bytes(*bytes),
+                Endianness::Little => i32::from_le_bytes(*bytes),
+            }
+        })
+    }
+}
+
+/// Endianness of the incoming data.
+enum Endianness {
+    Little,
+    Big,
+}
+
+impl Endianness {
+    #[cfg(target_endian = "little")]
+    fn native() -> Self {
+        Endianness::Little
+    }
+
+    #[cfg(target_endian = "big")]
+    fn native() -> Self {
+        Endianness::Big
+    }
+}
+
+/// Parser errors.
+#[derive(Debug)]
+pub enum ParserError {
+    /// Ran out of bytes.
+    NoMoreBytes {
+        expected: NonZeroUsize,
+        found: usize,
+    },
+
+    /// Invalid type.
+    InvalidType(i8),
+
+    /// Bad setting type.
+    BadType(SettingType),
+}
+
+impl ParserError {
+    fn ran_out(expected: usize, found: usize) -> ParserError {
+        let expected = NonZeroUsize::new(expected).unwrap();
+        Self::NoMoreBytes { expected, found }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    //! Tests for the XSETTINGS parser.
+
+    use super::*;
+
+    const XSETTINGS: &str = include_str!("tests/xsettings.dat");
+
+    #[test]
+    fn empty() {
+        let err = match read_settings(&[]) {
+            Ok(_) => panic!(),
+            Err(err) => err,
+        };
+        match err {
+            ParserError::NoMoreBytes { expected, found } => {
+                assert_eq!(expected.get(), 1);
+                assert_eq!(found, 0);
+            }
+
+            _ => panic!(),
+        }
+    }
+
+    #[test]
+    fn parse_xsettings() {
+        let data = XSETTINGS
+            .trim()
+            .split(',')
+            .map(|tok| {
+                let val = tok.strip_prefix("0x").unwrap();
+                u8::from_str_radix(val, 16).unwrap()
+            })
+            .collect::<Vec<_>>();
+
+        let settings = read_settings(&data)
+            .unwrap()
+            .collect::<Result<Vec<_>>>()
+            .unwrap();
+
+        let dpi = settings.iter().find(|s| s.name == b"Xft/DPI").unwrap();
+        assert_int(&dpi.data, 96 * 1024);
+        let hinting = settings.iter().find(|s| s.name == b"Xft/Hinting").unwrap();
+        assert_int(&hinting.data, 1);
+
+        let rgba = settings.iter().find(|s| s.name == b"Xft/RGBA").unwrap();
+        assert_string(&rgba.data, "rgb");
+        let lcd = settings
+            .iter()
+            .find(|s| s.name == b"Xft/Lcdfilter")
+            .unwrap();
+        assert_string(&lcd.data, "lcddefault");
+    }
+
+    fn assert_string(dat: &SettingData<'_>, s: &str) {
+        match dat {
+            SettingData::String(left) => assert_eq!(*left, s.as_bytes()),
+            _ => panic!("invalid data type"),
+        }
+    }
+
+    fn assert_int(dat: &SettingData<'_>, i: i32) {
+        match dat {
+            SettingData::Integer(left) => assert_eq!(*left, i),
+            _ => panic!("invalid data type"),
+        }
+    }
+}