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

Doesn't return focus to i3 when closing #516

Open
anderspitman opened this issue May 13, 2018 · 22 comments
Open

Doesn't return focus to i3 when closing #516

anderspitman opened this issue May 13, 2018 · 22 comments
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - x11

Comments

@anderspitman
Copy link

I'm using winit with conrod on the i3 window manager. I've noticed that often when I close my app with another i3 window open (usually the terminal I launched the app from), i3 stops responding until I move the mouse to the top of the other window. I'd be happy to look into fixing this myself if anyone has suggestions on where to start. I don't have any experience with X11 and I'm still rather new to Rust.

@rukai
Copy link
Contributor

rukai commented May 13, 2018

Have you tried on the latest version of winit? I used to get that but it stopped happening and I dont remember if that was because of something I changed in my own program or an update to winit.

@francesca64
Copy link
Member

It looks like conrod is still on winit 0.12.0, so to test the latest winit there, you'd have to make some modifications to account for the breaking changes in winit 0.13.0. It would be a good opportunity to make an easy PR to conrod, since all you generally have to do is replace WindowEvent::Closed with WindowEvent::CloseRequested.

The quickest thing would be try with the winit example programs, though. With some luck, @rukai is right (thanks for helping out here!).

I'd be happy to look into fixing this myself if anyone has suggestions on where to start. I don't have any experience with X11 and I'm still rather new to Rust.

If the problem is still present, I'll gladly take you on winit's wild ride. Just be warned that direct exposure to X11 has been known to cause deep-seated cynicism about Linux as a desktop platform, and in later stages, may also result in you believing that implementing protocols using clipboard mechanisms is a beautifully flexible and economical design choice.

@francesca64 francesca64 added B - bug Dang, that shouldn't have happened DS - x11 C - needs investigation Issue must be confirmed and researched labels May 13, 2018
@rukai
Copy link
Contributor

rukai commented May 13, 2018

To try and reproduce the issue I did:

git clone https://github.com/PistonDevelopers/conrod.git
cd conrod/examples
cargo run --release --example triangles --features="winit glium"

Then I would close the window by either ctrl-c in the terminal I was using or killall triangles in a different terminal.
I repeated about 20 times as you mentioned, that the issue doesnt always occur.
However I couldnt reproduce the issue.

It might have been an issue with i3 that was fixed there.
I use arch linux so my i3 is version 4.15 (2018-03-10)

@b-r-u
Copy link
Contributor

b-r-u commented May 13, 2018

This issue is still present with i3 version 4.15 and the current winit master, although after this giant commit of improvements c4b92eb it has gotten way harder to reproduce.

I used this program:

extern crate winit;

fn main() {
    let mut events_loop = winit::EventsLoop::new();

    let _window = winit::WindowBuilder::new()
        .with_title("A fantastic window!")
        .build(&events_loop)
        .unwrap();

    events_loop.run_forever(|event| {
        println!("{:?}", event);

        match event {
            winit::Event::WindowEvent {
                event: winit::WindowEvent::CloseRequested,
                ..
            } => winit::ControlFlow::Break,
            winit::Event::WindowEvent {
                event: winit::WindowEvent::KeyboardInput {
                    input: winit::KeyboardInput {
                        state: winit::ElementState::Pressed,
                        virtual_keycode: Some(winit::VirtualKeyCode::Escape),
                        .. },
                    .. },
                ..
            } => {
                winit::ControlFlow::Break
            },
            _ => winit::ControlFlow::Continue,
        }
    });
}

With older commits (before c4b92eb) I could reproduce this nearly every time by delaying the execution sleep 2; cargo run --release, switching to another workspace and pressing Esc to close the application. After that, it became so difficult to reproduce that I am now starting to doubt my memory...

@b-r-u
Copy link
Contributor

b-r-u commented May 13, 2018

Here is an even shorter version that works now and then (just running it without delay and workspace switching):

extern crate winit;

fn main() {
    let mut events_loop = winit::EventsLoop::new();

    let _window = winit::WindowBuilder::new()
        .build(&events_loop)
        .unwrap();
}

@anderspitman
Copy link
Author

Thanks for the code @b-r-u! I can confirm that your last example reproduces my exact problem with both winit 0.12 and 0.13. I've uploaded a quick video to show how the problem looks on my system, to make sure we're all on the same page.

Thanks for the offer @francesca64. While becoming familiar with the internals of X11 isn't exactly on my bucket list, I'd really like to help out with winit if I can. Whatever you think the best step forward is I can allocate a few hours to it this week.

@anderspitman
Copy link
Author

Oh here's the video

@rukai
Copy link
Contributor

rukai commented May 14, 2018

Tested the second example posted by b-r-u

  • winit 0.13.1 reproduces the issue everytime
  • winit 0.14 never reproduces the issue (tried 20 times)

@anderspitman
Copy link
Author

Oops I didn't even realize 0.13 wasn't the latest. 0.14 is definitely a huge improvement, but I'm still able to reproduce it non-deterministically. New video.

@anderspitman
Copy link
Author

@rukai can you try running it ~50 times? At one point I did about 30 in a row without it triggering. Then sometimes there will be a couple pretty close to each other.

@rukai
Copy link
Contributor

rukai commented May 14, 2018

I did: for i in `seq 1 200`; do cargo run --release; sleep 0.2s; done;
Then held down the a key while it was looping.
Given that every loop there were a's entered into the terminal, it is not occuring on my machine (at least for the 200 times I tried)

When I do this with winit 0.13 there are no a's.

But this doesnt mean much, only that whatever race condition etc. is causing this doesnt line up the same on my machine.

One approach to figuring out what is going on, would be to try changing things in the current winit master to be like 0.13 until you can reproduce the issue consistently like in 0.13. (Or go from 0.13 to master)

@francesca64
Copy link
Member

@anderspitman thanks! That frees me up to work on fixing all the other problems. I'd recommend following @rukai's recommendation:

One approach to figuring out what is going on, would be to try changing things in the current winit master to be like 0.13 until you can reproduce the issue consistently like in 0.13.

Since I don't really have any ideas for what could be happening here, this seems like the most reliable way to narrow it down. I'll also explain any code (or general X11 stuff) you ask about.

@anderspitman
Copy link
Author

Thanks @rukai.

@francesca64 sounds good. That approach is a bit brute force but it will probably also be a great way to start getting familiar with the codebase. I'll spend some time poking at it the next few days. Is this thread a good place to ask questions? Is there a dedicated chat for smaller issues that require more real-time interaction?

@francesca64
Copy link
Member

Asking questions here is fine, though I'll gladly create a gitter if you think it would be useful.

@anderspitman
Copy link
Author

@francesca64 I've identified commit c4b92eb (#491) as the commit that greatly reduces the frequency of this bug appearing. Any idea what you changed in there that might have done it?

@anderspitman
Copy link
Author

Also it might be a good idea to verify that I've identified the proper commit before you spend too much time thinking about it.

@b-r-u
Copy link
Contributor

b-r-u commented May 15, 2018

@anderspitman I guess we arrived at the same conclusion :) #516 (comment)

@francesca64
Copy link
Member

@anderspitman I can't say I know which of those 2kLOC of changes did this. I can go into detail on any individual changes though.

@anderspitman
Copy link
Author

@b-r-u ha! Somehow I completely missed that comment. Would have saved me a few minutes. I guess it's good we picked the same commit though. I'll start working through the commit and see what I can find.

@anderspitman
Copy link
Author

Also @francesca64 is it normal to have commits as large as c4b92eb in winit? I know some codebases like to squash their commits for various reasons.

@francesca64
Copy link
Member

All PRs merged are automatically squashed. Most commits are fortunately smaller than that, or at the very least, of a more normal scope.

@svenstaro
Copy link
Contributor

Isn't the issue likely caused by the call to XSetInputFocus which sets ffi::RevertToParent? Maybe you can try setting ffi:RevertToNone and just see what happens. See also: https://tronche.com/gui/x/xlib/input/XSetInputFocus.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - x11
Development

No branches or pull requests

6 participants