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

Remove RedrawEventsCleared + MainEventsCleared, and add AboutToWait #2976

Merged

Conversation

rib
Copy link
Contributor

@rib rib commented Jul 25, 2023

This is one of the follow ups for #2900, assuming that #2767 hopefully lands soon.

The idea that redraw events are dispatched with a specific ordering that makes it possible to specifically report when we have finished dispatching redraw events isn't portable and the way in which we dispatched RedrawEventsCleared was inconsistent across backends.

More generally speaking, there is no inherent relationship between redrawing and event loop iterations. An event loop may wake up at any frequency depending on what sources of input events are being listened to but redrawing is generally throttled and in some way synchronized with the display frequency.

Similarly there's no inherent relationship between a single event loop iteration and the dispatching of any specific kind of "main" event.

An event loop wakes up when there are events to read (e.g. input events or responses from a display server / compositor) and goes back to waiting when there's nothing else to read.

There isn't really a special kind of "main" event that is dispatched in order with respect to other events.

For what MainEventsCleared is used for by applications then the most portable interpretation of what applications want is to know when the event loop is about to block and wait for new events.

In practice most applications almost certainly shouldn't care about this because the frequency of event loop iterations is essentially arbitrary but it's reasonably portable to expose this.

MainEventsCleared has been renamed to AboutToWait and is the last event emitted in each iteration of the event loop.

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior

Note: the base for this PR isn't currently set to master, since it's stacked on the changes for #2975

@rib rib force-pushed the rib/pr/remove-redraw-cleared-rename-main-events-cleared branch from 9c9cf41 to 7c10ff8 Compare July 25, 2023 13:53
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is implemented correctly internally, however the top level docs are wrong, given that you've basically renamed RedrawEventsCleared, but made it in a way that it'll make sense.

And MainEventsCleared is the thing you've actually removed, not RedrawEventsCleared.

The top level docs are all wrong in the end. Basically we should say that the event is emitted when the event loop is about to go to sleep. We could also comment that you could request_redraw from it to wake up a loop once OS will think that it's a good time to draw.

@daxpedda I remember that on Web and likely on macOS due to its callback design the time between so called poll intervals could be really long. So the users should rely on redraw_requested for contiguous drawing based on the request animation frame (and on macOS based on some display link calls).

@rib that means that we should probably just say that the AboutToWait is called when the event is about to go back to waiting for events, meaning that we should drop all the mentions in relative to rendering.

We could also say that no event could occur after this event (right?).

src/event.rs Outdated Show resolved Hide resolved
src/event.rs Outdated Show resolved Hide resolved
src/event.rs Show resolved Hide resolved
src/platform_impl/linux/wayland/event_loop/mod.rs Outdated Show resolved Hide resolved
@rib
Copy link
Contributor Author

rib commented Jul 26, 2023

And MainEventsCleared is the thing you've actually removed, not RedrawEventsCleared

This is semantics.

I actually did a sed to initially rename MainEventsCleared (the command in my bash history was find ./ -iname '*.rs' -exec sed -i 's/MainEventsCleared/AboutToWait/g' {} \;) and then while going through and manually removing RedrawEventsCleared I made sure AboutToWait was the last event emitted.

The exact provenance isn't super important but at least the way that I intuitively see it is that I removed RedrawEventsCleared and renamed MainEventsCleared since that's actually what I did from a typing pov.

all that really matters here is the end result.

@kchibisov
Copy link
Member

This is semantics.

This is true, but the documentation is now wrong.

@rib rib force-pushed the rib/pr/remove-redraw-cleared-rename-main-events-cleared branch from 7c10ff8 to a4217d5 Compare July 26, 2023 10:24
@rib rib force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from 615c359 to df8eaea Compare July 26, 2023 10:24
@rib
Copy link
Contributor Author

rib commented Jul 26, 2023

Okey, I've tried to do a better job of updating the docs - sorry I didn't really do that properly before.

@rib
Copy link
Contributor Author

rib commented Jul 26, 2023

This is semantics.

This is true, but the documentation is now wrong.

With the updates I've just pushed I've generally reframed it as having removed both events and added a new AboutToWait event.

This only really affects the commit message and the changelog though.

@rib rib requested a review from kchibisov July 26, 2023 10:30
@rib
Copy link
Contributor Author

rib commented Jul 26, 2023

Although I put a small note about Web in the commit message, I wonder if we should have a small platform-specific note for AboutToWait that highlights that this is emulated on Web (similar to how MainEventsCleared was dispatched) since the real event loop is an implementation detail of the browser that we can't observe.

@rib rib changed the title Remove RedrawEventsCleared + rename MainEventsCleared to AboutToWait Remove RedrawEventsCleared + MainEventsCleared, and add AboutToWait Jul 28, 2023
@rib rib mentioned this pull request Jul 28, 2023
17 tasks
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to target a different branch now though.

@rib rib force-pushed the rib/pr/remove-redraw-cleared-rename-main-events-cleared branch from a4217d5 to d2709b6 Compare July 28, 2023 13:46
@rib rib force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from df8eaea to 4ed3e31 Compare July 28, 2023 13:46
@rib
Copy link
Contributor Author

rib commented Jul 28, 2023

We'll have to target a different branch now though.

github will automatically move the base branch along if they are merged in the order they are stacked

@kchibisov kchibisov force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from 4ed3e31 to aabc349 Compare July 28, 2023 15:56
Base automatically changed from rib/pr/rename-loop-destroyed-loop-exiting to master July 28, 2023 16:19
The idea that redraw events are dispatched with a specific ordering
that makes it possible to specifically report when we have finished
dispatching redraw events isn't portable and the way in which we
dispatched RedrawEventsCleared was inconsistent across backends.

More generally speaking, there is no inherent relationship between
redrawing and event loop iterations. An event loop may wake up at any
frequency depending on what sources of input events are being listened
to but redrawing is generally throttled and in some way synchronized
with the display frequency.

Similarly there's no inherent relationship between a single event loop
iteration and the dispatching of any specific kind of "main" event.

An event loop wakes up when there are events to read (e.g. input
events or responses from a display server / compositor) and goes back
to waiting when there's nothing else to read.

There isn't really a special kind of "main" event that is dispatched
in order with respect to other events.

What we can do more portably is emit an event when the event loop
is about to block and wait for new events.

In practice this is very similar to how MainEventsCleared was
implemented except it wasn't the very last event previously since
redraw events could be dispatched afterwards.

The main backend where we don't strictly know when we're going to
wait for events is Web (since the real event loop is internal to
the browser). For now we emulate AboutToWait on Web similar to how
MainEventsCleared was dispatched.

In practice most applications almost certainly shouldn't care about
AboutToWait because the frequency of event loop iterations is
essentially arbitrary and usually irrelevant.
@kchibisov kchibisov force-pushed the rib/pr/remove-redraw-cleared-rename-main-events-cleared branch from d2709b6 to 9c7f440 Compare July 28, 2023 16:20
@kchibisov kchibisov merged commit ae7497e into master Jul 28, 2023
55 checks passed
@kchibisov kchibisov deleted the rib/pr/remove-redraw-cleared-rename-main-events-cleared branch July 28, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants