From 2419605cc4740845e514bdcda3642f0da493cf6f Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:11:04 +0700 Subject: [PATCH 01/10] Timeline: a press in the time ruler moves the current time (#284) The ruler moved the current-time indicator only once the pointer had dragged past egui's drag threshold, or on release for a click. As in After Effects, the time now jumps to the pointer as soon as the button goes down in the ruler, then follows the drag. Test: a_press_in_the_ruler_moves_the_current_time (ui_timeline_zoom). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/timeline.rs | 5 +++-- crates/ui-egui/tests/ui_timeline_zoom.rs | 28 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 44d340b7..3ed530c1 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -1320,9 +1320,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } s += secs_per_label; } - // Scrub in the ruler. + // Scrub in the ruler: the current time jumps to the pointer as soon as the button goes down + // (After Effects), then follows it. let rresp = ui.interact(Rect::from_min_max(pos2(graph_x0, ruler.min.y + 12.0), ruler.max), egui::Id::new("tl-ruler"), Sense::click_and_drag()); - if (rresp.dragged() || rresp.clicked()) + if ((rresp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_down())) || rresp.clicked()) && let Some(pt) = rresp.interact_pointer_pos() { let mut secs = tm.t(pt.x).max(0.0); diff --git a/crates/ui-egui/tests/ui_timeline_zoom.rs b/crates/ui-egui/tests/ui_timeline_zoom.rs index 29f26b08..2a9c2ba9 100644 --- a/crates/ui-egui/tests/ui_timeline_zoom.rs +++ b/crates/ui-egui/tests/ui_timeline_zoom.rs @@ -213,3 +213,31 @@ fn shift_dragging_the_work_area_snaps_to_the_current_time() { drag_with(&mut h, mid, to, Modifiers::SHIFT); assert!(near(work_area(&h), (3.0, 6.0)), "the bar snaps its end to the CTI: {:?}", work_area(&h)); } + +/// #284: pressing in the time ruler moves the current time there at once, before the pointer +/// moves or the button comes up (After Effects); dragging on then scrubs. +#[test] +fn a_press_in_the_ruler_moves_the_current_time() { + let (mut h, _) = harness(); + // (Opening the comp showed all of it.) + h.state_mut().ui.timeline.pps = Some(200.0); + h.state_mut().ui.timeline.start = 10.0; + h.run_steps(2); + let ruler = rect(&h, "timeline.ruler"); + let (start, _) = visible(&h); + // 2 s into the visible span at 200 px/s (the ruler maps time from 6 px in). + let at = pos2(ruler.min.x + 6.0 + 400.0, ruler.min.y + 17.0); + h.event(Event::PointerMoved(at)); + h.step(); + h.event(Event::PointerButton { pos: at, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + let now = h.state().session.time().seconds(); + assert!((now - (start + 2.0)).abs() < 0.05, "the press moved the CTI to {}: {now}", start + 2.0); + // Still held: dragging scrubs on. + h.event(Event::PointerMoved(at + vec2(100.0, 0.0))); + h.step(); + let now = h.state().session.time().seconds(); + assert!((now - (start + 2.5)).abs() < 0.05, "{now}"); + h.event(Event::PointerButton { pos: at + vec2(100.0, 0.0), button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.step(); +} From f9570d783ace0bcf252b44ec909be6e646a72075 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:11:27 +0700 Subject: [PATCH 02/10] Viewer: scrubbing drops the viewer frames still waiting (#284) Scrubbing asks for the viewer frame under the pointer at every step. Each newer request only demoted the earlier ones to prefetch, so every frame the pointer passed was still rendered, oldest first, holding the frame threads the frame under the pointer needed. A newer viewer frame of a comp now drops that comp's viewer frames that haven't started (and releases them, so they can be asked for again). Frames already rendering finish into the cache (rendering can't be interrupted mid-frame), and the viewer keeps its last frame until the new one is in. Prefetch frames and other comps' viewer frames stay queued. Test: a_newer_viewer_frame_drops_the_ones_still_waiting (frames). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/frames.rs | 64 ++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/frames.rs b/crates/ui-egui/src/frames.rs index 6ff49387..a157359f 100644 --- a/crates/ui-egui/src/frames.rs +++ b/crates/ui-egui/src/frames.rs @@ -667,30 +667,37 @@ impl Frames { return; } let Ok(mut q) = self.queue.lock() else { return }; + let Ok(mut inf) = self.inflight.lock() else { return }; // Jobs for an older project revision can never be shown: drop them. let before = q.jobs.len(); q.jobs.retain(|j| j.key.revision >= key.revision); - let dropped = before - q.jobs.len(); - if dropped > 0 - && let Ok(mut inf) = self.inflight.lock() - { + if q.jobs.len() < before { inf.retain(|k| k.revision >= key.revision || q.running.contains(k)); } if urgent { - // Only the newest viewer frame is urgent; earlier ones become prefetch. + // Only the newest viewer frame is urgent. The comp's earlier viewer frames that + // haven't started are dropped (scrubbing: rendering every frame the pointer passed + // would hold the workers the frame under it needs); frames already rendering finish + // into the cache, and the viewer shows its last frame until the new one is in + // (#284). Other comps' viewer frames become prefetch. + q.jobs.retain(|j| { + let superseded = j.urgent && j.key != key && j.key.comp == key.comp; + if superseded { + inf.remove(&j.key); + } + !superseded + }); for j in q.jobs.iter_mut() { j.urgent = j.key == key; } } - { - let Ok(mut inf) = self.inflight.lock() else { return }; - if !inf.insert(key) { - return; - } + if !inf.insert(key) { + return; } q.seq += 1; let seq = q.seq; q.jobs.push(Job { key, src: src.clone(), comp, t, opts, urgent, seq }); + drop(inf); drop(q); #[cfg(not(target_arch = "wasm32"))] if self.remote.is_none() { @@ -1161,6 +1168,43 @@ mod tests { assert!(!f.is_cached(&key)); } + /// #284: scrubbing asks for a viewer frame at every step; a newer one drops the comp's viewer + /// frames still waiting (they would all render first), but not prefetch frames or another + /// comp's viewer frame. + #[test] + fn a_newer_viewer_frame_drops_the_ones_still_waiting() { + /// Starts nothing, so every request stays queued. + struct Idle; + impl RemoteFrames for Idle { + fn slots(&self) -> usize { + 0 + } + fn start(&self, _: RemoteJob, _: RemoteDone) {} + } + let mut f = Frames::default(); + f.set_remote(Some(Arc::new(Idle))); + let src = RenderSource { + project: Arc::default(), + footage: Arc::new(effectcraft_engine::render::NoFootage), + expr: None, + layer_cache: Arc::default(), + gpu: None, + gpu_display: false, + disk: None, + }; + let request = |k: FrameKey, urgent: bool| f.request_with(&src, k, ItemId(k.comp), Tick::ZERO, RenderOpts::default(), urgent); + let other = FrameKey { comp: 2, ..key(1, 0, 0) }; + request(other, true); + request(key(1, 20, 0), false); + for frame in 0..10 { + request(key(1, frame, 0), true); + } + let mut queued: Vec<(u64, i64, bool)> = f.queue.lock().unwrap().jobs.iter().map(|j| (j.key.comp, j.key.frame, j.urgent)).collect(); + queued.sort(); + assert_eq!(queued, [(1, 9, true), (1, 20, false), (2, 0, false)]); + assert_eq!(f.inflight(), 3, "dropped frames aren't in flight: the viewer can ask again"); + } + /// Each time the GPU runs out of memory, half as many GPU frames are kept, down to a floor. #[test] fn running_out_of_video_memory_halves_the_gpu_frame_budget() { From 976784b0a66f9112207482669dd345b5d0862d9a Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:13:37 +0700 Subject: [PATCH 03/10] Timeline: Shift-clicking layer bars selects the range (#284) Shift-clicking a layer's name selected the layers from the last one clicked to it, but Shift-clicking a layer bar in the time graph only added that layer. Bars now select the same range (shared with the name column: `shift_range`), and a click on a bar becomes where the next range starts, as a click on a name does. Test: timeline_shift_click_on_layer_bars_selects_the_range (ui_workflow_selection). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/timeline.rs | 59 ++++++++++++------- crates/ui-egui/tests/ui_workflow_selection.rs | 20 +++++++ 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 3ed530c1..96a4b686 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -8,7 +8,7 @@ use effectcraft_engine::color::BlendMode; use effectcraft_engine::keyframe::{Interp, Value}; -use effectcraft_engine::project::{Comp, GroupKind, Layer, LayerId, LayerSource, MatteKind, Node, ParamUi, PropGroup, Property}; +use effectcraft_engine::project::{Comp, GroupKind, ItemId, Layer, LayerId, LayerSource, MatteKind, Node, ParamUi, PropGroup, Property}; use effectcraft_engine::render::EvalCtx; use effectcraft_engine::time::Tick; use egui::{Align2, Color32, Pos2, Rect, Sense, Stroke, StrokeKind, pos2, vec2}; @@ -1029,6 +1029,27 @@ fn layer_switch( actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name}))); } } + +fn anchor_id(comp: ItemId) -> egui::Id { + egui::Id::new(("timeline-selection-anchor", comp.0)) +} + +/// Remember `layer` as where the next Shift-click selection starts. +fn set_anchor(ctx: &egui::Context, comp: ItemId, layer: LayerId) { + ctx.data_mut(|d| d.insert_temp(anchor_id(comp), layer)); +} + +/// The layers a Shift-click on `layer` (its name or its bar) selects: those from the last layer +/// clicked (else the first selected) to it in stack order, but locked layers and layers the +/// Timeline doesn't show. `None` without such a layer. +fn shift_range(ctx: &egui::Context, cid: ItemId, comp: &Comp, rows: &[Row], selected: &[LayerId], layer: LayerId) -> Option> { + let anchor = ctx.data(|d| d.get_temp::(anchor_id(cid))).or_else(|| selected.first().copied())?; + let a = comp.layers.iter().position(|l| l.id == anchor)?; + let b = comp.layers.iter().position(|l| l.id == layer)?; + let span = comp.layers.get(a.min(b)..=a.max(b))?; + Some(span.iter().filter(|l| !l.switches.locked && rows.iter().any(|r| r.layer == l.id)).map(|l| l.id.0).collect()) +} + /// Set on the frame the reorder drag is released. /// Where layers dropped at height `py` land among the visible layer rows: above the first row /// whose middle is below it (its index in `layer_rows`), else below the last row. Returns that @@ -1621,27 +1642,11 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { app.auto.add(&format!("timeline.layer.{}.row", layer.id.0), left, &layer.name); if row_resp.clicked() && !locked { let m = ui.input(|i| i.modifiers); - let anchor_key = egui::Id::new(("timeline-selection-anchor", cid.0)); - let anchor = ctx.data(|d| d.get_temp::(anchor_key)).or_else(|| selected.first().copied()); - let range = if m.shift { - anchor.and_then(|a| comp.layers.iter().position(|l| l.id == a)).map(|a| { - let b = comp.layers.iter().position(|l| l.id == layer.id).unwrap_or(a); - comp.layers - .get(a.min(b)..=a.max(b)) - .unwrap_or_default() - .iter() - .filter(|l| !l.switches.locked && rows.iter().any(|r| r.layer == l.id)) - .map(|l| l.id.0) - .collect::>() - }) - } else { - None - }; - if let Some(layers) = range { + if let Some(layers) = m.shift.then(|| shift_range(&ctx, cid, &comp, &rows, &selected, layer.id)).flatten() { actions.push(("layer.select".into(), json!({"layers": layers}))); } else { actions.push(("layer.select".into(), json!({"layers": [layer.id.0], "toggle": m.command}))); - ctx.data_mut(|d| d.insert_temp(anchor_key, layer.id)); + set_anchor(&ctx, cid, layer.id); } } if row_resp.double_clicked() { @@ -1842,8 +1847,18 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if lin.hovered() || lout.hovered() || lin.dragged() || lout.dragged() { ctx.set_cursor_icon(egui::CursorIcon::ResizeHorizontal); } - if (body.clicked() || body.drag_started()) && !is_sel { - actions.push(("layer.select".into(), json!({"layers": [layer.id.0], "add": ui.input(|i| i.modifiers.shift)}))); + let shift = ui.input(|i| i.modifiers.shift); + if let Some(layers) = (body.clicked() && shift).then(|| shift_range(&ctx, cid, &comp, &rows, &selected, layer.id)).flatten() { + // Shift-click: the layers from the last one clicked to this one, as in + // the layer outline. + actions.push(("layer.select".into(), json!({"layers": layers}))); + } else { + if (body.clicked() || body.drag_started()) && !is_sel { + actions.push(("layer.select".into(), json!({"layers": [layer.id.0], "add": shift}))); + } + if (body.clicked() || body.drag_started()) && !shift { + set_anchor(&ctx, cid, layer.id); + } } layer_context_menu(&body, layer, &mut actions); let drag_key = format!("bar-{}", layer.id.0); @@ -1978,7 +1993,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let wr = Rect::from_min_max(pos2(graph_x0, r.min.y), r.max); gp.rect_filled(wr, 0.0, t.tl_bg); app.auto.add(&format!("timeline.layer.{}.waveform", layer.id.0), wr, "Waveform"); - match super::waveform::summary(app, &ctx, effectcraft_engine::project::ItemId(*item)) { + match super::waveform::summary(app, &ctx, ItemId(*item)) { Some(s) => super::waveform::draw(&gp, wr.shrink2(vec2(0.0, 2.0)), &tm, layer, &s, Color32::from_rgb(0x5f, 0xc8, 0x8a)), None => { gp.text( diff --git a/crates/ui-egui/tests/ui_workflow_selection.rs b/crates/ui-egui/tests/ui_workflow_selection.rs index 1880bb86..7b1ca768 100644 --- a/crates/ui-egui/tests/ui_workflow_selection.rs +++ b/crates/ui-egui/tests/ui_workflow_selection.rs @@ -58,6 +58,26 @@ fn timeline_shift_click_selects_range_and_ctrl_toggles() { click(&mut h, p, Modifiers::COMMAND); assert_eq!(h.state().session.state.selected_layers, vec![ids[0], ids[2]]); } +/// #284: Shift-clicking layer bars in the time graph selects the range from the last layer +/// clicked, as Shift-clicking their names does. +#[test] +fn timeline_shift_click_on_layer_bars_selects_the_range() { + let mut h = harness(); + let ids: Vec = h.state().session.active_comp().unwrap().layers.iter().map(|l| l.id).collect(); + let bar = |h: &Harness<'_, EffectcraftApp>, id: LayerId| rect(h, &format!("timeline.layer.{}.bar", id.0)).center(); + let p = bar(&h, ids[3]); + click(&mut h, p, Modifiers::NONE); + assert_eq!(h.state().session.state.selected_layers, vec![ids[3]]); + let p = bar(&h, ids[1]); + click(&mut h, p, Modifiers::SHIFT); + assert_eq!(h.state().session.state.selected_layers, ids[1..]); + // From a name to a bar too. + let r = rect(&h, &format!("timeline.layer.{}.row", ids[0].0)); + click(&mut h, pos2(r.min.x + 210.0, r.center().y), Modifiers::NONE); + let p = bar(&h, ids[2]); + click(&mut h, p, Modifiers::SHIFT); + assert_eq!(h.state().session.state.selected_layers, ids[..3]); +} #[test] fn timeline_empty_outline_marquee_selects_layers() { let mut h = harness(); From 2eb56cff1c454ee26828e10f153b6084f7df7c41 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:13:54 +0700 Subject: [PATCH 04/10] Timeline: a test that the cached frames show in green (#284) #284 reported no cache indication in the Timeline. Main has one: a green line under the work area bar for the frames cached at the viewer's resolution, view and options (blue for disk-cached frames). This test pins it down: once the viewer's frame has rendered, the Timeline paints the green line from the start of the work area. Test: the_timeline_shows_the_cached_frames_in_green (ui_ram_preview). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/tests/ui_ram_preview.rs | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/crates/ui-egui/tests/ui_ram_preview.rs b/crates/ui-egui/tests/ui_ram_preview.rs index df62c028..84610b7c 100644 --- a/crates/ui-egui/tests/ui_ram_preview.rs +++ b/crates/ui-egui/tests/ui_ram_preview.rs @@ -52,6 +52,36 @@ fn an_edit_in_another_comp_keeps_the_cached_frames_and_undo_finds_them_again() { assert!(h.state().frames.is_cached(&effectcraft_ui_egui::frames::FrameKey { frame: 0, ..b_frame })); } +/// #284: the Timeline shows the cached frames as a green line under the work area bar, from +/// the first frame rendered on. +#[test] +fn the_timeline_shows_the_cached_frames_in_green() { + let mut s = Session::default(); + s.execute("comp.new", json!({"name": "Cache", "width": 64, "height": 36, "duration": 2})).unwrap(); + s.execute("layer.newSolid", json!({"color": "#3080ff"})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_| EffectcraftApp::new(s)); + settle(&mut h); + h.run_steps(2); + let green = h.state().tokens.cache_green; + let bar = h.state().auto.find("timeline.workArea.bar").unwrap().rect; + fn rects(s: &egui::Shape, out: &mut Vec) { + match s { + egui::Shape::Rect(r) => out.push(r.clone()), + egui::Shape::Vec(v) => v.iter().for_each(|s| rects(s, out)), + _ => {} + } + } + let mut all = vec![]; + for c in &h.output().shapes { + rects(&c.shape, &mut all); + } + let line: Vec = all.iter().filter(|r| r.fill == green).map(|r| r.rect).collect(); + assert!(!line.is_empty(), "a green cache line is drawn"); + // Under the work area bar, at its start (frame 0 is cached), a visible width. + let under = |r: &egui::Rect| r.min.y >= bar[1] + bar[3] - 1.0 && r.max.y <= bar[1] + bar[3] + 8.0; + assert!(line.iter().any(|r| under(r) && (r.min.x - bar[0]).abs() < 2.0 && r.width() >= 1.0), "{line:?} bar {bar:?}"); +} + /// Issue #65: while edits keep coming (a layer dragged in the viewer), only the viewer's frame /// renders; prefetching the frames around it waits until the edits pause (they would be stale at /// the next step, and would hold the CPU and GPU the next viewer frame needs), then resumes. From 59c55e8a267a11c23ccad8490e76887fb89dab44 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:14:52 +0700 Subject: [PATCH 05/10] Timeline: dragging over stopwatches sets them all (#284) As in After Effects, pressing a property's stopwatch toggles it, and dragging on over the stopwatches above or below gives every property the pointer passes the state the first one got (animated or not), whatever its own, in one undo step. Alt+click still adds an expression. The gesture is the layer switches' press-and-drag from #238, moved into `switch_gesture` and shared by both. `prop.toggleAnimation` takes `merge` so the drag folds into one undo step. Tests: dragging_over_stopwatches_sets_them_all (ui_timeline_switches), stopwatch_toggles_with_a_merge_key_are_one_undo_step (engine). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/prop.rs | 4 +- crates/engine/src/tests_timeline.rs | 13 +++++ crates/ui-egui/src/panels/timeline.rs | 57 +++++++++++--------- crates/ui-egui/tests/ui_timeline_switches.rs | 48 +++++++++++++++++ 4 files changed, 95 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/commands/prop.rs b/crates/engine/src/commands/prop.rs index 7babf85d..d50c4353 100644 --- a/crates/engine/src/commands/prop.rs +++ b/crates/engine/src/commands/prop.rs @@ -144,7 +144,7 @@ fn get(s: &mut Session, p: &Value) -> Result { fn toggle_anim(s: &mut Session, p: &Value) -> Result { let (cid, lid, uid) = prop_ref(s, p, "prop.toggleAnimation")?; let on = b_p(p, "value"); - with_prop(s, "Toggle Animation", None, cid, lid, uid, |pr, lt| { + with_prop(s, "Toggle Animation", merge_p(p), cid, lid, uid, |pr, lt| { if pr.static_only { return Err(bad("prop.toggleAnimation", "this property can't be animated")); } @@ -769,7 +769,7 @@ pub fn specs() -> Vec { vec![ query!("prop.get", "Get Property", "{layer?, path|prop, time? (comp s)}", get), cmd!("prop.set", "Set Property Value", [], None, "{layer?, path|prop, value, time?, merge?}", has_layers, set), - cmd!("prop.toggleAnimation", "Toggle Stopwatch", [], None, "{layer?, path|prop, value?}", has_layers, toggle_anim), + cmd!("prop.toggleAnimation", "Toggle Stopwatch", [], None, "{layer?, path|prop, value?, merge?}", has_layers, toggle_anim), cmd!("prop.addKey", "Add Keyframe", [], None, "{layer?, path|prop, time?, value?}", has_layers, add_key), cmd!("prop.toggleKey", "Add or Remove Keyframe at Current Time", [], None, "{layer?, path|prop}", has_layers, toggle_key), cmd!( diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index d33a3c1d..03b91c28 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -428,6 +428,19 @@ fn parenting_keeps_the_layer_in_place() { assert!(s.active_comp().unwrap().layer(effectcraft_project::LayerId(child)).unwrap().parent.is_some()); } +/// #284: toggling stopwatches with one `merge` key (dragging over them) is one undo step. +#[test] +fn stopwatch_toggles_with_a_merge_key_are_one_undo_step() { + let (mut s, l) = setup(); + let steps = s.history.undo.len(); + for path in ["transform/position", "transform/scale", "transform/opacity"] { + s.execute("prop.toggleAnimation", json!({"layer": l, "path": path, "value": true, "merge": "drag"})).unwrap(); + } + assert_eq!(s.history.undo.len(), steps + 1); + s.execute("edit.undo", json!({})).unwrap(); + assert!(["transform/position", "transform/scale", "transform/opacity"].iter().all(|p| !prop(&s, l, p).is_animated())); +} + #[test] fn pick_whip_generates_ae_reference_expressions() { let (mut s, l) = setup(); diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 96a4b686..457a9396 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -980,8 +980,8 @@ fn key_drag_id() -> egui::Id { egui::Id::new("tl-key-drag") } -/// A layer switch pressed and dragged over other layers: the switch, the state the press gave -/// it, and the undo step the drag folds into. +/// A switch pressed and dragged over other rows: the switch, the state the press gave it, and +/// the undo step the drag folds into. #[derive(Clone, Debug)] struct SwitchDrag { switch: &'static str, @@ -993,9 +993,24 @@ fn switch_drag_id() -> egui::Id { egui::Id::new("tl-switch-drag") } -/// A layer's switch (A/V Features and Switches columns) in `row`. Pressing it sets it, and -/// dragging on gives the same switch of every layer the pointer passes the same new state, in -/// one undo step (After Effects). +/// The press-and-drag gesture of the Timeline's switches and stopwatches (`switch`, on `row`, +/// now `on`, pressed through `resp`): the press gives it the opposite state, and dragging on +/// gives every `switch` the pointer passes that same state, in one undo step, whatever its own +/// (After Effects). The state to set and the undo step to fold it into, when it changes now. +fn switch_gesture(ui: &egui::Ui, resp: &egui::Response, row: Rect, switch: &'static str, on: bool) -> Option<(bool, String)> { + if resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) { + let merge = format!("tl-switch-{}", ui.input(|i| i.time)); + ui.data_mut(|d| d.insert_temp(switch_drag_id(), SwitchDrag { switch, value: !on, merge: merge.clone() })); + return Some((!on, merge)); + } + let d = ui.data(|d| d.get_temp::(switch_drag_id())).filter(|d| d.switch == switch && d.value != on)?; + // The heights the pointer moved over since the last frame, so a quick drag skips no row. + let (a, b) = ui.input(|i| i.pointer.interact_pos().map(|p| (p.y - i.pointer.delta().y, p.y)))?; + (a.min(b) < row.max.y && a.max(b) >= row.min.y).then_some((d.value, d.merge)) +} + +/// A layer's switch (A/V Features and Switches columns) in `row`, with the +/// [`switch_gesture`]. #[allow(clippy::too_many_arguments)] fn layer_switch( ui: &mut egui::Ui, @@ -1010,20 +1025,8 @@ fn layer_switch( actions: &mut Vec<(String, serde_json::Value)>, ) { let resp = widgets::icon_toggle(ui, br, icon, on, t, id, Sense::click_and_drag()); - let set = |actions: &mut Vec<(String, serde_json::Value)>, value: bool, merge: &str| { - actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name, "value": value, "merge": merge}))) - }; - if resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) { - let merge = format!("tl-switch-{}", ui.input(|i| i.time)); - set(actions, !on, &merge); - ui.data_mut(|d| d.insert_temp(switch_drag_id(), SwitchDrag { switch: name, value: !on, merge })); - } else if let Some(d) = ui.data(|d| d.get_temp::(switch_drag_id())).filter(|d| d.switch == name && d.value != on) - // The heights the pointer moved over since the last frame, so a quick drag skips no row. - && let Some((a, b)) = ui.input(|i| i.pointer.interact_pos().map(|p| (p.y - i.pointer.delta().y, p.y))) - && a.min(b) < row.max.y - && a.max(b) >= row.min.y - { - set(actions, d.value, &d.merge); + if let Some((value, merge)) = switch_gesture(ui, &resp, row, name, on) { + actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name, "value": value, "merge": merge}))); } else if resp.clicked() && !resp.clicked_by(egui::PointerButton::Primary) { // Keyboard or accessibility activation (a pointer press was handled above). actions.push(("layer.setSwitch".into(), json!({"layers": [layer], "switch": name}))); @@ -2161,10 +2164,11 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } app.auto.add(&format!("timeline.prop.{uid}.addKey"), mid, "Add or remove keyframe"); } - // Stopwatch. + // Stopwatch: Alt+click adds an expression; a press toggles the animation, and + // dragging on over other stopwatches gives them the same state (#284). if !prop.static_only { let swr = Rect::from_center_size(pos2(indent, cy), vec2(15.0, 15.0)); - let resp = ui.interact(swr, egui::Id::new(("stopwatch", uid)), Sense::click()); + let resp = ui.interact(swr, egui::Id::new(("stopwatch", uid)), Sense::click_and_drag()); let col = if prop.is_animated() { t.hot_text } else if resp.hovered() { @@ -2174,12 +2178,15 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { }; icons::paint(&lp, swr, Icon::Stopwatch, col); app.auto.add(&format!("timeline.prop.{uid}.stopwatch"), swr, &prop.name); - if resp.clicked() { - if ui.input(|i| i.modifiers.alt) { + if ui.input(|i| i.modifiers.alt) { + if resp.clicked() { actions.push(("prop.setExpression".into(), json!({"layer": layer.id.0, "prop": uid}))); - } else { - actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid}))); } + } else if let Some((value, merge)) = switch_gesture(ui, &resp, r, "stopwatch", prop.is_animated()) { + actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid, "value": value, "merge": merge}))); + } else if resp.clicked() && !resp.clicked_by(egui::PointerButton::Primary) { + // Keyboard or accessibility activation. + actions.push(("prop.toggleAnimation".into(), json!({"layer": layer.id.0, "prop": uid}))); } } let name_x = indent + 12.0; diff --git a/crates/ui-egui/tests/ui_timeline_switches.rs b/crates/ui-egui/tests/ui_timeline_switches.rs index 501d2096..a37125b7 100644 --- a/crates/ui-egui/tests/ui_timeline_switches.rs +++ b/crates/ui-egui/tests/ui_timeline_switches.rs @@ -96,3 +96,51 @@ fn dragging_over_layer_switches_sets_them_all() { assert_eq!(states(&h, &ids, motion_blur), [false, true, true, true]); assert_eq!(states(&h, &ids, video), [false, true, false, true], "other switches untouched"); } + +/// #284: pressing a stopwatch and dragging over the stopwatches below gives every property the +/// state the first one got (animated or not), in one undo step; Alt+click still adds an +/// expression and a click toggles one property. +#[test] +fn dragging_over_stopwatches_sets_them_all() { + let (mut h, ids) = harness(); + // Layer A twirled open to its Transform properties. + let (layer, uids) = { + let comp = h.state().session.active_comp().unwrap(); + let l = comp.layer(LayerId(ids[0])).unwrap(); + let tr = l.props.sub("transform").unwrap(); + let uids: Vec = ["anchor", "position", "scale", "rotation", "opacity"].iter().map(|m| tr.get(m).unwrap().uid).collect(); + (l.id, (tr.uid, uids)) + }; + let (group, uids) = uids; + h.state_mut().ui.timeline.open_layers.insert(layer.0); + h.state_mut().ui.timeline.open_groups.insert(group); + h.run_steps(3); + let animated = |h: &Harness<'_, EffectcraftApp>| -> Vec { + let comp = h.state().session.active_comp().unwrap(); + let l = comp.layer(layer).unwrap(); + uids.iter().map(|u| l.props.find(*u).unwrap().is_animated()).collect() + }; + let sw = |h: &Harness<'_, EffectcraftApp>, i: usize| rect(h, &format!("timeline.prop.{}.stopwatch", uids[i])).center(); + let steps = h.state().session.history.undo.len(); + + // Scale animated first; from Anchor Point down to Opacity: all animated, one undo step. + h.state_mut().session.execute("prop.toggleAnimation", json!({"layer": layer.0, "prop": uids[2]})).unwrap(); + h.run_steps(2); + let steps = steps + 1; + let (a, o) = (sw(&h, 0), sw(&h, 4)); + drag(&mut h, a, o, 10); + assert_eq!(animated(&h), [true; 5]); + assert_eq!(h.state().session.history.undo.len(), steps + 1, "one undo step"); + // From Rotation (animated → not) up to Position, quickly: those three stop animating. + let (r, p) = (sw(&h, 3), sw(&h, 1)); + drag(&mut h, r, p, 2); + assert_eq!(animated(&h), [true, false, false, false, true]); + // A click toggles only that property. + let s = sw(&h, 2); + drag(&mut h, s, s, 1); + assert_eq!(animated(&h), [true, false, true, false, true]); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.state_mut().session.execute("edit.undo", json!({})).unwrap(); + h.run_steps(2); + assert_eq!(animated(&h), [true; 5], "each drag was one step"); +} From 04ccf6f906a2a36751b851e89ff450e5324f18eb Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:15:17 +0700 Subject: [PATCH 06/10] Timeline: keyframes without a dark outline (#284) Each half of a keyframe glyph had a 1 px near-black border, so keys that overlap read as a tangle of borders. Keys are now flat, as in After Effects: each half is filled and edged in the key's own colour (the hairline closes the seam between the halves), so overlapping keys read as one shape. The Light theme's keyframe grey is darker so unselected keys still stand out on its light time graph without the border. Test: keyframes_are_drawn_without_a_dark_outline (icons). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/icons.rs | 41 ++++++++++++++++++++++++--- crates/ui-egui/src/panels/timeline.rs | 2 +- crates/ui-egui/src/theme.rs | 2 ++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/icons.rs b/crates/ui-egui/src/icons.rs index de2c2b9d..1068acab 100644 --- a/crates/ui-egui/src/icons.rs +++ b/crates/ui-egui/src/icons.rs @@ -641,13 +641,24 @@ pub fn keyframe( left: effectcraft_engine::keyframe::KeyHalf, right: effectcraft_engine::keyframe::KeyHalf, fill: Color32, - outline: Color32, ) { + painter.extend(keyframe_shapes(center, size, left, right, fill)); +} + +/// The halves of a [`keyframe`] glyph. Flat, without a dark outline (as in After Effects), so +/// keys that overlap read as one shape instead of a tangle of borders (#284); the hairline in +/// the fill colour closes the seam between the halves. +fn keyframe_shapes( + center: Pos2, + size: f32, + left: effectcraft_engine::keyframe::KeyHalf, + right: effectcraft_engine::keyframe::KeyHalf, + fill: Color32, +) -> Vec { use effectcraft_engine::keyframe::KeyHalf as H; let r = size / 2.0; - let half = |pts: Vec| { - painter.add(PathShape::convex_polygon(pts, fill, Stroke::new(1.0, outline))); - }; + let mut out = Vec::with_capacity(2); + let mut half = |pts: Vec| out.push(PathShape::convex_polygon(pts, fill, Stroke::new(0.5, fill)).into()); let c = center; for (side, h) in [(-1.0f32, left), (1.0, right)] { match h { @@ -678,4 +689,26 @@ pub fn keyframe( } } } + out +} + +#[cfg(test)] +mod tests { + use effectcraft_engine::keyframe::KeyHalf as H; + + /// #284: keyframes have no dark border (overlapping keys looked messy): each half is filled + /// and edged in the key's own colour. + #[test] + fn keyframes_are_drawn_without_a_dark_outline() { + let fill = egui::Color32::from_gray(0xc0); + for (l, r) in [(H::Linear, H::Linear), (H::Bezier, H::Hold), (H::Auto, H::Linear)] { + let shapes = super::keyframe_shapes(egui::pos2(10.0, 10.0), 11.0, l, r, fill); + assert_eq!(shapes.len(), 2); + for s in shapes { + let egui::Shape::Path(p) = s else { panic!("a path: {s:?}") }; + assert_eq!(p.fill, fill); + assert!(p.stroke.width <= 0.5 && p.stroke.color == egui::epaint::ColorMode::Solid(fill), "{:?}", p.stroke); + } + } + } } diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index 457a9396..bdedd202 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -2255,7 +2255,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // Roving keys: a small dot (their time follows their neighbours). gp.circle_filled(c, 2.5, kcol); } else { - icons::keyframe(&gp, c, 11.0, icon.left, icon.right, kcol, Color32::from_black_alpha(200)); + icons::keyframe(&gp, c, 11.0, icon.left, icon.right, kcol); } let kr = Rect::from_center_size(c, vec2(12.0, 14.0)); let kresp = ui.interact(kr, egui::Id::new(("key", uid, k.time.0)), Sense::click_and_drag()); diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index 1a9e1a1a..bfe4de75 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -187,6 +187,8 @@ impl Tokens { tl_ruler_bg: Color32::from_rgb(0xe2, 0xe2, 0xe2), tl_ruler_text: Color32::from_rgb(0x50, 0x50, 0x50), pasteboard: Color32::from_rgb(0xa8, 0xa8, 0xa8), + // Keyframes have no outline: dark enough to stand out on the light time graph. + keyframe: Color32::from_rgb(0x6c, 0x6c, 0x6c), ..dark }, } From cf7263d6c65ef21bcb557bc17adbee35078f397a Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:15:34 +0700 Subject: [PATCH 07/10] Graph Editor: a handle drag moves the handles of every selected key (#284) Dragging a Bezier handle in the Graph Editor changed only that key's ease. As in After Effects, the same side's handles of every selected key now move with it, by as much as the pointer moved (time and value or speed alike), in one undo step. Each handle keeps its offset from where the drag began, so a handle stopped at a limit (100 % influence) doesn't drag the others on. Test: a_graph_handle_drag_moves_the_handles_of_every_selected_key (ui_keyframes). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/graph.rs | 95 +++++++++++++++++++++++----- crates/ui-egui/tests/ui_keyframes.rs | 40 ++++++++++++ 2 files changed, 120 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/panels/graph.rs b/crates/ui-egui/src/panels/graph.rs index 0b03ef39..8d7bcbdd 100644 --- a/crates/ui-egui/src/panels/graph.rs +++ b/crates/ui-egui/src/panels/graph.rs @@ -306,28 +306,46 @@ pub(crate) fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, p: &egui::Painte let hr = Rect::from_center_size(hp, vec2(10.0, 10.0)); let hresp = ui.interact(hr, egui::Id::new(("ghandle", uid, d, k.idx, side)), Sense::drag()); 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() { + let grabs: Vec = cs + .iter() + .flat_map(|c2| { + c2.keys.iter().flat_map(move |k2| { + k2.handles.iter().filter(|h2| h2.out == h.out).map(move |h2| HandleGrab { + layer: c2.layer.id, + prop: c2.prop.uid, + dim: c2.editable.then_some(c2.dim), + time: k2.time, + out: h2.out, + key: (k2.t, k2.v), + end: (h2.t, h2.v), + negative: h2.speed < 0.0, + dur: h2.dur, + }) + }) + }) + .collect(); + 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() && let Some(pt) = hresp.interact_pointer_pos() + && let Some((press, grabs)) = ctx.data(|dd| dd.get_temp::(handle_drag_id())) { ctx.data_mut(|dd| dd.insert_temp(drag_id, true)); - let mut dt = (tm.t(pt.x) - k.t) * if h.out { 1.0 } else { -1.0 }; - dt = dt.max(h.dur * 0.001); - let influence = (dt / h.dur.max(1e-9) * 100.0).clamp(0.1, 100.0); - let sp = if speed { - let mag = vmap(pt.y).max(0.0); - if h.speed < 0.0 { -mag } else { mag } - } else { - let dv = vmap(pt.y) - k.v; - dv / dt * if h.out { 1.0 } else { -1.0 } - }; - let mut params = json!({"layer": lid.0, "prop": uid, "time": k.time.seconds(), "side": side, "speed": sp, "influence": influence, "merge": format!("ghandle-{uid}-{}", k.idx)}); - if c.editable { - params["dim"] = json!(d); + let (dt, dv) = (tm.t(pt.x) - tm.t(press.x), vmap(pt.y) - vmap(press.y)); + let merge = format!("ghandle-{uid}-{}", k.idx); + for g in &grabs { + actions.push(("keys.setEase".into(), g.ease(g.end.0 + dt, g.end.1 + dv, speed, &merge))); } - actions.push(("keys.setEase".into(), params)); } if hresp.drag_stopped() { - ctx.data_mut(|dd| dd.insert_temp(drag_id, false)); + ctx.data_mut(|dd| { + dd.insert_temp(drag_id, false); + dd.remove::(handle_drag_id()); + }); actions.push(("__endMerge".into(), json!({}))); } } @@ -558,3 +576,50 @@ struct KeyGrab { fn key_grab_id() -> egui::Id { egui::Id::new("graph-key-grab") } + +/// A Bezier handle moving with a handle drag: its key, side and dimension, the key's place +/// (comp seconds, graph value) and the handle's end when the drag began. +#[derive(Clone, Debug)] +struct HandleGrab { + layer: LayerId, + prop: u64, + /// `None` for a property drawn as one curve (spatial). + dim: Option, + time: Tick, + out: bool, + key: (f64, f64), + end: (f64, f64), + /// The speed was negative (the speed graph shows its magnitude). + negative: bool, + /// Segment duration (layer seconds) the influence is relative to. + dur: f64, +} + +impl HandleGrab { + /// `keys.setEase` putting the handle's end at (`t` comp seconds, `v` in the graph). + fn ease(&self, t: f64, v: f64, speed: bool, merge: &str) -> serde_json::Value { + let dir = if self.out { 1.0 } else { -1.0 }; + let dt = ((t - self.key.0) * dir).max(self.dur * 0.001); + let influence = (dt / self.dur.max(1e-9) * 100.0).clamp(0.1, 100.0); + let sp = if speed { + let mag = v.max(0.0); + if self.negative { -mag } else { mag } + } else { + (v - self.key.1) / dt * dir + }; + let side = if self.out { "out" } else { "in" }; + let mut params = + json!({"layer": self.layer.0, "prop": self.prop, "time": self.time.seconds(), "side": side, "speed": sp, "influence": influence, "merge": merge}); + if let Some(d) = self.dim { + params["dim"] = json!(d); + } + params + } +} + +/// A handle drag: where the button went down, and the handles that move. +type HandleDrag = (Pos2, Vec); + +fn handle_drag_id() -> egui::Id { + egui::Id::new("graph-handle-drag") +} diff --git a/crates/ui-egui/tests/ui_keyframes.rs b/crates/ui-egui/tests/ui_keyframes.rs index 0d0846f7..82805f60 100644 --- a/crates/ui-egui/tests/ui_keyframes.rs +++ b/crates/ui-egui/tests/ui_keyframes.rs @@ -293,3 +293,43 @@ fn clicking_an_ease_preset_eases_the_selected_keys_and_handles_edit_the_curve() assert!((applied * 100.0 - out).abs() < 1e-9, "{applied} vs {out}"); assert_eq!(h.state().session.history.undo.len(), steps + 2); } + +/// #284: in the Graph Editor, dragging the handle of one selected key moves the same handle of +/// every selected key with it (After Effects), in one undo step. +#[test] +fn a_graph_handle_drag_moves_the_handles_of_every_selected_key() { + let (mut h, id, uid) = harness(); + let s = &mut h.state_mut().session; + s.execute("prop.addKey", json!({"layer": id.0, "path": "transform/opacity", "time": 2.0, "value": 0.0})).unwrap(); + s.execute("prop.select", json!({"layer": id.0, "prop": uid})).unwrap(); + s.execute("keys.selectAll", json!({})).unwrap(); + s.execute("keys.easyEase", json!({})).unwrap(); + h.state_mut().ui.timeline.graph_editor = true; + h.run_steps(4); + // (in, out) influence of each key. + let influences = |h: &Harness<'_, EffectcraftApp>| -> Vec<(f64, f64)> { + let l = h.state().session.active_comp().unwrap().layer(id).unwrap().clone(); + let keys = l.props.prop("transform/opacity").unwrap().keys.clone(); + keys.iter().map(|k| (k.in_ease.first().map_or(0.0, |e| e.influence), k.out_ease.first().map_or(0.0, |e| e.influence))).collect() + }; + let before = influences(&h); + let handle = |h: &Harness<'_, EffectcraftApp>, key: usize| { + let e = h.state().auto.find(&format!("timeline.graph.handle.{uid}.0.{key}.out")).unwrap_or_else(|| panic!("no handle {key}")).clone(); + pos2(e.rect[0] + e.rect[2] / 2.0, e.rect[1] + e.rect[3] / 2.0) + }; + let px_per_s = { + let at = |i: usize| h.state().auto.find(&format!("timeline.graph.key.{uid}.0.{i}")).unwrap().rect[0]; + at(1) - at(0) + }; + let (k0, k1) = (handle(&h, 0), handle(&h, 1)); + let undo = h.state().session.history.undo.len(); + // Key 1's out handle 0.2 s further out (the keys are 1 s apart): key 0's goes along. + drag(&mut h, k1, k1 + egui::vec2(px_per_s * 0.2, 0.0), Default::default()); + let after = influences(&h); + for i in [0, 1] { + assert!((after[i].1 - before[i].1 - 0.2).abs() < 0.01, "key {i}: {:?} → {:?}", before[i], after[i]); + } + assert_eq!((after[1].0, after[2].0), (before[1].0, before[2].0), "the in handles stay"); + assert_eq!(h.state().session.history.undo.len(), undo + 1, "one undo step"); + assert!((handle(&h, 0).x - k0.x - px_per_s * 0.2).abs() < 2.0, "key 0's handle moved on screen"); +} From 065f0c91e7c7ea44d1e47422e9e10064b4c93273 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:16:03 +0700 Subject: [PATCH 08/10] Viewer: the eyedropper samples under the pointer in a region of interest (#284) Effect eyedroppers (other than keyers', which pick from the effect's input) sample the composite the viewer shows; a test covers that on main. But they, and the Info panel, read the shown frame as if it covered the whole comp. With a Region of Interest, or the Extended Viewer of a 3D view, the frame covers only that area, so they read a pixel somewhere else. They now map the point through the area the frame covers (`shown_region`, `frame_pixel`). Tests: the_eyedropper_samples_under_the_pointer_in_a_region_of_interest (ui_effect_controls), eyedropper_samples_straight_colour (fx_widgets). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/effect_controls.rs | 4 +- crates/ui-egui/src/panels/fx_widgets.rs | 41 +++++++++++++------- crates/ui-egui/src/panels/info.rs | 13 ++----- crates/ui-egui/src/panels/viewer_tools.rs | 6 +++ crates/ui-egui/tests/ui_effect_controls.rs | 22 +++++++++++ 5 files changed, 60 insertions(+), 26 deletions(-) diff --git a/crates/ui-egui/src/panels/effect_controls.rs b/crates/ui-egui/src/panels/effect_controls.rs index b9711579..55a08632 100644 --- a/crates/ui-egui/src/panels/effect_controls.rs +++ b/crates/ui-egui/src/panels/effect_controls.rs @@ -1167,7 +1167,7 @@ pub fn viewer_hook( if pick.kind == "color" && keyer.is_none() && let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, map.comp, c) + && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), c) { let sw = Rect::from_min_size(hp + vec2(14.0, 14.0), vec2(26.0, 18.0)); painter.rect_filled(sw, 2.0, Color32::from_rgb((s[0] * 255.0) as u8, (s[1] * 255.0) as u8, (s[2] * 255.0) as u8)); @@ -1192,7 +1192,7 @@ pub fn viewer_hook( )); } } else if let Some(img) = &app.viewer_image - && let Some(s) = fw::sample_frame(img, map.comp, c) + && let Some(s) = fw::sample_frame(img, super::viewer_tools::shown_region(&ctx, map.comp), c) { actions.push(("prop.set".into(), json!({"layer": pick.layer, "prop": pick.prop, "value": [s[0], s[1], s[2], 1.0]}))); } diff --git a/crates/ui-egui/src/panels/fx_widgets.rs b/crates/ui-egui/src/panels/fx_widgets.rs index 644e64bc..036ae070 100644 --- a/crates/ui-egui/src/panels/fx_widgets.rs +++ b/crates/ui-egui/src/panels/fx_widgets.rs @@ -185,18 +185,23 @@ pub fn layer_to_comp(l2c: &Mat3, p: [f64; 2]) -> [f64; 2] { [v.x, v.y] } -/// Straight RGBA (0..1) of the displayed frame at comp point `pt` (the frame may be rendered -/// at a reduced resolution: `comp` is the comp size in pixels). `None` outside the frame. -pub fn sample_frame(img: &egui::ColorImage, comp: [f32; 2], pt: [f64; 2]) -> Option<[f32; 4]> { - if comp[0] <= 0.0 || comp[1] <= 0.0 || pt[0] < 0.0 || pt[1] < 0.0 { +/// The premultiplied pixel of the displayed frame at comp point `pt`. The frame covers +/// `region` ([x, y, w, h] in comp pixels: the whole comp, or the region of interest or Extended +/// Viewer area it was rendered for), maybe at a reduced resolution. `None` outside it. +pub fn frame_pixel(img: &egui::ColorImage, region: [f64; 4], pt: [f64; 2]) -> Option { + let [x, y, w, h] = region; + let (fx, fy) = ((pt[0] - x) / w, (pt[1] - y) / h); + if !(0.0..1.0).contains(&fx) || !(0.0..1.0).contains(&fy) { return None; } - let sx = (pt[0] / comp[0] as f64 * img.size[0] as f64).floor() as usize; - let sy = (pt[1] / comp[1] as f64 * img.size[1] as f64).floor() as usize; - if sx >= img.size[0] || sy >= img.size[1] { - return None; - } - let c = img.pixels[sy * img.size[0] + sx]; + let sx = ((fx * img.size[0] as f64).floor() as usize).min(img.size[0].saturating_sub(1)); + let sy = ((fy * img.size[1] as f64).floor() as usize).min(img.size[1].saturating_sub(1)); + img.pixels.get(sy.checked_mul(img.size[0])?.checked_add(sx)?).copied() +} + +/// Straight RGBA (0..1) of the displayed frame at comp point `pt` ([`frame_pixel`]). +pub fn sample_frame(img: &egui::ColorImage, region: [f64; 4], pt: [f64; 2]) -> Option<[f32; 4]> { + let c = frame_pixel(img, region, pt)?; let a = c.a() as f32 / 255.0; if a <= 0.0 { return Some([0.0, 0.0, 0.0, 0.0]); @@ -639,12 +644,20 @@ mod tests { img.pixels[4 + 3] = Color32::from_rgba_premultiplied(128, 64, 0, 128); // (3, 1), half alpha img.pixels[0] = Color32::from_rgb(255, 0, 0); // The frame is rendered at half resolution of an 8 × 4 comp. - let c = sample_frame(&img, [8.0, 4.0], [7.5, 3.0]).unwrap(); + let comp = [0.0, 0.0, 8.0, 4.0]; + let c = sample_frame(&img, comp, [7.5, 3.0]).unwrap(); assert!((c[0] - 1.0).abs() < 0.01 && (c[1] - 0.5).abs() < 0.01 && c[2] == 0.0, "{c:?}"); assert!((c[3] - 128.0 / 255.0).abs() < 1e-6); - assert_eq!(sample_frame(&img, [8.0, 4.0], [0.5, 0.5]), Some([1.0, 0.0, 0.0, 1.0])); - assert_eq!(sample_frame(&img, [8.0, 4.0], [8.0, 1.0]), None, "outside"); - assert_eq!(sample_frame(&img, [8.0, 4.0], [-1.0, 1.0]), None); + assert_eq!(sample_frame(&img, comp, [0.5, 0.5]), Some([1.0, 0.0, 0.0, 1.0])); + assert_eq!(sample_frame(&img, comp, [8.0, 1.0]), None, "outside"); + assert_eq!(sample_frame(&img, comp, [-1.0, 1.0]), None); + assert_eq!(sample_frame(&img, [0.0, 0.0, 0.0, 4.0], [0.0, 1.0]), None, "an empty region"); + // A frame of the region of interest (16, 8) to (24, 12) of a bigger comp (#284: it was + // read as if it covered the whole comp). + let roi = [16.0, 8.0, 8.0, 4.0]; + assert_eq!(sample_frame(&img, roi, [16.5, 8.5]), Some([1.0, 0.0, 0.0, 1.0])); + assert!(sample_frame(&img, roi, [23.5, 11.0]).is_some_and(|c| c[3] > 0.4 && c[3] < 0.6)); + assert_eq!(sample_frame(&img, roi, [0.5, 0.5]), None, "outside the region"); } #[test] diff --git a/crates/ui-egui/src/panels/info.rs b/crates/ui-egui/src/panels/info.rs index 37d6a8a7..b792538b 100644 --- a/crates/ui-egui/src/panels/info.rs +++ b/crates/ui-egui/src/panels/info.rs @@ -17,16 +17,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } let (rgba, xy) = match (app.pointer_comp, &app.viewer_image) { (Some([cx, cy]), Some(img)) => { - let comp = app.session.active_comp(); - let (cw, ch) = comp.map(|c| (c.width as f32, c.height as f32)).unwrap_or((1.0, 1.0)); - let sx = (cx / cw * img.size[0] as f32) as i64; - let sy = (cy / ch * img.size[1] as f32) as i64; - let px = if sx >= 0 && sy >= 0 && (sx as usize) < img.size[0] && (sy as usize) < img.size[1] { - Some(img.pixels[sy as usize * img.size[0] + sx as usize]) - } else { - None - }; - (px, Some((cx, cy))) + let comp = app.session.active_comp().map_or([1.0, 1.0], |c| [c.width as f32, c.height as f32]); + let region = super::viewer_tools::shown_region(ui.ctx(), comp); + (super::fx_widgets::frame_pixel(img, region, [cx as f64, cy as f64]), Some((cx, cy))) } _ => (None, None), }; diff --git a/crates/ui-egui/src/panels/viewer_tools.rs b/crates/ui-egui/src/panels/viewer_tools.rs index 63f23356..70bb8464 100644 --- a/crates/ui-egui/src/panels/viewer_tools.rs +++ b/crates/ui-egui/src/panels/viewer_tools.rs @@ -325,6 +325,12 @@ fn texture_roi(ctx: &egui::Context) -> Option<[f64; 4]> { ctx.data(|d| d.get_temp::>(egui::Id::new("viewer-tex-roi"))).flatten() } +/// The comp area ([x, y, w, h], comp pixels) the shown frame covers: its region of interest or +/// Extended Viewer area, else the whole `comp` (width, height). +pub(crate) fn shown_region(ctx: &egui::Context, comp: [f32; 2]) -> [f64; 4] { + texture_roi(ctx).unwrap_or([0.0, 0.0, comp[0] as f64, comp[1] as f64]) +} + /// Show Snapshot: the stored snapshot is on screen (button or F5 held). pub(crate) fn showing_snapshot(app: &EffectcraftApp, ctx: &egui::Context) -> bool { let f5 = ctx.input(|i| i.key_down(egui::Key::F5)) && !ctx.egui_wants_keyboard_input(); diff --git a/crates/ui-egui/tests/ui_effect_controls.rs b/crates/ui-egui/tests/ui_effect_controls.rs index a31e9315..5cf9f518 100644 --- a/crates/ui-egui/tests/ui_effect_controls.rs +++ b/crates/ui-egui/tests/ui_effect_controls.rs @@ -167,6 +167,28 @@ fn crosshair_and_eyedropper_pick_from_the_viewer() { assert_eq!(c[3], 1.0); } +/// #284: with a Region of Interest the shown frame covers only that region; the eyedropper +/// samples the composite under the pointer there (it read the frame as if it covered the whole +/// comp, so it sampled somewhere else). +#[test] +fn the_eyedropper_samples_under_the_pointer_in_a_region_of_interest() { + let (mut app, x) = app(); + app.session.execute("view.setRegionOfInterest", json!({"rect": [0.0, 0.0, 320.0, 180.0]})).unwrap(); + // (Not the red it will pick.) + app.session.execute("prop.set", json!({"layer": x.small, "prop": x.color, "value": [0.0, 0.0, 1.0, 1.0]})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1700.0, 1100.0)).build_eframe(|_| app); + settle(&mut h); + h.state_mut().ui.fx_pick = Some(FxPick { kind: "color".into(), layer: x.small, prop: x.color, name: "Color".into() }); + h.step(); + // (200, 150) is on the red layer (it covers 120..520 × 80..280); read as a frame of the + // whole comp, the green background at (100, 75). + let pos = effectcraft_ui_egui::panels::viewer::comp_to_screen(&h.ctx, [200.0, 150.0]).unwrap(); + click(&mut h, pos); + let v = layer(&h.state().session, x.small).effects().unwrap().find(x.color).unwrap().value.clone(); + let KV::Color(c) = v else { panic!("{v:?}") }; + assert!(c[0] > 0.98 && c[1] < 0.02 && c[2] < 0.02, "{c:?}"); +} + /// Key Light's Screen Colour eyedropper picks from the effect's input: the shown frame is /// already keyed (the default screen colour takes most of a green), so sampling it would miss. #[test] From 02bc2b8f9aa064363064744f786a0b31e00bcdb5 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:16:26 +0700 Subject: [PATCH 09/10] Puppet: the tools stay on the rigged layer and pins go on the deformed mesh (#284) With a Puppet tool, a click off the pins picked the topmost layer under the pointer by its own bounds, and selected it. So a click where the deformation had stretched the mesh past the layer's bounds went to the layer behind (selecting it and giving it a Puppet effect), and any click on another layer switched to it. While selected layers have a Puppet effect, a click (or a press that might start a pin marquee) now works only on them, as in After Effects: on the layer whose mesh, as deformed now, is under the pointer (the pin lands on the deformed shape), else the one whose own art is (a new mesh); elsewhere nothing happens and the selection stays. Without a rigged layer selected, the tools pick and rig the layer under the pointer as before. `puppet::contains` tests a point against a deformed mesh; `puppet.addPin` uses it too (its own test only held for one triangle winding). Test: puppet_pins_go_on_the_deformed_mesh_and_the_tool_stays_on_the_rigged_layer (ui_viewer). Co-Authored-By: Claude Opus 5.5 --- crates/effects/src/puppet/mod.rs | 8 +++++ crates/engine/src/commands/puppet.rs | 6 +--- crates/ui-egui/src/panels/puppet_tool.rs | 14 +++++++++ crates/ui-egui/src/panels/viewer.rs | 17 +++++++++-- crates/ui-egui/tests/ui_viewer.rs | 37 ++++++++++++++++++++++++ 5 files changed, 75 insertions(+), 7 deletions(-) diff --git a/crates/effects/src/puppet/mod.rs b/crates/effects/src/puppet/mod.rs index c7ce9ea0..6ec2ce5b 100644 --- a/crates/effects/src/puppet/mod.rs +++ b/crates/effects/src/puppet/mod.rs @@ -387,6 +387,14 @@ pub fn unmap(mesh: &Mesh, deformed: &[[f64; 2]], p: [f64; 2]) -> [f64; 2] { } } +/// Whether `p` (layer space) is on the mesh as `deformed` places its vertices. +pub fn contains(mesh: &Mesh, deformed: &[[f64; 2]], p: [f64; 2]) -> bool { + mesh.tris.iter().any(|t| match (deformed.get(t[0]), deformed.get(t[1]), deformed.get(t[2])) { + (Some(a), Some(b), Some(c)) => bary(*a, *b, *c, p).is_some_and(|l| l.iter().all(|x| *x >= -1e-9)), + _ => false, + }) +} + #[inline] fn bary(a: [f64; 2], b: [f64; 2], c: [f64; 2], p: [f64; 2]) -> Option<[f64; 3]> { let d = (b[1] - c[1]) * (a[0] - c[0]) + (c[0] - b[0]) * (a[1] - c[1]); diff --git a/crates/engine/src/commands/puppet.rs b/crates/engine/src/commands/puppet.rs index 8d11fc57..cc0c4c6b 100644 --- a/crates/engine/src/commands/puppet.rs +++ b/crates/engine/src/commands/puppet.rs @@ -87,11 +87,7 @@ fn add_pin(s: &mut Session, p: &Value) -> Result { if want_mesh.is_some_and(|w| w != *uid) || mesh.tris.is_empty() { continue; } - let inside = mesh.tris.iter().any(|tr| { - let (a, b, c) = (def[tr[0]], def[tr[1]], def[tr[2]]); - let cr = |o: [f64; 2], u: [f64; 2], v: [f64; 2]| (u[0] - o[0]) * (v[1] - o[1]) - (u[1] - o[1]) * (v[0] - o[0]); - cr(a, b, at) >= 0.0 && cr(b, c, at) >= 0.0 && cr(c, a, at) >= 0.0 - }); + let inside = puppet::contains(mesh, def, at); let near = def.iter().map(|v| (v[0] - at[0]).hypot(v[1] - at[1])).fold(f64::MAX, f64::min) < 12.0; if inside || near || want_mesh.is_some() { target = Some((*uid, puppet::unmap(mesh, def, at))); diff --git a/crates/ui-egui/src/panels/puppet_tool.rs b/crates/ui-egui/src/panels/puppet_tool.rs index 4b0b8e9a..361a914f 100644 --- a/crates/ui-egui/src/panels/puppet_tool.rs +++ b/crates/ui-egui/src/panels/puppet_tool.rs @@ -82,6 +82,20 @@ pub fn overlay(app: &EffectcraftApp, ctx: &egui::Context, cid: ItemId, layer: &L Some(overlay) } +/// The layer a Puppet tool press at comp point `cpt` works on, of the selected `layers` with a +/// Puppet effect (stack order): the one whose mesh, as deformed now, is under it (pins go on the +/// deformed shape, also where it reaches past the layer's own bounds), else the one whose art is +/// (a new mesh there). Never a layer without a Puppet effect: the tools stay on the layers being +/// rigged, as in After Effects (#284). +pub fn target(ectx: &EvalCtx, layers: &[(&Layer, Overlay)], cpt: [f64; 2], on_art: impl Fn(&Layer) -> bool) -> Option { + let on_mesh = |l: &Layer, ov: &Overlay| { + let Some(inv) = l2c(ectx, l).0.inverse() else { return false }; + let p = inv.apply(gv2(cpt[0], cpt[1])); + ov.iter().any(|(_, mesh, def, _)| puppet::contains(mesh, def, [p.x, p.y])) + }; + layers.iter().find(|(l, ov)| on_mesh(l, ov)).or_else(|| layers.iter().find(|(l, _)| on_art(l))).map(|(l, _)| l.id) +} + /// Keep a pin's keyframed Position in view in the Timeline after placing, moving or recording the /// pin in the viewer (#273). pub fn reveal_pin(app: &mut EffectcraftApp, layer: LayerId, pin: u64) { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index b2107570..52172910 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -919,6 +919,9 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { // Puppet pins and meshes of the selected layers (Puppet tools). let mut pin_hits: Vec = vec![]; + // The selected layers with a Puppet effect, top of the stack first (the layers the Puppet + // tools work on). + let mut puppet_layers: Vec<(&Layer, super::puppet_tool::Overlay)> = vec![]; if app.ui.tool.puppet_kind().is_some() { let sel_props: Vec = app.session.state.selected_props.iter().map(|(_, u)| *u).collect(); for lid in &selected { @@ -927,8 +930,10 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { { let hover = ui.input(|i| i.pointer.hover_pos()); pin_hits.extend(super::puppet_tool::draw(&painter, &map, &ectx, l, &ov, app.session.state.puppet.show_mesh, &sel_props, hover)); + puppet_layers.push((l, ov)); } } + puppet_layers.sort_by_key(|(l, _)| comp.index_of(l.id)); for h in &pin_hits { app.auto.add(&format!("viewer.puppetPin.{}", h.pin), Rect::from_center_size(h.pos, vec2(10.0, 10.0)), "Puppet pin"); if let Some(hp) = h.handle { @@ -938,6 +943,14 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } } } + // The layer a Puppet tool press works on: while selected layers are rigged, only those + // ([`super::puppet_tool::target`]); otherwise the layer under the pointer, selected. + let puppet_layer = |app: &mut EffectcraftApp, cpt: [f64; 2]| -> Option { + if puppet_layers.is_empty() { + return pick(app, &ectx, cpt, false); + } + super::puppet_tool::target(&ectx, &puppet_layers, cpt, |l| layers_at(&ectx, cpt).any(|h| h.id == l.id)) + }; // Project items, files and effects dropped on the viewer. super::viewer_drop::show(app, ui, &painter, &map, &ectx); @@ -1121,7 +1134,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { t if t.is_shape() => Some(Gesture::Create { tool: t, start: cpt }), t if t.puppet_kind().is_some() && super::puppet_tool::hit(&pin_hits, press).is_none() => { // Off the pins: Alt or a press outside the art drags a marquee that selects pins. - (mods.alt || pick(app, &ectx, cpt, false).is_none()).then_some(Gesture::PinMarquee { start: press }) + (mods.alt || puppet_layer(app, cpt).is_none()).then_some(Gesture::PinMarquee { start: press }) } t if t.puppet_kind().is_some() => super::puppet_tool::hit(&pin_hits, press).and_then(|(h, part)| { use super::puppet_tool::PinPart; @@ -1557,7 +1570,7 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { if let Some((h, _)) = super::puppet_tool::hit(&pin_hits, pos) { // Click selects the pin; Shift-click adds or removes it. let _ = app.session.execute("puppet.selectPins", json!({"layer": h.layer.0, "pins": [h.pin], "toggle": mods.shift})); - } else if let Some(l) = pick(app, &ectx, cpt, false).and_then(|l| comp.layer(l)) + } else if let Some(l) = puppet_layer(app, cpt).and_then(|l| comp.layer(l)) && let Some(inv) = l2c(&ectx, l).0.inverse() { let lp = inv.apply(gv2(cpt[0], cpt[1])); diff --git a/crates/ui-egui/tests/ui_viewer.rs b/crates/ui-egui/tests/ui_viewer.rs index 61ea76ef..4a84659f 100644 --- a/crates/ui-egui/tests/ui_viewer.rs +++ b/crates/ui-egui/tests/ui_viewer.rs @@ -891,6 +891,43 @@ fn placing_a_pin_opens_its_groups_on_a_twirled_open_layer() { assert_eq!(timeline_keys(&h, pin_keys(&h, id, pin).0).len(), 1, "the new pin's Position key shows"); } +/// #284: the Puppet tools stay on the layer being rigged: a pin goes on its mesh where the +/// deformation has stretched it past the layer's own bounds (over another layer), and a click on +/// another layer neither selects it nor rigs it. +#[test] +fn puppet_pins_go_on_the_deformed_mesh_and_the_tool_stays_on_the_rigged_layer() { + let mut h = harness(); + let s = &mut h.state_mut().session; + s.execute("layer.select", json!({"layers": ["Box"]})).unwrap(); + let (boxl, plate) = (s.state.selected_layers[0], s.active_comp().unwrap().layers[1].id); + s.execute("puppet.addPin", json!({"layer": boxl.0, "position": [10, 40]})).unwrap(); + let right = s.execute("puppet.addPin", json!({"layer": boxl.0, "position": [70, 40]})).unwrap()["pin"].as_u64().unwrap(); + // Stretch the Box to the right: its right edge (layer x 80, comp x 360) moves well past 400. + s.execute("puppet.movePin", json!({"layer": boxl.0, "pin": right, "position": [140, 40]})).unwrap(); + h.state_mut().ui.tool = Tool::Puppet; + h.run_steps(3); + let pins = |h: &mut Harness<'_, EffectcraftApp>| { + let info = h.state_mut().session.execute("puppet.info", json!({"layer": boxl.0})).unwrap(); + info["meshes"].as_array().unwrap().iter().map(|m| m["pins"].as_array().unwrap().len()).sum::() + }; + let rigged = |h: &Harness<'_, EffectcraftApp>, l: LayerId| { + h.state().session.active_comp().unwrap().layer(l).unwrap().effects().is_some_and(|f| f.groups().next().is_some()) + }; + // Comp (380, 180) is layer (100, 40): past the Box's own edge, over the Plate, on the + // stretched mesh. + let at = screen(&h, [380.0, 180.0]); + click(&mut h, at); + assert_eq!(pins(&mut h), 3, "the pin went on the deformed mesh"); + assert_eq!(h.state().session.state.selected_layers, vec![boxl]); + assert!(!rigged(&h, plate), "the Plate got no Puppet"); + // Far from the Box, on the Plate only: nothing. + let at = screen(&h, [40.0, 40.0]); + click(&mut h, at); + assert_eq!(pins(&mut h), 3); + assert_eq!(h.state().session.state.selected_layers, vec![boxl], "the tool stays on the Box"); + assert!(!rigged(&h, plate)); +} + /// Filled circles painted this frame (flattening nested shape lists). fn circles(h: &Harness<'_, EffectcraftApp>) -> Vec { fn walk(s: &egui::Shape, out: &mut Vec) { From bf09a4fcad68f7dbb7f69879c05ad808e7f7d276 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 8 Oct 2026 23:16:46 +0700 Subject: [PATCH 10/10] Expressions: the pick whip inserts at the cursor while editing (#284) Dragging an expression's pick whip always replaced the whole expression, also while it was being edited (and the editor then wrote its old text back over the reference). As in After Effects, a pick whip pressed while the expression is being edited now puts the reference at the cursor, replacing the selected text, and the editing goes on with the cursor after the reference. Without an edit in progress it replaces the expression as before. `prop.pickWhip` takes the text being edited (`expression`) and the selection in it (`range`, in characters) to insert there. Tests: the_expression_pick_whip_inserts_at_the_cursor_while_editing (ui_inline_edit), pick_whip_inserts_into_the_expression_being_edited (engine). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/commands/link.rs | 21 ++++++++- crates/engine/src/tests_timeline.rs | 27 +++++++++++ crates/ui-egui/src/panels/timeline.rs | 65 +++++++++++++++++++++----- crates/ui-egui/tests/ui_inline_edit.rs | 54 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/commands/link.rs b/crates/engine/src/commands/link.rs index 3892bd06..73697698 100644 --- a/crates/engine/src/commands/link.rs +++ b/crates/engine/src/commands/link.rs @@ -6,7 +6,7 @@ use effectcraft_time::{TICKS_PER_SECOND, Tick}; use serde_json::{Value, json}; use super::prop::prop_ref; -use super::{CommandSpec, b_p, bad, has_layers, layer_mut, layer_p}; +use super::{CommandSpec, b_p, bad, has_layers, layer_mut, layer_p, str_p}; use crate::{EngineError, Result, Session, cmd}; const AXES: [(&str, &str); 3] = [("positionX", "X Position"), ("positionY", "Y Position"), ("positionZ", "Z Position")]; @@ -164,6 +164,23 @@ fn pick_whip(s: &mut Session, p: &Value) -> Result { let r = reference(comp, from, target, tu, s.prefs.general.expression_pick_whip_compact).ok_or_else(|| bad("prop.pickWhip", "no such target property"))?; let (me, them) = (from.props.find(uid).ok_or(EngineError::NoComp)?, target.props.find(tu).ok_or(EngineError::NoComp)?); let (dm, dt) = (dims(from, me), dims(target, them)); + // While the expression is being edited the reference goes in at the cursor, replacing the + // selected text (After Effects): `expression` is the text being edited, `range` the + // selection [start, end] in characters. + if let Some(base) = str_p(p, "expression") { + let range: Option> = p.get("range").and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_u64).collect()); + let n = base.chars().count() as u64; + let (a, b) = match range.as_deref() { + Some(&[a, b]) => (a.min(b).min(n), a.max(b).min(n)), + None => (n, n), + Some(_) => return Err(bad("prop.pickWhip", "`range` must be [start, end] (characters)")), + }; + let head: String = base.chars().take(a as usize).collect(); + let tail: String = base.chars().skip(b as usize).collect(); + let text = format!("{head}{r}{tail}"); + s.execute("prop.setExpression", json!({"layer": lid.0, "prop": uid, "expression": text}))?; + return Ok(json!(text)); + } let text = if dt == 1 && dm > 1 { format!("temp = {r};\n[{}]", vec!["temp"; dm].join(", ")) } else if dt > 1 && dm == 1 { @@ -183,7 +200,7 @@ pub fn specs() -> Vec { "Pick Whip (Link Property)", [], None, - "{layer?, path|prop, target: {layer, path|prop}} → sets an AE reference expression", + "{layer?, path|prop, target: {layer, path|prop}, expression?, range?: [start, end]} → sets an AE reference expression (with `expression`: inserts it there, replacing the characters in `range`, default the end)", has_layers, pick_whip ), diff --git a/crates/engine/src/tests_timeline.rs b/crates/engine/src/tests_timeline.rs index 03b91c28..8356aec7 100644 --- a/crates/engine/src/tests_timeline.rs +++ b/crates/engine/src/tests_timeline.rs @@ -466,6 +466,33 @@ fn pick_whip_generates_ae_reference_expressions() { assert!(s.execute("prop.pickWhip", json!({"layer": l, "path": "transform/opacity", "target": {"layer": l, "path": "transform/opacity"}})).is_err()); } +/// #284: with the expression being edited, the pick whip puts the reference at the cursor, +/// replacing the selected characters (After Effects), in one undo step. +#[test] +fn pick_whip_inserts_into_the_expression_being_edited() { + let (mut s, l) = setup(); + let o = s.execute("layer.newSolid", json!({"name": "Ö", "color": "#00ff00"})).unwrap()["layer"].as_u64().unwrap(); + let whip = |s: &mut Session, base: &str, range: Value| { + let mut p = json!({"layer": l, "path": "transform/opacity", "target": {"layer": o, "path": "transform/rotation"}, "expression": base}); + if !range.is_null() { + p["range"] = range; + } + s.execute("prop.pickWhip", p) + }; + let r = "thisComp.layer(\"Ö\").transform.rotation"; + // Characters, not bytes: "é" before the cursor. + assert_eq!(whip(&mut s, "é + 5", json!([1, 1])).unwrap(), json!(format!("é{r} + 5"))); + assert_eq!(prop(&s, l, "transform/opacity").expr.unwrap().text, format!("é{r} + 5")); + // A selection (either way round) is replaced; no range: at the end. + assert_eq!(whip(&mut s, "a*2+b", json!([4, 2])).unwrap(), json!(format!("a*{r}b"))); + assert_eq!(whip(&mut s, "x + ", Value::Null).unwrap(), json!(format!("x + {r}"))); + // Past the end clamps; a malformed range is an error. + assert_eq!(whip(&mut s, "7", json!([9, 99])).unwrap(), json!(format!("7{r}"))); + assert!(whip(&mut s, "7", json!([1])).is_err()); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(prop(&s, l, "transform/opacity").expr.unwrap().text, format!("x + {r}")); +} + /// Outer comp (4 s) containing a precomp whose 10×10 white solid moves x = 10 + 20·t (t in s). /// Returns (session, precomp layer id in the outer comp). fn precomp_setup() -> (Session, u64) { diff --git a/crates/ui-egui/src/panels/timeline.rs b/crates/ui-egui/src/panels/timeline.rs index bdedd202..662efdb5 100644 --- a/crates/ui-egui/src/panels/timeline.rs +++ b/crates/ui-egui/src/panels/timeline.rs @@ -115,6 +115,20 @@ fn pick_whip_id() -> egui::Id { egui::Id::new("tl-pickwhip") } +/// The text of an expression being edited when its pick whip was pressed, and the selection in +/// it (characters): the pick whip's reference goes in there. +type WhipInto = (String, [usize; 2]); + +fn whip_into_id(prop: u64) -> egui::Id { + egui::Id::new(("tl-pickwhip-into", prop)) +} + +/// After a pick whip into the expression being edited: its editor takes the focus back, the +/// cursor this many characters before the end (just after the reference). +fn expr_refocus_id(prop: u64) -> egui::Id { + egui::Id::new(("tl-expr-refocus", prop)) +} + #[derive(Clone, Debug)] struct Row { layer: LayerId, @@ -2036,6 +2050,25 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { let pw = ui.interact(pw_rect, egui::Id::new(("expr-whip", uid)), Sense::drag()).on_hover_text("Expression pick whip: drag onto a property"); icons::paint(&lp, pw_rect.shrink(1.0), Icon::PickWhip, if pw.hovered() || pw.dragged() { t.text } else { t.text_dim }); app.auto.add(&format!("timeline.prop.{uid}.pickWhip"), pw_rect, "Expression pick whip"); + if pw.is_pointer_button_down_on() && ui.input(|i| i.pointer.primary_pressed()) { + // Pressed while the expression is being edited (its text is buffered while + // the editor has the focus): the reference will go in at the cursor, + // replacing the selection (#284). + let into = ctx.data(|d| d.get_temp::(egui::Id::new(("expr-buf", uid)))).map(|buf| { + let n = buf.chars().count(); + let sel = egui::text_edit::TextEditState::load(&ctx, egui::Id::new(("expr-edit", uid))) + .and_then(|st| st.cursor.char_range()) + .map_or([n, n], |r| [r.primary.index.0.min(r.secondary.index.0).min(n), r.primary.index.0.max(r.secondary.index.0).min(n)]); + (buf, sel) + }); + ctx.data_mut(|d| { + if let Some(into) = into { + d.insert_temp(whip_into_id(*uid), into); + } else { + d.remove::(whip_into_id(*uid)); + } + }); + } if pw.drag_started() { let pw_state: PickWhip = (1, layer.id.0, *uid, pw_rect.center()); ctx.data_mut(|d| d.insert_temp(pick_whip_id(), pw_state)); @@ -2089,19 +2122,20 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { gp.rect_filled(Rect::from_min_max(pos2(graph_x0, r.min.y), r.max), 0.0, Color32::from_rgb(0x1a, 0x1a, 0x1a)); let er = Rect::from_min_max(pos2(graph_x0 + 8.0, r.min.y + 3.0), pos2(rect.max.x - 14.0, r.max.y - 3.0)); let buf_id = egui::Id::new(("expr-buf", uid)); + let edit_id = egui::Id::new(("expr-edit", uid)); let mut buf: String = ctx.data(|d| d.get_temp(buf_id)).unwrap_or_else(|| ex.text.clone()); + if let Some(after) = ctx.data_mut(|d| d.remove_temp::(expr_refocus_id(*uid))) { + // A pick whip just put a reference in: editing goes on in the new text, + // the cursor after the reference. + buf = ex.text.clone(); + let mut st = egui::text_edit::TextEditState::load(&ctx, edit_id).unwrap_or_default(); + st.cursor.set_char_range(Some(egui::text::CCursorRange::one(egui::text::CCursor::new(buf.chars().count().saturating_sub(after))))); + st.store(&ctx, edit_id); + ctx.memory_mut(|m| m.request_focus(edit_id)); + } // Settings ▸ Scripting & Expressions ▸ Expressions Editor. let sp = app.session.prefs.scripting.clone(); - let resp = super::expr_editor::editor( - ui, - egui::Id::new(("expr-edit", uid)), - &mut buf, - er, - &sp, - if ex.enabled { expr_col } else { t.text_dim }, - (*lines).clamp(1, 8), - &t, - ); + let resp = super::expr_editor::editor(ui, edit_id, &mut buf, er, &sp, if ex.enabled { expr_col } else { t.text_dim }, (*lines).clamp(1, 8), &t); app.auto.add(&format!("timeline.prop.{uid}.expression"), er, &prop.name); let commit = resp.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter) && (i.modifiers.command || i.modifiers.ctrl)); if commit { @@ -2443,12 +2477,21 @@ pub fn show(app: &mut EffectcraftApp, ui: &mut egui::Ui, rect: Rect) { } if ctx.input(|i| !i.pointer.any_down()) { ctx.data_mut(|d| d.remove::(pick_whip_id())); + let into = (kind == 1).then(|| ctx.data_mut(|d| d.remove_temp::(whip_into_id(src_prop)))).flatten(); match (kind, target) { (0, Some((_, row))) if row.layer.0 != src_layer => { actions.push(("layer.setParent".into(), json!({"layers": [src_layer], "parent": row.layer.0}))); } (1, Some((_, Row { layer, kind: RowKind::Prop { uid }, .. }))) if uid != src_prop => { - actions.push(("prop.pickWhip".into(), json!({"layer": src_layer, "prop": src_prop, "target": {"layer": layer.0, "prop": uid}}))); + let mut params = json!({"layer": src_layer, "prop": src_prop, "target": {"layer": layer.0, "prop": uid}}); + if let Some((text, [a, b])) = into { + // Into the expression being edited, which goes on being edited. + params["expression"] = json!(text); + params["range"] = json!([a, b]); + let after = text.chars().count().saturating_sub(b); + ctx.data_mut(|d| d.insert_temp(expr_refocus_id(src_prop), after)); + } + actions.push(("prop.pickWhip".into(), params)); } _ => {} } diff --git a/crates/ui-egui/tests/ui_inline_edit.rs b/crates/ui-egui/tests/ui_inline_edit.rs index a8232de3..47b77471 100644 --- a/crates/ui-egui/tests/ui_inline_edit.rs +++ b/crates/ui-egui/tests/ui_inline_edit.rs @@ -73,3 +73,57 @@ fn timecode_field_replaces_commits_and_cancels() { assert_eq!(frame(&h), 24, "Escape cancels"); assert!(!h.ctx.egui_wants_keyboard_input(), "Escape releases the keyboard"); } + +/// #284: dragging the expression pick whip while the expression is being edited puts the +/// reference at the cursor (After Effects), and the editing goes on after it; without an edit +/// in progress it replaces the expression. +#[test] +fn the_expression_pick_whip_inserts_at_the_cursor_while_editing() { + let (mut h, _) = harness(); + let (layer, opacity, rotation, 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("prop.setExpression", json!({"layer": l, "path": "transform/opacity", "expression": "50 + 0"})).unwrap(); + let tr = s.active_comp().unwrap().layer(effectcraft_engine::project::LayerId(l)).unwrap().props.sub("transform").unwrap().clone(); + (l, tr.get("opacity").unwrap().uid, tr.get("rotation").unwrap().uid, tr.uid) + }; + h.state_mut().ui.timeline.open_layers.insert(layer); + h.state_mut().ui.timeline.open_groups.insert(transform); + h.run_steps(3); + let expr = |h: &Harness<'_, EffectcraftApp>| { + let comp = h.state().session.active_comp().unwrap(); + comp.layer(effectcraft_engine::project::LayerId(layer)).unwrap().props.find(opacity).unwrap().expr.as_ref().unwrap().text.clone() + }; + let whip = |h: &mut Harness<'_, EffectcraftApp>| { + let (from, to) = (center(h, &format!("timeline.prop.{opacity}.pickWhip")), center(h, &format!("timeline.prop.{rotation}.name"))); + h.input_mut().events.push(Event::PointerMoved(from)); + h.step(); + h.input_mut().events.push(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + for k in 1..=8 { + h.input_mut().events.push(Event::PointerMoved(from + (to - from) * (k as f32 / 8.0))); + h.step(); + } + h.input_mut().events.push(Event::PointerButton { pos: to, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(3); + }; + // Editing: the cursor at the end after typing " * ". + let p = center(&h, &format!("timeline.prop.{opacity}.expression")); + click(&mut h, p, 1); + key(&mut h, Key::End); + type_text(&mut h, " * "); + whip(&mut h); + assert_eq!(expr(&h), "50 + 0 * transform.rotation"); + // The editor has the keyboard again, its cursor after the reference. + type_text(&mut h, " + 1"); + // Ctrl/Cmd+Enter commits. + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::COMMAND)); + h.input_mut().events.push(Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::COMMAND }); + h.step(); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); + h.run_steps(3); + assert_eq!(expr(&h), "50 + 0 * transform.rotation + 1"); + // Not editing: the reference replaces the expression. + whip(&mut h); + assert_eq!(expr(&h), "transform.rotation"); +}