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

Re-work event loop run APIs: adds run -> Result<>, run_ondemand and pump_events #2767

Merged
merged 15 commits into from
Jul 27, 2023

Commits on Jul 25, 2023

  1. Add EventLoopExtPumpEvents and EventLoopExtRunOnDemand

    This adds two new extensions for running a Winit event loop which will
    replace `EventLoopExtRunReturn`
    
    The `run_return` API is trying to solve multiple problems and address
    multiple, unrelated, use cases but in doing so it is not succeeding
    at addressing any of them fully.
    
    The notable use cases we have are:
    1. Applications want to be able to implement their own external
       event loop and call some Winit API to poll / pump events, once
       per iteration of their own loop, without blocking the outer,
       external loop. Addressing rust-windowing#2706
    2. Applications want to be able to re-run separate instantiations
       of some Winit-based GUI and want to allow the event loop to exit with
       a status, and then later be able to run the loop again for a new
       instantiation of their GUI. Addressing rust-windowing#2431
    
    It's very notable that these use cases can't be supported across
    all platforms and so they are extensions, similar to
    `EventLoopExtRunReturn`
    
    The intention is to support these extensions on:
    - Windows
    - Linux (X11 + Wayland)
    - macOS
    - Android
    
    These extensions aren't compatible with Web or iOS though.
    
    Each method of running the loop will behave consistently in terms of how
    `NewEvents(Init)`, `Resumed` and `LoopDestroyed` events are dispatched
    (so portable application code wouldn't necessarily need to have any awareness
    of which method of running the loop was being used)
    
    Once all backends have support for these extensions then we can
    remove `EventLoopExtRunReturn`
    
    For simplicity, the extensions are documented with the assumption that
    the above platforms will be supported.
    
    This patch makes no functional change, it only introduces these new
    extensions so we can then handle adding platform-specific backends
    in separate pull requests, so the work can be landed in stages.
    rib committed Jul 25, 2023
    Configuration menu
    Copy the full SHA
    54b41ff View commit details
    Browse the repository at this point in the history

Commits on Jul 26, 2023

  1. Configuration menu
    Copy the full SHA
    9ff6a42 View commit details
    Browse the repository at this point in the history
  2. Windows: Implement EventLoopExtPumpEvents and EventLoopExtRunOnDemand

    A surprising amount of work was required to enable these extensions
    on Windows.
    
    I had originally assumed that pump_events was going to be very similar
    to run except would use PeekMessageW instead of GetMessageW to avoid
    blocking the external loop but I found the Windows backend broke
    several assumptions I had.
    
    Overall I think these changes can hopefully be considered a quite a
    significant simplification (I think it's a net deletion of a fair amount
    of code) and I think it also helps bring it into slightly closer alignment
    with other backends too
    
    Key changes:
    - I have removed the `wait_thread` that was a fairly fiddly way of handling
      `ControlFlow::WaitUntil` timeouts in favor of using `SetTimer` which works
      with the same messages picked up by `GetMessage` and `PeekMessage`.
    - I have removed the ordering guarantees between `MainEventsCleared`,
      `RedrawRequested` and `RedrawEventsCleared` events due to the complexity in
      maintaining this artificial ordering, which is already not supported
      consistently across backends anyway (in particular this ordering already
      isn't compatible with how MacOS / iOS work).
    - `RedrawRequested` events are now directly dispatched via `WM_PAINT` messages
      - comparable to how `RedrawRequested` is dispatched via `drawRect` in the
      MacOS backend.
    - I have re-worked how `NewEvents`, `MainEventsCleared`, and `RedrawEventsCleared`
      get dispatched to be more in line with the MacOS backend and also more in line
      with how we have recently discussed defining them for all platforms.
    
      `NewEvents` is conceptually delivered when the event loop "wakes up" and
      `MainEventsCleared` gets dispatched when the event loop is about to ask the
      OS to wait for new events.
    
      This is a more portable model, and is already how these events work in the
      MacOS backend.
    
      `RedrawEventsCleared` are just delivered after `MainEventsCleared` but this
      event no longer has a useful meaning.
    
    Probably the most controversial thing here is that this "breaks" the ordering
    rules for redraw event handling, but since my changes interacted with how the
    order is maintained I was very reluctant to figure out how to continue
    maintaining something that we have recently been discussing changing:
    
    rust-windowing#2640.
    
    Additionally, since the MacOS backend already doesn't strictly maintain this
    order it's somewhat academic to see this as a breakage if Winit applications
    can't really rely on it already.
    
    This updates the documentation for `request_redraw()` to reflect that we
    no longer guarantee that `RedrawRequested` events must be dispatched
    after `MainEventsCleared`.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    80deeb3 View commit details
    Browse the repository at this point in the history
  3. MacOS: Implement EventLoopExtPumpEvents and EventLoopExtRunOnDemand

    The implementation of `pump_events` essentially works by hooking into the
    `RunLoopObserver` and requesting that the app should be stopped the next time
    that the `RunLoop` prepares to wait for new events.
    
    Originally I had thought I would poke the `CFRunLoop` for the app directly and
    I was originally going to implement `pump_events` based on a timeout which I'd
    seen SDL doing.
    
    I found that `[NSApp run]` wasn't actually being stopped by asking the RunLoop
    to stop directly and inferred that `NSApp run` will actually catch this and
    re-start the loop.
    
    Hooking into the observer and calling `[NSApp stop]` actually seems like a
    better solution that doesn't need a hacky constant timeout.
    
    The end result is quite similar to what happens with existing apps that
    call `run_return` inside an external loop and cause the loop to exit for
    each iteration (that also results in the `NSApp` stopping each
    iteration).
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    82f9108 View commit details
    Browse the repository at this point in the history
  4. Linux: Implement EventLoopExtPumpEvents and EventLoopExtRunOnDemand

    Wayland:
    
    I found the calloop abstraction a little awkward to work with while I was
    trying to understand why there was surprising workaround code in the wayland
    backend for manually dispatching pending events.
    
    Investigating this further it looks like there may currently be several issues
    with the calloop WaylandSource (with how prepare_read is used and with (not)
    flushing writes before polling)
    
    Considering the current minimal needs for polling in all winit backends I do
    personally tend to think it would be simpler to just own the responsibility for
    polling more directly, so the logic for wayland-client `prepare_read` wouldn't
    be in a separate crate (and in this current situation would also be easier to fix)
    
    I've tried to maintain the status quo with calloop + workarounds.
    
    X11:
    
    I found that the recent changes (4ac2006) to port the X11 backend
    from mio to calloop lost the ability to check for pending events before
    needing to poll/dispatch. (The `has_pending` state being queried
    before dispatching() was based on state that was filled in during
    dispatching)
    
    As part of the rebase this re-introduces the PeekableReceiver and
    WakeSender which are small utilities on top of
    `std::sync::mpsc::channel()`. This adds a calloop `PingSource`
    so we can use a `Ping` as a generic event loop waker.
    
    For taking into account false positive wake ups the X11 source now
    tracks when the file descriptor is readable so after we poll via
    calloop we can then specifically check if there are new X11 events
    or pending redraw/user events when deciding whether to skip the
    event loop iteration.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    798a6f0 View commit details
    Browse the repository at this point in the history
  5. Remove EventLoopExtRunReturn

    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    5a80523 View commit details
    Browse the repository at this point in the history
  6. Re-work event loop run() API so it can return a Result

    This re-works the portable `run()` API that consumes the `EventLoop` and
    runs the loop on the calling thread until the app exits.
    
    This can be supported across _all_ platforms and compared to the
    previous `run() -> !` API is now able to return a `Result` status on all
    platforms except iOS and Web. Fixes: rust-windowing#2709
    
    By moving away from `run() -> !` we stop calling `std::process::exit()`
    internally as a means to kill the process without returning which means
    it's possible to return an exit status and applications can return from
    their `main()` function normally.
    
    This also fixes Android support where an Activity runs in a thread but
    we can't assume to have full ownership of the process (other services
    could be running in separate threads).
    
    Additionally all examples have generally been updated so that `main()`
    returns a `Result` from `run()`
    
    Fixes: rust-windowing#2709
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    8a1a815 View commit details
    Browse the repository at this point in the history
  7. Add examples/window_pump_events

    A minimal example of an application based on an external event loop that
    calls `pump_events` for each iteration of the external loop.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    dc75dab View commit details
    Browse the repository at this point in the history
  8. Add examples/window_ondemand

    A minimal example that shows an application running the event loop more
    than once via `run_ondemand`
    
    There is a 5 second delay between each run to help highlight problems
    with destroying the window from the first loop.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    d32841b View commit details
    Browse the repository at this point in the history
  9. Update CHANGELOG.md

    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    7215843 View commit details
    Browse the repository at this point in the history
  10. Linux: Sync with server/compositor before exiting run_ondemand

    Although we document that applications can't keep windows between
    separate run_ondemand calls it's possible that the application has only
    just dropped their windows and we need to flush these requests to the
    server/compositor.
    
    This fixes the window_ondemand example - by ensuring the window from
    the first loop really is destroyed before waiting for 5 seconds
    and starting the second loop.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    c4eb692 View commit details
    Browse the repository at this point in the history
  11. window_ondemand: wait for Destroyed event before exiting app

    Considering the strict requirement that applications can't keep windows
    across run_ondemand calls, this tries to make the window_ondemand example
    explicitly wait for its Window to be destroyed before exiting each
    run_ondemand iteration.
    
    This updates the example to only `.set_exit()` after it gets a
    `Destroyed` event after the Window has been dropped.
    
    On Windows this works to ensure the Window is destroyed before the
    example waits for 5 seconds.
    
    Unfortunately though:
    1. The Wayland backend doesn't emit `Destroyed` events for windows
    2. The macOS backend emits `Destroyed` events before the window is
       really destroyed.
    
    and so the example isn't currently portable.
    rib committed Jul 26, 2023
    Configuration menu
    Copy the full SHA
    c36bee8 View commit details
    Browse the repository at this point in the history

Commits on Jul 27, 2023

  1. MacOS: implement pump_events_with_timeout internally

    This layers pump_events on a pump_events_with_timeout API, like we have
    for Linux and Android.
    
    This is just an internal implementation detail for now but we could
    consider making pump_events_with_timeout public, or just making it so
    that pump_events() takes the timeout argument.
    rib authored and kchibisov committed Jul 27, 2023
    Configuration menu
    Copy the full SHA
    4f095f0 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    3f67f94 View commit details
    Browse the repository at this point in the history
  3. Add timeout argument to pump_events

    This renames all internal implementations of pump_events_with_timeout
    to pump_events and makes them public.
    
    Since all platforms that support pump_events support timeouts there's
    no need to have a separate API.
    rib authored and kchibisov committed Jul 27, 2023
    Configuration menu
    Copy the full SHA
    f2bd7e6 View commit details
    Browse the repository at this point in the history