Merge #1865: Text Range Selector: Start, End and Offset show as counts with Units = Index (#1814) (@SBaksa)

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();

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);

crates/ui-egui/src/panels/timeline.rs

@@ -4122,7 +4122,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 {

crates/ui-egui/tests/it/ui_text_range_handles.rs

@@ -194,3 +194,76 @@ 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()
+}
+
+/// The centre of the first painted text that reads `label` exactly (a popup's option).
+fn text_at(h: &Harness<'_, EffectcraftApp>, label: &str) -> Option<Pos2> {
+    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. 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();
+    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 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, 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));
+    }
+    // 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:?}");
+    }
+}