Merge pull request #1762 from dotCooCoo/fix/macos-quit-crash

Fix #341: on macOS, a quit goes through `terminate:`

Cargo.lock

@@ -1086,6 +1086,17 @@ dependencies = [
  "thiserror 2.0.21",
 ]
 
+[[package]]
+name = "craft-quit"
+version = "0.1.0"
+source = "git+https://github.com/storytold/craft-libs?rev=92d392cff132eaf53e13afff77f38b5c11b73aa0#92d392cff132eaf53e13afff77f38b5c11b73aa0"
+dependencies = [
+ "dispatch2",
+ "egui",
+ "objc2 0.6.5",
+ "objc2-app-kit 0.3.2",
+]
+
 [[package]]
 name = "craft-ui"
 version = "0.1.0"
@@ -1822,6 +1833,7 @@ name = "effectcraft-ui-egui"
 version = "0.18.0"
 dependencies = [
  "chrono",
+ "craft-quit",
  "craft-ui",
  "craft-update",
  "effectcraft-color",

Cargo.toml

@@ -13,6 +13,7 @@ repository = "https://github.com/storytold/effectcraft"
 [workspace.dependencies]
 craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0", version = "=0.1.0", default-features = false, features = ["read"] }
 craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0" }
+craft-quit = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0" }
 craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "92d392cff132eaf53e13afff77f38b5c11b73aa0", features = ["egui"] }
 # internal
 effectcraft-time = { path = "crates/time" }

apps/effectcraft/src/main.rs

@@ -365,14 +365,11 @@ fn main() -> eframe::Result {
             }))
         }),
     );
-    match &result {
-        Ok(()) => log::info!("the window closed"),
-        Err(e) => {
-            if on_gl {
-                launch_guard::opengl_failed(effectcraft_host::config_dir().as_deref());
-            }
-            startup_failed(e, !on_gl && cfg!(not(target_os = "macos")));
+    if let Err(e) = &result {
+        if on_gl {
+            launch_guard::opengl_failed(effectcraft_host::config_dir().as_deref());
         }
+        startup_failed(e, !on_gl && cfg!(not(target_os = "macos")));
     }
     result
 }
@@ -455,7 +452,8 @@ struct Desktop {
 }
 
 impl eframe::App for Desktop {
-    /// The desktop host's logic tick: update the macOS menu bar, run the shared app tick and close the IME pass.
+    /// The desktop host's logic tick: update the macOS menu bar, run the shared app tick, close the IME pass, show the
+    /// file dialogs asked for and, last, hand a close to the platform quit.
     fn logic(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
         #[cfg(target_os = "macos")]
         self.menu.update(&mut self.app, ctx);
@@ -464,6 +462,7 @@ impl eframe::App for Desktop {
         if let Some(asked) = &self.file_dialogs {
             sheet_dialogs::show_asked(asked, frame, ctx);
         }
+        effectcraft_ui_egui::panels::unsaved::quit_on_close(ctx);
     }
 
     fn ui(&mut self, ui: &mut egui::Ui, frame: &mut eframe::Frame) {
@@ -484,6 +483,9 @@ impl eframe::App for Desktop {
     }
 
     fn on_exit(&mut self) {
+        // On macOS the app quits through `-[NSApplication terminate:]`, and the process ends
+        // before `eframe::run_native` returns (#341).
+        log::info!("the window is closing");
         self.launch.exited();
         self.window.save(self.app.session.config.as_deref());
         self.app.on_exit();

crates/ui-egui/Cargo.toml

@@ -11,6 +11,7 @@ workspace = true
 
 [dependencies]
 craft-ui = { workspace = true }
+craft-quit = { workspace = true, features = ["egui"] }
 effectcraft-time = { workspace = true }
 effectcraft-geom = { workspace = true }
 effectcraft-color = { workspace = true }

crates/ui-egui/src/app.rs

@@ -150,13 +150,15 @@ impl eframe::App for EffectcraftApp {
         }
     }
 
-    /// The logic-only tick: run `tick` and close the IME pass with `finish_ime_logic`. It does not read pass-time
+    /// The logic-only tick: run `tick`, close the IME pass with `finish_ime_logic` and, last, hand a close to the
+    /// platform quit (`panels::unsaved::quit_on_close`). It does not read pass-time
     /// input state (events, pointer and time are stale here), only the viewport information eframe refreshed.
     fn logic(&mut self, ctx: &egui::Context, frame: &mut eframe::Frame) {
         // Never `ctx.input(|i| i.time)` here: `Context::run_logic` leaves events, pointer state and
         // time at the previous full pass's values (only the viewport information is refreshed).
         self.tick(ctx, frame);
         self.finish_ime_logic(ctx);
+        crate::panels::unsaved::quit_on_close(ctx);
     }
 
     /// Draw one UI pass through `draw`.

crates/ui-egui/src/app/frame.rs

@@ -198,6 +198,8 @@ impl EffectcraftApp {
             self.command_inbox = Some(rx);
         }
         // A close request (the window's button, Quit, `app.quit`) must be answered while hidden.
+        // An answered file dialog can change the project, so the answers above run first. The host
+        // calls `panels::unsaved::quit_on_close` last in its `logic`.
         panels::unsaved::on_close_requested(self, ctx);
         let title = panels::unsaved::window_title(self);
         if title != self.window_title {

crates/ui-egui/src/panels/unsaved.rs

@@ -58,6 +58,15 @@ pub fn on_close_requested(app: &mut EffectcraftApp, ctx: &egui::Context) {
     }
 }
 
+/// On macOS a close that nothing held open quits through `-[NSApplication terminate:]` instead:
+/// closing the window while AppKit's run loop is running crashes on a Mac with a Touch Bar
+/// (#341). `on_exit` still runs. A desktop host calls it last in `logic`, after
+/// `EffectcraftApp::tick` (which runs [`on_close_requested`]) and its own work. Returns whether
+/// that quit was scheduled; other platforms close the window as before.
+pub fn quit_on_close(ctx: &egui::Context) -> bool {
+    craft_quit::on_close_request(ctx)
+}
+
 /// The project's display name: its file name, or `Untitled Project.ecproj`.
 pub fn project_name(app: &EffectcraftApp) -> String {
     app.session
@@ -207,6 +216,24 @@ mod tests {
         out.viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose))
     }
 
+    /// One frame of the dialogs with the window's close button pressed, through
+    /// [`on_close_requested`] and then [`quit_on_close`] as the app runs them; returns whether the
+    /// close was canceled and whether the platform quit was scheduled.
+    fn close_frame(app: &mut EffectcraftApp, ctx: &egui::Context) -> (bool, bool) {
+        let mut raw = egui::RawInput::default();
+        raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+        let mut quits = false;
+        let mut out = ctx.run_ui(raw, |ui| {
+            app.auto.begin_frame();
+            on_close_requested(app, ui.ctx());
+            quits = quit_on_close(ui.ctx());
+            crate::panels::dialogs::show(app, ui.ctx());
+        });
+        out.textures_delta.clear();
+        let canceled = out.viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose));
+        (canceled, quits)
+    }
+
     fn modified(app: &mut EffectcraftApp) {
         app.session.execute("comp.new", json!({"name": "Main", "width": 64, "height": 64})).unwrap();
         assert!(app.session.is_dirty());
@@ -340,4 +367,81 @@ mod tests {
         assert!(!app.session.is_dirty());
         assert!(!frame(&mut app, &ctx, true));
     }
+
+    /// Closing the window while AppKit's run loop is running crashes on a Mac with a Touch Bar
+    /// (#341). On macOS the close is canceled and the app quits through
+    /// `-[NSApplication terminate:]`. Other platforms close the window.
+    #[test]
+    fn closing_a_saved_project_quits_through_the_platform_on_macos() {
+        let (mut app, ctx) = app();
+        let macos = cfg!(target_os = "macos");
+        assert_eq!(close_frame(&mut app, &ctx), (macos, macos));
+        assert!(app.dialog.is_none(), "nothing to ask");
+    }
+
+    #[test]
+    fn closing_a_modified_project_asks_before_the_platform_quit() {
+        let (mut app, ctx) = app();
+        modified(&mut app);
+        assert_eq!(close_frame(&mut app, &ctx), (true, false), "the prompt holds the window and nothing quits");
+        assert_eq!(app.dialog, Some(Dialog::UnsavedChanges));
+        answer_with(&mut app, &ctx, Answer::Cancel);
+        assert!(app.session.is_dirty() && !app.dialog_state.unsaved.quitting);
+        assert_eq!(close_frame(&mut app, &ctx), (true, false), "asked again after Cancel");
+        answer_with(&mut app, &ctx, Answer::Discard);
+        let macos = cfg!(target_os = "macos");
+        assert_eq!(close_frame(&mut app, &ctx), (macos, macos), "Don't Save quits");
+    }
+
+    /// While the window is minimized or hidden, eframe runs `App::logic` without `App::ui` and
+    /// still handles a close: `logic` asks about a modified project.
+    #[test]
+    fn a_close_while_only_logic_runs_asks_about_a_modified_project() {
+        let (mut app, ctx) = app();
+        modified(&mut app);
+        let mut raw = egui::RawInput::default();
+        raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+        let mut frame = eframe::Frame::_new_kittest();
+        let out = ctx.run_logic(&raw, |ctx| eframe::App::logic(&mut app, ctx, &mut frame));
+        let canceled = out.viewport_commands.get(&egui::ViewportId::ROOT).is_some_and(|c| c.contains(&egui::ViewportCommand::CancelClose));
+        assert!(canceled, "the close is canceled");
+        assert_eq!(app.dialog, Some(Dialog::UnsavedChanges));
+    }
+
+    /// A file dialog answered in the frame that closes the window runs its command before the
+    /// close decision, so a change it makes is asked about.
+    #[test]
+    fn a_dialog_answer_runs_before_the_close_decision() {
+        let (mut app, ctx) = app();
+        assert!(!app.session.is_dirty());
+        let answer = crate::DialogAnswer { command: "comp.new".into(), key: "name", params: json!({"width": 64, "height": 64}), paths: vec!["Main".into()] };
+        app.dialog_answers.0.send(answer).unwrap();
+        let mut raw = egui::RawInput::default();
+        raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+        let mut frame = eframe::Frame::_new_kittest();
+        let out = ctx.run_logic(&raw, |ctx| eframe::App::logic(&mut app, ctx, &mut frame));
+        let canceled = out.viewport_commands.get(&egui::ViewportId::ROOT).is_some_and(|c| c.contains(&egui::ViewportCommand::CancelClose));
+        assert!(app.session.is_dirty(), "the answer ran");
+        assert!(canceled, "the close is canceled");
+        assert_eq!(app.dialog, Some(Dialog::UnsavedChanges));
+    }
+
+    /// The tick makes the close decision and leaves the platform quit to the host, which runs it
+    /// last in `logic`, after its own work (the desktop host shows the file dialogs asked for).
+    /// On macOS `logic` cancels the close and quits; other platforms close the window.
+    #[test]
+    fn the_platform_quit_runs_after_the_tick() {
+        let (mut app, ctx) = app();
+        let mut raw = egui::RawInput::default();
+        raw.viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+        let frame = eframe::Frame::_new_kittest();
+        let canceled =
+            |out: &egui::LogicOutput| out.viewport_commands.get(&egui::ViewportId::ROOT).is_some_and(|c| c.contains(&egui::ViewportCommand::CancelClose));
+        let out = ctx.run_logic(&raw, |ctx| app.tick(ctx, &frame));
+        assert!(!canceled(&out), "the tick leaves a saved project's close to the host");
+        let mut frame = eframe::Frame::_new_kittest();
+        let out = ctx.run_logic(&raw, |ctx| eframe::App::logic(&mut app, ctx, &mut frame));
+        assert_eq!(canceled(&out), cfg!(target_os = "macos"), "logic quits through the platform on macOS");
+        assert!(app.dialog.is_none(), "nothing to ask");
+    }
 }

crates/ui-egui/tests/it/ui_gpu.rs

@@ -115,5 +115,12 @@ fn a_lost_device_draws_nothing_autosaves_and_closes() {
     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");
+    // The close arrives: on macOS the app quits through `-[NSApplication terminate:]` instead of
+    // closing the window while AppKit's run loop is running (#341).
+    h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close);
+    h.step();
+    let kept = h.output().viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose));
+    assert_eq!(kept, cfg!(target_os = "macos"), "the platform quit closes the window on macOS");
+    assert_eq!(told.borrow().len(), 1);
     let _ = std::fs::remove_dir_all(&dir);
 }