Skip to content

Commit

Permalink
x11: Check visuals for validity
Browse files Browse the repository at this point in the history
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: #184
Signed-off-by: Uli Schlachter <psychon@znc.in>
Co-authored-by: John Nunley <dev@notgull.net>
  • Loading branch information
psychon and notgull authored Dec 11, 2023
1 parent 64dad39 commit 09b8126
Showing 1 changed file with 70 additions and 1 deletion.
71 changes: 70 additions & 1 deletion src/x11.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustix::{
};

use std::{
collections::HashSet,
fmt,
fs::File,
io, mem,
Expand All @@ -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<D: ?Sized> {
Expand All @@ -41,6 +42,9 @@ pub struct X11DisplayImpl<D: ?Sized> {
/// SHM extension is available.
is_shm_available: bool,

/// All visuals using softbuffer's pixel representation
supported_visuals: HashSet<Visualid>,

/// The generic display where the `connection` field comes from.
///
/// Without `&mut`, the underlying connection cannot be closed without other unsafe behavior.
Expand Down Expand Up @@ -100,9 +104,12 @@ impl<D: HasDisplayHandle + ?Sized> X11DisplayImpl<D> {
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,
})
}
Expand Down Expand Up @@ -254,6 +261,16 @@ impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> X11Impl<D, W> {
(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.
Expand Down Expand Up @@ -855,6 +872,58 @@ 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<Visualid> {
// 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
visual.class == VisualClass::TRUE_COLOR
|| visual.class == 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 {
Expand Down

0 comments on commit 09b8126

Please sign in to comment.