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

Fix perf degradation on web builds #11227

Merged
merged 4 commits into from
Jan 6, 2024
Merged

Fix perf degradation on web builds #11227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a1430d7
Fix perf degradation on web builds
nicopap Jan 5, 2024
4e6d8f7
Use Window::request_redraw over ControlFlow::Poll
nicopap Jan 6, 2024
6ec7ac0
Move react_to_resize
nicopap Jan 6, 2024
c4ad9ec
Move around redraw_requested handling
nicopap Jan 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/bevy_render/src/renderer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +74 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling Window::pre_present_notify() before SurfaceTexture::present() has worked fine for me so far.

Looking at the Vulkan and OpenGL implementations (because this is no-op on everything except Wayland), I can't see a queue, but I'm definitely no Wgpu expert. Would be nice to get clarification here from somebody knowledgeable about Wgpu.

surface_texture.present();
}
}
Expand Down
248 changes: 144 additions & 104 deletions crates/bevy_winit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
);
}
_ => {}
}

if window.is_changed() {
cache.window = window.clone();
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),
Expand Down Expand Up @@ -726,119 +743,142 @@ 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.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 entity = query.single(&app.world);
app.world.entity_mut(entity).remove::<RawHandleWrapper>();
event_loop.set_control_flow(ControlFlow::Wait);
}
}
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::ReactiveLowPower { .. } => {
runner_state.wait_elapsed
|| runner_state.redraw_requested
|| runner_state.window_event_received
}
};

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();
_ => (),
}
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();
}
}
};

// decide when to run the next update
let (config, windows) = focused_windows_state.get(&app.world);
let focused = windows.iter().any(|window| window.focused);
match config.update_mode(focused) {
UpdateMode::Continuous => {
event_loop.set_control_flow(ControlFlow::Poll);
}
UpdateMode::Reactive { wait }
| UpdateMode::ReactiveLowPower { wait } => {
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));
} else {
runner_state.scheduled_update = None;
event_loop.set_control_flow(ControlFlow::Wait);
}
}
}
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}");
}
}

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);
}
}
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 entity = query.single(&app.world);
app.world.entity_mut(entity).remove::<RawHandleWrapper>();
event_loop.set_control_flow(ControlFlow::Wait);
}
}
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) {
// `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.
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
|| runner_state.window_event_received
}
};

if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() {
if app_exit_event_reader.read(app_exit_events).last().is_some() {
event_loop.exit();
}
}
}
if app.plugins_state() == PluginsState::Cleaned && should_update {
// reset these on each update
runner_state.wait_elapsed = false;
runner_state.last_update = Instant::now();

// create any new windows
// (even if app did not update, some may have been created by plugin setup)
let (
commands,
mut windows,
event_writer,
winit_windows,
adapters,
handlers,
accessibility_requested,
) = create_window_system_state.get_mut(&mut app.world);

create_windows(
event_loop,
commands,
windows.iter_mut(),
event_writer,
winit_windows,
adapters,
handlers,
accessibility_requested,
);
app.update();

create_window_system_state.apply(&mut app.world);
// decide when to run the next update
let (config, windows) = focused_windows_state.get(&app.world);
let focused = windows.iter().any(|window| window.focused);
match config.update_mode(focused) {
UpdateMode::Continuous => {
runner_state.redraw_requested = true;
}
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));
} else {
runner_state.scheduled_update = None;
event_loop.set_control_flow(ControlFlow::Wait);
}
}
_ => (),
}
};

trace!("starting winit event loop");
if let Err(err) = event_loop.run(event_handler) {
error!("winit event loop returned an error: {err}");
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;
}
}

if let Some(app_exit_events) = app.world.get_resource::<Events<AppExit>>() {
if app_exit_event_reader.read(app_exit_events).last().is_some() {
event_loop.exit();
}
}
}

// create any new windows
// (even if app did not update, some may have been created by plugin setup)
let (
commands,
mut windows,
event_writer,
winit_windows,
adapters,
handlers,
accessibility_requested,
) = create_window_system_state.get_mut(&mut app.world);

create_windows(
event_loop,
commands,
windows.iter_mut(),
event_writer,
winit_windows,
adapters,
handlers,
accessibility_requested,
);

create_window_system_state.apply(&mut app.world);
}

fn react_to_resize(
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_winit/src/winit_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand All @@ -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`].
Expand Down