Skip to content

Consolidate poll_events() and run_forever() into get_event(Timeout) #231

@willglynn

Description

@willglynn

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:

  1. Refactoring each platform to expose this get_event(Timeout) interface,
  2. Refactoring poll_events() and run_forever() to be implemented in terms of get_event(Timeout), and
  3. Deprecating poll_events() and run_forever() in favor of get_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?

Metadata

Metadata

Assignees

No one assigned

    Labels

    C - waiting on authorWaiting for a response or another PRD - hardLikely harder than most tasks hereP - highVital to haveS - apiDesign and usability

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions