Merged
Conversation
It was only processing a single event per call. The docs say > Fetches all the events that are pending, calls the callback function > for each of them, and returns. which suggests that was incorrect.
This is the same behavior as with WindowProxy::wakeup_event_loop in
previous versions.
Unfortunately, `EventsLoop::interrupt` is also the recommend way to exit
a `run_forever` loop from within the event handler callback. Pushing an
extra event on the queue in that case is simply wasteful. Changing this
would require a refactor taking one of two possible forms:
1. Add a method *in addition* to interrupt intended for waking up the
event loop
2. Add a return type to the event callback like
enum Continue { True, False }
which would be used in lieu of the atomic interrupt flag.
This makes is possible for consumers to use cargo [replace] with the latest Glutin.
Contributor
|
LGTM. Thanks for catching those! |
Ralith
approved these changes
May 10, 2017
Contributor
Author
|
Thanks for the review and the recent refactoring work :) |
For X11 interrupt, we can just use the root window which doesn't require taking a lock to find.
mitchmindtree
added a commit
to mitchmindtree/winit
that referenced
this pull request
Jun 24, 2017
Includes: - Recent removal of sync (breaking change) rust-windowing#191. - Wayland fixes: rust-windowing#190, rust-windowing#188, rust-windowing#181 - X11 fixes: rust-windowing#174, rust-windowing#178,
Merged
madsmtm
pushed a commit
to madsmtm/winit
that referenced
this pull request
Jun 11, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
poll_eventsactually drains queue as the documentation suggestsEventLoop::interruptactually wakeup the event loop. Previously it would just raise a flag without allowingrun_foreverto spin. This is the same behavior as the oldWindowProxy::wakeup_event_loopmethod.Unfortunately,
EventsLoop::interruptis also the recommend way to exit arun_foreverloop from within the event handler callback. Pushing an extra event on the queue in that case is simply wasteful. Changing this would require a refactor taking one of two possible forms:event loop
which would be used in lieu of the atomic interrupt flag. With the return value here, interrupt would only be needed from other threads.
At any rate, that's out of scope for this PR. Should I open a ticket discussing this API issue?