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 f79939c8..dd7c5362 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/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 { diff --git a/crates/ui-egui/tests/it/ui_text_range_handles.rs b/crates/ui-egui/tests/it/ui_text_range_handles.rs index 1d048be5..7ce301b3 100644 --- a/crates/ui-egui/tests/it/ui_text_range_handles.rs +++ b/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 { + 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:?}"); + } +}