From 1765dc53dfbbc87a1f22a8ac720552951cb0b793 Mon Sep 17 00:00:00 2001 From: Jannik Wappler <167129173+RavoxX@users.noreply.github.com> Date: Tue, 13 Oct 2026 04:33:43 -0400 Subject: [PATCH] Tint: Swap Colors (#963) (#1060) Squash of #1060 by @RavoxX, without the docs/images screenshots (kept out of the repo). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/effect.rs | 40 ++++++++- crates/engine/src/tests_effects.rs | 94 ++++++++++++++++++++ crates/ui-egui/src/i18n/ui.rs | 6 ++ crates/ui-egui/src/panels/effect_controls.rs | 11 +++ crates/ui-egui/tests/ui_effect_controls.rs | 34 +++++++ docs/effects-parity.md | 3 +- 6 files changed, 186 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/commands/effect.rs b/crates/engine/src/commands/effect.rs index 1e67e071..30237ede 100644 --- a/crates/engine/src/commands/effect.rs +++ b/crates/engine/src/commands/effect.rs @@ -117,7 +117,7 @@ fn find_fx(s: &Session, p: &Value, cmd: &str) -> Result<(effectcraft_project::It let g = match p.get("effect") { Some(Value::Number(n)) => { let n = n.as_u64().unwrap_or(0); - fx.groups().find(|g| g.uid == n).or_else(|| fx.groups().nth(n.saturating_sub(1) as usize)) + fx.groups().find(|g| g.uid == n).or_else(|| usize::try_from(n.saturating_sub(1)).ok().and_then(|i| fx.groups().nth(i))) } Some(Value::String(name)) => fx.groups().find(|g| &g.name == name || g.match_id == *name), _ => s.state.selected_props.iter().find_map(|(l, u)| (*l == lid).then(|| fx.groups().find(|g| g.uid == *u)).flatten()), @@ -416,6 +416,35 @@ fn reset(s: &mut Session, p: &Value) -> Result { Ok(Value::Null) } +/// Tint's Swap Colors button edits both swatches at the CTI in one undo step. Like Reset, +/// it keys animated parameters without moving their existing keys or replacing expressions. +fn swap_tint_colors(s: &mut Session, p: &Value) -> Result { + let cmd = "effect.swapTintColors"; + if !p.is_object() || p.get("effect").is_some_and(|v| !v.is_string() && v.as_u64().is_none_or(|n| n == 0)) { + return Err(bad(cmd, "effect must be a positive index, uid or name")); + } + let (cid, lid, uid) = find_fx(s, p, cmd)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let layer = comp.layer(lid).ok_or(EngineError::NoComp)?; + let g = layer.props.find_group(uid).filter(|g| g.match_id == "ec.color.tint").ok_or_else(|| bad(cmd, "not a Tint effect"))?; + let lt = layer.layer_time(s.time_of(cid)); + let color = |id| { + g.prop(id) + .map(|pr| pr.value_at(lt)) + .filter(|v| matches!(v, effectcraft_keyframe::Value::Color(_))) + .ok_or_else(|| bad(cmd, "Tint is missing a colour parameter")) + }; + let (black, white) = (color("black")?, color("white")?); + s.edit("Swap Tint Colors", None, |proj, _| { + let g = layer_mut(proj, cid, lid)?.props.find_group_mut(uid).ok_or_else(|| bad(cmd, "effect gone"))?; + for (id, value) in [("black", white), ("white", black)] { + g.prop_mut(id).ok_or_else(|| bad(cmd, "Tint is missing a colour parameter"))?.set_value_at(lt, value); + } + Ok(()) + })?; + Ok(json!({"effect": uid})) +} + fn last(s: &mut Session, p: &Value) -> Result { let id = s.state.last_effect.clone().ok_or_else(|| bad("effect.applyLast", "no effect applied yet"))?; let mut q = p.clone(); @@ -507,6 +536,15 @@ pub fn specs() -> Vec { cmd!("effect.copy", "Copy Effects", [], None, "{layer?, effect? | effects?: [uid|name|index]} (default: the selected effects)", has_layers, copy), cmd!("effect.paste", "Paste Effects", [], None, "{layers?} — adds the copied effects to the layers", has_layers, paste), cmd!("effect.reset", "Reset Effect", [], None, "{layer?, effect}", has_layers, reset), + cmd!( + "effect.swapTintColors", + "Swap Tint Colors", + [], + None, + "{comp?, layer?, effect?: index|uid|name} — swaps Tint's Map Black To and Map White To at the current layer time in one undo step", + has_layers, + swap_tint_colors + ), crate::query!( "effect.list", "List Effects", diff --git a/crates/engine/src/tests_effects.rs b/crates/engine/src/tests_effects.rs index bec84b9a..83ea9c04 100644 --- a/crates/engine/src/tests_effects.rs +++ b/crates/engine/src/tests_effects.rs @@ -147,6 +147,100 @@ fn reset_keys_animated_params_at_the_cti() { assert_eq!(pr.keys[1].value, KV::Scalar(0.0)); } +#[test] +fn tint_swaps_only_its_colors_in_one_undo_step() { + let (mut s, a, b) = comp_with_two_solids(); + let first = apply(&mut s, a, "Tint"); + let target = apply(&mut s, a, "Tint"); + let other = apply(&mut s, b, "Tint"); + for (path, value) in [("black", json!([1.0, 0.0, 0.0, 1.0])), ("white", json!([0.0, 0.25, 1.0, 1.0])), ("amount", json!(45.0))] { + s.execute("prop.set", json!({"layer": a, "path": format!("effects/#2/{path}"), "value": value})).unwrap(); + } + let before_a = layer(&s, a); + let before_b = layer(&s, b); + let n = s.history.undo.len(); + s.execute("effect.swapTintColors", json!({"layer": a, "effect": target})).unwrap(); + let swapped = layer(&s, a); + let g = swapped.props.find_group(target).unwrap(); + assert_eq!(g.prop("black").unwrap().value, KV::Color([0.0, 0.25, 1.0, 1.0])); + assert_eq!(g.prop("white").unwrap().value, KV::Color([1.0, 0.0, 0.0, 1.0])); + assert_eq!(g.prop("amount").unwrap().value, KV::Scalar(45.0)); + assert_eq!(swapped.props.find_group(first), before_a.props.find_group(first)); + assert_eq!(layer(&s, b).props.find_group(other), before_b.props.find_group(other)); + assert_eq!(s.history.undo.len(), n + 1); + assert!(s.undo()); + assert_eq!(layer(&s, a).props, before_a.props); + assert!(s.redo()); + assert_eq!(layer(&s, a).props, swapped.props); +} + +#[test] +fn tint_swap_keys_animated_colors_at_layer_time() { + let (mut s, a, _) = comp_with_two_solids(); + let target = apply(&mut s, a, "Tint"); + s.execute("prop.set", json!({"layer": a, "path": "effects/#1/black", "value": [1.0, 0.0, 0.0, 1.0]})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "effects/#1/white", "value": [0.0, 0.0, 1.0, 1.0]})).unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": a, "path": "effects/#1/black"})).unwrap(); + s.execute("time.set", json!({"time": 2.0})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "effects/#1/black", "value": [0.0, 1.0, 0.0, 1.0]})).unwrap(); + s.execute("layer.timeStretch", json!({"layers": [a], "percent": 200.0})).unwrap(); + // At comp time 2s the stretched layer is at 1s, between its unchanged 0s and 2s keys. + let before = layer(&s, a); + let g = before.props.find_group(target).unwrap(); + let lt = before.layer_time(s.time()); + let expected = g.prop("black").unwrap().value_at(lt); + let keys = g.prop("black").unwrap().keys.clone(); + select_effect(&mut s, a, target); + s.execute("effect.swapTintColors", json!({})).unwrap(); + let after = layer(&s, a); + let g = after.props.find_group(target).unwrap(); + let black = g.prop("black").unwrap(); + assert_eq!(black.keys.len(), 3); + assert_eq!(black.keys[0], keys[0]); + assert_eq!(black.keys[2], keys[1]); + assert_eq!(black.keys[1].time, lt); + assert_eq!(black.keys[1].value, KV::Color([0.0, 0.0, 1.0, 1.0])); + assert_eq!(g.prop("white").unwrap().value, expected); + assert!(g.prop("white").unwrap().keys.is_empty()); + assert!(s.undo()); + assert_eq!(layer(&s, a).props, before.props); +} + +#[test] +fn tint_swap_rejects_invalid_targets_without_editing() { + let mut empty = Session::default(); + assert!(empty.execute("effect.swapTintColors", json!({})).is_err()); + let (mut s, a, _) = comp_with_two_solids(); + let blur = apply(&mut s, a, "Gaussian Blur"); + let tint = apply(&mut s, a, "Tint"); + for params in [ + json!({"layer": a, "effect": blur}), + json!({"layer": a, "effect": "missing"}), + json!({"layer": a, "effect": []}), + json!({"layer": a, "effect": 1.5}), + json!({"layer": a, "effect": 0}), + json!(false), + ] { + let before = layer(&s, a); + let n = s.history.undo.len(); + assert!(s.execute("effect.swapTintColors", params).is_err()); + assert_eq!(layer(&s, a).props, before.props); + assert_eq!(s.history.undo.len(), n); + } + // A damaged project's colour parameter must also fail atomically. + let cid = s.active_comp_id().unwrap(); + s.edit("Break Tint", None, |p, _| { + p.comp_mut(cid).unwrap().layer_mut(LayerId(a)).unwrap().props.find_group_mut(tint).unwrap().prop_mut("white").unwrap().value = KV::Scalar(1.0); + Ok(()) + }) + .unwrap(); + let before = layer(&s, a); + let n = s.history.undo.len(); + assert!(s.execute("effect.swapTintColors", json!({"layer": a, "effect": tint})).is_err()); + assert_eq!(layer(&s, a).props, before.props); + assert_eq!(s.history.undo.len(), n); +} + #[test] fn copy_layers_clears_effect_clipboard() { let (mut s, a, b) = comp_with_two_solids(); diff --git a/crates/ui-egui/src/i18n/ui.rs b/crates/ui-egui/src/i18n/ui.rs index b38ee1a3..89152356 100644 --- a/crates/ui-egui/src/i18n/ui.rs +++ b/crates/ui-egui/src/i18n/ui.rs @@ -486,6 +486,7 @@ const JAPANESE: &[(&str, &str)] = &[ ("Stretch Factor:", "伸縮率:"), ("Stylistic Sets", "スタイルセット"), ("Subpixel Positioning", "サブピクセル位置決め"), + ("Swap Colors", "色を入れ替え"), ("Swap Paint Colors (X)", "ペイントカラーを入れ替え(X)"), ("Sync with System", "システムに合わせる"), ("System Default", "システムの既定"), @@ -1025,6 +1026,7 @@ const SIMPLIFIED_CHINESE: &[(&str, &str)] = &[ ("Stretch Factor:", "伸展系数："), ("Stylistic Sets", "样式集"), ("Subpixel Positioning", "次像素定位"), + ("Swap Colors", "交换颜色"), ("Swap Paint Colors (X)", "交换绘画颜色（X）"), ("Sync with System", "与系统同步"), ("System Default", "系统默认"), @@ -1755,6 +1757,7 @@ const TRADITIONAL_CHINESE: &[(&str, &str)] = &[ ("Stretch Factor:", "伸展係數："), ("Stylistic Sets", "樣式集"), ("Subpixel Positioning", "次像素定位"), + ("Swap Colors", "交換顏色"), ("Swap Paint Colors (X)", "交換繪畫顏色（X）"), ("Sync with System", "與系統同步"), ("System Default", "系統預設"), @@ -2493,6 +2496,7 @@ const UKRAINIAN: &[(&str, &str)] = &[ ("Stretch Factor:", "Коефіцієнт розтягнення:"), ("Stylistic Sets", "Стилістичні набори"), ("Subpixel Positioning", "Субпіксельне позиціювання"), + ("Swap Colors", "Поміняти кольори"), ("Swap Paint Colors (X)", "Поміняти місцями кольори малювання (X)"), ("Sync with System", "Синхронізувати з системою"), ("System Default", "Стандартне системне"), @@ -3035,6 +3039,7 @@ const PORTUGUESE: &[(&str, &str)] = &[ ("Stretch Factor:", "Fator de esticamento:"), ("Stylistic Sets", "Conjuntos estilísticos"), ("Subpixel Positioning", "Posicionamento em subpixel"), + ("Swap Colors", "Trocar cores"), ("Swap Paint Colors (X)", "Trocar cores de pintura (X)"), ("System Default", "Padrão do sistema"), ("Table", "Tabela"), @@ -3486,6 +3491,7 @@ const CZECH: &[(&str, &str)] = &[ ("Stretch Factor:", "Koeficient roztažení:"), ("Stylistic Sets", "Stylistické sady"), ("Subpixel Positioning", "Subpixelové umístění"), + ("Swap Colors", "Prohodit barvy"), ("Swap Paint Colors (X)", "Prohodit barvy malování (X)"), ("Sync with System", "Synchronizovat se systémem"), ("System Default", "Výchozí systémové"), diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index d61936ba..bad1db39 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1347,6 +1347,17 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } let scope = FxScope { effect: effect.as_str(), root: g, path: String::new() }; group_rows(app, ui, &bp, &layer, g, &ectx, body, &mut y, 0, &scope, &mut actions); + if effect == "ec.color.tint" { + let r = Rect::from_min_size(pos2(body.min.x + PARAM_NAME_X, y + 2.0), vec2(120.0, ROW)); + let label = crate::i18n::tr("Swap Colors"); + let resp = widgets::text_button(ui, r, label, false, &t, egui::Id::new(("ec-tint-swap", g.uid))); + app.auto.add_response(&format!("effectControls.effect.{}.swapColors", g.uid), &resp, label); + if resp.clicked() { + actions.push(("prop.select".into(), json!({"layer": layer.id.0, "prop": g.uid}))); + actions.push(("effect.swapTintColors".into(), json!({"layer": layer.id.0, "effect": g.uid}))); + } + y += ROW + 4.0; + } } y += 4.0; } diff --git a/crates/ui-egui/tests/ui_effect_controls.rs b/crates/ui-egui/tests/ui_effect_controls.rs index b019c2ef..cdf8c703 100644 --- a/crates/ui-egui/tests/ui_effect_controls.rs +++ b/crates/ui-egui/tests/ui_effect_controls.rs @@ -135,6 +135,40 @@ fn effect_controls_widgets_register_automation_ids() { assert!(ids(&h).contains(&pt_id), "effect point control shown for the selected effect"); } +#[test] +fn tint_swap_button_targets_its_instance_and_undoes_both_colors() { + let mut s = Session::default(); + s.execute("comp.new", json!({"width": 640, "height": 360, "frameRate": 30, "duration": 5})).unwrap(); + let lid = s.execute("layer.newSolid", json!({"color": "#000000"})).unwrap()["layer"].as_u64().unwrap(); + let first = s.execute("effect.apply", json!({"effect": "Tint"})).unwrap()["effects"][0].as_u64().unwrap(); + let target = s.execute("effect.apply", json!({"effect": "Tint"})).unwrap()["effects"][0].as_u64().unwrap(); + let blur = s.execute("effect.apply", json!({"effect": "Gaussian Blur"})).unwrap()["effects"][0].as_u64().unwrap(); + let mut app = EffectcraftApp::new(s); + app.show_panel(PanelKind::EffectControls); + let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).build_eframe(|_| app); + h.run_steps(4); + let id = format!("effectControls.effect.{target}.swapColors"); + let rect = h.state().auto.find(&id).unwrap().rect; + assert!(ids(&h).contains(&format!("effectControls.effect.{first}.swapColors"))); + assert!(!ids(&h).contains(&format!("effectControls.effect.{blur}.swapColors"))); + let n = h.state().session.history.undo.len(); + click(&mut h, egui::pos2(rect[0] + rect[2] / 2.0, rect[1] + rect[3] / 2.0)); + let l = layer(&h.state().session, lid); + let g = l.props.find_group(target).unwrap(); + assert_eq!(g.prop("black").unwrap().value, KV::Color([1.0; 4])); + assert_eq!(g.prop("white").unwrap().value, KV::Color([0.0, 0.0, 0.0, 1.0])); + assert_eq!(l.props.find_group(first).unwrap().prop("black").unwrap().value, KV::Color([0.0, 0.0, 0.0, 1.0])); + assert_eq!(h.state().session.history.undo.len(), n + 1); + assert_eq!(h.state().session.state.selected_props, vec![(LayerId(lid), target)]); + assert!(h.state_mut().session.undo()); + let l = layer(&h.state().session, lid); + assert_eq!(l.props.find_group(target).unwrap().prop("black").unwrap().value, KV::Color([0.0, 0.0, 0.0, 1.0])); + assert_eq!(l.props.find_group(target).unwrap().prop("white").unwrap().value, KV::Color([1.0; 4])); + h.state_mut().ui.fx_closed.insert(target); + h.run_steps(2); + assert!(!h.state().auto.elements.iter().any(|e| e.id == id)); +} + #[test] fn crosshair_and_eyedropper_pick_from_the_viewer() { let (app, x) = app(); diff --git a/docs/effects-parity.md b/docs/effects-parity.md index a293d60b..c5fbc5ba 100644 --- a/docs/effects-parity.md +++ b/docs/effects-parity.md @@ -30,7 +30,7 @@ corpus compares renders, each effect is assumed to differ somewhere, and users k | Audio | 13 | 0 | CPU by design (applied in the mixdown) | | Blur & Sharpen | 16 | 15 | Gaussian Blur interaction and output to be measured first ([fidelity/ae-2026-acceptance.md](fidelity/ae-2026-acceptance.md)) | | Channel | 13 | 13 | — | -| Color Correction | 40 | 40 | ACES 2.0 / OCIO 2.5 (26.5); Apply Color LUT (#514) and 32-bpc blending (#494) are closed | +| Color Correction | 40 | 40 | Tint has Swap Colors (#963); ACES 2.0 / OCIO 2.5 (26.5); Apply Color LUT (#514) and 32-bpc blending (#494) are closed | | Distort | 39 | 35 | Liquify has no viewer brush; #303 (Mesh Warp drag) and #534 (CC Power Pin Unstretch) are closed | | Expression Controls | 9 | 9 | — | | Generate | 26 | 25 | Fractal on the CPU (needs f64); #526 (Reset) and #536 (Write-on marks) are closed | @@ -66,4 +66,5 @@ the GPU is performance, not parity. |---|---|---| | 2026-10-12 | minor | Parameters 3,014 to 3,017 (effects.md); #303, #514, #526, #533–#536 marked closed instead of pending | | 2026-10-12 | minor | Effect colour labels (26.5) added in Effect Controls | +| 2026-10-12 | minor | Tint's Swap Colors button and agent-drivable command (#963); undo/redo, instance targeting, animated layer-time edits and invalid-input tests | | 2026-10-10 | major | First effects parity checklist |