winit icon indicating copy to clipboard operation
winit copied to clipboard

Rework event handling to be based on a trait

Open madsmtm opened this issue 7 months ago • 5 comments

Part of https://github.com/rust-windowing/winit/issues/3367, opening to discuss separately.

Winit needs some way for the user to synchronously respond to events with a value.

Prominent examples include:

  • the dead key API (it goes via globals now).
  • https://github.com/rust-windowing/winit/issues/2921
  • https://github.com/rust-windowing/winit/issues/3349
  • https://github.com/rust-windowing/winit/issues/2304 (requires saying that you've handled, otherwise you'll block everything)
  • https://github.com/rust-windowing/winit/issues/3192 (you need to ask during window creation)
  • Clipboard APIs (we need to pick mime-type), the same with DND.
  • Live resizes (when you can adjust the size during resize).

To do this, we propose allowing the user to implement a trait, where each method is a callback that will be called when a certain event happens. Roughly:

// API
pub trait ApplicationHandler {
    fn new_events(&mut self, event_loop: ActiveEventLoop<'_>, start_cause: StartCause);

    fn resized(
        &mut self,
        event_loop: ActiveEventLoop<'_>,
        window_id: WindowId,
        size: PhysicalSize<u32>
    ) -> bool;

    // ... Further events
}

// User code
struct App {
    window: Option<Window>,
}
impl Application for App { ... }

fn main() {
    event_loop.run(App { window: None })?;
}

Feel free to update this code-block once we narrow down the actual API.


Identified problems to which we need some sort of solution:

  • [ ] It is hard to implement middleware / "mini-batching". Prominent example: egui-winits on_window_event.
  • [ ] How do we allow dbg!(event) and similar logging that access all events.
  • [ ] How do we allow platform-specific / backend-specific events? Related: https://github.com/rust-windowing/winit/issues/3064 and https://github.com/rust-windowing/winit/issues/3474.
  • [ ] TODO: Further problems???

Implementation plan:

  • [ ] Create new user-facing handler trait ApplicationHandler, while still keeping the Event-based API around to ease transition. Initial work in https://github.com/rust-windowing/winit/pull/3386.
  • [ ] Change backends such that they do not (unnecessarily) queue events.
    • [ ] macOS
    • [ ] iOS
    • [ ] Windows
    • [ ] Web
    • [ ] Wayland
  • [ ] Move each backend to these traits internally. Example in https://github.com/rust-windowing/winit/pull/3387.
  • [ ] Implement APIs that require feedback from the user, like clipboard.

madsmtm avatar Jan 27 '24 02:01 madsmtm

Re mini-batching problem:

@daxpedda suggested a design similar to tracing-subscriber, something like an extra crate winit-subscriber, linking his in here, but he ultimately realized it isn't going to work, so we need some other way to solve it.

We discussed a potential solution at our last meeting, I'll try to write a bit more about that in the coming days.

madsmtm avatar Jan 27 '24 02:01 madsmtm

I'd like to add that for 0.30.0 we agreed that we do what the current design is doing and that tracing subscriber approach is more for 0.31.0 version. The resolution for 0.30.0 was to have a method that can will be called before each event.

kchibisov avatar Jan 27 '24 12:01 kchibisov

We discussed the mini-batching problem and the platform event trait problem today, meeting notes here.

One of us still needs to write up some more details on this, but we think it should be possible.

madsmtm avatar Feb 02 '24 15:02 madsmtm

Regarding platform-specific events, why not define the methods on all platforms but just not call them on irrelevant platform(s)? (or alternatively have them as conditionally compiled trait methods). So long as the methods had a default implementation then it would be quite usable I think.

nicoburns avatar Feb 19 '24 10:02 nicoburns

The said issue is solved for 0.31, adding subpar solution for that into 0.30 is not appealing to me. In general IDETs/vtable + as_any solves that really good, but you need to go hard on traits, as long as you still have enum Event it won't really work.

kchibisov avatar Feb 19 '24 10:02 kchibisov

As a downstream user of winit (glium), I want to file my opposition to the deprecation of the run function on event loops. It's okay if you add a second trait based API, but it increases the overhead in writing small programs that use winit. The main problem is that trait impls require you to repeat all parameters of all implemented functions, together with their types.

This causes overhead, as now you have to type out the parameters, and potentially also import the types. It makes it harder to use winit.

If you want to ignore a certain event, you can hide it in a => () match arm right now already.

est31 avatar Aug 01 '24 05:08 est31

For feedback please post in #3626.

That said, we have wrote quite a lot about the motivation to switch to traits, the simplest one is return values. I encourage any feedback to address the shortcoming of the old event loop; without an alternative solution, we are really left without a choice.

daxpedda avatar Aug 01 '24 06:08 daxpedda

I don't understand this argument, traits will be all with default impls, so you don't have to implement anything unless you want something, and once you want something, yes, you'd have to write the method down, but it's not really an issue.

Every program should implement methods/traits it needs, so in the end of the day you'll have less code to compile and less methods.

kchibisov avatar Aug 03 '24 22:08 kchibisov