mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-14 19:27:05 -04:00
Render Queue: output files end in their format's extension once (#293)
Renders on macOS came out as `Comp 1.mov.ecproj`, and an image sequence's every frame as `Comp 1_00001.png.ecproj`; another output was `Comp 2.mov.mov`. Cause: the save dialogs for anything but a project went through the project dialog hook, which filters to `.ecproj`; macOS's save panel appends the allowed extension to any other name. 0.4.0's Render Queue Choose File… used it (#192 moved Output To to its own dialog since), and Save Frame As, Save Current Preview, Export Lottie, Save a Copy As XML, Collect Files, Watch Folder and the Premiere export form's Browse still did. Nothing then fixed the name: Output To kept `.ecproj` (no format claims it), and changing the format replaced only the last extension (`Comp 2.mov.ecproj` → `Comp 2.mov.mov`). `[compName]` of a comp named `Comp 2.mov` also doubled it. Fix: `Hooks::save_dialog` asks the dialog for the default name's own extension (`pick_save_file`), the project dialog only for projects or on hosts without another (the web); the desktop dialog has no filter for a name without an extension (folders). `OutputFormat::with_extension` makes an output path end in the format's extension exactly once (project extensions and repeats of the format's dropped, a missing one added; `logo.png.mov` kept), used by Output To, format changes and `resolve_output`, so existing projects and templates render to the right name too. Output To picks the file type from the extension before a stray `.ecproj`. Test: output_names_end_in_the_format_extension_once (project), output_to_ends_in_the_format_extension_once (engine) and save_dialogs_offer_the_file_extension (ui); the engine and ui tests fail with the fix reverted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
a62a298f0c
commit
be926adb92
9 files changed
+191
-30
No files matched your search
@@ -182,7 +182,8 @@ fn main() -> eframe::Result {
|
||||
app.hooks.pick_save_file = Some(Box::new(|name: &str, ext: &str| {
|
||||
// A default with a folder that exists opens the dialog there.
|
||||
let path = std::path::Path::new(name);
|
||||
let mut d = rfd::FileDialog::new().add_filter(ext, &[ext]);
|
||||
// (No filter for a name without an extension, such as a folder to create.)
|
||||
let mut d = if ext.is_empty() { rfd::FileDialog::new() } else { rfd::FileDialog::new().add_filter(ext, &[ext]) };
|
||||
if let Some(dir) = path.parent().filter(|d| d.is_dir()) {
|
||||
d = d.set_directory(dir);
|
||||
}
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
use effectcraft_project::render_queue::{
|
||||
AlphaMode, AudioFormat, AudioOutput, Channels, ColorDepth, CurrentOrOff, DEFAULT_SEQUENCE_TEMPLATE, DEFAULT_TEMPLATE, EffectsMode, FieldRender,
|
||||
OutputFormat, OutputModule, PostRenderAction, ProResProfile, Pulldown, RESIZE_PRESETS, RenderLog, RenderQuality, RenderQueueItem, RenderSettings,
|
||||
RenderStatus, ResizeQuality, SwitchOverride, TimeSpan, post_render_parse,
|
||||
RenderStatus, ResizeQuality, SwitchOverride, TimeSpan, post_render_parse, without_project_extension,
|
||||
};
|
||||
use effectcraft_project::render_templates::{RenderTemplates, TemplateKind, TemplateSlot};
|
||||
use effectcraft_time::{FrameRate, Tick};
|
||||
@@ -364,7 +364,8 @@ fn apply_output(om: &mut OutputModule, templates: &RenderTemplates, roi: Option<
|
||||
if let Some(l) = b_p(p, "includeProjectLink") {
|
||||
om.include_project_link = l;
|
||||
}
|
||||
if let Some(o) = str_p(p, "output").or(str_p(p, "path")) {
|
||||
// (Without the `.ecproj` a save dialog filtered to projects appended, #293.)
|
||||
if let Some(o) = str_p(p, "output").or(str_p(p, "path")).map(without_project_extension) {
|
||||
om.output = o.to_string();
|
||||
// Only an extension the format doesn't write changes it: `.mp4` is H.264's, HEVC's and AV1's.
|
||||
if let Some(f) = OutputFormat::from_path(o).filter(|f| f.extension() != om.format.extension() && !o.contains("[fileExtension]")) {
|
||||
@@ -379,6 +380,8 @@ fn apply_output(om: &mut OutputModule, templates: &RenderTemplates, roi: Option<
|
||||
om.output = keep;
|
||||
}
|
||||
}
|
||||
// The format's extension exactly once (`Comp 2.mov.mov` → `Comp 2.mov`).
|
||||
om.output = om.format.with_extension(&om.output);
|
||||
}
|
||||
if *om != before {
|
||||
// Output To alone doesn't make the module custom.
|
||||
|
||||
@@ -321,7 +321,8 @@ impl Session {
|
||||
.unwrap_or_else(|| "Untitled Project".into())
|
||||
}
|
||||
|
||||
/// The output path of a queue item: template tokens expanded, relative paths resolved against
|
||||
/// The output path of a queue item: template tokens expanded, ending in the format's
|
||||
/// extension exactly once ([`OutputFormat::with_extension`]), relative paths resolved against
|
||||
/// the project's folder (or the working directory). `None` when the comp is gone or the
|
||||
/// output is empty ("Needs Output").
|
||||
pub fn resolve_output(&self, item: &RenderQueueItem) -> Option<String> {
|
||||
@@ -338,7 +339,8 @@ impl Session {
|
||||
let pname = self.project_name();
|
||||
let vars =
|
||||
TemplateVars { comp_name: &name, project_name: &pname, width: w, height: h, frame_rate: rate.as_f64(), start_frame: f0, end_frame: f0 + n - 1 };
|
||||
let p = expand_template(tpl, item.output.format, &vars);
|
||||
// The format's extension exactly once, whatever the name or a save dialog added (#293).
|
||||
let p = item.output.format.with_extension(&expand_template(tpl, item.output.format, &vars));
|
||||
let path = std::path::Path::new(&p);
|
||||
if path.is_absolute() {
|
||||
return Some(p);
|
||||
|
||||
@@ -110,6 +110,34 @@ fn output_to_keeps_a_format_that_writes_the_extension() {
|
||||
assert_eq!((&r["output"]["format"], &r["outputModuleSummary"]), (&json!("Hevc"), &a["outputModuleSummary"]), "{r}");
|
||||
}
|
||||
|
||||
/// Output To ends in the format's extension exactly once (#293). A name from a save dialog
|
||||
/// filtered to projects (`Comp 1.mov.ecproj`; for a PNG sequence every frame got `.png.ecproj`)
|
||||
/// loses the `.ecproj` and picks the file type from the extension before it; a comp named
|
||||
/// `Comp 2.mov` renders to `Comp 2.mov`, not `Comp 2.mov.mov`.
|
||||
#[test]
|
||||
fn output_to_ends_in_the_format_extension_once() {
|
||||
let mut s = session();
|
||||
let cases = [
|
||||
("h264", "/out/Comp 1.mov.ecproj", OutputFormat::ProRes, "/out/Comp 1.mov"),
|
||||
("png", "/out/Comp 1_[#####].png.ecproj", OutputFormat::PngSequence, "/out/Comp 1_[#####].png"),
|
||||
("prores", "/out/Comp 2.mov.mov", OutputFormat::ProRes, "/out/Comp 2.mov"),
|
||||
("prores", "/out/render", OutputFormat::ProRes, "/out/render.mov"),
|
||||
];
|
||||
for (format, out, want, path) in cases {
|
||||
let a = s.execute("renderQueue.add", json!({"format": format})).unwrap();
|
||||
let r = s.execute("renderQueue.setOutput", json!({"item": a["item"], "path": out})).unwrap();
|
||||
let om = &s.project.render_queue.last().unwrap().output;
|
||||
assert_eq!((om.format, om.output.as_str()), (want, path), "{format} → {out}");
|
||||
assert!(r["outputPath"].as_str().unwrap().ends_with(&path[5..]), "{r}");
|
||||
}
|
||||
s.execute("comp.new", json!({"name": "Comp 2.mov", "width": 64, "height": 36, "frameRate": 30, "duration": 1})).unwrap();
|
||||
for (format, name) in [("prores", "Comp 2.mov"), ("h264", "Comp 2.mov.mp4"), ("png", "Comp 2.mov_[#####].png")] {
|
||||
let a = s.execute("renderQueue.add", json!({"format": format})).unwrap();
|
||||
let path = a["outputPath"].as_str().unwrap();
|
||||
assert_eq!(std::path::Path::new(path).file_name().unwrap().to_string_lossy(), name, "{format}: {path}");
|
||||
}
|
||||
}
|
||||
|
||||
/// An explicit RGB + Alpha request with the AV1 WebM codec (which has no alpha) is refused, not
|
||||
/// silently rendered opaque (#166). Without `channels`, AV1 WebM renders RGB as before.
|
||||
#[test]
|
||||
|
||||
@@ -622,6 +622,51 @@ impl OutputFormat {
|
||||
let ext = std::path::Path::new(path).extension()?.to_string_lossy().to_ascii_lowercase();
|
||||
OutputFormat::from_name(&ext)
|
||||
}
|
||||
/// An output `path` without its extensions, for this format to add its own: the project
|
||||
/// extensions a save dialog filtered to projects appended (`Comp 1.png.ecproj`, #293), one
|
||||
/// output format's extension (the one being replaced), and any more of this format's
|
||||
/// (`Comp 2.mov.mov`, from a format change or a comp named `Comp 2.mov`). Another extension
|
||||
/// stays part of the name (`logo.png.mov`).
|
||||
pub fn output_stem(self, path: &str) -> &str {
|
||||
let stem = without_project_extension(path);
|
||||
let mut stem = strip_extension(stem, |e| OutputFormat::from_name(e).is_some()).unwrap_or(stem);
|
||||
while let Some(s) = strip_extension(stem, |e| e.eq_ignore_ascii_case(self.extension())) {
|
||||
stem = s;
|
||||
}
|
||||
stem
|
||||
}
|
||||
/// `path` ending in this format's extension exactly once (see [`OutputFormat::output_stem`]);
|
||||
/// a path that already does is kept as it is. Templates still to expand `[fileExtension]` are
|
||||
/// kept too.
|
||||
pub fn with_extension(self, path: &str) -> String {
|
||||
if path.contains("[fileExtension]") {
|
||||
return path.to_string();
|
||||
}
|
||||
let fixed = format!("{}.{}", self.output_stem(path), self.extension());
|
||||
if fixed.eq_ignore_ascii_case(path) { path.to_string() } else { fixed }
|
||||
}
|
||||
}
|
||||
|
||||
/// Project file extensions: never a render's output.
|
||||
const PROJECT_EXTENSIONS: [&str; 2] = ["ecproj", "ecprojx"];
|
||||
|
||||
/// An output `path` without the project extensions a save dialog filtered to projects appended
|
||||
/// to it (`Comp 1.mov.ecproj` → `Comp 1.mov`, #293).
|
||||
pub fn without_project_extension(path: &str) -> &str {
|
||||
let mut p = path;
|
||||
while let Some(s) = strip_extension(p, |e| PROJECT_EXTENSIONS.iter().any(|x| e.eq_ignore_ascii_case(x))) {
|
||||
p = s;
|
||||
}
|
||||
p
|
||||
}
|
||||
|
||||
/// `path` without its last extension when `strip` accepts that extension.
|
||||
fn strip_extension(path: &str, strip: impl Fn(&str) -> bool) -> Option<&str> {
|
||||
let ext = std::path::Path::new(path).extension()?.to_str()?;
|
||||
if !strip(ext) {
|
||||
return None;
|
||||
}
|
||||
path.strip_suffix(ext)?.strip_suffix('.')
|
||||
}
|
||||
|
||||
/// Output Module ▸ Channels.
|
||||
@@ -1130,8 +1175,7 @@ impl OutputModule {
|
||||
}
|
||||
return;
|
||||
}
|
||||
let p = std::path::Path::new(&self.output);
|
||||
let mut stem = p.with_extension("").to_string_lossy().to_string();
|
||||
let mut stem = format.output_stem(&self.output).to_string();
|
||||
if format.is_sequence() && !stem.contains('#') {
|
||||
stem.push_str("_[#####]");
|
||||
} else if !format.is_sequence() {
|
||||
@@ -1451,6 +1495,42 @@ mod tests {
|
||||
assert_eq!(ProResProfile::from_name("hq"), Some(ProResProfile::Hq));
|
||||
}
|
||||
|
||||
/// Output names end in the format's extension exactly once (#293): a save dialog filtered to
|
||||
/// projects appended `.ecproj` (`Comp 1.mov.ecproj`, `Comp 1_00001.png.ecproj` for every
|
||||
/// frame), and a format change or a comp named `Comp 2.mov` doubled it (`Comp 2.mov.mov`).
|
||||
#[test]
|
||||
fn output_names_end_in_the_format_extension_once() {
|
||||
use OutputFormat::*;
|
||||
for (path, format, want) in [
|
||||
("/out/Comp 1.mov.ecproj", ProRes, "/out/Comp 1.mov"),
|
||||
("/out/Comp 1_[#####].png.ecproj", PngSequence, "/out/Comp 1_[#####].png"),
|
||||
("/out/Comp 1.mov.ecprojx.ecproj", ProRes, "/out/Comp 1.mov"),
|
||||
("/out/Comp 2.mov.mov", ProRes, "/out/Comp 2.mov"),
|
||||
("/out/Comp 2.mov.mov.mov", ProRes, "/out/Comp 2.mov"),
|
||||
("/out/render", ProRes, "/out/render.mov"),
|
||||
("/out/frames_[#####]", PngSequence, "/out/frames_[#####].png"),
|
||||
("/out/logo.png.mov", ProRes, "/out/logo.png.mov"),
|
||||
("/out/v1.2", H264, "/out/v1.2.mp4"),
|
||||
("/my.folder/Clip.MOV", ProRes, "/my.folder/Clip.MOV"),
|
||||
("/out/x.mp4", Hevc, "/out/x.mp4"),
|
||||
("[compName].[fileExtension]", ProRes, "[compName].[fileExtension]"),
|
||||
] {
|
||||
assert_eq!(format.with_extension(path), want, "{path} as {format:?}");
|
||||
}
|
||||
assert_eq!(without_project_extension("C:/out/Comp 1.mov.ecproj"), "C:/out/Comp 1.mov");
|
||||
assert_eq!(without_project_extension("Comp 1.mov"), "Comp 1.mov");
|
||||
// Changing the format replaces one extension and drops the project one.
|
||||
let mut m = OutputModule { output: "/out/Comp 1.mov.ecproj".into(), ..OutputModule::for_format(ProRes) };
|
||||
m.set_format(H264);
|
||||
assert_eq!(m.output, "/out/Comp 1.mp4");
|
||||
m.output = "/out/Comp 2.mov.mov".into();
|
||||
m.set_format(ProRes);
|
||||
assert_eq!(m.output, "/out/Comp 2.mov");
|
||||
m.output = "/out/Comp 2.mov.ecproj".into();
|
||||
m.set_format(PngSequence);
|
||||
assert_eq!(m.output, "/out/Comp 2_[#####].png");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pulldown_cadence() {
|
||||
assert_eq!(Pulldown::Wwssw.pattern(), "WWSSW");
|
||||
|
||||
@@ -103,8 +103,8 @@ pub struct Hooks {
|
||||
pub audio_devices: Option<Box<dyn Fn() -> Vec<String>>>,
|
||||
/// Picks a folder (Settings paths).
|
||||
pub pick_folder: Option<Box<dyn Fn() -> Option<String>>>,
|
||||
/// Save dialog for other file kinds: (default name or path, extension). A default path opens
|
||||
/// the dialog in its folder.
|
||||
/// Save dialog for other file kinds: (default name or path, extension; empty: no filter). A
|
||||
/// default path opens the dialog in its folder.
|
||||
pub pick_save_file: Option<Box<dyn Fn(&str, &str) -> Option<String>>>,
|
||||
/// The system clipboard's text (native menu Edit ▸ Paste into a text field).
|
||||
pub clipboard_text: Option<Box<dyn Fn() -> Option<String>>>,
|
||||
@@ -113,6 +113,24 @@ pub struct Hooks {
|
||||
pub app_action: Option<Box<dyn Fn(&str) -> bool>>,
|
||||
}
|
||||
|
||||
impl Hooks {
|
||||
/// The host's save dialog for `default` (a file name or path), offering its extension: the
|
||||
/// project dialog for project files, else [`Hooks::pick_save_file`]. The project dialog is
|
||||
/// the fallback for hosts without that (the web, whose dialog only names the file): on macOS
|
||||
/// a dialog filtered to projects appends `.ecproj` to any other name (#293). `None` when the
|
||||
/// host has no save dialog; `Some(None)` when it was cancelled.
|
||||
pub fn save_dialog(&self, default: &str) -> Option<Option<String>> {
|
||||
let path = std::path::Path::new(default);
|
||||
let ext = path.extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default();
|
||||
match (&self.pick_save_file, &self.pick_save) {
|
||||
(Some(pick), _) if !ext.eq_ignore_ascii_case("ecproj") => Some(pick(default, &ext)),
|
||||
(_, Some(pick)) => Some(pick(&path.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default())),
|
||||
(Some(pick), None) => Some(pick(default, &ext)),
|
||||
(None, None) => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct Playback {
|
||||
pub playing: bool,
|
||||
|
||||
@@ -1152,8 +1152,8 @@ fn file_dialog(app: &mut EffectcraftApp, id: &str, params: &Value) -> Option<Res
|
||||
f(exts).first().map(|s| json!(s))
|
||||
}
|
||||
Ask::Save(default) => {
|
||||
let Some(f) = app.hooks.pick_save.as_ref() else { return Some(Err("no file dialog available (pass `path`)".into())) };
|
||||
f(default).map(|s| json!(s))
|
||||
let Some(picked) = app.hooks.save_dialog(default) else { return Some(Err("no file dialog available (pass `path`)".into())) };
|
||||
picked.map(|s| json!(s))
|
||||
}
|
||||
};
|
||||
let Some(v) = picked else { return Some(Ok(Value::Null)) };
|
||||
@@ -1709,6 +1709,47 @@ mod tests {
|
||||
assert!(!labels.contains(&"Yellow".to_string()));
|
||||
}
|
||||
|
||||
/// Saving anything but a project asks the save dialog for that file's extension (#293): the
|
||||
/// project dialog, filtered to `.ecproj`, made macOS append `.ecproj` (`Frame.png.ecproj`).
|
||||
/// Hosts with only the project dialog (the web) still get it, given the file name.
|
||||
#[test]
|
||||
fn save_dialogs_offer_the_file_extension() {
|
||||
use std::cell::RefCell;
|
||||
use std::rc::Rc;
|
||||
let mut app = EffectcraftApp::new(effectcraft_engine::Session::default());
|
||||
app.session.execute("comp.new", json!({"name": "A", "width": 64, "height": 36, "frameRate": 30, "duration": 1})).unwrap();
|
||||
let ctx = egui::Context::default();
|
||||
let asked: Rc<RefCell<Vec<String>>> = Rc::default();
|
||||
let (a, b) = (asked.clone(), asked.clone());
|
||||
app.hooks.pick_save = Some(Box::new(move |name: &str| {
|
||||
a.borrow_mut().push(format!("project {name}"));
|
||||
None
|
||||
}));
|
||||
app.hooks.pick_save_file = Some(Box::new(move |name: &str, ext: &str| {
|
||||
b.borrow_mut().push(format!("file {name} [{ext}]"));
|
||||
None
|
||||
}));
|
||||
for id in ["comp.saveFrameAs", "render.saveCurrentPreview", "file.exportLottie", "file.saveCopyAsXml", "file.collectFiles", "file.saveAs"] {
|
||||
invoke(&mut app, &ctx, id, json!({})).unwrap();
|
||||
}
|
||||
assert_eq!(
|
||||
*asked.borrow(),
|
||||
[
|
||||
"file Frame.png [png]",
|
||||
"file Preview.mp4 [mp4]",
|
||||
"file Animation.json [json]",
|
||||
"file Untitled Project.ecprojx [ecprojx]",
|
||||
"file Collected Files []",
|
||||
"project Untitled Project.ecproj",
|
||||
]
|
||||
);
|
||||
// Without the other dialog, the project one names any file.
|
||||
app.hooks.pick_save_file = None;
|
||||
asked.borrow_mut().clear();
|
||||
invoke(&mut app, &ctx, "comp.saveFrameAs", json!({})).unwrap();
|
||||
assert_eq!(*asked.borrow(), ["project Frame.png"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn menu_items_cover_the_tree() {
|
||||
assert_eq!(menus().last(), Some(&"Help"));
|
||||
|
||||
@@ -758,13 +758,9 @@ pub fn show_form(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) {
|
||||
if let Some(i) = browse
|
||||
&& let Some(FieldKind::SavePath(value)) = f.fields.get_mut(i).map(|f| &mut f.kind)
|
||||
{
|
||||
let default = std::path::Path::new(value.as_str()).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
|
||||
match app.hooks.pick_save.as_ref() {
|
||||
Some(pick) => {
|
||||
if let Some(path) = pick(&default) {
|
||||
*value = path;
|
||||
}
|
||||
}
|
||||
match app.hooks.save_dialog(value) {
|
||||
Some(Some(path)) => *value = path,
|
||||
Some(None) => {}
|
||||
None => app.ui.status = "no file dialog available: type the path".into(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -396,12 +396,10 @@ fn output_to_menu(it: &RenderQueueItem) -> Vec<(String, Value)> {
|
||||
}
|
||||
|
||||
/// Output To's save dialog, as After Effects' Output Movie To: it opens at the current output
|
||||
/// (folder and name) and offers the output format's extension. `None` when the host has no
|
||||
/// save dialog for other file kinds; `Some(None)` when the dialog was cancelled.
|
||||
/// (folder and name, which ends in the format's extension) and offers that extension. `None`
|
||||
/// when the host has no save dialog; `Some(None)` when the dialog was cancelled.
|
||||
fn pick_output(app: &EffectcraftApp, it: &RenderQueueItem) -> Option<Option<String>> {
|
||||
let pick = app.hooks.pick_save_file.as_ref()?;
|
||||
let default = app.session.resolve_output(it).unwrap_or_default();
|
||||
Some(pick(&default, it.output.format.extension()))
|
||||
app.hooks.save_dialog(&app.session.resolve_output(it).unwrap_or_default())
|
||||
}
|
||||
|
||||
pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
|
||||
@@ -697,13 +695,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) {
|
||||
let olabels: Vec<String> = omenu.iter().map(|(l, _)| l.clone()).collect();
|
||||
if let Some(i) = widgets::popup_menu(ui, oid, odd.left_bottom(), &olabels, None) {
|
||||
if omenu[i].1.get("choose").is_some() {
|
||||
// The web has no folders to choose from: its save "dialog" only names the file.
|
||||
let picked = pick_output(app, it).or_else(|| {
|
||||
let default = app.session.resolve_output(it).unwrap_or_default();
|
||||
let name = std::path::Path::new(&default).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
|
||||
app.hooks.pick_save.as_ref().map(|f| f(&name))
|
||||
});
|
||||
match picked {
|
||||
match pick_output(app, it) {
|
||||
Some(Some(path)) => actions.push(("renderQueue.setOutput", json!({"item": it.id, "path": path}))),
|
||||
Some(None) => {}
|
||||
None => editing = Some((it.id, it.output.output.clone())),
|
||||
|
||||
Reference in new issue
Block a user