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.

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 requested repaints not causing Bevy to redraw. #240

Merged
merged 2 commits into from
Jan 7, 2024

Conversation

andriyDev
Copy link
Contributor

The previous implementation seems to have lost track of what the redraw was supposed to be tracking. This meant button clicks would not trigger a redraw on WinitSettings::desktop_app() (only a mouse move after would cause a redraw).

I've tested this on my own project and things work much more correct!

Fixes #239.

@aevyrie
Copy link
Contributor

aevyrie commented Jan 3, 2024

It looks like the correct solution is to use this field from egui's FullOutput: https://docs.rs/egui/latest/egui/viewport/struct.ViewportOutput.html#structfield.repaint_delay
and find if any viewport wants a repaint. Even better, use bevy's winit proxy to schedule the exact wakeup time based on the minimum repaint delay.

It appears this was mangled during the last release.

Copy link
Contributor

@aevyrie aevyrie left a comment

Choose a reason for hiding this comment

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

Although I noted another "better" solution, this change is an improvement that fixes a regression, and matches the original implementation.

@andriyDev
Copy link
Contributor Author

It looks like the correct solution is to use this field from egui's FullOutput: https://docs.rs/egui/latest/egui/viewport/struct.ViewportOutput.html#structfield.repaint_delay and find if any viewport wants a repaint. Even better, use bevy's winit proxy to schedule the exact wakeup time based on the minimum repaint delay.

As far as I can tell, viewports don't actually know if they need a redraw or not. As in "request redraw" only applies to the whole context (so all viewports). That said I agree that the best solution also takes into account the redraw_delay as you noted.

My point being: this PR will not need to be reverted or rewritten, just more code will need to be added to fully handle all the cases.

@mvlabat
Copy link
Owner

mvlabat commented Jan 7, 2024

Thank you for the fix! I'm good to merge it as is, at least that will fix the immediate regression. If anyone wants to submit a PR with a proper fix, I'll gladly accept it as well (I don't have the time to look into that myself, unfortunately).

@mvlabat mvlabat merged commit 26a4264 into mvlabat:main Jan 7, 2024
27 checks passed
@andriyDev andriyDev deleted the fix-redraw branch January 8, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using WinitSettings::desktop_app() does not update egui UI
3 participants