Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

X11: Wrong behaviour for "weird" visuals? #184

Closed
psychon opened this issue Dec 10, 2023 · 2 comments · Fixed by #185
Closed

X11: Wrong behaviour for "weird" visuals? #184

psychon opened this issue Dec 10, 2023 · 2 comments · Fixed by #185

Comments

@psychon
Copy link
Contributor

psychon commented Dec 10, 2023

According to the docs, pixels are represented as:

softbuffer/src/lib.rs

Lines 371 to 378 in 64dad39

/// Pixel format (`u32`):
///
/// 00000000RRRRRRRRGGGGGGGGBBBBBBBB
///
/// 0: Bit is 0
/// R: Red channel
/// G: Green channel
/// B: Blue channel

However, I cannot find anything in src/x11.rs that actually ensures this. Any visual will do.

For my currently running X11 server, all visuals actually seem to use this format:

$ xdpyinfo| grep red,\ g | uniq  
    red, green, blue masks:    0xff0000, 0xff00, 0xff

However, not all of these are 24 bit depth:

$ xdpyinfo| grep depth: | uniq
    depth:    24 planes
    depth:    32 planes

The last one of these is actually ARGB (and I bet many others, too; I only checked this one).

$ xdpyinfo | grep -B 2 'depth:    32' | tail -n 3
    visual id:    0x767
    class:    TrueColor
    depth:    32 planes
$ xdpyinfo -ext RENDER | grep -1 0x767 | tail -n 3
      visual format:
        visual id:      0x767
        pict format id: 0x25
$ xdpyinfo -ext RENDER | grep -A 5 'format id:    0x25' 
	format id:    0x25
	type:         Direct
	depth:        32
	alpha:        24 mask 0xff
	red:          16 mask 0xff
	green:         8 mask 0xff

How should softbuffer handle these? Should it reject any visual that does not match its expectation? Should it just ignore this and hope for the best? Should it try to actively convert between visuals (sloooooooow)?

This might also be endianness-related, so #109. I don't actually know what big endian X11 servers usually do here. Worse, what happens to a client running on a big-endian machine and talking to a little endian server (or vice-versa)?

@psychon
Copy link
Contributor Author

psychon commented Dec 10, 2023

Patch to the winit example that makes it print some pixel values that are invalid according to the docs. Output for me is

Using visual 7a of class TRUE_COLOR, 8 bits per rgb, masks ff0000, ff00, ff
Some pixel values: [70000000, ff010000, 10020000, 90030000, a0040000]

(After having written this, I noticed that the output doesn't change even if I do not pick a non-default visual and stay with depth=42. Apparently the X11 server doesn't clear "unimportant" bits.)

diff --git a/examples/winit.rs b/examples/winit.rs
index 7c903e2..679720f 100644
--- a/examples/winit.rs
+++ b/examples/winit.rs
@@ -5,9 +5,28 @@ use winit::event_loop::{ControlFlow, EventLoop};
 use winit::keyboard::{Key, NamedKey};
 use winit::window::WindowBuilder;
 
+use winit::platform::x11::WindowBuilderExtX11;
+use winit::raw_window_handle::{HasDisplayHandle, DisplayHandle, RawDisplayHandle};
+
+fn pick_visual() -> u32 {
+    use x11rb::connection::Connection;
+
+    let (conn, screen) = x11rb::rust_connection::RustConnection::connect(None).unwrap();
+    // Find a random visual with depth=32
+    conn.setup().roots[screen].allowed_depths.iter().filter(|depth| depth.depth == 32)
+        .map(|depth| {
+            let visual = depth.visuals[0];
+            println!("Using visual {:x} of class {:?}, {} bits per rgb, masks {:x}, {:x}, {:x}",
+                visual.visual_id, visual.class, visual.bits_per_rgb_value, visual.red_mask, visual.green_mask, visual.blue_mask);
+            visual.visual_id
+        })
+        .next()
+        .unwrap()
+}
+
 fn main() {
     let event_loop = EventLoop::new().unwrap();
-    let window = Rc::new(WindowBuilder::new().build(&event_loop).unwrap());
+    let window = Rc::new(WindowBuilder::new().with_x11_visual(pick_visual()).build(&event_loop).unwrap());
 
     #[cfg(target_arch = "wasm32")]
     {
@@ -52,7 +71,15 @@ fn main() {
                             }
                         }
 
+                        buffer[0] |= 0x70 << 24;
+                        buffer[1] |= 0xff << 24;
+                        buffer[2] |= 0x10 << 24;
+                        buffer[3] |= 0x90 << 24;
+                        buffer[4] |= 0xa0 << 24;
                         buffer.present().unwrap();
+
+                        let fetched = surface.fetch().unwrap();
+                        println!("Some pixel values: {:08x?}", &fetched[..5]);
                     }
                 }
                 Event::WindowEvent {

@notgull
Copy link
Member

notgull commented Dec 10, 2023

Generally "assuming RGBA" is a problem for softbuffer at the moment. It’s the main reason we don’t support Android (#44) and the API for non-RGBA surfaces still needs to be figured out (#98).

In X11’s case though you have to go out of your way to use a non-RGBA format. I think there is a lot of software that breaks on non-RGBA aside from softbuffer anyways. So I think a good solution for this for now would be to raise an error if we try to use a window with a strange format.

psychon added a commit to psychon/softbuffer that referenced this issue Dec 10, 2023
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: rust-windowing#184
Signed-off-by: Uli Schlachter <[email protected]>
psychon added a commit to psychon/softbuffer that referenced this issue Dec 10, 2023
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: rust-windowing#184
Signed-off-by: Uli Schlachter <[email protected]>
notgull added a commit that referenced this issue Dec 11, 2023
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 <[email protected]>
Co-authored-by: John Nunley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants