X11 screenshot example captures multiple screens

[polarathene]

Looking at the example code it states there is primary() and all() but primary() seems to capture both my monitor displays? Is that intended?

let display = Display::primary().expect("Couldn't find primary display.");
let mut capturer = Capturer::new(display).expect("Couldn't begin capture.");
let (w, h) = (capturer.width(), capturer.height());

and

/// The primary screen.
pub fn primary() -> io::Result<Display>;

/// All the screens.
pub fn all() -> io::Result<Vec<Display>>;

[quadrupleslap]

Sorry, I haven't had the opportunity to test this on multiple screens! The answer is... maybe? If X11 says that a virtual monitor made up of both your monitors is "preferred", then that's what Display::primary will return. Just checking, Display::all returns at least three displays, right?

[polarathene]

Why should Display::all return at least three displays? I get a vec with one display item. Is it meant to contain this "virtual" display as well as the two individual physical ones?

It probably doesn't help that my system has been having a few issues lately. One was unplugging the display didn't result in my DE/system recognizing that event and keeping my windows offset to where the display was as if it were still plugged in.

Viewing the settings app, the display section lists my two monitors. They seem to have the correct product names but are listed as HDMI-2 and HDMI-3, only one is connected via HDMI(a DVI-D adaptor), and the other is DVI-D. This settings section also has a field "Primary Display:" with interestingly the dropbox value as "No Primary Output"...

It could be your crate is working as expected and the problem is with my system being a bit borked at present.. EDIT: Setting the Primary Display field to either display has not improved the situation.

[polarathene]

I've been able to get individual display data via this x11Cap example: https://github.com/bryal/X11Cap/blob/master/examples/avg.rs

Adjusting the value given to Monitor, from 0 or 1 will toggle different width/height matching my monitors(1680x1050, 1920x1080). I've been trying to make sense of what change needs to be done with your code, but it's a bit beyond me. I have come across this StackOverflow question that has an interesting answer about getting some data from xcb, I'm not sure if it's relevant though.

[polarathene]

I've been trying out an alternative rust crate for capturing X11 displays. Both seem to have similar capture perf in the 10-21ms range, averaging in the lower bounds for the frame() call. Yours is performing a little more better however, even with it capturing both displays(perhaps that's what the other crate does but returns a region of that?

If you'd like to reference it, it is a small crate: https://github.com/bryal/X11Cap/blob/master/src/lib.rs leveraging the x11 one, I see you've gone a more direct route. xlib/xcb and xrandr are supported via the x11 crate: https://docs.rs/x11/2.17.5/x11/ So you might be able to figure out the cause of the issue from there?

Since I'm trying to capture one of the monitors output and display that in a window with SDL, I'm having trouble reaching 60FPS, the capture alone from either crate alone is a bit too high, but that doesn't seem to be Rust specific?

[quadrupleslap]

I totally forgot that X11 has screens, monitors, and displays. I guess a screen capture library should naturally be expected to map to monitors and not screens. The XCB documentation is really hard to read, but I think I got the basic idea. DisplayIter should actually be a two-level (idk, like a nested for-loop?) iterator, first over the roots iterator, like it currently does, but then also over the xcb_randr_get_monitors_monitors_iterator for each root. Then it should pass the monitor (x, y, width height) to the constructor of Display, instead of the screen (width, height) like it does now. The files to change would be:

• x11/iter.rs - iterate over the displays of each screen, not just the screens themselves.
• x11/display.rs - add x and y fields.
• x11/capturer.rs - pass (x, y) to xcb instead of (0, 0).

Do you want to implement this, or should I? Also, the slowness is a bit sad, and I don't know why that's happening. :(

(On a side note, this would've also let me add monitor names to the display struct, if only macOS didn't keep deprecating useful APIs without any replacement. Maybe I should just make it return Option<&str>, where it's always None for macOS.)

[polarathene]

Do you want to implement this, or should I?

I could give it a try if you don't have the time :)

Also, the slowness is a bit sad, and I don't know why that's happening. :(

You're using XGetImage, I've been learning about the technical details myself to investigate. Seems XShmGetImage is preferrable(it should be available in most cases), it will only allocate the memory for a texture once, then if you keep that around for additional captures, you can update it. I'm presently getting a capture in 1-2ms with that. Although a C example reports 10-15k FPS for me instead of the 1k I'm getting via Rust FFI. I'm new to FFI, so perhaps I'm not doing something right.

Someone has a partial implementation of XShm here, I've played with that and added the XShmGetImage() method. I was thinking later I might try contribute it to scrap as an alternative/improvement.

[quadrupleslap]

I'm pretty sure I'm using SHM.

[polarathene]

I'm pretty sure I'm using SHM...

Sorry mixed your lib up with x11cap(which does use XGetImage but is reporting similar speed). The timing I'm getting from calling the two is similar though, with yours being 7-11ms on average for the following:

let start = PreciseTime::now();

let buffer = capturer.frame().unwrap();

let end = PreciseTime::now();
println!("{} seconds for whatever you did.", start.to(end));

This code I've put together below I don't think is using xcb, not sure why it's more performant(1-2ms):

let start = PreciseTime::now();

const plane_mask: u64 = 0x00ffffff;
unsafe {
    xlib::XShmGetImage(
        img.display.raw,
        root_wnd_id,
        img.img,
        0,
        0,
        plane_mask
    );
}

//convert from pointer to actual data so can query like regular struct
let imgdata = &mut unsafe{*img.img};
//total pixels 4 channels BGRX
let size = (imgdata.width * imgdata.height * 4) as usize;
// get ptr for pixel data, data is a *mut c_char, an i8 ptr, convert to u8 ptr
let img_data_ptr = unsafe{ imgdata.data} as *mut i8 as *mut u8;

//from the pointer, take bytes from memory to create slice containing image data
use std::slice;
let slice = unsafe {
    slice::from_raw_parts(img_data_ptr, size)
};
let end = PreciseTime::now();
println!("{} seconds for whatever you did.", start.to(end));

Edit: I was wrong about the linked C example being much faster, I just noticed an issue on the repo that points out the FPS is calculated wrong. Adding the proper measurement brings it more inline with what I'm seeing on Rust.

[quadrupleslap]

Yeah, I'll try implementing proper display support tomorrow, but I just wanted to get your opinion on three things:

1. Should the full screen (as it is captured currently) also be added to the X11 iterator? This might be very confusing, but it might also be useful. I'm leaning towards no.
2. Should I also add name(&str) -> Option<&str> to Display? It would always have a value on Windows and Linux, and never on macOS, but it might be useful for displaying to users.
3. This naive benchmark shows ~101 FPS for x11cap and ~370 FPS for scrap. Do you get similar numbers?

[quadrupleslap]

@polarathene I pushed a fix to master, tell me if it works and if it needs any more fixing, or if I can publish it as a new version on crates.io! :)

[polarathene]

Should the full screen (as it is captured currently) also be added to the X11 iterator? This might be very confusing, but it might also be useful. I'm leaning towards no.

I don't think it should be, as a user I'd expect to get my two individual displays, a surprise virtual one that is both displays(and in my case with the 1050 vs 1080 display heights, extra content I can't see on the physical display) isn't desirable.

A separate method to get the fullscreen capture is probably better, I don't see how it's useful to mix with individual displays via iterator. I can see how it'd be useful for those who want to capture everything together, similar to a prntscreen.

Should I also add name(&str) -> Option<&str> to Display? It would always have a value on Windows and Linux, and never on macOS, but it might be useful for displaying to users.

Up to you, I think it wouldn't hurt? :) Just need the docs to be clear about the macOS never returning a result.

This naive benchmark shows ~101 FPS for x11cap and ~370 FPS for scrap. Do you get similar numbers?

142 FPS for x11cap 415 FPS for scrap 418 FPS for my XShmGetImage logic put into your benchmark format

I originally had 943 FPS, until I realized scrap was probably doing 3600x1080(1680x1050 + 1920x1080, shared max height of 1080)... so now I realize I was also comparing my 1680x1050 captures earlier against the fullscreen captures by scrap that I had forgotten about! x11cap was also only capturing the single monitor afaik, yet performs much worse.

I'll switch back to using scrap once I can target my monitors

[quadrupleslap]

Thanks for the suggestions! I guess I'll keep looking for a way to get the monitor names on macOS, because if System Preferences can do it, surely there's a non-deprecated way to do it?

(Also, in case you missed it, there's a fix on master.)

[polarathene]

Maybe winit handles the macOS information you're having diffiiculty finding here?

I tested master and it works brilliantly. Both displays reported via the list example and the benchmark also is defaulting to my primary and working well. 1680x1050 manages about 920 FPS. Not sure where that slight overhead comes from vs the more barebones code I've been using, nothing to worry about though :)

Thanks for the quick fix!

[quadrupleslap]

Thanks for the link! That's just the model number, not the name, although it's a good substitute while waiting for Apple to provide a proper, not-deprecated, API.