Remove Sync bound from EventsLoop - Add a proxy type as the exclusive way of interrupting the loop from other threads.

[mitchmindtree]

This is a follow up to the discussion in #136 and for tracking the change discussed here. Related to #175 cc @jwilm.

[jwilm]

What is the purpose of the interrupt method from the Proxy? Seems like only the thread running the EventLoop needs to be able to decide when to stop processing in run_forever or poll_events. Will Proxy::interrupt be more like a wakeup as it was before EventsLoop existed?

[mitchmindtree]

The interrupt method on the proxy type would mimic the behaviour the current EventsLoop behaviour.

What is the purpose of the interrupt method from the Proxy?

As an example, I tend to run my GUI on a separate thread in order to free up main for rendering. When my GUI is ready to draw (as the result of some update), I use a channel to send the batch of graphics primitives from the GUI thread to the main thread. Normally my EventsLoop is blocking on run_forever, so once I've pushed my graphics primitives to the channel I call interrupt from the GUI thread causing the EventsLoop to "break", check the receiving end of the channel, draw the pending primitives and then loop back to waiting for events on run_forever.

[jwilm]

In your use case, it sounds like you use both the interrupt feature (which exits run_forever) and the wakeup feature which actually allows run_forever to advance in the first place.

In Alacritty, there are two threads: the EventsLoop thread and a second thread for handling interactions with the pty. Whenever data arrives from the pty, the parser runs and updates the terminal state. At this point, it signals back to the EventsLoop thread to wakeup. From that perspective, both the interrupt and wakeup functionality are needed.

The EventsLoop thread works roughly like this:

1. block on run_forever
   1. break after first event is received (maybe interrupt() should be called break() to increase similarity to a traditional loop?)
2. poll_events until queue is empty
3. draw if terminal state changed

In that case, pushing a wakeup event on the queue is wasteful, which was the point of the proposal in #175 to separate them.

Under this system, your GUI thread would call wakeup() on the proxy, and the event handler in run_forever could break on WindowEvent::Awakened.

[mitchmindtree]

Ah yes, reading over this and over #175 again I can see how it could be useful to divide up this behaviour (even though I personally haven't come across the need yet). As I understand it, the two behaviours would be split up like this:

• interrupt() - Exists only as a method on the EventsLoop and simply causes the loop to immediately exit poll_events or run_forever once the end of the current callback scope is reached.
• wakeup() - Exists only as a method on the proxy type and causes an Awakened event to be emitted by the EventsLoop

Does this sound correct? If so I think I'd be happy with this change 👍

(maybe interrupt() should be called break() to increase similarity to a traditional loop?)

I agree this would be nice, however I don't think this is possible due to the use of break as a keyword. Edit: perhaps events_loop.exit() would make a nice alternative?

[jwilm]

Does this sound correct? If so I think I'd be happy with this change

Yes! 😄

I agree this would be nice, however I don't think this is possible due to the use of break as a keyword.

Oh, right. At the point we can't simply use break, maybe there's no need to change it. The alternatives aren't that much better. Although, maybe something slightly more verbose like break_events_loop could work? This would parallel the labelled break syntax break 'events_loop.

[mitchmindtree]

Should Awakened be a variant of the Event enum or the WindowEvent enum?

I'm leaning towards the Event enum as I don't really see what waking the EventsLoop has to do with specific windows. I'm guessing the Awakened variant that currently exists in WindowEvent is just there as it never got moved following the addition of the more general Event enum (the TODO on it says it was going to be removed anyways). I could be wrong though!

[jwilm]

Previously with WindowProxy, there was always just one event loop per window, and thus the Awakened event was always for that window. I suppose you could say it's the application's responsibility to track some sort of state to identify which window it was intended for, but winit already exposes WindowId which is perfect for that.

In addition to it being helpful for winit consumers, the underlying APIs expect in some cases to have the event directed at a particular window. For X11, it's required to provide the window id to send the interrupt event.

The API I was toying with locally for wakeup looked like this:

fn wakeup(&self, window: WindowId);

Maybe the case is much different for other backends, though?