diff --git a/crates/engine/src/commands/file_more.rs b/crates/engine/src/commands/file_more.rs index aa9e1be9..9aecd8d6 100644 --- a/crates/engine/src/commands/file_more.rs +++ b/crates/engine/src/commands/file_more.rs @@ -465,6 +465,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { } _ => return Err(bad("file.runScript", "missing `path` (or inline `steps`)")), }; + // A command script read from a file is a script: it can't change what scripts may do. + let from_file = p.get("steps").is_none(); let mut results = vec![]; for (i, step) in steps.iter().enumerate() { let (id, params) = match (step.get("command").or(step.get("id")).and_then(Value::as_str), step.get("method").and_then(Value::as_str)) { @@ -478,7 +480,8 @@ fn run_script(s: &mut Session, p: &Value) -> Result { if id == "file.runScript" || id == "script.run" { return Err(bad("file.runScript", "scripts can't run scripts")); } - let r = s.execute(&id, params).map_err(|e| EngineError::Other(format!("step {} ({id}): {e}", i + 1)))?; + let r = if from_file { s.execute_for_script(&id, params) } else { s.execute(&id, params) }; + let r = r.map_err(|e| EngineError::Other(format!("step {} ({id}): {e}", i + 1)))?; results.push(r); } Ok(json!({"steps": results.len(), "results": results})) diff --git a/crates/engine/src/commands/settings.rs b/crates/engine/src/commands/settings.rs index 320a6c18..1a5789e2 100644 --- a/crates/engine/src/commands/settings.rs +++ b/crates/engine/src/commands/settings.rs @@ -9,6 +9,10 @@ use crate::{EngineError, Result, Session, cmd, query}; // ---------------------------------------------------------------- prefs +/// Why a script can't change Application Scripting settings. +const SCRIPT_PERMISSIONS: &str = + "Scripts can't change what scripts are allowed to do. Change it yourself in Settings ▸ Scripting & Expressions ▸ Application Scripting."; + fn prefs_get(s: &mut Session, p: &Value) -> Result { let key = str_p(p, "key").unwrap_or(""); s.prefs.get(key).ok_or_else(|| bad("prefs.get", format!("unknown setting `{key}`"))) @@ -30,6 +34,9 @@ fn prefs_set(s: &mut Session, p: &Value) -> Result { for (k, v) in &pairs { next.set(k, v.clone()).map_err(|e| bad("prefs.set", e))?; } + if s.running_for_script() && !next.scripting.same_permissions(&s.prefs.scripting) { + return Err(EngineError::Other(SCRIPT_PERMISSIONS.into())); + } s.prefs = next; s.prefs_changed(); s.save_prefs(); @@ -39,7 +46,12 @@ fn prefs_set(s: &mut Session, p: &Value) -> Result { fn prefs_reset(s: &mut Session, p: &Value) -> Result { let page = str_p(p, "page"); + let before = s.prefs.scripting.clone(); s.prefs.reset(page).map_err(|e| bad("prefs.reset", e))?; + // A script's reset leaves what scripts may do as the user set it. + if s.running_for_script() { + s.prefs.scripting.keep_permissions(&before); + } s.prefs_changed(); s.save_prefs(); s.toast(match page { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index a9ee6b89..d09cbf2b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -472,8 +472,16 @@ pub struct Session { pub footage_stamps: std::collections::HashMap, /// The stamps being taken on a worker thread ([`footage_reload::start_scan`]). pub footage_scan: Option, + /// How many script-issued commands are running ([`Session::execute_for_script`]): while one + /// is, settings that guard what scripts may do can't change (see + /// [`prefs::Scripting::same_permissions`]). + pub script_calls: ScriptCalls, } +/// The count behind [`Session::running_for_script`]; only the engine changes it. +#[derive(Clone, Copy, Debug, Default)] +pub struct ScriptCalls(u32); + /// A thumbnail render ([`Session::thumbnail_job`]): (width, height, RGBA8). pub type ThumbnailJob = Box Option<(u32, u32, Vec)> + Send>; @@ -559,6 +567,7 @@ impl Default for Session { check_footage_on_open: false, footage_stamps: Default::default(), footage_scan: None, + script_calls: ScriptCalls::default(), } } } @@ -601,6 +610,22 @@ impl Session { self.execute(id, params) } + /// [`Session::execute`] for a command a script asked for (the JavaScript engine's + /// `app.run`, ScriptUI callbacks…). The command and everything it runs count as the script's: + /// it may not change the settings that limit scripts (Settings ▸ Scripting & Expressions ▸ + /// Application Scripting), which only the user can, from the UI, CLI or control channel. + pub fn execute_for_script(&mut self, id: &str, params: Value) -> Result { + self.script_calls.0 = self.script_calls.0.saturating_add(1); + let r = self.execute(id, params); + self.script_calls.0 = self.script_calls.0.saturating_sub(1); + r + } + + /// Whether the command running was issued by a script ([`Session::execute_for_script`]). + pub fn running_for_script(&self) -> bool { + self.script_calls.0 > 0 + } + pub fn is_enabled(&self, id: &str) -> bool { commands::find(id).is_some_and(|c| (c.enabled)(self).is_ok()) } diff --git a/crates/engine/src/prefs.rs b/crates/engine/src/prefs.rs index 83f0498b..8f6af9c2 100644 --- a/crates/engine/src/prefs.rs +++ b/crates/engine/src/prefs.rs @@ -309,6 +309,25 @@ page!(Scripting { error_banner: bool = true, }); +impl Scripting { + /// Whether `other` grants scripts the same as this: Application Scripting (file and network + /// access, the warning before executing files, the debugger). Scripts can't change these + /// ([`crate::Session::execute_for_script`]): a script could otherwise give itself the + /// access the user withheld. + pub fn same_permissions(&self, other: &Scripting) -> bool { + self.allow_scripts_write_files == other.allow_scripts_write_files + && self.warn_executing_files == other.warn_executing_files + && self.enable_js_debugger == other.enable_js_debugger + } + + /// Take `from`'s Application Scripting settings (see [`Scripting::same_permissions`]). + pub fn keep_permissions(&mut self, from: &Scripting) { + self.allow_scripts_write_files = from.allow_scripts_write_files; + self.warn_executing_files = from.warn_executing_files; + self.enable_js_debugger = from.enable_js_debugger; + } +} + /// One editable label: name and `#rrggbb` colour. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct LabelPref { diff --git a/crates/media/src/exr_channels.rs b/crates/media/src/exr_channels.rs index 06bda925..732cd3c4 100644 --- a/crates/media/src/exr_channels.rs +++ b/crates/media/src/exr_channels.rs @@ -37,6 +37,18 @@ pub fn parse_manifest(json: &str) -> Vec<(String, u32)> { v } +/// Why an OpenEXR file's pixels can't be decoded when its headers name a compression this build +/// can't decompress (HTJ2K, OpenEXR 3.4's High-Throughput JPEG 2000): the decoders only said +/// "no non-deep rgb channels" or showed a transparent frame (#482). `None` for other files. +pub fn unsupported_compression(bytes: &[u8]) -> Option<&'static str> { + use exr::compression::Compression; + let meta = exr::meta::MetaData::read_from_buffered(std::io::Cursor::new(bytes), false).ok()?; + meta.headers + .iter() + .any(|h| matches!(h.compression, Compression::HTJ2K32 | Compression::HTJ2K256)) + .then_some("OpenEXR HTJ2K compression isn't supported yet: re-save the file with ZIP, PIZ or DWAA compression") +} + /// Read all channels of an EXR file's bytes. `None` when it is not a readable EXR. pub fn read_exr_channels(bytes: &[u8]) -> Option { use exr::prelude::*; diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 5ce5730f..9db2f225 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -53,8 +53,10 @@ struct Entry { } /// Conformed audio files written by builds before this version are ignored (and written again): -/// version 1 files of sources longer than about 12:36 at 48 kHz were silent from there on (#172). -const CONFORM_VERSION: u32 = 2; +/// version 1 files of sources longer than about 12:36 at 48 kHz were silent from there on (#172); +/// version 2 files were written silent where the source failed to decode, and kept the footage +/// silent from then on (#324). +const CONFORM_VERSION: u32 = 3; /// Footage longer than this (24 hours) is read from its decoder rather than conformed: its /// duration comes from the file's header, and a damaged one claiming days of audio had the @@ -200,6 +202,10 @@ struct Inner { /// Conformed files being written right now. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] conforming: Mutex>, + /// Conformed files that failed to write (the source didn't decode) this session: their + /// audio is read from the decoder instead of trying again on every read. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + conform_failed: Mutex>, } /// Decodes footage for the renderer: stills, image sequences and movies (FilmCraft codecs), plus @@ -266,6 +272,7 @@ impl MediaPool { opened: AtomicU64::new(0), conform: Mutex::default(), conforming: Mutex::default(), + conform_failed: Mutex::default(), }), } } @@ -397,18 +404,23 @@ impl MediaPool { /// thread (once). #[cfg(not(target_arch = "wasm32"))] fn conform(&self, footage: &Footage, file: std::path::PathBuf, frames: usize, rate: u32) { - if !lock(&self.inner.conforming).insert(file.clone()) { + if lock(&self.inner.conform_failed).contains(&file) || !lock(&self.inner.conforming).insert(file.clone()) { return; } let (pool, f) = (self.clone(), footage.clone()); let spawned = std::thread::Builder::new().name("ec-conform-audio".into()).spawn(move || { let tmp = file.with_extension("tmp"); - let ok = file.parent().is_some_and(|d| std::fs::create_dir_all(d).is_ok()) - && pool.write_conformed(&f, &tmp, frames, rate).is_ok() - && std::fs::rename(&tmp, &file).is_ok(); - if !ok { + let written = match file.parent().map(std::fs::create_dir_all) { + Some(Ok(())) => pool.write_conformed(&f, &tmp, frames, rate).and_then(|()| std::fs::rename(&tmp, &file)), + Some(Err(e)) => Err(e), + None => Err(std::io::Error::other("no folder")), + }; + // A failed decode leaves no file: one written silent would keep the footage silent + // in every later session (#324). + if let Err(e) = written { let _ = std::fs::remove_file(&tmp); - log::warn!("media: could not write conformed audio {}", file.display()); + log::warn!("media: could not write conformed audio {} of {}: {e}", file.display(), f.path); + lock(&pool.inner.conform_failed).insert(file.clone()); } lock(&pool.inner.conforming).remove(&file); }); @@ -419,6 +431,7 @@ impl MediaPool { /// Write `frames` sample frames of `footage`'s audio at `rate` to `path` (raw interleaved /// stereo `f32`) a chunk at a time: the whole track was held in memory before it was written. + /// Fails when any chunk doesn't decode. #[cfg(not(target_arch = "wasm32"))] fn write_conformed(&self, footage: &Footage, path: &std::path::Path, frames: usize, rate: u32) -> std::io::Result<()> { use std::io::Write; @@ -426,7 +439,7 @@ impl MediaPool { let mut at = 0usize; while at < frames { let n = (frames - at).min(1 << 16); - for v in self.decode_audio(footage, sample_time(at, rate), n, rate) { + for v in self.try_decode_audio(footage, sample_time(at, rate), n, rate).map_err(std::io::Error::other)? { out.write_all(&v.to_le_bytes())?; } at += n; @@ -470,19 +483,27 @@ impl MediaPool { /// applied the edit list's priming offset twice and ramped the start of every read: AAC /// footage not at the mix rate stuttered in previews and exports (#274). fn decode_audio(&self, footage: &Footage, start: Tick, frames: usize, rate: u32) -> Vec { + self.try_decode_audio(footage, start, frames, rate).unwrap_or_else(|e| { + log::warn!("media: audio of {}: {e}", footage.path); + vec![0.0; frames * 2] + }) + } + + /// [`MediaPool::decode_audio`], failing when the source can't be opened or read. + fn try_decode_audio(&self, footage: &Footage, start: Tick, frames: usize, rate: u32) -> std::result::Result, String> { let path: Arc = footage.path.as_str().into(); - let Some(src) = self.inner.source(&path) else { return vec![0.0; frames * 2] }; + let src = self.inner.source(&path).ok_or("the file can't be opened")?; let s0 = start.to_units_floor(rate as i64); let native = src.info().audio.as_ref().map_or(rate, |a| a.sample_rate); if native == 0 || rate == 0 || native == rate { - return read_stereo(&src, &path, s0, frames, rate); + return read_stereo(&src, s0, frames, rate); } // Output sample `n` sits at source position n × native / rate. let (native_i, rate_i) = (i128::from(native), i128::from(rate)); let first = (i128::from(s0) * native_i).div_euclid(rate_i); let last = ((i128::from(s0) + frames as i128) * native_i).div_euclid(rate_i) + 1; - let (Ok(first64), Ok(count)) = (i64::try_from(first), usize::try_from(last - first + 1)) else { return vec![0.0; frames * 2] }; - let buf = read_stereo(&src, &path, first64, count, native); + let (Ok(first64), Ok(count)) = (i64::try_from(first), usize::try_from(last - first + 1)) else { return Ok(vec![0.0; frames * 2]) }; + let buf = read_stereo(&src, first64, count, native)?; let mut out = Vec::with_capacity(frames * 2); for k in 0..frames as i128 { let pos = (i128::from(s0) + k) * native_i; @@ -494,7 +515,7 @@ impl MediaPool { out.push(a + (b - a) * f); } } - out + Ok(out) } /// A thumbnail of `footage` (its first frame) fitting in `max_side` × `max_side`, aspect kept. @@ -710,6 +731,13 @@ impl Inner { if let Some(img) = crate::layered::decode(path, &bytes, footage, op)? { return Ok(img); } + // (an OpenEXR compression the decoder lacks came out transparent or as a missing + // channel error) + if path.to_ascii_lowercase().ends_with(".exr") + && let Some(why) = crate::exr_channels::unsupported_compression(&bytes) + { + return Err(MediaError::Decode(format!("{path}: {why}"))); + } let img = match image::load_from_memory(&bytes) { Ok(img) => img, // A multi-layer OpenEXR file without an unnamed RGB layer: its colour layer. @@ -800,27 +828,21 @@ impl FootageSource for MediaPool { /// `frames` stereo sample frames (interleaved) of `src` from sample `s0` at `rate` Hz, which /// FilmCraft serves as decoded when it is the source's own rate. Mono is duplicated to both -/// channels; silence before the start and where decoding fails. -fn read_stereo(src: &SharedSource, path: &str, s0: i64, frames: usize, rate: u32) -> Vec { +/// channels; silence before the start. An error where decoding fails. +fn read_stereo(src: &SharedSource, s0: i64, frames: usize, rate: u32) -> std::result::Result, String> { let mut out = vec![0.0; frames * 2]; let skip = usize::try_from(s0.saturating_neg()).unwrap_or(0); if skip >= frames { - return out; + return Ok(out); } - let buf = match src.audio(s0.max(0), frames - skip, rate) { - Ok(b) => b, - Err(e) => { - log::warn!("media: audio of {path}: {e}"); - return out; - } - }; - let (Some(l), Some(r)) = (buf.channels.first(), buf.channels.get(1).or(buf.channels.first())) else { return out }; - let Some(dst) = skip.checked_mul(2).and_then(|i| out.get_mut(i..)) else { return out }; + let buf = src.audio(s0.max(0), frames - skip, rate).map_err(|e| e.to_string())?; + let (Some(l), Some(r)) = (buf.channels.first(), buf.channels.get(1).or(buf.channels.first())) else { return Ok(out) }; + let Some(dst) = skip.checked_mul(2).and_then(|i| out.get_mut(i..)) else { return Ok(out) }; for ([o0, o1], (a, b)) in dst.as_chunks_mut::<2>().0.iter_mut().zip(l.iter().zip(r)) { *o0 = *a; *o1 = *b; } - out + Ok(out) } fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { @@ -880,6 +902,43 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// #324: audio that fails to decode isn't conformed. A conformed file of silence was written + /// instead, and as it is found by the file's path, size and date the footage stayed silent + /// in every later session (a renamed copy played). + #[test] + fn audio_that_fails_to_decode_is_not_conformed() { + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/test-conform-fail"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("song.mp3"); + std::fs::write(&path, b"ID3, not really").unwrap(); + let footage = Footage { + path: path.to_string_lossy().into(), + kind: FootageKind::Audio, + duration: Tick::from_seconds_f64(2.0), + has_audio: true, + ..Default::default() + }; + let pool = MediaPool::new(); + let conformed = dir.join("conformed"); + pool.set_conform_folder(Some(conformed.clone())); + let file = pool.conformed_path(&footage.path, 48_000).unwrap(); + assert!(pool.audio_samples(&footage, Tick::ZERO, 1024, 48_000).iter().all(|v| *v == 0.0)); + for _ in 0..500 { + if lock(&pool.inner.conforming).is_empty() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + assert!(lock(&pool.inner.conforming).is_empty(), "the conform thread finished"); + assert!(!file.exists() && !file.with_extension("tmp").exists(), "no conformed file was left"); + assert!(lock(&pool.inner.conform_failed).contains(&file)); + // Later reads go to the decoder without trying again. + pool.audio_samples(&footage, Tick::ZERO, 1024, 48_000); + assert!(lock(&pool.inner.conforming).is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn frame_index_loops_and_clamps() { let r = FrameRate::FPS_30; @@ -911,6 +970,32 @@ mod tests { assert!(c.take_spare(17).is_empty()); } + /// #482: an OpenEXR file with HTJ2K compression (which the decoder lacks) says so on import + /// and when its frame is read, instead of "no non-deep rgb channels" or a transparent frame. + #[test] + fn htj2k_exr_is_reported_as_unsupported() { + use exr::prelude::*; + let channels = + AnyChannels::sort(vec![AnyChannel::new("R", FlatSamples::F32(vec![0.5; 4])), AnyChannel::new("G", FlatSamples::F32(vec![0.5; 4]))].into()); + let image = exr::image::Image::from_layer(Layer::new((2, 2), LayerAttributes::default(), Encoding::UNCOMPRESSED, channels)); + let mut bytes = std::io::Cursor::new(Vec::new()); + image.write().to_buffered(&mut bytes).unwrap(); + let mut bytes = bytes.into_inner(); + assert_eq!(crate::exr_channels::unsupported_compression(&bytes), None); + // The header's compression attribute (name, type, size 1, value) says HTJ2K32 (11). + let tag = b"compression\0compression\0\x01\0\0\0"; + let at = bytes.windows(tag.len()).position(|w| w == tag).unwrap() + tag.len(); + bytes[at] = 11; + let b: Arc<[u8]> = bytes.into(); + let e = crate::probe_bytes("/shot.exr", b.clone()).unwrap_err().to_string(); + assert!(e.contains("/shot.exr: OpenEXR HTJ2K compression isn't supported yet"), "{e}"); + let pool = MediaPool::new(); + pool.add_bytes("/shot.exr", b); + let f = Footage { path: "/shot.exr".into(), kind: FootageKind::Still, width: 2, height: 2, has_video: true, ..Default::default() }; + let e = pool.frame_at(&f, Tick::ZERO).unwrap_err().to_string(); + assert!(e.contains("HTJ2K compression isn't supported yet"), "{e}"); + } + /// #411: Interpret Footage ▸ Preserve RGB reads a float OpenEXR's values as the file stores /// them (no automatic linear → sRGB encode), so an OCIO effect is the only conversion; the /// default interpretation still encodes. Both interpretations of one file decode separately. diff --git a/crates/media/src/probe.rs b/crates/media/src/probe.rs index 4e986151..6e8718fd 100644 --- a/crates/media/src/probe.rs +++ b/crates/media/src/probe.rs @@ -158,6 +158,11 @@ pub(crate) fn still_footage(path: &str, w: u32, h: u32, fmt: image::ImageFormat, fn probe_still_bytes(path: &str, bytes: &[u8], fmt: image::ImageFormat) -> Result { use image::ImageDecoder; + if fmt == image::ImageFormat::OpenExr + && let Some(why) = crate::exr_channels::unsupported_compression(bytes) + { + return Err(MediaError::Decode(format!("{path}: {why}"))); + } let reader = image::ImageReader::with_format(std::io::Cursor::new(bytes), fmt); let (w, h, has_alpha) = match reader.into_decoder() { Ok(dec) => (dec.dimensions().0, dec.dimensions().1, dec.color_type().has_alpha()), diff --git a/crates/script/src/runtime.rs b/crates/script/src/runtime.rs index 62479c14..712999ee 100644 --- a/crates/script/src/runtime.rs +++ b/crates/script/src/runtime.rs @@ -152,7 +152,7 @@ pub(crate) fn exec(s: &mut Session, id: &str, params: J) -> Result { if CREATES_LAYER.contains(&id) { s.state.selected_layers.clear(); } - let r = s.execute(id, params); + let r = s.execute_for_script(id, params); if let Some(c) = target && prev != Some(c) && s.state.active_comp == Some(c) diff --git a/crates/script/src/tests.rs b/crates/script/src/tests.rs index 946db49d..b50a7bfb 100644 --- a/crates/script/src/tests.rs +++ b/crates/script/src/tests.rs @@ -362,6 +362,47 @@ fn security_gate_for_files() { let _ = std::fs::remove_dir_all(&dir); } +/// #590: a script can't give itself file and network access (or turn off the warning before +/// executing files) by changing Settings ▸ Scripting & Expressions; the user still can. +#[test] +fn scripts_cant_change_what_scripts_may_do() { + let mut s = session(); + let attempts = [ + "app.run('prefs.set', {key: 'scripting.allowScriptsWriteFiles', value: true})", + "app.run('prefs.set', {values: {'scripting.warnExecutingFiles': false}})", + "app.run('prefs.set', {key: 'scripting', value: {allowScriptsWriteFiles: true}})", + "app.run('engine.batch', {steps: [{command: 'prefs.set', params: {key: 'scripting.enableJsDebugger', value: true}}]})", + ]; + for code in attempts { + let o = run_code(&mut s, code, "grant.jsx"); + let e = o.error.unwrap_or_else(|| panic!("`{code}` was allowed")); + assert!(e.message.contains("Scripts can't change what scripts are allowed to do"), "{code}: {e:?}"); + let sc = &s.prefs.scripting; + assert!(!sc.allow_scripts_write_files && sc.warn_executing_files && !sc.enable_js_debugger, "{code}"); + } + assert!(run_code(&mut s, "new Socket()", "net.jsx").error.is_some()); + // Other settings, and the expression editor's, are still the script's to change. + ok(&mut s, "app.run('prefs.set', {values: {'general.undoLevels': 40, 'scripting.editorFontSize': 16}})"); + assert_eq!((s.prefs.general.undo_levels, s.prefs.scripting.editor_font_size), (40, 16)); + // The user grants access; a script's Reset Settings doesn't take it back or change it. + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": true})).unwrap(); + ok(&mut s, "app.run('prefs.reset', {})"); + assert!(s.prefs.scripting.allow_scripts_write_files); + assert_eq!(s.prefs.general.undo_levels, effectcraft_engine::prefs::Prefs::default().general.undo_levels); + // A command script file is a script too. + let dir = std::env::temp_dir().join(format!("ec-script-grant-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let f = dir.join("grant.json"); + std::fs::write(&f, r#"[{"command": "prefs.set", "params": {"key": "scripting.allowScriptsWriteFiles", "value": false}}]"#).unwrap(); + let e = s.execute("file.runScript", json!({"path": f.to_string_lossy()})).unwrap_err().to_string(); + assert!(e.contains("Scripts can't change"), "{e}"); + assert!(s.prefs.scripting.allow_scripts_write_files); + // The user (UI, CLI, control channel) can take it back. + s.execute("prefs.set", json!({"key": "scripting.allowScriptsWriteFiles", "value": false})).unwrap(); + assert!(!s.prefs.scripting.allow_scripts_write_files); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn engine_entry_points() { let mut s = session(); diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 55c75f0b..33fd676d 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -280,8 +280,11 @@ fn comp_content_with(project: &Project, comp: ItemId, addr: impl Fn(ItemId, &Arc h.finish() } -/// Hash of render options (part of [`FrameKey`] and of the disk key). +/// Hash of render options (part of [`FrameKey`] and of the disk key). The scale counts as the +/// key's thousandths ([`FrameKey::scale`]): a series rebuilt from the key (the cache bars) has +/// the same options as the frames rendered at 1/3 = 0.333… (#595). pub fn opts_hash(o: &RenderOpts) -> u64 { + let o = RenderOpts { scale: (o.scale * 1000.0).round() / 1000.0, ..*o }; let mut h = disk_cache::Hash128::default(); h.write(format!("{o:?}").as_bytes()); h.finish() as u64 diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6c4053c0..64997057 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -297,6 +297,17 @@ pub struct EffectcraftApp { pub(crate) template_list: Option<(Vec, Vec)>, } +/// The saved workspaces in the settings store ([`effectcraft_engine::config::ConfigStore`]). +pub const WORKSPACES_FILE: &str = "workspaces.json"; + +#[derive(serde::Serialize, serde::Deserialize)] +struct SavedWorkspaces { + #[serde(default)] + workspaces: std::collections::BTreeMap, + #[serde(default)] + floating: std::collections::BTreeMap>, +} + impl EffectcraftApp { pub fn new(session: Session) -> Self { EffectcraftApp { @@ -355,6 +366,34 @@ impl EffectcraftApp { template_list: None, } .with_ui_commands() + .with_saved_workspaces() + } + + /// The workspaces saved in an earlier session (Window ▸ Workspace ▸ Save as New Workspace, + /// Save Changes to this Workspace), from the settings store. A file that doesn't read is + /// ignored. + fn with_saved_workspaces(mut self) -> Self { + let text = self.session.config.as_ref().and_then(|c| c.read(WORKSPACES_FILE)); + match text.map(|t| serde_json::from_str::(&t)) { + Some(Ok(w)) => { + self.ui.saved_workspaces = w.workspaces; + self.ui.saved_floating = w.floating; + } + Some(Err(e)) => log::warn!("{WORKSPACES_FILE}: {e}"), + None => {} + } + self + } + + /// Keep the saved workspaces for the next session (see [`EffectcraftApp::new`]): After + /// Effects' saved workspaces last until they are deleted (#320). + pub(crate) fn store_saved_workspaces(&self) { + let Some(c) = &self.session.config else { return }; + let w = SavedWorkspaces { workspaces: self.ui.saved_workspaces.clone(), floating: self.ui.saved_floating.clone() }; + let written = serde_json::to_string(&w).map_err(std::io::Error::other).and_then(|t| c.write(WORKSPACES_FILE, &t)); + if let Err(e) = written { + log::warn!("{WORKSPACES_FILE}: {e}"); + } } /// Offer the frontend's commands (tools, timeline navigation…) for keyboard shortcuts. diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 03c12da7..cfbd6e52 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -954,6 +954,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu let name = app.ui.workspace.clone(); app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone()); + app.store_saved_workspaces(); json!({"workspace": name}) } "window.saveWorkspaceAs" => { @@ -970,6 +971,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu app.ui.saved_workspaces.insert(name.clone(), app.ui.dock.clone()); app.ui.saved_floating.insert(name.clone(), app.ui.floating.clone()); app.ui.workspace = name.clone(); + app.store_saved_workspaces(); json!({"workspace": name}) } "window.editWorkspaces" => { @@ -994,6 +996,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if app.ui.saved_workspaces.remove(&name).is_none() && !builtin { return Err(format!("no workspace `{name}`")); } + app.store_saved_workspaces(); if app.ui.workspace == name { app.set_workspace("Default"); } @@ -1011,6 +1014,7 @@ pub fn frontend(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, p: Valu if app.ui.workspace == name { app.ui.workspace = new.to_string(); } + app.store_saved_workspaces(); return Ok(json!({"renamed": new})); } json!({"workspaces": app.workspace_names()}) diff --git a/crates/ui-egui/tests/ui_panels.rs b/crates/ui-egui/tests/ui_panels.rs index 3b580921..ac84a550 100644 --- a/crates/ui-egui/tests/ui_panels.rs +++ b/crates/ui-egui/tests/ui_panels.rs @@ -128,6 +128,42 @@ fn hover(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { h.run_steps(3); } +/// Issue #320: saved workspaces last between sessions (they were only kept in memory): a new +/// app on the same settings store lists them and brings their layout back; renamed and deleted +/// ones stay that way. +#[test] +fn saved_workspaces_persist_across_sessions() { + use effectcraft_engine::config::{ConfigStore, MemoryConfig}; + use effectcraft_ui_egui::menus::invoke; + let store = std::sync::Arc::new(MemoryConfig::default()); + let launch = || { + let s = Session { config: Some(store.clone()), ..Default::default() }; + let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.run_steps(2); + h + }; + let mut h = launch(); + let ctx = h.ctx.clone(); + invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "Minimal"})).unwrap(); + invoke(h.state_mut(), &ctx, "window.saveWorkspaceAs", json!({"name": "My Layout"})).unwrap(); + invoke(h.state_mut(), &ctx, "window.saveWorkspaceAs", json!({"name": "Scratch"})).unwrap(); + let saved = h.state().ui.dock.clone(); + drop(h); + let mut h = launch(); + assert_eq!(h.state().saved_workspace_names(), ["My Layout", "Scratch"]); + assert_eq!(h.state().ui.workspace, "Default"); + let ctx = h.ctx.clone(); + invoke(h.state_mut(), &ctx, "window.workspace", json!({"name": "My Layout"})).unwrap(); + assert_eq!(h.state().ui.dock, saved); + invoke(h.state_mut(), &ctx, "window.editWorkspaces", json!({"name": "Scratch", "delete": true})).unwrap(); + invoke(h.state_mut(), &ctx, "window.editWorkspaces", json!({"name": "My Layout", "rename": "Edit Bay"})).unwrap(); + drop(h); + assert_eq!(launch().state().saved_workspace_names(), ["Edit Bay"]); + // A damaged file is ignored. + store.write(effectcraft_ui_egui::WORKSPACES_FILE, "{not json").unwrap(); + assert!(launch().state().saved_workspace_names().is_empty()); +} + /// Issue #191: a workspace saved with Save as New Workspace is listed in Window ▸ Workspace /// (below the built-ins, where the submenu used to be cut off) and choosing it there brings its /// layout back. diff --git a/crates/ui-egui/tests/ui_ram_preview.rs b/crates/ui-egui/tests/ui_ram_preview.rs index 84610b7c..5459be8c 100644 --- a/crates/ui-egui/tests/ui_ram_preview.rs +++ b/crates/ui-egui/tests/ui_ram_preview.rs @@ -125,3 +125,23 @@ fn prefetch_waits_while_edits_keep_coming() { let series = h.state().shown_series(ItemId(c)); assert!(h.state().frames.cached_frames(&series).len() > 1, "prefetch resumed"); } + +/// #595: at Third (and other resolutions that aren't a whole number of thousandths) the cache +/// bars find the rendered frames. The series they count was rebuilt from the frame key's scale +/// (333 → 0.333), whose render options hashed apart from the frames rendered at 0.3333…. +#[test] +fn cached_frames_are_counted_at_one_third_resolution() { + use effectcraft_ui_egui::state::Resolution; + for res in [Resolution::Third, Resolution::Custom(7)] { + let mut s = Session::default(); + let c = s.execute("comp.new", json!({"name": "Third", "width": 64, "height": 36, "duration": 1})).unwrap()["comp"].as_u64().unwrap(); + s.execute("layer.newSolid", json!({"color": "#3080ff"})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_| EffectcraftApp::new(s)); + h.state_mut().ui.viewer.res = res; + settle(&mut h); + let series = h.state().shown_series(ItemId(c)); + assert_eq!(series.scale, (res.scale(1.0, 1.0) * 1000.0).round() as u32, "{res:?}"); + assert!(shown_cached(&h, c), "{res:?}: the shown frame counts as cached"); + assert!(!h.state().frames.cached_frames(&series).is_empty(), "{res:?}"); + } +}