rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.76k stars 892 forks source link

Only run observers in the default run loop mode #3767

Open madsmtm opened 3 months ago

madsmtm commented 3 months ago

AppKit/UIKit has a concept of "run loops", which are effectively a queue of events that are repeatedly consumed from, and when empty, the consuming thread is std::thread::parked until new events appear.

Apple has locked the exact details of these down quite tightly, especially on iOS, which is part of the reason why everything must be in a callback there instead of the original pump_events API, but effectively this is the basis of our event loop.

We are still given the option to listen to certain events, however! And this is how we implement the new_events/about_to_wait events, as well as the proxy_wake_up event.

One somewhat odd thing about run loops is that they support being run re-entrantly / in a nested fashion! In this way, the system takes events out of the queue that the nested caller requests, and keeps the rest of the events queued up until the nested caller returns and the outer caller can now process the queued events. Internally, it looks roughly like the following:

'outer loop {
    match run_loop.get_event_matching(_) {
        ClickOnCornerOfWindow => {
            'inner loop {
                match run_loop.get_event_matching(ReleaseClick | MouseMoved) {
                    ReleaseClick => break 'inner,
                    MouseMoved => window.resized(),
                    _ => unreachable!(),
                }
            }
        }
        ClickOnCloseButton => {
            break 'outer;
        }
        _ => {} // Other events
    }
}

Notably, both calls to the fictional "get_event_matching" can end up parking the thread when there is no more work to be done. Now, what this means for us is that we're given basically two options for how to emit new_events/about_to_wait/proxy_wake_up:

  1. Emit it from inside each "get_event_matching" (kCFRunLoopCommonModes).
  2. Emit it only from the outer event loop's "get_event_matching" (kCFRunLoopDefaultMode).

Nested run loops like these are used notably in the following instances: A. When opening e.g. a file dialog with the rfd crate. B. When clicking the corner of the window and dragging to live-resize it.

I am unsure what the desired behaviour is for new_events/about_to_wait/proxy_wake_up? In case A, the file dialog is spawned from inside the event loop itself, and as such we cannot emit the event (since that would be re-entrant). But in case B, we might want to emit the new_events/about_to_wait/proxy_wake_up, even though that is not technically what is happening with our outer event loop.

I felt like the most consistent answer would be that we always use kCFRunLoopDefaultMode when originally writing the PR, though then again, maybe resizing should be considered an implementation detail here, and we should just paper over it (like we somewhat currently do)? CC @daxpedda @kchibisov, I'd like input on this! Would the user still expect new_events/about_to_wait to be emitted when live-resizing a window, even though the application is still somewhat "alive" at this stage?

(In any case, I will first have to fix calling request_redraw inside of RedrawRequested, since the approach of calling it inside about_to_wait would break resizing with this PR in its current form).

TODO: