mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-29 12:37:08 -06:00
Fix #341: on macOS, a quit from a visible window goes through terminate:
Quit EffectCraft and Finder's Quit send `ViewportCommand::Close`, and the close button sends the window's close request. In each case eframe drops the window while AppKit's run loop is still running. On a Mac with a Touch Bar, AppKit's Touch Bar code then raises an uncaught Objective-C exception (`-[_NSTouchBarFinderObservation invalidate]`) and the app crashes (storytold/craft-libs#3). After the unsaved-changes check, `close_or_quit` calls `craft_quit::on_close_request` from craft-libs, pinned to the rev that craft-docpath already uses. When nothing holds the window open, it cancels the close and quits through `-[NSApplication terminate:]`, which still runs `on_exit` (window position, workspace, crash-recovery cleanup). The prompt for a modified project still holds the window. The window close after a lost graphics device takes the same route. On other platforms it does nothing. pdfcraft adds craft-quit the same way (storytold/pdfcraft#1533). A quit while the window is minimized or hidden still closes the window as before: eframe does not run `ui` then, and the unsaved-changes check and the new call both run from `ui`. `terminate:` ends the process before `eframe::run_native` returns, so the log line for the closing window moves from `main` into `on_exit`.
This commit is contained in:
Generated
+12
@@ -982,6 +982,17 @@ dependencies = [
|
||||
"thiserror 2.0.21",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "craft-quit"
|
||||
version = "0.1.0"
|
||||
source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63"
|
||||
dependencies = [
|
||||
"dispatch2",
|
||||
"egui",
|
||||
"objc2 0.6.4",
|
||||
"objc2-app-kit 0.3.2",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "craft-ui"
|
||||
version = "0.1.0"
|
||||
@@ -1651,6 +1662,7 @@ dependencies = [
|
||||
name = "effectcraft-ui-egui"
|
||||
version = "0.11.0"
|
||||
dependencies = [
|
||||
"craft-quit",
|
||||
"craft-ui",
|
||||
"effectcraft-color",
|
||||
"effectcraft-engine",
|
||||
|
||||
@@ -13,6 +13,7 @@ repository = "https://github.com/storytold/effectcraft"
|
||||
[workspace.dependencies]
|
||||
craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "ca50d63b773c99f96e75c77ce5a1d21e7c9cda0d", version = "=0.1.0", default-features = false, features = ["read"] }
|
||||
craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" }
|
||||
craft-quit = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" }
|
||||
# internal
|
||||
effectcraft-time = { path = "crates/time" }
|
||||
effectcraft-geom = { path = "crates/geom" }
|
||||
|
||||
@@ -327,14 +327,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
|
||||
}
|
||||
@@ -429,6 +426,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();
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -1657,7 +1657,7 @@ impl EffectcraftApp {
|
||||
self.run_script_tasks(&ctx);
|
||||
self.apply_prefs(&ctx);
|
||||
self.handle_events(&ctx);
|
||||
panels::unsaved::on_close_requested(self, &ctx);
|
||||
panels::unsaved::close_or_quit(self, &ctx);
|
||||
let title = panels::unsaved::window_title(self);
|
||||
if title != self.window_title {
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::Title(title.clone()));
|
||||
@@ -1903,6 +1903,9 @@ impl eframe::App for EffectcraftApp {
|
||||
fn ui(&mut self, ui: &mut egui::Ui, frame: &mut eframe::Frame) {
|
||||
if self.gpu_failure_applied == Some(true) {
|
||||
self.close_after_device_loss(ui.ctx());
|
||||
// On macOS the app quits through `-[NSApplication terminate:]` instead of closing the
|
||||
// window while AppKit's run loop is running (#341).
|
||||
craft_quit::on_close_request(ui.ctx());
|
||||
return;
|
||||
}
|
||||
self.init_gpu(frame);
|
||||
|
||||
@@ -58,6 +58,15 @@ pub fn on_close_requested(app: &mut EffectcraftApp, ctx: &egui::Context) {
|
||||
}
|
||||
}
|
||||
|
||||
/// [`on_close_requested`], then 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. Returns whether that quit was
|
||||
/// scheduled; other platforms close the window as before.
|
||||
pub fn close_or_quit(app: &mut EffectcraftApp, ctx: &egui::Context) -> bool {
|
||||
on_close_requested(app, ctx);
|
||||
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,23 @@ 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 [`close_or_quit`]
|
||||
/// as the app runs it; 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();
|
||||
quits = close_or_quit(app, 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 +366,29 @@ 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");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user