Fix #1743 (follow-up): commands that write to a given path never replace footage, a proxy or the project file

Session::write_output (and refuse_export) is the one place that checks; Save Frame As
(PNG/PSD/EXR), Export Lottie, Save a Copy (+XML), Collect Files, Export Timeline (and its
pre-rendered movies), Export Template, Save Animation Preset, Export Guides and Export
Shortcuts write through it. The control channel's render.frame/ui.screenshot, MCP
render_frame and CLI render-frame --out check first (new query file.checkOutput for the
bridged cases). Save/Save As/auto-save are untouched.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Kristian Myram
2026-10-30 08:06:48 +01:00
co-authored by Claude Sonnet 5.5
parent 1f15f8a53b
commit d2930b2554
23 changed files with 767 additions and 72 deletions
+2
View File
@@ -726,6 +726,8 @@ fn run(cmd: &str, args: &Args, json_out: bool) -> Result<(), Failure> {
}
}
let out = args.opt("--out").unwrap_or("frame.png").to_string();
// Never over footage, a proxy or the project file (checked before rendering).
b.check_output(&out)?;
let t0 = std::time::Instant::now();
let f = b.render_with(comp.as_ref(), time, max_side, args.flag("--transparent"))?;
let ms = t0.elapsed().as_secs_f64() * 1000.0;
+29
View File
@@ -419,6 +419,35 @@ fn render_out_is_relative_to_the_working_directory() {
let _ = std::fs::remove_dir_all(&root);
}
/// #1743: `render-frame --out` writes a PNG to the path it is given, and never over footage or
/// the project file.
#[test]
fn render_frame_never_writes_over_footage_or_the_project() {
let root = tmp("render-frame-overwrite");
let _ = std::fs::remove_dir_all(&root);
std::fs::create_dir_all(&root).unwrap();
let (clip, project) = (root.join("clip.png"), root.join("p.ecproj"));
let (c, p) = (clip.to_str().unwrap(), project.to_str().unwrap());
// A real PNG to import: a frame of the demo project.
ok_json(&["render-frame", "--max-side", "64", "--out", c]);
let imported = json!({"paths": [c]}).to_string();
ok_json(&["run", "file.import", &imported, "comp.new", r#"{"name":"T","width":64,"height":36,"duration":1}"#, "--empty", "--save-as", p]);
let (footage, saved) = (std::fs::read(&clip).unwrap(), std::fs::read(&project).unwrap());
for (out, what) in [(c, "footage"), (p, "project")] {
let (code, v) = run_json(&["render-frame", "--project", p, "--out", out]);
assert_eq!(code, 1, "{what}: {v}");
let error = v["error"].as_str().unwrap();
assert!(error.contains(out) && error.contains(what), "{what}: {v}");
}
assert_eq!(std::fs::read(&clip).unwrap(), footage, "the footage is as it was");
assert_eq!(std::fs::read(&project).unwrap(), saved, "the project file is as it was");
// Anywhere else it writes.
let free = root.join("frame.png");
ok_json(&["render-frame", "--project", p, "--out", free.to_str().unwrap()]);
assert_eq!(&std::fs::read(&free).unwrap()[..8], b"\x89PNG\r\n\x1a\n");
let _ = std::fs::remove_dir_all(&root);
}
/// A real stdio client: each edit's reply is read before disconnecting or killing the server.
struct McpChild {
child: std::process::Child,
+13
View File
@@ -85,6 +85,19 @@ impl Backend {
}
}
/// Refuse `path`, a file this caller is about to write itself (a frame, a screenshot), when
/// the open project reads it: footage, a proxy or the project file. The project is the
/// session's or the app's, so both are asked through `file.checkOutput`; a relative path is
/// made absolute here, against this process's folder, because the app has its own.
pub fn check_output(&mut self, path: &str) -> Result<()> {
let absolute = std::path::absolute(path).map_err(|e| Error::BadArgs(format!("cannot resolve {path}: {e}")))?;
let found = self.exec("file.checkOutput", json!({"path": absolute.to_string_lossy()}))?;
match found.get("blocked").and_then(Value::as_bool) {
Some(false) => Ok(()),
_ => Err(Error::BadArgs(found.get("reason").and_then(Value::as_str).unwrap_or("this file is one the project reads").to_string())),
}
}
/// Call a raw control-channel method (bridge only).
pub fn control(&mut self, method: &str, params: Value) -> Result<Value> {
match &mut self.target {
+105
View File
@@ -1615,3 +1615,108 @@ fn blender_export_requires_an_absolute_path_in_a_write_root() {
}
std::fs::remove_dir_all(&base).unwrap();
}
/// `render_frame` writes the frame to the `path` it is given, and never over a file the open
/// project reads (#1743): footage, a proxy or the project file. The refusal comes before the
/// frame is rendered, and the file is untouched.
#[test]
fn render_frame_never_writes_over_a_file_the_project_reads() {
use effectcraft_engine::{
color::Label,
project::{Footage, FootageKind, ItemKind},
};
use std::sync::Arc;
let dir = std::env::temp_dir().join(format!("effectcraft-mcp-overwrite-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let (clip, project) = (dir.join("clip.png"), dir.join("Project.ecproj"));
std::fs::write(&clip, b"SOURCE FOOTAGE").unwrap();
let mut session = Session::default();
session.execute("comp.new", json!({"width": 16, "height": 16})).unwrap();
session.execute("file.saveAs", json!({"path": project.to_string_lossy()})).unwrap();
Arc::make_mut(&mut session.project).add_item(
"clip.png",
Label::Aqua,
None,
ItemKind::Footage(Footage {
path: clip.to_string_lossy().into_owned(),
kind: FootageKind::Still,
width: 16,
height: 16,
has_video: true,
..Default::default()
}),
);
let saved = std::fs::read(&project).unwrap();
let mut s = McpServer::new(Backend::headless(session));
for (file, what) in [(&clip, "footage"), (&project, "project")] {
let (c, err) = call(&mut s, "render_frame", json!({"path": file.to_string_lossy(), "inline": false}));
let text = c[0]["text"].as_str().unwrap();
assert!(err && text.contains(&*file.to_string_lossy()) && text.contains(what), "{what}: {text}");
}
assert_eq!(std::fs::read(&clip).unwrap(), b"SOURCE FOOTAGE", "the footage is as it was");
assert_eq!(std::fs::read(&project).unwrap(), saved, "the project file is as it was");
// Anywhere else it writes the PNG.
let free = dir.join("frame.png");
let (c, err) = call(&mut s, "render_frame", json!({"path": free.to_string_lossy(), "inline": false}));
assert!(!err, "{c:?}");
assert_eq!(&std::fs::read(&free).unwrap()[..8], b"\x89PNG\r\n\x1a\n");
let _ = std::fs::remove_dir_all(&dir);
}
/// The same through the desktop app: the app knows the project's files, so the tool asks it
/// (`file.checkOutput`) before it writes anything.
#[test]
fn bridged_render_frame_asks_the_app_before_it_writes() {
use effectcraft_engine::project::{Footage, FootageKind};
struct Probe;
impl effectcraft_engine::Importer for Probe {
fn probe(&self, path: &str) -> Result<Footage, String> {
Ok(Footage { path: path.to_string(), kind: FootageKind::Still, width: 4, height: 4, has_video: true, ..Default::default() })
}
}
let dir = std::env::temp_dir().join(format!("effectcraft-mcp-bridge-overwrite-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
let clip = dir.join("clip.png");
std::fs::write(&clip, b"SOURCE FOOTAGE").unwrap();
let l = TcpListener::bind("127.0.0.1:0").unwrap();
let port = l.local_addr().unwrap().port();
std::thread::spawn(move || {
let (stream, _) = l.accept().unwrap();
let mut out = stream.try_clone().unwrap();
let mut session = Session { importer: Some(std::sync::Arc::new(Probe)), ..Default::default() };
for line in BufReader::new(stream).lines() {
let msg: Value = serde_json::from_str(&line.unwrap()).unwrap();
let p = &msg["params"];
let mut reply = match msg["method"].as_str().unwrap() {
"engine.execute" => match session.execute(p["command"].as_str().unwrap(), p["params"].clone()) {
Ok(v) => json!({"ok": true, "result": v}),
Err(e) => json!({"ok": false, "error": e.to_string()}),
},
"render.frame" => {
let png = crate::encode_png(4, 2, vec![255; 32], 0).unwrap();
json!({"ok": true, "result": {"comp": 1, "time": 0.5, "width": 4, "height": 2, "png": base64::encode(&png)}})
}
m => json!({"ok": false, "error": format!("unknown method `{m}`")}),
};
reply["id"] = msg["id"].clone();
writeln!(out, "{reply}").unwrap();
}
});
let mut s = McpServer::new(Backend::bridge(&port.to_string()).unwrap());
call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"width": 16, "height": 16}}));
call_json(&mut s, "execute_command", json!({"command": "file.import", "params": {"paths": [clip.to_string_lossy()]}}));
let (c, err) = call(&mut s, "render_frame", json!({"path": clip.to_string_lossy(), "inline": false}));
let text = c[0]["text"].as_str().unwrap();
assert!(err && text.contains("clip.png") && text.contains("footage"), "{text}");
assert_eq!(std::fs::read(&clip).unwrap(), b"SOURCE FOOTAGE", "the footage is as it was");
// Anywhere else it writes the PNG.
let free = dir.join("frame.png");
let (c, err) = call(&mut s, "render_frame", json!({"path": free.to_string_lossy(), "inline": false}));
assert!(!err, "{c:?}");
assert_eq!(&std::fs::read(&free).unwrap()[..8], b"\x89PNG\r\n\x1a\n");
let _ = std::fs::remove_dir_all(&dir);
}
+8 -3
View File
@@ -406,12 +406,17 @@ fn add_keyframe(b: &mut Backend, a: &Value) -> Result<Reply> {
fn render_frame(b: &mut Backend, a: &Value) -> Result<Reply> {
let max_side = get(a, "max_side").and_then(Value::as_u64).unwrap_or(960) as u32;
let transparent = get(a, "transparent").and_then(Value::as_bool).unwrap_or(false);
let f = b.render_with(get(a, "comp"), get(a, "time").and_then(Value::as_f64), max_side, transparent)?;
let mut info = json!({"comp": f.comp, "time": f.time, "width": f.width, "height": f.height});
if let Some(p) = get(a, "path").and_then(Value::as_str) {
let path = get(a, "path").and_then(Value::as_str);
if let Some(p) = path {
if let Some(c) = b.confinement() {
c.check(p, crate::confine::Access::Write)?;
}
// Before the frame is rendered: never over footage, a proxy or the project file.
b.check_output(p)?;
}
let f = b.render_with(get(a, "comp"), get(a, "time").and_then(Value::as_f64), max_side, transparent)?;
let mut info = json!({"comp": f.comp, "time": f.time, "width": f.width, "height": f.height});
if let Some(p) = path {
std::fs::write(p, &f.png).map_err(|e| Error::Other(format!("cannot write {p}: {e}")))?;
info["path"] = json!(p);
}
+2 -1
View File
@@ -67,6 +67,7 @@ fn earliest_key<'a>(groups: impl IntoIterator<Item = &'a PropGroup>) -> Option<T
fn save_preset(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("anim.savePreset", "missing `path`"))?.to_string();
s.refuse_export(&path)?;
let comp = s.active_comp().ok_or(EngineError::NoComp)?;
let t = s.time();
let mut effects: Vec<(LayerId, PropGroup)> = selected_effects(s);
@@ -106,7 +107,7 @@ fn save_preset(s: &mut Session, p: &Value) -> Result<Value> {
.unwrap_or_else(|| std::path::Path::new(&path).file_stem().map(|s| s.to_string_lossy().to_string()).unwrap_or_default());
let doc = json!({PRESET_TAG: PRESET_VERSION, "name": name, "properties": props_json, "effects": fx_json});
let text = serde_json::to_string_pretty(&doc).unwrap_or_default();
s.services.write_file(&path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, text.as_bytes())?;
Ok(json!({"path": path, "properties": props.len(), "effects": effects.len()}))
}
+2 -1
View File
@@ -152,8 +152,9 @@ fn clear_recent_presets(s: &mut Session, _: &Value) -> Result<Value> {
fn save_copy_xml(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("file.saveCopyAsXml", "missing `path` (.ecprojx)"))?;
s.refuse_export(path)?;
let xml = crate::xml_project::to_xml(&s.project.to_file_json_with_view(&s.saved_view(), false)?).map_err(EngineError::Other)?;
s.services.write_file(path, xml.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(path, xml.as_bytes())?;
s.toast(format!("Saved an XML copy to {path}"));
Ok(json!({"path": path, "bytes": xml.len()}))
}
+2 -1
View File
@@ -275,6 +275,7 @@ fn save_frame(s: &mut Session, p: &Value) -> Result<Value> {
return queue_frame(s, p);
}
let path = str_p(p, "path").ok_or_else(|| bad("comp.saveFrameAs", "missing `path` (.png)"))?.to_string();
s.refuse_export(&path)?;
let cid = comp_id(s, p)?;
let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?;
let t = f_p(p, "time").map(Tick::from_seconds_f64).unwrap_or(s.time());
@@ -283,7 +284,7 @@ fn save_frame(s: &mut Session, p: &Value) -> Result<Value> {
let img = s.render(cid, t, effectcraft_render::RenderOpts { scale, backend: effectcraft_render::Backend::Auto, ..Default::default() });
let rgba = if b_p(p, "transparent") == Some(false) { img.to_rgba8_over(bg) } else { img.to_rgba8() };
let png = encode_png(&rgba, img.width, img.height).map_err(|e| EngineError::Other(format!("PNG encoding failed: {e}")))?;
s.services.write_file(&path, &png).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, &png)?;
s.toast(format!("Saved frame to {path}"));
Ok(json!({"path": path, "width": img.width, "height": img.height, "time": t.seconds()}))
}
+2 -1
View File
@@ -807,6 +807,7 @@ fn export_template(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "essential.exportTemplate";
let cid = eg_comp(s, p)?;
let path = str_p(p, "path").ok_or_else(|| bad(C, "missing `path` (.ectemplate)"))?.to_string();
s.refuse_export(&path)?;
let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?.clone();
let eg = comp.essential.clone().filter(|e| !e.controls.is_empty()).ok_or_else(|| bad(C, "the composition has no Essential Graphics controls"))?;
let comp_name = s.project.item(cid).map(|i| i.name.clone()).unwrap_or_default();
@@ -868,7 +869,7 @@ fn export_template(s: &mut Session, p: &Value) -> Result<Value> {
entries.insert(0, ("project.ecproj".into(), sub.to_json().into_bytes()));
entries.insert(0, ("manifest.json".into(), serde_json::to_vec_pretty(&manifest).unwrap_or_default()));
let bytes = effectcraft_lottie::zip::store(&entries);
s.services.write_file(&path, &bytes).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, &bytes)?;
s.toast(format!("Exported template “{name}” to {path}"));
Ok(json!({"path": path, "name": name, "bytes": bytes.len(), "controls": eg.flat().len(), "items": deps.len()}))
}
+77 -38
View File
@@ -11,7 +11,7 @@ use serde_json::{Value, json};
use super::{CommandSpec, always, b_p, bad, f_p, has_comp, has_project_selection, str_p};
use crate::sequence::SequenceOptions;
use crate::{EngineError, Result, Session, cmd};
use crate::{EngineError, Result, Session, cmd, query};
fn has_footage_selection(s: &Session) -> std::result::Result<(), String> {
if selected_footage(s).is_empty() { Err("select footage in the Project panel".into()) } else { Ok(()) }
@@ -78,11 +78,19 @@ fn close_project(s: &mut Session, _: &Value) -> Result<Value> {
fn save_copy(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("file.saveCopy", "missing `path`"))?;
s.refuse_export(path)?;
let json = super::file::file_text(s, path)?;
s.services.write_file(path, json.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(path, json.as_bytes())?;
Ok(json!({"path": path, "bytes": json.len()}))
}
/// `file.checkOutput {path}`: the commands that write to a path never replace footage, a proxy
/// or the project file; this says beforehand, for callers that write the file themselves.
fn check_output(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("file.checkOutput", "missing `path`"))?;
Ok(s.output_status(path))
}
fn rate_of(p: &Value) -> FrameRate {
f_p(p, "frameRate").map(|f| limits::frame_rate(FrameRate::from_f64(f), FrameRate::FPS_29_97)).unwrap_or(FrameRate::FPS_29_97)
}
@@ -376,41 +384,6 @@ fn find_missing(s: &mut Session, p: &Value) -> Result<Value> {
fn collect_files(s: &mut Session, p: &Value) -> Result<Value> {
let folder = str_p(p, "folder").ok_or_else(|| bad("file.collectFiles", "missing `folder`"))?.trim_end_matches('/').to_string();
// Collect Files makes its folder (and the footage folder inside it).
#[cfg(not(target_arch = "wasm32"))]
std::fs::create_dir_all(format!("{folder}/(Footage)")).map_err(|e| EngineError::Other(format!("cannot create {folder}: {e}")))?;
let mut proj = (*s.project).clone();
let mut copied = 0;
let mut errors = vec![];
let mut used_names = BTreeSet::new();
for it in proj.items.values_mut() {
let ItemKind::Footage(f) = &mut it.kind else { continue };
if f.path.is_empty() {
continue;
}
let files: Vec<String> = if f.sequence.is_empty() { vec![f.path.clone()] } else { f.sequence.clone() };
let mut new_paths = vec![];
for src in &files {
let mut name = std::path::Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "footage".into());
let mut k = 2;
while !used_names.insert(name.clone()) {
name = format!("{k}_{name}");
k += 1;
}
let dst = format!("{folder}/(Footage)/{name}");
match s.services.read_file(src).and_then(|bytes| s.services.write_file(&dst, &bytes)) {
Ok(()) => copied += 1,
Err(e) => errors.push(format!("{src}: {e}")),
}
new_paths.push(dst);
}
if f.sequence.is_empty() {
f.path = new_paths.remove(0);
} else {
f.path = new_paths.first().cloned().unwrap_or_default();
f.sequence = new_paths;
}
}
let stem = s
.path
.as_deref()
@@ -418,7 +391,67 @@ fn collect_files(s: &mut Session, p: &Value) -> Result<Value> {
.map(|n| n.to_string_lossy().to_string())
.unwrap_or_else(|| "Untitled Project".into());
let out = format!("{folder}/{stem}.ecproj");
s.services.write_file(&out, proj.to_json().as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {out}: {e}")))?;
// Where every copy goes, before anything is made: a copy never lands on a file the project
// reads (other footage, a proxy, the project file itself), and then nothing is written.
let protected = s.protected_files();
let mut used_names = BTreeSet::new();
let mut plan: Vec<(ItemId, Vec<(String, String)>)> = vec![];
for it in s.project.items.values() {
let ItemKind::Footage(f) = &it.kind else { continue };
if f.path.is_empty() {
continue;
}
let files: Vec<&String> = if f.sequence.is_empty() { vec![&f.path] } else { f.sequence.iter().collect() };
let mut pairs = vec![];
for src in files {
let mut name = std::path::Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "footage".into());
let mut k = 2;
while !used_names.insert(name.clone()) {
name = format!("{k}_{name}");
k += 1;
}
let dst = format!("{folder}/(Footage)/{name}");
// A file that is already where its copy goes stays (it is not copied onto itself).
if !crate::output_guard::same_file(std::path::Path::new(src), std::path::Path::new(&dst))
&& let Some(source) = protected.find(std::path::Path::new(&dst))
{
return Err(EngineError::Other(format!("Nothing was collected. {}", source.refusal(&dst, crate::output_guard::Use::Export))));
}
pairs.push((src.clone(), dst));
}
plan.push((it.id, pairs));
}
if let Some(source) = protected.find(std::path::Path::new(&out)) {
return Err(EngineError::Other(format!("Nothing was collected. {}", source.refusal(&out, crate::output_guard::Use::Export))));
}
// Collect Files makes its folder (and the footage folder inside it).
#[cfg(not(target_arch = "wasm32"))]
std::fs::create_dir_all(format!("{folder}/(Footage)")).map_err(|e| EngineError::Other(format!("cannot create {folder}: {e}")))?;
let mut proj = (*s.project).clone();
let mut copied = 0;
let mut errors = vec![];
for (id, pairs) in plan {
let mut new_paths = vec![];
for (src, dst) in &pairs {
if crate::output_guard::same_file(std::path::Path::new(src), std::path::Path::new(dst)) {
copied += 1;
} else {
match s.services.read_file(src).and_then(|bytes| s.write_output_with(&protected, dst, &bytes).map_err(std::io::Error::other)) {
Ok(()) => copied += 1,
Err(e) => errors.push(format!("{src}: {e}")),
}
}
new_paths.push(dst.clone());
}
let Some(ItemKind::Footage(f)) = proj.items.get_mut(&id).map(|it| &mut it.kind) else { continue };
if f.sequence.is_empty() {
f.path = new_paths.pop().unwrap_or_default();
} else {
f.path = new_paths.first().cloned().unwrap_or_default();
f.sequence = new_paths;
}
}
s.write_output_with(&protected, &out, proj.to_json().as_bytes())?;
s.toast(format!("Collected {copied} file(s) into {folder}"));
Ok(json!({"project": out, "copied": copied, "errors": errors}))
}
@@ -1173,6 +1206,12 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!("file.close", "Close", ["File"], Some("Cmd+W"), "{comp?}", has_comp, close),
cmd!("file.closeProject", "Close Project", ["File"], None, "{}", always, close_project),
cmd!("file.saveCopy", "Save a Copy...", ["File", "Save As"], None, "{path}", always, save_copy),
query!(
"file.checkOutput",
"Check Output File",
"{path} → {path, blocked, source?: footage|proxy|project, reason?}: whether an export may be written to path (it may not replace a file the project reads)",
check_output
),
cmd!("file.importMultiple", "Multiple Files...", ["File", "Import"], Some("Cmd+Alt+I"), "{paths: [string]}", always, |s, p| s
.execute("file.import", p.clone())),
cmd!(
+3 -2
View File
@@ -195,6 +195,7 @@ pub fn layered_exr(s: &Session, cid: ItemId, t: Tick, scale: f64) -> Result<(Vec
fn frame_args(s: &Session, p: &Value, cmd: &str, ext: &str) -> Result<(String, ItemId, Tick, f64)> {
let path = str_p(p, "path").ok_or_else(|| bad(cmd, format!("missing `path` (.{ext})")))?.to_string();
s.refuse_export(&path)?;
let cid = comp_id(s, p)?;
let t = f_p(p, "time").map(Tick::from_seconds_f64).unwrap_or(s.time());
let scale = f_p(p, "scale").unwrap_or(1.0).clamp(0.01, 4.0);
@@ -204,7 +205,7 @@ fn frame_args(s: &Session, p: &Value, cmd: &str, ext: &str) -> Result<(String, I
fn save_psd(s: &mut Session, p: &Value) -> Result<Value> {
let (path, cid, t, scale) = frame_args(s, p, "comp.saveFrameAsPsd", "psd")?;
let (bytes, n, w, h) = layered_psd(s, cid, t, scale)?;
s.services.write_file(&path, &bytes).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, &bytes)?;
s.toast(format!("Saved {n} layers to {path}"));
Ok(json!({"path": path, "layers": n, "width": w, "height": h, "time": t.seconds()}))
}
@@ -212,7 +213,7 @@ fn save_psd(s: &mut Session, p: &Value) -> Result<Value> {
fn save_exr(s: &mut Session, p: &Value) -> Result<Value> {
let (path, cid, t, scale) = frame_args(s, p, "comp.saveFrameAsExr", "exr")?;
let (bytes, channels, w, h) = layered_exr(s, cid, t, scale)?;
s.services.write_file(&path, &bytes).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, &bytes)?;
s.toast(format!("Saved a multi-layer EXR to {path}"));
Ok(json!({"path": path, "channels": channels, "width": w, "height": h, "time": t.seconds()}))
}
+2 -1
View File
@@ -8,6 +8,7 @@ use crate::{EngineError, Result, Session, cmd};
fn export_lottie(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("file.exportLottie", "missing `path`"))?.to_string();
s.refuse_export(&path)?;
let cid = comp_id(s, p)?;
let opts = ExportOptions { include_expressions: b_p(p, "includeExpressions").unwrap_or(false), text_as_shapes: b_p(p, "textAsShapes").unwrap_or(false) };
let services = s.services.clone();
@@ -20,7 +21,7 @@ fn export_lottie(s: &mut Session, p: &Value) -> Result<Value> {
} else {
res.to_string_compact().into_bytes()
};
s.services.write_file(&path, &bytes).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(&path, &bytes)?;
let n = res.warnings.len();
s.toast(if n == 0 { format!("Exported Lottie to {path}") } else { format!("Exported Lottie to {path} ({n} warning{})", if n == 1 { "" } else { "s" }) });
Ok(json!({"path": path, "bytes": bytes.len(), "warnings": res.warnings}))
+1 -1
View File
@@ -195,7 +195,7 @@ fn sc_export(s: &mut Session, p: &Value) -> Result<Value> {
let doc = s.keymaps.export(str_p(p, "preset")).map_err(|e| bad("shortcuts.export", e))?;
if let Some(path) = str_p(p, "path") {
let text = serde_json::to_string_pretty(&doc).unwrap_or_default();
s.services.write_file(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(path, text.as_bytes())?;
return Ok(json!({"path": path, "preset": doc["name"]}));
}
Ok(doc)
+11
View File
@@ -99,6 +99,7 @@ fn isolate(project: &Project, cid: ItemId, lid: LayerId) -> Option<Project> {
/// What a pre-render needs from the session, so it can run on a background thread.
struct PrerenderCx {
protected: crate::output_guard::Protected,
exporter: Option<Arc<dyn Exporter>>,
footage: Arc<dyn effectcraft_render::FootageSource>,
expr: Option<Arc<dyn effectcraft_render::ExprHost>>,
@@ -157,6 +158,7 @@ fn export_timeline(s: &mut Session, p: &Value) -> Result<Value> {
}
path = format!("{path}.{}", format.extension());
}
s.refuse_export(&path)?;
let cid = comp_id(s, p)?;
let prerender_mode = match str_p(p, "prerender") {
None => match p.get("prerender").and_then(Value::as_bool) {
@@ -178,6 +180,7 @@ fn export_timeline(s: &mut Session, p: &Value) -> Result<Value> {
let project = s.project.clone();
let services = s.services.clone();
let cx = PrerenderCx {
protected: crate::output_guard::Protected::of(s),
exporter: s.exporter.clone(),
footage: s.footage.clone(),
expr: s.expr.clone(),
@@ -189,6 +192,9 @@ fn export_timeline(s: &mut Session, p: &Value) -> Result<Value> {
let wait = !b_p(p, "background").unwrap_or(false);
s.spawn_task("exportTimeline", format!("Export {}", format.label()), wait, move |ctl| {
let (bytes, mut result) = export_timeline_work(&cx, &project, cid, &opts, &dir, &stem, ctl)?;
if let Some(source) = cx.protected.find(std::path::Path::new(&path)) {
return Err(source.refusal(&path, crate::output_guard::Use::Export));
}
services.write_file(&path, &bytes).map_err(|e| format!("cannot write {path}: {e}"))?;
result["path"] = json!(path);
result["toast"] = json!(match format {
@@ -220,6 +226,11 @@ fn export_timeline_work(
let layer_name = project.comp(c).and_then(|cc| cc.layer(l)).map(|x| x.name.clone()).unwrap_or_default();
ctl.message(format!("Pre-render {layer_name}"));
let file = dir.join(format!("{stem}_{}_{}_{}.mov", safe(&comp_name), safe(&layer_name), l.0)).to_string_lossy().to_string();
// The movies are outputs too: never over footage, a proxy or the project file.
if let Some(source) = cx.protected.find(std::path::Path::new(&file)) {
errors.push(format!("{comp_name} ▸ {layer_name}: {}", source.refusal(&file, crate::output_guard::Use::Export)));
continue;
}
// Progress over all the layers, each one's frames a share of it.
let mut progress = |done: u64, total: u64| ctl.progress(i * 1000 + done.saturating_mul(1000) / total.max(1), n * 1000);
match prerender(cx, project, c, l, &file, &mut progress) {
+2 -1
View File
@@ -83,10 +83,11 @@ fn clear_guides(s: &mut Session, p: &Value) -> Result<Value> {
fn export_guides(s: &mut Session, p: &Value) -> Result<Value> {
let path = str_p(p, "path").ok_or_else(|| bad("view.exportGuides", "missing `path`"))?;
s.refuse_export(path)?;
let comp = s.project.comp(comp_id(s, p)?).ok_or(EngineError::NoComp)?;
let doc = json!({"effectcraftGuides": 1, "width": comp.width, "height": comp.height, "guides": comp.guides});
let text = serde_json::to_string_pretty(&doc).unwrap_or_default();
s.services.write_file(path, text.as_bytes()).map_err(|e| EngineError::Other(format!("cannot write {path}: {e}")))?;
s.write_output(path, text.as_bytes())?;
Ok(json!({"path": path, "guides": comp.guides.len()}))
}
+83 -8
View File
@@ -5,6 +5,11 @@
//! exporter. Existing aliases are compared by native file identity, including hard links.
//! Missing sources remain protected by their normalized path. Checks happen before dispatch;
//! filesystem changes between the check and the write require exporter-side revalidation.
//!
//! The commands that write to a path they are given (Save Frame As, Export Lottie, Save a Copy,
//! Collect Files, Export Timeline…) never replace one of those files either, with no flag:
//! they write through [`Session::write_output`], the one place that checks. Save and Save As
//! write the project's own file and do not.
use std::collections::{HashMap, HashSet};
use std::path::{Component, Path, PathBuf};
@@ -12,6 +17,7 @@ use std::sync::Arc;
use effectcraft_project::render_queue::{RenderQueueItem, log_path, numbered_template, sequence_frame, sequence_path};
use effectcraft_project::{Footage, ItemKind};
use serde_json::{Value, json};
use crate::Session;
@@ -26,6 +32,15 @@ const PROBE_LIMIT: u64 = 100_000;
/// render time.
const MAX_NUMBER: u32 = 1_000;
/// What a file is written for, which words a refusal.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) enum Use {
/// A Render Queue output (or a preview render): the fix is Output To.
Render,
/// Anything else written to a path: a frame, an export, a copy.
Export,
}
/// What reads a file the project depends on.
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum Source {
@@ -38,12 +53,28 @@ pub(crate) enum Source {
}
impl Source {
/// Why `path`, a file this reads, cannot be a render's output.
pub(crate) fn refusal(&self, path: &str) -> String {
/// `footage`, `proxy` or `project`: the `source` of a blocked output.
pub(crate) fn kind(&self) -> &'static str {
match self {
Source::Footage(name) => format!("{path} is the footage “{name}” of this project, and a render never replaces footage; choose another Output To"),
Source::Proxy(name) => format!("{path} is the proxy of “{name}”, and a render never replaces a proxy; choose another Output To"),
Source::Project => format!("{path} is this project's own file, and a render never replaces it; choose another Output To"),
Source::Footage(_) => "footage",
Source::Proxy(_) => "proxy",
Source::Project => "project",
}
}
/// Why `path`, a file this reads, cannot be written for `used`.
pub(crate) fn refusal(&self, path: &str, used: Use) -> String {
match (used, self) {
(Use::Render, Source::Footage(name)) => {
format!("{path} is the footage “{name}” of this project, and a render never replaces footage; choose another Output To")
}
(Use::Render, Source::Proxy(name)) => format!("{path} is the proxy of “{name}”, and a render never replaces a proxy; choose another Output To"),
(Use::Render, Source::Project) => format!("{path} is this project's own file, and a render never replaces it; choose another Output To"),
(Use::Export, Source::Footage(name)) => {
format!("{path} is the footage “{name}” of this project, and footage is never written over; choose another file")
}
(Use::Export, Source::Proxy(name)) => format!("{path} is the proxy of “{name}”, and a proxy is never written over; choose another file"),
(Use::Export, Source::Project) => format!("{path} is this project's own file, which only Save writes; choose another file"),
}
}
}
@@ -178,8 +209,8 @@ fn fold(name: &str) -> String {
if IGNORES_CASE { name.to_lowercase() } else { name.to_string() }
}
#[cfg(test)]
fn same_file(a: &Path, b: &Path) -> bool {
/// Whether two paths are one file: the same name once resolved, or the same file on disk.
pub(crate) fn same_file(a: &Path, b: &Path) -> bool {
if key(a) == key(b) {
return true;
}
@@ -241,6 +272,50 @@ impl Session {
Protected::of(self)
}
/// What reads the file at `path`, if the project does.
fn blocking(&self, path: &str) -> Option<Source> {
if cfg!(target_arch = "wasm32") {
return None;
}
self.protected_files().find(Path::new(path)).cloned()
}
/// `Err` when `path` is a file the project reads (footage, a proxy, the project file) and so
/// is not for an export to replace. For callers that write the file themselves; call it
/// before the work, not after it.
pub fn refuse_export(&self, path: &str) -> crate::Result<()> {
match self.blocking(path) {
Some(source) => Err(crate::EngineError::Other(source.refusal(path, Use::Export))),
None => Ok(()),
}
}
/// Write `bytes` to `path` through the host's services, unless the project reads the file
/// there ([`Session::refuse_export`]). Every command that writes to a path it is given
/// writes through this; Save and Save As (the project's own file) do not.
pub fn write_output(&self, path: &str, bytes: &[u8]) -> crate::Result<()> {
self.write_output_with(&self.protected_files(), path, bytes)
}
/// [`Session::write_output`] against files already found ([`Session::protected_files`]),
/// for a command that writes many.
pub(crate) fn write_output_with(&self, protected: &Protected, path: &str, bytes: &[u8]) -> crate::Result<()> {
if !cfg!(target_arch = "wasm32")
&& let Some(source) = protected.find(Path::new(path))
{
return Err(crate::EngineError::Other(source.refusal(path, Use::Export)));
}
self.services.write_file(path, bytes).map_err(|e| crate::EngineError::Other(format!("cannot write {path}: {e}")))
}
/// `file.checkOutput`'s reply for `path`.
pub(crate) fn output_status(&self, path: &str) -> Value {
match self.blocking(path) {
Some(source) => json!({"path": path, "blocked": true, "source": source.kind(), "reason": source.refusal(path, Use::Export)}),
None => json!({"path": path, "blocked": false}),
}
}
/// Reject protected outputs and ranges that cannot be checked completely. Ordinary-file
/// replacement consent is left to the exporter, independently of this source guard.
pub(crate) fn check_outputs(&self, items: &[Vec<(RenderQueueItem, String)>]) -> Vec<Refused> {
@@ -287,7 +362,7 @@ impl Session {
.map(PathBuf::as_path)
.chain(movie)
.chain(Some(Path::new(&log)))
.find_map(|path| protected.find(path).map(|source| source.refusal(&path.to_string_lossy()))))
.find_map(|path| protected.find(path).map(|source| source.refusal(&path.to_string_lossy(), Use::Render))))
}
/// A new item's default output must not be a file the project reads: `[compName]_1`,
+1
View File
@@ -10,6 +10,7 @@ mod disk_cache;
mod effects;
mod essential;
mod essential_more;
mod export_overwrite;
mod face;
mod fidelity;
mod frame_export;
+271
View File
@@ -0,0 +1,271 @@
//! The commands that write to a path they are given leave a file the project reads alone (#1743,
//! beyond the Render Queue): footage, a proxy, a frame of an imported image sequence and the
//! project file are refused with nothing changed, and any other path is written. Save and Save As
//! write the project's own file and are not among them.
use std::sync::Arc;
use effectcraft_project::{Footage, FootageKind, ItemId, Proxy};
use serde_json::{Value, json};
use super::render_overwrite::{Fixture, SOURCE};
/// Footage that is a file named `path`.
fn video(path: String) -> Footage {
Footage { path, kind: FootageKind::Video, has_video: true, ..Default::default() }
}
/// `item` gets the file `file` of the project's folder as its proxy.
fn give_proxy(f: &mut Fixture, item: ItemId, file: &str) {
f.write(file, SOURCE);
let footage = video(f.text(file));
Arc::make_mut(&mut f.s.project).item_mut(item).unwrap().proxy = Some(Box::new(Proxy { footage, enabled: true }));
}
/// A project, saved as `Project.ecproj`, that reads `clip.<ext>` (footage) with `proxy.<ext>` as
/// its proxy and the frames `shot_00000.<ext>` to `shot_00004.<ext>` of an image sequence, all
/// holding `SOURCE`. Its composition `Main` holds a red solid.
fn reading(name: &str, ext: &str) -> Fixture {
let mut f = Fixture::new(name);
let clip = f.footage(&format!("clip.{ext}"));
give_proxy(&mut f, clip, &format!("proxy.{ext}"));
f.sequence_as("shot", ext, 0..5);
f.comp("Main");
f.s.execute("layer.newSolid", json!({"name": "Red", "color": "#ff0000"})).unwrap();
f
}
/// The names in `dir`, sorted.
fn listing(dir: &std::path::Path) -> Vec<String> {
let mut names: Vec<String> = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
names.sort();
names
}
/// A command that writes to a path.
struct Case {
command: &'static str,
/// The extension of a path for it.
ext: &'static str,
/// What it needs beyond a composition holding a solid.
setup: fn(&mut Fixture),
/// Its parameters for `path`.
params: fn(&str) -> Value,
}
fn nothing(_: &mut Fixture) {}
/// The first layer of the active composition.
fn first_layer(f: &Fixture) -> u64 {
f.s.active_comp().unwrap().layers[0].id.0
}
/// The composition shows one control, for `essential.exportTemplate`.
fn expose_opacity(f: &mut Fixture) {
let layer = first_layer(f);
f.s.execute("essential.addProperty", json!({"layer": layer, "path": "transform/opacity"})).unwrap();
}
/// A property is selected, for `anim.savePreset`.
fn select_opacity(f: &mut Fixture) {
let layer = first_layer(f);
f.s.execute("prop.select", json!({"layer": layer, "path": "transform/opacity"})).unwrap();
}
/// The composition has a guide, for `view.exportGuides`.
fn add_guide(f: &mut Fixture) {
f.s.execute("view.addGuide", json!({"orientation": "vertical"})).unwrap();
}
/// Run `case` on every file the project reads: each is refused, the project's folder is as it was
/// and nothing was rendered; then on a free path, where it writes.
fn never_replaces_what_the_project_reads(case: &Case) {
let ext = case.ext;
let mut f = reading(&format!("{}-{ext}", case.command), ext);
(case.setup)(&mut f);
let mut reads = vec![(format!("clip.{ext}"), "footage"), (format!("proxy.{ext}"), "proxy"), (format!("shot_00002.{ext}"), "footage")];
// (A timeline export adds its extension to `Project.ecproj`: it has a test of its own.)
if case.command != "file.exportTimeline" {
reads.push(("Project.ecproj".to_string(), "project"));
}
let before: Vec<Vec<u8>> = reads.iter().map(|(file, _)| f.read(file)).collect();
let listed = listing(&f.dir);
for ((file, source), bytes) in reads.iter().zip(&before) {
let path = f.text(file);
let e = f.s.execute(case.command, (case.params)(&path)).expect_err(&format!("{} wrote over {file}", case.command)).to_string();
assert!(e.contains(file.as_str()) && e.contains(source), "{}: {file}: {e}", case.command);
assert_eq!(&f.read(file), bytes, "{}: {file} is as it was", case.command);
}
assert_eq!(listing(&f.dir), listed, "{}: refusing changed nothing in the folder", case.command);
assert!(f.exported().is_empty(), "{}: nothing was rendered for it", case.command);
let free = format!("out.{ext}");
f.s.execute(case.command, (case.params)(&f.text(&free))).unwrap_or_else(|e| panic!("{}: {e}", case.command));
assert_ne!(f.read(&free), SOURCE, "{}: wrote its file", case.command);
for ((file, _), bytes) in reads.iter().zip(&before) {
assert_eq!(&f.read(file), bytes, "{}: {file} is still as it was", case.command);
}
}
macro_rules! never_replaces {
($name:ident, $command:literal, $ext:literal, $setup:expr) => {
#[test]
fn $name() {
never_replaces_what_the_project_reads(&Case { command: $command, ext: $ext, setup: $setup, params: |path| json!({"path": path}) });
}
};
}
never_replaces!(save_frame_as_png, "comp.saveFrameAs", "png", nothing);
never_replaces!(save_frame_as_photoshop_layers, "comp.saveFrameAsPsd", "psd", nothing);
never_replaces!(save_frame_as_proexr, "comp.saveFrameAsExr", "exr", nothing);
never_replaces!(export_lottie_json, "file.exportLottie", "json", nothing);
never_replaces!(export_lottie_dotlottie, "file.exportLottie", "lottie", nothing);
never_replaces!(save_a_copy, "file.saveCopy", "ecproj", nothing);
never_replaces!(save_a_copy_as_xml, "file.saveCopyAsXml", "ecprojx", nothing);
never_replaces!(export_timeline, "file.exportTimeline", "xml", nothing);
never_replaces!(export_template, "essential.exportTemplate", "ectemplate", expose_opacity);
never_replaces!(save_animation_preset, "anim.savePreset", "ecpreset", select_opacity);
never_replaces!(export_guides, "view.exportGuides", "json", add_guide);
never_replaces!(export_shortcuts, "shortcuts.export", "json", nothing);
// ---------------------------------------------------------------- Collect Files
/// A project reading the footage `in/<file>`, with an empty `out/(Footage)` where Collect Files
/// would put its copy.
fn collecting(name: &str, file: &str) -> Fixture {
let mut f = Fixture::new(name);
std::fs::create_dir_all(f.path("in")).unwrap();
std::fs::create_dir_all(f.path("out/(Footage)")).unwrap();
f.footage(&format!("in/{file}"));
f
}
/// Collect Files into the project folder's `out`.
fn collect(f: &mut Fixture) -> crate::Result<Value> {
let folder = f.text("out");
f.s.execute("file.collectFiles", json!({"folder": folder}))
}
/// `there`, the place the copy of the footage `in/<original>` would go, is something the project
/// reads (`source`): Collect Files is refused before it copies or writes anything.
fn collect_is_refused(mut f: Fixture, original: &str, there: &str, source: &str) {
f.write(there, b"NOT TO BE REPLACED");
let listed = listing(&f.path("out/(Footage)"));
let e = collect(&mut f).unwrap_err().to_string();
assert!(e.contains(original) && e.contains(source), "{e}");
assert_eq!(f.read(there), b"NOT TO BE REPLACED", "the file the project reads is as it was");
assert_eq!(f.read(&format!("in/{original}")), SOURCE, "the original is as it was");
assert!(!f.path("out/Project.ecproj").exists(), "the collected project was not written");
assert_eq!(listing(&f.path("out/(Footage)")), listed, "nothing else was copied");
}
#[test]
fn collect_files_does_not_copy_over_other_footage() {
let mut f = collecting("collect-footage", "a.mov");
f.footage("out/(Footage)/a.mov");
collect_is_refused(f, "a.mov", "out/(Footage)/a.mov", "footage");
}
#[test]
fn collect_files_does_not_copy_over_a_proxy() {
let mut f = collecting("collect-proxy", "a.mov");
let a = f.s.project.items.keys().next().copied().unwrap();
give_proxy(&mut f, a, "out/(Footage)/a.mov");
collect_is_refused(f, "a.mov", "out/(Footage)/a.mov", "proxy");
}
#[test]
fn collect_files_does_not_copy_over_a_sequence_frame() {
let mut f = collecting("collect-frame", "shot_00002.png");
f.sequence_as("out/(Footage)/shot", "png", 0..5);
collect_is_refused(f, "shot_00002.png", "out/(Footage)/shot_00002.png", "footage");
}
/// Collecting into the project's own folder would write the collected copy over the project.
#[test]
fn collect_files_does_not_write_over_the_project_file() {
let mut f = collecting("collect-project", "a.mov");
let project = f.read("Project.ecproj");
let folder = f.dir.to_string_lossy().into_owned();
let e = f.s.execute("file.collectFiles", json!({"folder": folder})).unwrap_err().to_string();
assert!(e.contains("Project.ecproj") && e.contains("project"), "{e}");
assert_eq!(f.read("Project.ecproj"), project, "the project file is as it was");
assert!(!f.path("(Footage)").exists(), "no folder was made, nothing was copied");
}
#[test]
fn collect_files_copies_into_a_free_folder() {
let mut f = collecting("collect-free", "a.mov");
let r = collect(&mut f).unwrap();
assert_eq!((r["copied"].as_u64(), &r["errors"]), (Some(1), &json!([])), "{r}");
assert_eq!(f.read("out/(Footage)/a.mov"), SOURCE);
assert_eq!(f.read("in/a.mov"), SOURCE, "the original stays");
assert!(f.path("out/Project.ecproj").exists());
}
/// Footage that is where its copy would go is already collected: it is not copied onto itself.
#[test]
fn collect_files_leaves_footage_that_is_already_in_the_folder() {
let mut f = collecting("collect-in-place", "a.mov");
f.footage("out/(Footage)/c.mov");
f.write("out/(Footage)/c.mov", b"ALREADY COLLECTED");
let r = collect(&mut f).unwrap();
assert_eq!((r["copied"].as_u64(), &r["errors"]), (Some(2), &json!([])), "{r}");
assert_eq!(f.read("out/(Footage)/c.mov"), b"ALREADY COLLECTED");
assert_eq!(f.read("out/(Footage)/a.mov"), SOURCE);
}
// ---------------------------------------------------------------- the rest
/// The movies a timeline export renders beside it are written like any other output.
#[test]
fn a_timeline_export_does_not_pre_render_over_footage() {
let mut f = Fixture::new("timeline-prerender");
f.comp("Main");
f.s.execute("layer.newSolid", json!({"name": "Red", "color": "#ff0000"})).unwrap();
// OpenTimelineIO has no solids: the solid is rendered to a movie beside the document.
let params = json!({"path": f.text("Main.otio"), "prerender": "unsupported"});
let r = f.s.execute("file.exportTimeline", params.clone()).unwrap();
let movies = f.exported();
assert_eq!(movies.len(), 1, "{r}");
assert_eq!(r["prerendered"].as_array().map(Vec::len), Some(1), "{r}");
// That movie is footage now.
let movie = std::path::Path::new(&movies[0]).file_name().unwrap().to_string_lossy().into_owned();
f.footage(&movie);
let r = f.s.execute("file.exportTimeline", params).unwrap();
assert_eq!(f.read(&movie), SOURCE, "the footage is as it was");
assert_eq!(f.exported().len(), 1, "no second render was started");
assert_eq!(r["prerendered"], json!([]), "{r}");
let warnings = r["warnings"].as_array().unwrap().iter().filter_map(Value::as_str).collect::<Vec<_>>().join("\n");
assert!(warnings.contains(&movie) && warnings.contains("footage"), "the export says why: {r}");
}
/// `file.checkOutput` answers before anything is written, for callers that write the file
/// themselves (the CLI, MCP's `render_frame`).
#[test]
fn check_output_names_the_files_an_export_may_not_use() {
let mut f = reading("check-output", "png");
for (file, source) in [("clip.png", "footage"), ("proxy.png", "proxy"), ("shot_00003.png", "footage"), ("Project.ecproj", "project")] {
let r = f.s.execute("file.checkOutput", json!({"path": f.text(file)})).unwrap();
assert_eq!((r["blocked"].as_bool(), r["source"].as_str()), (Some(true), Some(source)), "{file}: {r}");
assert!(r["reason"].as_str().unwrap().contains(file), "{file}: {r}");
assert_eq!(r["path"], json!(f.text(file)), "{r}");
}
let r = f.s.execute("file.checkOutput", json!({"path": f.text("free.png")})).unwrap();
assert_eq!((r["blocked"].as_bool(), r["source"].is_null()), (Some(false), true), "{r}");
assert!(f.s.execute("file.checkOutput", json!({})).is_err(), "a path is needed");
assert!(!f.path("free.png").exists(), "it only looks");
}
/// A project saved under a timeline's name is not overwritten by the timeline.
#[test]
fn a_timeline_export_does_not_write_over_the_project_file() {
let mut f = Fixture::new("timeline-project");
f.comp("Main");
f.s.execute("file.saveAs", json!({"path": f.text("Edit.xml")})).unwrap();
let saved = f.read("Edit.xml");
let e = f.s.execute("file.exportTimeline", json!({"path": f.text("Edit.xml")})).unwrap_err().to_string();
assert!(e.contains("Edit.xml") && e.contains("project"), "{e}");
assert_eq!(f.read("Edit.xml"), saved);
}
+19 -14
View File
@@ -12,7 +12,7 @@ use serde_json::{Value, json};
use crate::{ExportJob, ExportResult, Exporter, Session};
const SOURCE: &[u8] = b"SOURCE FOOTAGE";
pub(crate) const SOURCE: &[u8] = b"SOURCE FOOTAGE";
const EARLIER: &[u8] = b"AN EARLIER RENDER";
const RENDERED: &[u8] = b"RENDERED";
@@ -51,14 +51,14 @@ impl Exporter for Writer {
/// A session whose project is saved in a fresh folder (the footage goes there too), with an
/// exporter that writes real files.
struct Fixture {
s: Session,
pub(crate) struct Fixture {
pub(crate) s: Session,
writer: Arc<Writer>,
dir: PathBuf,
pub(crate) dir: PathBuf,
}
impl Fixture {
fn new(name: &str) -> Fixture {
pub(crate) fn new(name: &str) -> Fixture {
let dir = std::env::temp_dir().join("effectcraft-render-overwrite").join(format!("{}-{name}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
@@ -69,24 +69,24 @@ impl Fixture {
Fixture { s, writer, dir }
}
fn path(&self, file: &str) -> PathBuf {
pub(crate) fn path(&self, file: &str) -> PathBuf {
self.dir.join(file)
}
fn text(&self, file: &str) -> String {
pub(crate) fn text(&self, file: &str) -> String {
self.path(file).to_string_lossy().into_owned()
}
fn read(&self, file: &str) -> Vec<u8> {
pub(crate) fn read(&self, file: &str) -> Vec<u8> {
std::fs::read(self.path(file)).unwrap()
}
fn write(&self, file: &str, bytes: &[u8]) {
pub(crate) fn write(&self, file: &str, bytes: &[u8]) {
std::fs::write(self.path(file), bytes).unwrap();
}
/// A video file in the project's folder, imported.
fn footage(&mut self, file: &str) -> ItemId {
pub(crate) fn footage(&mut self, file: &str) -> ItemId {
self.write(file, SOURCE);
let f = Footage {
path: self.text(file),
@@ -102,9 +102,14 @@ impl Fixture {
/// Numbered stills in the project's folder, imported as one image sequence.
fn sequence(&mut self, name: &str, frames: std::ops::Range<u32>) -> ItemId {
self.sequence_as(name, "png", frames)
}
/// [`Fixture::sequence`] of files with extension `ext`.
pub(crate) fn sequence_as(&mut self, name: &str, ext: &str, frames: std::ops::Range<u32>) -> ItemId {
let files: Vec<String> = frames
.map(|n| {
let file = format!("{name}_{n:05}.png");
let file = format!("{name}_{n:05}.{ext}");
self.write(&file, SOURCE);
self.text(&file)
})
@@ -119,11 +124,11 @@ impl Fixture {
duration: Tick::from_seconds_f64(0.5),
..Default::default()
};
Arc::make_mut(&mut self.s.project).add_item(&format!("{name}_[00000-00004].png"), Label::None, None, ItemKind::Footage(f))
Arc::make_mut(&mut self.s.project).add_item(&format!("{name}_[00000-00004].{ext}"), Label::None, None, ItemKind::Footage(f))
}
/// A five-frame composition.
fn comp(&mut self, name: &str) {
pub(crate) fn comp(&mut self, name: &str) {
self.s.execute("comp.new", json!({"name": name, "width": 16, "height": 16, "frameRate": 10, "duration": 0.5})).unwrap();
}
@@ -133,7 +138,7 @@ impl Fixture {
}
/// The paths the exporter was started for.
fn exported(&self) -> Vec<String> {
pub(crate) fn exported(&self) -> Vec<String> {
self.writer.jobs.lock().unwrap().clone()
}
+12
View File
@@ -619,6 +619,12 @@ fn playback(app: &mut EffectcraftApp, p: &Value, now: f64) -> Outcome {
/// when the capture is issued.
fn screenshot(app: &mut EffectcraftApp, ctx: &egui::Context, p: &Value) -> Outcome {
let s = |k: &str| p.get(k).and_then(Value::as_str);
// A file the project reads is never the output.
if let Some(path) = s("path")
&& let Err(e) = app.session.refuse_export(path)
{
return err(e);
}
// The panel/element is an identity, not a rectangle: it is resolved when the capture
// is issued (after the UI settles), so a panel that moves meanwhile is not cropped
// with its old rectangle.
@@ -672,6 +678,12 @@ fn quit(app: &mut EffectcraftApp, ctx: &egui::Context, p: &Value) -> Outcome {
/// temp file) or inline as base64 when `base64: true`.
fn render_frame(app: &EffectcraftApp, p: &Value) -> Outcome {
let s = &app.session;
// A file the project reads is never the output (checked before rendering).
if let Some(path) = p.get("path").and_then(Value::as_str)
&& let Err(e) = s.refuse_export(path)
{
return err(e);
}
let cid = match s.resolve_comp(p.get("comp")) {
Ok(c) => c,
Err(e) => return err(e),
+1
View File
@@ -26,6 +26,7 @@ mod ui_effect_controls;
mod ui_effect_drop_highlight;
mod ui_effect_names;
mod ui_env_light_gpu;
mod ui_export_overwrite;
mod ui_expression_values;
mod ui_font_search;
mod ui_form_fit;
@@ -0,0 +1,107 @@
//! The control channel's `render.frame` and `ui.screenshot` write a PNG to the `path` they are
//! given, and never over a file the project reads (#1743): footage, a proxy or the project file.
use std::path::PathBuf;
use effectcraft_engine::Session;
use effectcraft_engine::color::Label;
use effectcraft_engine::project::{Footage, FootageKind, ItemKind};
use effectcraft_ui_egui::EffectcraftApp;
use effectcraft_ui_egui::control::{self, ControlRequest, Outcome};
use egui_kittest::Harness;
use serde_json::{Value, json};
const SOURCE: &[u8] = b"SOURCE FOOTAGE";
struct Rig {
h: Harness<'static, EffectcraftApp>,
dir: PathBuf,
}
impl Rig {
/// A project saved as `Project.ecproj` in a fresh folder, reading `clip.png` beside it.
fn new(name: &str) -> Rig {
let dir = std::env::temp_dir().join("effectcraft-ui-export-overwrite").join(format!("{}-{name}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("clip.png"), SOURCE).unwrap();
let mut s = Session::default();
s.execute("comp.new", json!({"name": "Main", "width": 16, "height": 16, "frameRate": 10, "duration": 0.5})).unwrap();
s.execute("file.saveAs", json!({"path": dir.join("Project.ecproj").to_string_lossy()})).unwrap();
let clip = Footage {
path: dir.join("clip.png").to_string_lossy().into_owned(),
kind: FootageKind::Still,
width: 16,
height: 16,
has_video: true,
..Default::default()
};
std::sync::Arc::make_mut(&mut s.project).add_item("clip.png", Label::Aqua, None, ItemKind::Footage(clip));
let mut app = Some(EffectcraftApp::new(s));
let mut h = Harness::builder().with_size(egui::vec2(1600.0, 1000.0)).build_eframe(|_| app.take().expect("app"));
h.run_steps(2);
Rig { h, dir }
}
fn text(&self, file: &str) -> String {
self.dir.join(file).to_string_lossy().into_owned()
}
fn read(&self, file: &str) -> Vec<u8> {
std::fs::read(self.dir.join(file)).unwrap()
}
/// A control-channel request, answered at once or not.
fn send(&mut self, method: &str, params: Value) -> Outcome {
let ctx = self.h.ctx.clone();
let (req, _rx) = ControlRequest::new(method, params);
let now = self.h.state().now();
control::handle(self.h.state_mut(), &ctx, &req, now)
}
/// The reply of a request that is answered at once.
fn reply(&mut self, method: &str, params: Value) -> Value {
match self.send(method, params) {
Outcome::Done(v) => v,
_ => panic!("{method}: no immediate reply"),
}
}
}
impl Drop for Rig {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.dir);
}
}
#[test]
fn render_frame_never_writes_over_a_file_the_project_reads() {
let mut r = Rig::new("render-frame");
let project = r.read("Project.ecproj");
for (file, what) in [("clip.png", "footage"), ("Project.ecproj", "project")] {
let v = r.reply("render.frame", json!({"path": r.text(file)}));
let error = v["error"].as_str().unwrap_or_default();
assert!(v["ok"] == false && error.contains(file) && error.contains(what), "{file}: {v}");
}
assert_eq!(r.read("clip.png"), SOURCE, "the footage is as it was");
assert_eq!(r.read("Project.ecproj"), project, "the project file is as it was");
// Anywhere else it writes the PNG.
let v = r.reply("render.frame", json!({"path": r.text("frame.png")}));
assert_eq!(v["ok"], true, "{v}");
assert_eq!(&r.read("frame.png")[..8], b"\x89PNG\r\n\x1a\n");
}
#[test]
fn screenshot_never_writes_over_a_file_the_project_reads() {
let mut r = Rig::new("screenshot");
let project = r.read("Project.ecproj");
for (file, what) in [("clip.png", "footage"), ("Project.ecproj", "project")] {
let Outcome::Done(v) = r.send("ui.screenshot", json!({"path": r.text(file)})) else { panic!("{file}: the screenshot was taken for it") };
let error = v["error"].as_str().unwrap_or_default();
assert!(v["ok"] == false && error.contains(file) && error.contains(what), "{file}: {v}");
}
assert_eq!(r.read("clip.png"), SOURCE, "the footage is as it was");
assert_eq!(r.read("Project.ecproj"), project, "the project file is as it was");
// Anywhere else it is taken (the frame that carries it comes later).
assert!(matches!(r.send("ui.screenshot", json!({"path": r.text("shot.png")})), Outcome::Screenshot { .. }));
}
+12
View File
@@ -319,6 +319,18 @@ build real projects through these interfaces; they are worked examples of everyt
proresProfile: 4444, output}` then `renderQueue.render {"wait": true}`. Check a render by
importing it (`file.import`, `file.newCompFromSelection`) and `render_frame {transparent: true}`
for its alpha (ProRes 4444 and WebM VP9 alpha both read back as straight alpha).
* **Other files written to a path you give** follow the same footage rule, with no flag and no
prompt: `comp.saveFrameAs` (also `…Psd`, `…Exr`), `file.exportLottie`, `file.saveCopy`,
`file.saveCopyAsXml`, `file.collectFiles` (every copy and the collected project; nothing is
written if one would land on a file the project reads), `file.exportTimeline` (and the movies it
pre-renders), `essential.exportTemplate`, `anim.savePreset`, `view.exportGuides`,
`shortcuts.export`, the control channel's `render.frame` / `ui.screenshot` with a `path`, the
MCP `render_frame` tool and `effectcraft-cli render-frame --out`. Footage (every frame of a
sequence), a proxy and the project file are refused with the reason and left untouched.
`file.checkOutput {path}` says beforehand (`{blocked, source, reason}`). Not covered: Save /
Save As / Increment and Save / auto-save (they write the project's own file), script `File`
writes (a script may rewrite a data file it also imports), and files the app extracts for
itself (template media, vector images).
* **Essential Graphics**: controls can be addressed by name (`essential.set {"layer":"#1",
"control":"Title","value":"John Smith"}`); a command that targets an explicit `comp` runs even
when the active comp would disable it (`essential.exportTemplate {"comp":"Lower Third", …}`).