Merge pull request #675 from storytold/claude/community-pr-review-lwtrlr

Community batch: pointer-wrap test texture deltas, device-lost dialog

apps/effectcraft/src/main.rs

@@ -210,6 +210,22 @@ fn main() -> eframe::Result {
             app.hooks.pick_save = Some(Box::new(|name: &str| {
                 rfd::FileDialog::new().add_filter("EffectCraft Project", &["ecproj"]).set_file_name(name).save_file().map(|p| p.to_string_lossy().to_string())
             }));
+            // The window closes after a lost graphics device (#591); say why, unless an agent drives
+            // the app through the control channel.
+            if control_port.is_none() {
+                app.hooks.device_lost = Some(Box::new(|saved: Option<Result<String, String>>| {
+                    let saved = match saved {
+                        Some(Ok(path)) => format!("\n\nYour unsaved changes were saved to:\n{path}\n\nReopen EffectCraft to recover them."),
+                        Some(Err(e)) => format!("\n\nYour unsaved changes could not be saved ({e})."),
+                        None => String::new(),
+                    };
+                    rfd::MessageDialog::new()
+                        .set_level(rfd::MessageLevel::Error)
+                        .set_title("EffectCraft: graphics device lost")
+                        .set_description(format!("The graphics driver stopped responding, so EffectCraft has to close.{saved}"))
+                        .show();
+                }));
+            }
             app.hooks.pick_open_project = Some(Box::new(|| {
                 rfd::FileDialog::new().add_filter("EffectCraft Project", &["ecproj", "ecprojx"]).pick_file().map(|p| p.to_string_lossy().to_string())
             }));

contributors/people.toml

@@ -49,3 +49,8 @@ sync_github_name = false
 real_name = "Julen Llop"
 display_name = "Julen Llop"
 sync_github_name = false
+
+[people.vbasky]
+real_name = "Vikram Bhaskaran"
+display_name = "Vikram Bhaskaran"
+sync_github_name = false

crates/ui-egui/src/lib.rs

@@ -159,6 +159,9 @@ pub struct Hooks {
     /// the commands' dialogs (File ▸ Import, Open, Save As…) where a modal dialog's nested event
     /// loop inside a frame is unsafe: on macOS a drag over the window then aborted the app (#511).
     pub file_dialog_async: Option<Box<dyn Fn(FileDialog, DialogDone)>>,
+    /// Tells the user the graphics device was lost and the window is closing, once. `saved` is
+    /// the auto-save of unsaved changes (`None`: nothing was unsaved), its path or why it failed.
+    pub device_lost: Option<Box<dyn Fn(Option<Result<String, String>>)>>,
 }
 
 impl Hooks {
@@ -697,14 +700,21 @@ impl EffectcraftApp {
             return;
         }
         let keep_recovery = self.session.is_dirty();
-        if keep_recovery {
-            match self.session.autosave_now().and_then(|path| self.session.autosave_wait().map(|()| path)) {
-                Ok(path) => log::warn!("graphics device lost: auto-saved {path}; the next launch offers to recover it"),
-                Err(e) => log::error!("graphics device lost: {e}"),
+        let saved = keep_recovery.then(|| match self.session.autosave_now().and_then(|path| self.session.autosave_wait().map(|()| path)) {
+            Ok(path) => {
+                log::warn!("graphics device lost: auto-saved {path}; the next launch offers to recover it");
+                Ok(path)
             }
-        }
+            Err(e) => {
+                log::error!("graphics device lost: {e}");
+                Err(e.to_string())
+            }
+        });
         self.device_lost_close = Some(keep_recovery);
         log::warn!("graphics device lost: closing the window");
+        if let Some(tell) = &self.hooks.device_lost {
+            tell(saved);
+        }
         ctx.send_viewport_cmd(egui::ViewportCommand::Close);
     }
 

crates/ui-egui/src/pointer_wrap.rs

@@ -224,7 +224,7 @@ mod tests {
             };
             raw.events.push(Event::ModifiersChanged(self.modifiers));
             prepare_input(&self.ctx, &mut raw, self.injected);
-            let _ = self.ctx.run_ui(raw, |ui| {
+            let mut out = self.ctx.run_ui(raw, |ui| {
                 if self.standard {
                     ui.put(
                         Rect::from_min_size(pos2(100.0, 100.0), vec2(60.0, 20.0)),
@@ -266,6 +266,7 @@ mod tests {
                     self.accept_warp.then_some(p)
                 });
             });
+            out.textures_delta.clear();
         }
 
         fn move_to(&mut self, p: Pos2) {
@@ -482,7 +483,7 @@ mod tests {
                 let mut raw = RawInput { screen_rect: Some(bounds), time: Some(time), events, ..Default::default() };
                 app.raw_input_hook(&ctx, &mut raw);
                 let mut warp = None;
-                let _ = ctx.run_ui(raw, |ui| {
+                let mut out = ctx.run_ui(raw, |ui| {
                     app.logic(ui.ctx(), &mut frame);
                     app.ui(ui, &mut frame);
                     finish_frame(ui.ctx(), bounds, |p| {
@@ -490,6 +491,7 @@ mod tests {
                         Some(p)
                     });
                 });
+                out.textures_delta.clear();
                 warp
             };
             for _ in 0..4 {

crates/ui-egui/tests/ui_gpu.rs

@@ -98,6 +98,9 @@ fn a_lost_device_draws_nothing_autosaves_and_closes() {
     s.execute("comp.new", json!({"name": "Unsaved", "width": 64, "height": 64, "duration": 1})).unwrap();
     assert!(s.is_dirty());
     let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| EffectcraftApp::new(s));
+    let told = std::rc::Rc::new(std::cell::RefCell::new(Vec::new()));
+    let log = told.clone();
+    h.state_mut().hooks.device_lost = Some(Box::new(move |saved| log.borrow_mut().push(saved)));
     h.run_steps(3);
     assert!(!h.output().shapes.is_empty(), "the UI draws before the loss");
     let bridge = effectcraft_ui_egui::gpu_failure::GpuFailureBridge::new(&h.ctx);
@@ -111,5 +114,6 @@ fn a_lost_device_draws_nothing_autosaves_and_closes() {
     assert!(h.output().shapes.is_empty());
     let saved = h.state().session.autosave.last_path.clone().expect("unsaved changes auto-saved");
     assert!(std::path::Path::new(&saved).is_file(), "{saved}");
+    assert_eq!(*told.borrow(), vec![Some(Ok(saved))], "the user is told once, with the recovery copy");
     let _ = std::fs::remove_dir_all(&dir);
 }