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.

Sign up for GitHub

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

Jump to bottom

Rework winit runner #9034

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Rework winit runner #9034

wants to merge 13 commits into from

Conversation

B-head
Copy link
Contributor

@B-head B-head commented Jul 3, 2023 •
edited
Loading

This pull request is stacked on #8976.
Resolves #1343, Resolves #5984, Resolves #9087, Fixes #5713, Fixes #8080, Fixes #9040, Closes #7611.

Objective

  • Separate tick updates and redraws.
    • The tick rate and frame rate can be set separately.
    • One can be run without the other.
    • Many tick updates can be done even when VSync is enabled.
  • Automatically limit redraws to match the monitor refresh rate even when VSync is disabled.
  • Provide a better control method for reactive redraw.
  • Support tick auto-update stops and step tick updates.
  • Respond to input events as quickly as possible.

Code example here.

Solution

  • Set ControlFlow::WaitUntil to the next tick update or redraw, whichever is closer.
  • Send events for control from resource WinitHandler, which contains EventLoopProxy.
  • Prepare for changes in EventLoop 3.0 Changes rust-windowing/winit#2900.

Redefine top schedules

The top schedule was redefined so that fine control would not be impossible.

  • Schedule Main was split into StartupFlow and UpdateFlow.
    • The names of the sub-schedules (Startup, Update, etc.) have not been changed, so in many cases no migration is necessary.
  • Add Control schedule.
    • Each time an event is received from windows and devices, this schedule is run.
    • This is useful for responding to events regardless of whether tick updates take place.
  • Add FrameReady schedule.
    • Each time a frame is ready to be updated, this schedule is run.
    • This is the best place to decide whether to redraw.
  • Rename Render schedule to RenderFlow.

Schedules that run sub-schedules are now named uniformly as ***Flow.

To do

Known Issues

  • The current event system does not conform well to this feature.
  • TextBundle randomly fails to initialize the character textures.

If you encounter any other issues, please let us know.

Changelog

To be written later.

Migration Guide

  • The tick auto-update has been changed so that it is no longer enabled at startup.
    • In the early stages of development, it is useful to add a system that calls WinitHandler::run to PostStartup schedule. 
      .add_systems(PostStartup, |mut handler: ResMut<WinitHandler>| { handler.run(); })
  • Confirmation is needed as WinitSettings has been redefined in the rework.
  • Note that code which does not expect tick updates and redraws to be performed asynchronously will cause bugs.

B-head added 6 commits June 27, 2023 03:34
@B-head
Relaxed runner type from Fn to FnOnce
5685ec3
@B-head
Move Render label
a919628
@B-head
Implement Sync to App
e10458b 
Want to include `SubApp` in a system function,
but `ExclusiveFunctionSystem` requires `Sync`.
@B-head
Move render runs from update() to Schedule
f210bed
@B-head
Replace App::update() to World::run_schedule()
91bb64d 
Moved the responsibility for running top-level schedules
from `App` struct to the runner function.
@B-head
First aid bug fix
28bcb95
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Windowing Platform-agnostic interface layer to run your app in C-Code-Quality A section of code that is hard to understand or change D-Complex Quite challenging from either a design or technical perspective. Ask for help! and removed C-Code-Quality A section of code that is hard to understand or change labels Jul 4, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Jul 4, 2023
B-head added 6 commits July 8, 2023 07:27
@B-head
Separate tick rate and frame rate
ede9e64 
Set `ControlFlow::WaitUntil` to the next tick update or redraw,
whichever is closer.

Prepare for changes in
rust-windowing/winit#2900.
@B-head
Add resource to control winit runner
0f21d9c 
Send events for control from resource `WinitHandler`,
which contains `EventLoopProxy`.
@B-head
Add prelude to the winit module
122a0d9
@B-head
Rename the main schedule label
4b3b5b5
@B-head
Add top-level schedules
ebbfd6c 
The top schedule was redefined so that fine control would not be impossible.

- Schedule `Main` was split into `StartupFlow` and `UpdateFlow`.
  - The names of the sub-schedules (`Startup`, `Update`, etc.) have not been changed, so in many cases no migration is necessary.
- Add `Control` schedule.
  - Each time an event is received from windows and devices, this schedule is run.
  - This is useful for responding to events regardless of whether tick updates take place.
- Add `FrameReady` schedule.
  - Each time a frame is ready to be updated, this schedule is run.
  - This is the best place to decide whether to redraw.
- Rename `Render` schedule to `RenderFlow`.

Schedules that run sub-schedules are now named uniformly as `***Flow`.
@B-head
Demonic revamp to example low_power
fe794a8
@B-head B-head force-pushed the rework-winit-runner branch from c77e100 to d2416a4 Compare July 8, 2023 00:57
@B-head B-head force-pushed the rework-winit-runner branch from d2416a4 to 557408e Compare July 8, 2023 04:18
@B-head B-head mentioned this pull request Jul 10, 2023
Merged
14 tasks
@adsick
Copy link
Contributor

adsick commented Jul 17, 2023

I believe this will also fix #4669, right?

@nicopap nicopap self-requested a review July 29, 2023 15:26
@mockersf mockersf mentioned this pull request Aug 2, 2023
Merged
@Vrixyz
Copy link
Member

Vrixyz commented Aug 24, 2023

Should this PR be given priority over #8745 ? I'm starting to hit complex migrations on my PR, impacting event loop.

@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Sep 12, 2023
@Telov
Copy link

Telov commented Sep 19, 2023 •
edited
Loading

@B-head I think you can remove the uncertainty of what time PR to adapt to. 2 of them are closed in favor of the last one.

@alice-i-cecile alice-i-cecile removed this from the 0.12 milestone Sep 29, 2023
@Maximetinu Maximetinu mentioned this pull request Nov 8, 2023
Open
@Maximetinu
Copy link
Contributor

Maximetinu commented Nov 8, 2023 •
edited
Loading

Hi folks,

I just wanted to share that I am interested in this feature as a way to manually control when to render: https://discord.com/channels/691052431525675048/1166887503454810132/1166887503454810132

Is there any interest or plan to continue with this PR, or it's doomed to be forgotten/closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicopap nicopap Awaiting requested review from nicopap

Assignees
No one assigned
Labels
A-Windowing Platform-agnostic interface layer to run your app in C-Feature A new feature, making something new possible D-Complex Quite challenging from either a design or technical perspective. Ask for help! X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
Milestone
No milestone
6 participants
@B-head @adsick @Vrixyz @Telov @Maximetinu @alice-i-cecile