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

EventsLoopProxy::wakeup does not fire until mouse is moved over window (Windows) #550

Closed
fschutt opened this issue Jun 4, 2018 · 11 comments · Fixed by #710
Closed

EventsLoopProxy::wakeup does not fire until mouse is moved over window (Windows) #550

fschutt opened this issue Jun 4, 2018 · 11 comments · Fixed by #710
Labels
B - bug Dang, that shouldn't have happened C - waiting on author Waiting for a response or another PR DS - windows
Milestone

Comments

@fschutt
Copy link

fschutt commented Jun 4, 2018

The problem is this: I have a Window, which stores the EventsLoop. Now, webrender, which I use for drawing, is async in nature, so if I say "render a frame", I don't know when the frame is going to be finished. For this, webrender has a RenderNotifier trait, which I can implement to call EventsLoopProxy::wakeup().

The wakeup() is called correctly, but I don't get an "awakened" event until the mouse is moved over the window. But Event::Awakened is not a window event, it doesn't have anything to do with the window. This is especially noticable when resizing windows:

The code where I am trying this is here:

RenderNotifier: https://github.com/maps4print/azul/blob/402f21d72e012530f07156044def701d79043321/src/window.rs#L318-L334

Submitting new frames: https://github.com/maps4print/azul/blob/402f21d72e012530f07156044def701d79043321/src/app.rs#L567-L568

Polling: https://github.com/maps4print/azul/blob/402f21d72e012530f07156044def701d79043321/src/app.rs#L159-L164

Handling the awakened event: https://github.com/maps4print/azul/blob/402f21d72e012530f07156044def701d79043321/src/app.rs#L517-L520

I poll indefinitely (but not using run_forever(), because reasons). In theory it should always print:

generating frame...
sending wakeup event...
received awakened event...

However, all I get is the first two messages, only if I move the mouse over the window, then it updates the screen. If the mouse is over the window, but doesn't move, it doesn't send the Awakened event either. You can clone the repository and run cargo run --example debug to replicate this.

Initial state:

image

After resizing:

image

After moving the mouse over the window:

image

The problem is that while I could swap the buffers at 60FPS, this blocks until the backbuffer is finished, so I'd be waiting on webrender to finish, which blocks events to be processed immediately and leads to lagging when resizing the window, so I'd rather not do that. I'm currently experiencing this on Windows 8, I'm not sure if this also happens on other windowing systems.

@francesca64 francesca64 added B - bug Dang, that shouldn't have happened DS - windows C - needs investigation Issue must be confirmed and researched labels Jun 5, 2018
@francesca64
Copy link
Member

I would've appreciated a heads up that this requires nightly, since things compile pretty slowly on my laptop. Maybe you could add a rust-toolchain file?

Anyway, I can reproduce this on Windows 10. Do you only have this issue with trying to awaken after resizing, or is it with any event?

@francesca64
Copy link
Member

If you run this:

extern crate winit;

use std::sync::{Arc, Mutex};

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

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

    let proxy = events_loop.create_proxy();

    let count = Arc::new(Mutex::new(0usize));
    {
        let count = Arc::clone(&count);
        std::thread::spawn(move || {
            // Wake up the `events_loop` once every second.
            loop {
                std::thread::sleep(std::time::Duration::from_secs(1));
                proxy.wakeup().unwrap();
                *count.lock().unwrap() += 1;
            }
        });
    }

    events_loop.run_forever(|event| {
        match event {
            winit::Event::Awakened => {
                println!("WAKE UP {}", count.lock().unwrap());
                winit::ControlFlow::Continue
            }
            winit::Event::WindowEvent { event: winit::WindowEvent::CloseRequested, .. } =>
                winit::ControlFlow::Break,
            _ => winit::ControlFlow::Continue,
        }
    });
}

You'll see that any wakeup messages sent during a resize are eaten. If your issue is specific to resizing (which has special handling), then it seems quite likely to be related to this.

@fschutt
Copy link
Author

fschutt commented Jun 5, 2018

It shouldn't require nightly... rustc 1.26 stable works for me, which is also what I'm using for CI. What problems did you have with that? Maybe you had an older compiler installed (1.26 is rather recent)?

Your example works for me, too - the timer is incremented, but the wakeup event is ignored. This example confirms the bug, no? I do not know if other events "eat" the wakeup events, too. For now, if it would just work properly during resize events, that'd be great.

That would explain why the screen doesn't even update after the resize is finished - the wakeup event gets eaten, and the events loop never knows that the next frame is ready and it should just swap the buffers.

But I'm curious why the events loop is different for resizing vs custom events. Isn't it just one big queue that both the resize event (called by Windows) and the .wakeup() have access to?

@francesca64
Copy link
Member

Maybe you had an older compiler installed (1.26 is rather recent)?

Ah, that's surely the case; I only use my Windows machine for working on winit, so I never really update. Sorry.

But I'm curious why the events loop is different for resizing vs custom events

So you remember #506? Well, just like that, this comes down to the same problem that #445 exists to solve. Due to how Windows works, resizing the window blocks the event processing thread (which is why we have it on a separate thread in the first place). The details are outlined more in #459.

Seeing as that complexity has foiled you twice now, I'll give you another update on the resolution timeline.

  • Last time I said that HiDPI support had to happen first, and as you know, that's going to be a solved issue by the end of this week.
  • Since we've managed to converge on a new EventLoop design, work can ostensibly begin right after that.
  • I promised to add a Windows raw input and macOS backend to gilrs once the owner finishes a refactor, so that's going to occupy about a week of my time at an unknown point during this month.
  • For the new EventLoop design, I'll be the one who restructures the X11, macOS, Emscripten, Android, and iOS backends, so that's going to be something of a bottleneck.
  • Besides everything else, I also have to fix bugs that get reported and do other maintainer stuff, so there could be unforeseen diversions of significant duration or quantity.

So, with all of that, this could probably be fixed by July 4th!

Alternatively, I could stick a pending_wakeup: Arc<AtomicBool> into EventsLoop like we do on X11, and then you'll get the Awakened late instead of never. That could be included in 0.15.1, so you'd have a fix this week.

@fschutt
Copy link
Author

fschutt commented Jun 6, 2018

Yeah, don't overwork yourself. Thanks for taking a look in the first place. I don't think a hotfix is that necessary right now, if it get's removed anyway, so I'd rather wait for the proper solution.

Regarding #506, I've thought about this and come to the conclusion, that Windows doesn't really leave me any other choice other than drawing my own menus (in seperate windows) - because as soon as I use a different thread, it locks up the whole app. It's not that hard when I have text rendering, etc. working. It's just that the Win32 API was written for a different century, it seems.

@francesca64 francesca64 added C - waiting on author Waiting for a response or another PR and removed C - needs investigation Issue must be confirmed and researched labels Jun 7, 2018
@francesca64 francesca64 added this to the EventsLoop 2.0 milestone Jun 7, 2018
@fschutt
Copy link
Author

fschutt commented Aug 14, 2018

Any news on this? Just asking.

@francesca64
Copy link
Member

#459 is in flux again, and at this point, my attention to winit is mostly limited to essential maintenance duties. @Osspial also seems to be busy?

@anderejd
Copy link

#459 is in flux again

I got the impression that the overall design of #459 has stabilized?

@felixrabe
Copy link
Contributor

felixrabe commented Aug 17, 2018

I think it became "in flux" again in July after #459 (comment), after which several questions / alternative proposals came in.

Edit (reading through it again took some time ☺️): As of two days ago (#459 (comment)) it seems to become stabilized / @Osspial -implemented soon.

@anderejd
Copy link

Ah right, but Now we seem to be getting close to something. I think @Osspial is doing a great job of summarizing the thread.

@francesca64
Copy link
Member

Does #634 help with this?

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 - waiting on author Waiting for a response or another PR DS - windows
Development

Successfully merging a pull request may close this issue.

4 participants