From c17f9eb959a4d27843dbabb905a0bea2feae3d0c Mon Sep 17 00:00:00 2001 From: SBaksa Date: Mon, 19 Oct 2026 21:08:16 +1100 Subject: [PATCH 1/3] Text Range Selector: Start, End and Offset show as counts with Units = Index (#1814) With Units set to Index the selector already counts characters (words, lines), but the timeline kept showing Start, End and Offset as percentages ("3.0%"), so switching looked like it did nothing. They are now shown without the % sign while Units is Index (render::text::in_index_units). --- crates/render/src/text.rs | 8 ++++++++ crates/render/src/text/tests.rs | 18 ++++++++++++++++++ crates/ui-egui/src/panels/timeline.rs | 4 +++- 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/render/src/text.rs b/crates/render/src/text.rs index fbb0feb2..6abc2ae9 100644 --- a/crates/render/src/text.rs +++ b/crates/render/src/text.rs @@ -187,6 +187,14 @@ pub fn range_value_of(f: f64, offset: f64, units_index: bool, n: usize) -> f64 { if units_index { (v * n as f64).round() } else { v * 100.0 } } +/// Whether property `uid` of `layer` is a Range Selector's Start, End or Offset while its Units +/// are Index: the value is then a count of characters (words, lines), not a percentage. +pub fn in_index_units(layer: &Layer, uid: effectcraft_project::Uid) -> bool { + let Some(sel) = layer.props.parent_of(uid).filter(|g| g.match_id == "rangeSelector") else { return false }; + let is_range_value = sel.props().any(|p| p.uid == uid && matches!(p.match_id.as_str(), "start" | "end" | "offset")); + is_range_value && sel.prop("advanced/units").is_some_and(|u| u.value == Value::Enum(1)) +} + /// Selection of every glyph by an animator's selectors, per dimension. pub fn animator_selection(ctx: &EvalCtx, layer: &Layer, anim: &PropGroup, lay: &TextLayout) -> Vec<[f64; 3]> { let sels: Vec<&PropGroup> = anim.sub("selectors").map(|s| s.groups().filter(|g| g.enabled).collect()).unwrap_or_default(); diff --git a/crates/render/src/text/tests.rs b/crates/render/src/text/tests.rs index 020e0851..7721d1fb 100644 --- a/crates/render/src/text/tests.rs +++ b/crates/render/src/text/tests.rs @@ -99,6 +99,24 @@ fn range_based_on_words_and_lines() { assert_eq!(s, vec![0.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0], "{s:?}"); } +/// With Units = Index a Range Selector's Start, End and Offset are character counts, not +/// percentages (#1814); the selector's other values, and other selectors, are unaffected. +#[test] +fn index_units_mark_start_end_and_offset() { + let (mut p, _cid, comp) = setup(400, 200); + let mut l = text_layer(&mut p, &comp, doc("ABCDEFG", 30.0)); + animator(&mut p, &mut l, &["opacity"], &["range", "wiggly"]); + let uid = |l: &Layer, path: &str| l.props.prop(path).unwrap_or_else(|| panic!("no {path}")).uid; + let base = "text/animators/#1/selectors/#1"; + let range_values = ["start", "end", "offset"].map(|k| uid(&l, &format!("{base}/{k}"))); + assert!(range_values.iter().all(|u| !in_index_units(&l, *u)), "Percentage, the default"); + set(&mut l, &format!("{base}/advanced/units"), Value::Enum(1)); + assert!(range_values.iter().all(|u| in_index_units(&l, *u))); + assert!(!in_index_units(&l, uid(&l, &format!("{base}/advanced/amount"))), "Amount stays a percentage"); + assert!(!in_index_units(&l, uid(&l, "text/animators/#1/properties/opacity"))); + assert!(!in_index_units(&l, uid(&l, "transform/opacity"))); +} + #[test] fn range_start_end_and_offset_are_capped() { let (mut p, cid, comp) = setup(400, 200); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index b34fbd9d..493fd63b 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -4117,7 +4117,9 @@ fn value_editor( set(actions, json!(nv)); } } else { - let suffix = if matches!(prop.ui, ParamUi::Percent) || prop.match_id == "opacity" { "%" } 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(ui, pos2(x, y), egui::Id::new(("v", uid, 0)), *v, speed, (min, max), dec.max(1), suffix, &t); app.auto.add(&format!("timeline.prop.{uid}.value"), r, &prop.name); if let Some(nv) = nv { From 71aafd6307cf7c6812fba0f2fcfe6229debabb7d Mon Sep 17 00:00:00 2001 From: SBaksa Date: Tue, 20 Oct 2026 01:05:15 +1100 Subject: [PATCH 2/3] Text Range Selector: test the Timeline shows Index-unit values without % (#1814) Start, End and Offset are read as painted in the Timeline: with % under Units = Percentage, without it under Units = Index. Fails on the old suffix rule ("0.0%"). --- crates/ui-egui/tests/ui_text_range_handles.rs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/ui-egui/tests/ui_text_range_handles.rs b/crates/ui-egui/tests/ui_text_range_handles.rs index 1d048be5..aab80100 100644 --- a/crates/ui-egui/tests/ui_text_range_handles.rs +++ b/crates/ui-egui/tests/ui_text_range_handles.rs @@ -194,3 +194,46 @@ fn a_bar_on_a_box_handle_leaves_it_the_press() { assert_eq!(layer.props.find(start).unwrap().value.as_f64(), 0.0, "Start stays"); assert_ne!(scale(&h), before, "the handle scaled the layer"); } + +/// The text the Timeline paints in property `uid`'s value field. +fn timeline_value(h: &Harness<'_, EffectcraftApp>, uid: u64) -> String { + let e = h.state().auto.find(&format!("timeline.prop.{uid}.value")).unwrap_or_else(|| panic!("timeline.prop.{uid}.value isn't shown")); + let field = egui::Rect::from_min_size(pos2(e.rect[0], e.rect[1]), vec2(e.rect[2], e.rect[3])); + h.output() + .shapes + .iter() + .filter_map(|c| match &c.shape { + egui::Shape::Text(t) if field.contains(t.visual_bounding_rect().center()) => Some(t.galley.text().to_string()), + _ => None, + }) + .collect() +} + +/// #1814: with Units = Index the Timeline shows the Range Selector's Start, End and Offset as +/// counts, without the % sign they carry with Units = Percentage. +#[test] +fn index_units_show_start_end_and_offset_without_a_percent_sign() { + let (s, ids) = setup(); + let l = LayerId(ids.layer); + let layer = s.active_comp().unwrap().layer(l).unwrap().clone(); + let text = layer.props.sub("text").unwrap(); + let anim = animator(&s, ids.layer); + let selectors = anim.sub("selectors").unwrap(); + let sel = selectors.groups().next().unwrap(); + let (offset, units) = (sel.get("offset").unwrap().uid, sel.prop("advanced/units").unwrap().uid); + let mut h = harness(s); + let tl = &mut h.state_mut().ui.timeline; + tl.open_layers.insert(ids.layer); + let animators = text.sub("animators").unwrap().uid; + tl.open_groups.extend([text.uid, animators, anim.uid, selectors.uid, sel.uid]); + h.run_steps(3); + for uid in [ids.start, ids.end, offset] { + assert!(timeline_value(&h, uid).ends_with('%'), "Percentage units: {}", timeline_value(&h, uid)); + } + h.state_mut().session.execute("prop.set", json!({"layer": ids.layer, "prop": units, "value": 1})).unwrap(); + h.run_steps(3); + for uid in [ids.start, ids.end, offset] { + let v = timeline_value(&h, uid); + assert!(!v.is_empty() && !v.contains('%'), "Index units: {v:?}"); + } +} From f41aa134a864dd0d02cfa95f4b8d0101eec7c163 Mon Sep 17 00:00:00 2001 From: SBaksa Date: Tue, 20 Oct 2026 12:09:16 +1100 Subject: [PATCH 3/3] Text Range Selector test: change Units through the Timeline popup (#1814) The Index-units test now opens the Units popup in the Timeline and picks Index, asserts the stored Units value, then reads the painted Start, End and Offset, so it also fails when the Units control doesn't update the session. --- crates/ui-egui/tests/ui_text_range_handles.rs | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/tests/ui_text_range_handles.rs b/crates/ui-egui/tests/ui_text_range_handles.rs index aab80100..7ce301b3 100644 --- a/crates/ui-egui/tests/ui_text_range_handles.rs +++ b/crates/ui-egui/tests/ui_text_range_handles.rs @@ -209,8 +209,26 @@ fn timeline_value(h: &Harness<'_, EffectcraftApp>, uid: u64) -> String { .collect() } +/// The centre of the first painted text that reads `label` exactly (a popup's option). +fn text_at(h: &Harness<'_, EffectcraftApp>, label: &str) -> Option { + h.output().shapes.iter().find_map(|c| match &c.shape { + egui::Shape::Text(t) if t.galley.text() == label => Some(t.visual_bounding_rect().center()), + _ => None, + }) +} + +fn click_at(h: &mut Harness<'_, EffectcraftApp>, p: Pos2) { + h.event(Event::PointerMoved(p)); + h.step(); + h.event(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + h.event(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(3); +} + /// #1814: with Units = Index the Timeline shows the Range Selector's Start, End and Offset as -/// counts, without the % sign they carry with Units = Percentage. +/// counts, without the % sign they carry with Units = Percentage. Units is changed the way a +/// user changes it: its Timeline popup, then the Index option. #[test] fn index_units_show_start_end_and_offset_without_a_percent_sign() { let (s, ids) = setup(); @@ -221,17 +239,29 @@ fn index_units_show_start_end_and_offset_without_a_percent_sign() { let selectors = anim.sub("selectors").unwrap(); let sel = selectors.groups().next().unwrap(); let (offset, units) = (sel.get("offset").unwrap().uid, sel.prop("advanced/units").unwrap().uid); + let advanced = sel.sub("advanced").unwrap().uid; let mut h = harness(s); let tl = &mut h.state_mut().ui.timeline; tl.open_layers.insert(ids.layer); let animators = text.sub("animators").unwrap().uid; - tl.open_groups.extend([text.uid, animators, anim.uid, selectors.uid, sel.uid]); + tl.open_groups.extend([text.uid, animators, anim.uid, selectors.uid, sel.uid, advanced]); h.run_steps(3); for uid in [ids.start, ids.end, offset] { assert!(timeline_value(&h, uid).ends_with('%'), "Percentage units: {}", timeline_value(&h, uid)); } - h.state_mut().session.execute("prop.set", json!({"layer": ids.layer, "prop": units, "value": 1})).unwrap(); - h.run_steps(3); + // Units ▸ Index through the Timeline: open its popup, pick the option. + let units_value = |h: &Harness<'_, EffectcraftApp>| h.state().session.active_comp().unwrap().layer(l).unwrap().props.find(units).unwrap().value.clone(); + assert_eq!(units_value(&h), effectcraft_engine::keyframe::Value::Enum(0), "Percentage to start with"); + let field = h + .state() + .auto + .find(&format!("timeline.prop.{units}.value")) + .map(|e| pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0)) + .expect("Units is shown"); + click_at(&mut h, field); + let index = text_at(&h, "Index").expect("the Units popup lists Index"); + click_at(&mut h, index); + assert_eq!(units_value(&h), effectcraft_engine::keyframe::Value::Enum(1), "the popup set Units to Index"); for uid in [ids.start, ids.end, offset] { let v = timeline_value(&h, uid); assert!(!v.is_empty() && !v.contains('%'), "Index units: {v:?}");