emoon / rust_minifb

Cross platfrom window and framebuffer crate for Rust
MIT License
1k stars 95 forks source link

is_key_down misses key down + lag after letting key go #284

Closed TJB-99 closed 2 years ago

TJB-99 commented 2 years ago

Thanks a lot for this library, it's a joy to use!

Running the below example and holding down the space key for a longer time (3 seconds+) will result in odd behavior. The space! and nothing! prints will "alternate", even though the space key is pressed all the time. Furthermore after letting the space key go, it takes a while till space! is not printed anymore. The key down detection lags behind.

I couldn't find anything wrong with my code but maybe I am missing something?

I am on Ubuntu 20.04, using X11 and rustc 1.59 with minifb 0.20.0.

use minifb::{Key, Window, WindowOptions};

fn main() {
    let mut window = Window::new("Test", 500, 500, WindowOptions::default()).unwrap();

    window.limit_update_rate(Some(std::time::Duration::from_millis(16)));
    while window.is_open() {
        if window.is_key_down(Key::Space) {
            println!("space!");
        } else {
            println!("nothing!");
        }

        window.update();
    }
}

Alternating behavior:

space!
space!
space!
space!
space!
space!
space!
space!
space!
space!
space!
space!
space!
space!
nothing!
nothing!
space!
space!
nothing!
nothing!
space!
space!
nothing!
nothing!
space!
nothing!
nothing!
space!
space!
nothing!
nothing!
space!
space!
nothing!
nothing!
emoon commented 2 years ago

Hey! Thanks for the praise!

Your code looks correct. I will take a look and come back with more info.

emoon commented 2 years ago

So I have looked at this. From my point of view the code looks correct, but I get the same behavior as you do.

I have some code that looks somewhat like this for X11

    unsafe fn raw_process_one_event(&mut self, mut ev: xlib::XEvent) -> ProcessEventResult {
        match ev.type_ {
            xlib::KeyPress => {
                self.process_key(ev, true /* is_down */);
            }

            xlib::KeyRelease => {
                self.process_key(ev, false /* is_down */);
            }

The odd thing is that even if I hold space as you do above I would still get calls to KeyRelease here.

Edit: Found some more info on this, investigating more.

So it may be related to this https://stackoverflow.com/questions/39939478/x11-key-held-down-leads-to-key-pressed-and-released tho, I'm not really sure how to fix it

TJB-99 commented 2 years ago

Oh great find!

Apparently X11 isn't really made for this usecase, I found another related post: https://unix.stackexchange.com/questions/275037/is-there-a-keyhold-event-in-xor .

These posts explain the alternation but not really the lag🤔.

Without the lag it could maybe be resolved by keeping state per key about the last events.

emoon commented 2 years ago

The lag might be due to events being buffered. Imagine that pressing space actually generates a lot of KeyPress / KeyRelease (which seems to be the case) then if the X11 buffers them it means that after you release space there might be some events left to "flush".

Notice I'm speculating here, but it may be the case.

TJB-99 commented 2 years ago

Jep sounds very plausible.

emoon commented 2 years ago

I guess it would be possible to workaround this issue by adding some buffering. So if KeyRelease is done it won't be released until a bit later to make sure no more KeyPress event has happened for the same key(s). Now the problem with this is that it will introduce some input latency.

emoon commented 2 years ago

Also I tried to implement the first solution mention here https://stackoverflow.com/questions/2100654/ignore-auto-repeat-in-x11-applications/3222566 but it doesn't seem to work, I will have another look at it.

emoon commented 2 years ago

I may have a fix!

Can you try this out? in x11.rs and on line 136 add this

            let mut supported = 0;
            (lib.XkbSetDetectableAutoRepeat)(display, 1, &mut supported);
TJB-99 commented 2 years ago

Oh great!

I will try it asap.

TJB-99 commented 2 years ago

That works 🥳

Thanks so much for your help!

emoon commented 2 years ago

Great! I will add it to the code :)

emoon commented 2 years ago

0.22 has been released including this fix