Fix perf degradation on web builds (#11227)
# Objective - Since #10702, the way bevy updates the window leads to major slowdowns as seen in - #11122 - #11220 - Slow is bad, furthermore, _very_ slow is _very_ bad. We should fix this issue. ## Solution - Move the app update code into the `Event::WindowEvent { event: WindowEvent::RedrawRequested }` branch of the event loop. - Run `window.request_redraw()` When `runner_state.redraw_requested` - Instead of swapping `ControlFlow` between `Poll` and `Wait`, we always keep it at `Wait`, and use `window.request_redraw()` to schedule an immediate call to the event loop. - `runner_state.redraw_requested` is set to `true` when `UpdateMode::Continuous` and when a `RequestRedraw` event is received. - Extract the redraw code into a separate function, because otherwise I'd go crazy with the indentation level. - Fix #11122. ## Testing I tested the WASM builds as follow: ```sh cargo run -p build-wasm-example -- --api webgl2 bevymark python -m http.server --directory examples/wasm/ 8080 # Open browser at http://localhost:8080 ``` On main, even spawning a couple sprites is super choppy. Even if it says "300 FPS". While on this branch, it is smooth as butter. I also found that it fixes all choppiness on window resize (tested on Linux/X11). This was another issue from #10702 IIRC. So here is what I tested: - On `wasm`: `many_foxes` and `bevymark`, with `argh::from_env()` commented out, otherwise we get a cryptic error. - Both with `PresentMode::AutoVsync` and `PresentMode::AutoNoVsync` - On main, it is consistently choppy. - With this PR, the visible frame rate is consistent with the diagnostic numbers - On native (linux/x11) I ran similar tests, making sure that `AutoVsync` limits to monitor framerate, and `AutoNoVsync` doesn't. ## Future work Code could be improved, I wanted a quick solution easy to review, but we really need to make the code more accessible. - #9768 - ~~**`WinitSettings::desktop_app()` is completely borked.**~~ actually broken on main as well ### Review guide Consider enable the non-whitespace diff to see the _real_ change set.
This commit is contained in:
parent
a35a151f47
commit
79021c78c6
@ -71,6 +71,10 @@ pub fn render_system(world: &mut World) {
|
||||
for window in windows.values_mut() {
|
||||
if let Some(wrapped_texture) = window.swap_chain_texture.take() {
|
||||
if let Some(surface_texture) = wrapped_texture.try_unwrap() {
|
||||
// TODO(clean): winit docs recommends calling pre_present_notify before this.
|
||||
// though `present()` doesn't present the frame, it schedules it to be presented
|
||||
// by wgpu.
|
||||
// https://docs.rs/winit/0.29.9/wasm32-unknown-unknown/winit/window/struct.Window.html#method.pre_present_notify
|
||||
surface_texture.present();
|
||||
}
|
||||
}
|
||||
|
||||
@ -360,6 +360,7 @@ pub fn winit_runner(mut app: App) {
|
||||
}
|
||||
}
|
||||
}
|
||||
runner_state.redraw_requested = false;
|
||||
|
||||
match event {
|
||||
Event::NewEvents(start_cause) => match start_cause {
|
||||
@ -412,7 +413,7 @@ pub fn winit_runner(mut app: App) {
|
||||
return;
|
||||
};
|
||||
|
||||
let Ok((mut window, mut cache)) = windows.get_mut(window_entity) else {
|
||||
let Ok((mut window, _)) = windows.get_mut(window_entity) else {
|
||||
warn!(
|
||||
"Window {:?} is missing `Window` component, skipping event {:?}",
|
||||
window_entity, event
|
||||
@ -655,17 +656,33 @@ pub fn winit_runner(mut app: App) {
|
||||
window: window_entity,
|
||||
});
|
||||
}
|
||||
WindowEvent::RedrawRequested => {
|
||||
runner_state.redraw_requested = false;
|
||||
run_app_update_if_should(
|
||||
&mut runner_state,
|
||||
&mut app,
|
||||
&mut focused_windows_state,
|
||||
event_loop,
|
||||
&mut create_window_system_state,
|
||||
&mut app_exit_event_reader,
|
||||
&mut redraw_event_reader,
|
||||
);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
let mut windows = app.world.query::<(&mut Window, &mut CachedWindow)>();
|
||||
if let Ok((window, mut cache)) = windows.get_mut(&mut app.world, window_entity) {
|
||||
if window.is_changed() {
|
||||
cache.window = window.clone();
|
||||
}
|
||||
}
|
||||
}
|
||||
Event::DeviceEvent {
|
||||
event: DeviceEvent::MouseMotion { delta: (x, y) },
|
||||
..
|
||||
} => {
|
||||
runner_state.redraw_requested = true;
|
||||
let (mut event_writers, ..) = event_writer_system_state.get_mut(&mut app.world);
|
||||
event_writers.mouse_motion.send(MouseMotion {
|
||||
delta: Vec2::new(x as f32, y as f32),
|
||||
@ -726,19 +743,53 @@ pub fn winit_runner(mut app: App) {
|
||||
|
||||
app.world.entity_mut(entity).insert(wrapper);
|
||||
}
|
||||
event_loop.set_control_flow(ControlFlow::Poll);
|
||||
event_loop.set_control_flow(ControlFlow::Wait);
|
||||
}
|
||||
}
|
||||
Event::AboutToWait => {
|
||||
if runner_state.active.should_run() {
|
||||
_ => (),
|
||||
}
|
||||
if runner_state.redraw_requested {
|
||||
let (_, winit_windows, _, _) = event_writer_system_state.get_mut(&mut app.world);
|
||||
for window in winit_windows.windows.values() {
|
||||
window.request_redraw();
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
trace!("starting winit event loop");
|
||||
// TODO(clean): the winit docs mention using `spawn` instead of `run` on WASM.
|
||||
if let Err(err) = event_loop.run(event_handler) {
|
||||
error!("winit event loop returned an error: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
fn run_app_update_if_should(
|
||||
runner_state: &mut WinitAppRunnerState,
|
||||
app: &mut App,
|
||||
focused_windows_state: &mut SystemState<(Res<WinitSettings>, Query<&Window>)>,
|
||||
event_loop: &EventLoopWindowTarget<()>,
|
||||
create_window_system_state: &mut SystemState<(
|
||||
Commands,
|
||||
Query<(Entity, &mut Window), Added<Window>>,
|
||||
EventWriter<WindowCreated>,
|
||||
NonSendMut<WinitWindows>,
|
||||
NonSendMut<AccessKitAdapters>,
|
||||
ResMut<WinitActionHandlers>,
|
||||
ResMut<AccessibilityRequested>,
|
||||
)>,
|
||||
app_exit_event_reader: &mut ManualEventReader<AppExit>,
|
||||
redraw_event_reader: &mut ManualEventReader<RequestRedraw>,
|
||||
) {
|
||||
if !runner_state.active.should_run() {
|
||||
return;
|
||||
}
|
||||
if runner_state.active == ActiveState::WillSuspend {
|
||||
runner_state.active = ActiveState::Suspended;
|
||||
#[cfg(target_os = "android")]
|
||||
{
|
||||
// Remove the `RawHandleWrapper` from the primary window.
|
||||
// This will trigger the surface destruction.
|
||||
let mut query =
|
||||
app.world.query_filtered::<Entity, With<PrimaryWindow>>();
|
||||
let mut query = app.world.query_filtered::<Entity, With<PrimaryWindow>>();
|
||||
let entity = query.single(&app.world);
|
||||
app.world.entity_mut(entity).remove::<RawHandleWrapper>();
|
||||
event_loop.set_control_flow(ControlFlow::Wait);
|
||||
@ -747,13 +798,13 @@ pub fn winit_runner(mut app: App) {
|
||||
let (config, windows) = focused_windows_state.get(&app.world);
|
||||
let focused = windows.iter().any(|window| window.focused);
|
||||
let should_update = match config.update_mode(focused) {
|
||||
UpdateMode::Continuous | UpdateMode::Reactive { .. } => {
|
||||
// `Reactive`: In order for `event_handler` to have been called, either
|
||||
// we received a window or raw input event, the `wait` elapsed, or a
|
||||
// redraw was requested (by the app or the OS). There are no other
|
||||
// conditions, so we can just return `true` here.
|
||||
true
|
||||
}
|
||||
UpdateMode::Continuous | UpdateMode::Reactive { .. } => true,
|
||||
// TODO(bug): This is currently always true since we only run this function
|
||||
// if we received a `RequestRedraw` event.
|
||||
UpdateMode::ReactiveLowPower { .. } => {
|
||||
runner_state.wait_elapsed
|
||||
|| runner_state.redraw_requested
|
||||
@ -764,8 +815,6 @@ pub fn winit_runner(mut app: App) {
|
||||
if app.plugins_state() == PluginsState::Cleaned && should_update {
|
||||
// reset these on each update
|
||||
runner_state.wait_elapsed = false;
|
||||
runner_state.window_event_received = false;
|
||||
runner_state.redraw_requested = false;
|
||||
runner_state.last_update = Instant::now();
|
||||
|
||||
app.update();
|
||||
@ -775,10 +824,14 @@ pub fn winit_runner(mut app: App) {
|
||||
let focused = windows.iter().any(|window| window.focused);
|
||||
match config.update_mode(focused) {
|
||||
UpdateMode::Continuous => {
|
||||
event_loop.set_control_flow(ControlFlow::Poll);
|
||||
runner_state.redraw_requested = true;
|
||||
}
|
||||
UpdateMode::Reactive { wait }
|
||||
| UpdateMode::ReactiveLowPower { wait } => {
|
||||
UpdateMode::Reactive { wait } | UpdateMode::ReactiveLowPower { wait } => {
|
||||
// TODO(bug): this is unexpected behavior.
|
||||
// When Reactive, user expects bevy to actually wait that amount of time,
|
||||
// and not potentially infinitely depending on plateform specifics (which this does)
|
||||
// Need to verify the plateform specifics (whether this can occur in
|
||||
// rare-but-possible cases) and replace this with a panic or a log warn!
|
||||
if let Some(next) = runner_state.last_update.checked_add(*wait) {
|
||||
runner_state.scheduled_update = Some(next);
|
||||
event_loop.set_control_flow(ControlFlow::WaitUntil(next));
|
||||
@ -789,12 +842,9 @@ pub fn winit_runner(mut app: App) {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(app_redraw_events) =
|
||||
app.world.get_resource::<Events<RequestRedraw>>()
|
||||
{
|
||||
if let Some(app_redraw_events) = app.world.get_resource::<Events<RequestRedraw>>() {
|
||||
if redraw_event_reader.read(app_redraw_events).last().is_some() {
|
||||
runner_state.redraw_requested = true;
|
||||
event_loop.set_control_flow(ControlFlow::Poll);
|
||||
}
|
||||
}
|
||||
|
||||
@ -830,16 +880,6 @@ pub fn winit_runner(mut app: App) {
|
||||
|
||||
create_window_system_state.apply(&mut app.world);
|
||||
}
|
||||
}
|
||||
_ => (),
|
||||
}
|
||||
};
|
||||
|
||||
trace!("starting winit event loop");
|
||||
if let Err(err) = event_loop.run(event_handler) {
|
||||
error!("winit event loop returned an error: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
fn react_to_resize(
|
||||
window: &mut Mut<'_, Window>,
|
||||
|
||||
@ -76,7 +76,7 @@ pub enum UpdateMode {
|
||||
/// - new [window](`winit::event::WindowEvent`) or [raw input](`winit::event::DeviceEvent`)
|
||||
/// events have appeared
|
||||
Reactive {
|
||||
/// The minimum time from the start of one update to the next.
|
||||
/// The approximate time from the start of one update to the next.
|
||||
///
|
||||
/// **Note:** This has no upper limit.
|
||||
/// The [`App`](bevy_app::App) will wait indefinitely if you set this to [`Duration::MAX`].
|
||||
@ -93,7 +93,7 @@ pub enum UpdateMode {
|
||||
/// Use this mode if, for example, you only want your app to update when the mouse cursor is
|
||||
/// moving over a window, not just moving in general. This can greatly reduce power consumption.
|
||||
ReactiveLowPower {
|
||||
/// The minimum time from the start of one update to the next.
|
||||
/// The approximate time from the start of one update to the next.
|
||||
///
|
||||
/// **Note:** This has no upper limit.
|
||||
/// The [`App`](bevy_app::App) will wait indefinitely if you set this to [`Duration::MAX`].
|
||||
|
||||
Loading…
Reference in New Issue
Block a user