diff --git a/Cargo.lock b/Cargo.lock index 3ff1c844a..52812471c 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 9c3676cd7..aa917fc63 100644 --- a/Cargo.toml +++ b/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" } diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index 17bbb7e1b..64ab5443d 100644 --- a/apps/effectcraft/src/main.rs +++ b/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(); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index ede0196e2..b1e281627 100644 --- a/crates/ui-egui/Cargo.toml +++ b/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 } diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index e92adfff8..115c5f33a 100644 --- a/crates/ui-egui/src/app.rs +++ b/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`. diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index c0f9c7bd1..d41e0a9d0 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/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 { diff --git a/crates/ui-egui/src/panels/unsaved.rs b/crates/ui-egui/src/panels/unsaved.rs index 05f719fa9..b5fc582b2 100644 --- a/crates/ui-egui/src/panels/unsaved.rs +++ b/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"); + } } diff --git a/crates/ui-egui/tests/it/ui_gpu.rs b/crates/ui-egui/tests/it/ui_gpu.rs index 906ac0cd1..355c35bf5 100644 --- a/crates/ui-egui/tests/it/ui_gpu.rs +++ b/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); }