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 Reactive and ReactiveLowPower update modes #11325

Merged
merged 5 commits into from
Jan 15, 2024
Merged

Fix Reactive and ReactiveLowPower update modes #11325

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a4908d8
check for update conditions outside of update
mockersf Jan 13, 2024
5235b00
don't ignore other device events
mockersf Jan 13, 2024
e66a628
clippy
mockersf Jan 13, 2024
20cce3d
fixes for windows
mockersf Jan 13, 2024
c243aaf
rename and comment
mockersf Jan 13, 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
110 changes: 76 additions & 34 deletions crates/bevy_winit/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ struct WinitAppRunnerState {
active: ActiveState,
/// Is `true` if a new [`WindowEvent`] has been received since the last update.
window_event_received: bool,
/// Is `true` if a new [`DeviceEvent`] has been received since the last update.
device_event_received: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

In a future PR we could replace this with a bitbybit or bilge bitflags To reduce the size of this struct

/// Is `true` if the app has requested a redraw since the last update.
redraw_requested: bool,
/// Is `true` if enough time has elapsed since `last_update` to run another update.
Expand All @@ -262,6 +264,17 @@ struct WinitAppRunnerState {
last_update: Instant,
/// The time the next update is scheduled to start.
scheduled_update: Option<Instant>,
/// Number of "forced" updates to trigger on application start
startup_forced_updates: u32,
}

impl WinitAppRunnerState {
fn reset_on_update(&mut self) {
self.redraw_requested = false;
self.window_event_received = false;
self.device_event_received = false;
self.wait_elapsed = false;
}
}

#[derive(PartialEq, Eq)]
Expand All @@ -287,10 +300,13 @@ impl Default for WinitAppRunnerState {
Self {
active: ActiveState::NotYetStarted,
window_event_received: false,
device_event_received: false,
redraw_requested: false,
wait_elapsed: false,
last_update: Instant::now(),
scheduled_update: None,
// 3 seems to be enough, 5 is a safe margin
startup_forced_updates: 5,
}
}
}
Expand Down Expand Up @@ -364,14 +380,56 @@ pub fn winit_runner(mut app: App) {

match event {
Event::AboutToWait => {
if runner_state.redraw_requested {
let (config, windows) = focused_windows_state.get(&app.world);
let focused = windows.iter().any(|window| window.focused);
let mut should_update = match config.update_mode(focused) {
UpdateMode::Continuous => {
runner_state.redraw_requested
|| runner_state.window_event_received
|| runner_state.device_event_received
}
UpdateMode::Reactive { .. } => {
runner_state.wait_elapsed
|| runner_state.redraw_requested
|| runner_state.window_event_received
|| runner_state.device_event_received
}
UpdateMode::ReactiveLowPower { .. } => {
runner_state.wait_elapsed
|| runner_state.redraw_requested
|| runner_state.window_event_received
}
};

// Ensure that an update is triggered on the first iterations for app initialization
if runner_state.startup_forced_updates > 0 {
runner_state.startup_forced_updates -= 1;
should_update = true;
}

if should_update {
let visible = windows.iter().any(|window| window.visible);
let (_, winit_windows, _, _) =
event_writer_system_state.get_mut(&mut app.world);
for window in winit_windows.windows.values() {
window.request_redraw();
if visible {
for window in winit_windows.windows.values() {
window.request_redraw();
}
} else {
// there are no windows, or they are not visible.
// Winit won't send events on some platforms, so trigger an update manually.
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,
);
event_loop.set_control_flow(ControlFlow::Poll);
}
}
runner_state.redraw_requested = false;
}
Event::NewEvents(_) => {
if let Some(t) = runner_state.scheduled_update {
Expand Down Expand Up @@ -638,7 +696,6 @@ pub fn winit_runner(mut app: App) {
});
}
WindowEvent::RedrawRequested => {
runner_state.redraw_requested = false;
run_app_update_if_should(
&mut runner_state,
&mut app,
Expand All @@ -659,14 +716,14 @@ pub fn winit_runner(mut app: App) {
}
}
}
Event::DeviceEvent {
event: DeviceEvent::MouseMotion { delta: (x, y) },
..
} => {
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),
});
Event::DeviceEvent { event, .. } => {
runner_state.device_event_received = true;
if let DeviceEvent::MouseMotion { delta: (x, y) } = event {
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),
});
}
}
Event::Suspended => {
let (mut event_writers, ..) = event_writer_system_state.get_mut(&mut app.world);
Expand Down Expand Up @@ -700,7 +757,7 @@ pub fn winit_runner(mut app: App) {
) = create_window_system_state.get_mut(&mut app.world);

create_windows(
&event_loop,
event_loop,
commands,
windows.iter_mut(),
event_writer,
Expand Down Expand Up @@ -793,6 +850,8 @@ fn run_app_update_if_should(
app_exit_event_reader: &mut ManualEventReader<AppExit>,
redraw_event_reader: &mut ManualEventReader<RequestRedraw>,
) {
runner_state.reset_on_update();

if !runner_state.active.should_run() {
return;
}
Expand All @@ -808,32 +867,15 @@ fn run_app_update_if_should(
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 app.plugins_state() == PluginsState::Cleaned && should_update {
// reset these on each update
runner_state.wait_elapsed = false;
if app.plugins_state() == PluginsState::Cleaned {
runner_state.last_update = Instant::now();

app.update();

// decide when to run the next update
let (config, _) = focused_windows_state.get(&app.world);
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;
Expand Down