Button example is broken

[nicopap]

• Bevy version: 425570a (main as of today)

What you did

• Run the button example with cargo run --example button.

What went wrong

• At first, the window is transparent.
• When moving the mouse, it displays the button
• When hovering over the button, the button says "hover"
• When pressing and holding down the mouse left button, it keeps saying "hover"
• It only says "pressed" after releasing the mouse left button
• If I move afterward the mouse, it goes back to "hover"

What I expected

• Window shows up immediately
• Button should say "pressed" as soon as I start holding down the button
• Button should stop saying "pressed" when I stop holding down the button

Currently it's basically the inverse.

Additional information

Removing WinitSettings::desktop_app() fixes it.

This might be related to #11052. Supposedly, the updating mechanisms do not work as expected anymore.

[daxpedda]

Digging into this I can see that Bevy doesn't call Window::request_redraw() when using UpdateMode::Reactive when needed.
Currently it only calls it when:

• the user sends the RequestRedraw event
• when DeviceEvent::MouseMotion is received

This probably worked before because EventLoopWindowTarget::listen_device_events() was supported with very few targets.

So the solution is to call Window::request_redraw() when actually required instead of relying on circumstantial events.

I also noticed that Winit has set up a bool, WinitAppRunnerState::redraw_requested, to collect and call Window::request_redraw() later. This is unnecessary, Winit de-duplicates these requests already.

[daxpedda]

• At first, the window is transparent.

This is because SurfaceTexture::present() is not called on the first call to App::update(). I noticed that the whole render_system is not called for some reason.

If Bevy isn't ready to draw when RedrawRequested is sent, it has to schedule a new one to render when it is. Otherwise nothing will happen.

• When pressing and holding down the mouse left button, it keeps saying "hover"
• It only says "pressed" after releasing the mouse left button
• If I move afterward the mouse, it goes back to "hover"

If you move the mouse while holding it down it will actually update and show "pressed". This is because changes made to the button are only rendered in the next update cycle, not in the one they are changed in.
Keep in mind I'm just describing what I'm observing, I don't actually know why any of this is the way it is.

I'm not sure exactly how the rendering system in Bevy works, but sending RequestRedraw in Interaction::Pressed fixes that issue.

So either Bevy has to correctly render the current state in the same call to App::update(), or it has to schedule another redraw after changes are made.

[mockersf]

• When pressing and holding down the mouse left button, it keeps saying "hover"
• It only says "pressed" after releasing the mouse left button
• If I move afterward the mouse, it goes back to "hover"

system extract_uinodes is executed after system queue_uinodes, so a given frame is rendered with the nodes from the previous frame. This means that UI rendering is always one frame late

[mockersf]

I've made a branch to showcase the issue with more logs: https://github.com/mockersf/bevy/tree/one-frame-lag

here is the log I get:

ERROR bevy_winit: MouseInput event received: Pressed Left
 WARN bevy_winit: updating
 WARN bevy_ui::render:   queue_uinodes
 INFO bevy_ui::render:       entity: 3v1, color: Rgba { red: 0.25, green: 0.25, blue: 0.25, alpha: 1.0 }
 WARN bevy_ui::render:   prepare_uinodes
 INFO bevy_ui::render:       entity: 3v1, color: Rgba { red: 0.25, green: 0.25, blue: 0.25, alpha: 1.0 }
 WARN bevy_ui::render:   extract_uinodes
 INFO bevy_ui::render:       entity: 3v1, color: Rgba { red: 0.35, green: 0.75, blue: 0.35, alpha: 1.0 }

so queue_uinodes and prepare_uinodes run on the extracted uinodes from the frame before the click, then extract_uinodes run which extract the node with the changed green background

this is causing a one frame lag for every change in the UI. Chances are there is also the same issue with every thing being rendered as the schedules are shared

[BorisBoutillier]

Isn't the one frame lag, related to the pipelined renderer ? What you see is the queue_uinodes from the end of the render app of the previous frame, and the extract is from the next update which happens in parallel ?

In general, doesn't the pipelined renderer inevitably introduce a one frame delay between events and associated render ?

I have done some investigation on my side regarding the Reactive mode case here with the button example. And what I have seen is that the RenderApp is stopped somewhere between its ExtractCommands set and its Queue set. The executor is waiting for one system to finish( i have not yet found exactly which one). This system is in direct relation with winit as it finishes when either there is an event or the 'wait delay' is reached.

So in ReactiveMode, the ExtractCommands part of the Render app is executed during the frame with the event, but not finished.
This perhaps also is in relation with the pipeline rendered, because the end of the RenderApp would be done on the next frame in parallel to the main update, but this next frame never happens ?

This is the best I have been able to go in debugging this, I don't have enough knowledge to really go further for the moment.

[alice-i-cecile]

This can no longer be reproduced: closing out.

If there's more weirdness (or you want to pursue improvements to Reactive mode), please feel free to open dedicated issues.

[mockersf]

Isn't the one frame lag, related to the pipelined renderer ?

Yup you're right, with pipelined rendering, we need at least two updates to see the result

the button example makes it very obvious if you can send exactly one event: clicking the mouse without moving the mouse, or the touchpad not on a Mac laptop (it sends touchpadpressure events)

this will impact everything rendered, not just UI

we could:

• document it, but it would mean the example would still look broken
• change the desktop_app mode to happen faster than every 5 seconds, and the timeout would make the update happen. this would hide the problem in the example
• queue an extra update after the last user event to update the rendering

[BorisBoutillier]

I don't like the example being broken, as this is real obvious when using it. Reducing the timeout also means that it increasing consumption even when nothing really happens.
I personnaly prefer, in this world of 'choose your best enemy' to queue an extra update on user events.
As far as I remember it just need an extra bool on the runner to ensure 2 frame update per user events.

I can look into that and provide a PR with this kind of solution during the week-end if we think this is the way to go.

[BorisBoutillier]

I was going back to this when I noted that now on main the button example is working as expected on my setup.

I have bi-sect the commit that "fixes" it for me to the commit associated to the PR #11660.
AFAIK this PR only aims to allow more platform to have prepare_windows possibily run in a different thread. Which seems to mean that on my linux setup, the 'render_app' thread was blocked because the prepare_windows system could not be run because it needed to be on the main thread, which was stopped waiting for events.

But as this change still keep prepare_windows on the main thread for macos, or ios, I suspect the button example issue still exists on those platforms. I don't have a macos or ios setup, so if anybody with this platform can confirm this ?

For a full fix for the Reactive mode, should we, independently of the target, ensure 2 updates happens after each window or device event received ?

[alice-i-cecile]

#11672 enabled that behavior on all remaining platforms :)

[BorisBoutillier]

Unfortunately this is not sufficient to solve the button issue on all platform.
The prepare_windows is now split in two function, but the new function create_surfaces still has a NonSend marker for macos or ios, and thus is run in the main thread, and thus is blocked waiting for the winit event.

I don't have a macos environment, but if I enable the NonSend marker of the create_surfaces function on all platform, I can still reproduce this issue locally, so I suspect it is still an issue with main branch on macos and ios.

[mockersf]

#11720 should not block on the main thread if there's not need to, which is most of the time anyway