diff --git a/crates/automation/src/backend.rs b/crates/automation/src/backend.rs index 8f7931239..5bba7172a 100644 --- a/crates/automation/src/backend.rs +++ b/crates/automation/src/backend.rs @@ -4,7 +4,7 @@ use effectcraft_engine::Session; use effectcraft_engine::time::Tick; use serde_json::{Value, json}; -use crate::confine::Confinement; +use crate::confine::{Confinement, Lookup}; use crate::{BridgeClient, Error, Result, base64, encode_png, recompress_png}; pub struct Backend { @@ -63,12 +63,11 @@ impl Backend { pub fn exec(&mut self, id: &str, params: Value) -> Result { let params = if params.is_null() { json!({}) } else { params }; if let Some(c) = self.confinement.clone() { - let lookup = match c.check_command(id, ¶ms)? { - Some(l @ (crate::confine::Lookup::ProjectFile | crate::confine::Lookup::QueueDefault)) => Some((l, self.run("project.summary", json!({}))?)), - Some(crate::confine::Lookup::RenderOutputs) => Some((crate::confine::Lookup::RenderOutputs, self.run("renderQueue.list", json!({}))?)), - None => None, - }; - if let Some((lookup, found)) = lookup { + for lookup in c.check_command(id, ¶ms)? { + let found = match lookup { + Lookup::ProjectFile | Lookup::QueueDefault => self.run("project.summary", json!({}))?, + Lookup::RenderOutputs => self.run("renderQueue.list", json!({}))?, + }; c.check_lookup(&lookup, &found)?; } } diff --git a/crates/automation/src/confine.rs b/crates/automation/src/confine.rs index 9b126bdcb..0e9d79d4b 100644 --- a/crates/automation/src/confine.rs +++ b/crates/automation/src/confine.rs @@ -6,10 +6,11 @@ //! `engine.batch`) is checked before it runs: its file parameters must be absolute paths inside //! the roots (symbolic links resolved, no `..`), any other absolute path in its parameters too, //! and commands that write to a file they are not given (Save, Increment and Save, rendering the -//! Render Queue) are checked against where they would write. Commands that run code or open -//! other applications (scripts, ScriptUI panels and the events that reach them, Execute File, -//! Reveal in Finder…) are refused while confined, as is granting scripts the file and network -//! access the user withheld. +//! Render Queue) are checked against where they would write. Queueing a render without naming +//! its output needs a saved project, because the default output of an unsaved one is the home +//! folder. Commands that run code or open other applications (scripts, ScriptUI panels and the +//! events that reach them, Execute File, Reveal in Finder…) are refused while confined, as is +//! granting scripts the file and network access the user withheld. use std::path::{Component, Path, PathBuf}; @@ -121,6 +122,9 @@ const WRITES: &[(&str, &[&str])] = &[ /// Commands that write where they are not told: the project's file, the Render Queue's outputs. const IMPLICIT_WRITES: &[&str] = &["file.save", "file.incrementAndSave", "renderQueue.render", "render.preRender"]; +/// Commands that queue a render, whose output defaults to the project's folder when not given. +const QUEUES_AN_OUTPUT: &[&str] = &["renderQueue.add", "render.preRender", "file.createProxy"]; + /// What must be looked up before an implicitly writing command may run. #[derive(Debug, PartialEq, Eq)] pub enum Lookup { @@ -128,8 +132,9 @@ pub enum Lookup { ProjectFile, /// Every output of the queued Render Queue items (`renderQueue.list`). RenderOutputs, - /// Whether the open project has a file (`project.summary` → `path`): a Render Queue item - /// added without `output` writes beside it, or to the home folder before the first save. + /// Whether the open project has a file (`project.summary` → `path`): a render queued without + /// `output` (Render Queue, Pre-render, Create Proxy) writes beside it, or to the home folder + /// before the first save. QueueDefault, } @@ -169,7 +174,7 @@ impl Confinement { /// Check engine command `id` with `params` before it runs. Returns what must still be looked /// up and passed to [`Confinement::check_lookup`] for commands that write implicitly. - pub fn check_command(&self, id: &str, params: &Value) -> Result> { + pub fn check_command(&self, id: &str, params: &Value) -> Result> { if REFUSED.contains(&id) || effectcraft_engine::commands::scripts::RUNS_SCRIPTS.contains(&id) { return Err(Error::BadArgs(format!("`{id}` is not available while file access is confined (--automation-read-root/--automation-write-root)"))); } @@ -192,11 +197,11 @@ impl Confinement { if named.iter().chain(mixed).filter_map(|k| inner_params.get(*k)).any(has_reference) { return Err(Error::BadArgs(format!("`{inner}` in a batch: file paths can't be `$N` references while file access is confined"))); } - if self.check_command(inner, &inner_params)?.is_some() { + if !self.check_command(inner, &inner_params)?.is_empty() { return Err(Error::BadArgs(format!("`{inner}` writes to its project's or queue's files: run it on its own while file access is confined"))); } } - return Ok(None); + return Ok(vec![]); } let (named, access) = match (keys_of(WRITES, id), keys_of(READS, id)) { (Some(k), _) => (k, Access::Write), @@ -205,6 +210,10 @@ impl Confinement { (None, None) => (&[][..], Access::Read), }; let mut given = false; + // Whether one of them is a file: not a Render Queue template, whose own output is relative + // and resolved like one the agent never gave, and not an array, which the commands that + // take a single output read as nothing at all. + let mut gave_a_file = false; for key in named { let Some(v) = params.get(*key).filter(|v| !v.is_null()) else { continue }; given = true; @@ -217,6 +226,7 @@ impl Confinement { match p.as_str() { Some(s) if matches!(id, "render.preRender" | "file.createProxy") && !looks_like_path(s) => {} Some(s) => { + gave_a_file |= v.is_string(); self.check(s, access)?; } None => return Err(Error::BadArgs(format!("`{id}`: `{key}` must be a path"))), @@ -242,14 +252,19 @@ impl Confinement { if matches!(id, "file.openRecent" | "file.importRecent" | "anim.applyRecentPreset") && !given { return Err(Error::BadArgs(format!("`{id}`: give `path` while file access is confined"))); } - // Without `output`, an unsaved project's item would write to the home folder (#2479). - if id == "renderQueue.add" && !given { - return Ok(Some(Lookup::QueueDefault)); + let mut lookups = vec![]; + // An output the agent did not name is derived from where the project sits: the Default + // Output Folder preference when that is set, beside the project file otherwise — and the + // home folder when the project has never been saved, outside every root (#2479). Only + // that last case is certainly outside, and reading the preference here would duplicate + // the engine's precedence, so an unsaved project is told to name its output instead. + if QUEUES_AN_OUTPUT.contains(&id) && !gave_a_file { + lookups.push(Lookup::QueueDefault); } if IMPLICIT_WRITES.contains(&id) && !(given && id == "file.save") { - return Ok(Some(if id == "file.save" || id == "file.incrementAndSave" { Lookup::ProjectFile } else { Lookup::RenderOutputs })); + lookups.push(if id == "file.save" || id == "file.incrementAndSave" { Lookup::ProjectFile } else { Lookup::RenderOutputs }); } - Ok(None) + Ok(lookups) } /// Check what a [`Lookup`] found: `project.summary` or `renderQueue.list`'s result. @@ -264,7 +279,7 @@ impl Confinement { Lookup::QueueDefault => match found.get("path").and_then(Value::as_str) { Some(_) => Ok(()), None => Err(Error::BadArgs( - "`renderQueue.add`: the project has no file yet, so the default output would go to the home folder: give `output` inside an automation write root".into(), + "the project has no file yet, so the default output would go to the home folder: give `output` inside an automation write root, or save the project inside one first".into(), )), }, Lookup::RenderOutputs => { diff --git a/crates/automation/src/tests.rs b/crates/automation/src/tests.rs index 68376fa46..ff7eedef8 100644 --- a/crates/automation/src/tests.rs +++ b/crates/automation/src/tests.rs @@ -1479,6 +1479,59 @@ fn automation_roots_confine_file_access() { let _ = std::fs::remove_dir_all(&base); } +/// #2479: a queued output the agent didn't name is derived from where the project sits, so on a +/// project that has never been saved it lands in the home folder, outside every automation root. +/// Queueing it there is refused, rather than leaving an item that can never be rendered. +#[test] +fn automation_roots_refuse_a_queued_output_from_an_unsaved_project() { + use crate::confine::Confinement; + let base = std::env::temp_dir().join(format!("ec-mcp-default-out-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let write = base.join("out"); + std::fs::create_dir_all(&write).unwrap(); + let p = |f: &str| write.join(f).to_string_lossy().into_owned(); + let mut s = server(); + s.backend().confine(Confinement::new(&[], std::slice::from_ref(&write)).unwrap()); + call_json(&mut s, "execute_command", json!({"command": "comp.new", "params": {"name": "A", "width": 64, "height": 64}})); + // Never saved: neither way of queueing a render may fall back to the home folder. + for command in ["renderQueue.add", "render.preRender", "file.createProxy"] { + let (c, err) = call(&mut s, "execute_command", json!({"command": command, "params": {"comp": "A"}})); + assert!(err, "`{command}` without an output should be refused: {c:?}"); + let msg = c[0]["text"].as_str().unwrap_or_default(); + assert!(msg.contains("the project has no file yet"), "`{command}`: {msg}"); + } + // An array where a path goes: the engine reads a scalar, so this is no output either. + let (c, err) = call(&mut s, "execute_command", json!({"command": "file.createProxy", "params": {"comp": "A", "path": [p("arr.mov")]}})); + assert!(err, "an array where the output goes should be refused: {c:?}"); + assert!(c[0]["text"].as_str().unwrap_or_default().contains("the project has no file yet"), "{c:?}"); + // Naming an Output Module template instead of a file is the same: its own output is relative. + let (c, err) = call(&mut s, "execute_command", json!({"command": "render.preRender", "params": {"comp": "A", "output": "Lossless"}})); + assert!(err, "a template output should be refused too: {c:?}"); + assert!(c[0]["text"].as_str().unwrap_or_default().contains("the project has no file yet"), "{c:?}"); + let queue = call_json(&mut s, "execute_command", json!({"command": "renderQueue.list"})); + assert_eq!(queue["items"].as_array().map(Vec::len), Some(0), "nothing was queued: {queue}"); + // Naming an output inside a write root is how it is done. + call_json(&mut s, "execute_command", json!({"command": "renderQueue.add", "params": {"comp": "A", "output": p("named.mp4")}})); + // A proxy named outside the roots is refused. Whether that reads as a write or a read is + // #1347's to classify, so this asserts the refusal and not its wording. The path is a sibling + // of the write root rather than `/elsewhere/…`, which is not absolute on Windows and would be + // refused for that instead. + let outside = base.join("elsewhere").join("proxy.mov").to_string_lossy().into_owned(); + let (c, err) = call(&mut s, "execute_command", json!({"command": "file.createProxy", "params": {"comp": "A", "path": outside}})); + assert!(err, "a proxy outside the roots should be refused: {c:?}"); + assert!(c[0]["text"].as_str().unwrap_or_default().contains("outside the automation roots"), "{c:?}"); + // Once the project is saved there, the default output (beside it) is queued as before. + call_json(&mut s, "save_project", json!({"path": p("proj.ecproj")})); + // A name that is not a path is an Output Module template, and the output it resolves to lands + // beside the saved project: inside the write root, so it is allowed (#1347 relies on this). + call_json(&mut s, "execute_command", json!({"command": "file.createProxy", "params": {"comp": "A", "path": "ProxyTemplate"}})); + call_json(&mut s, "execute_command", json!({"command": "renderQueue.add", "params": {"comp": "A"}})); + let queue = call_json(&mut s, "execute_command", json!({"command": "renderQueue.list"})); + let out = queue["items"][1]["outputPath"].as_str().unwrap_or_default(); + assert!(out.starts_with(&*write.to_string_lossy()), "{queue}"); + let _ = std::fs::remove_dir_all(&base); +} + /// #2131 (audit F-11, "Running code"): `script.run` and the Scripts menu are refused while file /// access is confined, but a ScriptUI panel's script runs just as freely — when the panel opens, /// when an event reaches one of its controls, or when one of its tasks comes due — and @@ -1546,7 +1599,7 @@ fn blender_export_requires_an_absolute_path_in_a_write_root() { } let confinement = Confinement::new(std::slice::from_ref(&read), std::slice::from_ref(&write)).unwrap(); let check = |path: &std::path::Path| confinement.check_command("camera.exportBlender", &json!({"path": path})); - assert_eq!(check(&write.join("scene.py")).unwrap(), None); + assert!(check(&write.join("scene.py")).unwrap().is_empty()); for path in [read.join("scene.py"), base.join("outside.py"), write.join("../read/scene.py")] { assert!(check(&path).is_err(), "must deny {}", path.display()); } diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 8442ad74e..1168c431b 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -164,7 +164,7 @@ pub use enabled::always; pub(crate) use enabled::{has_comp, has_keys, has_layers, has_project_selection, not_yet, not_yet_run}; pub use params::match_path_of; pub(crate) use params::{ - b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_p, merge_p, popup_options, resolve_effect_path, resolve_layer, selectable, + b_p, bad, comp_id, f_p, frontend, layer_mut, layer_p, layers_or, layers_p, merge_p, popup_options, resolve_effect_path, resolve_layer, selectable, selected_leaf_props, str_p, time_p, unlocked, visible_p, }; pub use schema::{accepted_params, check_params, params_schema}; diff --git a/crates/engine/src/cmd/anim.rs b/crates/engine/src/cmd/anim.rs index e711392e0..e23fc736f 100644 --- a/crates/engine/src/cmd/anim.rs +++ b/crates/engine/src/cmd/anim.rs @@ -56,6 +56,10 @@ fn compatible(a: &Property, b: &effectcraft_keyframe::Value) -> bool { a.value.kind_name() == b.kind_name() && a.value.components().len() == b.components().len() } +pub(crate) fn locked_selection(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { + selected.iter().filter(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)).copied().collect() +} + /// Paste copied keyframes at the current time into the target layers' properties. fn paste(s: &mut Session, p: &Value) -> Result { let clip = s.state.key_clipboard.clone(); @@ -90,6 +94,9 @@ fn paste(s: &mut Session, p: &Value) -> Result { let mut plan: Vec<(LayerId, Uid, usize)> = vec![]; for (ti, lid) in targets.iter().enumerate() { let Some(l) = comp.layer(*lid) else { continue }; + if l.switches.locked { + continue; + } for (ci, c) in clip.iter().enumerate() { if paired && sources.get(ti) != Some(&c.layer) { continue; @@ -111,11 +118,12 @@ fn paste(s: &mut Session, p: &Value) -> Result { } } if plan.is_empty() { - return Err(bad("keys.paste", "no compatible property to paste into (select a property of the same type)")); + return Err(bad("keys.paste", "no compatible unlocked property to paste into (unlock the target or select a property of the same type)")); } s.edit("Paste Keyframes", merge_p(p), |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; - let mut sel = vec![]; + let mut sel = locked_selection(comp, &st.selected_keys); + let retained = sel.len(); for (lid, uid, ci) in &plan { let Some(l) = comp.layer_mut(*lid) else { continue }; let times: Vec = clip[*ci].keys.iter().map(|k| l.layer_time(t + k.time)).collect(); @@ -137,8 +145,9 @@ fn paste(s: &mut Session, p: &Value) -> Result { } retime_roving(&mut pr.keys, pr.spatial); } + let pasted = sel.len().saturating_sub(retained); st.selected_keys = sel; - Ok(json!(st.selected_keys.len())) + Ok(json!(pasted)) }) } @@ -176,6 +185,9 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let t = time_p(s, p, Some(comp)); let target = comp.layer(lid).ok_or(EngineError::NoComp)?; + if target.switches.locked { + return Err(bad("keys.paste", "the target layer is locked")); + } let mut order: Vec = (0..path.vertices.len()).collect(); if path.closed && !order.is_empty() { order.push(0); @@ -203,6 +215,9 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L }) .collect(); s.edit("Paste Keyframes", merge_p(p), |proj, st| { + let comp = proj.comp(cid).ok_or(EngineError::NoComp)?; + let mut selected = locked_selection(comp, &st.selected_keys); + let retained = selected.len(); let l = super::layer_mut(proj, cid, lid)?; let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.paste", "no property"))?; let mut sel = vec![]; @@ -217,8 +232,10 @@ pub(super) fn paste_path(s: &mut Session, p: &Value, (cid, lid, uid): (ItemId, L // Re-timing keeps the keys' order, so the pasted keys are found again by index. let pasted: Vec = pr.keys.iter().enumerate().filter(|(_, k)| sel.contains(&k.time)).map(|(i, _)| i).collect(); retime_roving(&mut pr.keys, pr.spatial); - st.selected_keys = pasted.iter().filter_map(|&i| pr.keys.get(i)).map(|k| KeyRef { layer: lid, prop: uid, time: k.time }).collect(); - Ok(json!(st.selected_keys.len())) + selected.extend(pasted.iter().filter_map(|&i| pr.keys.get(i)).map(|k| KeyRef { layer: lid, prop: uid, time: k.time })); + let count = selected.len().saturating_sub(retained); + st.selected_keys = selected; + Ok(json!(count)) }) } @@ -297,6 +314,9 @@ fn set_key(s: &mut Session, p: &Value) -> Result { let base = KeyTimeBase::from_params(p, "keys.set")?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; let layer = comp.layer(lid).ok_or(EngineError::NoComp)?; + if layer.switches.locked { + return Err(bad("keys.set", "the layer is locked")); + } let t = key_time(layer, comp.frame_rate, p, "time", base, "keys.set", false)?.ok_or_else(|| bad("keys.set", "missing `time` (layer time by default)"))?; let new_t = key_time(layer, comp.frame_rate, p, "newTime", base, "keys.set", true)?; let value = p.get("value").cloned(); @@ -350,6 +370,9 @@ fn set_ease(s: &mut Session, p: &Value) -> Result { let influence = f_p(p, "influence").map(|v| (v / 100.0).clamp(0.001, 1.0)); s.edit("Keyframe Velocity", merge_p(p), |proj, _| { let l = super::layer_mut(proj, cid, lid)?; + if l.switches.locked { + return Err(bad("keys.setEase", "the layer is locked")); + } let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.setEase", "no property"))?; let i = nearest_key(&pr.keys, t).ok_or_else(|| bad("keys.setEase", "no keyframes"))?; let spatial = pr.spatial; @@ -496,7 +519,13 @@ fn time_reverse(s: &mut Session, _: &Value) -> Result { } let mut new_sel = vec![]; for ((lid, uid), times) in groups { - let Some(pr) = comp.layer_mut(lid).and_then(|l| l.props.find_mut(uid)) else { continue }; + let Some(layer) = comp.layer_mut(lid) else { continue }; + // A locked layer's keys stay as they are, and stay selected. + if layer.switches.locked { + new_sel.extend(times.iter().map(|time| KeyRef { layer: lid, prop: uid, time: *time })); + continue; + } + let Some(pr) = layer.props.find_mut(uid) else { continue }; let (mut picked, rest): (Vec, Vec) = pr.keys.drain(..).partition(|k| times.contains(&k.time)); effectcraft_keyframe::time_reverse(&mut picked); new_sel.extend(picked.iter().map(|k| KeyRef { layer: lid, prop: uid, time: k.time })); diff --git a/crates/engine/src/cmd/anim_tools.rs b/crates/engine/src/cmd/anim_tools.rs index 06e0d341b..33ee39ae2 100644 --- a/crates/engine/src/cmd/anim_tools.rs +++ b/crates/engine/src/cmd/anim_tools.rs @@ -213,7 +213,8 @@ fn sample_curve( fn grouped_selection(s: &Session) -> std::collections::BTreeMap<(LayerId, Uid), Vec> { let mut g: std::collections::BTreeMap<(LayerId, Uid), Vec> = Default::default(); - for k in &s.state.selected_keys { + let Some(comp) = s.active_comp() else { return g }; + for k in super::prop::editable_keys(comp, &s.state.selected_keys) { g.entry((k.layer, k.prop)).or_default().push(k.time); } for v in g.values_mut() { @@ -273,7 +274,7 @@ fn run_on_selection( s.edit(label, None, |proj, st| { let comp = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; let fr = comp.frame_rate; - let mut sel = vec![]; + let mut sel = super::anim::locked_selection(comp, &st.selected_keys); for ((lid, uid), times) in groups { let Some(l) = comp.layer_mut(lid) else { continue }; let lc = l.clone(); diff --git a/crates/engine/src/cmd/animation.rs b/crates/engine/src/cmd/animation.rs index 58eaa060d..4808d7538 100644 --- a/crates/engine/src/cmd/animation.rs +++ b/crates/engine/src/cmd/animation.rs @@ -271,7 +271,8 @@ fn add_keyframe(s: &mut Session, _: &Value) -> Result { /// Selected keys grouped by (layer, property). fn keys_by_prop(s: &Session) -> BTreeMap<(LayerId, Uid), Vec> { let mut m: BTreeMap<(LayerId, Uid), Vec> = BTreeMap::new(); - for k in &s.state.selected_keys { + let Some(comp) = s.active_comp() else { return m }; + for k in super::prop::editable_keys(comp, &s.state.selected_keys) { m.entry((k.layer, k.prop)).or_default().push(k.time); } m diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index dd1685722..b33f4cc5c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -6,7 +6,7 @@ use effectcraft_project::{Comp, Expression, ItemId, ItemKind, Layer, LayerId, La use effectcraft_time::Tick; use serde_json::{Value, json}; -use super::{CommandSpec, has_comp, has_layers, layers_p, match_path_of, selected_leaf_props, str_p}; +use super::{CommandSpec, has_comp, has_layers, layers_or, layers_p, match_path_of, selected_leaf_props, str_p}; use crate::{EngineError, KeyRef, LinkClip, Result, Session, cmd}; fn can_undo(s: &Session) -> std::result::Result<(), String> { @@ -104,9 +104,16 @@ pub(crate) fn reid(layer: &mut Layer, next: &mut u64) { *next += 1; } +/// `edit.duplicate`: copies the targets in place. Without `layers` the targets are, in order: +/// during a text edit the edited layer (the edit is committed first, as `text.endEdit` does), +/// else the selected effects, shape items or text animators, else the selected layers. Layer +/// copies are selected and their ids returned; the other paths return their delegated result. fn duplicate(s: &mut Session, p: &Value) -> Result { - // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). - if p.get("layers").is_none() { + let editing = s.state.text_edit.is_some(); + // Effects selected → duplicate them (Edit ▸ Duplicate in Effect Controls). An open text edit + // comes first, as it does for Clear: opening the editor keeps the property selection, but + // while typing, Duplicate means the edited layer, not an effect or item selected before. + if p.get("layers").is_none() && !editing { let fx = super::effect::selected_effects(s); if !fx.is_empty() { let mut out = vec![]; @@ -126,7 +133,17 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { return super::prop_groups::duplicate_contents(s, &items); } } - let (cid, ids) = layers_p(s, p)?; + // A deselected layer never keeps an open text edit (#2226): the copies take the selection, so + // commit an open edit first, exactly as `text.endEdit` and Deselect All do — one call on the + // command path so every route (menu, shortcut, palette, MCP, script) ends the edit and leaves + // the same final state. Unnamed, the target is the edited layer even when an engine route + // selected another one meanwhile. + let edited = s.state.text_edit.as_ref().map(|e| e.layer); + super::text_edit::end_edit(s, &Value::Null)?; + let (cid, ids) = match edited { + Some(layer) => layers_or(s, p, &[layer])?, + None => layers_p(s, p)?, + }; let new = s.edit("Duplicate", None, |proj, st| { let mut next = proj.next_id; let comp = proj.comp_mut(cid).ok_or(crate::EngineError::NoComp)?; @@ -143,14 +160,16 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { } relink_copies(comp, &map); proj.next_id = next; - st.selected_layers = created.clone(); + // The property, keyframe and mask-vertex selection the edit kept belongs to the originals; + // left in place, the next Duplicate or Clear would act on it, not on the copies. So during + // an edit the copies take the selection the way a click on them would. + if editing { + super::layer::select_only(st, created.clone()); + } else { + st.selected_layers = created.clone(); + } Ok(created) })?; - // Duplicating selects the copies, so an open text edit on a now-deselected layer ends - // (#2226): typing must not keep editing an unselected layer. - if s.state.text_edit.as_ref().is_some_and(|e| !s.state.selected_layers.contains(&e.layer)) { - super::text_edit::end_edit(s, &Value::Null)?; - } Ok(json!(new.iter().map(|l| l.0).collect::>())) } @@ -968,7 +987,15 @@ pub fn specs() -> Vec { paste_reversed ), cmd!("edit.clear", "Clear", ["Edit"], Some("Delete"), "{layers?}", layers_or_keys, delete), - cmd!("edit.duplicate", "Duplicate", ["Edit"], Some("Cmd+D"), "{layers?} (effects or shape items when they are selected)", has_layers, duplicate), + cmd!( + "edit.duplicate", + "Duplicate", + ["Edit"], + Some("Cmd+D"), + "{layers?} (without layers: during a text edit the edited layer, else the selected effects, shape items or text animators, else the selected layers)", + layers_or_text_edit, + duplicate + ), cmd!("edit.splitLayer", "Split Layer", ["Edit"], Some("Cmd+Shift+D"), "{layers?}", has_layers, split), cmd!("edit.liftWorkArea", "Lift Work Area", ["Edit"], None, "{layers?}", has_comp, lift), cmd!("edit.extractWorkArea", "Extract Work Area", ["Edit"], None, "{layers?}", has_comp, extract), @@ -1038,6 +1065,11 @@ fn layers_or_keys(s: &Session) -> std::result::Result<(), String> { if !s.state.selected_keys.is_empty() || !s.state.selected_vertices.is_empty() { super::has_comp(s) } else { has_layers(s) } } +/// Duplicate's unnamed target: the edited layer during a text edit, else the selection. +fn layers_or_text_edit(s: &Session) -> std::result::Result<(), String> { + if s.state.text_edit.is_some() { super::has_comp(s) } else { has_layers(s) } +} + fn always_ok(_: &Session) -> std::result::Result<(), String> { Ok(()) } diff --git a/crates/engine/src/cmd/key_transform.rs b/crates/engine/src/cmd/key_transform.rs index b78439266..397db4de5 100644 --- a/crates/engine/src/cmd/key_transform.rs +++ b/crates/engine/src/cmd/key_transform.rs @@ -72,6 +72,10 @@ fn transform(s: &mut Session, p: &Value) -> Result { let mut new_sel = vec![]; for ((lid, uid), times) in groups { let Some(layer) = comp.layer_mut(lid) else { continue }; + if layer.switches.locked { + new_sel.extend(times.into_iter().map(|time| KeyRef { layer: lid, prop: uid, time })); + continue; + } let (start, stretch) = (layer.start_time, layer.stretch); let to_comp = |lt: Tick| if (stretch - 100.0).abs() < 1e-9 { start + lt } else { start + Tick((lt.0 as f64 * stretch / 100.0) as i64) }; let to_layer = |ct: Tick| { @@ -122,6 +126,9 @@ fn spatial_tangents(s: &mut Session, p: &Value) -> Result { let brk = b_p(p, "break").unwrap_or(false); s.edit("Edit Spatial Tangents", merge_p(p), |proj, _| { let l = super::layer_mut(proj, cid, lid)?; + if l.switches.locked { + return Err(bad("keys.setSpatialTangents", "the layer is locked")); + } let pr = l.props.find_mut(uid).ok_or_else(|| bad("keys.setSpatialTangents", "no property"))?; if !pr.spatial { return Err(bad("keys.setSpatialTangents", "not a spatial property")); diff --git a/crates/engine/src/cmd/keys_more.rs b/crates/engine/src/cmd/keys_more.rs index c7785d08c..4edef885a 100644 --- a/crates/engine/src/cmd/keys_more.rs +++ b/crates/engine/src/cmd/keys_more.rs @@ -37,6 +37,8 @@ pub(crate) fn label_keys(s: &mut Session, p: &Value) -> Result { return Err(bad("keys.label", "select keyframes first")); } let cid = comp_id(s, p)?; + let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let keys = super::prop::editable_keys(comp, &keys); let n = s.edit("Keyframe Label", None, |proj, _| { let mut n = 0; for k in &keys { diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index eb9641547..6b6a0ecd9 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -28,6 +28,7 @@ pub(crate) use create::{color_p, index_p, insert_layer, insert_layer_at, place, pub(crate) use parent::parent_fixes; use parent::set_parent; use replace::{has_replace_source, replace_source}; +pub(crate) use select::select_only; use select::{select, select_next, select_prev, select_step}; use switches::{blend, rename, set_comment, set_switch, track_matte}; use timing::{arrange, move_frames, precompose, slip, timing}; diff --git a/crates/engine/src/cmd/layer/select.rs b/crates/engine/src/cmd/layer/select.rs index 0edf7a72c..efe038a0a 100644 --- a/crates/engine/src/cmd/layer/select.rs +++ b/crates/engine/src/cmd/layer/select.rs @@ -25,15 +25,21 @@ pub(super) fn select(s: &mut Session, p: &Value) -> Result { } } } else { - s.state.selected_layers = ids; - s.state.selected_props.clear(); - s.state.selected_keys.clear(); - let keep = s.state.selected_layers.clone(); - s.state.selected_vertices.retain(|v| keep.contains(&v.layer)); + select_only(&mut s.state, ids); } Ok(json!(s.state.selected_layers.iter().map(|l| l.0).collect::>())) } +/// Make `ids` the whole layer selection, as a plain click on them does: no property or keyframe +/// stays selected, and mask vertices stay selected only on layers that are still selected. +pub(crate) fn select_only(st: &mut crate::EditorState, ids: Vec) { + st.selected_layers = ids; + st.selected_props.clear(); + st.selected_keys.clear(); + let keep = st.selected_layers.clone(); + st.selected_vertices.retain(|v| keep.contains(&v.layer)); +} + pub(super) fn select_step(s: &mut Session, p: &Value, dir: i64) -> Result { let comp = s.active_comp().ok_or(EngineError::NoComp)?; if comp.layers.is_empty() { diff --git a/crates/engine/src/cmd/mask_interp.rs b/crates/engine/src/cmd/mask_interp.rs index e2b53f8a4..ed7ed5190 100644 --- a/crates/engine/src/cmd/mask_interp.rs +++ b/crates/engine/src/cmd/mask_interp.rs @@ -438,20 +438,22 @@ fn interpolate(s: &mut Session, p: &Value) -> Result { // The layer: given, else the layer of selected Mask Path keys, else the selection. let cid = super::comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; + let selected = super::prop::editable_keys(comp, &s.state.selected_keys); let lid = match p.get("layer") { Some(v) => super::resolve_layer(comp, v).ok_or_else(|| bad(CMD, format!("no layer {v}")))?, - None => match s.state.selected_keys.iter().find(|k| comp.layer(k.layer).is_some_and(|l| l.masks().is_some_and(|m| m.find(k.prop).is_some()))) { + None => match selected.iter().find(|k| comp.layer(k.layer).is_some_and(|l| l.masks().is_some_and(|m| m.find(k.prop).is_some()))) { Some(k) => k.layer, None => layer_p(s, p, CMD)?.1, }, }; let layer = comp.layer(lid).ok_or(EngineError::NoComp)?; + if !super::prop::key_layer_editable(comp, lid) { + return Err(bad(CMD, "the layer is locked")); + } let masks = layer.masks().ok_or_else(|| bad(CMD, "the layer has no masks"))?; let uid = match p.get("mask") { Some(_) => mask_uid(s, layer, p), - None => s - .state - .selected_keys + None => selected .iter() .filter(|k| k.layer == lid) .find_map(|k| masks.groups().find(|g| g.get("path").is_some_and(|pr| pr.uid == k.prop)).map(|g| g.uid)) @@ -466,7 +468,7 @@ fn interpolate(s: &mut Session, p: &Value) -> Result { let ts: Vec = a.iter().filter_map(Value::as_f64).map(|t| layer.layer_time(comp.frame_rate.snap_nearest(Tick::from_seconds_f64(t)))).collect(); ts.into_iter().filter_map(|t| path_prop.keys.iter().find(|k| (k.time.0 - t.0).abs() <= comp.frame_duration().0 / 2).map(|k| k.time)).collect() } - None => s.state.selected_keys.iter().filter(|k| k.layer == lid && k.prop == path_prop.uid).map(|k| k.time).collect(), + None => selected.iter().filter(|k| k.layer == lid && k.prop == path_prop.uid).map(|k| k.time).collect(), }; times.sort(); times.dedup(); diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs index ff4d443ea..2929d3347 100644 --- a/crates/engine/src/cmd/params.rs +++ b/crates/engine/src/cmd/params.rs @@ -148,6 +148,11 @@ pub(crate) fn layer_p(s: &Session, p: &Value, cmd: &str) -> Result<(ItemId, Laye /// `layers` param (array) or the selection. pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> { + layers_or(s, p, &s.state.selected_layers) +} + +/// `layers` param (array), else `layer`, else those of `fallback` still in the comp. +pub(crate) fn layers_or(s: &Session, p: &Value, fallback: &[LayerId]) -> Result<(ItemId, Vec)> { let cid = comp_id(s, p)?; let comp = s.project.comp(cid).ok_or(EngineError::NoComp)?; // Explicit references that match no layer are an error (agents otherwise get a silent no-op). @@ -159,7 +164,7 @@ pub(crate) fn layers_p(s: &Session, p: &Value) -> Result<(ItemId, Vec)> if let Some(v) = p.get("layer").filter(|v| !v.is_null()) { return Ok((cid, vec![resolve(v)?])); } - Ok((cid, s.state.selected_layers.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) + Ok((cid, fallback.iter().copied().filter(|l| comp.layer(*l).is_some()).collect())) } /// Whether a layer can be picked by the user (Select All, Ctrl+Up/Down, a Timeline click): not diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 2f3345942..fb4b525a2 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -930,6 +930,15 @@ fn select_keys(s: &mut Session, p: &Value) -> Result { Ok(out) } +/// References that can be edited, including explicit key lists supplied by commands. +pub(crate) fn editable_keys(comp: &effectcraft_project::Comp, selected: &[KeyRef]) -> Vec { + selected.iter().filter(|key| key_layer_editable(comp, key.layer)).copied().collect() +} + +pub(crate) fn key_layer_editable(comp: &effectcraft_project::Comp, layer: LayerId) -> bool { + comp.layer(layer).is_some_and(|layer| !layer.switches.locked) +} + /// Apply `f` to every selected key; keeps the selection pointing at moved keys. pub(crate) fn edit_keys(s: &mut Session, label: &str, merge: Option<&str>, f: impl Fn(&mut Vec, usize, &mut Tick) -> bool) -> Result { let cid = s.active_comp_id().ok_or(EngineError::NoComp)?; @@ -942,7 +951,16 @@ pub(crate) fn edit_keys(s: &mut Session, label: &str, merge: Option<&str>, f: im groups.entry((k.layer, k.prop)).or_default().push(k.time); } for ((lid, uid), times) in groups { - let Some(pr) = comp.layer_mut(lid).and_then(|l| l.props.find_mut(uid)) else { continue }; + let Some(layer) = comp.layer_mut(lid) else { continue }; + // A locked layer's keys stay as they are, and stay selected. + if layer.switches.locked { + for time in times { + let r = KeyRef { layer: lid, prop: uid, time }; + remapped.insert(r, r); + } + continue; + } + let Some(pr) = layer.props.find_mut(uid) else { continue }; // Process each stored key once, in an order that avoids collisions when moving. let mut idx: Vec<(usize, Tick)> = times.iter().filter_map(|t| key_at(&pr.keys, *t).map(|i| (i, *t))).collect(); idx.sort_by_key(|x| x.0); @@ -1095,6 +1113,10 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { let mut removed = 0usize; let mut changed = std::collections::BTreeSet::new(); for k in &sel { + // A layer locked after its keys were selected keeps them (the selection stays too). + if comp.layer(k.layer).is_some_and(|l| l.switches.locked) { + continue; + } if let Some(pr) = comp.layer_mut(k.layer).and_then(|l| l.props.find_mut(k.prop)) && let Some(i) = key_at(&pr.keys, k.time) { @@ -1108,7 +1130,7 @@ fn delete_keys(s: &mut Session, _: &Value) -> Result { } } } - st.selected_keys.clear(); + st.selected_keys.retain(|k| comp.layer(k.layer).is_some_and(|l| l.switches.locked)); for (layer, prop) in changed { sync_polystar_property(proj, st, cid, layer, prop, time)?; } diff --git a/crates/engine/src/tests/anim_tools.rs b/crates/engine/src/tests/anim_tools.rs index 06d194001..96b861972 100644 --- a/crates/engine/src/tests/anim_tools.rs +++ b/crates/engine/src/tests/anim_tools.rs @@ -9,6 +9,80 @@ fn select(s: &mut crate::Session, layer: u64, path: &str) { s.execute("prop.select", json!({"layer": layer, "path": path})).unwrap(); } +fn selection_command_skips_locked_keys(command: &str, path: &str, values: [serde_json::Value; 3], mut params: serde_json::Value) { + let (mut s, locked) = setup(); + let unlocked = s.execute("layer.newSolid", json!({"name": "Editable", "width": 100, "height": 100})).unwrap()["layer"].as_u64().unwrap(); + for layer in [locked, unlocked] { + animate(&mut s, layer, path, &[(0.0, values[0].clone()), (0.5, values[1].clone()), (1.0, values[2].clone())]); + } + let before = prop(&s, locked, path).keys; + let selected: Vec<_> = [locked, unlocked] + .into_iter() + .flat_map(|layer| { + let pr = prop(&s, layer, path); + pr.keys.into_iter().map(move |key| crate::KeyRef { layer: effectcraft_project::LayerId(layer), prop: pr.uid, time: key.time }) + }) + .collect(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = selected.clone(); + if params.get("keys").is_some() { + params["keys"] = serde_json::to_value(&selected).unwrap(); + } + s.execute_checked(command, params.clone()).unwrap(); + assert_eq!(prop(&s, locked, path).keys, before, "{command} changed a locked layer"); + let after = prop(&s, unlocked, path).keys; + assert_ne!(after, before, "{command} must still edit unlocked keys"); + let after_selection = s.state.selected_keys.clone(); + assert!(selected.iter().filter(|k| k.layer.0 == locked).all(|key| after_selection.contains(key))); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, unlocked, path).keys, before); + assert!(selected.iter().filter(|key| key.layer.0 == locked).all(|key| s.state.selected_keys.contains(key))); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(prop(&s, unlocked, path).keys, after); + assert_eq!(s.state.selected_keys, after_selection); + // All-locked input must preserve the selection and document as well. + s.state.selected_keys.retain(|k| k.layer.0 == locked); + if params.get("keys").is_some() { + params["keys"] = serde_json::to_value(&s.state.selected_keys).unwrap(); + } + let locked_selection = s.state.selected_keys.clone(); + let project = serde_json::to_value(&*s.project).unwrap(); + let _ = s.execute_checked(command, params); + assert_eq!(serde_json::to_value(&*s.project).unwrap(), project); + assert_eq!(s.state.selected_keys, locked_selection); +} + +#[test] +fn wiggler_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys( + "keys.wiggle", + "transform/opacity", + [json!(10), json!(90), json!(20)], + json!({"apply": "temporal", "frequency": 8, "magnitude": 10}), + ); +} + +#[test] +fn smoother_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys("keys.smooth", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"tolerance": 1})); +} + +#[test] +fn exponential_scale_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys( + "keys.exponentialScale", + "transform/scale", + [json!([100, 100, 100]), json!([150, 150, 150]), json!([400, 400, 400])], + json!({}), + ); +} + +#[test] +fn edit_label_preserves_locked_keys_in_mixed_selection() { + selection_command_skips_locked_keys("edit.label", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"label": "Red"})); + selection_command_skips_locked_keys("edit.label", "transform/opacity", [json!(10), json!(50), json!(90)], json!({"label": "Red", "keys": true})); +} + #[test] fn wiggle_adds_keys_at_frequency_within_magnitude() { let (mut s, l) = setup(); diff --git a/crates/engine/src/tests/delete_keys.rs b/crates/engine/src/tests/delete_keys.rs index b1b9af316..ec6ec888e 100644 --- a/crates/engine/src/tests/delete_keys.rs +++ b/crates/engine/src/tests/delete_keys.rs @@ -43,3 +43,207 @@ fn delete_keys_reports_zero_when_no_selected_key_exists() { assert_eq!(removed, json!(0)); assert_eq!(keys_of(&s, layer, prop), keys); } + +/// A layer locked after its keys were selected keeps them: Delete and Edit Value leave them alone. +#[test] +fn a_locked_layers_keys_cannot_be_deleted_or_edited() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + s.state.selected_keys = keys.clone(); + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let removed = s.execute_checked("keys.delete", json!({})).unwrap(); + assert_eq!(removed, json!(0)); + assert_eq!(keys_of(&s, layer, prop), keys); + assert_eq!(s.state.selected_keys, keys, "the selection stays"); + let e = s.execute("keys.set", json!({"layer": layer, "prop": prop, "time": 0, "value": 90})).unwrap_err().to_string(); + assert!(e.contains("locked"), "{e}"); + let value = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys[0].value.as_f64(); + assert_eq!(value, 0.0); + // Unlocked again, both work. + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + s.execute_checked("keys.set", json!({"layer": layer, "prop": prop, "time": 0, "value": 90})).unwrap(); + assert_eq!(s.execute_checked("keys.delete", json!({})).unwrap(), json!(3)); +} + +/// Easy Ease, Toggle Hold and Time-Reverse leave a locked layer's keys as they are and keep them +/// selected; unlocked, they change them. +#[test] +fn selection_based_key_edits_skip_a_locked_layer() { + let (mut s, layer, prop) = setup(); + let keys = keys_of(&s, layer, prop); + let stored = |s: &Session| s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys.clone(); + let before = stored(&s); + s.state.selected_keys = keys.clone(); + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + for command in ["keys.easyEase", "keys.toggleHold", "keys.timeReverse"] { + s.execute_checked(command, json!({})).unwrap(); + assert_eq!(stored(&s), before, "{command} changed a locked layer's keys"); + assert_eq!(s.state.selected_keys, keys, "{command} dropped the selection of a locked layer's keys"); + } + s.execute_checked("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + s.execute_checked("keys.easyEase", json!({})).unwrap(); + assert_ne!(stored(&s), before, "unlocked, Easy Ease changes the keys"); +} + +#[test] +fn spatial_handles_cannot_edit_a_locked_layer() { + let (mut s, layer, _) = setup(); + for (time, value) in [(0, [10, 20, 0]), (1, [80, 60, 0])] { + s.execute("prop.addKey", json!({"layer": layer, "path": "transform/position", "time": time, "value": value})).unwrap(); + } + let prop = s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("transform/position").unwrap().uid; + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": prop, "time": 0}]})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let before = s.project.to_json(); + let selected = s.state.selected_keys.clone(); + let undo_len = s.history.undo.len(); + let params = json!({"layer": layer, "prop": prop, "time": 0, "out": [15, 25, 0]}); + let error = s.execute("keys.setSpatialTangents", params.clone()).unwrap_err().to_string(); + assert!(error.contains("locked"), "{error}"); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo_len); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + let unlocked = s.project.to_json(); + s.execute("keys.setSpatialTangents", params).unwrap(); + let edited = s.project.to_json(); + assert_ne!(edited, unlocked); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), unlocked); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), edited); +} + +#[test] +fn pasting_keys_or_motion_paths_cannot_edit_locked_targets() { + for path_clip in [false, true] { + for explicit in [false, true] { + let (mut s, layer, opacity) = setup(); + let target_path = if path_clip { "transform/position" } else { "transform/opacity" }; + let source = if path_clip { + s.execute("mask.new", json!({"layer": layer, "vertices": [[0, 0], [20, 10], [40, 0]], "closed": false})).unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": layer, "path": "masks/#1/path"})).unwrap(); + s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.prop("masks/#1/path").unwrap().uid + } else { + opacity + }; + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": source, "time": 0}]})).unwrap(); + s.execute("keys.copy", json!({})).unwrap(); + s.execute("prop.select", json!({"layer": layer, "path": target_path})).unwrap(); + s.execute("time.set", json!({"time": 3})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": true})).unwrap(); + let params = if explicit { json!({"layer": layer, "path": target_path}) } else { json!({}) }; + let before = s.project.to_json(); + let selected = s.state.selected_keys.clone(); + let undo_len = s.history.undo.len(); + let result = s.execute("keys.paste", params.clone()); + assert!(result.is_err(), "path={path_clip}, explicit={explicit}: {result:?}"); + assert_eq!(s.project.to_json(), before); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo_len); + s.execute("layer.setSwitch", json!({"layers": [layer], "switch": "locked", "value": false})).unwrap(); + let unlocked = s.project.to_json(); + s.execute("keys.paste", params).unwrap(); + let pasted = s.project.to_json(); + assert_ne!(pasted, unlocked); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), unlocked); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); + } + } +} + +#[test] +fn key_paste_skips_locked_targets_without_changing_layer_pairing() { + let (mut s, locked, locked_prop) = setup(); + let other = s.execute("layer.newSolid", json!({"name": "Other", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(0, 100), (1, 80), (2, 60)] { + s.execute("prop.addKey", json!({"layer": other, "path": "transform/opacity", "time": time, "value": value})).unwrap(); + } + let other_prop = s.active_comp().unwrap().layer(LayerId(other)).unwrap().props.prop("transform/opacity").unwrap().uid; + let locked_keys = keys_of(&s, locked, locked_prop); + s.state.selected_keys = locked_keys.iter().chain(keys_of(&s, other, other_prop).iter()).copied().collect(); + s.execute("keys.copy", json!({})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.execute("time.set", json!({"time": 3})).unwrap(); + let before = s.project.to_json(); + assert_eq!(s.execute("keys.paste", json!({"layers": [locked, other]})).unwrap(), json!(3), "only newly pasted keys are counted"); + assert_eq!(keys_of(&s, locked, locked_prop), locked_keys); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k))); + assert_eq!(s.state.selected_keys.len(), 6); + let keys = &s.active_comp().unwrap().layer(LayerId(other)).unwrap().props.find(other_prop).unwrap().keys; + assert_eq!(keys.iter().filter(|k| k.time.seconds() >= 3.0).map(|k| k.value.as_f64()).collect::>(), [100.0, 80.0, 60.0]); + let pasted = s.project.to_json(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); +} + +#[test] +fn motion_path_paste_preserves_locked_source_selection() { + for explicit in [false, true] { + let (mut s, source, _) = setup(); + s.execute("mask.new", json!({"layer": source, "vertices": [[0, 0], [20, 10], [40, 0]], "closed": false})).unwrap(); + s.execute("prop.toggleAnimation", json!({"layer": source, "path": "masks/#1/path"})).unwrap(); + let uid = s.active_comp().unwrap().layer(LayerId(source)).unwrap().props.prop("masks/#1/path").unwrap().uid; + s.execute("keys.select", json!({"keys": [{"layer": source, "prop": uid, "time": 0}]})).unwrap(); + s.execute("keys.copy", json!({})).unwrap(); + let locked_keys = s.state.selected_keys.clone(); + let target = s.execute("layer.newSolid", json!({"name": "Target", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.select", json!({"layer": target, "path": "transform/position"})).unwrap(); + s.execute("layer.setSwitch", json!({"layers": [source], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = locked_keys.clone(); + let params = if explicit { json!({"layer": target, "path": "transform/position"}) } else { json!({}) }; + let before = s.project.to_json(); + assert_eq!(s.execute("keys.paste", params).unwrap(), json!(3)); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k))); + assert_eq!(s.state.selected_keys.len(), 4); + assert_eq!(s.state.selected_keys.iter().filter(|k| k.layer == LayerId(target)).count(), 3); + let pasted = s.project.to_json(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), pasted); + } +} + +#[test] +fn key_transform_preserves_locked_keys_in_a_mixed_selection() { + let (mut s, locked, locked_prop) = setup(); + let unlocked = s.execute("layer.newSolid", json!({"name": "Unlocked", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for t in 0..3 { + s.execute("prop.addKey", json!({"layer": unlocked, "path": "transform/opacity", "time": t, "value": t * 25})).unwrap(); + } + let unlocked_prop = s.active_comp().unwrap().layer(LayerId(unlocked)).unwrap().props.prop("transform/opacity").unwrap().uid; + let locked_keys = keys_of(&s, locked, locked_prop); + s.state.selected_keys = locked_keys.iter().chain(keys_of(&s, unlocked, unlocked_prop).iter()).copied().collect(); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + let stored = |s: &Session, layer, prop| s.active_comp().unwrap().layer(LayerId(layer)).unwrap().props.find(prop).unwrap().keys.clone(); + let before = stored(&s, locked, locked_prop); + let undo = s.history.undo.len(); + for offset in [0.5, 1.0] { + s.execute("keys.transform", json!({"timeOffset": offset, "valueOffset": offset * 10.0, "merge": "mixed-transform", "fromStart": true})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(s.state.selected_keys.len(), 6); + assert!(locked_keys.iter().all(|k| s.state.selected_keys.contains(k)), "locked selections stay at their original times"); + assert!(keys_of(&s, unlocked, unlocked_prop).iter().all(|k| s.state.selected_keys.contains(k)), "unlocked selections follow the keys"); + let moved = stored(&s, unlocked, unlocked_prop); + for (i, key) in moved.iter().enumerate() { + assert_eq!(key.time.seconds(), i as f64 + offset); + assert_eq!(key.value.as_f64(), i as f64 * 25.0 + offset * 10.0); + } + } + assert_eq!(s.history.undo.len(), undo + 1, "one merged drag"); + let after = stored(&s, unlocked, unlocked_prop); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(stored(&s, unlocked, unlocked_prop), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(stored(&s, locked, locked_prop), before); + assert_eq!(stored(&s, unlocked, unlocked_prop), after); + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": false})).unwrap(); + s.execute("keys.transform", json!({"timeOffset": 0.5, "valueOffset": 10})).unwrap(); + assert_ne!(stored(&s, locked, locked_prop), before, "unlocking makes the keys editable again"); +} diff --git a/crates/engine/src/tests/mask_warp.rs b/crates/engine/src/tests/mask_warp.rs index adcea44be..5387fe562 100644 --- a/crates/engine/src/tests/mask_warp.rs +++ b/crates/engine/src/tests/mask_warp.rs @@ -184,6 +184,51 @@ fn mask_interpolation_creates_matching_keys() { assert!(s.execute_checked("mask.interpolate", json!({"matchingMethod": "magic", "times": [0, 0.8], "layer": clip.0})).is_err()); } +#[test] +fn mask_interpolation_skips_locked_selection_and_rejects_explicit_locked_targets() { + let (mut s, locked, _) = setup(patch_frames(rotating_pose)); + let unlocked = effectcraft_project::LayerId(s.execute("layer.newSolid", json!({"width": 100, "height": 100})).unwrap()["layer"].as_u64().unwrap()); + let a = ShapePath::rect([20.0, 20.0], 10.0, 10.0); + let b = ShapePath::rect([40.0, 30.0], 20.0, 20.0); + let cid = s.active_comp_id().unwrap(); + let mut selected = vec![]; + let mut masks = vec![]; + for layer in [locked, unlocked] { + let uid = new_mask(&mut s, layer, &a); + masks.push(uid); + s.edit("keys", None, |project, _| { + let pr = project.comp_mut(cid).unwrap().layer_mut(layer).unwrap().props.find_group_mut(uid).unwrap().get_mut("path").unwrap(); + pr.keys = vec![Keyframe::new(frame_time(0), KV::Path(a.clone())), Keyframe::new(frame_time(20), KV::Path(b.clone()))]; + selected.extend(pr.keys.iter().map(|key| crate::KeyRef { layer, prop: pr.uid, time: key.time })); + Ok(()) + }) + .unwrap(); + } + s.execute("layer.setSwitch", json!({"layers": [locked.0], "switch": "locked", "value": true})).unwrap(); + s.state.selected_keys = selected.clone(); + let before = mask_path(&s, locked, masks[0]).keys; + let options = serde_json::to_value(&s.state.mask_interp).unwrap(); + let project = serde_json::to_value(&*s.project).unwrap(); + let error = s.execute_checked("mask.interpolate", json!({"layer": locked.0, "mask": masks[0], "times": [0.0, 0.8], "quality": 75})).unwrap_err(); + assert!(error.to_string().contains("locked"), "{error}"); + assert_eq!(serde_json::to_value(&*s.project).unwrap(), project); + assert_eq!(serde_json::to_value(&s.state.mask_interp).unwrap(), options); + assert_eq!(s.state.selected_keys, selected); + s.execute_checked("mask.interpolate", json!({})).unwrap(); + assert_eq!(mask_path(&s, locked, masks[0]).keys, before); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 21); + assert_eq!(s.state.selected_keys, selected); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 2); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(mask_path(&s, unlocked, masks[1]).keys.len(), 21); + s.state.selected_keys.retain(|key| key.layer == locked); + let selection = s.state.selected_keys.clone(); + assert!(s.execute_checked("mask.interpolate", json!({})).is_err()); + assert_eq!(mask_path(&s, locked, masks[0]).keys, before); + assert_eq!(s.state.selected_keys, selection); +} + // ---------------------------------------------------------------- Warp Stabilizer /// Camera offset on frame `f`: a slow pan plus jitter. diff --git a/crates/engine/src/tests/text_edit.rs b/crates/engine/src/tests/text_edit.rs index 7a04ea155..eb1de6f89 100644 --- a/crates/engine/src/tests/text_edit.rs +++ b/crates/engine/src/tests/text_edit.rs @@ -7,6 +7,7 @@ use serde_json::json; use crate::Session; use crate::commands::text_edit::{MAX_IME_TEXT_BYTES, ime_stamp_revision, ime_surrounding, layer_doc}; +use crate::tests::three_d::layer_count; fn session(text: &str) -> (Session, u64) { let mut s = Session::default(); @@ -1231,10 +1232,188 @@ fn deselect_all_keeps_an_empty_type_tool_layer() { s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == effectcraft_project::LayerId(new)), "the Type tool opens the edit on its new, empty layer" ); - let count = |s: &Session| s.active_comp().map_or(0, |c| c.layers.len()); - let before = count(&s); + let before = layer_count(&s); s.execute("edit.deselectAll", json!({})).unwrap(); assert!(s.state.text_edit.is_none()); assert!(s.state.selected_layers.is_empty()); - assert_eq!(count(&s), before, "an empty Type-tool layer is kept, as in After Effects"); + assert_eq!(layer_count(&s), before, "an empty Type-tool layer is kept, as in After Effects"); +} + +#[test] +fn duplicate_ends_an_edit_on_already_committed_text() { + // The engine route (MCP, control channel, script) with text that was committed before the + // edit opened and nothing typed inside it: the edit still ends and the copy is selected. + let (mut s, t) = editing("Hello", 5); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "the open edit ends on a direct edit.duplicate"); + assert_eq!(s.state.selected_layers, vec![effectcraft_project::LayerId(copy)], "and the copy is selected"); + assert_eq!(doc(&s, t).text, "Hello", "the committed text is kept on the original"); + assert_eq!(doc(&s, copy).text, "Hello", "and carried by the copy"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(layer_count(&s), 1, "undo removes the copy"); + assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); +} + +#[test] +fn duplicate_during_an_edit_copies_the_layer_not_a_selected_property_group() { + use effectcraft_project::LayerId; + // Opening the editor by command, as a Type-tool click does, keeps the property selection, so + // an effect, shape item or text animator can still be selected while the user types. + for what in ["effect", "shape item", "text animator"] { + let (mut s, t) = session("Hello"); + let (owner, path) = match what { + "effect" => { + s.execute("effect.apply", json!({"layers": [t], "effect": "Gaussian Blur"})).unwrap(); + (t, "effects") + } + "shape item" => (s.execute("layer.newShape", json!({"kind": "rect"})).unwrap()["layer"].as_u64().unwrap(), "contents"), + _ => { + s.execute("layer.addTextAnimator", json!({"layer": t, "properties": ["opacity"]})).unwrap(); + (t, "text/animators") + } + }; + let items = + |s: &Session| s.active_comp().unwrap().layer(LayerId(owner)).unwrap().props.group(path).unwrap().groups().map(|g| g.uid).collect::>(); + let item = items(&s)[0]; + let n = items(&s).len(); + let before = layer_count(&s); + s.execute("prop.select", json!({"layer": owner, "prop": item})).unwrap(); + begin_edit_at(&mut s, t, 5); + let r = s.execute("edit.duplicate", json!({})).unwrap(); + assert!(s.state.text_edit.is_none(), "{what}: Duplicate ends the open edit"); + assert_eq!(items(&s).len(), n, "{what}: the selected {what} is not duplicated"); + assert_eq!(layer_count(&s), before + 1, "{what}: the edited layer is"); + assert_eq!(s.state.selected_layers, vec![LayerId(r[0].as_u64().unwrap())], "{what}: and its copy is selected"); + // The selection the edit passed over goes with it: the next Duplicate copies the selected + // copy rather than reaching back to the original's {what}. + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(items(&s).len(), n, "{what}: a second Duplicate leaves the original's {what} alone"); + assert_eq!(layer_count(&s), before + 2, "{what}: and copies the copy"); + // With no edit open, the selected item keeps its precedence over the layers. + s.execute("prop.select", json!({"layer": owner, "prop": item})).unwrap(); + s.execute("edit.duplicate", json!({})).unwrap(); + assert_eq!(items(&s).len(), n + 1, "{what}: outside an edit Duplicate copies the selected {what}"); + assert_eq!(layer_count(&s), before + 2, "{what}: and no layer"); + } +} + +#[test] +fn duplicate_during_an_edit_leaves_no_key_or_vertex_selected_on_the_original() { + use effectcraft_project::LayerId; + // Opening the editor keeps the keyframe and mask-vertex selection too. Once the copy takes the + // selection, Clear deletes the copy, not a key or a vertex of the original. + for what in ["keyframe", "mask vertex"] { + let (mut s, t) = session("Hello"); + let mask = match what { + "keyframe" => { + for time in [0.0, 1.0] { + s.execute("prop.addKey", json!({"layer": t, "path": "transform/opacity", "time": time, "value": 50})).unwrap(); + } + s.execute("keys.select", json!({"keys": [{"layer": t, "path": "transform/opacity", "time": 0.0}]})).unwrap(); + None + } + // The Pen leaves the new mask's last vertex selected. + _ => Some(s.execute("mask.new", json!({"layer": t, "vertices": [[0, 0], [40, 0], [40, 40]], "closed": true})).unwrap()["mask"].as_u64().unwrap()), + }; + let original = |s: &Session| { + let l = s.active_comp().unwrap().layer(LayerId(t)).unwrap(); + match mask { + None => l.props.prop("transform/opacity").unwrap().keys.len(), + Some(m) => l.props.find_group(m).unwrap().get("path").unwrap().value.as_path().unwrap().vertices.len(), + } + }; + let (n, before) = (original(&s), layer_count(&s)); + begin_edit_at(&mut s, t, 5); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "{what}: the copy is selected"); + s.execute("edit.clear", json!({})).unwrap(); + assert_eq!(layer_count(&s), before, "{what}: Clear deletes the selected copy"); + assert_eq!(original(&s), n, "{what}: and leaves the original's {what} alone"); + } +} + +#[test] +fn duplicate_during_an_edit_copies_the_edited_layer_after_another_is_selected() { + use effectcraft_project::LayerId; + // An engine route (MCP, control channel, script) can select another layer while the edit + // stays open. The implicit Duplicate still means the edited layer, as Clear does; a named + // target keeps its meaning. + let (mut s, t) = session("Hello"); + let other = s.execute("layer.newText", json!({"text": "Other"})).unwrap()["layer"].as_u64().unwrap(); + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + assert!(s.state.text_edit.as_ref().is_some_and(|e| e.layer == LayerId(t)), "selecting another layer leaves the edit open"); + let before = layer_count(&s); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit"); + assert_eq!(layer_count(&s), before + 1, "one layer is copied"); + assert_eq!(doc(&s, copy).text, "Hello", "the edited layer is copied, not the newly selected one"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "and its copy is selected"); + // Null parameters name no layer, as everywhere else layers are read. + for unnamed in [json!({"layers": null}), json!({"layer": null})] { + begin_edit_at(&mut s, t, 5); + s.execute("layer.select", json!({"layer": other})).unwrap(); + let copy = s.execute("edit.duplicate", unnamed.clone()).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, copy).text, "Hello", "{unnamed}: the edited layer is copied"); + } + begin_edit_at(&mut s, t, 5); + let named = s.execute("edit.duplicate", json!({"layer": other})).unwrap()[0].as_u64().unwrap(); + assert_eq!(doc(&s, named).text, "Other", "a named layer is copied as named"); +} + +#[test] +fn duplicate_during_an_edit_with_nothing_selected_copies_the_edited_layer() { + use effectcraft_project::LayerId; + // An engine route can also clear the selection while the edit stays open. Duplicate stays + // available, since its unnamed target is the edited layer, not the selection. + let (mut s, _) = editing("Hello", 5); + s.execute("layer.select", json!({"layers": []})).unwrap(); + assert!(s.state.selected_layers.is_empty() && s.state.text_edit.is_some(), "the edit is open with nothing selected"); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit"); + assert_eq!(doc(&s, copy).text, "Hello", "and copies the edited layer"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "which is selected"); + // With no edit open, nothing selected still means nothing to duplicate. + s.execute("layer.select", json!({"layers": []})).unwrap(); + let before = layer_count(&s); + assert!(s.execute("edit.duplicate", json!({})).is_err(), "outside an edit Duplicate needs a selection"); + assert_eq!(layer_count(&s), before, "and copies nothing"); +} + +#[test] +fn duplicate_ends_the_open_edit_and_selects_the_copy() { + let (mut s, t) = editing("Hello", 5); + s.execute("text.insert", json!({"text": "!"})).unwrap(); + assert_eq!(doc(&s, t).text, "Hello!"); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + // A deselected layer never keeps an open text edit: Duplicate commits the edit through the + // same route as text.endEdit before the selection moves, so every route (menu, shortcut, + // palette, MCP, script) leaves one final state. + assert!(s.state.text_edit.is_none(), "Duplicate ends the open text edit, as Deselect All does"); + assert_eq!(s.state.selected_layers, vec![effectcraft_project::LayerId(copy)], "and selects the copy"); + assert_eq!(doc(&s, t).text, "Hello!", "the original keeps the typed text"); + assert_eq!(doc(&s, copy).text, "Hello!", "the copy carries it too"); + // The edit ending is no undoable step of its own (as with text.endEdit): undo takes the + // duplicate, it does not reopen the edit or undo the typing. + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(layer_count(&s), 1, "undo removed the copy"); + assert_eq!(doc(&s, t).text, "Hello!", "the typed text stays committed"); + assert!(s.state.text_edit.is_none(), "undo does not reopen the edit"); + // With no open edit Duplicate is untouched by all of this: same behavior, no extra steps. + s.execute("edit.duplicate", json!({"layers": [t]})).unwrap(); + assert_eq!(layer_count(&s), 2, "a plain duplicate still duplicates"); + assert!(s.state.text_edit.is_none()); +} + +#[test] +fn duplicate_of_an_empty_type_tool_layer_ends_the_edit_and_copies_it() { + use effectcraft_project::LayerId; + let (mut s, _keep) = session("Hello"); + let new = s.execute("layer.newText", json!({"text": "", "position": [80.0, 80.0], "edit": true})).unwrap()["layer"].as_u64().unwrap(); + assert!(s.state.text_edit.as_ref().is_some_and(|e| e.created && e.layer == LayerId(new)), "the Type tool opens the edit on its new, empty layer"); + let before = layer_count(&s); + let copy = s.execute("edit.duplicate", json!({})).unwrap()[0].as_u64().unwrap(); + assert!(s.state.text_edit.is_none(), "the edit ends the way text.endEdit ends it"); + assert_eq!(layer_count(&s), before + 1, "the empty layer is kept, as text.endEdit keeps it, and copied"); + assert_eq!(s.state.selected_layers, vec![LayerId(copy)], "and the copy is selected"); } diff --git a/crates/engine/src/tests/three_d.rs b/crates/engine/src/tests/three_d.rs index 748f8a32f..5b5af594d 100644 --- a/crates/engine/src/tests/three_d.rs +++ b/crates/engine/src/tests/three_d.rs @@ -26,7 +26,8 @@ fn auto_orient_command() { assert!(!effectcraft_render::three_d::camera::is_two_node(&s.active_comp().unwrap().layers[0])); } -fn layer_count(s: &Session) -> usize { +/// Layers in the active comp. +pub(crate) fn layer_count(s: &Session) -> usize { s.active_comp().unwrap().layers.len() } diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index fd877ef5b..55475eef3 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -298,6 +298,25 @@ fn interpolation_dialog_temporal_spatial_and_roving() { assert_ne!(prop(&s, l, "transform/position").keys[0].out_interp, Interp::Hold); } +/// Editing another key's value or time keeps the roving key at its share of the path (#1284: the +/// roving key went back to where it was after edits to other keys). +#[test] +fn roving_key_follows_value_and_time_edits_to_other_keys() { + let (mut s, l) = setup(); + animate(&mut s, l, "transform/position", &[(0.0, json!([0, 0])), (0.3, json!([300, 0])), (2.0, json!([400, 0]))]); + let uid = prop(&s, l, "transform/position").uid; + s.execute("keys.select", json!({"keys": [{"layer": l, "prop": uid, "time": 0.3}]})).unwrap(); + s.execute("keys.interpolation", json!({"roving": true})).unwrap(); + let roving_at = |s: &Session| prop(s, l, "transform/position").keys[1].time.seconds(); + assert!((roving_at(&s) - 1.5).abs() < 0.01, "start: {}", roving_at(&s)); + // The last key's value moves to 800 px: the path is 300 + 500 px, so the roving key is at 300 / 800. + s.execute("keys.set", json!({"layer": l, "path": "transform/position", "time": 2.0, "value": [800, 0]})).unwrap(); + assert!((roving_at(&s) - 0.75).abs() < 0.02, "value edit: {}", roving_at(&s)); + // The last key moves to 3 s: the same 300 / 800 share of 3 s. + s.execute("keys.set", json!({"layer": l, "path": "transform/position", "time": 2.0, "newTime": 3.0})).unwrap(); + assert!((roving_at(&s) - 1.125).abs() < 0.02, "time edit: {}", roving_at(&s)); +} + #[test] fn graph_editor_key_and_handle_edits() { let (mut s, l) = setup(); @@ -325,6 +344,34 @@ fn graph_editor_key_and_handle_edits() { assert_eq!(prop(&s, l, "transform/opacity").keys[0].out_interp, Interp::Linear); } +#[test] +fn graph_editor_ease_edits_reject_a_locked_layer() { + let (mut s, l) = setup(); + animate(&mut s, l, "transform/opacity", &[(0.0, json!(0)), (1.0, json!(100)), (2.0, json!(0))]); + select_prop_keys(&mut s, l, "transform/opacity"); + s.execute("layer.setSwitch", json!({"layers": [l], "switch": "locked", "value": true})).unwrap(); + let before = prop(&s, l, "transform/opacity"); + let selected = s.state.selected_keys.clone(); + let undo = s.history.undo.len(); + for side in ["in", "out"] { + let error = + s.execute("keys.setEase", json!({"layer": l, "path": "transform/opacity", "time": 1.0, "side": side, "speed": 200, "influence": 60})).unwrap_err(); + assert!(error.to_string().contains("locked"), "{error}"); + assert_eq!(prop(&s, l, "transform/opacity").keys, before.keys); + assert_eq!(s.state.selected_keys, selected); + assert_eq!(s.history.undo.len(), undo); + } + s.execute("layer.setSwitch", json!({"layers": [l], "switch": "locked", "value": false})).unwrap(); + s.execute("keys.setEase", json!({"layer": l, "path": "transform/opacity", "time": 1.0, "side": "out", "speed": 200, "influence": 60})).unwrap(); + let after = prop(&s, l, "transform/opacity"); + assert_eq!(after.keys[1].out_interp, Interp::Bezier); + assert_eq!(after.keys[1].out_ease[0].speed, 200.0); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "transform/opacity").keys, before.keys); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "transform/opacity").keys, after.keys); +} + #[test] fn convert_expression_to_keyframes_on_a_reversed_layer_keeps_values() { let (mut s, l) = setup(); diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index 1c7c51177..ffbf57257 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -90,7 +90,7 @@ impl EffectcraftApp { /// Apply the engine events drained this tick: open a comp, show a toast, open a URL, run a frontend command, /// purge frame caches or copy an image; command and clipboard failures go to `app.ui.status`. - pub(super) fn handle_events(&mut self, ctx: &egui::Context) { + pub(crate) fn handle_events(&mut self, ctx: &egui::Context) { for ev in self.session.drain_events() { match ev { effectcraft_engine::Event::OpenComp(c) => { diff --git a/crates/ui-egui/src/dock_ui.rs b/crates/ui-egui/src/dock_ui.rs index 483dcf4bf..b66267d4d 100644 --- a/crates/ui-egui/src/dock_ui.rs +++ b/crates/ui-egui/src/dock_ui.rs @@ -406,7 +406,7 @@ impl EffectcraftApp { /// A press in a panel (its body, tab or group header) focuses it; another Composition viewer /// also becomes the active one. - fn focus_panel(&mut self, ctx: &egui::Context, p: PanelKind) { + pub(crate) fn focus_panel(&mut self, ctx: &egui::Context, p: PanelKind) { self.ui.focused = p; panels::viewer_text::panel_clicked(self, ctx, p); if let Some(id) = panels::viewers::id_of(p) { diff --git a/crates/ui-egui/src/header.rs b/crates/ui-egui/src/header.rs index 8710f1f9a..3f82c3908 100644 --- a/crates/ui-egui/src/header.rs +++ b/crates/ui-egui/src/header.rs @@ -226,7 +226,7 @@ fn snapping_toggle(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let snap = Rect::from_min_size(pos2(x, cy - 10.0), vec2(20.0, 20.0)); // The engine owns snapping (View ▸ Snapping); the checkbox mirrors it. app.ui.snapping = app.session.state.snapping; - if widgets::checkbox(ui, snap, app.ui.snapping, &t, egui::Id::unique("snapping")).clicked() { + if widgets::checkbox(ui, snap, app.ui.snapping, &t, egui::Id::unique("snapping")).changed() { let _ = app.session.execute("view.snapping", serde_json::json!({})); app.ui.snapping = app.session.state.snapping; } @@ -745,7 +745,7 @@ fn puppet_options(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter p.text(pos2(x, cy), Align2::LEFT_CENTER, tr("Mesh:"), Tokens::ui(12.0), t.text_dim); x += 40.0; let show = Rect::from_min_size(pos2(x, cy - 10.0), vec2(20.0, 20.0)); - if widgets::checkbox(ui, show, o.show_mesh, &t, egui::Id::unique("puppet-show-mesh")).clicked() { + if widgets::checkbox(ui, show, o.show_mesh, &t, egui::Id::unique("puppet-show-mesh")).changed() { let _ = app.session.execute("puppet.mesh", json!({"showMesh": !o.show_mesh})); } app.auto.add("header.puppet.showMesh", show, "Show mesh"); diff --git a/crates/ui-egui/src/menus/entries.rs b/crates/ui-egui/src/menus/entries.rs index d7b5d56ca..3391e47bd 100644 --- a/crates/ui-egui/src/menus/entries.rs +++ b/crates/ui-egui/src/menus/entries.rs @@ -125,6 +125,9 @@ pub(crate) fn entry_enabled(app: &EffectcraftApp, e: &MenuEntry) -> bool { if e.command == "edit.clear" && app.ui.focused == PanelKind::RenderQueue { return !app.session.is_rendering() && !app.session.project.render_queue.is_empty(); } + if e.command == "file.close" && super::invoke::no_params(&e.params) && app.ui.focused == PanelKind::RenderQueue { + return !app.ui.dock.locked; + } if e.command == "window.panel" { return true; } diff --git a/crates/ui-egui/src/menus/invoke.rs b/crates/ui-egui/src/menus/invoke.rs index 23065067f..67eb168be 100644 --- a/crates/ui-egui/src/menus/invoke.rs +++ b/crates/ui-egui/src/menus/invoke.rs @@ -168,7 +168,7 @@ fn vertex_delta(app: &EffectcraftApp, layer: effectcraft_engine::project::LayerI }) } -fn no_params(p: &Value) -> bool { +pub(super) fn no_params(p: &Value) -> bool { p.as_object().is_none_or(|m| m.is_empty()) } @@ -223,6 +223,21 @@ pub fn invoke(app: &mut EffectcraftApp, ctx: &egui::Context, id: &str, params: V _ => {} } } + // Close follows panel focus too: with the Render Queue active, Cmd+W closes that panel and + // leaves the active composition open (it used to close the composition next to it). + if id == "file.close" && no_params(¶ms) && app.ui.focused == PanelKind::RenderQueue { + app.apply_dock(craft_ui::docking::Action::Close { panel: PanelKind::RenderQueue })?; + // The focus goes to a panel that is still there, so the next Close acts on the composition + // instead of on the queue that is gone. + let next = [PanelKind::Timeline, PanelKind::Composition, PanelKind::Project].into_iter().find(|p| panel_shown(app, *p)).unwrap_or(PanelKind::Project); + app.show_panel(next); + app.focus_panel(ctx, next); + // Activating a passive viewer emits OpenComp, which raises Timeline. Finish that routing + // before restoring the destination panel, including when they share the same tab group. + app.handle_events(ctx); + app.show_panel(next); + return Ok(Value::Null); + } // Edit ▸ Paste with SVG markup on the system clipboard pastes it as a shape layer (After // Effects 26.3). Copying in the app replaces the system clipboard with a note naming what // was copied, so SVG there was copied after anything copied in the app. diff --git a/crates/ui-egui/src/menus/shortcuts.rs b/crates/ui-egui/src/menus/shortcuts.rs index f50fbdb3c..b343d2e40 100644 --- a/crates/ui-egui/src/menus/shortcuts.rs +++ b/crates/ui-egui/src/menus/shortcuts.rs @@ -136,8 +136,8 @@ pub fn chord_matches(session: &effectcraft_engine::Session, command: &str, key: bindings(session).into_iter().any(|(m, k, id, _)| id == command && k == key && mods_match(m, mods)) } -/// Whether a Deselect All chord matched by [`chord_matches`] may run while a text editor holds -/// the keyboard. Chords carrying Cmd/Ctrl/Alt are command chords (the editor treats them as +/// Whether a Deselect All or Duplicate chord matched by [`chord_matches`] may run while a text +/// editor holds the keyboard. Chords carrying Cmd/Ctrl/Alt are command chords (the editor treats them as /// such for undo and the rest), and function keys never type, so those forward from the editor; /// a chord that could instead produce a character — a bare or Shift-only letter, digit or /// punctuation key, as from a pathological bare-letter rebinding — stays with the editor so diff --git a/crates/ui-egui/src/menus/tests.rs b/crates/ui-egui/src/menus/tests.rs index ed2fcf6ca..36d667a72 100644 --- a/crates/ui-egui/src/menus/tests.rs +++ b/crates/ui-egui/src/menus/tests.rs @@ -418,6 +418,175 @@ fn tga_is_available_in_import_replace_and_proxy_dialogs() { } } +/// Cmd+W with the Render Queue focused closes the queue; the composition next to it stays +/// open (it used to be closed, and the queue stayed). +#[test] +fn cmd_w_in_the_render_queue_closes_the_queue_not_the_comp() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let a = app.session.execute("comp.new", json!({"name": "A", "duration": 4})).unwrap()["comp"].as_u64(); + app.show_panel(PanelKind::RenderQueue); + assert_eq!(app.ui.focused, PanelKind::RenderQueue); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.ui.dock.is_visible(PanelKind::RenderQueue)); + assert_eq!(app.session.state.active_comp.map(|c| c.0), a); + assert_eq!(app.session.state.open_comps.len(), 1); + // The focus left the closed queue: the next Close closes the composition. + assert_ne!(app.ui.focused, PanelKind::RenderQueue); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.session.state.open_comps.len(), 0, "a second Close acts on the composition"); +} + +#[test] +fn file_close_is_available_for_an_empty_render_queue_without_a_comp() { + for params in [Value::Null, json!({})] { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + assert!(app.session.active_comp().is_none()); + app.show_panel(PanelKind::RenderQueue); + assert!(menu_items(&app).iter().find(|item| item.id == "file.close").unwrap().enabled); + invoke(&mut app, &ctx, "file.close", params).unwrap(); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); + } +} + +#[test] +fn closing_the_render_queue_reveals_the_panel_receiving_focus() { + for maximized in [false, true] { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.close_panel(PanelKind::Timeline); + app.close_panel(PanelKind::Composition); + app.show_panel(PanelKind::EffectControls); + assert!(!app.ui.dock.is_visible(PanelKind::Project)); + app.show_panel(PanelKind::RenderQueue); + if maximized { + app.apply_dock(craft_ui::docking::Action::Float { panel: PanelKind::RenderQueue, rect: [50.0, 60.0, 400.0, 200.0] }).unwrap(); + app.toggle_maximize(PanelKind::EffectControls); + app.ui.focused = PanelKind::RenderQueue; + assert!(app.ui.dock.is_floating(PanelKind::RenderQueue)); + } + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.ui.focused, PanelKind::Project); + assert!(app.ui.dock.is_visible(app.ui.focused), "focus must belong to a front tab"); + assert_eq!(app.ui.maximized, None, "the focused docked panel must be on screen"); + } +} + +/// Closing a floating queue must focus the viewer that is actually shown, retaining maximization. +#[test] +fn closing_a_floating_queue_preserves_the_maximized_composition() { + let mut session = effectcraft_engine::Session::default(); + session.execute("comp.new", json!({"name": "A", "width": 32, "height": 32, "duration": 2})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.run_steps(3); + let ctx = h.ctx.clone(); + let app = h.state_mut(); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + app.apply_dock(craft_ui::docking::Action::Float { panel: PanelKind::RenderQueue, rect: [50.0, 60.0, 400.0, 200.0] }).unwrap(); + app.toggle_maximize(PanelKind::Composition); + app.ui.focused = PanelKind::RenderQueue; + assert!(app.ui.dock.is_visible(PanelKind::Timeline), "the hidden underlying tab triggers the regression"); + assert!(!panel_shown(app, PanelKind::Timeline)); + invoke(app, &ctx, "file.close", json!({})).unwrap(); + h.run_steps(3); + assert_eq!(h.state().ui.focused, PanelKind::Composition); + assert_eq!(h.state().ui.maximized, Some(PanelKind::Composition)); + assert!(!h.state().ui.dock.is_open(PanelKind::RenderQueue)); + assert_eq!(h.state().session.state.open_comps.len(), 1); +} + +#[test] +fn queue_close_preserves_state_when_workspace_is_locked() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + app.session.execute("comp.new", json!({"name": "A", "duration": 2})).unwrap(); + app.show_panel(PanelKind::RenderQueue); + app.ui.dock.locked = true; + let workspace = app.ui.dock.clone(); + let comp = app.session.active_comp_id(); + for _ in 0..2 { + assert!(invoke(&mut app, &ctx, "file.close", Value::Null).is_err()); + assert_eq!(app.ui.dock, workspace); + assert_eq!(app.ui.focused, PanelKind::RenderQueue); + assert_eq!(app.session.active_comp_id(), comp); + } + assert!(!menu_items(&app).iter().find(|item| item.id == "file.close").unwrap().enabled); + app.ui.dock.locked = false; + invoke(&mut app, &ctx, "file.close", Value::Null).unwrap(); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); +} + +#[test] +fn queue_close_preserves_viewer_command_target() { + let ctx = egui::Context::default(); + let mut app = EffectcraftApp::new(effectcraft_engine::Session::default()); + let a = app.session.execute("comp.new", json!({"name": "A", "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let b = app.session.execute("comp.new", json!({"name": "B", "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + app.show_panel(PanelKind::Viewer(1)); + app.ui.viewers.insert(0, Some(a)); + app.ui.viewers.insert(1, Some(b)); + app.ui.active_viewer = 1; + app.apply_dock(craft_ui::docking::Action::Move { + panel: PanelKind::Timeline, + anchor: PanelKind::Composition, + placement: craft_ui::docking::Placement::Tab { before: None }, + }) + .unwrap(); + app.close_panel(PanelKind::Timeline); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(b)); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert_eq!(app.ui.focused, PanelKind::Composition); + assert_eq!(app.ui.active_viewer, 0); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(a)); + invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.session.state.open_comps.iter().any(|id| id.0 == a)); + assert!(app.session.state.open_comps.iter().any(|id| id.0 == b)); +} + +#[test] +fn queue_close_keeps_passive_viewer_focus_across_frames() { + let mut session = effectcraft_engine::Session::default(); + let a = session.execute("comp.new", json!({"name": "A", "width": 32, "height": 32, "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let b = session.execute("comp.new", json!({"name": "B", "width": 32, "height": 32, "duration": 2})).unwrap()["comp"].as_u64().unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.run_steps(3); + let ctx = h.ctx.clone(); + let app = h.state_mut(); + app.show_panel(PanelKind::Viewer(1)); + app.ui.viewers.insert(0, Some(a)); + app.ui.viewers.insert(1, Some(b)); + app.ui.active_viewer = 1; + app.apply_dock(craft_ui::docking::Action::Move { + panel: PanelKind::Timeline, + anchor: PanelKind::Composition, + placement: craft_ui::docking::Placement::Tab { before: None }, + }) + .unwrap(); + app.close_panel(PanelKind::Timeline); + app.show_panel(PanelKind::Composition); + app.show_panel(PanelKind::RenderQueue); + invoke(app, &ctx, "file.close", Value::Null).unwrap(); + h.run_steps(4); + let app = h.state_mut(); + assert_eq!(app.ui.focused, PanelKind::Composition); + assert_eq!(app.ui.active_viewer, 0); + assert_eq!(app.session.active_comp_id().map(|id| id.0), Some(a)); + assert!(app.ui.dock.is_visible(PanelKind::Composition)); + assert!(!app.ui.dock.is_open(PanelKind::RenderQueue)); + invoke(app, &ctx, "file.close", json!({})).unwrap(); + assert!(!app.session.state.open_comps.iter().any(|id| id.0 == a)); + assert!(app.session.state.open_comps.iter().any(|id| id.0 == b)); + // A later explicit Open still routes normally, rather than inheriting a deferred focus override. + invoke(app, &ctx, "comp.open", json!({"comp": b})).unwrap(); + h.run_steps(3); + assert!(h.state().ui.dock.is_visible(PanelKind::Timeline)); + assert_eq!(h.state().session.active_comp_id().map(|id| id.0), Some(b)); +} + #[test] fn menu_items_cover_the_tree() { assert_eq!(menus().last(), Some(&"Help")); diff --git a/crates/ui-egui/src/panels/anim_tools.rs b/crates/ui-egui/src/panels/anim_tools.rs index e6b62efb4..67ae71c0b 100644 --- a/crates/ui-egui/src/panels/anim_tools.rs +++ b/crates/ui-egui/src/panels/anim_tools.rs @@ -45,7 +45,7 @@ fn number(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &st fn checkbox(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(at, vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("animtool-cb", auto))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("animtool-cb", auto))).changed(); ui.painter().text(pos2(r.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); app.auto.add(auto, r, text); if clicked { !on } else { on } diff --git a/crates/ui-egui/src/panels/comp_settings.rs b/crates/ui-egui/src/panels/comp_settings.rs index 01fbc5995..7e49d8a7d 100644 --- a/crates/ui-egui/src/panels/comp_settings.rs +++ b/crates/ui-egui/src/panels/comp_settings.rs @@ -310,7 +310,7 @@ fn tab_body( let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut d.width).range(4..=30000).suffix(" px"))); auto.add("dialog.comp.width", r.rect, "Width"); let lbl = tr_args("Lock Aspect Ratio to {}", &[&(aspect_label(ow as f64, oh as f64))]); - let r = ui.checkbox(&mut d.lock_aspect, lbl); + let r = crate::widgets::labelled_checkbox(ui, &mut d.lock_aspect, lbl); auto.add("dialog.comp.lockAspect", r.rect, "Lock Aspect Ratio"); if d.lock_aspect && ow != d.width && ow > 0 { d.height = ((d.width as f64) * oh as f64 / ow as f64).round().max(4.0) as u32; @@ -426,11 +426,11 @@ fn tab_body( }); ui.end_row(); ui.label(""); - let r = ui.checkbox(&mut d.preserve_frame_rate, tr("Preserve frame rate when nested or in render queue")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.preserve_frame_rate, tr("Preserve frame rate when nested or in render queue")); auto.add("dialog.comp.preserveFrameRate", r.rect, "Preserve frame rate when nested or in render queue"); ui.end_row(); ui.label(""); - let r = ui.checkbox(&mut d.preserve_resolution, tr("Preserve resolution when nested")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.preserve_resolution, tr("Preserve resolution when nested")); auto.add("dialog.comp.preserveResolution", r.rect, "Preserve resolution when nested"); ui.end_row(); ui.label(egui::RichText::new(tr("Motion Blur")).strong()); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index a0c4b7651..9b09bd1ef 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -584,7 +584,11 @@ fn solid(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.horizontal(|ui| { let r = ui.add(crate::widgets::drag_value(egui::DragValue::new(&mut size[0]).range(1..=30000).suffix(" px"))); auto.add("dialog.solid.width", r.rect, "Width"); - let r = ui.checkbox(&mut lock, tr_args("Lock Aspect Ratio to {}", &[&(super::comp_settings::aspect_label(ow as f64, oh as f64))])); + let r = crate::widgets::labelled_checkbox( + ui, + &mut lock, + tr_args("Lock Aspect Ratio to {}", &[&(super::comp_settings::aspect_label(ow as f64, oh as f64))]), + ); auto.add("dialog.solid.lockAspect", r.rect, "Lock Aspect Ratio"); }); ui.end_row(); @@ -640,7 +644,7 @@ fn solid(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if editing.is_some() { ui.add_space(8.0); let r = ui - .add_enabled(shared, egui::Checkbox::new(&mut affect_all, tr("Affect all layers that use this solid"))) + .add_enabled(shared, crate::widgets::DragCheckbox::new(&mut affect_all, tr("Affect all layers that use this solid"))) .on_disabled_hover_text(tr("Only this layer uses this solid.")); auto.add("dialog.solid.affectAll", r.rect, "Affect all layers that use this solid"); if shared && !affect_all { diff --git a/crates/ui-egui/src/panels/dialogs_3d.rs b/crates/ui-egui/src/panels/dialogs_3d.rs index 9b3535288..16137b599 100644 --- a/crates/ui-egui/src/panels/dialogs_3d.rs +++ b/crates/ui-egui/src/panels/dialogs_3d.rs @@ -231,12 +231,12 @@ pub fn camera(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.end_row(); ui.label(tr("Depth of Field")); - ui.checkbox(&mut d.dof, tr("Enable Depth of Field")); + crate::widgets::labelled_checkbox(ui, &mut d.dof, tr("Enable Depth of Field")); ui.end_row(); ui.label(tr("Focus Distance")); ui.horizontal(|ui| { ui.add_enabled(!d.lock_to_zoom, crate::widgets::drag_value(egui::DragValue::new(&mut d.focus).range(0.0..=100000.0).suffix(" px"))); - ui.checkbox(&mut d.lock_to_zoom, tr("Lock to Zoom")); + crate::widgets::labelled_checkbox(ui, &mut d.lock_to_zoom, tr("Lock to Zoom")); }); ui.end_row(); ui.label(tr("Aperture")); @@ -354,7 +354,7 @@ pub fn light(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } if casts_shadows(d.kind) { ui.label(tr("Shadows")); - let cb = ui.checkbox(&mut d.casts_shadows, tr("Casts Shadows")); + let cb = crate::widgets::labelled_checkbox(ui, &mut d.casts_shadows, tr("Casts Shadows")); app.auto.add_response("dialog.light.casts", &cb, "Casts Shadows"); ui.end_row(); ui.label(tr("Shadow Darkness")); diff --git a/crates/ui-egui/src/panels/effect_controls/rows.rs b/crates/ui-egui/src/panels/effect_controls/rows.rs index 249c8462b..8a75dbff6 100644 --- a/crates/ui-egui/src/panels/effect_controls/rows.rs +++ b/crates/ui-egui/src/panels/effect_controls/rows.rs @@ -367,7 +367,7 @@ fn value_toggle(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: }; let PropCx { t, uid, cy, vx, p, prop, .. } = *cx; let cr = Rect::from_min_size(pos2(vx, cy - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, cr, *b, &t, egui::Id::unique(("ec-b", uid))).clicked() { + if widgets::checkbox(ui, cr, *b, &t, egui::Id::unique(("ec-b", uid))).changed() { cx.set(actions, json!(!b)); } p.text(pos2(cr.max.x + 6.0, cy), Align2::LEFT_CENTER, &prop.name, Tokens::ui(12.0), t.text_dim); diff --git a/crates/ui-egui/src/panels/essential.rs b/crates/ui-egui/src/panels/essential.rs index a08b96b62..02e077508 100644 --- a/crates/ui-egui/src/panels/essential.rs +++ b/crates/ui-egui/src/panels/essential.rs @@ -362,7 +362,7 @@ fn value_widget(app: &mut EffectcraftApp, ui: &mut egui::Ui, auto: &str, ty: Opt } ControlType::Checkbox => { let mut on = v.as_bool(); - let r = ui.checkbox(&mut on, ""); + let r = crate::widgets::labelled_checkbox(ui, &mut on, ""); if r.changed() { out = Some(json!(on)); } diff --git a/crates/ui-egui/src/panels/forms.rs b/crates/ui-egui/src/panels/forms.rs index 29364275b..bb686f15e 100644 --- a/crates/ui-egui/src/panels/forms.rs +++ b/crates/ui-egui/src/panels/forms.rs @@ -888,7 +888,7 @@ fn form_grid(ui: &mut egui::Ui, f: &mut Form, regs: &mut Vec<(String, egui::Rect .inner .rect } - FieldKind::Bool(b) => ui.checkbox(b, "").rect, + FieldKind::Bool(b) => crate::widgets::labelled_checkbox(ui, b, "").rect, FieldKind::Choice { options, sel } => { let cur = options.get(*sel).map(|o| o.0.clone()).unwrap_or_default(); // The column keeps its width: a long choice (a GPU adapter's name) ends in an @@ -1018,13 +1018,13 @@ pub fn show_view_options(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Toke let mut close = false; super::dialogs::modal(ctx, "View Options", vec2(360.0, 330.0), t, |ui| { let v = &mut app.ui.viewer; - ui.checkbox(&mut v.show_layer_controls, tr("Layer controls")); - ui.checkbox(&mut v.show_masks, tr("Masks")); - ui.checkbox(&mut v.rulers, tr("Rulers")); - ui.checkbox(&mut v.guides, tr("Guides")); - ui.checkbox(&mut v.grid, tr("Grid")); - ui.checkbox(&mut v.safe_margins, tr("Title/action safe")); - ui.checkbox(&mut v.transparency_grid, tr("Transparency grid")); + crate::widgets::labelled_checkbox(ui, &mut v.show_layer_controls, tr("Layer controls")); + crate::widgets::labelled_checkbox(ui, &mut v.show_masks, tr("Masks")); + crate::widgets::labelled_checkbox(ui, &mut v.rulers, tr("Rulers")); + crate::widgets::labelled_checkbox(ui, &mut v.guides, tr("Guides")); + crate::widgets::labelled_checkbox(ui, &mut v.grid, tr("Grid")); + crate::widgets::labelled_checkbox(ui, &mut v.safe_margins, tr("Title/action safe")); + crate::widgets::labelled_checkbox(ui, &mut v.transparency_grid, tr("Transparency grid")); ui.add_space(12.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { if ui.add(egui::Button::new(egui::RichText::new(tr(" OK ")).color(Color32::WHITE)).fill(t.accent)).clicked() { diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index 35a745c81..be18999d4 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -470,6 +470,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let mut key_screens: Vec<(serde_json::Value, Pos2)> = vec![]; for c in &cs { let (lid, uid, d) = (c.layer.id, c.prop.uid, c.dim); + let unlocked = !c.layer.switches.locked; for k in &c.keys { let kp = pos2(tm.x(k.t), ymap(k.v)); // In the speed graph an end key rises with the handle being dragged; skipping it once it @@ -491,9 +492,10 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte app.auto.add(&format!("timeline.graph.handle.{uid}.{d}.{}.{side}", k.idx), hr, "Bezier handle"); // The same side's handles of every selected key move with it, by as much as the // pointer moved (After Effects), in one undo step. - if hresp.drag_started() { + if unlocked && hresp.drag_started() { let grabs: Vec = cs .iter() + .filter(|c2| !c2.layer.switches.locked) .flat_map(|c2| { c2.keys.iter().flat_map(move |k2| { k2.handles.iter().filter(|h2| h2.out == h.out).map(move |h2| HandleGrab { @@ -513,7 +515,8 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let press = ui.input(|i| i.pointer.press_origin()).unwrap_or(hp); ctx.data_mut(|dd| dd.insert_temp::(handle_drag_id(), (press, grabs))); } - if hresp.dragged() + if unlocked + && hresp.dragged() && let Some(pt) = hresp.interact_pointer_pos() && let Some((press, grabs)) = ctx.data(|dd| dd.get_temp::(handle_drag_id())) { @@ -544,7 +547,7 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte app.auto.add(&format!("timeline.graph.key.{uid}.{d}.{}", k.idx), kr, &c.prop.name); let mods = ui.input(|i| i.modifiers); let key = &c.prop.keys[k.idx]; - if kresp.clicked() && mods.command { + if unlocked && kresp.clicked() && mods.command { // Ctrl+click: Linear ↔ Auto Bezier; Ctrl+Alt+click: Hold on / off (as in the Timeline). actions.push(("keys.select".into(), json!({"keys": [kjson.clone()]}))); if mods.alt { @@ -553,20 +556,22 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let linear = key.in_interp == Interp::Linear && key.out_interp == Interp::Linear; actions.push(("keys.interpolation".into(), json!({"interpolation": if linear { "autoBezier" } else { "linear" }}))); } - } else if kresp.clicked() && mods.shift { + } else if unlocked && kresp.clicked() && mods.shift { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "toggle": true}))); - } else if (kresp.clicked() || kresp.drag_started()) && !k.sel { + } else if unlocked && (kresp.clicked() || kresp.drag_started() || kresp.secondary_clicked()) && !k.sel { actions.push(("keys.select".into(), json!({"keys": [kjson.clone()], "add": mods.shift}))); } // A drag moves every selected key, by how far the pointer went from where the button // went down (the key doesn't jump under the pointer); Shift keeps it to the time or // the value axis, whichever moved more. - if kresp.drag_started() + if unlocked + && kresp.drag_started() && let Some(o) = ui.input(|i| i.pointer.press_origin()) { ctx.data_mut(|dd| dd.insert_temp(key_grab_id(), KeyGrab { press: o, t: k.t })); } - if kresp.dragged() + if unlocked + && kresp.dragged() && let Some(pt) = kresp.interact_pointer_pos() && let Some(g) = ctx.data(|dd| dd.get_temp::(key_grab_id())) { @@ -587,6 +592,8 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte ctx.data_mut(|dd| dd.insert_temp(drag_id, false)); actions.push(("__endMerge".into(), json!({}))); } + // Right-click: the Timeline's key menu, with the same commands. + crate::widgets::context_menu(&kresp, |ui| super::timeline::key_menu(ui, app, c.layer, c.prop, key, k.sel, actions)); } } diff --git a/crates/ui-egui/src/panels/key_dialogs.rs b/crates/ui-egui/src/panels/key_dialogs.rs index 22e31c468..2a5579837 100644 --- a/crates/ui-egui/src/panels/key_dialogs.rs +++ b/crates/ui-egui/src/panels/key_dialogs.rs @@ -168,7 +168,7 @@ pub fn velocity(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.add_space(8.0); } - let r = ui.checkbox(&mut d.continuous, tr("Continuous (Lock Outgoing to Incoming)")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.continuous, tr("Continuous (Lock Outgoing to Incoming)")); app.auto.add("dialog.velocity.continuous", r.rect, "Continuous"); if d.continuous { d.out_speed = d.in_speed.clone(); diff --git a/crates/ui-egui/src/panels/layer_styles_dialog.rs b/crates/ui-egui/src/panels/layer_styles_dialog.rs index b646b002e..5b415187d 100644 --- a/crates/ui-egui/src/panels/layer_styles_dialog.rs +++ b/crates/ui-egui/src/panels/layer_styles_dialog.rs @@ -89,7 +89,7 @@ fn control(ui: &mut egui::Ui, p: &Property, v: &KV) -> Option { match (v, &p.ui) { (KV::Bool(b), _) => { let mut on = *b; - ui.checkbox(&mut on, "").changed().then(|| json!(on)) + crate::widgets::labelled_checkbox(ui, &mut on, "").changed().then(|| json!(on)) } (KV::Enum(i), ParamUi::Popup { options }) => { let mut sel = *i as usize; @@ -177,7 +177,7 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { if id != BLENDING { let g = styles.as_ref().and_then(|s| s.sub(id)); let mut on = g.is_some_and(|g| g.enabled); - if ui.checkbox(&mut on, "").changed() { + if crate::widgets::labelled_checkbox(ui, &mut on, "").changed() { if g.is_none() { actions.push(("layer.style.add".into(), json!({"layers": [lid], "style": id}))); } else { diff --git a/crates/ui-egui/src/panels/markers_ui.rs b/crates/ui-egui/src/panels/markers_ui.rs index 03c45d32b..2c21a293b 100644 --- a/crates/ui-egui/src/panels/markers_ui.rs +++ b/crates/ui-egui/src/panels/markers_ui.rs @@ -155,7 +155,7 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ui.end_row(); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.cue, tr("Cue Point")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.cue, tr("Cue Point")); app.auto.add("dialog.marker.cue", r.rect, "Cue Point"); ui.add_enabled_ui(d.cue, |ui| { ui.horizontal(|ui| { @@ -191,7 +191,7 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { }); ui.add_space(8.0); ui.horizontal(|ui| { - let r = ui.checkbox(&mut d.protected, tr("Protected Region")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.protected, tr("Protected Region")); app.auto.add("dialog.marker.protected", r.rect, "Protected Region"); ui.add_space(20.0); ui.label(tr("Label:")); diff --git a/crates/ui-egui/src/panels/misc.rs b/crates/ui-egui/src/panels/misc.rs index 2fec0e4c2..73a0b3a1c 100644 --- a/crates/ui-egui/src/panels/misc.rs +++ b/crates/ui-egui/src/panels/misc.rs @@ -264,7 +264,7 @@ fn fmt_rate(r: f64) -> String { fn check_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, at: egui::Pos2, key: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(pos2(at.x, at.y - 8.0), vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("pv-check", key))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("pv-check", key))).changed(); p.text(pos2(r.max.x + 5.0, at.y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); app.auto.add(&format!("preview.{key}"), r, text); clicked diff --git a/crates/ui-egui/src/panels/paint_panels.rs b/crates/ui-egui/src/panels/paint_panels.rs index c867eb63a..edf12ed7a 100644 --- a/crates/ui-egui/src/panels/paint_panels.rs +++ b/crates/ui-egui/src/panels/paint_panels.rs @@ -239,7 +239,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .enumerate() { let r = Rect::from_min_size(pos2(x0 + i as f32 * (w / 2.0), y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, r, on, &t, egui::Id::unique(key)).clicked() { + if widgets::checkbox(ui, r, on, &t, egui::Id::unique(key)).changed() { set(app, json!({k: !on})); } p.text(pos2(r.max.x + 4.0, r.center().y), Align2::LEFT_CENTER, label, Tokens::ui(11.5), t.text_dim); @@ -273,7 +273,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } y += 26.0; let r = Rect::from_min_size(pos2(x0, y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, r, o.show_overlay, &t, egui::Id::unique("paint.showOverlay")).clicked() { + if widgets::checkbox(ui, r, o.show_overlay, &t, egui::Id::unique("paint.showOverlay")).changed() { set(app, json!({"showOverlay": !o.show_overlay})); } app.auto.add("paint.showOverlay", r, "Clone Source Overlay"); @@ -282,7 +282,7 @@ pub fn paint(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { set(app, json!({"overlayOpacity": nv})); } let dr = Rect::from_min_size(pos2(x0 + w - 18.0, y), vec2(18.0, 20.0)); - if widgets::checkbox(ui, dr, o.overlay_difference, &t, egui::Id::unique("paint.overlayDifference")).clicked() { + if widgets::checkbox(ui, dr, o.overlay_difference, &t, egui::Id::unique("paint.overlayDifference")).changed() { set(app, json!({"overlayDifference": !o.overlay_difference})); } app.auto.add("paint.overlayDifference", dr, "Difference mode overlay"); diff --git a/crates/ui-egui/src/panels/panel_kit.rs b/crates/ui-egui/src/panels/panel_kit.rs index 501ddf64f..c1d88a417 100644 --- a/crates/ui-egui/src/panels/panel_kit.rs +++ b/crates/ui-egui/src/panels/panel_kit.rs @@ -23,7 +23,7 @@ pub fn dropdown(app: &mut EffectcraftApp, ui: &mut egui::Ui, r: Rect, auto: &str pub fn checkbox(app: &mut EffectcraftApp, ui: &mut egui::Ui, at: egui::Pos2, auto: &str, text: &str, on: bool) -> bool { let t = app.tokens; let r = Rect::from_min_size(at, vec2(16.0, 16.0)); - let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("kit-cb", auto))).clicked(); + let clicked = widgets::checkbox(ui, r, on, &t, egui::Id::unique(("kit-cb", auto))).changed(); ui.painter().text(pos2(r.max.x + 6.0, r.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), t.text); let full = Rect::from_min_size(at, vec2(22.0 + text.len() as f32 * 6.5, 16.0)); app.auto.add(auto, full, text); diff --git a/crates/ui-egui/src/panels/path_vr_panels.rs b/crates/ui-egui/src/panels/path_vr_panels.rs index 680d9f5a0..fcba902e4 100644 --- a/crates/ui-egui/src/panels/path_vr_panels.rs +++ b/crates/ui-egui/src/panels/path_vr_panels.rs @@ -54,7 +54,7 @@ pub fn create_nulls(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { acts.push((id, if id == "paths.tracePath" { json!({"loop": looping}) } else { json!({}) })); } } - let r = ui.checkbox(&mut looping, tr("Loop (Trace Path)")); + let r = crate::widgets::labelled_checkbox(&mut ui, &mut looping, tr("Loop (Trace Path)")); app.auto.add("createNulls.loop", r.rect, "Loop"); ui.data_mut(|d| d.insert_temp(loop_id, looping)); run(app, &ctx, acts); diff --git a/crates/ui-egui/src/panels/precomp.rs b/crates/ui-egui/src/panels/precomp.rs index 7c1750d5e..6ec9072f3 100644 --- a/crates/ui-egui/src/panels/precomp.rs +++ b/crates/ui-egui/src/panels/precomp.rs @@ -141,11 +141,11 @@ pub fn dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { ); ui.add_space(4.0); ui.add_enabled_ui(!d.leave, |ui| { - let r = ui.checkbox(&mut d.adjust, tr("Adjust composition duration to the time span of the selected layers")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.adjust, tr("Adjust composition duration to the time span of the selected layers")); app.auto.add("dialog.precompose.adjust", r.rect, "Adjust composition duration"); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.open, tr("Open New Composition")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.open, tr("Open New Composition")); app.auto.add("dialog.precompose.open", r.rect, "Open New Composition"); ui.add_space(14.0); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { diff --git a/crates/ui-egui/src/panels/project.rs b/crates/ui-egui/src/panels/project.rs index cdf57de27..2da839fe3 100644 --- a/crates/ui-egui/src/panels/project.rs +++ b/crates/ui-egui/src/panels/project.rs @@ -1341,7 +1341,7 @@ fn column_menu(app: &mut EffectcraftApp, resp: &egui::Response) { ui.label(egui::RichText::new(crate::i18n::tr("Columns")).weak()); for (key, label, _) in COLUMNS { let mut on = app.ui.project_columns.iter().any(|c| c == key); - if ui.checkbox(&mut on, crate::i18n::tr(label)).changed() { + if crate::widgets::labelled_checkbox(ui, &mut on, crate::i18n::tr(label)).changed() { set_column(&mut app.ui.project_columns, key, on); } } diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index a7563a6ff..432c8f627 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -456,7 +456,7 @@ fn text_section( for (i, (key, label, on, c)) in [("fill", "Fill", doc.apply_fill, doc.fill), ("stroke", "Stroke", doc.apply_stroke, doc.stroke)].into_iter().enumerate() { let ry = y + i as f32 * 24.0; let cr = Rect::from_min_size(pos2(x0, ry + 2.0), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("props-apply", key))).clicked() { + if widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("props-apply", key))).changed() { let k = if key == "fill" { "applyFill" } else { "applyStroke" }; set(actions, json!({k: !on})); } diff --git a/crates/ui-egui/src/panels/render_queue.rs b/crates/ui-egui/src/panels/render_queue.rs index 321224c64..46ddc77ff 100644 --- a/crates/ui-egui/src/panels/render_queue.rs +++ b/crates/ui-egui/src/panels/render_queue.rs @@ -501,7 +501,7 @@ fn current_render_strip(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::P // Notify (sound + notification when the queue finishes). let notify = app.session.project.render_prefs.notify; let nb = Rect::from_min_size(pos2(render_b.min.x - 8.0 - 2.0 * bw - 8.0, render_b.min.y + 4.0), vec2(18.0, 18.0)); - if widgets::checkbox(ui, nb, notify, &t, egui::Id::unique("rq-notify")).clicked() { + if widgets::checkbox(ui, nb, notify, &t, egui::Id::unique("rq-notify")).changed() { actions.push(("renderQueue.setNotify", json!({"notify": !notify}))); } p.text(pos2(nb.max.x + 5.0, nb.center().y), Align2::LEFT_CENTER, tr("Notify"), Tokens::ui(11.5), t.text_dim); @@ -797,7 +797,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } app.auto.add(&aid("twirl"), tw, "Twirl"); let cb = Rect::from_center_size(pos2(cx_render + 20.0, row.center().y), vec2(18.0, 18.0)); - if widgets::checkbox(ui, cb, it.render, &t, egui::Id::unique(("rq-cb", it.id))).clicked() && !rendering { + if ui.add_enabled_ui(!rendering, |ui| widgets::checkbox(ui, cb, it.render, &t, egui::Id::unique(("rq-cb", it.id)))).inner.changed() && !rendering { actions.push(("renderQueue.setRender", json!({"item": it.id, "render": !it.render}))); } app.auto.add(&aid("render"), cb, if it.render { "Render: on" } else { "Render: off" }); diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 562a90132..6b5c371f8 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -106,7 +106,7 @@ fn page_ui(app: &mut EffectcraftApp, ui: &mut egui::Ui, page: &Page, cur: &Value match kind { Kind::Bool => { let mut b = v.as_bool().unwrap_or(false); - let r = ui.checkbox(&mut b, tr(label)); + let r = crate::widgets::labelled_checkbox(ui, &mut b, tr(label)); if r.changed() { acts.push(Act::Set(key.into(), json!(b))); } diff --git a/crates/ui-egui/src/panels/text_panels.rs b/crates/ui-egui/src/panels/text_panels.rs index 4a2a8b6cd..caded11fd 100644 --- a/crates/ui-egui/src/panels/text_panels.rs +++ b/crates/ui-egui/src/panels/text_panels.rs @@ -297,7 +297,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { for (i, (key, label, on, c)) in [("fill", "Fill", doc.apply_fill, doc.fill), ("stroke", "Stroke", doc.apply_stroke, doc.stroke)].into_iter().enumerate() { let fx = x0 + i as f32 * (w / 2.0); let cr = Rect::from_min_size(pos2(fx, y + 3.0), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("char-apply", key))).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("char-apply", key)))).inner.changed() && enabled { actions.push(json!({if key == "fill" { "applyFill" } else { "applyStroke" }: !on})); } app.auto.add(&format!("character.{key}.enabled"), cr, label); @@ -350,7 +350,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { y += 32.0; // Ligatures. let lr = Rect::from_min_size(pos2(x0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, lr, doc.ligatures, &t, egui::Id::unique("char-ligatures")).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, lr, doc.ligatures, &t, egui::Id::unique("char-ligatures"))).inner.changed() && enabled { actions.push(json!({"ligatures": !doc.ligatures})); } app.auto.add("character.ligatures", lr, "Ligatures"); @@ -374,7 +374,7 @@ pub fn character(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { .enumerate() { let cr = Rect::from_min_size(pos2(x0 + i as f32 * 120.0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("char-vert", key))).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, cr, on, &t, egui::Id::unique(("char-vert", key)))).inner.changed() && enabled { actions.push(json!({key: !on})); } app.auto.add(&format!("character.{key}"), cr, label); @@ -677,7 +677,7 @@ pub fn paragraph(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } y += 30.0; let hr = Rect::from_min_size(pos2(x0, y), vec2(14.0, 14.0)); - if widgets::checkbox(ui, hr, doc.hanging_punctuation, &t, egui::Id::unique("para-hanging")).clicked() && enabled { + if ui.add_enabled_ui(enabled, |ui| widgets::checkbox(ui, hr, doc.hanging_punctuation, &t, egui::Id::unique("para-hanging"))).inner.changed() && enabled { actions.push(json!({"hangingPunctuation": !doc.hanging_punctuation})); } app.auto.add("paragraph.hangingPunctuation", hr, "Roman Hanging Punctuation"); @@ -777,11 +777,7 @@ pub(super) fn font_picker(app: &mut EffectcraftApp, ui: &mut egui::Ui, id: egui: ui.set_width(rect.width().max(180.0)); // Filter row: a checkbox that keeps only the variable fonts. let filter_h = 22.0; - let row = ui.add_sized([ui.available_width(), filter_h], egui::Button::selectable(false, "")); - let box_rect = Rect::from_min_size(row.rect.min, vec2(18.0, filter_h)); - let label_rect = Rect::from_min_max(pos2(box_rect.max.x + 2.0, row.rect.min.y), row.rect.max); - let boxed = widgets::checkbox(ui, box_rect, only_variable, &t, id.with("variable-check")); - ui.painter().text(label_rect.left_center(), Align2::LEFT_CENTER, tr("Variable fonts only"), Tokens::ui(11.5), t.text); + let row = ui.add_sized([ui.available_width(), filter_h], widgets::DragCheckbox::new(&mut only_variable, tr("Variable fonts only"))); app_ref.auto.add(&format!("{auto}.variable"), row.rect, "Variable fonts only"); // #1089: fonts installed after launch appear without restarting. The button is its own // row: the popup is only 180 px wide, so a control pinned to the filter row covers @@ -792,11 +788,13 @@ pub(super) fn font_picker(app: &mut EffectcraftApp, ui: &mut egui::Ui, id: egui: app_ref.ui.status = format!("{}…", tr("Rescan Fonts")); } app_ref.auto.add(&format!("{auto}.rescan"), resc, "Rescan Fonts"); - let toggled = row.clicked() || boxed.clicked(); + let toggled = row.changed(); if toggled { - only_variable = !only_variable; ui.data_mut(|d| d.insert_temp(variable_id, only_variable)); - // Typing goes on into the search field. + } + // The native checkbox can take focus on release after changing on press. + // Typing goes on into the search field after either event. + if toggled || row.clicked() { ui.memory_mut(|m| m.request_focus(search_id)); } let query = state.query.trim().to_lowercase(); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 3fcd5b875..19a8d9fc2 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -3058,7 +3058,7 @@ fn layer_mode_cell(app: &mut EffectcraftApp, ui: &mut egui::Ui, g: &mut Rw, rc: } let tr = Rect::from_center_size(pos2(cw.mode + 76.0 + 6.0, cy), vec2(14.0, 14.0)); app.auto.add(&format!("timeline.layer.{}.preserveTransparency", layer.id.0), tr, "Preserve Transparency"); - if widgets::checkbox(ui, tr, layer.preserve_transparency, &t, egui::Id::unique(("pt", layer.id.0))).clicked() { + if widgets::checkbox(ui, tr, layer.preserve_transparency, &t, egui::Id::unique(("pt", layer.id.0))).changed() { // The click gives the selection the state the clicked row toggles to, // whatever the first selected layer's value (as the switches do). actions.push(( @@ -3565,7 +3565,7 @@ fn group_mask_mode(ui: &mut egui::Ui, g: &mut Rw, rc: &RowCtx<'_>) { actions.push(("layer.setMask".into(), json!({"layer": layer.id.0, "mask": uid, "mode": opts[i]}))); } let ir = Rect::from_min_size(pos2(mr.max.x + 8.0, cy - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, ir, inverted, &t, egui::Id::unique(("minv", uid))).clicked() { + if widgets::checkbox(ui, ir, inverted, &t, egui::Id::unique(("minv", uid))).changed() { actions.push(("layer.setMask".into(), json!({"layer": layer.id.0, "mask": uid, "inverted": !inverted}))); } lp.text(pos2(ir.max.x + 4.0, cy), Align2::LEFT_CENTER, crate::i18n::tr("Inverted"), Tokens::ui(11.0), t.text_dim); @@ -4100,72 +4100,91 @@ fn key_double_click(g: &mut Rw, rc: &RowCtx<'_>, prop: &Property, kh: &KeyHit<'_ /// The keyframe right-click menu. fn key_context_menu(app: &mut EffectcraftApp, g: &mut Rw, rc: &RowCtx<'_>, prop: &Property, kh: &KeyHit<'_>) { let KeyHit { k, ks, ref kresp, .. } = *kh; - let t = g.tl.t; let actions = &mut g.tl.actions; let layer = rc.layer; - let uid = &prop.uid; + crate::widgets::context_menu(kresp, |ui| key_menu(ui, app, layer, prop, k, ks, actions)); +} - crate::widgets::context_menu(kresp, |ui| { - for (lbl, cmd, params) in [ - ("Edit Value…", "keys.set", json!({"layer": layer.id.0, "prop": uid, "time": k.time.seconds()})), - ("-", "", json!({})), - ("Copy", "keys.copy", json!({})), - ("Paste", "keys.paste", json!({})), - ("-", "", json!({})), - ("Select Equal Keyframes", "keys.selectEqual", json!({})), - ("Select Previous Keyframes", "keys.selectPrevious", json!({})), - ("Select Following Keyframes", "keys.selectFollowing", json!({})), - ("-", "", json!({})), - ("Keyframe Interpolation…", "keys.interpolation", json!({})), - ("Keyframe Velocity…", "keys.velocity", json!({})), - ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), - (if k.muted { "Unmute Keyframes" } else { "Mute Keyframes" }, "keys.setMuted", json!({"muted": !k.muted})), - ("Rove Across Time", "keys.interpolation", json!({"roving": !k.roving})), - ("-", "", json!({})), - ("Easy Ease", "keys.easyEase", json!({})), - ("Easy Ease In", "keys.easyEaseIn", json!({})), - ("Easy Ease Out", "keys.easyEaseOut", json!({})), - ("Time-Reverse Keyframes", "keys.timeReverse", json!({})), - ("-", "", json!({})), - ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), - ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), - ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), - ("Select All Keyframes", "keys.selectAll", json!({})), - ("Delete", "keys.delete", json!({})), - ] { - if lbl == "Select All Keyframes" { - // Keyframe colour labels. +/// The commands of a keyframe's right-click menu, for the Timeline and the Graph Editor alike. They act +/// on the selected keys, so a menu opened on an unselected key selects it first. A locked layer's keys +/// can still be copied and selected, but not edited. +pub(crate) fn key_menu( + ui: &mut egui::Ui, + app: &EffectcraftApp, + layer: &Layer, + prop: &Property, + k: &effectcraft_engine::keyframe::Keyframe, + selected: bool, + actions: &mut Vec<(String, serde_json::Value)>, +) { + let t = app.tokens; + let locked = layer.switches.locked; + let uid = &prop.uid; + let this_key = json!({"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}); + for (lbl, cmd, params) in [ + ("Edit Value…", "keys.set", this_key.clone()), + ("-", "", json!({})), + ("Copy", "keys.copy", json!({})), + ("Paste", "keys.paste", json!({})), + ("-", "", json!({})), + ("Select Equal Keyframes", "keys.selectEqual", json!({})), + ("Select Previous Keyframes", "keys.selectPrevious", json!({})), + ("Select Following Keyframes", "keys.selectFollowing", json!({})), + ("-", "", json!({})), + ("Keyframe Interpolation…", "keys.interpolation", json!({})), + ("Keyframe Velocity…", "keys.velocity", json!({})), + ("Toggle Hold Keyframe", "keys.toggleHold", json!({})), + (if k.muted { "Unmute Keyframes" } else { "Mute Keyframes" }, "keys.setMuted", json!({"muted": !k.muted})), + ("Rove Across Time", "keys.interpolation", json!({"roving": !k.roving})), + ("-", "", json!({})), + ("Easy Ease", "keys.easyEase", json!({})), + ("Easy Ease In", "keys.easyEaseIn", json!({})), + ("Easy Ease Out", "keys.easyEaseOut", json!({})), + ("Time-Reverse Keyframes", "keys.timeReverse", json!({})), + ("-", "", json!({})), + ("Linear", "keys.interpolation", json!({"interpolation": "linear"})), + ("Bezier", "keys.interpolation", json!({"interpolation": "bezier"})), + ("Auto Bezier", "keys.interpolation", json!({"interpolation": "autoBezier"})), + ("Select All Keyframes", "keys.selectAll", json!({})), + ("Delete", "keys.delete", json!({})), + ] { + if lbl == "Select All Keyframes" { + // Keyframe colour labels. + ui.add_enabled_ui(!locked, |ui| { ui.menu_button(crate::i18n::tr("Label"), |ui| { for (i, l) in effectcraft_engine::color::Label::ALL.into_iter().enumerate() { if widgets::label_entry(ui, l, &app.session.prefs.label_name(l), k.label as usize == i, &t).clicked() { - if !ks { - actions.push(("keys.select".into(), json!({"keys": [{"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}]}))); + if !selected { + actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); } actions.push(("keys.setLabel".into(), json!({"label": i}))); ui.close(); } } }); - } - if lbl == "-" { - ui.separator(); - continue; - } - if lbl == "Rove Across Time" && !prop.spatial { - continue; - } - if lbl == "Edit Value…" && !matches!(k.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)) { - continue; - } - if ui.button(lbl).clicked() { - if !ks { - actions.push(("keys.select".into(), json!({"keys": [{"layer": layer.id.0, "prop": uid, "time": k.time.seconds()}]}))); - } - actions.push((cmd.into(), params)); - ui.close(); - } + }); } - }); + if lbl == "-" { + ui.separator(); + continue; + } + if lbl == "Rove Across Time" && !prop.spatial { + continue; + } + if lbl == "Edit Value…" && !matches!(k.value, Value::Scalar(_) | Value::Vec2(_) | Value::Vec3(_)) { + continue; + } + let edits = !matches!(cmd, "keys.copy" | "keys.selectEqual" | "keys.selectPrevious" | "keys.selectFollowing" | "keys.selectAll"); + // An unselected key is selected before the command runs, and locked keys cannot be selected. + let unselectable = locked && !selected && cmd != "keys.selectAll"; + if ui.add_enabled(!(unselectable || locked && edits), egui::Button::new(crate::i18n::context_item(cmd, ¶ms, lbl))).clicked() { + if !selected { + actions.push(("keys.select".into(), json!({"keys": [this_key.clone()]}))); + } + actions.push((cmd.into(), params)); + ui.close(); + } + } } /// Starts a key drag (or Alt-scale). @@ -5765,7 +5784,7 @@ fn bool_editor(app: &mut EffectcraftApp, ui: &mut egui::Ui, ve: &ValueEdit<'_>, let t = app.tokens; let x = at.x; let r = Rect::from_min_size(pos2(x, at.y - 8.0), vec2(16.0, 16.0)); - if widgets::checkbox(ui, r, *b, &t, egui::Id::unique(("cb", uid))).clicked() { + if widgets::checkbox(ui, r, *b, &t, egui::Id::unique(("cb", uid))).changed() { ve.set(actions, json!(!b)); } app.auto.add(&format!("timeline.prop.{uid}.value"), r, &prop.name); diff --git a/crates/ui-egui/src/panels/timeline/tests.rs b/crates/ui-egui/src/panels/timeline/tests.rs index 9a5c89f7d..ebcbfa8b9 100644 --- a/crates/ui-egui/src/panels/timeline/tests.rs +++ b/crates/ui-egui/src/panels/timeline/tests.rs @@ -540,6 +540,94 @@ fn disk_bar_range_covers_partial_frames() { let above_limit = TMap { start: 100_001.0, ..tm }; assert_eq!(disk_bar_range(&above_limit, clip, 1.0, i64::MAX), 100_000..100_000); } + +/// The keyframe menu is one for the Timeline and the Graph Editor: its commands are drawn in the +/// interface language, and a locked layer's keys can be copied and selected but not edited. +#[test] +fn the_key_menu_is_translated_and_cannot_edit_a_locked_layers_keys() { + use egui_kittest::Harness; + use egui_kittest::kittest::{NodeT, Queryable}; + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"name": "C", "width": 64, "height": 64, "duration": 2})).unwrap(); + let id = s.execute("layer.newSolid", json!({"name": "Box", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (t, v) in [(0.0, 0), (1.0, 50)] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": t, "value": v})).unwrap(); + } + let app = EffectcraftApp::new(s); + let layer = app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().clone(); + let prop = layer.props.prop("transform/opacity").unwrap().clone(); + let key = prop.keys[0].clone(); + let enabled = |locked: bool, selected: bool, label: &str| { + let mut layer = layer.clone(); + layer.switches.locked = locked; + let mut actions = vec![]; + let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &key, selected, &mut actions)); + h.run(); + !h.get_by_label(label).accesskit_node().is_disabled() + }; + for label in [ + "Copy", + "Select Equal Keyframes", + "Select All Keyframes", + "Delete", + "Edit Value…", + "Easy Ease", + "Time-Reverse Keyframes", + "Keyframe Velocity…", + "Mute Keyframes", + ] { + assert!(enabled(false, true, label), "{label} on an unlocked layer"); + } + for label in ["Copy", "Select Equal Keyframes", "Select All Keyframes"] { + assert!(enabled(true, true, label), "{label} is still available on a locked layer"); + } + for label in ["Delete", "Edit Value…", "Easy Ease", "Time-Reverse Keyframes", "Keyframe Velocity…", "Mute Keyframes", "Linear"] { + assert!(!enabled(true, true, label), "{label} is off on a locked layer"); + assert!(!enabled(true, false, label), "{label} is off on an unselected locked key"); + } + for label in ["Copy", "Select Equal Keyframes", "Select Previous Keyframes", "Select Following Keyframes"] { + assert!(!enabled(true, false, label), "{label} requires selecting the clicked key"); + } + assert!(enabled(true, false, "Select All Keyframes")); + // The labels follow the interface language. + crate::i18n::set_current("es"); + let copy = crate::i18n::context_item("keys.copy", &json!({}), "Copy"); + assert_ne!(copy, "Copy", "the catalog translates Copy"); + assert!(enabled(false, true, ©)); + crate::i18n::set_current("en"); +} + +#[test] +fn a_key_menu_edit_selects_the_clicked_key_before_applying_the_command() { + use egui_kittest::{Harness, kittest::Queryable}; + let mut s = effectcraft_engine::Session::default(); + s.execute("comp.new", json!({"width": 64, "height": 64, "duration": 2})).unwrap(); + let id = s.execute("layer.newSolid", json!({"color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(0.0, 0), (1.0, 50)] { + s.execute("prop.addKey", json!({"layer": id, "path": "transform/opacity", "time": time, "value": value})).unwrap(); + } + let layer = s.active_comp().unwrap().layer(LayerId(id)).unwrap().clone(); + let prop = layer.props.prop("transform/opacity").unwrap().clone(); + s.execute("keys.select", json!({"keys": [{"layer": id, "prop": prop.uid, "time": 1.0}]})).unwrap(); + let mut app = EffectcraftApp::new(s); + let mut actions = vec![]; + { + let mut h = Harness::new_ui(|ui| key_menu(ui, &app, &layer, &prop, &prop.keys[0], false, &mut actions)); + h.run(); + h.get_by_label("Easy Ease").click(); + h.run(); + } + assert_eq!(actions.iter().map(|(command, _)| command.as_str()).collect::>(), ["keys.select", "keys.easyEase"]); + for (command, params) in actions { + app.session.execute(&command, params).unwrap(); + } + let keys = &app.session.active_comp().unwrap().layer(LayerId(id)).unwrap().props.find(prop.uid).unwrap().keys; + assert_ne!(keys[0], prop.keys[0], "the clicked key was eased"); + assert_eq!(keys[1], prop.keys[1], "the previously selected key was not edited"); + assert_eq!(app.session.state.selected_keys.len(), 1); + assert_eq!(app.session.state.selected_keys[0].time, prop.keys[0].time); +} + #[test] fn layer_drop_target_skips_moving_layers_beyond_the_viewport() { let mut rows: Vec = (1..=6).map(|id| Row { layer: LayerId(id), depth: 0, kind: RowKind::Layer }).collect(); diff --git a/crates/ui-egui/src/panels/tracker.rs b/crates/ui-egui/src/panels/tracker.rs index dac9f7d2b..8d50b4860 100644 --- a/crates/ui-egui/src/panels/tracker.rs +++ b/crates/ui-egui/src/panels/tracker.rs @@ -257,10 +257,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { ("scale", "Scale", settings.as_ref().is_some_and(|s| s.scale)), ] { let cr = Rect::from_min_size(pos2(xx, y + 2.0), vec2(16.0, 16.0)); - let resp = widgets::checkbox(ui, cr, on && transformish, &t, egui::Id::unique(("tracker-cb", key))); + let resp = ui.add_enabled_ui(transformish, |ui| widgets::checkbox(ui, cr, on && transformish, &t, egui::Id::unique(("tracker-cb", key)))).inner; app.auto.add(&format!("tracker.{key}"), cr, text); p.text(pos2(cr.max.x + 4.0, cr.center().y), Align2::LEFT_CENTER, text, Tokens::ui(12.0), if transformish { t.text } else { t.text_faint }); - if resp.clicked() && transformish { + if resp.changed() && transformish { run(app, &ctx, "track.setType", with(json!({ key: !on }))); } xx += 84.0; @@ -517,20 +517,20 @@ pub fn options_dialog(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) ui.add_space(8.0); ui.label(egui::RichText::new(crate::i18n::tr("Process Before Match")).font(Tokens::medium(12.5))); ui.horizontal(|ui| { - let r = ui.checkbox(&mut d.blur_on, crate::i18n::tr("Blur")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.blur_on, crate::i18n::tr("Blur")); app.auto.add("dialog.trackOptions.blurOn", r.rect, "Blur"); let r = ui.add_enabled(d.blur_on, crate::widgets::drag_value(egui::DragValue::new(&mut d.opts.blur).speed(0.1).range(0.5..=50.0).suffix(" pixels"))); app.auto.add("dialog.trackOptions.blur", r.rect, "Blur pixels"); - let r = ui.checkbox(&mut d.opts.enhance, crate::i18n::tr("Enhance")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.enhance, crate::i18n::tr("Enhance")); app.auto.add("dialog.trackOptions.enhance", r.rect, "Enhance"); }); ui.add_space(8.0); - let r = ui.checkbox(&mut d.opts.subpixel, crate::i18n::tr("Subpixel Positioning")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.subpixel, crate::i18n::tr("Subpixel Positioning")); app.auto.add("dialog.trackOptions.subpixel", r.rect, "Subpixel Positioning"); - let r = ui.checkbox(&mut d.opts.adapt_every_frame, crate::i18n::tr("Adapt Feature On Every Frame")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.adapt_every_frame, crate::i18n::tr("Adapt Feature On Every Frame")); app.auto.add("dialog.trackOptions.adaptEveryFrame", r.rect, "Adapt Feature On Every Frame"); - let r = ui.checkbox(&mut d.opts.track_shape, crate::i18n::tr("Follow Feature Rotation and Scale")); + let r = crate::widgets::labelled_checkbox(ui, &mut d.opts.track_shape, crate::i18n::tr("Follow Feature Rotation and Scale")); app.auto.add("dialog.trackOptions.trackShape", r.rect, "Follow Feature Rotation and Scale"); ui.add_space(8.0); ui.horizontal(|ui| { diff --git a/crates/ui-egui/src/panels/viewer_text.rs b/crates/ui-egui/src/panels/viewer_text.rs index ce387f6ca..d4a5c8ccf 100644 --- a/crates/ui-egui/src/panels/viewer_text.rs +++ b/crates/ui-egui/src/panels/viewer_text.rs @@ -1362,14 +1362,18 @@ fn key_event( exec(app, "layer.setText", json!({"layer": sel.layer.0, "range": [r.start, r.end], "size": ns})); } // The app-wide shortcut dispatcher is skipped while this editor holds the - // keyboard, so the Deselect All chord (Cmd/Ctrl+Shift+A or F2 by default) - // must be matched here and run through the command dispatcher like undo - // above. Matching the command's current binding, not a fixed key, keeps - // rebound shortcuts working and gives every route one final state; the - // forwarding test also refuses chords a focused editor must never surrender - // (see `forwardable_during_text_edit`). - _ if crate::menus::chord_matches(&app.session, "edit.deselectAll", key, m) && crate::menus::forwardable_during_text_edit(m, key) => { - let _ = crate::menus::invoke(app, ctx, "edit.deselectAll", json!({})); + // keyboard, so commands that end the edit by moving the selection — Deselect + // All (Cmd/Ctrl+Shift+A or F2 by default) and Duplicate (Cmd/Ctrl+D) — must be + // matched here and run through the command dispatcher like undo above. + // Matching each command's current binding, not a fixed key, keeps rebound + // shortcuts working and gives every route one final state; a chord bound to + // both commands resolves in list order. The forwarding test also refuses + // chords a focused editor must never surrender (see + // `forwardable_during_text_edit`). + _ if crate::menus::forwardable_during_text_edit(m, key) + && let Some(cmd_id) = ["edit.deselectAll", "edit.duplicate"].into_iter().find(|id| crate::menus::chord_matches(&app.session, id, key, m)) => + { + let _ = crate::menus::invoke(app, ctx, cmd_id, json!({})); } _ => {} } diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 75010b78d..6b7a2720b 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -415,9 +415,173 @@ pub fn text_button(ui: &mut Ui, rect: Rect, text: &str, primary: bool, t: &Token resp } -/// Checkbox in the AE look (small square). +/// A primary-button gesture shared by custom and labelled checkboxes in this context. +#[derive(Clone, Default)] +struct CheckboxDrag { + press_frame: u64, + target: Option, + visited: std::collections::HashSet, + /// Checkboxes the pointer crossed in the frame of the press, before the checkbox it pressed on + /// had been drawn (so the value to set was not known): they take it in the next frame. + pending: std::collections::HashSet, +} + +/// A primary-button event of the frame, in the order it happened. +#[derive(Clone, Copy, PartialEq)] +enum Step { + Press, + Move, + Release, +} + +/// The frame's primary-button events in order, each with its position. egui hit-tests the whole +/// frame at the last pointer position, so a press followed by a move in one frame would otherwise +/// be taken for a press where the pointer ended. +fn pointer_steps(ui: &Ui) -> Vec<(Step, egui::Pos2)> { + ui.input(|i| { + i.events + .iter() + .filter_map(|e| match e { + egui::Event::PointerMoved(pos) => Some((Step::Move, *pos)), + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, .. } => { + Some((if *pressed { Step::Press } else { Step::Release }, *pos)) + } + _ => None, + }) + .collect() + }) +} + +/// Returns the requested value. Mixed checkboxes (`None`) and disabled controls are +/// skipped. Each checkbox is visited once, so crossing it again cannot toggle it back. The +/// checkbox a press lands on, and the ones the pointer passes over after it, are found from the +/// frame's ordered pointer events, not from where the pointer ended. +fn drag_toggle(ui: &Ui, response: &Response, value: Option) -> Option { + let (pressed, down, released) = ui.input(|i| (i.pointer.primary_pressed(), i.pointer.primary_down(), i.pointer.primary_released())); + let frame = ui.ctx().cumulative_frame_nr(); + // clicked() reads Context input; do not call it while Context data is locked. + let clicked = response.clicked(); + // Whether the pointer at `pos` is over this checkbox itself (not a window above it). + let over = |pos: egui::Pos2| response.rect.contains(pos) && ui.clip_rect().contains(pos) && ui.ctx().layer_id_at(pos) == Some(response.layer_id); + // Resolved before the data lock below: layer_id_at reads the Context. + let steps: Vec<(Step, bool)> = pointer_steps(ui).into_iter().map(|(step, pos)| (step, over(pos))).collect(); + ui.data_mut(|data| { + let drag = data.get_temp_mut_or_default::(egui::Id::unique("checkbox-drag")); + // A release in the press frame leaves crossings pending: keep them for the next frame. + let flushing = !drag.pending.is_empty() && drag.target.is_some() && frame <= drag.press_frame.saturating_add(1); + if (!down && !released && !flushing) || (pressed && drag.press_frame != frame) { + *drag = CheckboxDrag { press_frame: frame, ..Default::default() }; + } + let on = value?; + if !response.enabled() { + return None; + } + // Crossed in the frame of the press before the value was known: take it now. + if drag.pending.remove(&response.id) + && let Some(target) = drag.target + && drag.visited.insert(response.id) + && on != target + { + return Some(target); + } + // Whether the button is held, tracked in event order: a move after a release in the same frame is no drag. + // Before the first button event it is held exactly when that event is a release (or, with none, when it is down). + let mut held = match steps.iter().find(|(step, _)| *step != Step::Move) { + Some((step, _)) => *step == Step::Release, + None => down, + }; + let mut press_seen = false; + for (step, over) in steps { + match step { + Step::Press => { + press_seen = true; + held = true; + if drag.target.is_none() && over { + drag.target = Some(!on); + drag.visited.insert(response.id); + return Some(!on); + } + } + Step::Release => held = false, + Step::Move if held => match drag.target { + Some(target) if over => { + if drag.visited.insert(response.id) && on != target { + return Some(target); + } + } + None if press_seen && over => { + drag.pending.insert(response.id); + } + _ => {} + }, + _ => {} + } + } + // The pointer held still over this checkbox in a later frame of the gesture. + if down + && !pressed + && response.contains_pointer() + && let Some(target) = drag.target + { + if drag.visited.insert(response.id) && on != target { + return Some(target); + } + return None; + } + // The source already changed on press. Suppress its ordinary release click. + if released && drag.target.is_some() { + return None; + } + clicked.then_some(!on) + }) +} + +/// A labelled checkbox with the same drag behavior as the custom panel checkbox. +/// Use through `ui.add`, `ui.put`, or `ui.add_enabled` like `egui::Checkbox`. +pub struct DragCheckbox<'a> { + value: &'a mut bool, + text: egui::WidgetText, +} + +impl<'a> DragCheckbox<'a> { + pub fn new(value: &'a mut bool, text: impl Into) -> Self { + Self { value, text: text.into() } + } +} + +impl egui::Widget for DragCheckbox<'_> { + fn ui(self, ui: &mut Ui) -> Response { + let before = *self.value; + let mut response = ui.add(egui::Checkbox::new(self.value, self.text)); + // egui changes on release; our shared gesture changes on press and entry. + *self.value = before; + response.flags.remove(egui::response::Flags::CHANGED); + if response.clicked() { + ui.ctx().request_repaint(); + } + if let Some(value) = drag_toggle(ui, &response, Some(before)) { + *self.value = value; + response.mark_changed(); + ui.ctx().request_repaint(); + } + response + } +} + +/// A labelled checkbox that allocates its own rectangle. +pub fn labelled_checkbox(ui: &mut Ui, value: &mut bool, text: impl Into) -> Response { + ui.add(DragCheckbox::new(value, text)) +} + +/// Checkbox in the AE look (small square). `changed()` requests a toggle, including +/// keyboard activation and entry during a drag begun on another checkbox. pub fn checkbox(ui: &mut Ui, rect: Rect, on: bool, t: &Tokens, id: egui::Id) -> Response { - let resp = ui.interact(rect, id, Sense::click()); + let mut resp = ui.interact(rect, id, Sense::click()); + let next = drag_toggle(ui, &resp, Some(on)); + if next.is_some() { + resp.mark_changed(); + } + let on = next.unwrap_or(on); let b = Rect::from_center_size(rect.center(), vec2(12.0, 12.0)); ui.painter().rect_filled(b, 2.0, if on { t.accent } else { t.field_bg }); ui.painter().rect_stroke(b, 2.0, Stroke::new(1.0, if on { t.accent } else { t.field_border }), StrokeKind::Inside); @@ -831,6 +995,197 @@ pub(crate) fn shifted(c: &[f64], d: usize, v: f64) -> Vec { mod tests { use super::*; + fn checkbox_frame(ctx: &egui::Context, values: &mut [Option; 5], events: Vec) { + let t = Tokens::for_kind(Default::default()); + let input = egui::RawInput { events, ..Default::default() }; + let mut output = ctx.run_ui(input, |ui| { + for (index, value) in values.iter_mut().enumerate() { + let rect = Rect::from_min_size(pos2(20.0, 20.0 + index as f32 * 30.0), vec2(120.0, 20.0)); + ui.add_enabled_ui(index != 3, |ui| { + if index == 4 { + if let Some(on) = value { + ui.put(rect, DragCheckbox::new(on, crate::i18n::tr("Grid"))); + } + } else if let Some(on) = value { + if checkbox(ui, rect, *on, &t, egui::Id::unique(index)).changed() { + *on = !*on; + } + } else { + let response = ui.interact(rect, egui::Id::unique(index), Sense::click()); + assert_eq!(drag_toggle(ui, &response, None), None); + } + }); + } + }); + output.textures_delta.clear(); + } + + fn checkbox_pointer(y: f32, pressed: Option) -> Vec { + let pos = pos2(28.0, y); + let mut events = vec![egui::Event::PointerMoved(pos)]; + if let Some(pressed) = pressed { + events.push(egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }); + } + events + } + + #[test] + fn checkbox_drag_sets_one_state_skips_mixed_and_disabled_and_ends_on_release() { + let ctx = egui::Context::default(); + let mut values = [Some(false), Some(true), None, Some(false), Some(false)]; + checkbox_frame(&ctx, &mut values, vec![]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(true))); + assert_eq!(values[0], Some(true)); + // An already-matching neighbour must not invert, even after revisiting. + for y in [58.0, 88.0, 118.0, 148.0, 58.0, 28.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, None)); + } + assert_eq!(values, [Some(true), Some(true), None, Some(false), Some(true)]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(true), "release must not toggle the source again"); + // Begin on the labelled wrapper and drag back into custom checkboxes. + checkbox_frame(&ctx, &mut values, checkbox_pointer(148.0, Some(true))); + assert_eq!(values[4], Some(false)); + for y in [58.0, 28.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, None)); + } + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values, [Some(false), Some(false), None, Some(false), Some(false)]); + checkbox_frame(&ctx, &mut values, checkbox_pointer(58.0, None)); + assert_eq!(values[1], Some(false), "hover after release must not paint"); + } + + fn button(y: f32, pressed: bool) -> Vec { + let pos = pos2(28.0, y); + vec![egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }] + } + + fn moved(y: f32) -> egui::Event { + egui::Event::PointerMoved(pos2(28.0, y)) + } + + /// A press and the moves after it can arrive in one frame (a quick flick): the press belongs to + /// the checkbox it landed on, not to the one the pointer ended over, and the ones passed over + /// follow the pressed one's new state, whether they are drawn before it or after it. + #[test] + fn a_press_and_a_move_in_one_frame_start_on_the_checkbox_pressed() { + // Rows are 30 apart, from y = 28; index 3 is disabled. + let (a, b) = (28.0, 58.0); + // Pressed on the first, moved to the second (drawn after it). + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(a)]; + events.extend(button(a, true)); + events.push(moved(b)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true)), "the pressed one is the source, the one passed over follows"); + let mut events = vec![moved(b)]; + events.extend(button(b, false)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true)), "release changes nothing"); + // Pressed on the second, moved to the first (drawn before it): the first takes the state in + // the next frame, while the button is still down. + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(b)]; + events.extend(button(b, true)); + events.push(moved(a)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!(values[1], Some(true), "the pressed one is the source"); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!(values[0], Some(true), "the one passed over takes the state"); + let mut events = vec![moved(a)]; + events.extend(button(a, false)); + checkbox_frame(&ctx, &mut values, events); + assert_eq!((values[0], values[1]), (Some(true), Some(true))); + // Pressed outside every checkbox, then moved onto one in the same frame: nothing starts. + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(200.0)]; + events.extend(button(200.0, true)); + events.push(moved(a)); + checkbox_frame(&ctx, &mut values, events); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!(values, [Some(false), Some(false), None, Some(false), Some(false)], "a press outside starts no gesture"); + // Pressed on the second, moved to the first (drawn before it) and released, all in one + // frame: the first still takes the state in the next frame. + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(b)]; + events.extend(button(b, true)); + events.push(moved(a)); + events.extend(button(a, false)); + checkbox_frame(&ctx, &mut values, events); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!((values[0], values[1]), (Some(true), Some(true)), "a crossing before the release still takes the state"); + } + + /// A move that arrives after the release, in the frame of the click, is the pointer travelling without the button: + /// it must not carry the gesture onto the checkbox it ends over. + #[test] + fn a_move_after_the_release_in_the_same_frame_paints_nothing() { + let (a, b) = (28.0, 58.0); + let ctx = egui::Context::default(); + let mut values = [Some(false); 5]; + values[2] = None; + checkbox_frame(&ctx, &mut values, vec![]); + let mut events = vec![moved(a)]; + events.extend(button(a, true)); + events.extend(button(a, false)); + events.push(moved(b)); + checkbox_frame(&ctx, &mut values, events); + checkbox_frame(&ctx, &mut values, vec![]); + assert_eq!((values[0], values[1]), (Some(true), Some(false)), "the clicked one changes, the one moved onto afterwards does not"); + } + + #[test] + fn checkbox_click_and_keyboard_toggle_once_and_drag_must_start_on_a_checkbox() { + let ctx = egui::Context::default(); + let mut values = [Some(false), Some(false), None, Some(false), Some(false)]; + checkbox_frame(&ctx, &mut values, vec![]); + // Pressing outside the checkboxes must not start a gesture on entry. + checkbox_frame(&ctx, &mut values, checkbox_pointer(200.0, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, None)); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(false)); + for source_y in [88.0, 118.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(source_y, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, None)); + checkbox_frame(&ctx, &mut values, checkbox_pointer(28.0, Some(false))); + assert_eq!(values[0], Some(false), "mixed and disabled sources cannot begin a drag"); + } + for y in [28.0, 148.0] { + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, Some(true))); + checkbox_frame(&ctx, &mut values, checkbox_pointer(y, Some(false))); + } + assert_eq!(values[0], Some(true)); + assert_eq!(values[4], Some(true)); + ctx.memory_mut(|m| m.request_focus(egui::Id::unique(0usize))); + checkbox_frame( + &ctx, + &mut values, + vec![egui::Event::Key { key: egui::Key::Space, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }], + ); + assert_eq!(values[0], Some(false), "keyboard activation still works"); + // Fast clicks may deliver press and release together in one input frame. + // This also exercises clicked() without an active drag (outside the data lock). + for (index, y) in [(0, 28.0), (4, 148.0)] { + let before = values[index]; + let mut events = checkbox_pointer(y, Some(true)); + events.extend(checkbox_pointer(y, Some(false))); + checkbox_frame(&ctx, &mut values, events); + assert_eq!(values[index], before.map(|on| !on)); + } + } + /// #2097: the drag speed comes from the slider range, not a much wider hard range. #[test] fn slider_scrub_speed_uses_the_slider_range() { diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 144b24997..7624aeac8 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -15,6 +15,7 @@ mod ui_blank_context_menus; mod ui_camera_tracker; mod ui_character_eyedropper; mod ui_character_stroke_order; +mod ui_checkbox_drag; mod ui_color_hex; mod ui_comp_presets; mod ui_control_modifiers; diff --git a/crates/ui-egui/tests/it/ui_checkbox_drag.rs b/crates/ui-egui/tests/it/ui_checkbox_drag.rs new file mode 100644 index 000000000..ba65e54dc --- /dev/null +++ b/crates/ui-egui/tests/it/ui_checkbox_drag.rs @@ -0,0 +1,54 @@ +//! #1190: the shared drag gesture also reaches real dialog controls. +use effectcraft_engine::Session; +use effectcraft_ui_egui::{EffectcraftApp, menus}; +use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; +use egui_kittest::{Harness, kittest::Queryable as _}; +use serde_json::json; + +fn pointer(h: &mut Harness<'_, EffectcraftApp>, at: Pos2, pressed: Option) { + h.event(Event::PointerMoved(at)); + h.step(); + if let Some(pressed) = pressed { + h.event(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(2); +} + +#[test] +fn view_options_drag_checks_and_unchecks_neighbours() { + let mut session = Session::default(); + session.prefs.disk.disk_cache_enabled = false; + session.execute("comp.new", json!({"width":32,"height":32,"duration":1})).unwrap(); + let mut h = Harness::builder().with_size(vec2(1200.0, 900.0)).build_eframe(|_| EffectcraftApp::new(session)); + h.state_mut().ui.viewer.rulers = false; + h.state_mut().ui.viewer.guides = true; + h.state_mut().ui.viewer.grid = false; + h.run_steps(3); + let ctx = h.ctx.clone(); + menus::invoke(h.state_mut(), &ctx, "view.options", json!({})).unwrap(); + h.run_steps(3); + let rulers = h.get_by_label("Rulers").rect().center(); + let guides = h.get_by_label("Guides").rect().center(); + let grid = h.get_by_label("Grid").rect().center(); + pointer(&mut h, rulers, Some(true)); + assert!(h.state().ui.viewer.rulers); + pointer(&mut h, guides, None); + assert!(h.state().ui.viewer.guides, "an already-checked neighbour stays checked"); + pointer(&mut h, grid, None); + assert!(h.state().ui.viewer.grid); + pointer(&mut h, rulers, None); + pointer(&mut h, rulers, Some(false)); + assert!(h.state().ui.viewer.rulers, "revisiting and releasing does not undo the change"); + if let Ok(path) = std::env::var("CHECKBOX_REVIEW_PNG") { + h.render().unwrap().save(path).unwrap(); + } + pointer(&mut h, grid, Some(true)); + pointer(&mut h, guides, None); + pointer(&mut h, rulers, None); + pointer(&mut h, rulers, Some(false)); + let viewer = &h.state().ui.viewer; + assert!(!viewer.rulers && !viewer.guides && !viewer.grid); + pointer(&mut h, guides, None); + assert!(!h.state().ui.viewer.guides, "hover after release does not change a checkbox"); +} diff --git a/crates/ui-egui/tests/it/ui_graph_fit.rs b/crates/ui-egui/tests/it/ui_graph_fit.rs index c161dfef6..d80ae315e 100644 --- a/crates/ui-egui/tests/it/ui_graph_fit.rs +++ b/crates/ui-egui/tests/it/ui_graph_fit.rs @@ -77,3 +77,75 @@ fn open_graph_toggle_twice(h: &mut Harness<'_, EffectcraftApp>) { assert!(!h.state().ui.timeline.graph_editor); open_graph(h); } + +#[test] +fn locked_graph_keys_preserve_the_existing_selection_and_keyframes() { + use egui::{Event, Modifiers, PointerButton, pos2, vec2}; + for (button, modifiers, drag) in [ + (PointerButton::Primary, Modifiers::NONE, false), + (PointerButton::Primary, Modifiers::SHIFT, false), + (PointerButton::Primary, Modifiers::COMMAND, false), + (PointerButton::Secondary, Modifiers::NONE, false), + (PointerButton::Primary, Modifiers::NONE, true), + ] { + let (mut h, layer, uid) = harness(true); + let s = &mut h.state_mut().session; + let locked = s.execute("layer.newSolid", json!({"name": "Locked", "color": "#fff"})).unwrap()["layer"].as_u64().unwrap(); + for (time, value) in [(9.0, 100.0), (11.0, 200.0)] { + s.execute("prop.addKey", json!({"layer": locked, "path": "transform/rotation", "time": time, "value": value})).unwrap(); + } + let locked_id = effectcraft_engine::project::LayerId(locked); + let locked_uid = s.active_comp().unwrap().layer(locked_id).unwrap().props.prop("transform/rotation").unwrap().uid; + s.execute("layer.setSwitch", json!({"layers": [locked], "switch": "locked", "value": true})).unwrap(); + s.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap(); + s.state.selected_props = vec![(effectcraft_engine::project::LayerId(layer), uid), (locked_id, locked_uid)]; + let selection = s.state.selected_keys.clone(); + let before = s.project.clone(); + open_graph(&mut h); + h.state_mut().ui.timeline.graph_auto_zoom = false; + h.state_mut().ui.timeline.graph_range = Some((0.0, 800.0)); + h.run_steps(3); + let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{locked_uid}.0.0")).expect("locked key drawn").rect; + let point = pos2(x + w / 2.0, y + height / 2.0); + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(Event::PointerMoved(point)); + h.input_mut().events.push(Event::PointerButton { pos: point, button, pressed: true, modifiers }); + h.step(); + let end = if drag { point + vec2(30.0, -20.0) } else { point }; + h.input_mut().events.push(Event::PointerMoved(end)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: end, button, pressed: false, modifiers }); + h.run_steps(3); + assert_eq!(h.state().session.state.selected_keys, selection, "{button:?}, {modifiers:?}, drag={drag}"); + assert_eq!( + serde_json::to_value(&h.state().session.project).unwrap(), + serde_json::to_value(&before).unwrap(), + "locked gestures must not edit another key" + ); + } +} + +#[test] +fn an_unlocked_graph_keys_menu_edits_the_clicked_key() { + use egui::{Event, Modifiers, PointerButton, pos2}; + use egui_kittest::kittest::Queryable; + let (mut h, layer, uid) = harness(true); + h.state_mut().session.execute("keys.select", json!({"keys": [{"layer": layer, "prop": uid, "time": 12.0}]})).unwrap(); + open_graph(&mut h); + let [x, y, w, height] = h.state().auto.find(&format!("timeline.graph.key.{uid}.0.0")).unwrap().rect; + let at = pos2(x + w / 2.0, y + height / 2.0); + for pressed in [true, false] { + h.input_mut().events.push(Event::PointerMoved(at)); + h.input_mut().events.push(Event::PointerButton { pos: at, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); + h.step(); + } + h.run_steps(3); + let selected = &h.state().session.state.selected_keys; + assert_eq!(selected.len(), 1); + assert_eq!(selected[0].time.seconds(), 8.0); + h.get_by_label("Easy Ease").click(); + h.run_steps(3); + let prop = h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(uid).unwrap(); + assert_eq!(prop.keys[0].out_interp, effectcraft_engine::keyframe::Interp::Bezier); + assert_eq!(prop.keys[1].out_interp, effectcraft_engine::keyframe::Interp::Linear); +} diff --git a/crates/ui-egui/tests/it/ui_text_edit.rs b/crates/ui-egui/tests/it/ui_text_edit.rs index ea4b56d26..1573dd135 100644 --- a/crates/ui-egui/tests/it/ui_text_edit.rs +++ b/crates/ui-egui/tests/it/ui_text_edit.rs @@ -580,7 +580,7 @@ fn type_tool_selected_text_preference_skips_ineligible_layers_and_falls_back_to_ } /// Opens a fresh Type-tool edit holding `text` and returns the harness with the new layer id, -/// so the Deselect All tests start from the same state. +/// so the Deselect All and Duplicate tests start from the same state. fn type_tool_edit(a: EffectcraftApp, text: &str) -> (Harness<'static, EffectcraftApp>, u64) { let mut h = harness(a); h.run_steps(3); @@ -728,6 +728,110 @@ fn deselect_all_menu_item_follows_the_edit_while_the_project_panel_is_focused() assert!(!enabled(&h), "Deselect All follows the empty Project selection once the edit is over"); } +/// Layer ids of the active comp, bottom to top. +fn layer_ids(h: &Harness<'_, EffectcraftApp>) -> Vec { + h.state().session.active_comp().map_or(vec![], |c| c.layers.iter().map(|l| l.id.0).collect()) +} + +#[test] +fn duplicate_chord_ends_a_type_tool_edit_and_selects_the_copy() { + let (mut h, lid) = type_tool_edit(app(), "Hello"); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Duplicate ends the open edit, as Deselect All does"); + let copy = *s.state.selected_layers.first().expect("the copy is selected"); + assert_ne!(copy.0, lid, "the copy (not the edited original) holds the selection"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "the original keeps its text"); + assert_eq!(layer_doc(s, copy).unwrap().text, "Hello", "the copy carries it too"); +} + +#[test] +fn duplicate_chord_ends_an_edit_started_while_the_project_panel_is_focused() { + let (mut h, lid) = project_focused_text_edit("Hi"); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Cmd+D ends the edit even with Project focused"); + let copy = s.state.selected_layers.first().expect("the copy is selected"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hi", "the typed text is kept"); + assert_eq!(layer_doc(s, *copy).unwrap().text, "Hi", "on the copy as well"); +} + +#[test] +fn rebound_duplicate_chord_forwards_from_inside_the_edit() { + let mut a = app(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["Cmd+Alt+D"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + // The default key no longer belongs to Duplicate: while the editor holds the keyboard a + // chord that matches no command does nothing, and the current binding is what forwards. + let n = layer_ids(&h).len(); + key(&mut h, Key::D, Modifiers::COMMAND); + assert!(h.state().session.state.text_edit.is_some(), "Cmd+D was rebound away: the edit stays open"); + assert_eq!(layer_ids(&h).len(), n, "and nothing was duplicated"); + key(&mut h, Key::D, Modifiers { alt: true, ..Modifiers::COMMAND }); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "the rebound chord ends the edit"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "the text is kept"); + assert_eq!(layer_ids(&h).len(), n + 1, "and the copy exists"); +} + +#[test] +fn duplicate_rebound_onto_a_bare_letter_stays_typing() { + let mut a = app(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["D"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + let n = layer_ids(&h).len(); + // A binding without a command modifier is text as far as the editor is concerned: the editor + // must not surrender a key that types (same invariant as the app-wide dispatcher). + key(&mut h, Key::D, Modifiers::NONE); + assert!(h.state().session.state.text_edit.is_some(), "a bare-letter Duplicate binding does not end the edit"); + assert_eq!(layer_ids(&h).len(), n, "and does not duplicate"); + type_text(&mut h, "D"); + let (lid2, text, _, _) = edited(&h); + assert_eq!(lid2, lid, "the same edit on the same layer continues"); + assert_eq!(text, "HelloD", "and the letter typed into the text"); +} + +#[test] +fn duplicate_chord_copies_the_edited_layer_not_an_effect_still_selected() { + let mut a = app(); + let lid = a.session.execute("layer.newText", json!({"text": "alpha beta", "size": 40, "box": [40, 60, 250, 100]})).unwrap()["layer"].as_u64().unwrap(); + // Applying the blur selects it, as clicking it in Effect Controls does. + a.session.execute("effect.apply", json!({"layers": [lid], "effect": "Gaussian Blur"})).unwrap(); + a.ui.tool = effectcraft_ui_egui::state::Tool::Type; + let mut h = harness(a); + h.run_steps(3); + let comp = rect(&h, "viewer.comp"); + click(&mut h, comp.min + egui::vec2(80.0, 85.0) * (comp.width() / 640.0), Modifiers::NONE); + assert_eq!(edited(&h).0, lid, "the Type tool's click opens the text layer for editing"); + assert!(!h.state().session.state.selected_props.is_empty(), "and the blur stays selected"); + let n = layer_ids(&h).len(); + key(&mut h, Key::D, Modifiers::COMMAND); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "Duplicate ends the edit"); + assert_eq!(layer_ids(&h).len(), n + 1, "and copies the edited layer"); + let blurs = s.active_comp().unwrap().layer(LayerId(lid)).unwrap().effects().map_or(0, |fx| fx.groups().count()); + assert_eq!(blurs, 1, "not the blur"); +} + +#[test] +fn a_chord_bound_to_both_forwarded_commands_resolves_in_list_order() { + let mut a = app(); + // Shortcuts may hold one chord for several commands (the conflict is a toast, not a + // refusal), so the editor's forwarding resolves a shared chord in list order — Deselect + // All before Duplicate. This pins that documented contract: the shared chord must end the + // edit like Deselect All, without running Duplicate behind it. + a.session.execute("shortcuts.set", json!({"command": "edit.deselectAll", "keys": ["Cmd+Alt+G"]})).unwrap(); + a.session.execute("shortcuts.set", json!({"command": "edit.duplicate", "keys": ["Cmd+Alt+G"]})).unwrap(); + let (mut h, lid) = type_tool_edit(a, "Hello"); + let n = layer_ids(&h).len(); + key(&mut h, Key::G, Modifiers { alt: true, ..Modifiers::COMMAND }); + let s = &h.state().session; + assert!(s.state.text_edit.is_none(), "the shared chord still ends the edit"); + assert!(s.state.selected_layers.is_empty(), "Deselect All wins the shared chord: nothing is selected"); + assert_eq!(layer_ids(&h).len(), n, "so the chord did not duplicate"); + assert_eq!(layer_doc(s, LayerId(lid)).unwrap().text, "Hello", "and the text is kept"); +} + /// The on-canvas editor's outgoing IME producer: the descriptor carries the edited layer's /// committed text, the real owner, a stable generation and an explicit change cause. A normal /// typed character is application work; an accepted input-method commit is not. diff --git a/docs/mcp.md b/docs/mcp.md index 7685a66e2..686adcb35 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -12,8 +12,8 @@ written only under a write root; with no write root nothing is written. Every en parameters must be absolute paths inside the roots (symbolic links resolved, no `..`), and so must any other absolute path in its parameters. Save, Increment and Save and rendering the Render Queue are checked against the files they write (the open project's, the queued outputs); -`renderQueue.add` without `output` needs a saved project, since an unsaved one's default output -would go to the home folder (#2479). Scripts +`renderQueue.add`, `render.preRender` and `file.createProxy` without an output file need a saved +project, since an unsaved one's default output would go to the home folder (#2479). Scripts (`run_script`, `file.runScript`…), Execute File, Reveal in Finder, `$N` references to file paths in a batch, and the bridge's UI-driving tools (`ui_click`, `ui_key`, `ui_type`, `ui_drag`, `ui_set`, `script_ui`, `control`) are refused while confined. Without the flags nothing changes.