From d4e33b2bda4515d456408b9bd8afb7e536770a40 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 27 Oct 2026 22:46:27 -0700 Subject: [PATCH] Fix #341: the platform quit runs last in App::logic The merge of main put craft_quit::on_close_request inside EffectcraftApp::tick, so the control channel, autosave, screenshots and the desktop host's file dialogs still ran after it in the same logic callback. The tick keeps the unsaved-changes check; each desktop host's logic now calls panels::unsaved::quit_on_close last, after the device-loss close as well. --- apps/effectcraft/src/main.rs | 4 ++- crates/ui-egui/src/app.rs | 4 ++- crates/ui-egui/src/app/frame.rs | 8 +++--- crates/ui-egui/src/panels/unsaved.rs | 40 +++++++++++++++++++++------- 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/apps/effectcraft/src/main.rs b/apps/effectcraft/src/main.rs index 9c62e0688..43ee38add 100644 --- a/apps/effectcraft/src/main.rs +++ b/apps/effectcraft/src/main.rs @@ -452,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); @@ -461,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) { 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 fef72f68b..d41e0a9d0 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -131,9 +131,6 @@ impl EffectcraftApp { self.apply_gpu_failure(ctx); if self.gpu_failure_applied == Some(true) { self.close_after_device_loss(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(ctx); return; } // Focus comes from the fresh viewport copy; a full pass's `InputState::focused` is only a @@ -201,8 +198,9 @@ 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. - panels::unsaved::close_or_quit(self, ctx); + // 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 { ctx.send_viewport_cmd(egui::ViewportCommand::Title(title.clone())); diff --git a/crates/ui-egui/src/panels/unsaved.rs b/crates/ui-egui/src/panels/unsaved.rs index 4a94265b4..b5fc582b2 100644 --- a/crates/ui-egui/src/panels/unsaved.rs +++ b/crates/ui-egui/src/panels/unsaved.rs @@ -58,12 +58,12 @@ 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); +/// 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) } @@ -216,16 +216,17 @@ 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. + /// 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(); - quits = close_or_quit(app, ui.ctx()); + on_close_requested(app, ui.ctx()); + quits = quit_on_close(ui.ctx()); crate::panels::dialogs::show(app, ui.ctx()); }); out.textures_delta.clear(); @@ -424,4 +425,23 @@ mod tests { 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"); + } }