-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Consolidate poll_events() and run_forever() into get_event(Timeout) #231
Description
EDIT: THIS ISSUE HAS BEEN SUPERSEDED by #459.
In #219, I examined some unfortunate event handling complexities on MacOS X. That examination led me to view and re-implement both poll_events() and run_forever() as special cases of a more general API, get_event(Timeout) -> Option<Event>.
I propose:
- Refactoring each platform to expose this
get_event(Timeout)interface, - Refactoring
poll_events()andrun_forever()to be implemented in terms ofget_event(Timeout), and - Deprecating
poll_events()andrun_forever()in favor ofget_event(Timeout).
This is a user-facing API change, so I'm accompanying this proposal with a larger than usual explanation.
Receive-with-time-constraint is the fundamental operation
poll_events() allows the user to communicate "I want all the events that are ready, but I don't want you to wait". Let's call that get_event(Timeout::Now), and make the loop explicit:
// current
events_loop.poll_events(|event| {
match event {
Event::WindowEvent { event: WindowEvent::Resized(w, h), .. } => {
println!("The window was resized to {}x{}", w, h);
},
_ => ()
}
});
// proposed
while let Some(event) = events_loop.get_event(Timeout::Now) {
match event {
Event::WindowEvent { event: WindowEvent::Resized(w, h), .. }) => {
println!("The window was resized to {}x{}", w, h);
},
_ => (),
}
}run_forever() allows the user to communicate "I want all the events until I tell you to stop", which… is kind of weird, isn't it? At any rate, it's the user's intention to block indefinitely and eventually break out of the loop. Let's call that get_event(Timeout::Forever), and again push down the loop:
// current
events_loop.run_forever(|event| {
match event {
winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
winit::ControlFlow::Break
},
_ => winit::ControlFlow::Continue,
}
});
// proposed
loop {
match events_loop.get_event(Timeout::Forever) {
winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
break;
},
_ => ()
}
}Some users have additional needs, like "I want all the events between now and when I need to update my state/draw the next frame". pistoncore-glutin_window does this today by unconditionally spawning a thread that wakes up the event loop after a fixed period of time. We should be able to support it efficiently on every platform without needing extra threads, and with a timeout-centric API, we can:
// current
let events_loop_proxy = self.events_loop.create_proxy();
thread::spawn(move || {
thread::sleep(timeout);
events_loop_proxy.wakeup().ok(); // wakeup can fail only if the event loop went away
});
{
let ref mut events = self.events;
self.events_loop.run_forever(|ev| {
events.push_back(ev);
glutin::ControlFlow::Break
});
}
let event = self.events.pop_front();
// proposed
let event = events_loop.get_event(Timeout::Deadline(timeout));Timeouts are sufficient to justify a new API
Consider a game that wants to update and render at 60 fps. If the game is running faster than 60 fps, it should sleep and wait until it's time to start the next frame; if it's running slower, it should busy-wait.
This use case presently requires a calling run_forever(callback) to allow the thread to sleep, plus a second thread keeping track of time and calling Proxy::wakeup() as needed. That's wasteful and unnecessarily complex.
winit should support timeouts pattern directly. Additionally, this is a common requirement; every event loop has some way to block until a specified time, or some idiomaitc way to synthesize a timeout. winit should use this where appropriate.
Simplifies the API
winit puts user code at the bottom of the stack, and does not force the user to organize their code in terms of callbacks… except for event handling. Changing the API so that the user calls get_event(Timeout) in a loop adds consistency, and it lets the user define and manage their event loop with normal Rust statements instead of a callback that returns a ControlFlow enum.
This also makes EventsLoop look more like a futures::stream, or a generator, or many other things that users might already know. "I want to get a message from you" requires less explanation than having two different functions that take two different callbacks.
Reduces duplication
poll_events() and run_forever() have considerable overlap, and in the case of the MacOS platform, considerable code duplication. Not only can get_event(Timeout) provide more functionality, in the case of the MacOS platform, refactoring into this caused a net reduction in code.
Low risk
It's very easy to implement poll_events() and run_forever() in terms of get_event(Timeout), which means there's very little risk in deciding to move that direction.
winit can continue to provide the same poll_events() + run_forever() API to the user while refactoring platforms into get_event(Timeout) behind the scenes. Once they're all ready, winit can update the examples, deprecate poll_events() and run_forever(), and ultimately remove at some later point.
Next steps
I think this covers everything:
pub enum Timeout {
Now,
Forever,
Deadline(std::time::Duration),
}
impl EventsLoop {
pub fn get_event(Timeout) -> Option<Event> {
// …
}
}A change like this requires buy-in, and this isn't my project, so I don't know what all would need to happen. I'm happy to start by soliciting feedback, so: what does everyone think?