Make request_repaint_after actually respect the delay

[athre0z]

I noticed that request_repaint_after currently doesn't actually work as promised and causes redraws in a busy-loop, akin to just using request_repaint. I debugged into it a bit and believe that I found both the problem and a solution.

The branch that resets the repaint_delay to MAX was previously never actually taken, causing the repaint_delay to never be reset to MAX. This in turn effectively turned request_repaint_after into being equivalent to simply calling request_repaint, because the former only updates the repaint_time if it is shorter than the previous value:

egui/crates/egui/src/context.rs

Lines 147 to 148 in d72f92c

	if delay < viewport.repaint.repaint_delay {
	viewport.repaint.repaint_delay = delay;

Testing

Here's a patch that I used for testing these changes:

diff --git a/examples/hello_world/src/main.rs b/examples/hello_world/src/main.rs
index c27ae5a1..96b5a5f9 100644
--- a/examples/hello_world/src/main.rs
+++ b/examples/hello_world/src/main.rs
@@ -1,5 +1,7 @@
 #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] // hide console window on Windows in release

+use std::time::Duration;
+
 use eframe::egui;

 fn main() -> Result<(), eframe::Error> {
@@ -36,6 +38,10 @@ impl Default for MyApp {

 impl eframe::App for MyApp {
     fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
+        println!("repaint!");
+
+        ctx.request_repaint_after(Duration::from_secs(1));
+
         egui::CentralPanel::default().show(ctx, |ui| {
             ui.heading("My egui Application");
             ui.horizontal(|ui| {

[emilk]

The point of outstanding it to run two refreshes on request_repaint, and on startup, but with this PR that is now ignored.

That said, thanks to #3930 we might not need that "repaint twice" logic

[athre0z]

Ah okay, I didn't realize that you actually wanted two invocations. It does make sense though, as it serves as a simple form of resilience against all kinds of "frame updates stuff and then expects it to show up on screen soon" -- let's try to keep that.

Note however that the previous logic didn't actually work: the == 0 branch was previously never taken at all, because begin_frame_repaint_logic isn't ever actually called once outstanding reached 0. You can verify this by throwing in a breakpoint or panic!().

Would it make sense to move the decrement as suggested in this PR and then simply explicitly set outstanding to 2 in both the constructor and request_repaint? I believe that this would also convey the intention of redrawing twice more clearly.

[emilk]

Ah okay, I didn't realize that you actually wanted two invocations. It does make sense though, as it serves as a simple form of resilience against all kinds of "frame updates stuff and then expects it to show up on screen soon"

Exactly. It's a guard-rail, that's is nice to have. I'm not sure it is actually needed for anything (I'm turning it off now and testing, and nothing is obviously broken). Especially with #3930 merged the need should be smaller.

---

If you go to the backend panel of egui_demo_app there is a simple test for some of this:

Clicking that button once will produce exactly 2 frames to be repainted after a five second delay, so it's not all broken.

[athre0z]

Hmm yeah, true. If you just call request_repaint_after once, it is working as intended. However, if you call it on every frame (like my sample code / diff does) in order to request a redraw e.g. at least once per second, it falls over because outstanding == 0 is never reached.