psychon / x11rb

X11 bindings for the rust programming language, similar to xcb being the X11 C bindings
Apache License 2.0
372 stars 41 forks source link

Attaching an image to shared memory #793

Closed mWalrus closed 9 months ago

mWalrus commented 1 year ago

Hi, I'm currently working on an image viewer and I'm using x11rb for the first time and I'm kinda stuck :P

When I started this project a couple of days ago I began implementing it the "non-shm" way and that worked, kinda. The issue with the standard implementation is that the image flickers whenever I'm required to redraw it. This lead me to want to shared memory.

I've been reading the shared memory example and some of the discussions in issues related to shared memory to see if I could get a grasp of how shm is supposed to be used. I've also read a bunch of xcb documentation regarding shared memory but to no avail.

Here is my current code:

pub fn attach_image<'a, C: Connection>(
    conn: &'a C,
    img: &'a Image,
    s: &Screen,
    win: Window,
) -> Result<SHMInfo> {
    let (w, h) = (img.width(), img.height());
    let size = w as usize * h as usize * mem::size_of::<Bgr>();
    let shminfo = unsafe {
        let id = libc::shmget(libc::IPC_PRIVATE, size, libc::IPC_CREAT | 0o777);
        if id < 0 {
            mevi_err!("shmget failed");
            std::process::exit(1);
        }

        let addr = libc::shmat(id, ptr::null(), SHM_RDONLY);
        let seg = conn.generate_id()?;

        conn.shm_attach(seg, id as u32, false)?;
        SHMInfo {
            seg,
            id,
            addr: addr as *const u8,
        }
    };

    conn.shm_attach(shminfo.seg, shminfo.id as u32, false)?;

    let pm = conn.generate_id()?;
    let gc = conn.generate_id()?;

    conn.create_gc(gc, s.root, &CreateGCAux::default().graphics_exposures(0))?;
    mevi_info!("created graphics context: {gc}");

    shm::create_pixmap(conn, pm, win, w, h, s.root_depth, shminfo.seg, 0)?;
    mevi_info!("created shared pixmap: {pm}");

    shm::put_image(
        conn,
        pm,
        gc,
        w,
        h,
        0,
        0,
        w,
        h,
        0,
        0,
        s.root_depth,
        2,
        false,
        shminfo.seg,
        0,
    )?;
    Ok(shminfo)
}

First off, I get an error when I try to create the shared pixmap for some reason and the error message isn't too helpful: image

Second, am I going about this correctly or am I missing how to implement this completely?

The source for all of this can be found in my shared-memory branch.

Thanks in advance :)

psychon commented 1 year ago

Wow, this is the first time I see an Implementation error. According to https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#errors this means:

The server does not implement some aspect of the request. A server that generates this error for a core request is deficient. As such, this error is not listed for any of the requests, but clients should be prepared to receive such errors and handle or discard them.

https://gitlab.freedesktop.org/xorg/proto/xorgproto/-/blob/master/specs/xextproto/shm.xml does not mention this error.

What kind of server are you testing this with? I am confused....

Next, I tried your code. It took me a moment to get versions of the dependencies that work with Rust 1.63, but now I get:

[Error]: X11 server does not support the SHM extension

What is going on at the end of src/shm.rs?

    if v.shared_pixmaps {
        return Ok(None);
    }

If the X11 server supports shared pixmaps, you return "not supported"?

After adding a missing !, I get:

[Error]: Received error: X11Error { error_kind: IDChoice, error_code: 14, sequence: 19, bad_value: 56623107, minor_opcode: 1, major_opcode: 130, extension_name: Some("MIT-SHM"), request_name: Some("Attach") }
[Error]: Received error: X11Error { error_kind: Value, error_code: 2, sequence: 21, bad_value: 0, minor_opcode: 5, major_opcode: 130, extension_name: Some("MIT-SHM"), request_name: Some("CreatePixmap") }
[Error]: Received error: X11Error { error_kind: Drawable, error_code: 9, sequence: 22, bad_value: 56623108, minor_opcode: 3, major_opcode: 130, extension_name: Some("MIT-SHM"), request_name: Some("PutImage") }
[Error]: Received error: X11Error { error_kind: GContext, error_code: 13, sequence: 27, bad_value: 0, minor_opcode: 0, major_opcode: 62, extension_name: None, request_name: Some("CopyArea") }

Looking at this with xtrace (also known as x11trace) I see:

000:<:0012: 16: MIT-SHM-Request(130,1): Attach shmseg=0x03a00003 shmid=0x00000029 readonly=false(0x00)
000:<:0013: 16: MIT-SHM-Request(130,1): Attach shmseg=0x03a00003 shmid=0x00000029 readonly=false(0x00)

This attaches twice with the same id. This is fixed with:

diff --git a/src/shm.rs b/src/shm.rs
index c5ee0fa..4dcb21f 100644
--- a/src/shm.rs
+++ b/src/shm.rs
@@ -43,7 +43,6 @@ pub fn attach_image<'a, C: Connection>(
         let addr = libc::shmat(id, ptr::null(), SHM_RDONLY);
         let seg = conn.generate_id()?;

-        conn.shm_attach(seg, id as u32, false)?;
         SHMInfo {
             seg,
             id,

The next error comes from trying to create a pixmap with width/height 0:

000:<:0014: 28: MIT-SHM-Request(130,5): CreatePixmap pid=0x03a00004 drawable=0x03a00000 width=0 height=0 depth=24 shmseg=0x03a00003 offset=0x00000000

These values are in fact hardcoded in your call. Edit: Changing 0, 0 to w, h fixes this error.

The last remaining error comes from the incorrect GContext in the call to copy_area() (hardcoded as zero). Since you say you have a version without SHM working, I'll assume you know about GContexts and how to create/use them.

psychon commented 1 year ago

The issue with the standard implementation is that the image flickers whenever I'm required to redraw it. This lead me to want to shared memory.

The flickering comes from your background pixmap:

    let win_aux = CreateWindowAux::default()
        .event_mask(EventMask::EXPOSURE | EventMask::STRUCTURE_NOTIFY)
        .background_pixmap(bg_img_pm);

Before you get an Expose event, the X11 server will first redraw your window. And it will redraw everything, because your are not specifying a bit-gravity, the default bit-gravity is "Forget" (https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:CreateWindow) and https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:ConfigureWindow says:

A bit-gravity of Forget indicates that the window contents are always discarded after a size change

Setting a proper bit gravity makes the flickering go away here:

diff --git a/src/window.rs b/src/window.rs
index cd85e0d..80e572b 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -3,7 +3,7 @@ use x11rb::connection::Connection;
 use x11rb::image::Image;
 use x11rb::protocol::xproto::{
     ConnectionExt, CreateGCAux, CreateWindowAux, EventMask, Gcontext, Pixmap, PropMode, Screen,
-    Window, WindowClass,
+    Window, WindowClass, Gravity
 };
 use x11rb::wrapper::ConnectionExt as _;

@@ -44,6 +44,7 @@ pub fn init_window(

     let win_aux = CreateWindowAux::default()
         .event_mask(EventMask::EXPOSURE | EventMask::STRUCTURE_NOTIFY)
+        .bit_gravity(Gravity::NORTH_WEST)
         .background_pixmap(bg_img_pm);

     conn.create_window(

(What is a bit gravity? Imagine your window being resized to a larger size. If you set the bit gravity to south east, the existing content will be shifted so that it stays at the bottom right corner of the window. The top and left part of the window become exposed. Your viewer seems to want a bit gravity of NorthWest.)

mWalrus commented 1 year ago

Hi, thanks for the super thorough reply! :D

All of the weirdness that you point out in my code is just remnants of desperate attempts to implement something that works, and after like 8 hours of scratching my head I missed to remove some of them before creating this issue :P

And in regards to the Implementation error, I thought this was weird too since was creating the pixmap, to my knowledge, correctly but it wouldn't go away. I even read the page you linked about shm and also the one about the protocol errors, which made me even more confused since there was no mention of the error in regards to pixmaps.

I really appreciate the detailed explanation you provided! I'm new to lower level programming in general, but it's been super fun so far learning all this stuff, so thanks again :)

I'll go ahead and close this issue :)

mWalrus commented 1 year ago

Hi, I have another question. I've been trying to center the image in the window, but I cannot for the life of me figure out how I would go about doing this.

I can draw the image in the correct position and it all looks great whenever I first start the application: image

The issue becomes apparent whenever I try to resize the window. If I have any bit gravity set, trails of either the image or the background are left on the screen: image

The only way I can get around this issue is by unsetting the bit gravity, but that takes me back to square one since I reintroduce the flickering issues.

So, do I manually have to try to redraw the background around the image in some way, or is there some better way of doing this? Sorry if it's obvious, I've read the libxcb docs but I cant find anything specific that would help me solve this issue.

branch link with what I've currently done.

psychon commented 1 year ago

Hi, I have another question. I've been trying to center the image in the window, but I cannot for the life of me figure out how I would go about doing this.

There is a Center bit gravity. That could help you, but you'd still have to redraw things every time since there can be rounding issues (I think the X11 server always rounds down for this, so in a series of resizes with odd size increments (e.g. a slow resize with one pixel increments) could cause some drift otherwise). Oh and this size offsets could lead to visible wobbling...

My personal approach would be to use double buffering: Create a pixmap as big as your window, draw to that, then copy the final result to your window.

If you want to be more fancy, you could try to use core X11 rendering with stuff like SetClipRectangles to draw the background yourself, being careful not to draw over the actual image you want to show. In this case, I would use a bit gravity of "foreget" or whatever the name was for "dear X11 server, don't try to do anything smart; I'll deal with things myself".

psychon commented 1 year ago

So, do I manually have to try to redraw the background around the image in some way, or is there some better way of doing this?

Actually, I just remembered something more: ClearArea: https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:ClearArea

The x and y coordinates are relative to the window's origin and specify the upper-left corner of the rectangle. If width is zero, it is replaced with the current width of the window minus x. If height is zero, it is replaced with the current height of the window minus y. If the window has a defined background tile, the rectangle is tiled with a plane-mask of all ones and function of Copy and a subwindow-mode of ClipByChildren.

What this basically means is: This draws the background. So, you could use that to manually draw the background where needed. I'm not sure whether this is that much easier than the alternative, but it's at least a possibility.

mWalrus commented 1 year ago

Thanks for all the information again!

I decided to try out the double buffering solution and I'm almost there. Only issue is that I can't figure out how to tile the background image since its just 16x16. Loading it into a pixmap and setting it as background_pixmap on the window took care of the tiling for me before. I tried setting tile to background_pixmap like so: image but that doesn't seem to be doing anything: image Right now it seems like everything besides that 16x16 space in the top right corner is just random chunks of pixel data from elsewhere in memory. I'm probably (once again) missing something simple :P

psychon commented 1 year ago

I don't think putting an image does any tiling. You need to put the image to another pixmap that is just as large as your image and then you can use a GC to draw that to the target. Looking at https://www.x.org/releases/X11R7.6/doc/xproto/x11protocol.html#requests:CreateGC, there is a tile property of a GC that is a pixmap. Searching that page for tile, (including the trailing comma) suggests that you can then use PolyFillRectangle to do the actual tiling.

mWalrus commented 1 year ago

I did as you suggested and it works! :D Thank you so much for all the help <3