Repaint rework: more responsive, less energy

Previously, we repainted every frame on Windows at full refresh rate.
This is an enormous waste, as the UI will be static most of the time.
This was to work around a bug with `rfd` + `eframe`.
On other platforms, we only repainted every frame when a job was running,
which was better, but still not ideal. We also had a 100ms deadline, so
we'd repaint at ~10fps minimum to catch new events (file watcher, jobs).

This removes all repaint logic from the main loop and moves it into the
individual places where we change state from another thread.
For example, the file watcher thread will now immediately notify egui
to repaint, rather than relying on the 100ms deadline we had previously.
Jobs, when updating their status, also notify egui to repaint.

For `rfd` file dialogs, this migrates to using the async API built on top of
a polling thread + `pollster`. This interacts better with `eframe` on Windows.
Overall, this should reduce repaints and improve responsiveness to
file changes and background tasks.

Author: encounter

Cargo.lock

@@ -42,6 +42,7 @@ memmap2 = "0.9.0"
 notify = "6.1.1"
 object = { version = "0.32.1", features = ["read_core", "std", "elf"], default-features = false }
 png = "0.17.10"
+pollster = "0.3.0"
 ppc750cl = { git = "https://github.com/encounter/ppc750cl", rev = "4a2bbbc6f84dcb76255ab6f3595a8d4a0ce96618" }
 rabbitizer = "1.8.0"
 rfd = { version = "0.12.1" } #, default-features = false, features = ['xdg-portal']

Cargo.toml

@@ -7,7 +7,6 @@ use std::{
         atomic::{AtomicBool, Ordering},
         Arc, Mutex, RwLock,
     },
-    time::Duration,
 };
 
 use filetime::FileTime;
@@ -29,7 +28,9 @@ use crate::{
             config_ui, diff_options_window, project_window, ConfigViewState, DEFAULT_WATCH_PATTERNS,
         },
         data_diff::data_diff_ui,
+        debug::debug_window,
         demangle::{demangle_window, DemangleViewState},
+        frame_history::FrameHistory,
         function_diff::function_diff_ui,
         jobs::jobs_ui,
         symbol_diff::{symbol_diff_ui, DiffViewState, View},
@@ -42,10 +43,12 @@ pub struct ViewState {
     pub config_state: ConfigViewState,
     pub demangle_state: DemangleViewState,
     pub diff_state: DiffViewState,
+    pub frame_history: FrameHistory,
     pub show_appearance_config: bool,
     pub show_demangle: bool,
     pub show_project_config: bool,
     pub show_diff_options: bool,
+    pub show_debug: bool,
 }
 
 /// The configuration for a single object file.
@@ -257,7 +260,7 @@ impl App {
                     log::info!("Job {} finished", job.id);
                     match result {
                         JobResult::None => {
-                            if let Some(err) = &job.status.read().unwrap().error {
+                            if let Some(err) = &job.context.status.read().unwrap().error {
                                 log::error!("{:?}", err);
                             }
                         }
@@ -278,12 +281,12 @@ impl App {
                     } else {
                         anyhow::Error::msg("Thread panicked")
                     };
-                    let result = job.status.write();
+                    let result = job.context.status.write();
                     if let Ok(mut guard) = result {
                         guard.error = Some(err);
                     } else {
                         drop(result);
-                        job.status = Arc::new(RwLock::new(JobStatus {
+                        job.context.status = Arc::new(RwLock::new(JobStatus {
                             title: "Error".to_string(),
                             progress_percent: 0.0,
                             progress_items: None,
@@ -298,14 +301,14 @@ impl App {
         jobs.clear_finished();
 
         diff_state.pre_update(jobs, &self.config);
-        config_state.pre_update(jobs);
+        config_state.pre_update(jobs, &self.config);
         debug_assert!(jobs.results.is_empty());
     }
 
-    fn post_update(&mut self) {
+    fn post_update(&mut self, ctx: &egui::Context) {
         let ViewState { jobs, diff_state, config_state, .. } = &mut self.view_state;
-        config_state.post_update(jobs, &self.config);
-        diff_state.post_update(jobs, &self.config);
+        config_state.post_update(ctx, jobs, &self.config);
+        diff_state.post_update(&self.config);
 
         let Ok(mut config) = self.config.write() else {
             return;
@@ -335,7 +338,7 @@ impl App {
             if let Some(project_dir) = &config.project_dir {
                 match build_globset(&config.watch_patterns).map_err(anyhow::Error::new).and_then(
                     |globset| {
-                        create_watcher(self.modified.clone(), project_dir, globset)
+                        create_watcher(ctx.clone(), self.modified.clone(), project_dir, globset)
                             .map_err(anyhow::Error::new)
                     },
                 ) {
@@ -374,15 +377,15 @@ impl App {
         // Don't clear `queue_build` if a build is running. A file may have been modified during
         // the build, so we'll start another build after the current one finishes.
         if config.queue_build && config.selected_obj.is_some() && !jobs.is_running(Job::ObjDiff) {
-            jobs.push(start_build(ObjDiffConfig::from_config(config)));
+            jobs.push(start_build(ctx, ObjDiffConfig::from_config(config)));
             config.queue_build = false;
             config.queue_reload = false;
         } else if config.queue_reload && !jobs.is_running(Job::ObjDiff) {
             let mut diff_config = ObjDiffConfig::from_config(config);
             // Don't build, just reload the current files
             diff_config.build_base = false;
             diff_config.build_target = false;
-            jobs.push(start_build(diff_config));
+            jobs.push(start_build(ctx, diff_config));
             config.queue_reload = false;
         }
     }
@@ -404,18 +407,27 @@ impl eframe::App for App {
 
         let ViewState {
             jobs,
-            show_appearance_config,
+            config_state,
             demangle_state,
-            show_demangle,
             diff_state,
-            config_state,
+            frame_history,
+            show_appearance_config,
+            show_demangle,
             show_project_config,
             show_diff_options,
+            show_debug,
         } = view_state;
 
+        frame_history.on_new_frame(ctx.input(|i| i.time), frame.info().cpu_usage);
+
         egui::TopBottomPanel::top("top_panel").show(ctx, |ui| {
             egui::menu::bar(ui, |ui| {
                 ui.menu_button("File", |ui| {
+                    #[cfg(debug_assertions)]
+                    if ui.button("Debug…").clicked() {
+                        *show_debug = !*show_debug;
+                        ui.close_menu();
+                    }
                     if ui.button("Project…").clicked() {
                         *show_project_config = !*show_project_config;
                         ui.close_menu();
@@ -511,16 +523,9 @@ impl eframe::App for App {
         appearance_window(ctx, show_appearance_config, appearance);
         demangle_window(ctx, show_demangle, demangle_state, appearance);
         diff_options_window(ctx, config, show_diff_options, appearance);
+        debug_window(ctx, show_debug, frame_history, appearance);
 
-        self.post_update();
-
-        // Windows + request_repaint_after breaks dialogs:
-        // https://github.com/emilk/egui/issues/2003
-        if cfg!(windows) || self.view_state.jobs.any_running() {
-            ctx.request_repaint();
-        } else {
-            ctx.request_repaint_after(Duration::from_millis(100));
-        }
+        self.post_update(ctx);
     }
 
     /// Called by the frame work to save state before shutdown.
@@ -533,6 +538,7 @@ impl eframe::App for App {
 }
 
 fn create_watcher(
+    ctx: egui::Context,
     modified: Arc<AtomicBool>,
     project_dir: &Path,
     patterns: GlobSet,
@@ -552,7 +558,9 @@ fn create_watcher(
                             continue;
                         };
                         if patterns.is_match(path) {
+                            log::info!("File modified: {}", path.display());
                             modified.store(true, Ordering::Relaxed);
+                            ctx.request_repaint();
                         }
                     }
                 }

src/app.rs

@@ -4,7 +4,7 @@ use anyhow::{Context, Result};
 use self_update::{cargo_crate_version, update::Release};
 
 use crate::{
-    jobs::{start_job, update_status, Job, JobResult, JobState, JobStatusRef},
+    jobs::{start_job, update_status, Job, JobContext, JobResult, JobState},
     update::{build_updater, BIN_NAME},
 };
 
@@ -14,20 +14,20 @@ pub struct CheckUpdateResult {
     pub found_binary: bool,
 }
 
-fn run_check_update(status: &JobStatusRef, cancel: Receiver<()>) -> Result<Box<CheckUpdateResult>> {
-    update_status(status, "Fetching latest release".to_string(), 0, 1, &cancel)?;
+fn run_check_update(context: &JobContext, cancel: Receiver<()>) -> Result<Box<CheckUpdateResult>> {
+    update_status(context, "Fetching latest release".to_string(), 0, 1, &cancel)?;
     let updater = build_updater().context("Failed to create release updater")?;
     let latest_release = updater.get_latest_release()?;
     let update_available =
         self_update::version::bump_is_greater(cargo_crate_version!(), &latest_release.version)?;
     let found_binary = latest_release.assets.iter().any(|a| a.name == BIN_NAME);
 
-    update_status(status, "Complete".to_string(), 1, 1, &cancel)?;
+    update_status(context, "Complete".to_string(), 1, 1, &cancel)?;
     Ok(Box::new(CheckUpdateResult { update_available, latest_release, found_binary }))
 }
 
-pub fn start_check_update() -> JobState {
-    start_job("Check for updates", Job::CheckUpdate, move |status, cancel| {
-        run_check_update(status, cancel).map(|result| JobResult::CheckUpdate(Some(result)))
+pub fn start_check_update(ctx: &egui::Context) -> JobState {
+    start_job(ctx, "Check for updates", Job::CheckUpdate, move |context, cancel| {
+        run_check_update(&context, cancel).map(|result| JobResult::CheckUpdate(Some(result)))
     })
 }

src/jobs/check_update.rs

@@ -48,6 +48,7 @@ impl JobQueue {
     }
 
     /// Returns whether any job is running.
+    #[allow(dead_code)]
     pub fn any_running(&self) -> bool {
         self.jobs.iter().any(|job| {
             if let Some(handle) = &job.handle {
@@ -81,21 +82,25 @@ impl JobQueue {
         self.jobs.retain(|job| {
             !(job.should_remove
                 && job.handle.is_none()
-                && job.status.read().unwrap().error.is_none())
+                && job.context.status.read().unwrap().error.is_none())
         });
     }
 
     /// Removes a job from the queue given its ID.
     pub fn remove(&mut self, id: usize) { self.jobs.retain(|job| job.id != id); }
 }
 
-pub type JobStatusRef = Arc<RwLock<JobStatus>>;
+#[derive(Clone)]
+pub struct JobContext {
+    pub status: Arc<RwLock<JobStatus>>,
+    pub egui: egui::Context,
+}
 
 pub struct JobState {
     pub id: usize,
     pub kind: Job,
     pub handle: Option<JoinHandle<JobResult>>,
-    pub status: JobStatusRef,
+    pub context: JobContext,
     pub cancel: Sender<()>,
     pub should_remove: bool,
 }
@@ -124,9 +129,10 @@ fn should_cancel(rx: &Receiver<()>) -> bool {
 }
 
 fn start_job(
+    ctx: &egui::Context,
     title: &str,
     kind: Job,
-    run: impl FnOnce(&JobStatusRef, Receiver<()>) -> Result<JobResult> + Send + 'static,
+    run: impl FnOnce(JobContext, Receiver<()>) -> Result<JobResult> + Send + 'static,
 ) -> JobState {
     let status = Arc::new(RwLock::new(JobStatus {
         title: title.to_string(),
@@ -135,10 +141,11 @@ fn start_job(
         status: String::new(),
         error: None,
     }));
-    let status_clone = status.clone();
+    let context = JobContext { status: status.clone(), egui: ctx.clone() };
+    let context_inner = JobContext { status: status.clone(), egui: ctx.clone() };
     let (tx, rx) = std::sync::mpsc::channel();
     let handle = std::thread::spawn(move || {
-        return match run(&status, rx) {
+        return match run(context_inner, rx) {
             Ok(state) => state,
             Err(e) => {
                 if let Ok(mut w) = status.write() {
@@ -150,24 +157,18 @@ fn start_job(
     });
     let id = JOB_ID.fetch_add(1, Ordering::Relaxed);
     log::info!("Started job {}", id);
-    JobState {
-        id,
-        kind,
-        handle: Some(handle),
-        status: status_clone,
-        cancel: tx,
-        should_remove: true,
-    }
+    JobState { id, kind, handle: Some(handle), context, cancel: tx, should_remove: true }
 }
 
 fn update_status(
-    status: &JobStatusRef,
+    context: &JobContext,
     str: String,
     count: u32,
     total: u32,
     cancel: &Receiver<()>,
 ) -> Result<()> {
-    let mut w = status.write().map_err(|_| anyhow::Error::msg("Failed to lock job status"))?;
+    let mut w =
+        context.status.write().map_err(|_| anyhow::Error::msg("Failed to lock job status"))?;
     w.progress_items = Some([count, total]);
     w.progress_percent = count as f32 / total as f32;
     if should_cancel(cancel) {
@@ -176,5 +177,7 @@ fn update_status(
     } else {
         w.status = str;
     }
+    drop(w);
+    context.egui.request_repaint();
     Ok(())
 }

src/jobs/mod.rs

@@ -11,7 +11,7 @@ use time::OffsetDateTime;
 use crate::{
     app::{AppConfig, ObjectConfig},
     diff::{diff_objs, DiffAlg, DiffObjConfig},
-    jobs::{start_job, update_status, Job, JobResult, JobState, JobStatusRef},
+    jobs::{start_job, update_status, Job, JobContext, JobResult, JobState},
     obj::{elf, ObjInfo},
 };
 
@@ -102,7 +102,7 @@ fn run_make(cwd: &Path, arg: &Path, config: &ObjDiffConfig) -> BuildStatus {
 }
 
 fn run_build(
-    status: &JobStatusRef,
+    context: &JobContext,
     cancel: Receiver<()>,
     config: ObjDiffConfig,
 ) -> Result<Box<ObjDiffResult>> {
@@ -142,7 +142,7 @@ fn run_build(
     let first_status = match target_path_rel {
         Some(target_path_rel) if config.build_target => {
             update_status(
-                status,
+                context,
                 format!("Building target {}", target_path_rel.display()),
                 0,
                 total,
@@ -156,7 +156,7 @@ fn run_build(
     let second_status = match base_path_rel {
         Some(base_path_rel) if config.build_base => {
             update_status(
-                status,
+                context,
                 format!("Building base {}", base_path_rel.display()),
                 0,
                 total,
@@ -173,7 +173,7 @@ fn run_build(
         match &obj_config.target_path {
             Some(target_path) if first_status.success => {
                 update_status(
-                    status,
+                    context,
                     format!("Loading target {}", target_path_rel.unwrap().display()),
                     2,
                     total,
@@ -189,7 +189,7 @@ fn run_build(
     let mut second_obj = match &obj_config.base_path {
         Some(base_path) if second_status.success => {
             update_status(
-                status,
+                context,
                 format!("Loading base {}", base_path_rel.unwrap().display()),
                 3,
                 total,
@@ -203,16 +203,16 @@ fn run_build(
         _ => None,
     };
 
-    update_status(status, "Performing diff".to_string(), 4, total, &cancel)?;
+    update_status(context, "Performing diff".to_string(), 4, total, &cancel)?;
     let diff_config = DiffObjConfig { code_alg: config.code_alg, data_alg: config.data_alg };
     diff_objs(&diff_config, first_obj.as_mut(), second_obj.as_mut())?;
 
-    update_status(status, "Complete".to_string(), total, total, &cancel)?;
+    update_status(context, "Complete".to_string(), total, total, &cancel)?;
     Ok(Box::new(ObjDiffResult { first_status, second_status, first_obj, second_obj, time }))
 }
 
-pub fn start_build(config: ObjDiffConfig) -> JobState {
-    start_job("Object diff", Job::ObjDiff, move |status, cancel| {
-        run_build(status, cancel, config).map(|result| JobResult::ObjDiff(Some(result)))
+pub fn start_build(ctx: &egui::Context, config: ObjDiffConfig) -> JobState {
+    start_job(ctx, "Object diff", Job::ObjDiff, move |context, cancel| {
+        run_build(&context, cancel, config).map(|result| JobResult::ObjDiff(Some(result)))
     })
 }