control_flow by &mut rationale
#1018
Comments
goddessfreya
added
S - meta
|
Thanks for the question! I've actually answered this before, but the answer is buried pretty deeply in the original API discussion so it's pretty hard to find. I'll add it to the FAQ in the announcement post, but I'll also copy it here for posterity:
|
|
Thank you for answering. Now I'm wondering if I should put my update and render code (which are coupled 1:1 for now) in event_loop.run(|events| {
let mut keep_running = true;
let mut mouse_dx = 0; // accumulate multiple mouse move events.
for event in events {
// handle window closed etc. add mouse_dx
}
if keep_running { ControlFlow::Poll } else { ControlFlow::Exit } // or yield until next update/render start
// simulation update
// render
// swap buffers (blocks if vsync enabled)
})As far as I understand the main reason for having glutin take ownership of the thread because some of the underlying api's require that. My intuition tells me that the above design still does that, while being a more natural translation of the ideal |
It shouldn't matter a whole lot, but I'd personally put them in As far as the design you proposed - I don't recall that specific design ever getting discussed. There are a couple reasons I prefer the current design, though:
(There are nuances to all of those answers that I didn't bring up for the sake of brevity and not diving too far into the technical details, but I'll happily talk about those if you'd like). That being said, I'd agree that the API you describe is easier to use in a lot of ways. I've actually been doing work recently to use Async/Await to expose something similar to what you've described there: event_loop.run_async(async move |mut runner| {
'main: loop {
let mut recv_events = runner.recv_events().await;
while let Some(event) = recv_events.next().await {
match event {
Event::WindowEvent {
event: WindowEvent::CloseRequested,
..
} => {
break 'main;
},
_ => println!("\t{:?}", event),
}
}
window.request_redraw();
let mut redraw_requests = recv_events.redraw_requests().await;
while let Some(window_id) = redraw_requests.next().await {
println!("\tredraw {:?}", window_id);
}
println!();
}
})However, I've ended up uncovering quite a few bugs in our current code when implementing it, so I haven't published it quite yet. |
I would love to, especially because glutin is making this big step and I want to understand and embrace its new design. With that said, I would like to challenge you on the points you presented so that I can come to understand the new design.
In that case, can't glutin can emit an iterator of exactly 1 element? It is still in control of calling the
I've tried to imagine what glutin needs and wrote this implementation that allows events with lifetimes: // NOTE: Hide the std::vec::Drain by wrapping the iterator in a newtype.
pub fn run<F>(mut event_handler: F) -> !
where
F: 'static + for<'e> FnMut(std::vec::Drain<Event<'e>>) -> ControlFlow,
{
let resource = Resource {
name: String::from("I am some resource.")
};
let mut event_buffer: Vec<Event> = Vec::new();
loop {
// Collect events.
event_buffer.push(Event::Update(&resource));
// Emit events.
match event_handler(event_buffer.drain(0..)) {
ControlFlow::Poll => continue,
ControlFlow::Exit => break,
}
}
std::process::exit(0);
}
fn main() {
run(|event_iter| {
let mut should_exit = false;
for event in event_iter {
match event {
Event::Update(res) => {
println!("{}", res.name);
should_exit = true;
}
}
}
if should_exit {
ControlFlow::Exit
} else {
ControlFlow::Poll
}
})
}Full code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1d4c198e186438555b3fc4ed37ed2534
Intuitively I think it is easier to emit single events when you have an iterator rather collecting single events. An example implementation using the above implementation of pub fn run_single<F>(mut event_handler: F) -> !
where
F: 'static + for<'e> FnMut(Event<'e>, &mut ControlFlow),
{
run(move |event_iter| {
let mut control_flow = ControlFlow::Poll;
for event in event_iter {
event_handler(event, &mut control_flow);
}
control_flow
})
}
fn main() {
run_single(|event, control_flow| {
match event {
Event::Update(res) => {
println!("{}", res.name);
*control_flow = ControlFlow::Exit;
}
}
})
}As a final note, the design at point 3 looks nicer for this example. However it doesn't show the, confusion / multiple possible implementations, that the event loop events (EventStart/EventsCleared) introduce. Nor does the example accumulate state from the events before doing an actual simulation update. |
|
Sorry about the delay - it took a while for me to find the time to answer this properly.
See #1041 for info on how exactly smooth resizing works, and the technical reasons why the current
Strictly speaking, yes. However, that makes it really hard to write an event loop that functions in a sane way. Let's go over a couple potential implementations to see what the problems are, in order to figure out how to structure things properly: This first example is pretty much a direct copy of the first model you suggested, and I'd expect is the first way most people would think to do things: event_loop.run(|events| {
let mut keep_running = true;
let mut mouse_dx = 0; // accumulate multiple mouse move events.
for event in events {
// handle window closed etc. add mouse_dx
}
// simulation update
// render
// swap buffers (blocks if vsync enabled)
if keep_running { ControlFlow::Poll } else { ControlFlow::Exit } // or yield until next update/render start
})However, every time the OS requests a redraw (or, if the program uses Let's try another approach: event_loop.run(|events| {
let mut keep_running = true;
let mut mouse_dx = 0; // accumulate multiple mouse move events.
let mut redraw = false;
for event in events {
match event {
Event::RedrawRequested => redraw = true,
_ => {/* handle window closed etc. add mouse_dx */}
}
}
// simulation update
if redraw {
// render
// swap buffers (blocks if vsync enabled)
}
if keep_running { ControlFlow::Poll } else { ControlFlow::Exit } // or yield until next update/render start
})That fixes the duplicate rendering, but doesn't fix the duplicate simulation updates. A third approach, which only does simulation if the render flag is checked: event_loop.run(|events| {
let mut keep_running = true;
let mut mouse_dx = 0; // accumulate multiple mouse move events.
let mut update_and_redraw = false;
for event in events {
match event {
Event::RedrawRequested => update_and_redraw = true,
_ => {
// handle window closed etc. add mouse_dx
//
// We can't actually set the `update_and_redraw` flag here, since that just runs
// into the duplicate update/render issue in the first example.
// Instead, we have to call `request_redraw`, which Winit can hopefully turn into
// a single-event iterator that sets `update_and_redraw` and (mostly) properly handles
// updating.
window.request_redraw();
}
}
}
if update_and_redraw {
// simulation update
// render
// swap buffers (blocks if vsync enabled)
}
if keep_running { ControlFlow::Poll } else { ControlFlow::Exit } // or yield until next update/render start
})That should work for single-window applications, but still leads to duplicate updates when you have multiple windows. I'm pretty sure there are ways around that limitation, but I think my point has been made: it's really hard to structure a loop properly in this scenario, and this structure leads to myriad footguns. It'd be irresponsible of us to expose that.
The issue with that example is that it doesn't match up with how the OS structures its event loop. I've adapted that into a more accurate example here (playground link): enum OsEvent<'e> {
Update(&'e Resource)
}
fn handle_os_event(_handle_event: impl FnOnce(OsEvent<'_>)) -> bool {
unimplemented!("get event from OS and dispatch to handler closure")
}
// NOTE: Hide the std::vec::Drain by wrapping the iterator in a newtype.
pub fn run<F>(mut event_handler: F) -> !
where
F: 'static + for<'e> FnMut(std::vec::Drain<Event<'e>>) -> ControlFlow,
{
let mut event_buffer: Vec<Event> = Vec::new();
loop {
loop {
// Collect events.
//
// This may seem like a contrived way to structure the event loop,
// but it matches up pretty closely with how the OS actually
// structures its event loop.
let more_os_events = handle_os_event(|event| {
match event {
OsEvent::Update(resource) => event_buffer.push(Event::Update(resource)),
_ => {/* handle everything else */}
}
});
if !more_os_events {
break;
}
}
// Emit events.
match event_handler(event_buffer.drain(..)) {
ControlFlow::Poll => continue,
ControlFlow::Exit => break,
}
}
std::process::exit(0);
}You'll notice that it doesn't compile. The Rust compiler rightfully rejects it, since the borrowed As far as exact technical details go, check out this branch to see a real example of the lifetime issue in our codebase: winit/src/platform_impl/windows/event_loop.rs Lines 1514 to 1593 in 3a449af Also, here's a link to the actual implementation of the Windows event loop. There's a bit more indirection in the actual implementation, but structurally it's similar to what I posted above: winit/src/platform_impl/windows/event_loop.rs Lines 202 to 239 in 39e668f
I'll agree that this is a more intuitive model for the event loop, and if it were possible to expose it without compromises I'd do it. However, it results in losing out on the features and correctness improvements outlined above. |
|
Thank you for the write up. I am glad you've found the time and the effort is much appreciated! I like the idea of a single event that tells you when to draw, but I am concerned that more advanced applications, which need to interweave multiple event loops (like a file watcher or networking) and might use other API's (like OpenVR which requires a blocking call which does a predictive wait, trying to ensure the latest possible state when rendering) will be harder to author. If I find the time I should try to implement them using the new API.
So if I understand correctly, the goal is to debounce drawing operations. What I do not understand is why winit has to emit an iterator for the user input events, and then another iterator for the single render event. If you could merge them, then this is no longer a problem right?
In that case instead of using an external iterator we could expose an internal iterator (callback) like so: fn main() {
run(|event_group| {
let mut should_exit = false;
event_group.for_each(|event| {
match event {
Event::Update(res) => {
println!("{}", res.name);
should_exit = true;
}
}
});
if should_exit {
ControlFlow::Exit
} else {
ControlFlow::Poll
}
})
}This iterator is no longer composable like rust's iterators but it can yield events with references. Additionally you leverage the OS's buffer and don't need to create another buffer yourself, unless we need to merge events. |
I would like to understand the rationale for having
control_flowas a mutable parameter rather than the return value in theEventLoop::runand similar methods. I am confused because when returned by value it is harder to forget to set it. What are the downsides?The text was updated successfully, but these errors were encountered: