From 7e3407f3197bcd72820bae9f6ac48b3f64dd8f19 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 10 Dec 2023 18:50:57 +0100 Subject: [PATCH 1/2] x11: Check visuals for validity A visual describes how colors are laid out by the X11 server. So far, softbuffer did not do anything with visuals and just passed them around. This commit adds checks that should ensure that a given visual actually lays out pixels in the only format supported by softbuffer: - 32 bit per pixel - 00RRGGBB byte order In this patch, I also try to handle big endian systems and mixed endian situations where we are e.g. running on a big endian system and talking to a little endian X11 server. However, this is all theoretical and completely untested. I only tested my X11 server's default visual works and some ARGB visual is rejected. Fixes: https://github.com/rust-windowing/softbuffer/issues/184 Signed-off-by: Uli Schlachter --- src/x11.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/src/x11.rs b/src/x11.rs index d3d399a..253f29d 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -17,6 +17,7 @@ use rustix::{ }; use std::{ + collections::HashSet, fmt, fs::File, io, mem, @@ -31,7 +32,7 @@ use x11rb::connection::{Connection, SequenceNumber}; use x11rb::cookie::Cookie; use x11rb::errors::{ConnectionError, ReplyError, ReplyOrIdError}; use x11rb::protocol::shm::{self, ConnectionExt as _}; -use x11rb::protocol::xproto::{self, ConnectionExt as _}; +use x11rb::protocol::xproto::{self, ConnectionExt as _, ImageOrder, VisualClass, Visualid}; use x11rb::xcb_ffi::XCBConnection; pub struct X11DisplayImpl { @@ -41,6 +42,9 @@ pub struct X11DisplayImpl { /// SHM extension is available. is_shm_available: bool, + /// All visuals using softbuffer's pixel representation + supported_visuals: HashSet, + /// The generic display where the `connection` field comes from. /// /// Without `&mut`, the underlying connection cannot be closed without other unsafe behavior. @@ -100,9 +104,12 @@ impl X11DisplayImpl { log::warn!("SHM extension is not available. Performance may be poor."); } + let supported_visuals = supported_visuals(&connection); + Ok(Self { connection: Some(connection), is_shm_available, + supported_visuals, _display: display, }) } @@ -254,6 +261,16 @@ impl X11Impl { (geometry_reply, visual_id) }; + if !display.supported_visuals.contains(&visual_id) { + return Err(SoftBufferError::PlatformError( + Some(format!( + "Visual 0x{visual_id:x} does not use softbuffer's pixel format and is unsupported" + )), + None, + ) + .into()); + } + // See if SHM is available. let buffer = if display.is_shm_available { // SHM is available. @@ -855,6 +872,60 @@ fn is_shm_available(c: &impl Connection) -> bool { matches!((attach.check(), detach.check()), (Ok(()), Ok(()))) } +/// Collect all visuals that use softbuffer's pixel format +fn supported_visuals(c: &impl Connection) -> HashSet { + // Check that depth 24 uses 32 bits per pixels + if !c + .setup() + .pixmap_formats + .iter() + .any(|f| f.depth == 24 && f.bits_per_pixel == 32) + { + log::warn!("X11 server does not have a depth 24 format with 32 bits per pixel"); + return HashSet::new(); + } + + // How does the server represent red, green, blue components of a pixel? + #[cfg(target_endian = "little")] + let own_byte_order = ImageOrder::LSB_FIRST; + #[cfg(target_endian = "big")] + let own_byte_order = ImageOrder::MSB_FIRST; + let expected_masks = if c.setup().image_byte_order == own_byte_order { + (0xff0000, 0xff00, 0xff) + } else { + // This is the byte-swapped version of our wished-for format + (0xff00, 0xff0000, 0xff000000) + }; + + c.setup() + .roots + .iter() + .flat_map(|screen| { + screen + .allowed_depths + .iter() + .filter(|depth| depth.depth == 24) + .flat_map(|depth| { + depth + .visuals + .iter() + .filter(|visual| { + // Ignore grayscale or indexes / color palette visuals + matches!( + visual.class, + VisualClass::TRUE_COLOR | VisualClass::DIRECT_COLOR + ) + }) + .filter(|visual| { + // Colors must be laid out as softbuffer expects + expected_masks == (visual.red_mask, visual.green_mask, visual.blue_mask) + }) + .map(|visual| visual.visual_id) + }) + }) + .collect() +} + /// An error that can occur when pushing a buffer to the window. #[derive(Debug)] enum PushBufferError { From 8adbe60e0063c1a89db961e84991048e1741ed9b Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 10 Dec 2023 21:39:14 +0100 Subject: [PATCH 2/2] Apply code style suggestion to src/x11.rs This applies a suggestion by @notgull Co-authored-by: John Nunley --- src/x11.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/x11.rs b/src/x11.rs index 253f29d..a5baa82 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -911,10 +911,8 @@ fn supported_visuals(c: &impl Connection) -> HashSet { .iter() .filter(|visual| { // Ignore grayscale or indexes / color palette visuals - matches!( - visual.class, - VisualClass::TRUE_COLOR | VisualClass::DIRECT_COLOR - ) + visual.class == VisualClass::TRUE_COLOR + || visual.class == VisualClass::DIRECT_COLOR }) .filter(|visual| { // Colors must be laid out as softbuffer expects