Fix missed RedrawRequested and WindowCloseRequested events with UpdateMode::Reactive (part 2)

[aloucks]

Objective

This PR continues the work started by @boondocklabs in #17991 in order to fix missed RedrawRequested events that are sent during app update. Additionally, this PR now also fixes a similar issue where WindowCloseRequested events were also missed during app update. In both cases, the event would not be handled until the next app update.

This bug applies when the app is running with UpdateMode::Reactive. These missed events won't be handled until the next event loop timeout or triggered by a new window or device event (such as moving the mouse).

Use-Cases

When using UpdateMode::Reactive:

1. Sending a RedrawRequested event from an observe system should trigger a redraw immediately. The app should not stall until the next window event or event loop timeout.
2. Clicking the close button on a window should be handled immediately. The app should not stall until the next window event or event loop timeout.

Solution

• RedrawRequested events are already read and checked in the winit event loop runner. Move this read after the app update executes.
• WindowCloseRequested were not previously read or checked. Read and check if an event has fired after the app update executes.

Prior Art

The UpdateMode itself is (re)read after the app update with a note about re-extracting it since the app update may have changed the value and we should act accordingly. We're essentially applying this same logic (i.e. check after the app update) for RedrawRequested and WindowCloseRequested.

Original PR Notes from @boondocklabs

Solution

Currently RequestRedraw events can be missed in about_to_wait() as it is can call run_app_update() after reading the event cursor, thus the redraw_requested flag is stale.

This adds a subsequent read after the call to run_app_update to ensure redraw_requested is updated if any events were sent during the update.

Testing

Tested in Reactive mode with animations using a forked bevy_tweening which supports 0.16-dev and sends RequestRedraw events while there are any Animator components active.

Prior to this change, the animation would only run a single update, and would stall for the reactive mode timeout before continuing the animation.

With this patch, the animation runs smooth in Reactive mode. Tested on MacOS and iOS on iPad and iPhone.

[aloucks]

The test included in this PR uses observe which I suspect is resulting in the event delivery being deferred. The low_power example works as expected with or without this change but does not use an Observer.

[aloucks]

Prior to this change, the animation would only run a single update, and would stall for the reactive mode timeout before continuing the animation.

There's another similar issue where WindowCloseRequested is also not handled immediately in reactive mode. If you run the low_power example, press space to enter reactive mode, and then attempt to close the window, nothing happens for a second or two. The timing of these varies from run to run, but if you do it repeatedly, you'll definitely notice a short delay between pressing the the window close button and when the window actually closes.

The fix for the window delay issue is essentially the same as what's presented here - read the WindowCloseRequested events after the app update and toggle request_redraw if one came through.

[aloucks]

@alice-i-cecile

Is there any way we could get this in for 0.16? I understand you want to lock down new features, but this feels like a pretty decent UX bugfix. I've tested these changes on macOS and Windows. Please let me know if there is anything I can do to help make this happen for 0.16.

Thanks!

[alice-i-cecile]

I need testing and reviews from at least two other contributors, ideally on a wide range of platforms. I share your desire to get this sort of bug fix in, but windowing code is really fragile and hard to test.

[tychedelia]

Can you provide some brief info about how to test this? Just the low_power example?

[aloucks]

Can you provide some brief info about how to test this? Just the low_power example?

There's two things to test with UpdateMode::Reactive:

1. Handling of RedrawRequested:
   • @boondocklabs provided the initial test case here:
     https://github.com/boondocklabs/bevy-redraw-test/blob/main/src/main.rs
   • I've pulled it into this this PR with some light modifications (desktop_request_redraw). Clicking the cube toggles the animation of the spinning cube. Without the changes in this PR, the animation wouldn't toggle immediately and instead would wait until additional window events were triggered or the event loop timeout occurred. You had to be careful not to move the mouse when clicking the cube.
2. Handling of WindowCloseRequested :
   • On the main branch: If you run the low_power example, press space to enter reactive mode, and then attempt to close the window, nothing happens for a second or two. The timing of these varies from run to run, but if you do it repeatedly, you'll definitely notice a short delay between pressing the the window close button and when the window actually closes.
   • If you run these steps with this branch, the window will close immediately.

[aloucks]

Is there any other info needed for testing?

[tychedelia]

Hi! Thanks for checking back in. I haven't had time to look into this but it's in my inbox. I've also added it to 0.17 to make sure it doesn't get missed.