From 86c08aa0eee7eba3d754bdb676d5e841a629eeb1 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Sun, 8 Nov 2026 16:43:06 +0100 Subject: [PATCH] Keep native control renders responsive (#3306) * Keep native control renders responsive * Refresh replies for immediately completed render jobs * #3289: Bind waiting replies to their native render --- crates/engine/src/cmd/render_queue.rs | 6 +- crates/engine/src/render_queue.rs | 63 ++++- crates/engine/src/scriptui.rs | 22 ++ crates/engine/src/session/dispatch.rs | 6 + crates/engine/src/session/edit.rs | 12 + crates/script/src/lib.rs | 1 + crates/script/src/runtime.rs | 26 +- crates/script/src/tasks.rs | 4 +- crates/script/src/tests.rs | 2 + crates/script/src/tests/render_wait.rs | 243 ++++++++++++++++++ crates/script/src/ui.rs | 99 +++++++- crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/app/frame.rs | 94 ++++++- crates/ui-egui/src/control.rs | 105 +++++++- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/menus/invoke.rs | 1 + crates/ui-egui/tests/it/main.rs | 2 + crates/ui-egui/tests/it/ui_script_render.rs | 260 ++++++++++++++++++++ crates/ui-egui/tests/it/ui_web_offload.rs | 11 +- docs/control-protocol.md | 20 ++ 20 files changed, 950 insertions(+), 29 deletions(-) create mode 100644 crates/script/src/tests/render_wait.rs create mode 100644 crates/ui-egui/tests/it/ui_script_render.rs diff --git a/crates/engine/src/cmd/render_queue.rs b/crates/engine/src/cmd/render_queue.rs index 8c457ec77..6ca5b9ba0 100644 --- a/crates/engine/src/cmd/render_queue.rs +++ b/crates/engine/src/cmd/render_queue.rs @@ -734,7 +734,11 @@ fn stop(s: &mut Session, _: &Value) -> Result { } fn list(s: &mut Session, _: &Value) -> Result { - s.poll_render(); + // The suspended script's frontend applies post-render edits before resuming it. A + // control inspection must not apply those edits while the undo-group guard is active. + if s.script_ui.render_host.is_none() { + s.poll_render(); + } let items: Vec = s.project.render_queue.iter().enumerate().map(|(k, i)| item_json(s, i, k)).collect(); Ok(json!({"items": items, "rendering": s.is_rendering(), "progress": s.render_progress()})) } diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index efe9e6678..cc141f3fd 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -14,6 +14,7 @@ use effectcraft_project::render_queue::{OutputFormat, OutputModule, PostRenderAc use effectcraft_project::{ItemId, ItemKind, LayerSource, Project}; use effectcraft_render::{ExprHost, FootageSource}; use serde::{Deserialize, Serialize}; +use serde_json::{Value, json}; use crate::{Event, Session}; @@ -113,6 +114,14 @@ pub struct JobShared { pub cancel: AtomicBool, pub state: Mutex, pub updates: Mutex>, + /// A waiting control request retains its own items across requeue/removal/project edits. + reply: Mutex>, +} + +#[derive(Default)] +struct RenderReply { + items: Vec, + finished: bool, } pub(crate) fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { @@ -120,6 +129,48 @@ pub(crate) fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { } impl JobShared { + /// Retain the requesting command's items before the UI can edit or replace the queue. + pub fn retain_reply_items(&self, items: Vec) { + *lock(&self.reply) = Some(RenderReply { items, finished: false }); + } + + /// Whether completion and post-render actions have been applied to this run. + pub fn reply_finished(&self) -> bool { + lock(&self.reply).as_ref().is_some_and(|reply| reply.finished) + } + + /// Final items for this run, independent of the current project and any later render. + pub fn completed_items(&self) -> Option> { + lock(&self.reply).as_ref().filter(|reply| reply.finished).map(|reply| reply.items.clone()) + } + + fn update_reply(&self, updates: &[ItemUpdate]) { + let mut reply = lock(&self.reply); + let Some(reply) = reply.as_mut() else { return }; + for update in updates { + let id = match update { + ItemUpdate::Started { id, .. } | ItemUpdate::Finished { id, .. } => *id, + }; + let Some(item) = reply.items.iter_mut().find(|item| item["id"].as_u64() == Some(id)).and_then(Value::as_object_mut) else { continue }; + match update { + ItemUpdate::Started { unix, .. } => { + item.insert("status".into(), json!(RenderStatus::Rendering)); + item.insert("statusLabel".into(), json!(RenderStatus::Rendering.label())); + item.insert("started".into(), json!(unix)); + item.insert("render_time".into(), Value::Null); + } + ItemUpdate::Finished { status, seconds, output, .. } => { + item.insert("status".into(), json!(status)); + item.insert("statusLabel".into(), json!(status.label())); + item.insert("render_time".into(), json!(seconds)); + if let Some(output) = output { + item.insert("last_output".into(), json!(output)); + } + } + } + } + } + pub fn snapshot(&self) -> JobState { lock(&self.state).clone() } @@ -626,8 +677,11 @@ impl Session { pub fn poll_render(&mut self) -> bool { self.drain_remote_render(); let Some(job) = &self.render_job else { return false }; - let updates = std::mem::take(&mut *lock(&job.shared.updates)); + // Observe completion before draining updates so a worker finishing between the two + // reads cannot leave its final item statuses behind when this job is removed. let finished = job.is_finished(); + let updates = std::mem::take(&mut *lock(&job.shared.updates)); + job.shared.update_reply(&updates); let changed = !updates.is_empty(); let mut post: Vec<(u64, String)> = vec![]; if changed { @@ -659,6 +713,7 @@ impl Session { } self.bump(); } + let mut completed = None; if finished && let Some(mut job) = self.render_job.take() { if let Some(t) = job.thread.take() { let _ = t.join(); @@ -685,12 +740,18 @@ impl Session { self.events.push(Event::Frontend { command: "renderQueue.notify".into(), params: serde_json::json!({"message": msg, "error": failed > 0}) }); } self.events.push(Event::Toast { message: msg, error: failed > 0 }); + completed = Some(job.shared); } for (id, path) in post { if let Err(e) = self.post_render_action(id, &path) { self.events.push(Event::Toast { message: format!("Post-render action: {e}"), error: true }); } } + if let Some(shared) = completed + && let Some(reply) = lock(&shared.reply).as_mut() + { + reply.finished = true; + } changed || finished } diff --git a/crates/engine/src/scriptui.rs b/crates/engine/src/scriptui.rs index 2c211e77a..6d36a2e7e 100644 --- a/crates/engine/src/scriptui.rs +++ b/crates/engine/src/scriptui.rs @@ -388,6 +388,18 @@ pub type ScriptUiDispatch = fn(&mut Session, &ScriptUiEvent) -> std::result::Res /// and updates [`Session::script_ui`]. Frontends call it every frame (see [`ScriptUi::run_tasks`]). pub type ScriptTaskRunner = fn(&mut Session) -> Value; +/// Resume a suspended script once its Render Queue worker has finished. `None` means the +/// script is still rendering; `Some` is its eventual result (or a real modal-dialog yield). +pub type ScriptRenderPoll = fn(&mut Session) -> Option; + +pub const RENDER_WAIT_MESSAGE: &str = "A script is rendering. Stop the render or wait for it to finish before editing."; + +/// These inspections do not run callbacks or collect unrelated background edits. In +/// particular, `journal: false` is not a read-only guarantee (`engine.batch`, `jobs.wait`). +pub fn allowed_during_render(command: &str) -> bool { + matches!(command, "renderQueue.stop" | "renderQueue.list" | "editor.state" | "app.capabilities" | "command.list" | "scriptui.list" | "scriptui.get") +} + /// The open script windows. #[derive(Clone, Debug, Default, Serialize, Deserialize)] #[serde(default)] @@ -401,6 +413,16 @@ pub struct ScriptUi { /// The scripting engine's scheduled-task runner (set with [`Session::script`]). #[serde(skip)] pub tasks: Option, + /// A frontend that services [`Self::poll_render`] may opt a script invocation into yielding + /// during a blocking Render Queue call. Headless and lifecycle scripts remain synchronous. + #[serde(skip)] + pub yield_renders: bool, + /// The script host suspended in a Render Queue call. While set, frontends must not allow + /// other edits or script callbacks to interleave with its open undo groups. + #[serde(skip)] + pub render_host: Option, + #[serde(skip)] + pub poll_render: Option, /// Bumped whenever the project is closed or replaced (New / Open / Close Project): repeating /// tasks of scripts without an open window stop then, except those of Startup scripts. #[serde(skip)] diff --git a/crates/engine/src/session/dispatch.rs b/crates/engine/src/session/dispatch.rs index 36c7f0a51..aa83ac09f 100644 --- a/crates/engine/src/session/dispatch.rs +++ b/crates/engine/src/session/dispatch.rs @@ -7,6 +7,9 @@ impl Session { /// Run a command by id. pub fn execute(&mut self, id: &str, params: Value) -> Result { + if self.script_ui.render_host.is_some() && !crate::scriptui::allowed_during_render(id) { + return Err(EngineError::Other(crate::scriptui::RENDER_WAIT_MESSAGE.into())); + } let spec = commands::find(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?; if let Err(why) = (spec.enabled)(self) { // Explicit targets (agents, scripts) don't need a UI selection. @@ -57,6 +60,9 @@ impl Session { /// Whether `id` names a known command whose `enabled` predicate returns `Ok` for this session; /// an unknown id or a predicate error reads as disabled. pub fn is_enabled(&self, id: &str) -> bool { + if self.script_ui.render_host.is_some() && !crate::scriptui::allowed_during_render(id) { + return false; + } commands::find(id).is_some_and(|c| (c.enabled)(self).is_ok()) } diff --git a/crates/engine/src/session/edit.rs b/crates/engine/src/session/edit.rs index 67a8e4dcb..c35a5a13b 100644 --- a/crates/engine/src/session/edit.rs +++ b/crates/engine/src/session/edit.rs @@ -30,6 +30,9 @@ impl Session { /// undo step under `label`, unless it merges with the current merge key (then the redo branch /// is dropped). An `Err` from `f` propagates with the session unchanged. fn edit_inner(&mut self, label: &str, merge: Option<&str>, f: impl FnOnce(&mut Project, &mut EditorState) -> Result) -> Result { + if self.script_ui.render_host.is_some() { + return Err(EngineError::Other(crate::scriptui::RENDER_WAIT_MESSAGE.into())); + } let before = self.project.clone(); let mut p = (*self.project).clone(); let mut st = self.state.clone(); @@ -105,6 +108,9 @@ impl Session { /// modified) but is no undo step, and it changes no pixels: the comp's cached frames stay /// valid ([`effectcraft_project::Comp::same_pixels`]). pub fn set_resolution(&mut self, comp: ItemId, r: effectcraft_project::Resolution) -> Result<()> { + if self.script_ui.render_host.is_some() { + return Err(EngineError::Other(crate::scriptui::RENDER_WAIT_MESSAGE.into())); + } if self.project.comp(comp).ok_or(EngineError::NoComp)?.resolution == r { return Ok(()); } @@ -134,6 +140,9 @@ impl Session { /// on-canvas editor gets a new IME epoch only when its own document changed) and the session is /// bumped. Returns `false`, changing nothing, when there is nothing to undo. pub fn undo(&mut self) -> bool { + if self.script_ui.render_host.is_some() { + return false; + } let Some((label, p)) = self.history.undo.pop() else { return false }; let edited = commands::text_edit::active_editor_snapshot(self); let probe = commands::kept_crypto_probe(self); @@ -152,6 +161,9 @@ impl Session { /// text document starts a new IME epoch) and the session is bumped. Returns `false`, changing /// nothing, when there is nothing to redo. pub fn redo(&mut self) -> bool { + if self.script_ui.render_host.is_some() { + return false; + } let Some((label, p)) = self.history.redo.pop() else { return false }; let edited = commands::text_edit::active_editor_snapshot(self); let probe = commands::kept_crypto_probe(self); diff --git a/crates/script/src/lib.rs b/crates/script/src/lib.rs index 11d837677..21774cb24 100644 --- a/crates/script/src/lib.rs +++ b/crates/script/src/lib.rs @@ -79,6 +79,7 @@ pub fn install(s: &mut Session) { s.script = Some(runner); s.script_ui.dispatch = Some(dispatch_ui); s.script_ui.tasks = Some(run_tasks); + s.script_ui.poll_render = Some(ui::poll_render_wait); } /// Run `code` as a one-off script named `name`. diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index 2c18039e8..c23eaefb0 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -30,6 +30,8 @@ pub struct Outcome { /// The script is waiting in a modal ScriptUI dialog's `show()` (it goes on when the dialog /// closes; see [`crate::dispatch_ui`]). pub waiting: bool, + /// The native frontend owns the session while this host waits for its render worker. + pub render_host: Option, } /// An uncaught script error. @@ -55,6 +57,9 @@ impl Outcome { if self.waiting { j["waiting"] = json!(true); } + if let Some(host) = self.render_host { + j["renderHost"] = json!(host); + } j } } @@ -74,13 +79,16 @@ pub(crate) struct Active { /// Running on a script-host thread: modal dialogs hand the session back to the caller /// through this link while they wait. pub(crate) link: Option, + /// Captured at invocation start: the frontend resets its opt-in when the caller returns. + pub(crate) yield_renders: bool, /// The files `$.evalFile` is evaluating, outermost first. pub(crate) loading: Vec, } impl Active { pub(crate) fn new(session: Session, name: &str, host: u32) -> Active { - Active { session, output: vec![], groups: vec![], name: name.to_string(), host, link: None, loading: vec![] } + let yield_renders = session.script_ui.yield_renders; + Active { session, output: vec![], groups: vec![], name: name.to_string(), host, link: None, yield_renders, loading: vec![] } } } @@ -130,6 +138,13 @@ fn native_exec(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult exec_on_keys(&mut a.session, &id, params, k), @@ -137,7 +152,10 @@ fn native_exec(_: &JsValue, args: &[JsValue], ctx: &mut Context) -> JsResult Ok(js_str(&v.to_string())), + Ok(Ok(v)) => { + let v = if yield_render { crate::ui::wait_for_render(ctx, v).map_err(throw)? } else { v }; + Ok(js_str(&v.to_string())) + } Ok(Err(e)) | Err(e) => Err(throw(e)), } } @@ -718,6 +736,10 @@ pub fn run(session: &mut Session, req: &ScriptRequest) -> Outcome { }; let Some(x) = expanded else { return run_expanded(session, req) }; let mut out = run_expanded(session, &ScriptRequest { code: &x.code, name: req.name, console: req.console }); + if let Some(host) = out.render_host { + crate::ui::keep_render_source_map(host, x); + return out; + } if let Some(e) = out.error.as_mut() { x.locate(e); } diff --git a/crates/script/src/tasks.rs b/crates/script/src/tasks.rs index 563f3bbc0..8b5c0cd7a 100644 --- a/crates/script/src/tasks.rs +++ b/crates/script/src/tasks.rs @@ -208,7 +208,9 @@ pub(crate) fn run_tasks_at(session: &mut Session, now: Instant) -> J { if !running { #[cfg(not(target_arch = "wasm32"))] crate::ui::tick_threaded(session, now, &mut report); - crate::ui::tick_inline(session, now, &mut report); + if session.script_ui.render_host.is_none() { + crate::ui::tick_inline(session, now, &mut report); + } crate::ui::prune_task_hosts(session); } let errors: Vec = report.errors.iter().map(|e| json!({"message": e.message, "line": e.line, "column": e.column, "file": e.file})).collect(); diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 9e934b555..78a16e5d3 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -15,6 +15,8 @@ mod ease_presets; mod evalfile; mod include; mod panels; +#[cfg(not(target_arch = "wasm32"))] +mod render_wait; #[cfg(not(windows))] mod services; mod startup; diff --git a/crates/script/src/tests/render_wait.rs b/crates/script/src/tests/render_wait.rs new file mode 100644 index 000000000..591bd211a --- /dev/null +++ b/crates/script/src/tests/render_wait.rs @@ -0,0 +1,243 @@ +//! #3289: a native frontend may service the event loop while a script waits for rendering. + +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::{Duration, Instant}; + +use effectcraft_engine::project::render_queue::OutputFormat; +use effectcraft_engine::{ExportJob, ExportResult, Exporter, Session}; +use serde_json::{Value, json}; + +/// A slow exporter without expensive frames. A bounded wait makes the old blocking path +/// fail the assertions instead of hanging the test process. +struct Gate { + entered: AtomicUsize, + permits: AtomicUsize, + failed: bool, +} + +impl Exporter for Gate { + fn formats(&self) -> Vec { + vec![OutputFormat::H264] + } + fn export(&self, job: &ExportJob, progress: &mut dyn FnMut(u64, u64) -> bool) -> Result { + let call = self.entered.fetch_add(1, Ordering::SeqCst) + 1; + let end = Instant::now() + Duration::from_secs(2); + while self.permits.load(Ordering::SeqCst) < call { + if !progress(0, 1) { + return Err(effectcraft_engine::render_queue::CANCELLED.into()); + } + if Instant::now() > end { + return Err("test exporter timed out waiting for the frontend".into()); + } + std::thread::sleep(Duration::from_millis(1)); + } + if self.failed { Err("test encoder failure".into()) } else { Ok(ExportResult { path: job.path.into(), frames: 1, ..Default::default() }) } + } +} + +fn session(permits: usize, failed: bool) -> (Session, Arc) { + let gate = Arc::new(Gate { entered: AtomicUsize::new(0), permits: AtomicUsize::new(permits), failed }); + let mut s = super::session(); + s.exporter = Some(gate.clone()); + s.execute("comp.new", json!({"name": "Render", "width": 16, "height": 16, "duration": 1})).unwrap(); + let path = std::env::temp_dir().join(format!("ec-script-render-{}.mp4", std::process::id())); + s.execute("renderQueue.add", json!({"output": path.to_string_lossy()})).unwrap(); + s.prefs.auto_save.enabled = false; + (s, gate) +} + +fn finish(s: &mut Session) -> Value { + let end = Instant::now() + Duration::from_secs(3); + loop { + if let Some(result) = crate::ui::poll_render_wait(s) { + return result; + } + assert!(Instant::now() < end, "script did not resume"); + std::thread::sleep(Duration::from_millis(1)); + } +} + +#[test] +fn render_yields_without_finishing_the_script_or_its_undo_group() { + let (mut s, gate) = session(0, false); + s.script_ui.yield_renders = true; + let before = s.history.undo.len(); + let out = crate::run_code( + &mut s, + "app.beginUndoGroup('Render transaction'); app.project.activeItem.layers.addNull().name = 'Before'; $.writeln('before'); app.project.renderQueue.render(); app.project.activeItem.layers.addNull().name = 'After'; $.writeln('after'); app.endUndoGroup(); [42, app.project.renderQueue.rendering];", + "render.jsx", + ); + assert!(out.error.is_none(), "{out:?}"); + assert!(out.render_host.is_some(), "blocking render returned instead of yielding: {out:?}"); + assert!(!out.waiting, "a render suspension is not a modal dialog"); + assert!(out.output.is_empty(), "output belongs to the eventual reply"); + assert_eq!(s.active_comp().unwrap().layers.len(), 1, "the continuation has not run"); + assert!(crate::ui::poll_render_wait(&mut s).is_none()); + assert!(!s.undo(), "undo cannot discard the suspended script's live edits"); + assert!(s.execute("engine.batch", json!({"steps": [{"command": "layer.newNull"}]})).is_err()); + assert!(s.execute("jobs.list", json!({})).is_err(), "status polling must not apply unrelated jobs"); + // The frontend resets the invocation-only opt-in after script.run returns. + s.script_ui.yield_renders = false; + gate.permits.store(1, Ordering::SeqCst); + let done = finish(&mut s); + assert_eq!(done["ok"], true, "{done}"); + assert_eq!(done["result"], json!([42, false])); + assert_eq!(done["output"], "before\nafter"); + assert_eq!(s.project.render_queue[0].status.label(), "Done"); + assert_eq!(s.history.undo.len(), before + 1, "one undo group spans the render"); + assert_eq!(s.history.undo.last().unwrap().0, "Render transaction"); + assert!(s.script_ui.render_host.is_none()); + s.undo(); + assert!(s.active_comp().unwrap().layers.is_empty()); +} + +#[test] +fn a_second_render_keeps_the_same_script_pending() { + let (mut s, gate) = session(0, false); + s.script_ui.yield_renders = true; + let out = crate::run_code( + &mut s, + "app.project.renderQueue.render(); app.project.renderQueue.item(1).render = true; app.project.renderQueue.render(); 17;", + "twice.jsx", + ); + let host = out.render_host.unwrap(); + s.script_ui.yield_renders = false; + gate.permits.store(1, Ordering::SeqCst); + let end = Instant::now() + Duration::from_secs(3); + while gate.entered.load(Ordering::SeqCst) < 2 { + assert!(crate::ui::poll_render_wait(&mut s).is_none(), "no premature reply after the first render"); + assert!(Instant::now() < end); + std::thread::sleep(Duration::from_millis(1)); + } + assert_eq!(s.script_ui.render_host, Some(host)); + gate.permits.store(2, Ordering::SeqCst); + assert_eq!(finish(&mut s)["result"], 17); +} + +#[test] +fn cancellation_and_encoder_failure_resume_with_terminal_statuses() { + for failed in [false, true] { + let (mut s, gate) = session(0, failed); + s.script_ui.yield_renders = true; + let out = crate::run_code(&mut s, "app.project.renderQueue.render(); [9, app.project.renderQueue.rendering];", "stopped.jsx"); + assert!(out.render_host.is_some()); + if failed { + gate.permits.store(1, Ordering::SeqCst); + } else { + s.stop_render(); + } + let done = finish(&mut s); + assert_eq!(done["ok"], true, "the synchronous command reports status, not a thrown script error: {done}"); + assert_eq!(done["result"], json!([9, false])); + assert_ne!(s.project.render_queue[0].status.label(), "Done"); + } +} + +#[test] +fn headless_scripts_still_wait_and_start_errors_do_not_suspend() { + let (mut s, _) = session(1, false); + let out = crate::run_code(&mut s, "app.project.renderQueue.render(); [7, app.project.renderQueue.rendering];", "headless.jsx"); + assert!(out.error.is_none(), "{out:?}"); + assert_eq!(out.result, json!([7, false])); + assert!(out.render_host.is_none()); + assert_eq!(s.project.render_queue[0].status.label(), "Done"); + s.script_ui.yield_renders = true; + let out = crate::run_code(&mut s, "app.project.renderQueue.render();", "empty.jsx"); + assert!(out.error.is_some()); + assert!(out.render_host.is_none()); + assert!(s.script_ui.render_host.is_none()); +} + +#[test] +fn retained_palette_callbacks_can_yield_after_the_original_control_invocation() { + let (mut s, gate) = session(0, false); + s.script_ui.yield_renders = true; + let out = crate::run_code( + &mut s, + "var w = new Window('palette', 'Render'); var b = w.add('button', undefined, 'Go', {name:'go'}); b.onClick = function () { app.project.renderQueue.render(); $.writeln('resumed'); }; w.show();", + "palette-render.jsx", + ); + assert!(out.error.is_none(), "{out:?}"); + s.script_ui.yield_renders = false; + let result = s.execute("scriptui.click", json!({"widget": "go"})).unwrap(); + assert!(result["renderHost"].is_number(), "{result}"); + gate.permits.store(1, Ordering::SeqCst); + assert_eq!(finish(&mut s)["output"], "resumed"); + let window = s.script_ui.windows[0].id; + s.execute("scriptui.close", json!({"window": window})).unwrap(); +} + +#[test] +fn post_render_actions_complete_before_the_script_resumes() { + use effectcraft_engine::project::{Footage, FootageKind}; + struct Probe; + impl effectcraft_engine::Importer for Probe { + fn probe(&self, path: &str) -> Result { + Ok(Footage { path: path.into(), kind: FootageKind::Still, width: 16, height: 16, has_video: true, ..Default::default() }) + } + } + for action in ["import", "setProxy"] { + let (mut s, gate) = session(0, false); + s.importer = Some(Arc::new(Probe)); + let id = s.project.render_queue[0].id; + s.execute("renderQueue.setOutputModule", json!({"item": id, "postRenderAction": action})).unwrap(); + let before = s.project.items.len(); + let comp = s.active_comp_id().unwrap(); + s.script_ui.yield_renders = true; + let out = crate::run_code(&mut s, "app.project.renderQueue.render(); 29;", "post-render.jsx"); + assert!(out.render_host.is_some(), "{out:?}"); + gate.permits.store(1, Ordering::SeqCst); + // A query must not consume updates while the edit guard is still active. + let end = Instant::now() + Duration::from_secs(3); + while s.is_rendering() { + assert!(Instant::now() < end); + std::thread::sleep(Duration::from_millis(1)); + } + s.execute("renderQueue.list", json!({})).unwrap(); + assert_eq!(s.project.items.len(), before); + assert_eq!(finish(&mut s)["result"], 29); + if action == "import" { + assert_eq!(s.project.items.len(), before + 1); + } else { + assert!(s.project.item(comp).unwrap().proxy.is_some()); + } + } +} + +#[test] +fn deferred_errors_keep_their_included_source_location() { + let (mut s, gate) = session(0, false); + let dir = std::env::temp_dir().join(format!("ec-render-include-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("render.jsx"), "app.project.renderQueue.render();\nmissingAfterRender();").unwrap(); + std::fs::write(dir.join("main.jsx"), "#include 'render.jsx'").unwrap(); + s.script_ui.yield_renders = true; + let out = crate::run_code(&mut s, "#include 'render.jsx'", &dir.join("main.jsx").to_string_lossy()); + assert!(out.render_host.is_some(), "{out:?}"); + gate.permits.store(1, Ordering::SeqCst); + let done = finish(&mut s); + assert_eq!(done["ok"], false); + assert_eq!(done["error"]["line"], 2); + assert!(done["error"]["file"].as_str().unwrap().ends_with("render.jsx"), "{done}"); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn included_error_mapping_survives_a_modal_yield_after_rendering() { + let (mut s, gate) = session(0, false); + let dir = std::env::temp_dir().join(format!("ec-render-modal-include-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("render.jsx"), "app.project.renderQueue.render();\nvar d = new Window('dialog', 'Continue'); d.add('button', undefined, 'OK', {name:'ok'}); d.show();\nmissingAfterModal();").unwrap(); + std::fs::write(dir.join("main.jsx"), "#include 'render.jsx'").unwrap(); + s.script_ui.yield_renders = true; + let out = crate::run_code(&mut s, "#include 'render.jsx'", &dir.join("main.jsx").to_string_lossy()); + assert!(out.render_host.is_some(), "{out:?}"); + gate.permits.store(1, Ordering::SeqCst); + assert_eq!(finish(&mut s)["waiting"], true); + let done = s.execute("scriptui.click", json!({"widget": "ok"})).unwrap(); + assert_eq!(done["ok"], false); + assert_eq!(done["error"]["line"], 3); + assert!(done["error"]["file"].as_str().unwrap().ends_with("render.jsx"), "{done}"); + std::fs::remove_dir_all(dir).unwrap(); +} diff --git a/crates/script/src/ui.rs b/crates/script/src/ui.rs index ab6f69846..ecf4cd28b 100644 --- a/crates/script/src/ui.rs +++ b/crates/script/src/ui.rs @@ -3,7 +3,8 @@ //! //! * **Script files** (native builds) run on a *script-host thread*. The caller's session moves //! to that thread while JavaScript runs and comes back whenever the script stops: at the end, -//! or while a modal dialog waits in `show()`. Each [`ScriptUiEvent`] moves the session there +//! while a modal dialog waits in `show()`, or while an opted-in frontend waits for rendering. +//! Each [`ScriptUiEvent`] moves the session there //! again, runs the handler, and brings it back with the windows' new state. The host ends //! when its last window closes. Only one side holds the session at a time, so nothing runs //! concurrently and every edit is still an ordinary undoable engine command. @@ -137,6 +138,8 @@ pub(crate) enum ToHost { /// script-host threads). #[cfg_attr(target_arch = "wasm32", allow(dead_code))] Tick(Session, Instant), + /// Resume the suspended native render call, never a callback or scheduled task. + ResumeRender(Session), } impl ToHost { @@ -145,7 +148,7 @@ impl ToHost { match self { #[cfg(not(target_arch = "wasm32"))] ToHost::Start(s) => s, - ToHost::Event(s, _) | ToHost::Tick(s, _) => s, + ToHost::Event(s, _) | ToHost::Tick(s, _) | ToHost::ResumeRender(s) => s, } } } @@ -159,6 +162,7 @@ pub(crate) struct FromHost { done: bool, /// When its next scheduled task is due. next_due: Option, + render_wait: bool, } /// The host thread's end of the hand-off. @@ -177,6 +181,8 @@ struct HostEnd { /// [`effectcraft_engine::scriptui::ScriptUi::project_epoch`] when the host last ran. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] epoch: u64, + /// Retained across render yields so a later error still names its included source file. + source_map: Option, } fn hosts() -> &'static std::sync::Mutex> { @@ -195,6 +201,7 @@ enum Wake { Event(ScriptUiEvent), #[cfg_attr(target_arch = "wasm32", allow(dead_code))] Tick(Instant), + Render, } /// On the host thread: hand the session (with the windows' state and what the script printed) @@ -206,18 +213,20 @@ fn yield_wait(outcome: crate::Outcome, windows: Vec, done: bool) - match yield_wake(out, windows.clone(), done, None)? { Wake::Event(ev) => return Some(ev), Wake::Tick(_) => out = crate::Outcome { waiting: true, ..Default::default() }, + Wake::Render => return None, } } } /// [`yield_wait`] that also wakes for the scheduled tasks due at `next_due`. fn yield_wake(outcome: crate::Outcome, windows: Vec, done: bool, next_due: Option) -> Option { + let render_wait = outcome.render_host.is_some(); let (session, link, name) = ACTIVE.with(|a| { let mut a = a.borrow_mut(); let a = a.as_mut()?; Some((std::mem::take(&mut a.session), a.link.take()?, a.name.clone())) })?; - if link.tx.send(FromHost { session, outcome, windows, name, done, next_due }).is_err() { + if link.tx.send(FromHost { session, outcome, windows, name, done, next_due, render_wait }).is_err() { return None; } if done { @@ -226,6 +235,7 @@ fn yield_wake(outcome: crate::Outcome, windows: Vec, done: bool, n let (s, wake) = match link.rx.recv() { Ok(ToHost::Event(s, ev)) => (s, Wake::Event(ev)), Ok(ToHost::Tick(s, now)) => (s, Wake::Tick(now)), + Ok(ToHost::ResumeRender(s)) => (s, Wake::Render), _ => return None, }; ACTIVE.with(|a| { @@ -269,8 +279,9 @@ fn host_main(host: u32, name: String, code: String, link: Link) { } Some(Wake::Tick(now)) => { let (ran, error) = crate::tasks::run_due(&mut ctx, host, now); - out = crate::Outcome { result: json!(ran), output: settle(), error, waiting: false }; + out = crate::Outcome { result: json!(ran), output: settle(), error, ..Default::default() }; } + Some(Wake::Render) => break, None => break, } } @@ -283,15 +294,30 @@ fn host_main(host: u32, name: String, code: String, link: Link) { { let message = if body.is_err() { "the script engine stopped unexpectedly" } else { "the script ended" }; let out = crate::Outcome { error: Some(ScriptError { message: message.into(), file, ..Default::default() }), ..Default::default() }; - let _ = link.tx.send(FromHost { session: std::mem::take(&mut a.session), outcome: out, windows: vec![], name, done: true, next_due: None }); + let _ = link.tx.send(FromHost { + session: std::mem::take(&mut a.session), + outcome: out, + windows: vec![], + name, + done: true, + next_due: None, + render_wait: false, + }); } } /// Wait for the host to hand the session back; keep its link while it has windows open. fn receive(session: &mut Session, host: u32, end: HostEnd) -> crate::Outcome { match end.rx.recv() { - Ok(FromHost { session: s, outcome, windows, name, done, next_due }) => { + Ok(FromHost { session: s, mut outcome, windows, name, done, next_due, render_wait }) => { *session = s; + session.script_ui.render_host = render_wait.then_some(host); + if !render_wait + && let Some(map) = &end.source_map + && let Some(error) = &mut outcome.error + { + map.locate(error); + } publish(session, host, &name, windows); if done { session.script_ui.windows.retain(|w| w.host != host); @@ -324,7 +350,61 @@ pub(crate) fn run_threaded(session: &mut Session, req: &effectcraft_engine::Scri *session = msg.into_session(); return crate::Outcome { error: Some(ScriptError { message: "the script host did not start".into(), ..Default::default() }), ..Default::default() }; } - receive(session, host, HostEnd { tx: to_tx, rx: from_rx, next_due: None, epoch: 0 }) + receive(session, host, HostEnd { tx: to_tx, rx: from_rx, next_due: None, epoch: 0, source_map: None }) +} + +/// Suspend only the JavaScript stack, not the window's event loop (#3289). Keep the output +/// and open undo groups in `Active`: settling here would split the script's undo transaction. +pub(crate) fn wait_for_render(ctx: &mut Context, mut result: J) -> Result { + let host = with_active(|a| Ok(a.host))?; + let windows = snapshot(ctx); + let out = crate::Outcome { render_host: Some(host), ..Default::default() }; + if !matches!(yield_wake(out, windows, false, None), Some(Wake::Render)) { + return Err("the frontend stopped waiting for the script's render".into()); + } + with_active(|a| { + let list = a.session.execute_for_script("renderQueue.list", json!({})).map_err(|e| e.to_string())?; + let ids: Vec = result["items"].as_array().into_iter().flatten().map(|i| i["id"].clone()).collect(); + result["items"] = json!(list["items"].as_array().into_iter().flatten().filter(|i| ids.contains(&i["id"])).collect::>()); + result["rendering"] = list["rendering"].clone(); + Ok(result) + }) +} + +pub(crate) fn keep_render_source_map(host: u32, map: crate::include::Expanded) { + if let Ok(mut h) = hosts().lock() + && let Some(end) = h.get_mut(&host) + { + end.source_map = Some(map); + } +} + +/// Called by the native frontend while its normal editing/callback tick is paused. Polling +/// applies terminal statuses and post-render actions before the suspended JS call returns. +pub(crate) fn poll_render_wait(session: &mut Session) -> Option { + let host = session.script_ui.render_host?; + // Only this completion boundary may apply the render's own import/proxy undo steps. + session.script_ui.render_host = None; + session.poll_render(); + if session.is_rendering() { + session.script_ui.render_host = Some(host); + return None; + } + session.script_ui.render_host = None; + let fail = |message: &str| crate::Outcome { error: Some(ScriptError { message: message.into(), ..Default::default() }), ..Default::default() }.to_json(); + let Some(end) = hosts().lock().ok().and_then(|mut h| h.remove(&host)) else { + return Some(fail("the script host stopped while rendering")); + }; + if end.epoch != session.script_ui.project_epoch { + return Some(fail("the project changed while the script was rendering")); + } + if let Err(std::sync::mpsc::SendError(msg)) = end.tx.send(ToHost::ResumeRender(std::mem::take(session))) { + *session = msg.into_session(); + return Some(fail("the script host stopped while rendering")); + } + let out = receive(session, host, end); + // A second render in this invocation keeps the original control reply pending. + out.render_host.is_none().then(|| out.to_json()) } /// Record the windows a host published (named after the script that made them). @@ -429,6 +509,9 @@ pub(crate) fn tick_threaded(session: &mut Session, now: Instant, report: &mut cr } drop(stale); for host in due { + if session.script_ui.render_host.is_some() { + break; + } let Some(end) = hosts().lock().ok().and_then(|mut h| h.remove(&host)) else { continue }; if let Err(std::sync::mpsc::SendError(msg)) = end.tx.send(ToHost::Tick(std::mem::take(session), now)) { *session = msg.into_session(); @@ -490,7 +573,7 @@ fn tick_inline_host(session: &mut Session, host: u32, name: &str, now: Instant) drop(guard); crate::tasks::set_epoch(host, session.script_ui.project_epoch); publish(session, host, name, windows); - crate::Outcome { result: json!(ran), output, error, waiting: false } + crate::Outcome { result: json!(ran), output, error, ..Default::default() } } /// Forget the session's task hosts that have ended (inline ones: that have no tasks left). diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 9d32ba4cd..66387bc31 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -68,6 +68,7 @@ impl EffectcraftApp { forwarded: ForwardedInput::default(), input_waiters: vec![], job_waiters: vec![], + script_waiters: vec![], pending_screenshots: vec![], shot_bytes: 0, last_activity: (0.0, 0), diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index 8ebea08fc..c3a5bfbf6 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -47,6 +47,10 @@ impl EffectcraftApp { self.job_waiters.push((w, reply)); ctx.request_repaint(); } + control::Outcome::AwaitScript(host) => { + self.script_waiters.push((host, reply)); + ctx.request_repaint(); + } control::Outcome::Screenshot { path, target } => { // A covered or minimized window presents no frames (macOS skips its redraws), so // a screenshot would never arrive and nothing would tick its timeout. The @@ -70,11 +74,16 @@ impl EffectcraftApp { if self.job_waiters.is_empty() { return; } - self.session.poll_render(); - self.session.poll_offload(); + let script_busy = self.session.script_ui.render_host.is_some(); + if !script_busy { + self.session.poll_render(); + self.session.poll_offload(); + } let waiters = std::mem::take(&mut self.job_waiters); for (w, reply) in waiters { - if w.pending(self) { + // Completed native snapshots are immutable, so even a later suspended script + // cannot delay their replies. Other completions can edit its open undo group. + if (script_busy && w.render.is_none()) || w.pending(self) { self.job_waiters.push((w, reply)); } else { let v = w.finish(self); @@ -86,6 +95,42 @@ impl EffectcraftApp { } } + /// A suspended script still owns its undo groups. Only its render, inspection and Stop + /// run until the continuation reaches its next real script boundary; no scheduled callback + /// or background analysis may edit the document in the middle of the group. + fn tick_script_render(&mut self, ctx: &egui::Context) { + let Some(host) = self.session.script_ui.render_host else { return }; + let result = match self.session.script_ui.poll_render { + Some(poll) => poll(&mut self.session), + None => { + self.session.script_ui.render_host = None; + Some(json!({"ok": false, "error": {"message": "The script render waiter is unavailable."}})) + } + }; + if let Some(result) = result { + if let Some(message) = result["error"]["message"].as_str() { + self.ui.status = message.to_string(); + } else if let Some(output) = result["output"].as_str().filter(|s| !s.is_empty()) { + self.ui.status = output.lines().last().unwrap_or(output).to_string(); + } + self.script_waiters.retain(|(id, reply)| { + if *id == host { + let _ = reply.send(json!({"ok": true, "result": result})); + false + } else { + true + } + }); + } + // Do not interpret a close as permission to abandon the still-running script. Once + // Stop finishes it, the normal unsaved-changes prompt can handle a new close request. + if ctx.input(|i| i.viewport().close_requested()) { + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + } + self.drain_control(ctx, self.now); + ctx.request_repaint_after(std::time::Duration::from_millis(50)); + } + // ---------------------------------------------------------------- frame /// Apply the engine events drained this tick: open a comp, show a toast, open a URL, run a frontend command, @@ -136,6 +181,10 @@ impl EffectcraftApp { craft_quit::on_close_request(ctx); return; } + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } // Focus comes from the fresh viewport copy; a full pass's `InputState::focused` is only a // fallback (a logic-only tick leaves it at the previous pass's value). let focused = ctx.input(|i| i.viewport().focused.or(Some(i.focused))); @@ -195,8 +244,20 @@ impl EffectcraftApp { ctx.request_repaint_after(std::time::Duration::from_millis(100)); } self.run_script_tasks(ctx); + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } self.run_dialog_answers(); + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } self.handle_events(ctx); + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } if let Some(rx) = self.command_inbox.take() { while let Ok(id) = rx.try_recv() { if let Err(e) = menus::invoke(self, ctx, &id, json!({})) { @@ -205,6 +266,10 @@ impl EffectcraftApp { } self.command_inbox = Some(rx); } + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } // 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`. @@ -227,6 +292,12 @@ impl EffectcraftApp { self.advance_playback(ctx, scale, now, active, cid); } self.drain_control(ctx, now); + // A clean project may have passed the close guard before a control script created + // unsaved edits and suspended. Cancel that same tick's close before the host handles it. + if self.session.script_ui.render_host.is_some() { + self.tick_script_render(ctx); + return; + } // Screenshot completion, deadlines and issuance are app-side work: the callback only // stores pixels and wakes this viewport. self.tick_screenshots(ctx); @@ -375,6 +446,23 @@ impl EffectcraftApp { /// The panels, menus, dialogs and overlays of one pass. fn frame(&mut self, ui: &mut egui::Ui) { let ctx = ui.ctx().clone(); + if self.session.script_ui.render_host.is_some() { + self.auto.begin_frame(); + ui.vertical_centered(|ui| { + ui.add_space(32.0); + ui.heading("Rendering script"); + ui.label("The script will continue when rendering finishes. Editing is paused to preserve its undo group."); + if let Some(progress) = self.session.render_progress() { + ui.label(format!("{} / {} frames", progress.done, progress.total)); + } + let stop = ui.button("Stop Render"); + self.auto.add("scriptRender.stop", stop.rect, "Stop Render"); + if stop.clicked() { + self.session.stop_render(); + } + }); + return; + } // Only a viewer that draws in *this* pass records a scale: a closed viewer, the Home // screen or another comp must stop idle prefetch at a stale scale. self.last_viewer_scale = None; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 86bd20c93..f7aecdfcb 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -21,6 +21,7 @@ //! //! The full reference with examples is `docs/control-protocol.md`. +use std::sync::Arc; use std::sync::mpsc::Sender; use serde_json::{Value, json}; @@ -60,6 +61,8 @@ pub enum Outcome { }, /// Reply once background jobs end (a `wait: true` command whose job runs in a worker). AwaitJobs(JobWaiter), + /// A script yielded its session for rendering; reply after its continuation returns. + AwaitScript(u32), } /// A `wait: true` engine command whose job was sent to the session's offload (the browser's @@ -73,6 +76,8 @@ pub struct JobWaiter { pub jobs: Vec, /// Absolute completion-log position when it started (including evicted entries). pub log_cursor: usize, + /// The exact native render, not the reusable `jobs.list` identifier. + pub render: Option>, } /// Commands that block with `wait` (and its default). @@ -81,34 +86,74 @@ fn waits(command: &str, params: &Value) -> bool { params.get("wait").and_then(Value::as_bool).unwrap_or(default) } -/// Run an engine command. A waiting command (`wait: true`) whose job the session offloads -/// runs with `wait: false` instead, and the reply waits for the job ([`Outcome::AwaitJobs`]), -/// so the browser's UI keeps drawing while it runs. +/// Run an engine command. A waiting offloaded command or native render runs with `wait: false` +/// instead, and the reply waits for the job ([`Outcome::AwaitJobs`]), keeping the UI responsive. fn execute_engine(app: &mut EffectcraftApp, id: &str, params: Value) -> Result { - if !app.session.offloads() || !waits(id, ¶ms) { - return app.session.execute_checked(id, params).map(ok).map_err(|e| e.to_string()); + if !(app.session.offloads() || (cfg!(not(target_arch = "wasm32")) && id == "renderQueue.render")) || !waits(id, ¶ms) { + let result = app.session.execute_checked(id, params); + return result.map(script_outcome).map_err(|e| e.to_string()); } let before: Vec = app.session.jobs().into_iter().map(|j| j.id).collect(); let log_cursor = app.session.offload_log_start.saturating_add(app.session.offload_log.len()); let mut p = params.clone(); - p["wait"] = json!(false); + p.as_object_mut().ok_or("command params must be an object")?.insert("wait".into(), json!(false)); let result = app.session.execute_checked(id, p).map_err(|e| e.to_string())?; let jobs: Vec = app.session.jobs().into_iter().map(|j| j.id).filter(|j| !before.contains(j)).collect(); - if jobs.is_empty() { - return Ok(ok(result)); + let render = if cfg!(not(target_arch = "wasm32")) && id == "renderQueue.render" && !app.session.offloads() { + app.session.render_job.as_ref().map(|job| { + job.shared.retain_reply_items(result["items"].as_array().cloned().unwrap_or_default()); + job.shared.clone() + }) + } else { + None + }; + Ok(JobWaiter { command: id.to_string(), params, result, jobs, log_cursor, render }.into_outcome()) +} + +fn script_outcome(result: Value) -> Outcome { + match result.get("renderHost").and_then(Value::as_u64).and_then(|id| u32::try_from(id).ok()) { + Some(host) => Outcome::AwaitScript(host), + None => ok(result), + } +} + +/// Suspended scripts retain open undo groups. Let the UI inspect and stop the render, but +/// never fold an unrelated edit, another script or a project replacement into that group. +pub(crate) fn check_script_render(app: &EffectcraftApp, id: &str) -> Result<(), String> { + if app.session.script_ui.render_host.is_some() && !effectcraft_engine::scriptui::allowed_during_render(id) { + Err(effectcraft_engine::scriptui::RENDER_WAIT_MESSAGE.into()) + } else { + Ok(()) } - Ok(Outcome::AwaitJobs(JobWaiter { command: id.to_string(), params, result, jobs, log_cursor })) } impl JobWaiter { + fn into_outcome(self) -> Outcome { + // A finished but unpolled native worker still needs its completion boundary even + // when jobs() no longer lists it. Already-polled commands have their final result. + if self.jobs.is_empty() && self.render.is_none() { ok(self.result) } else { Outcome::AwaitJobs(self) } + } + /// Whether its jobs are still running. pub fn pending(&self, app: &EffectcraftApp) -> bool { + if let Some(render) = &self.render { + return !render.reply_finished() && app.session.render_job.as_ref().is_some_and(|job| Arc::ptr_eq(render, &job.shared)); + } app.session.jobs().iter().any(|j| self.jobs.contains(&j.id)) } /// The reply: what the blocking command would have returned (`running: false`, the final /// progress, render items and analysis status), or the job's error. pub fn finish(self, app: &mut EffectcraftApp) -> Value { + if let Some(render) = self.render { + let Some(items) = render.completed_items() else { + return json!({"ok": false, "error": "render result unavailable: the project or render job was replaced before completion"}); + }; + let mut result = self.result; + result["items"] = json!(items); + result["rendering"] = json!(false); + return json!({"ok": true, "result": result}); + } let s = &mut app.session; s.poll_render(); let error = s @@ -178,6 +223,14 @@ fn point(app: &EffectcraftApp, p: &Value) -> Result { /// [`Outcome::AwaitJobs`] instead of replying in this call. pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlRequest, now: f64) -> Outcome { let p = &req.params; + if app.session.script_ui.render_host.is_some() + && !matches!( + req.method.as_str(), + "engine.execute" | "ui.menu.invoke" | "engine.commands" | "ui.menu.list" | "ui.inspect" | "ui.elements" | "ui.resize" | "ui.focus" + ) + { + return err(effectcraft_engine::scriptui::RENDER_WAIT_MESSAGE); + } match req.method.as_str() { "engine.execute" | "ui.menu.invoke" => execute_command(app, ctx, &req.method, p), "engine.commands" => engine_commands(app, p), @@ -211,15 +264,25 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques fn execute_command(app: &mut EffectcraftApp, ctx: &egui::Context, method: &str, p: &Value) -> Outcome { let s = |k: &str| p.get(k).and_then(Value::as_str); let Some(id) = s("command").or(s("id")) else { return err("missing `command`") }; + if let Err(e) = check_script_render(app, id) { + return err(e); + } let params = p.get("params").cloned().filter(|v| !v.is_null()).unwrap_or(json!({})); + // Only this invocation opts in: CLI, lifecycle scripts and the inline console retain + // their synchronous contract. Its host remembers the opt-in across successive renders. + let previous = app.session.script_ui.yield_renders; + app.session.script_ui.yield_renders = cfg!(not(target_arch = "wasm32")) && id == "script.run"; // `engine.execute` runs engine commands exactly as headless (never opens a dialog, e.g. // `comp.new {}` creates a default comp); UI-only ids (`tool.*`, `view.*`, `window.*`, // `playback.*` …) and `ui.menu.invoke` go through the menu dispatcher like a click. - let r = if method == "engine.execute" && effectcraft_engine::find_command(id).is_some() { + let r = if (method == "engine.execute" && effectcraft_engine::find_command(id).is_some()) + || (id == "renderQueue.render" && params.get("wait").and_then(Value::as_bool) == Some(true)) + { execute_engine(app, id, params) } else { - crate::menus::invoke(app, ctx, id, params).map(ok) + crate::menus::invoke(app, ctx, id, params).map(script_outcome) }; + app.session.script_ui.yield_renders = previous; match r { Ok(o) => o, Err(e) => { @@ -893,6 +956,26 @@ pub fn encode_png(rgba: &[u8], w: u32, h: u32) -> Result, String> { #[cfg(test)] mod tests { + #[test] + fn a_native_render_with_no_running_jobs_still_checks_its_own_completion() { + let waiter = super::JobWaiter { + command: "renderQueue.render".into(), + params: serde_json::json!({}), + result: serde_json::json!({"rendering": true, "items": []}), + jobs: vec![], + log_cursor: 0, + render: Some(std::sync::Arc::new(effectcraft_engine::render_queue::JobShared::default())), + }; + let super::Outcome::AwaitJobs(waiter) = waiter.into_outcome() else { + panic!("a finished native render must still pass through completion polling"); + }; + let mut app = crate::EffectcraftApp::new(effectcraft_engine::Session::default()); + assert!(!waiter.pending(&app)); + let reply = waiter.finish(&mut app); + assert_eq!(reply["ok"], false, "an abandoned render cannot silently report a replacement project's queue"); + assert!(reply["error"].as_str().unwrap().contains("replaced before completion")); + } + #[test] fn base64_rfc4648_vectors() { for (i, o) in [("", ""), ("f", "Zg=="), ("fo", "Zm8="), ("foo", "Zm9v"), ("foob", "Zm9vYg=="), ("fooba", "Zm9vYmE="), ("foobar", "Zm9vYmFy")] { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e84ef79dd..be14cd8df 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -302,6 +302,7 @@ pub struct EffectcraftApp { pub(crate) input_waiters: Vec>, /// Control requests waiting for an offloaded job (`wait: true` in the browser). pub(crate) job_waiters: Vec<(control::JobWaiter, Sender)>, + pub(crate) script_waiters: Vec<(u32, Sender)>, pending_screenshots: Vec, /// Pixels reserved by queued and in-flight screenshot requests. shot_bytes: usize, diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index 76a70426e..bebb7f767 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -185,6 +185,7 @@ fn only_comp(p: &Value) -> Option { /// Execute a UI or engine command by id. pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { + crate::control::check_script_render(app, id)?; if matches!(id, "effects.copyPresetsFolderPath" | "mediaBrowser.copySourcePath") { return frontend(app, ctx, id, params); } diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 422f0223e..ca509e064 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -82,6 +82,8 @@ mod ui_roto_objects; mod ui_roto_span_confidence; mod ui_script_console; mod ui_script_console_keyboard; +#[cfg(not(target_arch = "wasm32"))] +mod ui_script_render; mod ui_scriptui; mod ui_scrolling; mod ui_self_drop; diff --git a/crates/ui-egui/tests/it/ui_script_render.rs b/crates/ui-egui/tests/it/ui_script_render.rs new file mode 100644 index 000000000..b3c24733c --- /dev/null +++ b/crates/ui-egui/tests/it/ui_script_render.rs @@ -0,0 +1,260 @@ +//! #3289: the control bridge must keep servicing UI frames during scripted and direct renders. + +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::mpsc::{Receiver, TryRecvError}; +use std::time::{Duration, Instant}; + +use effectcraft_engine::project::render_queue::OutputFormat; +use effectcraft_engine::{ExportJob, ExportResult, Exporter}; +use effectcraft_ui_egui::{ControlRequest, EffectcraftApp}; +use egui_kittest::Harness; +use serde_json::{Value, json}; + +struct Gate(AtomicBool); +impl Exporter for Gate { + fn formats(&self) -> Vec { + vec![OutputFormat::H264] + } + fn export(&self, job: &ExportJob, progress: &mut dyn FnMut(u64, u64) -> bool) -> Result { + let end = Instant::now() + Duration::from_secs(3); + while !self.0.load(Ordering::SeqCst) { + if !progress(0, 10) { + return Err(effectcraft_engine::render_queue::CANCELLED.into()); + } + if Instant::now() > end { + return Err("test exporter waited for a blocked UI".into()); + } + std::thread::sleep(Duration::from_millis(1)); + } + Ok(ExportResult { path: job.path.into(), frames: 10, ..Default::default() }) + } +} + +fn harness() -> (Harness<'static, EffectcraftApp>, Arc, std::sync::mpsc::Sender) { + let gate = Arc::new(Gate(AtomicBool::new(false))); + let mut s = effectcraft_host::session(); + s.exporter = Some(gate.clone()); + s.prefs.auto_save.enabled = false; + s.execute("comp.new", json!({"width": 16, "height": 16, "duration": 1})).unwrap(); + let path = std::env::temp_dir().join(format!("ec-ui-script-render-{}.mp4", std::process::id())); + s.execute("renderQueue.add", json!({"output": path.to_string_lossy()})).unwrap(); + let (tx, rx) = std::sync::mpsc::channel(); + let h = Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_eframe(|_| EffectcraftApp::new(s).with_control(rx)); + (h, gate, tx) +} + +fn send(tx: &std::sync::mpsc::Sender, method: &str, params: Value) -> Receiver { + let (request, reply) = ControlRequest::new(method, params); + tx.send(request).unwrap(); + reply +} + +fn finish(h: &mut Harness<'_, EffectcraftApp>, reply: &Receiver) -> Value { + let end = Instant::now() + Duration::from_secs(4); + loop { + h.run_steps(1); + if let Ok(result) = reply.try_recv() { + return result; + } + assert!(Instant::now() < end, "control reply did not complete"); + std::thread::sleep(Duration::from_millis(1)); + } +} + +#[test] +fn script_render_keeps_frames_and_inspection_alive_until_the_continuation_finishes() { + let (mut h, gate, tx) = harness(); + let reply = send( + &tx, + "engine.execute", + json!({"command": "script.run", "params": {"code": "$.writeln('before'); app.project.renderQueue.render(); app.project.activeItem.layers.addNull().name = 'After'; $.writeln('after'); 42;"}}), + ); + h.run_steps(3); + assert!(matches!(reply.try_recv(), Err(TryRecvError::Empty)), "script reply must wait for its continuation"); + assert!(h.state().session.active_comp().unwrap().layers.is_empty()); + let before = h.state().ui_passes(); + let inspect = send(&tx, "ui.inspect", json!({})); + let edit = send(&tx, "engine.execute", json!({"command": "layer.newNull"})); + let batch = send(&tx, "engine.execute", json!({"command": "engine.batch", "params": {"steps": [{"command": "layer.newNull"}]}})); + let jobs = send(&tx, "engine.execute", json!({"command": "jobs.list"})); + let quit = send(&tx, "app.quit", json!({"force": true})); + h.run_steps(3); + assert!(h.state().ui_passes() > before, "frames still draw while the render is gated"); + assert_eq!(inspect.try_recv().unwrap()["ok"], true); + assert_eq!(edit.try_recv().unwrap()["ok"], false, "unrelated edits cannot enter the script's undo group"); + assert_eq!(batch.try_recv().unwrap()["ok"], false, "batches are not pure queries"); + assert_eq!(jobs.try_recv().unwrap()["ok"], false, "job polling can apply unrelated edits"); + assert_eq!(quit.try_recv().unwrap()["ok"], false, "force quit cannot abandon the script's live session"); + assert!(h.state().auto.find("scriptRender.stop").is_some()); + gate.0.store(true, Ordering::SeqCst); + let done = finish(&mut h, &reply); + assert_eq!(done["ok"], true, "{done}"); + assert_eq!(done["result"]["ok"], true, "{done}"); + assert_eq!(done["result"]["result"], 42); + assert_eq!(done["result"]["output"], "before\nafter"); + assert_eq!(h.state().session.active_comp().unwrap().layers[0].name, "After"); + assert!(h.state().session.script_ui.render_host.is_none()); +} + +#[test] +fn stop_render_remains_available_while_a_script_waits() { + let (mut h, _gate, tx) = harness(); + let reply = send(&tx, "engine.execute", json!({"command": "script.run", "params": {"code": "app.project.renderQueue.render(); 23;"}})); + h.run_steps(2); + assert!(matches!(reply.try_recv(), Err(TryRecvError::Empty))); + let stop = send(&tx, "engine.execute", json!({"command": "renderQueue.stop"})); + h.run_steps(1); + assert_eq!(stop.try_recv().unwrap()["ok"], true); + assert_eq!(finish(&mut h, &reply)["result"]["result"], 23); + assert!(!h.state().session.is_rendering()); +} + +#[test] +fn closing_when_a_control_script_starts_cannot_abandon_its_new_unsaved_changes() { + let (mut h, gate, tx) = harness(); + h.state_mut().session.mark_saved(); + let reply = send( + &tx, + "engine.execute", + json!({"command": "script.run", "params": {"code": "app.project.activeItem.layers.addNull(); app.project.renderQueue.render(); 31;"}}), + ); + h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + h.step(); + let canceled = h.output().viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose)); + let dirty = h.state().session.is_dirty(); + let pending = matches!(reply.try_recv(), Err(TryRecvError::Empty)); + gate.0.store(true, Ordering::SeqCst); + assert_eq!(finish(&mut h, &reply)["result"]["result"], 31); + assert!(canceled, "a close accepted before the control request must be canceled once the script suspends"); + assert!(dirty && pending); +} + +#[test] +fn closing_on_the_completion_frame_cannot_skip_the_unsaved_changes_guard() { + let (mut h, gate, tx) = harness(); + let reply = send(&tx, "engine.execute", json!({"command": "script.run", "params": {"code": "app.project.renderQueue.render(); 23;"}})); + h.run_steps(2); + assert!(h.state().session.script_ui.render_host.is_some()); + gate.0.store(true, Ordering::SeqCst); + let end = Instant::now() + Duration::from_secs(4); + while h.state().session.is_rendering() { + assert!(Instant::now() < end); + std::thread::sleep(Duration::from_millis(1)); + } + h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + h.step(); + assert!(h.output().viewport_output.get(&egui::ViewportId::ROOT).is_some_and(|v| v.commands.contains(&egui::ViewportCommand::CancelClose))); + assert_eq!(reply.try_recv().unwrap()["result"]["result"], 23); + assert!(h.state().session.is_dirty()); +} + +#[test] +fn direct_native_render_waits_for_its_worker_without_blocking_frames() { + let (mut h, gate, tx) = harness(); + let reply = send(&tx, "engine.execute", json!({"command": "renderQueue.render"})); + h.run_steps(3); + assert!(matches!(reply.try_recv(), Err(TryRecvError::Empty))); + let inspect = send(&tx, "ui.inspect", json!({})); + h.run_steps(2); + assert_eq!(inspect.try_recv().unwrap()["ok"], true); + gate.0.store(true, Ordering::SeqCst); + let done = finish(&mut h, &reply); + assert_eq!(done["result"]["rendering"], false); + assert_eq!(done["result"]["items"][0]["statusLabel"], "Done", "{done}"); +} + +#[test] +fn malformed_waiting_render_params_return_an_error() { + let (mut h, _gate, tx) = harness(); + let reply = send(&tx, "engine.execute", json!({"command": "renderQueue.render", "params": []})); + h.run_steps(1); + assert_eq!(reply.try_recv().unwrap()["ok"], false); + assert!(!h.state().session.is_rendering()); +} + +fn wait_worker(h: &Harness<'_, EffectcraftApp>) { + let end = Instant::now() + Duration::from_secs(4); + while h.state().session.is_rendering() { + assert!(Instant::now() < end, "render worker did not finish"); + std::thread::sleep(Duration::from_millis(1)); + } +} + +#[test] +fn direct_render_reply_keeps_its_items_after_queue_removal_and_a_new_render() { + use effectcraft_ui_egui::control::{Outcome, handle}; + + for no_running_jobs in [false, true] { + let (mut h, gate, _tx) = harness(); + let ctx = egui::Context::default(); + let request = ControlRequest::new("engine.execute", json!({"command": "renderQueue.render"})).0; + let Outcome::AwaitJobs(mut first) = handle(h.state_mut(), &ctx, &request, 0.0) else { panic!("expected a deferred render") }; + let original = first.result["items"][0].clone(); + // Also cover completion between the command's result and its jobs() snapshot. + if no_running_jobs { + first.jobs.clear(); + } + gate.0.store(true, Ordering::SeqCst); + wait_worker(&h); + gate.0.store(false, Ordering::SeqCst); + // The worker is done, but no frontend completion pass has run. Removing the item + // must not erase the waiting reply, even when the next queue reuses the same id. + h.state_mut().session.execute("renderQueue.remove", json!({"item": original["id"]})).unwrap(); + h.state_mut().session.poll_render(); + let second_path = std::env::temp_dir().join(format!("ec-ui-second-render-{}.mp4", std::process::id())); + h.state_mut().session.execute("renderQueue.add", json!({"output": second_path.to_string_lossy()})).unwrap(); + let Outcome::AwaitJobs(second) = handle(h.state_mut(), &ctx, &request, 0.0) else { panic!("expected a second deferred render") }; + assert!(second.pending(h.state())); + assert!(!first.pending(h.state()), "the first reply must not wait for a later job with the same jobs.list id"); + let done = first.finish(h.state_mut()); + assert_eq!(done["result"]["rendering"], false, "{done}"); + assert_eq!(done["result"]["items"][0]["statusLabel"], "Done", "{done}"); + assert_eq!(done["result"]["items"][0]["outputPath"], original["outputPath"]); + assert_eq!(done["result"]["items"][0]["last_output"], original["outputPath"]); + assert!(h.state().session.is_rendering()); + gate.0.store(true, Ordering::SeqCst); + wait_worker(&h); + h.state_mut().session.poll_render(); + assert!(!second.pending(h.state())); + assert_eq!(second.finish(h.state_mut())["result"]["items"][0]["outputPath"], second_path.to_string_lossy().as_ref()); + } +} + +#[test] +fn completed_direct_render_reply_is_not_delayed_by_a_later_script_render() { + let (mut h, gate, tx) = harness(); + let first = send(&tx, "engine.execute", json!({"command": "renderQueue.render"})); + h.run_steps(2); + assert!(matches!(first.try_recv(), Err(TryRecvError::Empty))); + gate.0.store(true, Ordering::SeqCst); + wait_worker(&h); + gate.0.store(false, Ordering::SeqCst); + h.state_mut().session.poll_render(); + h.state_mut().session.execute("renderQueue.setRender", json!({"index": 1, "render": true})).unwrap(); + let second = send(&tx, "engine.execute", json!({"command": "script.run", "params": {"code": "app.project.renderQueue.render(); 47;"}})); + h.run_steps(1); + assert!(h.state().session.script_ui.render_host.is_some()); + assert!(matches!(second.try_recv(), Err(TryRecvError::Empty))); + let done = first.try_recv().expect("an immutable completed reply does not enter the new script's undo group"); + assert_eq!(done["result"]["rendering"], false); + assert_eq!(done["result"]["items"][0]["statusLabel"], "Done"); + gate.0.store(true, Ordering::SeqCst); + assert_eq!(finish(&mut h, &second)["result"]["result"], 47); +} + +#[test] +fn replacing_project_before_render_poll_returns_an_explicit_error() { + use effectcraft_ui_egui::control::{Outcome, handle}; + + let (mut h, gate, _tx) = harness(); + let request = ControlRequest::new("engine.execute", json!({"command": "renderQueue.render"})).0; + let Outcome::AwaitJobs(waiter) = handle(h.state_mut(), &egui::Context::default(), &request, 0.0) else { panic!("expected a deferred render") }; + gate.0.store(true, Ordering::SeqCst); + wait_worker(&h); + h.state_mut().session.replace_project(Default::default(), None); + assert!(!waiter.pending(h.state())); + let reply = waiter.finish(h.state_mut()); + assert_eq!(reply["ok"], false, "{reply}"); + assert!(reply["error"].as_str().unwrap().contains("replaced before completion")); +} diff --git a/crates/ui-egui/tests/it/ui_web_offload.rs b/crates/ui-egui/tests/it/ui_web_offload.rs index fc0a0cd32..fde177cb6 100644 --- a/crates/ui-egui/tests/it/ui_web_offload.rs +++ b/crates/ui-egui/tests/it/ui_web_offload.rs @@ -241,7 +241,14 @@ fn waiting_commands_report_only_their_own_completion_errors() { let other = if kind == JobKind::Track { JobKind::Warp } else { JobKind::Track }; let mut app = EffectcraftApp::new(Session::default()); app.session.offload_log = vec![(kind, Some("old error".into())), (other, Some("unrelated failure".into())), (kind, None)]; - let waiter = || JobWaiter { command: "analysis".into(), params: json!({}), result: json!({"running": true}), jobs: vec![job_id.into()], log_cursor: 1 }; + let waiter = || JobWaiter { + command: "analysis".into(), + params: json!({}), + result: json!({"running": true}), + jobs: vec![job_id.into()], + log_cursor: 1, + render: None, + }; let result = waiter().finish(&mut app); assert_eq!(result, json!({"ok": true, "result": {"running": false}}), "{kind:?}"); app.session.offload_log.push((kind, Some("nothing was applied".into()))); @@ -258,6 +265,6 @@ fn waiting_command_sees_its_error_after_the_completion_log_rolls_over() { app.session.offload_log = vec![(JobKind::Warp, None); 31]; app.session.offload_log.push((JobKind::Track, Some("nothing was applied".into()))); app.session.offload_log_start = 1; - let waiter = JobWaiter { command: "track.analyze".into(), params: json!({}), result: json!({}), jobs: vec!["track".into()], log_cursor: 32 }; + let waiter = JobWaiter { command: "track.analyze".into(), params: json!({}), result: json!({}), jobs: vec!["track".into()], log_cursor: 32, render: None }; assert_eq!(waiter.finish(&mut app), json!({"ok": false, "error": "nothing was applied"})); } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9f3d69ea5..9abc4b494 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -60,6 +60,26 @@ export formats, parity summary), `jobs.list` (Progress panel jobs, e.g. the back Every engine command is reachable through `engine.execute`, and every interactive widget of every panel registers an element id; both are enforced by `crates/ui-egui/tests/it/ui_agent_audit.rs`. +### Waiting for a render + +On native desktop builds, `engine.execute` with `renderQueue.render` and `wait: true` +(the default) keeps the window responsive while its reply waits for the render worker. +`script.run` invoked through the control channel also yields the window while +`app.project.renderQueue.render()` waits. JavaScript resumes after completion or cancellation; +the original request receives its result and accumulated output only after the continuation +finishes or shows a modal dialog. Several renders in one script keep the same request pending. +Headless scripts retain their synchronous behavior. + +While a script waits for rendering, the window shows progress and a **Stop Render** button +(`scriptRender.stop`). Other editing and script callbacks pause so they cannot enter the +suspended script's undo group. `ui.inspect`, `ui.elements`, command/menu listings, resize, +focus, and the engine commands `renderQueue.list`, `renderQueue.stop`, `editor.state`, +`app.capabilities`, `command.list`, `scriptui.list` and `scriptui.get` remain available. +Other control requests return an error. Use another connection for inspection or Stop, +since requests on one connection are answered in order. A window-close request is canceled +during this wait; stop the render and then close normally to use the unsaved-changes prompt. +The control transport's 60-second request timeout still applies to long renders. + ## UI state | Method | Params | Result |