From 0cb5bfefb5874247898751387f515c8ce74ffd69 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 15:06:13 +0100 Subject: [PATCH 1/4] Number fields: typed arithmetic (960/2, (3+4)*2) and a leading *, / or + applied to the value (#3195) Number fields read + - * /, brackets and signs through a small bounded parser (crates/ui-egui/src/num_math.rs): no code is run, length and nesting are capped, every step must stay finite and a division by zero keeps the old value with no undo step. Hot-text values (Timeline, Effect Controls, Properties and the other panels) also take a leading *, / or + applied to the value shown, as the reporter describes for After Effects (not confirmed in public docs). A leading - stays a negative number. Hot text used to read +50 as 50; it now adds 50. On an angle the step goes from the signed remainder (+10 on 0x-30 is 0x-20). With several layers selected each layer takes the step from its own value: + through the offset path, * and / through a new prop.set factor. Typed results in whole-number hot text round instead of truncating. Dialog fields (the shared drag_value wrapper, Essential Graphics, Gradient Editor, colour picker) re-read their text per keystroke, so they take absolute arithmetic only. Fixes #3195 --- crates/engine/src/cmd/prop.rs | 29 +- crates/engine/src/tests/timeline.rs | 23 + crates/ui-egui/src/color_picker.rs | 7 +- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/num_math.rs | 394 ++++++++++++++++++ .../src/panels/effect_controls/rows.rs | 9 +- crates/ui-egui/src/panels/essential.rs | 8 +- crates/ui-egui/src/panels/fx_widgets.rs | 32 +- crates/ui-egui/src/panels/gradient_editor.rs | 18 +- crates/ui-egui/src/panels/properties.rs | 9 +- crates/ui-egui/src/panels/timeline.rs | 9 +- crates/ui-egui/src/widgets.rs | 62 ++- crates/ui-egui/tests/it/ui_angles.rs | 15 + crates/ui-egui/tests/it/ui_inline_edit.rs | 78 ++++ 14 files changed, 658 insertions(+), 36 deletions(-) create mode 100644 crates/ui-egui/src/num_math.rs diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index b4432dd2f..dc043d78f 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -371,20 +371,35 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { }; let follow: Vec = p.get("follow").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_u64).filter_map(|d| usize::try_from(d).ok()).collect(); + // A typed `*2` or `/2` (#3195): each layer's own value times it, not the edited layer's result. + let factor = p.get("factor").and_then(Value::as_f64).filter(|k| k.is_finite() && delta.is_none()); // Each layer's own value where `set` writes it (a `time` other than the CTI keys there). let typed = |s: &Session, q: &mut Value, olid: LayerId, ouid: Uid| { - if let (Some(dims), Some(n)) = (&dims, &new) - && let Some(mut c) = components_at(s, cid, olid, ouid, snapped_key_time(s, cid, olid, q, "prop.set").ok().flatten()) + let at = snapped_key_time(s, cid, olid, q, "prop.set").ok().flatten(); + if let (true, Some(k)) = (scalar, factor) + && let Some(own) = components_at(s, cid, olid, ouid, at).and_then(|c| c.first().copied()) + && (own * k).is_finite() { - let change = dims.first().and_then(|&d| Some(n.get(d)? - c.get(d)?)).unwrap_or(0.0); + q["value"] = json!(own * k); + } + if let (Some(dims), Some(n)) = (&dims, &new) + && let Some(mut c) = components_at(s, cid, olid, ouid, at) + { + // The typed components' new values on this layer. + let typed_value = |c: &[f64], d: usize| match factor { + Some(k) => c.get(d).map(|v| v * k).filter(|v| v.is_finite()), + None => n.get(d).copied(), + }; + let change = dims.first().and_then(|&d| Some(typed_value(&c, d)? - c.get(d)?)).unwrap_or(0.0); + let values: Vec> = dims.iter().map(|&d| typed_value(&c, d)).collect(); for &d in &follow { if let Some(slot) = c.get_mut(d) { *slot += change; } } - for &d in dims { - if let (Some(slot), Some(v)) = (c.get_mut(d), n.get(d)) { - *slot = *v; + for (&d, v) in dims.iter().zip(values) { + if let (Some(slot), Some(v)) = (c.get_mut(d), v) { + *slot = v; } } q["value"] = json!(c); @@ -1421,7 +1436,7 @@ pub fn specs() -> Vec { "Set Property Value", [], None, - "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all), dims? (with selected, a typed value: the components edited, e.g. [2] for Z; the other layers keep the rest of their own value), follow? (with dims: components that move on each other layer by the change its first typed component takes)}", + "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all), dims? (with selected, a typed value: the components edited, e.g. [2] for Z; the other layers keep the rest of their own value), follow? (with dims: components that move on each other layer by the change its first typed component takes), factor? (with selected, a typed `*` or `/`: the other layers take their own value times it, in the dims components of a vector)}", has_layers, set_selected ), diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index 55475eef3..6ea4d1336 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -1531,6 +1531,29 @@ fn a_typed_component_applies_only_that_component_to_the_selected_layers() { assert_eq!(pos(&s, c), vec![1.0, 2.0, 3.0]); } +/// #3195: a typed `*2` or `/2` with several layers selected scales each layer's own value (in +/// the typed components of a vector), not the edited layer's result. +#[test] +fn a_typed_factor_scales_each_selected_layers_own_value() { + let (mut s, a) = setup(); + let b = s.execute("layer.newSolid", json!({"color": "#ff0000", "width": 50, "height": 50})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "transform/opacity", "value": 40.0})).unwrap(); + s.execute("prop.set", json!({"layer": b, "path": "transform/opacity", "value": 30.0})).unwrap(); + s.execute("prop.set", json!({"layer": a, "path": "transform/position", "value": [10.0, 20.0]})).unwrap(); + s.execute("prop.set", json!({"layer": b, "path": "transform/position", "value": [100.0, 50.0]})).unwrap(); + s.execute("layer.select", json!({"layers": [a, b]})).unwrap(); + let opacity = prop(&s, a, "transform/opacity").uid; + let undo = s.history.undo.len(); + s.execute_checked("prop.set", json!({"layer": a, "prop": opacity, "value": 80.0, "selected": true, "factor": 2.0})).unwrap(); + assert_eq!(prop(&s, a, "transform/opacity").value.as_f64(), 80.0); + assert_eq!(prop(&s, b, "transform/opacity").value.as_f64(), 60.0, "B doubles its own 30"); + assert_eq!(s.history.undo.len(), undo + 1, "one undo step"); + let position = prop(&s, a, "transform/position").uid; + s.execute_checked("prop.set", json!({"layer": a, "prop": position, "value": [5.0, 20.0], "selected": true, "dims": [0], "factor": 0.5})).unwrap(); + assert_eq!(prop(&s, a, "transform/position").value.components()[..2], [5.0, 20.0]); + assert_eq!(prop(&s, b, "transform/position").value.components()[..2], [50.0, 50.0], "B halves its own X, keeps its Y"); +} + /// #2485: linked Audio Levels typed with several layers selected: each takes the typed Left, and /// its Right moves by its own change, so every layer keeps its own stereo balance. #[test] diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index d434538e6..2b9982cec 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -447,7 +447,12 @@ fn color_picker(ui: &mut Ui, id: egui::Id, c: &mut [f32; 3], original: [f32; 3]) if ch == Chan::H && v >= 360.0 { v = 0.0; } - let drag = egui::DragValue::new(&mut v).range(0.0..=scale).fixed_decimals(0).suffix(suffix).speed(scale / 255.0); + let drag = egui::DragValue::new(&mut v) + .range(0.0..=scale) + .fixed_decimals(0) + .suffix(suffix) + .speed(scale / 255.0) + .custom_parser(|text| crate::num_math::calc(text, None)); if ui.add_sized(vec2(56.0, 18.0), drag).labelled_by(radio.id).changed() { pick = pick.set(ch, v.round() / scale); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 795a01e1e..0dc02f7d1 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -36,6 +36,7 @@ pub mod icons; pub mod menu_keys; pub mod menus; pub mod native_menu; +pub mod num_math; pub mod panels; mod pointer_wrap; pub mod prefs_live; diff --git a/crates/ui-egui/src/num_math.rs b/crates/ui-egui/src/num_math.rs new file mode 100644 index 000000000..3e329c9e4 --- /dev/null +++ b/crates/ui-egui/src/num_math.rs @@ -0,0 +1,394 @@ +//! Basic arithmetic typed into a number field (#3195): `960/2`, `(3+4)*2`, `100*2`, and, in a +//! field that knows the value it showed, a leading operator that applies to it: `*2` doubles, +//! `/2` halves, `+50` adds 50, as the issue's reporter describes for After Effects (not confirmed +//! in public docs). A leading `-` is a negative number (`-50`), not a subtraction. (Hot text used +//! to read `+50` as 50; it now adds 50.) +//! +//! This is a small parser for numbers and `+ - * / ( )` only: no names, no functions, nothing is +//! run as code (expressions, `crates/expr`, are a different thing). The text is bounded in +//! length and nesting, every step must stay finite, and a division by zero is no value. Text it +//! can't read is `None`, so the field keeps its value. + +/// The longest text read as arithmetic (characters, after removing spaces). +const MAX_LEN: usize = 256; +/// How deeply brackets and signs (`-(-(…))`, `--5`) may nest. +const MAX_DEPTH: usize = 32; + +/// A typed leading operator: what it does to the value the field showed. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum Step { + /// `+n` + Add(f64), + /// `*n` + Mul(f64), + /// `/n` (never zero) + Div(f64), +} + +impl Step { + /// `current` changed by the step; `None` when that isn't a finite number. + pub fn apply(self, current: f64) -> Option { + match self { + Step::Add(n) => finite(current + n), + Step::Mul(k) => finite(current * k), + Step::Div(n) => divide(current, n), + } + } + + /// The factor a `*` or `/` step multiplies by (`None` for `+`), for other layers that scale + /// their own value by it. + pub fn factor(self) -> Option { + match self { + Step::Add(_) => None, + Step::Mul(k) => finite(k), + Step::Div(n) => divide(1.0, n), + } + } +} + +/// What typed text says: a value, or (with a leading `+`, `*` or `/`) a step from the value shown. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum Typed { + Value(f64), + Step(Step), +} + +/// Read typed `text`. With `steps`, a leading `+`, `*` or `/` is a [`Step`]; without (a field +/// that reads its text on every keystroke, where a step would compound) a leading `+` is a plain +/// sign and a leading `*` or `/` is no value. Spaces, including thousands groups, are ignored +/// (as egui's own parser did) and `−` (U+2212) is a minus. `,` is a decimal point: hot text +/// always read it so, and the dialog fields, whose egui parser rejected it, now do too (so +/// `1,5` is 1.5 and `1,000` is 1, not a thousand). `None` for anything else that isn't a finite +/// number. +pub fn read(text: &str, steps: bool) -> Option { + let s: String = text + .chars() + .filter(|c| !c.is_whitespace()) + .map(|c| match c { + '−' => '-', + ',' => '.', + c => c, + }) + .collect(); + if s.is_empty() || s.chars().count() > MAX_LEN { + return None; + } + let first = s.chars().next()?; + if steps && matches!(first, '+' | '*' | '/') { + let operand = parse(s.get(first.len_utf8()..)?)?; + return Some(Typed::Step(match first { + '+' => Step::Add(operand), + '*' => Step::Mul(operand), + _ => Step::Div((operand != 0.0).then_some(operand)?), + })); + } + parse(&s).map(Typed::Value) +} + +/// The value typed `text` stands for: [`read`], with a step applied to `current` (the value the +/// field showed; without it a leading `*` or `/` is no value and `+` only a sign). +pub fn calc(text: &str, current: Option) -> Option { + match read(text, current.is_some())? { + Typed::Value(v) => Some(v), + Typed::Step(step) => step.apply(current.filter(|c| c.is_finite())?), + } +} + +/// The whole of `s` as an expression. +fn parse(s: &str) -> Option { + let mut p = Parser { chars: s.chars().peekable(), depth: 0 }; + let v = p.expr()?; + if p.chars.next().is_some() { + return None; + } + finite(v) +} + +/// `a / b`; `None` for a division by zero. +fn divide(a: f64, b: f64) -> Option { + if b == 0.0 { + return None; + } + finite(a / b) +} + +/// A finite result, or `None`. +fn finite(v: f64) -> Option { + v.is_finite().then_some(v) +} + +/// A recursive-descent reader of `expr := term (('+'|'-') term)*`, +/// `term := factor (('*'|'/') factor)*`, `factor := ('+'|'-') factor | '(' expr ')' | number`. +struct Parser<'a> { + chars: std::iter::Peekable>, + /// The brackets and signs open around the current point. + depth: usize, +} + +impl Parser<'_> { + fn expr(&mut self) -> Option { + let mut v = self.term()?; + while let Some(&op) = self.chars.peek() { + match op { + '+' => { + self.chars.next(); + v = finite(v + self.term()?)?; + } + '-' => { + self.chars.next(); + v = finite(v - self.term()?)?; + } + _ => break, + } + } + Some(v) + } + + fn term(&mut self) -> Option { + let mut v = self.factor()?; + while let Some(&op) = self.chars.peek() { + match op { + '*' => { + self.chars.next(); + v = finite(v * self.factor()?)?; + } + '/' => { + self.chars.next(); + v = divide(v, self.factor()?)?; + } + _ => break, + } + } + Some(v) + } + + fn factor(&mut self) -> Option { + let c = *self.chars.peek()?; + match c { + '+' | '-' | '(' => { + self.depth = self.depth.checked_add(1)?; + if self.depth > MAX_DEPTH { + return None; + } + self.chars.next(); + let v = if c == '(' { + let v = self.expr()?; + if self.chars.next() != Some(')') { + return None; + } + v + } else { + let v = self.factor()?; + if c == '-' { -v } else { v } + }; + self.depth = self.depth.saturating_sub(1); + Some(v) + } + _ => self.number(), + } + } + + /// Digits with at most one decimal point and an optional exponent (`12`, `1.5`, `.5`, `2.`, + /// `2e2`, `1.5E-3`). + fn number(&mut self) -> Option { + let mut text = String::new(); + let mut point = false; + while let Some(&c) = self.chars.peek() { + if c.is_ascii_digit() { + text.push(c); + } else if c == '.' && !point { + point = true; + text.push(c); + } else { + break; + } + self.chars.next(); + } + if !text.chars().any(|c| c.is_ascii_digit()) { + return None; + } + if let Some(&e) = self.chars.peek() + && matches!(e, 'e' | 'E') + { + self.chars.next(); + text.push('e'); + if let Some(&sign) = self.chars.peek() + && matches!(sign, '+' | '-') + { + self.chars.next(); + text.push(sign); + } + let mut digits = false; + while let Some(&c) = self.chars.peek() { + if !c.is_ascii_digit() { + break; + } + digits = true; + text.push(c); + self.chars.next(); + } + if !digits { + return None; + } + } + finite(text.parse::().ok()?) + } +} + +#[cfg(test)] +mod tests { + use super::calc as eval; + + fn abs(text: &str) -> Option { + eval(text, None) + } + + #[test] + fn plain_numbers_read_as_before() { + assert_eq!(abs("100"), Some(100.0)); + assert_eq!(abs("-12.5"), Some(-12.5)); + assert_eq!(abs(" 12,5 "), Some(12.5), "a decimal comma"); + assert_eq!(abs("1 000"), Some(1000.0), "a thousands space"); + assert_eq!(abs("−3"), Some(-3.0), "the minus sign U+2212"); + assert_eq!(abs("1e3"), Some(1000.0)); + assert_eq!(abs(".5"), Some(0.5)); + } + + #[test] + fn arithmetic() { + assert_eq!(abs("960/2"), Some(480.0)); + assert_eq!(abs("100*2"), Some(200.0)); + assert_eq!(abs("(3+4)*2"), Some(14.0)); + assert_eq!(abs("3+4*2"), Some(11.0), "* before +"); + assert_eq!(abs("10-2-3"), Some(5.0), "left to right"); + assert_eq!(abs("8/4/2"), Some(1.0), "left to right"); + assert_eq!(abs("-(2+3)"), Some(-5.0)); + assert_eq!(abs("2*-3"), Some(-6.0)); + assert_eq!(abs("1.5 * 2"), Some(3.0)); + assert_eq!(abs("((((1))))"), Some(1.0)); + assert_eq!(abs("100+50"), Some(150.0)); + assert_eq!(abs("100-50"), Some(50.0)); + assert_eq!(abs("2e2*2"), Some(400.0), "an exponent inside arithmetic"); + assert_eq!(abs("1E-3*1000"), Some(1.0)); + assert_eq!(abs("1e+2/4"), Some(25.0)); + } + + #[test] + fn steps_are_read_as_steps() { + use super::{Step, Typed, read}; + assert_eq!(read("*2", true), Some(Typed::Step(Step::Mul(2.0)))); + assert_eq!(read("/4", true), Some(Typed::Step(Step::Div(4.0)))); + assert_eq!(read("+(1+2)", true), Some(Typed::Step(Step::Add(3.0)))); + assert_eq!(read("-5", true), Some(Typed::Value(-5.0))); + assert_eq!(read("+5", false), Some(Typed::Value(5.0))); + assert_eq!(read("/0", true), None); + assert_eq!(Step::Div(4.0).factor(), Some(0.25)); + assert_eq!(Step::Mul(3.0).factor(), Some(3.0)); + assert_eq!(Step::Add(3.0).factor(), None); + assert_eq!(Step::Div(1e-320).factor(), None, "a factor that overflows"); + assert_eq!(Step::Div(3.0).apply(3.0), Some(1.0), "a division, not a reciprocal factor"); + } + + #[test] + fn a_leading_operator_applies_to_the_current_value() { + assert_eq!(eval("*2", Some(100.0)), Some(200.0)); + assert_eq!(eval("/2", Some(100.0)), Some(50.0)); + assert_eq!(eval("+50", Some(100.0)), Some(150.0)); + assert_eq!(eval("*(1+1)", Some(3.0)), Some(6.0)); + assert_eq!(eval("+-50", Some(100.0)), Some(50.0)); + // A leading minus is a negative number, not a subtraction. + assert_eq!(eval("-50", Some(100.0)), Some(-50.0)); + assert_eq!(eval("-50*2", Some(100.0)), Some(-100.0)); + // Without the current value there is nothing to apply it to; a `+` is only a sign. + assert_eq!(eval("*2", None), None); + assert_eq!(eval("/2", None), None); + assert_eq!(eval("+50", None), Some(50.0)); + assert_eq!(eval("*2", Some(f64::NAN)), None); + assert_eq!(eval("+1", Some(f64::INFINITY)), None); + assert_eq!(eval("/0", Some(100.0)), None); + assert_eq!(eval("/(1-1)", Some(100.0)), None); + assert_eq!(eval("*", Some(100.0)), None); + assert_eq!(eval("*1e400", Some(1.0)), None); + // Overflow past the largest finite value. + assert_eq!(eval(&format!("*{}", "9".repeat(20)), Some(1e300)), None); + assert_eq!(eval("/0.0000000001", Some(1e300)), None); + } + + #[test] + fn invalid_or_hostile_text_is_no_value() { + for text in [ + "", " ", "*", "/", "1+", "+", "-", "2**3", "2//3", "1/0", "0/0", "-1/0", "1/(2-2)", "(1", "1)", "()", "(1+2", "abc", "NaN", "nan", "inf", "-inf", + "infinity", "1e999", "-1e999", "1..2", "1.2.3", ".", "1e", "2x", "x*2", "1+a", "5;rm", "1,2,3", "0x10", "1_000", "%", "50%", + ] { + assert_eq!(abs(text), None, "{text:?}"); + assert_eq!(eval(text, Some(10.0)), None, "{text:?} with a current value"); + } + // Far too long: rejected by length before any reading. + assert_eq!(abs(&format!("{}1{}", "(".repeat(1000), ")".repeat(1000))), None); + assert_eq!(abs(&format!("{}5", "-".repeat(10_000))), None); + assert_eq!(abs(&"(".repeat(10_000)), None); + assert_eq!(abs(&"1".repeat(10_000)), None); + assert_eq!(abs(&"1+".repeat(5_000)), None); + // Short enough, but nested past the depth bound. + assert_eq!(abs(&format!("{}1{}", "(".repeat(100), ")".repeat(100))), None); + assert_eq!(abs(&format!("{}5", "-".repeat(200))), None); + assert_eq!(abs(&format!("2*{}5", "-+".repeat(100))), None); + // Nesting within the bound reads. + assert_eq!(abs(&format!("{}1{}", "(".repeat(20), ")".repeat(20))), Some(1.0)); + assert_eq!(abs("--5"), Some(5.0)); + // Overflow on the way is no value. + assert_eq!(abs("1e308*10"), None); + assert_eq!(abs("1e308+1e308"), None); + assert_eq!(abs("1e200*1e200/1e200"), None, "an infinite step is rejected even if the result would be finite"); + // The largest numbers the length allows still multiply to a finite value. + let big = "9".repeat(120); + assert!(abs(&format!("{big}*{big}")).is_some_and(f64::is_finite)); + } + + /// The result is always finite (no NaN reaches a property). + #[test] + fn results_are_finite() { + for text in ["0*1e308", "1e-320/1e10", "0-0", "-0"] { + if let Some(v) = abs(text) { + assert!(v.is_finite(), "{text:?} → {v}"); + } + } + } + + /// A standard numeric editor (dialogs) reads typed arithmetic too. + /// (egui's own parser read `960/2` as no value, so the field stayed 10.) + #[test] + fn a_drag_value_reads_typed_arithmetic() { + use egui::{Context, Event, Key, Modifiers, PointerButton, Pos2, RawInput, Rect, pos2, vec2}; + let ctx = Context::default(); + let mut value = 10.0_f64; + let mut time = 0.0; + let field = Rect::from_min_size(pos2(100.0, 100.0), vec2(80.0, 20.0)); + let mut frame = |events: Vec, value: &mut f64| { + time += 0.02; + let raw = RawInput { + screen_rect: Some(Rect::from_min_size(Pos2::ZERO, vec2(400.0, 300.0))), + time: Some(time), + events, + focused: true, + ..Default::default() + }; + let mut out = ctx.run_ui(raw, |ui| { + ui.put(field, crate::widgets::drag_value(egui::DragValue::new(&mut *value).speed(1.0))); + }); + out.textures_delta.clear(); + }; + let p = field.center(); + frame(vec![], &mut value); + frame( + vec![Event::PointerMoved(p), Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }], + &mut value, + ); + frame(vec![Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }], &mut value); + frame(vec![], &mut value); + frame(vec![Event::Text("960/2".into())], &mut value); + frame(vec![Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }], &mut value); + frame(vec![], &mut value); + assert_eq!(value, 480.0); + } +} diff --git a/crates/ui-egui/src/panels/effect_controls/rows.rs b/crates/ui-egui/src/panels/effect_controls/rows.rs index c14630af4..e2634d6b8 100644 --- a/crates/ui-egui/src/panels/effect_controls/rows.rs +++ b/crates/ui-egui/src/panels/effect_controls/rows.rs @@ -265,11 +265,12 @@ fn value_angle(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: return; }; let PropCx { uid, cy, vx, prop, .. } = *cx; - let (rr, dr, nv) = fw::angle_field(ui, pos2(vx, cy - 9.0), egui::Id::unique(("ec-v", uid)), *v, 1, &cx.value_tokens()); + let (rr, dr, nv, step) = fw::angle_field_step(ui, pos2(vx, cy - 9.0), egui::Id::unique(("ec-v", uid)), *v, 1, &cx.value_tokens()); app.auto.add(&format!("effectControls.prop.{uid}.value"), rr.union(dr), &fw::format_angle(*v, 1)); app.auto.add(&format!("effectControls.prop.{uid}.revolutions"), rr, &prop.name); if let Some(nv) = nv { cx.set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } cx.twirl_open(app, ui); } @@ -288,7 +289,7 @@ fn value_scalar(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: _ => "", }; let speed = widgets::slider_scrub_speed(&prop.ui); - let (vr, nv, _) = widgets::hot_number_at( + let (vr, nv, _, step) = widgets::hot_number_step_at( ui, pos2(vx, cy - 9.0), egui::Id::unique(("ec-v", uid)), @@ -299,6 +300,7 @@ fn value_scalar(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: app.auto.add(&format!("effectControls.prop.{uid}.value"), vr, &prop.name); if let Some(nv) = nv { cx.set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } // AE hides a slider param's slider until its twirl is opened (see `slider_row`). if matches!(prop.ui, ParamUi::Slider { .. }) { @@ -326,7 +328,7 @@ fn value_vector(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: x += 22.0; } for d in 0..n.min(c.len()) { - let (vr, nv, _) = widgets::hot_number_at( + let (vr, nv, _, step) = widgets::hot_number_step_at( ui, pos2(x, cy - 9.0), egui::Id::unique(("ec-v", uid, d)), @@ -345,6 +347,7 @@ fn value_vector(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: if let Some((_, a)) = actions.last_mut() { a["dims"] = json!([d]); } + widgets::mark_step(actions.last_mut(), step); } x = vr.max.x + 8.0; } diff --git a/crates/ui-egui/src/panels/essential.rs b/crates/ui-egui/src/panels/essential.rs index 02e077508..5df6f6b8c 100644 --- a/crates/ui-egui/src/panels/essential.rs +++ b/crates/ui-egui/src/panels/essential.rs @@ -399,7 +399,13 @@ fn value_widget(app: &mut EffectcraftApp, ui: &mut egui::Ui, auto: &str, ty: Opt let mut rect = lr; for (d, &old) in comps.iter().enumerate() { let mut x = old; - let r = ui.add(egui::DragValue::new(&mut x).speed(0.5).suffix(if d + 1 == comps.len() { "%" } else { "" }).max_decimals(1)); + let r = ui.add( + egui::DragValue::new(&mut x) + .speed(0.5) + .suffix(if d + 1 == comps.len() { "%" } else { "" }) + .max_decimals(1) + .custom_parser(|text| crate::num_math::calc(text, None)), + ); app.auto.add(&format!("{auto}.{d}"), r.rect, &p.name); rect = rect.union(r.rect); if r.changed() { diff --git a/crates/ui-egui/src/panels/fx_widgets.rs b/crates/ui-egui/src/panels/fx_widgets.rs index b3140d394..c39009fd3 100644 --- a/crates/ui-egui/src/panels/fx_widgets.rs +++ b/crates/ui-egui/src/panels/fx_widgets.rs @@ -8,6 +8,7 @@ use effectcraft_engine::geom::{Mat3, vec2 as gv2}; use egui::{Align2, Color32, Pos2, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; +use crate::num_math::Step; use crate::theme::Tokens; use crate::widgets; @@ -56,18 +57,35 @@ fn degrees_edit(v: f64, shown: f64, new: f64, dragging: bool) -> f64 { /// are hot numbers of their own (drag to scrub, click to type) and changing one keeps the other. /// Returns the revolutions' rect, the degrees' rect and the new angle when either changed. pub fn angle_field(ui: &mut egui::Ui, at: Pos2, id: egui::Id, v: f64, decimals: usize, t: &Tokens) -> (Rect, Rect, Option) { + let (rr, dr, out, _) = angle_field_step(ui, at, id, v, decimals, t); + (rr, dr, out) +} + +/// [`angle_field`], also returning a typed `+n` into the degrees (#3195): other selected layers +/// can turn their own angle by it. A typed step applies to the signed remainder, so `+10` on +/// `0x-30°` is `0x-20°`. +pub fn angle_field_step(ui: &mut egui::Ui, at: Pos2, id: egui::Id, v: f64, decimals: usize, t: &Tokens) -> (Rect, Rect, Option, Option) { let (rev, deg) = split_angle(v); let (rr, new_rev, _) = widgets::hot_int_at(ui, at, id.with("rev"), rev, 0.1, "x", t); let sign = ui.painter().text(pos2(rr.max.x - 2.0, rr.center().y), Align2::LEFT_CENTER, if deg < 0.0 { "-" } else { "+" }, Tokens::ui(12.0), t.hot_text); - let (dr, new_deg, _) = - widgets::hot_number_at(ui, pos2(sign.max.x - 2.0, at.y), id, deg.abs(), &widgets::HotSpec { speed: 0.5, range: (-1e9, 1e9), decimals, suffix: "°" }, t); + let (dr, new_deg, _, step) = widgets::hot_number_step_at( + ui, + pos2(sign.max.x - 2.0, at.y), + id, + deg.abs(), + &widgets::HotSpec { speed: 0.5, range: (-1e9, 1e9), decimals, suffix: "°" }, + t, + ); let dragging = new_deg.is_some() && ui.ctx().is_being_dragged(id); - let out = match (new_rev, new_deg) { - (Some(r), _) => Some(r as f64 * 360.0 + deg), - (None, Some(d)) => Some(degrees_edit(v, deg.abs(), d, dragging)), - (None, None) => None, + let out = match (new_rev, new_deg, step) { + (Some(r), _, _) => Some(r as f64 * 360.0 + deg), + // (The field shows the remainder's size; the step goes from the signed remainder.) + (None, Some(_), Some(s)) => s.apply(deg).map(|d| rev as f64 * 360.0 + d), + (None, Some(d), None) => Some(degrees_edit(v, deg.abs(), d, dragging)), + (None, None, _) => None, }; - (rr, dr, out) + let step = step.filter(|s| matches!(s, Step::Add(_)) && new_rev.is_none() && out.is_some()); + (rr, dr, out, step) } /// Dial angle (degrees, 0 = up, clockwise) of `p` around `c` in screen space. diff --git a/crates/ui-egui/src/panels/gradient_editor.rs b/crates/ui-egui/src/panels/gradient_editor.rs index 0f0bbe412..6f88519b7 100644 --- a/crates/ui-egui/src/panels/gradient_editor.rs +++ b/crates/ui-egui/src/panels/gradient_editor.rs @@ -440,7 +440,14 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { } .unwrap_or(0.0); let mut pct = pos * 100.0; - let r = ui.add(egui::DragValue::new(&mut pct).range(0.0..=100.0).speed(0.5).max_decimals(1).suffix("%")); + let r = ui.add( + egui::DragValue::new(&mut pct) + .range(0.0..=100.0) + .speed(0.5) + .max_decimals(1) + .suffix("%") + .custom_parser(|text| crate::num_math::calc(text, None)), + ); app.auto.add("dialog.gradient.location", r.rect, "Location"); if r.changed() { app.dialog_state.gradient.move_to(track, id, pct / 100.0); @@ -452,7 +459,14 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { Track::Opacity => { ui.label(tr("Opacity:")); let mut a = app.dialog_state.gradient.opacities.get(i).map_or(1.0, |s| s.value) as f64 * 100.0; - let r = ui.add(egui::DragValue::new(&mut a).range(0.0..=100.0).speed(0.5).max_decimals(1).suffix("%")); + let r = ui.add( + egui::DragValue::new(&mut a) + .range(0.0..=100.0) + .speed(0.5) + .max_decimals(1) + .suffix("%") + .custom_parser(|text| crate::num_math::calc(text, None)), + ); app.auto.add("dialog.gradient.opacity", r.rect, "Opacity"); if r.changed() { app.dialog_state.gradient.set_opacity(id, (a / 100.0) as f32); diff --git a/crates/ui-egui/src/panels/properties.rs b/crates/ui-egui/src/panels/properties.rs index 25829be7d..e6d3be35b 100644 --- a/crates/ui-egui/src/panels/properties.rs +++ b/crates/ui-egui/src/panels/properties.rs @@ -291,16 +291,17 @@ fn transform_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, match &value { Value::Scalar(v) if pr.name == "Rotation" || pr.name.ends_with(" Rotation") => { let (_, deg) = super::fx_widgets::split_angle(*v); - let (rr, dr, nv) = super::fx_widgets::angle_field(ui, pos2(vx, cy - 9.0), egui::Id::unique(("props-v", uid)), *v, decimals(deg), &vt); + let (rr, dr, nv, step) = super::fx_widgets::angle_field_step(ui, pos2(vx, cy - 9.0), egui::Id::unique(("props-v", uid)), *v, decimals(deg), &vt); app.auto.add(&format!("{base}.value"), dr, &pr.name); app.auto.add(&format!("{base}.revolutions"), rr, &pr.name); if let Some(nv) = nv { set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } } Value::Scalar(v) => { let range = if pr.name == "Opacity" { (0.0, 100.0) } else { (-1e9, 1e9) }; - let (vr, nv, _) = widgets::hot_number_at( + let (vr, nv, _, step) = widgets::hot_number_step_at( ui, pos2(vx, cy - 9.0), egui::Id::unique(("props-v", uid)), @@ -311,6 +312,7 @@ fn transform_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, app.auto.add(&format!("{base}.value"), vr, &pr.name); if let Some(nv) = nv { set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } } Value::Vec2(_) | Value::Vec3(_) => { @@ -332,7 +334,7 @@ fn transform_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, } } for d in 0..n.min(c.len()) { - let (vr, nv, _) = widgets::hot_number_at( + let (vr, nv, _, step) = widgets::hot_number_step_at( ui, pos2(x, cy - 9.0), egui::Id::unique(("props-v", uid, d)), @@ -357,6 +359,7 @@ fn transform_row(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painter, if let Some((_, a)) = actions.last_mut() { a["dims"] = json!((from..from + shown).collect::>()); } + widgets::mark_step(actions.last_mut(), step); } x = vr.max.x + 10.0; } diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 038dcc9b9..c6da2ccbd 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -5831,17 +5831,18 @@ fn scalar_editor(app: &mut EffectcraftApp, ui: &mut egui::Ui, ve: &ValueEdit<'_> _ => (-1e9, 1e9, 1, 1.0), }; if matches!(prop.ui, ParamUi::Angle) { - let (rr, dr, nv) = super::fx_widgets::angle_field(ui, pos2(x, y), egui::Id::unique(("v", uid, 0)), *v, 1, &vt); + let (rr, dr, nv, step) = super::fx_widgets::angle_field_step(ui, pos2(x, y), egui::Id::unique(("v", uid, 0)), *v, 1, &vt); app.auto.add(&format!("timeline.prop.{uid}.value"), dr, &prop.name); app.auto.add(&format!("timeline.prop.{uid}.revolutions"), rr, &prop.name); if let Some(nv) = nv { ve.set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } } else { // A Range Selector's Start, End and Offset count characters with Units = Index (#1814). let pct = (matches!(prop.ui, ParamUi::Percent) || prop.match_id == "opacity") && !effectcraft_engine::render::text::in_index_units(layer, uid); let suffix = if pct { "%" } else { "" }; - let (r, nv, _) = widgets::hot_number_at( + let (r, nv, _, step) = widgets::hot_number_step_at( ui, pos2(x, y), egui::Id::unique(("v", uid, 0)), @@ -5852,6 +5853,7 @@ fn scalar_editor(app: &mut EffectcraftApp, ui: &mut egui::Ui, ve: &ValueEdit<'_> app.auto.add(&format!("timeline.prop.{uid}.value"), r, &prop.name); if let Some(nv) = nv { ve.set(actions, json!(nv)); + widgets::mark_step(actions.last_mut(), step); } } } @@ -5890,7 +5892,7 @@ fn vector_editor(app: &mut EffectcraftApp, ui: &mut egui::Ui, ve: &ValueEdit<'_> } for d in 0..n.min(c.len()) { let suffix = if pct && d + 1 == n { "%" } else { "" }; - let (r, nv, _) = widgets::hot_number_at( + let (r, nv, _, step) = widgets::hot_number_step_at( ui, pos2(x, y), egui::Id::unique(("v", uid, d)), @@ -5924,6 +5926,7 @@ fn vector_editor(app: &mut EffectcraftApp, ui: &mut egui::Ui, ve: &ValueEdit<'_> a["dims"] = json!((from..from + shown).collect::>()); } } + widgets::mark_step(actions.last_mut(), step); } x = r.max.x + if d + 1 < n { 2.0 } else { 0.0 }; if d + 1 < n { diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 162a86c6a..c5b7223da 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -63,8 +63,23 @@ pub struct HotSpec<'a> { /// "Hot text": a blue number you drag horizontally to scrub, click to type. Tab / Shift+Tab /// while typing commits and types in the panel's next / previous one, as in After Effects. +/// Typed text may be arithmetic, `960/2` or `*2` for twice the value ([`crate::num_math`]). /// Returns (response, Some(new value) when changed, drag finished). pub fn hot_number_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: f64, spec: &HotSpec, t: &Tokens) -> (Rect, Option, bool) { + let (rect, out, done, _) = hot_number_step_at(ui, rect_min, id, value, spec, t); + (rect, out, done) +} + +/// [`hot_number_at`], also returning the typed step (`*2`, `+50`) the new value came from, so +/// that other selected layers can take the same step from their own value (#3195). +pub fn hot_number_step_at( + ui: &mut Ui, + rect_min: egui::Pos2, + id: egui::Id, + value: f64, + spec: &HotSpec, + t: &Tokens, +) -> (Rect, Option, bool, Option) { let HotSpec { speed, range, decimals, suffix } = *spec; let editing_id = id.with("editing"); let mut editing: Option = ui.data(|d| d.get_temp(editing_id)); @@ -87,14 +102,18 @@ pub fn hot_number_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: f64 .lock_focus(true), ); let tab = r.has_focus().then(|| ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Tab).then_some(i.modifiers.shift))).flatten(); - let mut out = None; + let (mut out, mut step) = (None, None); if r.lost_focus() || tab.is_some() { - let txt = buf.trim().trim_end_matches(suffix).trim().replace(',', "."); + // Typed arithmetic reads too (`960/2`), a leading `*`, `/` or `+` applied to the value + // shown (#3195). if !ui.input(|i| i.key_pressed(egui::Key::Escape)) - && let Ok(v) = txt.parse::() - && v.is_finite() + && let Some(typed) = crate::num_math::read(buf.trim().trim_end_matches(suffix), true) { - out = Some(v.clamp(range.0, range.1)); + let v = match typed { + crate::num_math::Typed::Value(v) => Some(v), + crate::num_math::Typed::Step(s) => s.apply(value).inspect(|_| step = Some(s)), + }; + out = v.map(|v| v.clamp(range.0, range.1)); } ui.data_mut(|d| d.remove::(editing_id)); if let Some(back) = tab { @@ -106,7 +125,7 @@ pub fn hot_number_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: f64 keep_focus(ui.ctx(), &r, &buf); ui.data_mut(|d| d.insert_temp(editing_id, buf)); } - return (er, out, out.is_some()); + return (er, out, out.is_some(), step); } let resp = ui.interact(rect, id, Sense::click_and_drag()); crate::pointer_wrap::track(&resp); @@ -140,7 +159,20 @@ pub fn hot_number_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: f64 if resp.clicked() { ui.data_mut(|d| d.insert_temp(editing_id, format!("{value:.decimals$}"))); } - (rect, out, resp.drag_stopped()) + (rect, out, resp.drag_stopped(), None) +} + +/// Mark a queued `prop.set` (with `selected`) as the typed `step` it came from, so every other +/// selected layer takes that step from its own value rather than the edited layer's result +/// (#3195): `+n` by the same change (`offset`), `*n` and `/n` by the same `factor`. +pub fn mark_step(action: Option<&mut (String, serde_json::Value)>, step: Option) { + let (Some((_, a)), Some(step)) = (action, step) else { return }; + match step.factor() { + Some(k) => a["factor"] = serde_json::json!(k), + None if matches!(step, crate::num_math::Step::Add(_)) => a["offset"] = serde_json::json!(true), + // (A factor too large to send: the other layers take the typed result.) + None => {} + } } /// Ctrl+Alt (Cmd+Opt on macOS) held while scrubbing hot text: After Effects' Proportional @@ -162,10 +194,19 @@ pub fn slider_scrub_speed(ui: &ParamUi) -> f64 { /// [`hot_number_at`] for a whole number (an angle's revolutions): a drag steps it once every /// `1 / speed` pixels however slowly the pointer moves (the fractions add up between frames), -/// and a typed value is truncated. Returns (rect, Some(new value) when changed, drag finished). +/// and a typed value is rounded (`5/2` is 3). Returns (rect, Some(new value) when changed, drag +/// finished). pub fn hot_int_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: i64, speed: f64, suffix: &str, t: &Tokens) -> (Rect, Option, bool) { let (rect, nv, done) = hot_number_at(ui, rect_min, id, value as f64, &HotSpec { speed, range: (-1e6, 1e6), decimals: 0, suffix }, t); let acc_id = id.with("acc"); + if let Some(n) = nv + && !ui.ctx().is_being_dragged(id) + { + // Typed (a drag changes the value only while it is dragged). (Within ±1e6: the cast is exact.) + ui.data_mut(|d| d.remove::(acc_id)); + let typed = n.round() as i64; + return (rect, (typed != value).then_some(typed), done); + } let acc = ui.data(|d| d.get_temp::(acc_id)).unwrap_or(0.0) + nv.map_or(0.0, |n| n - value as f64); // (Ten 0.1 steps add up to just under 1.) let step = (acc + acc.signum() * 1e-9).trunc(); @@ -177,8 +218,11 @@ pub fn hot_int_at(ui: &mut Ui, rect_min: egui::Pos2, id: egui::Id, value: i64, s (rect, (step != 0.0).then(|| value.saturating_add(step as i64)), done) } -/// Standard numeric editors use the same continuous native scrubbing as hot text. +/// Standard numeric editors use the same continuous native scrubbing as hot text, and read typed +/// arithmetic (`960/2`) too. They read their text on every keystroke, so a leading `*` or `/` +/// (relative to the value) is left to hot text, where it would otherwise compound (#3195). pub fn drag_value(value: egui::DragValue<'_>) -> impl egui::Widget { + let value = value.custom_parser(|text| crate::num_math::calc(text, None)); move |ui: &mut Ui| { let response = ui.add(value); crate::pointer_wrap::track(&response); diff --git a/crates/ui-egui/tests/it/ui_angles.rs b/crates/ui-egui/tests/it/ui_angles.rs index 007e11286..37eb01c8d 100644 --- a/crates/ui-egui/tests/it/ui_angles.rs +++ b/crates/ui-egui/tests/it/ui_angles.rs @@ -172,6 +172,21 @@ fn typing_replaces_the_clicked_value() { } } +/// #3195: a typed `+10` turns the angle by 10°, from its signed remainder: `0x-30°` becomes +/// `0x-20°` (the degrees field shows the remainder's size, 30, which would have given -40°). +#[test] +fn a_typed_step_applies_to_the_signed_angle() { + for panel in [Panel::EffectControls, Panel::Timeline, Panel::Properties] { + for (text, expected) in [("+10", -20.0), ("*2", -60.0), ("/3", -10.0)] { + let (mut h, layer, uid) = harness(panel, -30.0); + let p = component(&h, panel, uid, false); + begin_edit(&mut h, p, text); + key(&mut h, Key::Enter, Modifiers::NONE); + assert_eq!(angle(&h, layer, uid), expected, "{panel:?}: {text}"); + } + } +} + fn commit_edits(click_away: bool) { for panel in [Panel::EffectControls, Panel::Timeline, Panel::Properties] { for (revolutions, text, expected) in [(true, "3", 1110.5), (false, "45.5", 405.5)] { diff --git a/crates/ui-egui/tests/it/ui_inline_edit.rs b/crates/ui-egui/tests/it/ui_inline_edit.rs index 6c29045bb..c845863e4 100644 --- a/crates/ui-egui/tests/it/ui_inline_edit.rs +++ b/crates/ui-egui/tests/it/ui_inline_edit.rs @@ -404,6 +404,84 @@ fn tab_commits_a_value_and_moves_to_the_next() { assert_eq!(value(&h, "effects/#2/slider"), [42.0]); } +/// #3195: a value field reads typed arithmetic, as in After Effects: `960/2` sets 480, a leading +/// `*` applies to the value shown, and text that is no number (a division by zero) keeps the +/// value with no undo step. In the Timeline and in Effect Controls. +#[test] +fn typed_arithmetic_sets_a_value() { + let (mut h, _) = harness(); + let (layer, transform) = { + let s = &mut h.state_mut().session; + let l = s.execute("layer.newSolid", json!({"name": "Plate", "color": "#406080"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("layer.select", json!({"layers": [l]})).unwrap(); + s.execute("effect.apply", json!({"layers": [l], "effect": "Point Control"})).unwrap(); + s.execute("prop.set", json!({"layer": l, "path": "transform/position", "value": [160.0, 90.0]})).unwrap(); + let tr = s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.sub("transform").unwrap().uid; + (l, tr) + }; + h.state_mut().ui.timeline.open_layers.insert(layer); + h.state_mut().ui.timeline.open_groups.insert(transform); + h.state_mut().show_panel(effectcraft_ui_egui::dock::PanelKind::EffectControls); + h.run_steps(3); + let props = + |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.clone(); + let value = |h: &Harness<'_, EffectcraftApp>, path: &str| props(h).prop(path).unwrap().value.components(); + let position = props(&h).prop("transform/position").unwrap().uid; + assert_eq!(value(&h, "transform/position")[..2], [160.0, 90.0]); + let type_into = |h: &mut Harness<'_, EffectcraftApp>, id: &str, text: &str| { + // (Settle first: a click right after the last one's would count as part of a double click.) + h.run_steps(5); + let p = center(h, id); + click(h, p, 1); + type_text(h, text); + key(h, Key::Enter); + }; + type_into(&mut h, &format!("timeline.prop.{position}.value.0"), "960/2"); + assert_eq!(value(&h, "transform/position")[..2], [480.0, 90.0], "960/2 is 480"); + type_into(&mut h, &format!("timeline.prop.{position}.value.1"), "*2"); + assert_eq!(value(&h, "transform/position")[..2], [480.0, 180.0], "*2 doubles the value shown"); + let undo = h.state().session.history.undo.len(); + type_into(&mut h, &format!("timeline.prop.{position}.value.0"), "1/0"); + assert_eq!(value(&h, "transform/position")[..2], [480.0, 180.0], "a division by zero keeps the value"); + assert_eq!(h.state().session.history.undo.len(), undo, "and adds no undo step"); + let point = props(&h).prop("effects/#1/point").unwrap().uid; + type_into(&mut h, &format!("effectControls.prop.{point}.value.1"), "(3+4)*2"); + assert_eq!(value(&h, "effects/#1/point")[1], 14.0); +} + +/// #3195: with several layers selected, a typed `+10` or `*2` takes each layer's own value one +/// step on; it doesn't give them all the edited layer's result. +#[test] +fn a_typed_step_applies_to_each_selected_layer() { + let (mut h, _) = harness(); + let (a, b, transform) = { + let s = &mut h.state_mut().session; + let mut ids = vec![]; + for (name, opacity) in [("A", 40.0), ("B", 30.0)] { + let l = s.execute("layer.newSolid", json!({"name": name, "color": "#406080"})).unwrap()["layer"].as_u64().unwrap(); + s.execute("prop.set", json!({"layer": l, "path": "transform/opacity", "value": opacity})).unwrap(); + ids.push(l); + } + s.execute("layer.select", json!({"layers": ids})).unwrap(); + let tr = s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(ids[0])).unwrap().props.sub("transform").unwrap().uid; + (ids[0], ids[1], tr) + }; + h.state_mut().ui.timeline.open_layers.insert(a); + h.state_mut().ui.timeline.open_groups.insert(transform); + h.run_steps(3); + let opacity_of = |h: &Harness<'_, EffectcraftApp>, l: u64| { + h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.prop("transform/opacity").unwrap().value.as_f64() + }; + let opacity = h.state().session.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(a)).unwrap().props.prop("transform/opacity").unwrap().uid; + for (text, expected) in [("+10", (50.0, 40.0)), ("*2", (100.0, 80.0)), ("/4", (25.0, 20.0)), ("60", (60.0, 60.0))] { + let p = center(&h, &format!("timeline.prop.{opacity}.value")); + click(&mut h, p, 1); + type_text(&mut h, text); + key(&mut h, Key::Enter); + assert_eq!((opacity_of(&h, a), opacity_of(&h, b)), expected, "{text}"); + } +} + /// A click at `p` with `modifiers` held. fn click_with(h: &mut Harness<'_, EffectcraftApp>, p: Pos2, modifiers: Modifiers) { h.input_mut().events.push(Event::PointerMoved(p)); From b2d6690fd202c6a4a053839f946fc2e0fdb939c0 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 16:27:36 +0100 Subject: [PATCH 2/4] Number fields: Mask Feather stays non-negative on the other selected layers (#3195) A typed *-1 (the edited layer clamped at 0 by the panel) or a change by the same amount could take another selected layer's Mask Feather below 0. Clamp it at 0 there, as the panels do for the edited layer. --- crates/engine/src/cmd/prop.rs | 23 +++++++++++++++++++++++ crates/engine/src/tests/timeline.rs | 24 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index dc043d78f..ae37ea1b7 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -436,6 +436,9 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { let mut q = twin_params(p, olid, ouid); shifted(s, &mut q, olid, ouid, k); typed(s, &mut q, olid, ouid); + if is_mask_feather(s, cid, olid, ouid) { + non_negative(&mut q["value"]); + } // (a layer that can't take it, e.g. a clamped or static property, keeps its value) let _ = set(s, &q); } @@ -444,6 +447,26 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { if merge_p(p).is_some() { run(s) } else { super::app_more::grouped(s, "Change Property", run) } } +/// Whether property `uid` of `lid` is a mask's Mask Feather, which is never negative (After +/// Effects; the panels clamp a typed or scrubbed value at 0). +fn is_mask_feather(s: &Session, cid: ItemId, lid: LayerId, uid: Uid) -> bool { + let Some(l) = s.project.comp(cid).and_then(|c| c.layer(lid)) else { return false }; + l.props.find(uid).is_some_and(|q| q.match_id == "feather") && l.props.parent_of(uid).is_some_and(|g| matches!(g.kind, GroupKind::Mask { .. })) +} + +/// A JSON number, or each number of an array, raised to at least 0. +fn non_negative(v: &mut Value) { + match v { + Value::Number(n) => { + if let Some(x) = n.as_f64().filter(|x| *x < 0.0) { + *v = json!(x.max(0.0)); + } + } + Value::Array(a) => a.iter_mut().for_each(non_negative), + _ => {} + } +} + /// `prop.toggleAnimation`; with `selected`, the other selected layers' same property follows. fn toggle_anim_selected(s: &mut Session, p: &Value) -> Result { let twins = selected_twins(s, p, "prop.toggleAnimation")?; diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index 6ea4d1336..43eb922e2 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -1554,6 +1554,30 @@ fn a_typed_factor_scales_each_selected_layers_own_value() { assert_eq!(prop(&s, b, "transform/position").value.components()[..2], [50.0, 50.0], "B halves its own X, keeps its Y"); } +/// #3195: Mask Feather is never negative on the other selected layers either, when a typed `*-1` +/// (the edited layer clamped at 0 by the panel) or a change by the same amount takes theirs +/// below 0. +#[test] +fn a_selected_mask_feather_stays_non_negative() { + let (mut s, a) = setup(); + let b = s.execute("layer.newSolid", json!({"color": "#ff0000", "width": 50, "height": 50})).unwrap()["layer"].as_u64().unwrap(); + for (l, feather) in [(a, [10.0, 20.0]), (b, [4.0, 30.0])] { + s.execute("mask.new", json!({"layer": l, "vertices": [[0, 0], [20, 0], [20, 20]]})).unwrap(); + s.execute("prop.set", json!({"layer": l, "path": "masks/#1/feather", "value": feather})).unwrap(); + } + s.execute("layer.select", json!({"layers": [a, b]})).unwrap(); + let feather = |s: &Session, l: u64| prop(s, l, "masks/#1/feather").value.components(); + let uid = prop(&s, a, "masks/#1/feather").uid; + // `*-1` into the linked pair: the panel clamps the edited layer at [0, 0]. + s.execute_checked("prop.set", json!({"layer": a, "prop": uid, "value": [0.0, 0.0], "selected": true, "dims": [0, 1], "factor": -1.0})).unwrap(); + assert_eq!(feather(&s, a), [0.0, 0.0]); + assert_eq!(feather(&s, b), [0.0, 0.0], "B is clamped at 0, not [-4, -30]"); + // A change by the same amount (a scrub, or a typed `+`): A by [-10, -15] from [10, 20]. + s.execute("edit.undo", json!({})).unwrap(); + s.execute_checked("prop.set", json!({"layer": a, "prop": uid, "value": [0.0, 5.0], "selected": true, "offset": true})).unwrap(); + assert_eq!(feather(&s, b), [0.0, 15.0], "B's X is clamped at 0, its Y moves by -15"); +} + /// #2485: linked Audio Levels typed with several layers selected: each takes the typed Left, and /// its Right moves by its own change, so every layer keeps its own stereo balance. #[test] From 78acc6e1b1f02a19c18c7ecb7fe9d9afc368fd4b Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 17:41:58 +0100 Subject: [PATCH 3/4] Number fields: a typed +n adds n to each selected layer, even where the edited one is clamped (#3195) A typed +10 on several selected layers went through the same-change offset path, so when the edited layer's range cut its change short (Opacity 95 + 10 stops at 100) the others moved by 5. Send the typed operand as prop.set addend: each other layer takes its own value plus it, within its own range; scrubs keep the offset path. --- crates/engine/src/cmd/prop.rs | 26 ++++++++++++++--------- crates/engine/src/tests/timeline.rs | 11 ++++++++++ crates/ui-egui/src/widgets.rs | 11 +++++----- crates/ui-egui/tests/it/ui_inline_edit.rs | 10 +++++++++ 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index ae37ea1b7..028fdb0dd 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -371,24 +371,30 @@ fn set_selected(s: &mut Session, p: &Value) -> Result { }; let follow: Vec = p.get("follow").and_then(Value::as_array).into_iter().flatten().filter_map(Value::as_u64).filter_map(|d| usize::try_from(d).ok()).collect(); - // A typed `*2` or `/2` (#3195): each layer's own value times it, not the edited layer's result. + // A typed `*2`, `/2` (`factor`) or `+10` (`addend`) (#3195): each layer's own value times it + // or plus it, not the edited layer's result (which its range may have clamped). let factor = p.get("factor").and_then(Value::as_f64).filter(|k| k.is_finite() && delta.is_none()); + let addend = p.get("addend").and_then(Value::as_f64).filter(|n| n.is_finite() && delta.is_none()); + let own_step = |v: f64| -> Option { + let r = match (factor, addend) { + (Some(k), _) => v * k, + (None, Some(n)) => v + n, + (None, None) => return None, + }; + r.is_finite().then_some(r) + }; // Each layer's own value where `set` writes it (a `time` other than the CTI keys there). let typed = |s: &Session, q: &mut Value, olid: LayerId, ouid: Uid| { let at = snapped_key_time(s, cid, olid, q, "prop.set").ok().flatten(); - if let (true, Some(k)) = (scalar, factor) - && let Some(own) = components_at(s, cid, olid, ouid, at).and_then(|c| c.first().copied()) - && (own * k).is_finite() - { - q["value"] = json!(own * k); + if scalar && let Some(v) = components_at(s, cid, olid, ouid, at).and_then(|c| c.first().copied()).and_then(own_step) { + q["value"] = json!(v); } if let (Some(dims), Some(n)) = (&dims, &new) && let Some(mut c) = components_at(s, cid, olid, ouid, at) { // The typed components' new values on this layer. - let typed_value = |c: &[f64], d: usize| match factor { - Some(k) => c.get(d).map(|v| v * k).filter(|v| v.is_finite()), - None => n.get(d).copied(), + let typed_value = |c: &[f64], d: usize| { + if factor.is_some() || addend.is_some() { c.get(d).copied().and_then(own_step) } else { n.get(d).copied() } }; let change = dims.first().and_then(|&d| Some(typed_value(&c, d)? - c.get(d)?)).unwrap_or(0.0); let values: Vec> = dims.iter().map(|&d| typed_value(&c, d)).collect(); @@ -1459,7 +1465,7 @@ pub fn specs() -> Vec { "Set Property Value", [], None, - "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all), dims? (with selected, a typed value: the components edited, e.g. [2] for Z; the other layers keep the rest of their own value), follow? (with dims: components that move on each other layer by the change its first typed component takes), factor? (with selected, a typed `*` or `/`: the other layers take their own value times it, in the dims components of a vector)}", + "{layer?, path|prop, value, time? (layer s by default), timeBase?: layer|comp, merge?, selected? (also the other selected layers), offset? (them by the same change), proportional? (with offset: Proportional Scrubbing, each layer by its share in selection order, the first none, the last all), dims? (with selected, a typed value: the components edited, e.g. [2] for Z; the other layers keep the rest of their own value), follow? (with dims: components that move on each other layer by the change its first typed component takes), factor? (with selected, a typed `*` or `/`: the other layers take their own value times it, in the dims components of a vector), addend? (the same for a typed `+`: their own value plus it)}", has_layers, set_selected ), diff --git a/crates/engine/src/tests/timeline.rs b/crates/engine/src/tests/timeline.rs index 43eb922e2..f291de627 100644 --- a/crates/engine/src/tests/timeline.rs +++ b/crates/engine/src/tests/timeline.rs @@ -1552,6 +1552,17 @@ fn a_typed_factor_scales_each_selected_layers_own_value() { s.execute_checked("prop.set", json!({"layer": a, "prop": position, "value": [5.0, 20.0], "selected": true, "dims": [0], "factor": 0.5})).unwrap(); assert_eq!(prop(&s, a, "transform/position").value.components()[..2], [5.0, 20.0]); assert_eq!(prop(&s, b, "transform/position").value.components()[..2], [50.0, 50.0], "B halves its own X, keeps its Y"); + // A typed `+10` (`addend`): each layer's own value plus 10, within its own range, even where + // the edited layer's range cut its change short (95 + 10 stops at 100). + s.execute("prop.set", json!({"layer": a, "path": "transform/opacity", "value": 95.0})).unwrap(); + s.execute("prop.set", json!({"layer": b, "path": "transform/opacity", "value": 30.0})).unwrap(); + s.execute_checked("prop.set", json!({"layer": a, "prop": opacity, "value": 100.0, "selected": true, "addend": 10.0})).unwrap(); + assert_eq!(prop(&s, b, "transform/opacity").value.as_f64(), 40.0, "B adds 10, not the 5 A moved"); + s.execute("prop.set", json!({"layer": b, "path": "transform/opacity", "value": 95.0})).unwrap(); + s.execute_checked("prop.set", json!({"layer": a, "prop": opacity, "value": 100.0, "selected": true, "addend": 10.0})).unwrap(); + assert_eq!(prop(&s, b, "transform/opacity").value.as_f64(), 100.0, "B's own range still applies"); + s.execute_checked("prop.set", json!({"layer": a, "prop": position, "value": [15.0, 20.0], "selected": true, "dims": [0], "addend": 10.0})).unwrap(); + assert_eq!(prop(&s, b, "transform/position").value.components()[..2], [60.0, 50.0], "B adds 10 to its own X"); } /// #3195: Mask Feather is never negative on the other selected layers either, when a typed `*-1` diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index c5b7223da..87a6d217e 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -164,14 +164,15 @@ pub fn hot_number_step_at( /// Mark a queued `prop.set` (with `selected`) as the typed `step` it came from, so every other /// selected layer takes that step from its own value rather than the edited layer's result -/// (#3195): `+n` by the same change (`offset`), `*n` and `/n` by the same `factor`. +/// (#3195): `+n` adds `n` to each (`addend`; not the edited layer's change, which its range may +/// have clamped), `*n` and `/n` multiply each by the same `factor`. pub fn mark_step(action: Option<&mut (String, serde_json::Value)>, step: Option) { let (Some((_, a)), Some(step)) = (action, step) else { return }; - match step.factor() { - Some(k) => a["factor"] = serde_json::json!(k), - None if matches!(step, crate::num_math::Step::Add(_)) => a["offset"] = serde_json::json!(true), + match (step, step.factor()) { + (crate::num_math::Step::Add(n), _) => a["addend"] = serde_json::json!(n), + (_, Some(k)) => a["factor"] = serde_json::json!(k), // (A factor too large to send: the other layers take the typed result.) - None => {} + (_, None) => {} } } diff --git a/crates/ui-egui/tests/it/ui_inline_edit.rs b/crates/ui-egui/tests/it/ui_inline_edit.rs index c845863e4..7d51f0015 100644 --- a/crates/ui-egui/tests/it/ui_inline_edit.rs +++ b/crates/ui-egui/tests/it/ui_inline_edit.rs @@ -480,6 +480,16 @@ fn a_typed_step_applies_to_each_selected_layer() { key(&mut h, Key::Enter); assert_eq!((opacity_of(&h, a), opacity_of(&h, b)), expected, "{text}"); } + // `+10` where the edited layer stops at 100: the others still add 10, not the 5 it moved. + for (l, v) in [(a, 95.0), (b, 30.0)] { + h.state_mut().session.execute("prop.set", json!({"layer": l, "path": "transform/opacity", "value": v})).unwrap(); + } + h.run_steps(5); + let p = center(&h, &format!("timeline.prop.{opacity}.value")); + click(&mut h, p, 1); + type_text(&mut h, "+10"); + key(&mut h, Key::Enter); + assert_eq!((opacity_of(&h, a), opacity_of(&h, b)), (100.0, 40.0)); } /// A click at `p` with `modifiers` held. From 3ab35d0ff656530cd873c5cf293514ddfaa8b4b3 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 19:49:24 +0100 Subject: [PATCH 4/4] Doc comments for the functions this PR touches --- crates/engine/src/cmd/prop.rs | 1 + crates/ui-egui/src/num_math.rs | 7 +++++++ crates/ui-egui/src/panels/effect_controls/rows.rs | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/crates/engine/src/cmd/prop.rs b/crates/engine/src/cmd/prop.rs index 028fdb0dd..dec791b88 100644 --- a/crates/engine/src/cmd/prop.rs +++ b/crates/engine/src/cmd/prop.rs @@ -1457,6 +1457,7 @@ fn convert_one_expr(s: &mut Session, cid: ItemId, lid: LayerId, uid: Uid) -> Res }) } +/// Returns the command specs for the property commands (get, set, keyframes, expressions and the rest). pub fn specs() -> Vec { vec![ query!("prop.get", "Get Property", "{layer?, path|prop, time? (comp s)}", get), diff --git a/crates/ui-egui/src/num_math.rs b/crates/ui-egui/src/num_math.rs index 3e329c9e4..68225511c 100644 --- a/crates/ui-egui/src/num_math.rs +++ b/crates/ui-egui/src/num_math.rs @@ -126,6 +126,8 @@ struct Parser<'a> { } impl Parser<'_> { + /// Reads a sum or difference of terms, left to right. + /// Returns `None` on a syntax error or a non-finite result. fn expr(&mut self) -> Option { let mut v = self.term()?; while let Some(&op) = self.chars.peek() { @@ -144,6 +146,8 @@ impl Parser<'_> { Some(v) } + /// Reads a product or quotient of factors, left to right. + /// Returns `None` on a syntax error, a division by zero or a non-finite result. fn term(&mut self) -> Option { let mut v = self.factor()?; while let Some(&op) = self.chars.peek() { @@ -162,6 +166,8 @@ impl Parser<'_> { Some(v) } + /// Reads a signed factor, a bracketed expression or a number. + /// Nesting deeper than `MAX_DEPTH` gives `None`. fn factor(&mut self) -> Option { let c = *self.chars.peek()?; match c { @@ -239,6 +245,7 @@ impl Parser<'_> { mod tests { use super::calc as eval; + /// Evaluates `text` with no current value, so a leading operator has nothing to apply to. fn abs(text: &str) -> Option { eval(text, None) } diff --git a/crates/ui-egui/src/panels/effect_controls/rows.rs b/crates/ui-egui/src/panels/effect_controls/rows.rs index e2634d6b8..b21021bb3 100644 --- a/crates/ui-egui/src/panels/effect_controls/rows.rs +++ b/crates/ui-egui/src/panels/effect_controls/rows.rs @@ -260,6 +260,7 @@ fn value_mask(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: & } } +/// Draws an angle property's value cell as revolutions plus degrees and queues a set action when it is edited. fn value_angle(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: &Value, actions: &mut Actions) { let Value::Scalar(v) = value else { return; @@ -275,6 +276,7 @@ fn value_angle(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: cx.twirl_open(app, ui); } +/// Draws a single-number property's value cell with its range, unit suffix and scrub speed, and queues a set action when it is edited. fn value_scalar(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: &Value, actions: &mut Actions) { let Value::Scalar(v) = value else { return; @@ -308,6 +310,8 @@ fn value_scalar(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: } } +/// Draws one numeric cell per component of a point or vector property, with the pick crosshair for points. +/// Returns the cell rectangles. fn value_vector(app: &mut EffectcraftApp, ui: &mut egui::Ui, cx: &PropCx, value: &Value, actions: &mut Actions) -> Vec { let mut dims = vec![]; let PropCx { t, uid, cy, vx, layer, prop, .. } = *cx;