mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-13 23:17:30 +00:00
macOS: File menu dialogs open as sheets, so a drag over the window can't abort the app (#511)
The file dialogs were blocking rfd::FileDialog calls made inside eframe's frame, that is inside winit's event handler. On macOS the dialog runs a nested modal event loop there; a file dragged over the window while it was open reached winit's handler again, which refuses re-entry with a panic across the Objective-C boundary, aborting the app (Import with Cmd+I, then a drag). Hooks gains file_dialog_async: a host that sets it shows a command's file dialog (Import, Open, Save As, Save Frame As, presets, guides, templates, Collect Files, Relink...) without blocking, and the command runs with the answer on a later frame (menus::dialog_answered, drained at the start of each frame); a cancelled dialog runs nothing. The macOS app implements it with rfd::AsyncFileDialog, a sheet on the window answered when it closes, outside the frame. Other platforms keep the blocking dialogs (winit queues their re-entrant events), as do the few panel buttons that pick a file in place (Settings paths, shortcut import/export, effect file pickers). Test: with a non-blocking hook, Save As and Save Frame As return at once without the blocking dialog, the save happens when the answer arrives from another thread, and a cancelled dialog writes nothing. The macOS sheet itself needs a manual check (Cmd+I, drag a file over the window, pick or cancel). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f
This commit is contained in:
6 files changed
+220
-12
No files matched your search
Generated
+1
@@ -1148,6 +1148,7 @@ dependencies = [
|
||||
"objc2 0.6.4",
|
||||
"objc2-app-kit 0.3.2",
|
||||
"objc2-foundation 0.3.2",
|
||||
"pollster 0.4.0",
|
||||
"rfd",
|
||||
"serde_json",
|
||||
"winit",
|
||||
|
||||
@@ -18,6 +18,7 @@ egui = { workspace = true }
|
||||
eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "links", "wayland", "x11"] }
|
||||
serde_json = { workspace = true }
|
||||
rfd = { workspace = true }
|
||||
pollster = { workspace = true }
|
||||
cpal = { workspace = true }
|
||||
log = { workspace = true }
|
||||
|
||||
|
||||
@@ -21,6 +21,9 @@ mod control_server;
|
||||
mod launch_guard;
|
||||
#[cfg(target_os = "macos")]
|
||||
mod native_menu;
|
||||
// Built everywhere so it stays checked; only the macOS app uses it.
|
||||
#[cfg_attr(not(target_os = "macos"), allow(dead_code))]
|
||||
mod sheet_dialogs;
|
||||
|
||||
use effectcraft_ui_egui::EffectcraftApp;
|
||||
use serde_json::json;
|
||||
@@ -227,6 +230,9 @@ fn main() -> eframe::Result {
|
||||
}));
|
||||
#[cfg(target_os = "macos")]
|
||||
{
|
||||
// The commands' file dialogs open as sheets instead of nested modal loops (#511).
|
||||
let ctx = cc.egui_ctx.clone();
|
||||
app.hooks.file_dialog_async = Some(Box::new(move |dialog, done| sheet_dialogs::show(dialog, done, ctx.clone())));
|
||||
app.hooks.app_action = Some(Box::new(native_menu::app_action));
|
||||
app.hooks.clipboard_text = Some(Box::new(native_menu::clipboard_text));
|
||||
}
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
//! File dialogs that don't block the frame (macOS), for `Hooks::file_dialog_async`.
|
||||
//!
|
||||
//! A blocking `rfd::FileDialog` runs a nested modal event loop inside eframe's frame, that is
|
||||
//! inside winit's event handler. On macOS a file dragged over the window while that loop ran
|
||||
//! re-entered winit's handler, which panics across the Objective-C boundary and aborts the app
|
||||
//! (#511). `rfd::AsyncFileDialog` shows the dialog as a sheet on the window and returns at once;
|
||||
//! the answer arrives when the sheet closes, outside the frame, and the app runs the command then.
|
||||
|
||||
use effectcraft_ui_egui::{DialogDone, FileDialog};
|
||||
use rfd::{AsyncFileDialog, FileHandle};
|
||||
use std::future::Future;
|
||||
use std::pin::Pin;
|
||||
|
||||
type Answer = Pin<Box<dyn Future<Output = Vec<String>> + Send>>;
|
||||
|
||||
/// Show `dialog` (on the main thread: the sheet opens here) and hand its paths to `done` once it
|
||||
/// closes, then wake the UI.
|
||||
pub fn show(dialog: FileDialog, done: DialogDone, ctx: egui::Context) {
|
||||
let answer = open(dialog);
|
||||
let spawned = std::thread::Builder::new().name("ec-file-dialog".into()).spawn(move || {
|
||||
done(pollster::block_on(answer));
|
||||
ctx.request_repaint();
|
||||
});
|
||||
if let Err(e) = spawned {
|
||||
log::error!("file dialog: {e}");
|
||||
}
|
||||
}
|
||||
|
||||
fn paths(files: Option<Vec<FileHandle>>) -> Vec<String> {
|
||||
files.unwrap_or_default().into_iter().map(|f| f.path().to_string_lossy().to_string()).collect()
|
||||
}
|
||||
|
||||
fn path(file: Option<FileHandle>) -> Vec<String> {
|
||||
paths(file.map(|f| vec![f]))
|
||||
}
|
||||
|
||||
/// The dialog, as the blocking hooks in `main.rs` show it.
|
||||
fn open(dialog: FileDialog) -> Answer {
|
||||
match dialog {
|
||||
FileDialog::Open { exts, many } => {
|
||||
let d = AsyncFileDialog::new().add_filter("Media", &exts);
|
||||
if many {
|
||||
let f = d.pick_files();
|
||||
Box::pin(async move { paths(f.await) })
|
||||
} else {
|
||||
let f = d.pick_file();
|
||||
Box::pin(async move { path(f.await) })
|
||||
}
|
||||
}
|
||||
FileDialog::OpenProject => {
|
||||
let f = AsyncFileDialog::new().add_filter("EffectCraft Project", &["ecproj", "ecprojx"]).pick_file();
|
||||
Box::pin(async move { path(f.await) })
|
||||
}
|
||||
FileDialog::Folder => {
|
||||
let f = AsyncFileDialog::new().pick_folder();
|
||||
Box::pin(async move { path(f.await) })
|
||||
}
|
||||
FileDialog::Save { default, ext, project } => {
|
||||
let at = std::path::Path::new(&default);
|
||||
let name = at.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
|
||||
let mut d = if project {
|
||||
AsyncFileDialog::new().add_filter("EffectCraft Project", &["ecproj"])
|
||||
} else if ext.is_empty() {
|
||||
AsyncFileDialog::new()
|
||||
} else {
|
||||
AsyncFileDialog::new().add_filter(&ext, &[ext.as_str()])
|
||||
};
|
||||
// A default with a folder that exists opens the dialog there.
|
||||
if let Some(dir) = at.parent().filter(|d| d.is_dir()) {
|
||||
d = d.set_directory(dir);
|
||||
}
|
||||
let f = d.set_file_name(name).save_file();
|
||||
Box::pin(async move { path(f.await) })
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -98,6 +98,39 @@ pub enum Dialog {
|
||||
/// winit no theme). `None` = the host does not know.
|
||||
pub type SystemThemeFn = Box<dyn Fn(&egui::Context) -> Option<egui::Theme>>;
|
||||
|
||||
/// A file dialog a host shows without blocking the frame ([`Hooks::file_dialog_async`]).
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub enum FileDialog {
|
||||
/// Files to open or import with these extensions; several when `many`.
|
||||
Open { exts: Vec<String>, many: bool },
|
||||
/// An EffectCraft project to open.
|
||||
OpenProject,
|
||||
/// A folder.
|
||||
Folder,
|
||||
/// Where to save `default` (a file name or path) with extension `ext` (empty: no filter);
|
||||
/// `project`: the project save dialog, given the file name.
|
||||
Save { default: String, ext: String, project: bool },
|
||||
}
|
||||
|
||||
impl FileDialog {
|
||||
/// The save dialog for `default`, as [`Hooks::save_dialog`] picks it.
|
||||
pub fn save(default: &str) -> Self {
|
||||
let ext = std::path::Path::new(default).extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default();
|
||||
Self::Save { default: default.to_string(), project: ext.eq_ignore_ascii_case("ecproj"), ext }
|
||||
}
|
||||
}
|
||||
|
||||
/// Called with the paths a [`FileDialog`] picked (empty: cancelled), from any thread.
|
||||
pub type DialogDone = Box<dyn FnOnce(Vec<String>) + Send>;
|
||||
|
||||
/// A non-blocking file dialog's answer for the command that asked (`menus::dialog_answered`).
|
||||
pub(crate) struct DialogAnswer {
|
||||
pub command: String,
|
||||
pub key: &'static str,
|
||||
pub params: serde_json::Value,
|
||||
pub paths: Vec<String>,
|
||||
}
|
||||
|
||||
/// Host hooks provided by the native app (file pickers etc.).
|
||||
#[derive(Default)]
|
||||
pub struct Hooks {
|
||||
@@ -121,6 +154,11 @@ pub struct Hooks {
|
||||
/// Application actions the OS performs (`app.hide`, `app.hideOthers`, `app.showAll` on
|
||||
/// macOS). Returns false when the host doesn't handle the id.
|
||||
pub app_action: Option<Box<dyn Fn(&str) -> bool>>,
|
||||
/// Shows a command's file dialog without blocking the frame and calls `done` with the
|
||||
/// answer (the host then asks for a repaint). Used in place of the blocking pickers above for
|
||||
/// 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)>>,
|
||||
}
|
||||
|
||||
impl Hooks {
|
||||
@@ -264,6 +302,8 @@ pub struct EffectcraftApp {
|
||||
/// The graphics device was lost and the window is closing (see `close_after_device_loss`);
|
||||
/// `true` keeps crash recovery for the next launch.
|
||||
device_lost_close: Option<bool>,
|
||||
/// Answers of file dialogs shown without blocking ([`Hooks::file_dialog_async`]).
|
||||
pub(crate) dialog_answers: (Sender<DialogAnswer>, Receiver<DialogAnswer>),
|
||||
/// Commands from the native menu bar.
|
||||
pub command_inbox: Option<Receiver<String>>,
|
||||
/// Pointer position in comp pixels (Info panel).
|
||||
@@ -351,6 +391,7 @@ impl EffectcraftApp {
|
||||
gpu_failures: None,
|
||||
gpu_failure_applied: None,
|
||||
device_lost_close: None,
|
||||
dialog_answers: std::sync::mpsc::channel(),
|
||||
command_inbox: None,
|
||||
pointer_comp: None,
|
||||
dialog_state: Default::default(),
|
||||
@@ -667,6 +708,13 @@ impl EffectcraftApp {
|
||||
ctx.send_viewport_cmd(egui::ViewportCommand::Close);
|
||||
}
|
||||
|
||||
/// Run the commands whose non-blocking file dialogs have been answered.
|
||||
pub(crate) fn run_dialog_answers(&mut self) {
|
||||
while let Ok(answer) = self.dialog_answers.1.try_recv() {
|
||||
menus::dialog_answered(self, answer);
|
||||
}
|
||||
}
|
||||
|
||||
/// Set up the GPU compositor on egui-wgpu's device (once). Without an adapter that runs
|
||||
/// compute shaders (WebGL2, software GL) everything stays on the CPU.
|
||||
pub(crate) fn init_gpu(&mut self, frame: &eframe::Frame) {
|
||||
@@ -1507,6 +1555,7 @@ impl EffectcraftApp {
|
||||
fn frame(&mut self, ui: &mut egui::Ui) {
|
||||
let ctx = ui.ctx().clone();
|
||||
self.apply_gpu_failure(&ctx);
|
||||
self.run_dialog_answers();
|
||||
self.auto.begin_frame();
|
||||
self.frames.set_context(&ctx);
|
||||
if self.session.render_job.is_some() {
|
||||
|
||||
+87
-12
@@ -1200,7 +1200,31 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let mut p = params.as_object().cloned().unwrap_or_default();
|
||||
const MEDIA: &[&str] = &[
|
||||
"mp4", "mov", "m4v", "mkv", "webm", "png", "jpg", "jpeg", "gif", "webp", "tif", "tiff", "bmp", "exr", "wav", "aif", "aiff", "mp3", "flac", "ogg",
|
||||
"opus", "svg", "pdf", "ai", "eps", "psd", "psb", "gltf", "glb", "obj", "json", "csv", "tsv",
|
||||
];
|
||||
// A host whose modal dialogs would run a nested event loop inside this frame (macOS: a
|
||||
// drag over the window then re-entered winit's event handler and aborted, #511) shows the
|
||||
// dialog without blocking; the command continues when the answer arrives.
|
||||
if let Some(spawn) = app.hooks.file_dialog_async.as_ref() {
|
||||
let request = match ask {
|
||||
Ask::Folder => crate::FileDialog::Folder,
|
||||
Ask::Import => crate::FileDialog::Open { exts: MEDIA.iter().map(|e| e.to_string()).collect(), many: key == "paths" },
|
||||
Ask::OpenProject => crate::FileDialog::OpenProject,
|
||||
Ask::Open(exts) => crate::FileDialog::Open { exts: exts.iter().map(|e| e.to_string()).collect(), many: false },
|
||||
Ask::Save(default) => crate::FileDialog::save(default),
|
||||
};
|
||||
let tx = app.dialog_answers.0.clone();
|
||||
let answer = crate::DialogAnswer { command: id.to_string(), key, params: params.clone(), paths: Vec::new() };
|
||||
spawn(
|
||||
request,
|
||||
Box::new(move |paths| {
|
||||
let _ = tx.send(crate::DialogAnswer { paths, ..answer });
|
||||
}),
|
||||
);
|
||||
return Some(Ok(json!({"dialog": id})));
|
||||
}
|
||||
let picked: Option<Value> = match ask {
|
||||
Ask::Folder => {
|
||||
let Some(f) = app.hooks.pick_folder.as_ref() else { return Some(Err("no folder dialog available (pass `folder`)".into())) };
|
||||
@@ -1208,15 +1232,7 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
}
|
||||
Ask::Import => {
|
||||
let Some(f) = app.hooks.pick_files.as_ref() else { return Some(Err("no file dialog available (pass `paths`)".into())) };
|
||||
let paths = f(&[
|
||||
"mp4", "mov", "m4v", "mkv", "webm", "png", "jpg", "jpeg", "gif", "webp", "tif", "tiff", "bmp", "exr", "wav", "aif", "aiff", "mp3", "flac",
|
||||
"ogg", "opus", "svg", "pdf", "ai", "eps", "psd", "psb", "gltf", "glb", "obj", "json", "csv", "tsv",
|
||||
]);
|
||||
match (paths.is_empty(), key) {
|
||||
(true, _) => None,
|
||||
(false, "paths") => Some(json!(paths)),
|
||||
(false, _) => paths.first().map(|s| json!(s)),
|
||||
}
|
||||
picked_value(key, f(MEDIA))
|
||||
}
|
||||
Ask::OpenProject => {
|
||||
let Some(f) = app.hooks.pick_open_project.as_ref() else { return Some(Err("no file dialog available (pass `path`)".into())) };
|
||||
@@ -1232,6 +1248,30 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
}
|
||||
};
|
||||
let Some(v) = picked else { return Some(Ok(Value::Null)) };
|
||||
Some(run_with_picked(app, id, key, params, v))
|
||||
}
|
||||
|
||||
/// The dialog's answer as the command's `key` parameter (`None`: cancelled).
|
||||
fn picked_value(key: &str, paths: Vec<String>) -> Option<Value> {
|
||||
match (paths.is_empty(), key) {
|
||||
(true, _) => None,
|
||||
(false, "paths") => Some(json!(paths)),
|
||||
(false, _) => paths.into_iter().next().map(Value::from),
|
||||
}
|
||||
}
|
||||
|
||||
/// A file dialog shown without blocking ([`crate::Hooks::file_dialog_async`]) answered: run its
|
||||
/// command with the picked path (nothing when it was cancelled).
|
||||
pub(crate) fn dialog_answered(app: &mut EffectcraftApp, answer: crate::DialogAnswer) {
|
||||
let Some(v) = picked_value(answer.key, answer.paths) else { return };
|
||||
if let Err(e) = run_with_picked(app, &answer.command, answer.key, &answer.params, v) {
|
||||
app.ui.status = e;
|
||||
}
|
||||
}
|
||||
|
||||
/// Run command `id` with the path picked for it in parameter `key`.
|
||||
fn run_with_picked(app: &mut EffectcraftApp, id: &str, key: &str, params: &Value, v: Value) -> Result<Value, String> {
|
||||
let mut p = params.as_object().cloned().unwrap_or_default();
|
||||
p.insert(key.to_string(), v);
|
||||
// Picked files import in the background, with the Importing card showing progress (#270).
|
||||
if matches!(id, "file.import" | "file.importMultiple") {
|
||||
@@ -1243,7 +1283,7 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
if id == "file.import" && crate::panels::dialogs::open_form(app, id, &picked)
|
||||
|| matches!(id, "file.import" | "file.importMultiple") && crate::panels::forms::open_import_sequence(app, &picked)
|
||||
{
|
||||
return Some(Ok(json!({"dialog": id})));
|
||||
return Ok(json!({"dialog": id}));
|
||||
}
|
||||
// Save (untitled) becomes Save As.
|
||||
let id = if id == "file.save" { "file.saveAs" } else { id };
|
||||
@@ -1251,7 +1291,7 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
if let Err(e) = &r {
|
||||
app.ui.status = e.clone();
|
||||
}
|
||||
Some(r)
|
||||
r
|
||||
}
|
||||
|
||||
/// A menu entry for display / `ui.menu.list`.
|
||||
@@ -1981,6 +2021,41 @@ mod tests {
|
||||
assert_eq!(*asked.borrow(), ["project Frame.png"]);
|
||||
}
|
||||
|
||||
/// #511: with a host's non-blocking dialogs, a command's file dialog opens without running
|
||||
/// a nested event loop inside the frame (a drag over the window then aborted the app on
|
||||
/// macOS); the command runs on a later frame with the answer, and a cancelled one not at all.
|
||||
#[test]
|
||||
fn file_dialogs_can_answer_on_a_later_frame() {
|
||||
use std::sync::{Arc, Mutex};
|
||||
let mut app = EffectcraftApp::new(effectcraft_engine::Session::default());
|
||||
app.session.execute("comp.new", json!({"name": "A", "width": 64, "height": 36, "frameRate": 30, "duration": 1})).unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
app.hooks.pick_save = Some(Box::new(|_: &str| panic!("the blocking dialog is not used")));
|
||||
let pending: Arc<Mutex<Vec<(crate::FileDialog, crate::DialogDone)>>> = Arc::default();
|
||||
let queue = pending.clone();
|
||||
app.hooks.file_dialog_async = Some(Box::new(move |dialog, done| queue.lock().unwrap().push((dialog, done))));
|
||||
assert_eq!(invoke(&mut app, &ctx, "file.saveAs", json!({})).unwrap(), json!({"dialog": "file.saveAs"}));
|
||||
let (dialog, done) = pending.lock().unwrap().pop().expect("a dialog was shown");
|
||||
assert_eq!(dialog, crate::FileDialog::Save { default: "Untitled Project.ecproj".into(), ext: "ecproj".into(), project: true });
|
||||
assert!(app.session.path.is_none(), "nothing saved until the dialog answers");
|
||||
let dir = std::env::temp_dir().join(format!("ec-dialog-answer-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let path = dir.join("Answered.ecproj");
|
||||
// Answered from another thread, as the host's dialog does.
|
||||
std::thread::spawn(move || done(vec![path.to_string_lossy().to_string()])).join().unwrap();
|
||||
app.run_dialog_answers();
|
||||
assert_eq!(app.session.path.as_deref(), Some(dir.join("Answered.ecproj").to_string_lossy().as_ref()));
|
||||
assert!(dir.join("Answered.ecproj").is_file());
|
||||
// Cancelled: nothing runs.
|
||||
invoke(&mut app, &ctx, "comp.saveFrameAs", json!({})).unwrap();
|
||||
let (dialog, done) = pending.lock().unwrap().pop().expect("a dialog was shown");
|
||||
assert_eq!(dialog, crate::FileDialog::Save { default: "Frame.png".into(), ext: "png".into(), project: false });
|
||||
done(Vec::new());
|
||||
app.run_dialog_answers();
|
||||
assert!(!dir.join("Frame.png").exists());
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn menu_items_cover_the_tree() {
|
||||
assert_eq!(menus().last(), Some(&"Help"));
|
||||
|
||||
Reference in new issue
Block a user