egui icon indicating copy to clipboard operation
egui copied to clipboard

Duration passed to `request_repaint_after` is ignored on Windows

Open Sedeniono opened this issue 1 year ago ā€¢ 20 comments

Describe the bug Using egui 0.22.0, calling request_repaint_after() causes the application to redraw the UI with a high refresh rate, regardless of the duration passed into request_repaint_after(). It behaves (mostly) as if calling request_repaint(). The bug was not there in version 0.21.0.

Steps to reproduce the behavior: The following code was adapted from the hello world example. I replaced the controls with a fps (frames per second) and counter label. Rust file:

use eframe::egui;
use std::time::{Duration, Instant};

fn main() -> Result<(), eframe::Error> {
    let options = eframe::NativeOptions {
        initial_window_size: Some(egui::vec2(320.0, 240.0)),
        ..Default::default()
    };
    eframe::run_native(
        "My egui App",
        options,
        Box::new(|_cc| Box::<MyApp>::default()),
    )
}

struct MyApp {
    previous_instant: Instant,
    previous_count: u64,
}

impl Default for MyApp {
    fn default() -> Self {
        Self {
            previous_instant: Instant::now(),
            previous_count: 0,
        }
    }
}

impl eframe::App for MyApp {
    fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            ui.heading("My egui Application");
            let fps = 1.0 / self.previous_instant.elapsed().as_secs_f64();
            self.previous_instant = Instant::now();
            self.previous_count += 1;
            ui.label(format!("fps: {:.2}, counter: {}", fps, self.previous_count));
        });
        ctx.request_repaint_after(Duration::from_secs(1));
    }
}

toml:

[package]
name = "egui_request_repaint_bug"
version = "0.1.0"
edition = "2021"

[dependencies]
egui = "0.22.0"
eframe = { version = "0.22.0", default-features = false, features = [
    "default_fonts", # Embed the default egui fonts.
    "glow",          # Use the glow rendering backend. Alternative: "wgpu".
] }

[patch.crates-io]
egui = { git = "https://github.com/emilk/egui", branch = "master" }
eframe = { git = "https://github.com/emilk/egui", branch = "master" }

Build and run on Windows 10.

When the application has focus, the "fps" labels shows approximately a value of 144 on my monitor (which has a refresh rate of 144 Hz), regardless of where the mouse is. When the application does not have focus, the "fps" label continues updating with a high frequency and shows approximately 90. (I have no idea where the 90 is coming from.)

If I remove the call to request_repaint_after(), the application no longer updates with a high rate. Keeping the request_repaint_after() call and going back to egui 0.21.0, I get the expected behavior. Considering the release notes, I guess this is related to #2939.

Expected behavior When the application does NOT have focus, the "fps" label should show a value of approximately 1. In other words, the GUI should update only once a second because I passed 1s as duration to request_repaint_after(). When the application does have focus, the "fps" label should show a value of 144 only while moving around the mouse.

Desktop:

  • OS: Windows 10
  • Browser: Not applicable; running on native
  • Version of egui: 0.22.0 (version 0.21.0 is not affected)

Sedeniono avatar Jun 25 '23 10:06 Sedeniono

have the same problem in my app https://github.com/chrisheib/STStat

I'll downgrade for the time being, can't let my users waste their ressources like this.

chrisheib avatar Jun 28 '23 21:06 chrisheib

First time I got to run git bisect in the wild :D

The bad commit is 834e2e9f5090824b853ba585f5f8b3f3a47f2359.

Which makes sense since it touched the repaint logic.

@emilk any immediate thoughts on this?

chrisheib avatar Jun 29 '23 20:06 chrisheib

If you run with RUST_LOG=eframe=trace you should get some log output that explains why eframe is repainting; maybe that can help you find the bug. I would appreciate help here since I'm way too busy right now to take a proper look at it.

Most of the logic can be found in crates/eframe/src/native/run.rs (in the run_and_return function) and in struct Repaint in crates/egui/src/context.rs

emilk avatar Jun 30 '23 07:06 emilk

I tried your code on my Mac on latest master, and I fail to reproduce.

The results is as expected: High FPS when moving the mouse over the window, and 1 fps when the mouse is still or outside the egui window. The behavior is the same if the windows is in focus or in the background.

Can you please try again on latest master? Perhaps this is only a Windows bug. If so, perhaps it will be resolved when we update to winit 0.29

emilk avatar Aug 11 '23 14:08 emilk

I'm seeing this bug on Windows as well with latest master. I'll take a look and see if I can figure anything out.

OmegaJak avatar Aug 23 '23 19:08 OmegaJak

Alright - I understand why this is happening in the current code and have a possible (though dubious) fix in mind. I didn't run the code before the commit that introduced this (https://github.com/emilk/egui/commit/834e2e9f5090824b853ba585f5f8b3f3a47f2359) to figure out why it wasn't happening previously.

The workaround on this line is at least partially to blame.

When there's a RedrawEventsCleared event on Windows, we redraw. This event is triggered when request_repaint_after is called, because that leads to a winit RepaintAt or Wait event being triggered, which is immediately followed by RedrawEventsCleared. This then triggers another run of the main ui. So if the main ui is calling request_repaint_after, then the cycle continues and we continuously repaint. This only happens on Windows because that workaround is only applied for Windows - on other platforms, it uses the RedrawRequested event to trigger a repaint, which isn't triggered by Wait or RepaintAt immediately.

This bug only happens when the request_repaint_after is called inside the main ui paint loop. The button inside the demo app that calls it from another thread works fine, even on Windows.

There's another bug confusing things here - the frame_nr inside UserEvent::RequestRepaint is always 1 less than the frame number when checked inside the winit event loop if request_repaint_after was called during the main ui update. This prevents us from properly scheduling repaints much of the time because of this line.

I have two very dubious fixes and some unpolished tweaks to the demo app for testing this in #3275. I removed the workaround and allowed off-by-one for the frame number. Neither seems like a real fix. With these changes, calling request_repaint_after every time inside the main ui loop works as expected, and so does calling it from another thread - but a one-off call of request_repaint_after inside the ui loop does not trigger a repaint later.

I'm looking for suggestions of what the proper fix(es) might be - I don't think what I've done in my PR are adequate or correct.

OmegaJak avatar Aug 23 '23 23:08 OmegaJak

Thanks for the investigation!

Iā€™m hoping we can remove the windows-hack when the new winit version drops, which should be any week now šŸ¤ž The new winit has a substantially new event-loop

emilk avatar Aug 24 '23 05:08 emilk

Hi, I stumbled upon this on accident, when I noticed adding ctx.request_repaint_after(Duration::new(1,0)); to the code increased the CPU usage drastically compared to when not using it, and that the same behaviour occurs even with ctx.request_repaint_after(Duration::new(1000000,0)); (1000000 seconds = 11.5 day refresh interval, which obviously should not cause any refreshes). Are there any good workarounds?

sdasda7777 avatar Oct 15 '23 11:10 sdasda7777

I am also dealing with this problem. @OmegaJak , @emilk any temporal workarounds?

trsh avatar Oct 26 '23 17:10 trsh

I am also dealing with this problem. @OmegaJak , @emilk any temporal workarounds?

Here's my clunky temporary solution - just spin up another thread that sleeps and occasionally calls request_repaint: https://github.com/OmegaJak/gwaihir/blob/f132180306bfeca47321f14d5d881d8742d920d5/crates/gwaihir-client/src/periodic_repaint_thread.rs

OmegaJak avatar Oct 26 '23 17:10 OmegaJak

This is also happening on Linux on Wayland after I upgraded eframe from 0.23.0 to 0.24.1

dimtpap avatar Dec 13 '23 21:12 dimtpap

Likewise here, macOS. @OmegaJak's fix is a great tip, though šŸ‘

bglw avatar Dec 18 '23 02:12 bglw

Actually for those also running on the web, you'll need a non-std::thread implementation.

Here's an analog to the above using weird wasm/async stuff (I already had these specific crates hanging around, there might be better options here šŸ¤·):

Loop (using gloo-timers):

// TODO: Once https://github.com/emilk/egui/issues/3109 is resolved, use request_repaint_after in main loop instead
#[cfg(target_arch = "wasm32")]
pub async fn create_periodic_repaint_async(
    egui_ctx: egui::Context,
    millis_between_repaints: u32,
) {
    loop {
        egui_ctx.request_repaint();
        gloo_timers::future::TimeoutFuture::new(millis_between_repaints).await;
    }
}

Setup inside the creation context (using wasm-bindgen-futures):

// TODO: Once https://github.com/emilk/egui/issues/3109 is resolved, use request_repaint_after in main loop instead
#[cfg(target_arch = "wasm32")]
wasm_bindgen_futures::spawn_local(create_periodic_repaint_async(
    cc.egui_ctx.clone(),
));

bglw avatar Dec 18 '23 02:12 bglw

Is this still a problem in egui/eframe ~0.18~ 0.24? The repaint logic has been significantly rewritten

(EDIT: version snafu)

emilk avatar Dec 19 '23 12:12 emilk

@emilk Could you please elaborate what you mean by egui/eframe 0.18? I believe I am still able to replicate the issue on egui/eframe 0.24.1.

sdasda7777 avatar Dec 19 '23 14:12 sdasda7777

Yes I only encountered this issue this week after updating from .21.x to .24.x

bglw avatar Dec 19 '23 18:12 bglw

I don't know how I miss-typed "0.24" as "0.18", but I did šŸ¤¦

emilk avatar Dec 20 '23 16:12 emilk

I don't know how I miss-typed "0.24" as "0.18", but I did šŸ¤¦

Adding some more data points. I didn't encounter this issue until after upgrading to 0.24.1 from 0.23 (macOS). I don't have a thorough understanding of the implementation yet but it seems like this is where the request_repaint duration gets ignored:

https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L82

If I add a dummy callback to the egui context then the request_repaintduration ~~has effect again~~ is also ignored but the reactive behavior is back (instead of running full FPS always):

cc.egui_ctx.set_request_repaint_callback(|_| {});

jayzhudev avatar Dec 28 '23 17:12 jayzhudev

I don't have a thorough understanding of the implementation yet but it seems like this is where the request_repaint duration gets ignored:

https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L82

Both glow and wgpu integrations call set_request_repaint_callback on init

If I add a dummy callback to the egui context then the request_repaintduration ~~has effect again~~ is also ignored but the reactive behavior is back

I think that's just because you're overriding the integration's callback

dimtpap avatar Dec 28 '23 21:12 dimtpap

I don't have a thorough understanding of the implementation yet but it seems like this is where the request_repaint duration gets ignored: https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L82

Both glow and wgpu integrations call set_request_repaint_callback on init

If I add a dummy callback to the egui context then the request_repaintduration ~has effect again~ is also ignored but the reactive behavior is back

I think that's just because you're overriding the integration's callback

That's right, I read a bit more after my previous comment. Based on the limited time I got to wonder around. It seems like it's somewhere around

https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L80 and https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L261

that's causing this issue. I don't know what's behind https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L80 so I'm not sure what's the correct and simplest fix that won't change other expected behaviors. But I hope this observation helps.

jayzhudev avatar Dec 28 '23 22:12 jayzhudev