From 3ba43b20c7c7d7a06ae2094f32993707e3ce21cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 19:29:15 +0000 Subject: [PATCH] =?UTF-8?q?Effect=20Controls:=20file=20parameters=20get=20?= =?UTF-8?q?Choose=E2=80=A6,=20Apply=20Color=20LUT=20asks=20for=20its=20LUT?= =?UTF-8?q?=20on=20apply,=20and=20text=20parameters=20get=20a=20field=20(#?= =?UTF-8?q?514,=20#473)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cause: Apply Color LUT's only parameter, `lut`, was ParamUi::Hidden (set only through commands), so the effect could not be used from the UI. Effect Controls also had no widget for any ParamUi::Text parameter (only EXtractoR's channels had one), so Cryptomatte's Selection, the OCIO effects' config file / file / config names and Colorama's Output Cycle could not be set either. Fix: a new ParamUi::File { filters } for paths. Effect Controls shows the file's name (None when empty) and a Choose... button that opens the host's file dialog with those extensions and sets the path (prop.set, so agents use the same command). As in After Effects, applying an effect with an empty file parameter from the UI (Effect menu, Last Effect, drops, Effects & Presets) opens the file dialog straight away; cancelling keeps the effect without a file. Apply Color LUT's LUT (.cube) and the OCIO Config File (.ocio) and File (.cube/.3dl/.csp/.cc/.ccc/.cdl/.spi1d/.spi3d/.spimtx) use it. Text parameters get a single-line field that sets the value when it loses the focus (Esc keeps the old text). Saved projects are upgraded on open: a Hidden or Text parameter whose spec is now a File parameter takes the File UI and keeps its path (effects::migrate). docs/effects.md regenerated. This covers #473's first part (the Selection field); picking mattes in the viewer, the layer popup from the file's metadata, the GPU path and quoted commas are separate. Tests: effects hidden_and_text_paths_become_file_parameters (migration); ui file_and_text_parameters_have_widgets (the LUT is asked for on apply and inverts red to cyan, the Choose... button picks another LUT, typing into Cryptomatte's Selection sets it). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01QiAJeFttZp83Fh7YuzAR5f (cherry picked from commit 403863f703999d19d3a83d9976f76bdc7577e682) --- crates/effects/src/lib.rs | 4 + crates/effects/src/migrate.rs | 20 +++++ crates/effects/src/ocio.rs | 6 +- crates/effects/src/utility.rs | 5 +- crates/project/src/props.rs | 5 ++ crates/ui-egui/src/menus.rs | 3 +- crates/ui-egui/src/panels/effect_controls.rs | 93 ++++++++++++++++++++ crates/ui-egui/src/panels/effects_presets.rs | 3 +- crates/ui-egui/tests/ui_effect_controls.rs | 82 +++++++++++++++++ docs/effects.md | 2 +- 10 files changed, 215 insertions(+), 8 deletions(-) diff --git a/crates/effects/src/lib.rs b/crates/effects/src/lib.rs index 4bce06367..45e2be1ce 100644 --- a/crates/effects/src/lib.rs +++ b/crates/effects/src/lib.rs @@ -430,6 +430,10 @@ pub(crate) fn p(id: &'static str, name: &'static str, default: Value, ui: ParamU pub(crate) fn slider(min: f64, max: f64, smin: f64, smax: f64, decimals: u8) -> ParamUi { ParamUi::Slider { min, max, slider_min: smin, slider_max: smax, decimals } } +/// A file parameter offering files with these extensions (Effect Controls' Choose…). +pub(crate) fn file_ui(exts: &[&str]) -> ParamUi { + ParamUi::File { filters: exts.iter().map(|s| s.to_string()).collect() } +} pub(crate) fn popup(opts: &[&str]) -> ParamUi { ParamUi::Popup { options: opts.iter().map(|s| s.to_string()).collect() } } diff --git a/crates/effects/src/migrate.rs b/crates/effects/src/migrate.rs index c161677be..0c0dad9d1 100644 --- a/crates/effects/src/migrate.rs +++ b/crates/effects/src/migrate.rs @@ -236,6 +236,9 @@ fn refresh(pr: &mut Property, ps: &ParamSpec, spec: &EffectSpec) { } // A parameter that became internal (superseded by a new one) is hidden. (_, ParamUi::Hidden) => pr.ui = ParamUi::Hidden, + // A path typed as text, or set only by commands, that got a Choose… button (Apply Color + // LUT's LUT, the OCIO files, #514). + (ParamUi::Hidden | ParamUi::Text, ParamUi::File { .. }) => pr.ui = ps.ui.clone(), // Same kind of value: take the spec's hints (units, ranges). (a, b) if std::mem::discriminant(a) == std::mem::discriminant(b) || is_numeric_ui(a) && is_numeric_ui(b) => pr.ui = ps.ui.clone(), _ => {} @@ -494,6 +497,23 @@ mod tests { assert_eq!(pr.ui, popup.ui); } + /// #514: Apply Color LUT's LUT was hidden (set only by commands) and the OCIO files were + /// plain text; old projects get the Choose… file parameter, keeping the path. + #[test] + fn hidden_and_text_paths_become_file_parameters() { + for (fx, pid, old) in [("ec.utility.applylut", "lut", ParamUi::Hidden), ("ec.color.ociofile", "file", ParamUi::Text)] { + let mut next = 1; + let (spec, mut g) = instance(fx, &mut next); + let pr = g.get_mut(pid).unwrap(); + pr.ui = old; + pr.value = Value::Str("/luts/look.cube".into()); + assert!(upgrade_instance(spec, &mut g, &mut Ids(&mut next), [100.0, 100.0]), "{fx}"); + let pr = g.get(pid).unwrap(); + assert!(matches!(&pr.ui, ParamUi::File { filters } if filters.iter().any(|f| f == "cube")), "{fx}: {:?}", pr.ui); + assert_eq!(pr.value, Value::Str("/luts/look.cube".into())); + } + } + #[test] fn upgrade_moves_params_into_twirl_down_groups() { let spec = find(crate::warp_stab::ID).unwrap(); diff --git a/crates/effects/src/ocio.rs b/crates/effects/src/ocio.rs index c8fcf848e..74816b110 100644 --- a/crates/effects/src/ocio.rs +++ b/crates/effects/src/ocio.rs @@ -1427,7 +1427,7 @@ pub fn specs() -> Vec { cc, vec![ p("config", "Configuration", Value::Enum(0), popup(CONFIGS)), - p("configFile", "Config File (.ocio)", Value::Str(String::new()), ParamUi::Text), + p("configFile", "Config File (.ocio)", Value::Str(String::new()), crate::file_ui(&["ocio"])), space_param("source", "Source", lin709), p("sourceName", "Source (config name)", Value::Str(String::new()), ParamUi::Text), space_param("destination", "Destination", srgb), @@ -1442,7 +1442,7 @@ pub fn specs() -> Vec { cc, vec![ p("config", "Configuration", Value::Enum(0), popup(CONFIGS)), - p("configFile", "Config File (.ocio)", Value::Str(String::new()), ParamUi::Text), + p("configFile", "Config File (.ocio)", Value::Str(String::new()), crate::file_ui(&["ocio"])), space_param("source", "Input Color Space", lin709), p("sourceName", "Source (config name)", Value::Str(String::new()), ParamUi::Text), p("display", "Display Device", Value::Enum(0), popup(&DISPLAYS.iter().map(|d| d.0).collect::>())), @@ -1459,7 +1459,7 @@ pub fn specs() -> Vec { "OCIO File Transform", cc, vec![ - p("file", "File", Value::Str(String::new()), ParamUi::Text), + p("file", "File", Value::Str(String::new()), crate::file_ui(&["cube", "3dl", "csp", "cc", "ccc", "cdl", "spi1d", "spi3d", "spimtx"])), p("cccId", "CCC ID", Value::Str(String::new()), ParamUi::Text), p("interpolation", "Interpolation", Value::Enum(3), popup(&["Nearest", "Linear", "Tetrahedral", "Best"])), direction(), diff --git a/crates/effects/src/utility.rs b/crates/effects/src/utility.rs index 46780aec5..1d3b3ee4a 100644 --- a/crates/effects/src/utility.rs +++ b/crates/effects/src/utility.rs @@ -385,8 +385,9 @@ pub fn specs() -> Vec { ], cineon, ), - // `lut`: a path to a `.cube` file, or the `.cube` text itself (set via commands/automation). - spec("ec.utility.applylut", "Apply Color LUT", vec![p("lut", "LUT", Value::Str(String::new()), ParamUi::Hidden)], apply_lut), + // `lut`: a path to a `.cube` file (Effect Controls' Choose…, asked for when the effect is + // applied, as in After Effects), or the `.cube` text itself (commands/automation). + spec("ec.utility.applylut", "Apply Color LUT", vec![p("lut", "LUT", Value::Str(String::new()), crate::file_ui(&["cube"]))], apply_lut), ] } diff --git a/crates/project/src/props.rs b/crates/project/src/props.rs index cdacb3f1f..f1d5149d3 100644 --- a/crates/project/src/props.rs +++ b/crates/project/src/props.rs @@ -37,6 +37,11 @@ pub enum ParamUi { Mask, Path, Text, + /// A file path (a string value) chosen with a "Choose…" button, offering files with these + /// extensions: Apply Color LUT's LUT, an OCIO config. + File { + filters: Vec, + }, Gradient, /// Not shown in Effect Controls / the timeline (internal data). Hidden, diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index cfbd6e52c..6e7ec9fc5 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -640,8 +640,9 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V // up Effect Controls on that layer, as in After Effects. if matches!(id, "effect.apply" | "effect.applyLast") { let r = run_engine(app, ctx, id, params.clone()); - if r.is_ok() { + if let Ok(applied) = &r { crate::panels::effect_controls::reveal_applied(app, ¶ms); + crate::panels::effect_controls::choose_files_after_apply(app, applied); } return r; } diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index adab02474..6612187e5 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -79,6 +79,46 @@ pub fn reveal_applied(app: &mut EffectcraftApp, params: &serde_json::Value) { app.raise_panel(crate::dock::PanelKind::EffectControls); } +/// After Effects asks for an effect's file as it applies it (Apply Color LUT opens the file +/// dialog for its LUT). For the instances `effect.apply` made (`applied`, its result), open the +/// file dialog for their first empty file parameter and set the file on each; cancelling +/// leaves the effect without one. Does nothing without a file dialog (tests, agents). +pub fn choose_files_after_apply(app: &mut EffectcraftApp, applied: &serde_json::Value) { + let Some(pick) = app.hooks.pick_files.as_ref() else { return }; + let Some(comp) = app.session.active_comp() else { return }; + let uids: Vec = applied.get("effects").and_then(|v| v.as_array()).map(|a| a.iter().filter_map(|v| v.as_u64()).collect()).unwrap_or_default(); + let mut targets = vec![]; + let mut filters: Vec = vec![]; + for uid in uids { + for l in &comp.layers { + let Some(g) = l.props.find_group(uid) else { continue }; + let mut found = None; + g.walk("", &mut |_, pr| { + if found.is_none() + && let ParamUi::File { filters } = &pr.ui + && matches!(&pr.value, Value::Str(s) if s.trim().is_empty()) + { + found = Some((pr.uid, filters.clone())); + } + }); + if let Some((prop, f)) = found { + targets.push((l.id.0, prop)); + filters = f; + } + } + } + if targets.is_empty() { + return; + } + let exts: Vec<&str> = filters.iter().map(String::as_str).collect(); + let Some(path) = pick(&exts).into_iter().next() else { return }; + for (layer, prop) in targets { + if let Err(e) = app.session.execute("prop.set", json!({"layer": layer, "prop": prop, "value": path})) { + app.ui.status = e.to_string(); + } + } +} + /// An undo-merge key for one gesture on `id`: a new key each time a drag starts, so separate /// drags are separate undo steps while one drag is one step. pub(super) fn gesture_key(ui: &egui::Ui, id: egui::Id, started: bool) -> String { @@ -417,6 +457,59 @@ fn prop_row( let dr = Rect::from_min_size(pos2(vx, cy - 9.0), vec2((r.max.x - vx - 56.0).clamp(80.0, 200.0), 18.0)); super::fx_editors::channel_popup(app, ui, layer, prop, s, dr, actions); } + // A file parameter (Apply Color LUT's LUT, an OCIO config): its file name and Choose…, + // as in After Effects. + Value::Str(s) if matches!(prop.ui, ParamUi::File { .. }) => { + let bw = 70.0; + let br = Rect::from_min_size(pos2(vx, cy - 9.0), vec2(bw, 18.0)); + if widgets::text_button(ui, br, crate::i18n::tr("Choose..."), false, &t, egui::Id::new(("ec-file", uid))).clicked() { + let filters: Vec<&str> = match &prop.ui { + ParamUi::File { filters } => filters.iter().map(String::as_str).collect(), + _ => vec![], + }; + match app.hooks.pick_files.as_ref() { + Some(pick) => { + if let Some(path) = pick(&filters).into_iter().next() { + actions.push(("prop.set".into(), json!({"layer": layer.id.0, "prop": uid, "value": path}))); + } + } + None => app.ui.status = "no file dialog available: set the path with prop.set".into(), + } + } + app.auto.add(&format!("effectControls.prop.{uid}.choose"), br, &prop.name); + let shown = if s.trim().is_empty() { + crate::i18n::tr("None").to_string() + } else if s.contains('\n') { + prop.name.clone() + } else { + effectcraft_engine::sequence::file_name(s) + }; + let nr = Rect::from_min_max(pos2(br.max.x + 8.0, r.min.y), pos2(r.max.x - 8.0, r.max.y)); + let np = p.with_clip_rect(nr.intersect(p.clip_rect())); + np.text(pos2(nr.min.x, cy), Align2::LEFT_CENTER, &shown, Tokens::ui(12.0), if s.trim().is_empty() { t.text_dim } else { t.text }); + let hover = ui.interact(nr, egui::Id::new(("ec-file-name", uid)), Sense::hover()); + if !s.trim().is_empty() && !s.contains('\n') { + hover.on_hover_text(s.as_str()); + } + app.auto.add(&format!("effectControls.prop.{uid}.value"), nr, &shown); + } + // A text parameter (Cryptomatte's Selection, Colorama's Output Cycle, config names): + // typed in a field, set when it loses the focus (Esc keeps the old text). + Value::Str(s) if matches!(prop.ui, ParamUi::Text) => { + let fr = Rect::from_min_size(pos2(vx, cy - 9.0), vec2((r.max.x - vx - 12.0).clamp(80.0, 320.0), 18.0)); + let bid = egui::Id::new(("ec-text", uid)); + let mut buf: String = ui.data(|d| d.get_temp(bid)).unwrap_or_else(|| s.clone()); + let resp = widgets::text_field(ui, fr, &mut buf, "", &t); + app.auto.add(&format!("effectControls.prop.{uid}.value"), fr, &prop.name); + if resp.lost_focus() { + if buf != *s && !ui.input(|i| i.key_pressed(egui::Key::Escape)) { + actions.push(("prop.set".into(), json!({"layer": layer.id.0, "prop": uid, "value": buf}))); + } + ui.data_mut(|d| d.remove::(bid)); + } else if resp.has_focus() { + ui.data_mut(|d| d.insert_temp(bid, buf)); + } + } Value::Gradient(g) => { let gr = Rect::from_min_size(pos2(vx, cy - 7.0), vec2((r.max.x - vx - 56.0).clamp(60.0, 180.0), 14.0)); let n = 48; diff --git a/crates/ui-egui/src/panels/effects_presets.rs b/crates/ui-egui/src/panels/effects_presets.rs index 7b68ba7f2..e60354b15 100644 --- a/crates/ui-egui/src/panels/effects_presets.rs +++ b/crates/ui-egui/src/panels/effects_presets.rs @@ -422,11 +422,12 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { }; match app.session.execute(cmd, params) { Err(e) => app.ui.status = e.to_string(), - Ok(_) => { + Ok(applied) => { if let Apply::Effect(id) = &apply { push_recent(&mut app.ui.effects_recent, id); app.show_panel(crate::dock::PanelKind::EffectControls); app.ui.focused = crate::dock::PanelKind::EffectsPresets; + crate::panels::effect_controls::choose_files_after_apply(app, &applied); } } } diff --git a/crates/ui-egui/tests/ui_effect_controls.rs b/crates/ui-egui/tests/ui_effect_controls.rs index f6137f5f7..551f8ad49 100644 --- a/crates/ui-egui/tests/ui_effect_controls.rs +++ b/crates/ui-egui/tests/ui_effect_controls.rs @@ -574,3 +574,85 @@ fn custom_effect_editors_activate_their_owner() { click(&mut h, graph); assert!(h.state().session.state.selected_props.contains(&(LayerId(x.small), x.curves))); } + +/// #514: Apply Color LUT asks for its LUT as it is applied (as in After Effects), Effect +/// Controls shows the file with a Choose… button that picks another, and the LUT changes the +/// pixels. #473: a text parameter (Cryptomatte's Selection) has a field that sets it. +#[test] +fn file_and_text_parameters_have_widgets() { + let dir = std::env::temp_dir().join(format!("ec-ui-lut-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + // A 2×2×2 LUT that inverts, and one that does nothing. + let cube = |inv: bool| { + let mut t = String::from("LUT_3D_SIZE 2\n"); + for b in 0..2 { + for g in 0..2 { + for r in 0..2 { + let f = |c: i32| if inv { 1 - c } else { c }; + t += &format!("{} {} {}\n", f(r), f(g), f(b)); + } + } + } + t + }; + let (invert, identity) = (dir.join("invert.cube"), dir.join("identity.cube")); + std::fs::write(&invert, cube(true)).unwrap(); + std::fs::write(&identity, cube(false)).unwrap(); + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "LUT", "width": 64, "height": 32, "frameRate": 30, "duration": 1})).unwrap(); + let l = s.execute("layer.newSolid", json!({"name": "Plate", "color": "#ff0000", "width": 64, "height": 32})).unwrap()["layer"].as_u64().unwrap(); + s.state.selected_layers = vec![LayerId(l)]; + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::EffectControls); + let picked = std::sync::Arc::new(std::sync::Mutex::new(invert.to_string_lossy().to_string())); + let pick = picked.clone(); + app.hooks.pick_files = Some(Box::new(move |exts: &[&str]| { + assert_eq!(exts, ["cube"]); + vec![pick.lock().unwrap().clone()] + })); + let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).build_eframe(|_| app); + settle(&mut h); + let ctx = h.ctx.clone(); + let r = effectcraft_ui_egui::menus::invoke(h.state_mut(), &ctx, "effect.apply", json!({"effect": "Apply Color LUT"})).unwrap(); + let fx = r["effects"][0].as_u64().unwrap(); + let lut = |h: &Harness<'_, EffectcraftApp>| { + let g = layer(&h.state().session, l).effects().and_then(|f| f.groups().find(|g| g.uid == fx).cloned()).unwrap(); + let p = g.get("lut").unwrap().clone(); + ( + p.uid, + match p.value { + KV::Str(s) => s, + _ => String::new(), + }, + ) + }; + assert_eq!(lut(&h).1, invert.to_string_lossy(), "the LUT was asked for on apply"); + let px = |h: &Harness<'_, EffectcraftApp>| { + let s = &h.state().session; + s.render(s.active_comp_id().unwrap(), effectcraft_engine::time::Tick::ZERO, Default::default()).get(10, 10) + }; + assert!(px(&h)[0] < 0.05 && px(&h)[1] > 0.95, "red inverted to cyan: {:?}", px(&h)); + settle(&mut h); + let uid = lut(&h).0; + assert!(h.state().auto.find(&format!("effectControls.prop.{uid}.value")).is_some_and(|e| e.label == "invert.cube")); + *picked.lock().unwrap() = identity.to_string_lossy().to_string(); + let at = rect_of(&h, &format!("effectControls.prop.{uid}.choose")).center(); + click(&mut h, at); + assert_eq!(lut(&h).1, identity.to_string_lossy()); + assert!(px(&h)[0] > 0.95, "{:?}", px(&h)); + + // A text parameter: type into its field; it is set when the field loses the focus. + let crypto = h.state_mut().session.execute("effect.apply", json!({"layers": [l], "effect": "ec.3d.cryptomatte"})).unwrap()["effects"][0].as_u64().unwrap(); + settle(&mut h); + let sel = + layer(&h.state().session, l).effects().and_then(|f| f.groups().find(|g| g.uid == crypto).and_then(|g| g.get("selection")).map(|p| p.uid)).unwrap(); + let at = rect_of(&h, &format!("effectControls.prop.{sel}.value")).center(); + click(&mut h, at); + h.event(egui::Event::Text("left*".into())); + h.step(); + h.key_press(egui::Key::Enter); + h.run_steps(3); + let v = layer(&h.state().session, l).props.find(sel).unwrap().value.clone(); + assert_eq!(v, KV::Str("left*".into())); + let _ = std::fs::remove_dir_all(dir); +} diff --git a/docs/effects.md b/docs/effects.md index 4073251ae..e498cf1ae 100644 --- a/docs/effects.md +++ b/docs/effects.md @@ -412,7 +412,7 @@ Effects are addressed by id (`ec..`) or display name in commands | Effect | Id | Params | GPU | 32 | Status | |---|---|---:|:-:|:-:|---| -| Apply Color LUT | `ec.utility.applylut` | 0 | GPU | 32 | Implemented | +| Apply Color LUT | `ec.utility.applylut` | 1 | GPU | 32 | Implemented | | CC Overbrights | `ec.utility.ccoverbrights` | 2 | GPU | 32 | Implemented | | Cineon Converter | `ec.utility.cineon` | 7 | GPU | 32 | Implemented | | Color Profile Converter | `ec.utility.colorprofileconverter` | 7 | GPU | 32 | Implemented |