diff --git a/crates/ui-egui/src/panels/crypto_pick.rs b/crates/ui-egui/src/panels/crypto_pick.rs index 2310ba93b..e5e6dd88f 100644 --- a/crates/ui-egui/src/panels/crypto_pick.rs +++ b/crates/ui-egui/src/panels/crypto_pick.rs @@ -3,8 +3,17 @@ //! the viewer names the ID under the cursor, and a click picks it into the Selection: a plain //! click replaces it, Shift adds, Alt or Ctrl (Option or Cmd on a Mac) removes. It stays armed //! for the next click until Esc or the button again, and highlights the hovered ID's coverage. -//! Arming it whenever the effect is selected (After Effects' behaviour, #1811) would only need -//! to set the same pick. +//! +//! As in After Effects, the Selection tool picks the same way without arming anything while the +//! Cryptomatte effect is selected in Effect Controls (#2839, [`implicit_pick`]): over the +//! effect's layer (selected, so the viewer's press acts on it) a click picks and a drag moves nothing; +//! elsewhere the viewer keeps its clicks (selecting and moving layers). Esc has nothing to end. +//! "Over the layer" is its bounding quad, as the viewer's own hit test (not its alpha), less the +//! viewer's handles on it ([`super::viewer::grabs_at`]: bounding-box handles, mask vertices, +//! motion-path keys, guides), which keep scaling, reshaping and moving. A press in the very pass +//! the pointer leaves the layer is passed on to the viewer as a click. While the effect is +//! selected (or the eyedropper armed), a double-click on the layer picks (one step: the second +//! click changes nothing) instead of opening its precomp or editing its text. //! //! The hover needs the frame's Cryptomatte probe ([`effectcraft_engine::Session::crypto_probe_job`]): //! it is built off the UI thread, one at a time, not while playing or scrubbing; the chip says so @@ -21,7 +30,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use effectcraft_engine::commands::CryptoProbeJob; use effectcraft_engine::geom::Mat3; -use effectcraft_engine::project::Layer; +use effectcraft_engine::project::{GroupKind, Layer}; use effectcraft_engine::render::EvalCtx; use egui::{Color32, Rect, Sense, vec2}; use serde_json::{Value, json}; @@ -35,6 +44,9 @@ use crate::theme::Tokens; /// [`FxPick::kind`] of the Cryptomatte eyedropper. pub const KIND: &str = "crypto"; +/// The Cryptomatte effect's match name. +const EFFECT: &str = "ec.3d.cryptomatte"; + /// How long the time must stay put before a probe is built for it (scrubbing). const SETTLE: f64 = 0.1; @@ -111,6 +123,15 @@ struct State { seen: Option<((u64, u64), u64)>, /// The hovered ID's highlight: its hash and probe query, coverage texture, size and uv. mask: Option, + /// Picking with the Selection tool while the effect is selected ([`implicit_pick`]), not with + /// the armed eyedropper. + implicit: bool, + /// The last egui pass the overlay took clicks in (egui hands a press to what sensed it the + /// pass before). + sensed: Option, + /// The query of the last click queued: the second click of a double-click on the same pixel + /// with the same keys is not queued again. + last_click: Option, } /// Clears the busy flag and repaints when a build ends, a panic included. @@ -185,9 +206,20 @@ pub fn upkeep(app: &mut EffectcraftApp, ctx: &egui::Context) { } ctx.data_mut(|d| d.insert_temp(eid, epoch)); } - if app.ui.fx_pick.as_ref().is_none_or(|p| p.kind != KIND) && ctx.data(|d| d.get_temp::(sid)).is_some() { + // Picking with the Selection tool (the effect selected) keeps its state while the viewer runs + // it, every pass. + let old = ctx.data(|d| d.get_temp::(sid)); + let implicit = old.as_ref().is_some_and(|s| s.implicit && s.seen.is_some_and(|(_, n)| n == ctx.cumulative_pass_nr())); + if app.ui.fx_pick.as_ref().is_none_or(|p| p.kind != KIND) + && !implicit + && let Some(mut old) = old + { ctx.data_mut(|d| d.remove::(sid)); app.session.drop_crypto_probe(); + // The effect deselected (or the tool changed) while clicks waited: say they're not made. + if old.implicit && !old.pending.is_empty() { + drop_waiting(app, ctx, &mut old); + } } // The clicks waiting in the viewer, after everything else this pass did (its keys, edits and // frame steps included). @@ -273,10 +305,16 @@ fn viewer_pan_and_secondary(app: &mut EffectcraftApp, ctx: &egui::Context, resp: } } -/// Whether Space / Hand / Zoom (`tools`) or middle/right buttons (`nav`) own the pointer. -struct PointerNav { +/// Whether Space / Hand / Zoom (`tools`) or middle/right buttons (`nav`) own the pointer, and +/// whether a press at a point is the overlay's (`owned`: the Selection tool picking only over the +/// effect's layer, and only where the overlay took the press from the viewer). +struct PointerNav<'a> { tools: bool, nav: bool, + owned: &'a dyn Fn(egui::Pos2) -> bool, + /// The overlay took the pass's presses from the viewer (it sensed clicks the pass before): + /// one it does not own is passed on to the viewer ([`super::viewer::FORWARDED_PRIMARY`]). + forward: bool, } /// Queues Cryptomatte picks from primary presses and releases on the armed eyedropper overlay. @@ -290,7 +328,7 @@ fn viewer_click_to_pick( map: &super::viewer::ViewerMap, ectx: &EvalCtx, st: &mut State, - pointer: PointerNav, + pointer: PointerNav<'_>, at: &dyn Fn(egui::Pos2) -> Option, ) { let ctx = ui.ctx().clone(); @@ -320,7 +358,14 @@ fn viewer_click_to_pick( continue; }; if pressed { - let ours = map.area.contains(pos) && ctx.layer_id_at(pos) == Some(ui.layer_id()); + let in_viewer = map.area.contains(pos) && ctx.layer_id_at(pos) == Some(ui.layer_id()); + let ours = in_viewer && (pointer.owned)(pos); + // Taken from the viewer but not ours (the pointer left the layer in this very pass): + // the viewer clicks there next pass, so it isn't swallowed. + if pointer.forward && in_viewer && !ours && !pointer.nav { + let pass = ctx.cumulative_pass_nr(); + ctx.data_mut(|d| d.insert_temp(egui::Id::unique(super::viewer::FORWARDED_PRIMARY), (pos, keys, pass))); + } st.press = (!pointer.nav && ours).then(|| Press { pos, keys, @@ -348,6 +393,11 @@ fn viewer_click_to_pick( "replace" }); let same_frame = q["time"].as_f64() == Some(ectx.time.seconds()); + let double = ctx.input(|i| i.pointer.button_double_clicked(egui::PointerButton::Primary)); + if double && st.last_click.as_ref() == Some(&q) { + // The second click of a double-click, same pixel and keys: it would change nothing. + continue; + } if pos.distance(from) > SLOP { app.say(&ctx, tr("Drag cancelled the pick").to_string()); } else if app.playback.playing && !(same_frame && app.session.crypto_probe_ready(&q).unwrap_or(false)) { @@ -361,6 +411,7 @@ fn viewer_click_to_pick( st.pending_rev = rev; } st.after_typing |= typing || ctx.text_edit_focused(); + st.last_click = Some(q.clone()); st.pending.push(q); } } @@ -522,7 +573,8 @@ fn handle_escape(app: &mut EffectcraftApp, ui: &mut egui::Ui, st: &mut State) { } } -/// The armed eyedropper's part of the viewer. `m` maps the layer's effect space to the comp. +/// The armed eyedropper's part of the viewer, or (`implicit`) the Selection tool's while the +/// effect is selected ([`implicit_pick`]). `m` maps the layer's effect space to the comp. #[allow(clippy::too_many_arguments)] pub fn viewer_hook( app: &mut EffectcraftApp, @@ -533,6 +585,7 @@ pub fn viewer_hook( layer: &Layer, m: &Mat3, pick: &FxPick, + implicit: bool, ) { let ctx = ui.ctx().clone(); let sid = pick_id(); @@ -540,7 +593,9 @@ pub fn viewer_hook( reset_stale_state(app, &ctx, &mut st, pick); // The Cryptomatte effect that owns the Selection (gone: the pick ends). let Some(fx) = layer.effects().and_then(|f| f.groups().find(|g| g.find(pick.prop).is_some())).map(|g| g.uid) else { - app.ui.fx_pick = None; + if !implicit { + app.ui.fx_pick = None; + } return; }; let start = track_scrub_settle(app, &ctx, &mut st, ectx.time.seconds()); @@ -565,21 +620,83 @@ pub fn viewer_hook( // The Hand and Zoom tools (Space held too) and other buttons keep their own clicks and drags. let tools = super::space_hand(&ctx) || matches!(app.ui.tool, Tool::Hand | Tool::Zoom); let nav = tools || ctx.input(|i| i.pointer.middle_down() || i.pointer.secondary_down()); - // Drags too: a click that drifts must not move the layer under it. (egui hands a press to - // what sensed it the pass before, so middle and right presses land here: passed on below.) - let resp = ui.interact(map.area, egui::Id::unique("viewer-fx-pick"), if tools { Sense::hover() } else { Sense::click_and_drag() }); - viewer_pan_and_secondary(app, &ctx, &resp); - app.auto.add("viewer.fxPick", map.area, &pick.name); - let mut reading = hover_read(app, &ctx, map, &mut st, &resp, start, &at); - viewer_click_to_pick(app, ui, map, ectx, &mut st, PointerNav { tools, nav }, &at); + let oid = egui::Id::unique("viewer-fx-pick"); + // The Selection tool picks only over the effect's layer (#2839): the viewer's press there acts + // on it, a selected layer, even where layers in front cover it ([`super::viewer::pick`]). + // Elsewhere, and on the viewer's own handles there, the viewer keeps its clicks. The armed + // eyedropper picks anywhere. + let ui_layer = ui.layer_id(); + let on_layer = |p: egui::Pos2| { + !implicit + || (map.area.contains(p) + && ctx.layer_id_at(p) == Some(ui_layer) + && super::viewer::layer_hit(ectx, layer.id, map.to_comp(p)) + && !super::viewer::grabs_at(&ctx, p)) + }; + // A press the overlay holds (or its middle-button pan) stays its own wherever the pointer goes. + let panning = ctx.is_being_dragged(oid) && ctx.input(|i| i.pointer.middle_down()); + let over = st.press.is_some() || panning || ctx.pointer_hover_pos().is_some_and(on_layer); + let pass = ctx.cumulative_pass_nr(); + // A press is the overlay's only where it sensed clicks the pass before (egui handed it the + // press, not the viewer underneath, which would select or move the layer as well). + let owned_before = !implicit || st.sensed.is_some_and(|n| n.saturating_add(1) == pass); + st.implicit = implicit; + let mut reading = false; + if !implicit || over { + // Drags too: a click that drifts must not move the layer under it. (egui hands a press to + // what sensed it the pass before, so middle and right presses land here: passed on below.) + let resp = ui.interact(map.area, oid, if tools { Sense::hover() } else { Sense::click_and_drag() }); + if !tools { + st.sensed = Some(pass); + } + viewer_pan_and_secondary(app, &ctx, &resp); + app.auto.add(if implicit { "viewer.cryptoSelect" } else { "viewer.fxPick" }, map.area, &pick.name); + reading = hover_read(app, &ctx, map, &mut st, &resp, start, &at); + } else { + (st.asked, st.reply) = (None, None); + } + let owned = |p: egui::Pos2| owned_before && on_layer(p); + viewer_click_to_pick(app, ui, map, ectx, &mut st, PointerNav { tools, nav, owned: &owned, forward: implicit && owned_before }, &at); // Clicks waiting for their probe ([`make_waiting`], at the end of the pass). reading |= !st.pending.is_empty(); draw_coverage_highlight(app, &ctx, painter, map, m, &mut st); - draw_status_chip(app, &ctx, painter, map, &st, reading); - handle_escape(app, ui, &mut st); + if !implicit || over || reading { + draw_status_chip(app, &ctx, painter, map, &st, reading); + } + // (Selected, the effect has nothing for Esc to end.) + if !implicit { + handle_escape(app, ui, &mut st); + } ctx.data_mut(|d| d.insert_temp(sid, st)); } +/// The Cryptomatte effect selected in Effect Controls with the Selection tool active (After +/// Effects: clicks in the Composition panel pick its IDs then, no eyedropper armed; #2839): the +/// pick for its Selection. The last selected item on a selected layer counts, and only the +/// effect itself (its header in Effect Controls or the Timeline): a press on one of its rows (the +/// eyedropper's button, the Selection field) selects that property instead. An armed eyedropper +/// or crosshair (of any kind) comes first. +pub fn implicit_pick(app: &EffectcraftApp, ctx: &egui::Context) -> Option { + if app.ui.tool != Tool::Selection || app.ui.fx_pick.is_some() || app.ui.text_color_pick.is_some() || crate::color_picker::eyedropper_armed(ctx) { + return None; + } + // Drawing a region of interest or a Free Transform box: the viewer keeps every press. + if app.ui.viewer.roi_draw || super::viewer_overlays::free_transform_target(ctx).is_some() { + return None; + } + let comp = app.session.active_comp()?; + let time = app.session.time(); + let st = &app.session.state; + let (lid, uid) = st.selected_props.iter().rev().find(|(l, _)| st.selected_layers.contains(l))?; + let layer = comp.layer(*lid).filter(|l| l.is_active_at(time))?; + let fx = layer.effects()?.groups().find(|g| g.uid == *uid)?; + if !fx.enabled || !matches!(&fx.kind, GroupKind::Effect { effect } if effect == EFFECT) { + return None; + } + let prop = fx.get("selection")?; + Some(FxPick { kind: KIND.into(), layer: lid.0, prop: prop.uid, name: prop.name.clone() }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/ui-egui/src/panels/effect_controls/viewer.rs b/crates/ui-egui/src/panels/effect_controls/viewer.rs index 6d4eb7ffd..a2da5aa9b 100644 --- a/crates/ui-egui/src/panels/effect_controls/viewer.rs +++ b/crates/ui-egui/src/panels/effect_controls/viewer.rs @@ -221,12 +221,24 @@ pub fn viewer_hook( let ctx = ui.ctx().clone(); crate::color_picker::note_viewer(&ctx, map.area); let mut actions: Actions = vec![]; - let point_preview = if let Some(pick) = app.ui.fx_pick.clone().filter(|p| p.kind == crate::panels::crypto_pick::KIND) { + // The armed Cryptomatte eyedropper, or the Selection tool while the effect is selected (#2839). + let crypto = app.ui.fx_pick.clone().filter(|p| p.kind == crate::panels::crypto_pick::KIND).map(|p| (p, false)); + let crypto = crypto.or_else(|| crate::panels::crypto_pick::implicit_pick(app, &ctx).map(|p| (p, true))); + let point_preview = if let Some((pick, implicit)) = crypto { match ectx.comp.layer(effectcraft_engine::project::LayerId(pick.layer)) { - Some(l) => { - crate::panels::crypto_pick::viewer_hook(app, ui, painter, map, ectx, l, &crate::panels::viewer::from_effect_space(ectx, l, l2c(ectx, l)), &pick) - } - None => app.ui.fx_pick = None, + Some(l) => crate::panels::crypto_pick::viewer_hook( + app, + ui, + painter, + map, + ectx, + l, + &crate::panels::viewer::from_effect_space(ectx, l, l2c(ectx, l)), + &pick, + implicit, + ), + None if !implicit => app.ui.fx_pick = None, + None => {} } None } else { diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index b1899e1f8..d653dc662 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -67,6 +67,45 @@ fn map_id() -> egui::Id { /// the viewer opens its context menu there on the next pass. pub(crate) const FORWARDED_SECONDARY: &str = "viewer-forwarded-secondary"; +/// The egui data key (`(egui::Pos2, egui::Modifiers, u64)`: point, keys, egui pass) of a primary +/// press an overlay sensed but does not want (the Selection tool's Cryptomatte picking, #2839, when +/// the pointer left the layer in the pass of the press): the viewer clicks there on the next pass. +pub(crate) const FORWARDED_PRIMARY: &str = "viewer-forwarded-primary"; + +/// What the viewer's own handles reach this pass, for overlays drawn on top of it. +#[derive(Clone, Default)] +struct Grabs { + handles: Vec, + vertices: Vec, + keys: Vec, + axes: Vec<(LayerId, [Pos2; 2], [f64; 3], [f64; 3])>, + /// Region-of-interest handles. + roi: Vec, + /// Visible, unlocked guides on screen: (x or y, vertical). + guides: Vec<(f32, bool)>, +} + +fn grabs_id() -> egui::Id { + egui::Id::unique("viewer-grabs") +} + +/// Whether a Selection-tool press at `pos` grabs one of the viewer's own controls (a layer's +/// bounding-box handle or gizmo arrow, a mask vertex or tangent, a motion-path key, a guide, a +/// region-of-interest handle) instead of the layer under it, with the reach the viewer gives +/// them. An overlay that takes presses on a layer leaves these to the viewer. +pub(crate) fn grabs_at(ctx: &egui::Context, pos: Pos2) -> bool { + ctx.data(|d| { + let Some(g) = d.get_temp::(grabs_id()) else { return false }; + let reach = vertex_grab_radius(pos, g.handles.iter().copied()); + g.handles.iter().any(|h| h.distance(pos) < HANDLE + 2.0) + || g.vertices.iter().any(|v| v.distance(pos) < reach) + || g.keys.iter().any(|k| k.distance(pos) < 6.0) + || axis_at(&g.axes, pos).is_some() + || g.roi.iter().any(|h| h.distance(pos) < 6.0) + || g.guides.iter().any(|&(at, vertical)| (if vertical { pos.x } else { pos.y } - at).abs() < 4.0) + }) +} + pub fn comp_to_screen(ctx: &egui::Context, p: [f32; 2]) -> Option { let m: ViewerMap = ctx.data(|d| d.get_temp(map_id()))?; Some(m.to_screen([p[0] as f64, p[1] as f64])) @@ -1759,10 +1798,14 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response layers_at(ectx, cpt).any(|h| h.id == l.id) && super::puppet_tool::takes_pin(app, ectx, l, cpt) }) }; - if resp.clicked() - && !text_owns - && let Some(pos) = resp.interact_pointer_pos() - { + // A press an overlay passed on in the pass before (FORWARDED_PRIMARY) is a click here. + let pass = ctx.cumulative_pass_nr(); + let forwarded = ctx + .data_mut(|d| d.remove_temp::<(Pos2, egui::Modifiers, u64)>(egui::Id::unique(FORWARDED_PRIMARY))) + .filter(|f| f.2.saturating_add(1) == pass) + .map(|f| (f.0, f.1)); + let clicked = if resp.clicked() && !text_owns { resp.interact_pointer_pos().map(|p| (p, mods)) } else { None }; + if let Some((pos, mods)) = clicked.or(forwarded) { let cpt = map.to_comp(pos); match app.ui.tool { Tool::Zoom => { @@ -1937,6 +1980,32 @@ fn phase_tail(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: &m let ctx = v.ctx.clone(); let painter = v.painter.clone(); let comp = v.comp; + // The viewer's own controls, which overlays on top leave to it (#2839). + let roi = app.session.state.region_of_interest.map(|[rx, ry, rw, rh]| { + let r = Rect::from_min_size(v.comp_rect.min + vec2(rx as f32 * zoom, ry as f32 * zoom), vec2(rw as f32 * zoom, rh as f32 * zoom)); + roi_handles(r).to_vec() + }); + let guides = if app.ui.viewer.guides && !app.ui.viewer.lock_guides { + comp.guides + .iter() + .map(|g| { + let px = g.px(comp.width, comp.height); + let s = map.to_screen([px, px]); + (if g.vertical { s.x } else { s.y }, g.vertical) + }) + .collect() + } else { + vec![] + }; + let grabs = Grabs { + handles: v.handle_hits.iter().map(|h| h.2).collect(), + vertices: v.vertex_hits.iter().map(|h| h.pos).collect(), + keys: v.key_hits.iter().map(|h| h.pos).collect(), + axes: v.axis_hits.clone(), + roi: roi.unwrap_or_default(), + guides, + }; + ctx.data_mut(|d| d.insert_temp(grabs_id(), grabs)); // 3D Camera Tracker points, targets and their menu (on top of the viewer's gestures while // the effect is selected), and its analysis banner. super::camera_tracker_ui::viewer_hook(app, ui, &painter, &map, ectx, &|c, l| l2c(c, l).0); @@ -2001,7 +2070,7 @@ use geometry::{box_corners, project_clipped}; pub(crate) use geometry::{from_effect_space, fx2c, l2c, layer_outline, layer_quad}; use gesture::{Gesture, HANDLE, ROTO_HINT, pen_id, roi_drag, roi_handles, vertex_grab_radius}; use shapes::{create_shape, draw_shape_ghost, shape_drag}; -pub(crate) use shapes::{layer_at, layer_at_including_locked, layers_at, pick, selectable_layers}; +pub(crate) use shapes::{layer_at, layer_at_including_locked, layer_hit, layers_at, pick, selectable_layers}; #[cfg(test)] use tools::constrain_45; use tools::{add_motion_key, axis_at, axis_drag, drag_axes, draw_rigs, parent_world_inverse, pen_cursor, pen_cursor_icon, pen_press, select_motion_key}; diff --git a/crates/ui-egui/src/panels/viewer/shapes.rs b/crates/ui-egui/src/panels/viewer/shapes.rs index 0cd579883..f38b16e31 100644 --- a/crates/ui-egui/src/panels/viewer/shapes.rs +++ b/crates/ui-egui/src/panels/viewer/shapes.rs @@ -22,6 +22,12 @@ pub(crate) fn layers_at<'a, 'b>(ectx: &'b EvalCtx<'a>, cpt: [f64; 2]) -> impl It selectable_layers(ectx.comp, ectx.time).filter(move |l| layer_quad(ectx, l).is_some_and(|(_, q, _)| point_in_quad(cpt, &q))) } +/// Whether `layer` is among the selectable layers under a comp point ([`layers_at`] for that one +/// layer: its bounding quad, not its alpha). +pub(crate) fn layer_hit(ectx: &EvalCtx, layer: LayerId, cpt: [f64; 2]) -> bool { + selectable_layers(ectx.comp, ectx.time).find(|l| l.id == layer).is_some_and(|l| layer_quad(ectx, l).is_some_and(|(_, q, _)| point_in_quad(cpt, &q))) +} + /// Topmost selectable layer under a comp point. pub(crate) fn layer_at<'a>(ectx: &EvalCtx<'a>, cpt: [f64; 2]) -> Option<&'a Layer> { layers_at(ectx, cpt).next() diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 422f0223e..ace9f7222 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -26,6 +26,7 @@ mod ui_contrast; mod ui_control_modifiers; mod ui_crypto_pick; mod ui_crypto_pick_undo; +mod ui_crypto_select; mod ui_dialog_preview; mod ui_dock_resize; mod ui_drag_drop; diff --git a/crates/ui-egui/tests/it/ui_crypto_pick.rs b/crates/ui-egui/tests/it/ui_crypto_pick.rs index 0d1767c7e..893e5d562 100644 --- a/crates/ui-egui/tests/it/ui_crypto_pick.rs +++ b/crates/ui-egui/tests/it/ui_crypto_pick.rs @@ -76,6 +76,9 @@ fn crypto_session(tag: &str, w: usize, h: usize, blur: bool) -> (Session, Ids) { s.execute_checked("prop.set", json!({"path": "effects/#1/blurriness", "value": 40.0})).unwrap(); } s.execute_checked("effect.apply", json!({"effect": "Cryptomatte"})).unwrap(); + // Not selected: these test the armed eyedropper alone (the Selection tool picks too while + // the effect is selected, #2839: `ui_crypto_select`). + s.state.selected_props.clear(); let fx = s.active_comp().unwrap().layers[0].effects().unwrap().groups().last().unwrap(); let sel = fx.get("selection").unwrap().uid; (s, Ids { layer, sel, footage, dir }) diff --git a/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs b/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs index 22aaeb9a5..a35743ac0 100644 --- a/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs +++ b/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs @@ -538,6 +538,8 @@ fn run_step(h: &mut Harness<'_, EffectcraftApp>, f: &Fix, m: &mut Model, size: [ } Step::ApplyCrypto(layer) => { h.state_mut().session.execute_checked("effect.apply", json!({"comp": f.comp, "layers": [layer], "effect": "Cryptomatte"})).unwrap(); + // Not left selected: the model covers the armed eyedropper alone (#2839). + h.state_mut().session.state.selected_props.clear(); let uid = sel_uid(&h.state().session, f.comp, *layer).unwrap(); layer_mut(&mut o, *layer).effects.push((CRYPTO.into(), Some((uid, String::new())))); (format!("#{n} apply Cryptomatte to {layer}"), true) diff --git a/crates/ui-egui/tests/it/ui_crypto_select.rs b/crates/ui-egui/tests/it/ui_crypto_select.rs new file mode 100644 index 000000000..f4b693556 --- /dev/null +++ b/crates/ui-egui/tests/it/ui_crypto_select.rs @@ -0,0 +1,288 @@ +//! Picking Cryptomatte IDs with the Selection tool (#2839): as in After Effects, while the +//! Cryptomatte effect is selected in Effect Controls a click on its layer in the Composition +//! viewer picks the ID under the cursor (plain replaces, Shift adds, Alt or Ctrl removes), one +//! undo step each, without arming the eyedropper, and moves or selects nothing. With another +//! effect selected, off the layer, or with another tool, the viewer behaves as before. + +use crate::ui_crypto_pick::{chip, crypto_exr, settle, wait_probe}; +use effectcraft_engine::Session; +use effectcraft_engine::keyframe::Value as KV; +use effectcraft_engine::project::LayerId; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::dock::{self, PanelKind, SplitSize}; +use effectcraft_ui_egui::panels::viewer::comp_to_screen; +use effectcraft_ui_egui::state::Tool; +use egui::{Event, Modifiers, PointerButton, Pos2, vec2}; +use egui_kittest::Harness; +use serde_json::json; + +struct Ids { + plate: u64, + side: u64, + blur: u64, + crypto: u64, + sel: u64, + dir: std::path::PathBuf, +} + +/// A 480 × 180 comp: the 320 × 180 Cryptomatte plate in the middle (comp x 80 to 400) with a +/// Gaussian Blur and a Cryptomatte effect, and a 64 × 64 solid beside it (comp x 408 to 472). +/// The plate is selected, and its Cryptomatte effect in Effect Controls. +fn session(tag: &str) -> (Session, Ids) { + let dir = std::env::temp_dir().join(format!("ec-ui-crysel-{tag}-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = crypto_exr(&dir, "plate.exr", 320, 180, 1); + let mut s = effectcraft_host::session(); + let r = s.execute("file.import", json!({"paths": [path]})).unwrap(); + assert_eq!(r["errors"], json!([]), "{r}"); + let footage = r["items"][0].as_u64().unwrap(); + s.execute("comp.new", json!({"name": "Crypto", "width": 480, "height": 180, "frameRate": 24, "duration": 2})).unwrap(); + let plate = s.execute("layer.addItem", json!({"item": footage})).unwrap()["layer"].as_u64().unwrap(); + s.state.selected_layers = vec![LayerId(plate)]; + s.execute_checked("effect.apply", json!({"effect": "Gaussian Blur"})).unwrap(); + s.execute_checked("effect.apply", json!({"effect": "Cryptomatte"})).unwrap(); + let side = s.execute("layer.newSolid", json!({"name": "Side", "width": 64, "height": 64})).unwrap()["layer"].as_u64().unwrap(); + s.execute_checked("prop.set", json!({"layer": side, "path": "transform/position", "value": [440.0, 40.0]})).unwrap(); + let fx = s.active_comp().unwrap().layer(LayerId(plate)).unwrap().effects().unwrap(); + let (blur, crypto) = (fx.groups().next().unwrap(), fx.groups().nth(1).unwrap()); + assert_eq!(crypto.match_id, "ec.3d.cryptomatte"); + let (blur, crypto, sel) = (blur.uid, crypto.uid, crypto.get("selection").unwrap().uid); + s.state.selected_layers = vec![LayerId(plate)]; + s.state.selected_props = vec![(LayerId(plate), crypto)]; + s.history = Default::default(); + (s, Ids { plate, side, blur, crypto, sel, dir }) +} + +/// The Composition viewer and Effect Controls side by side, the Timeline below. +fn harness(s: Session) -> Harness<'static, EffectcraftApp> { + let mut app = EffectcraftApp::new(s); + app.ui.viewer.show_layer_controls = false; + app.ui.tool = Tool::Selection; + app.ui.dock = dock::docked(dock::vsplit( + SplitSize::FixedSecond(160.0), + dock::hsplit(SplitSize::FixedSecond(420.0), dock::tabs(&[PanelKind::Composition], 0), dock::tabs(&[PanelKind::EffectControls], 0)), + dock::tabs(&[PanelKind::Timeline], 0), + )); + let mut h = Harness::builder().with_size(vec2(1400.0, 800.0)).with_pixels_per_point(1.0).with_step_dt(0.25).build_eframe(|_| app); + settle(&mut h); + h +} + +/// The screen point of the plate's pixel (x, y). +fn on_plate(h: &Harness<'_, EffectcraftApp>, x: f64, y: f64) -> Pos2 { + comp_to_screen(&h.ctx, [(80.0 + x) as f32, y as f32]).unwrap() +} + +fn hover(h: &mut Harness<'_, EffectcraftApp>, pos: Pos2) -> Option { + h.event(Event::PointerMoved(pos)); + // The overlay comes up over the layer in this pass and is hovered from the next one on. + h.run_steps(2); + wait_probe(h); + h.step(); + chip(h) +} + +/// Hover (a pass for the viewer to see the pointer), then press and release with `modifiers`. +fn click(h: &mut Harness<'_, EffectcraftApp>, pos: Pos2, modifiers: Modifiers) { + h.event(Event::PointerMoved(pos)); + h.event(Event::ModifiersChanged(modifiers)); + h.step(); + for pressed in [true, false] { + h.event(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers }); + h.step(); + } + h.event(Event::ModifiersChanged(Modifiers::NONE)); + h.step(); + wait_probe(h); + h.run_steps(2); +} + +fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, by: egui::Vec2) { + h.event(Event::PointerMoved(from)); + h.step(); + h.event(Event::PointerButton { pos: from, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + for k in 1..=6 { + h.event(Event::PointerMoved(from + by * k as f32 / 6.0)); + h.step(); + } + h.event(Event::PointerButton { pos: from + by, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(3); +} + +fn selection(h: &Harness<'_, EffectcraftApp>, ids: &Ids) -> String { + let s = &h.state().session; + match &s.active_comp().unwrap().layer(LayerId(ids.plate)).unwrap().props.find(ids.sel).unwrap().value { + KV::Str(t) => t.clone(), + v => panic!("{v:?}"), + } +} + +fn position(h: &Harness<'_, EffectcraftApp>, layer: u64) -> String { + let l = h.state().session.active_comp().unwrap().layer(LayerId(layer)).unwrap().clone(); + format!("{:?}", l.transform().and_then(|t| t.get("position")).map(|p| p.value.clone())) +} + +fn undo_steps(h: &Harness<'_, EffectcraftApp>) -> usize { + h.state().session.history.undo.len() +} + +fn selected_layers(h: &Harness<'_, EffectcraftApp>) -> Vec { + h.state().session.state.selected_layers.iter().map(|l| l.0).collect() +} + +/// Without the change, the viewer took these clicks itself: the Selection stayed empty. +#[test] +fn the_selection_tool_picks_while_the_cryptomatte_effect_is_selected() { + let (s, ids) = session("picks"); + let mut h = harness(s); + let (steps, before) = (undo_steps(&h), position(&h, ids.plate)); + let (a, b) = (on_plate(&h, 50.0, 40.0), on_plate(&h, 120.0, 100.0)); + // The viewer names the ID under the cursor, as the armed eyedropper does. + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1")); + assert!(h.state().auto.find("viewer.cryptoSelect").is_some(), "the Selection tool picks over the layer"); + assert_eq!(hover(&mut h, b).as_deref(), Some("c3_3")); + click(&mut h, a, Modifiers::NONE); + assert_eq!(selection(&h, &ids), "c1_1", "a click replaces"); + click(&mut h, b, Modifiers::SHIFT); + assert_eq!(selection(&h, &ids), "c1_1, c3_3", "Shift adds"); + click(&mut h, a, Modifiers::ALT); + assert_eq!(selection(&h, &ids), "c3_3", "Alt removes"); + click(&mut h, b, Modifiers::COMMAND); + assert_eq!(selection(&h, &ids), "", "Ctrl removes"); + click(&mut h, b, Modifiers::NONE); + assert_eq!(selection(&h, &ids), "c3_3"); + click(&mut h, b, Modifiers::CTRL); + assert_eq!(selection(&h, &ids), "", "Ctrl (not Cmd) removes too"); + click(&mut h, b, Modifiers::NONE); + assert_eq!(selection(&h, &ids), "c3_3"); + assert_eq!(undo_steps(&h), steps + 7, "one undo step per pick"); + assert!(h.state().ui.fx_pick.is_none(), "nothing armed"); + // Nothing else changed: the layer stays put and stays the only one selected. + assert_eq!(position(&h, ids.plate), before); + assert_eq!(selected_layers(&h), vec![ids.plate]); + // A drag on the layer moves nothing and picks nothing. + drag(&mut h, a, vec2(120.0, 40.0)); + assert_eq!((position(&h, ids.plate), undo_steps(&h), selection(&h, &ids)), (before, steps + 7, "c3_3".to_string())); + // Esc has nothing to end: the next click still picks. + h.key_press(egui::Key::Escape); + h.run_steps(2); + click(&mut h, a, Modifiers::SHIFT); + assert_eq!(selection(&h, &ids), "c3_3, c1_1", "added at the end"); + // Undo takes the picks back one at a time. + h.state_mut().session.execute_checked("edit.undo", json!({})).unwrap(); + assert_eq!(selection(&h, &ids), "c3_3"); + let _ = std::fs::remove_dir_all(&ids.dir); +} + +/// With another effect selected, beside the layer, or with another tool, the viewer keeps its +/// clicks: the Selection tool moves and selects layers as before, and nothing is picked. +#[test] +fn the_viewer_keeps_its_clicks_with_another_effect_beside_the_layer_or_another_tool() { + let (s, ids) = session("keeps"); + let mut h = harness(s); + let a = on_plate(&h, 50.0, 40.0); + // Gaussian Blur selected instead: a drag moves the layer. + h.state_mut().session.state.selected_props = vec![(LayerId(ids.plate), ids.blur)]; + h.run_steps(2); + let before = position(&h, ids.plate); + assert_eq!(hover(&mut h, a), None, "no ID named"); + assert!(h.state().auto.find("viewer.cryptoSelect").is_none()); + drag(&mut h, a, vec2(60.0, 0.0)); + assert_ne!(position(&h, ids.plate), before, "the drag moved the layer"); + assert_eq!(selection(&h, &ids), ""); + // The Cryptomatte effect selected again: a click on the other layer beside it selects that one. + h.state_mut().session.execute_checked("edit.undo", json!({})).unwrap(); + h.state_mut().session.state.selected_layers = vec![LayerId(ids.plate)]; + h.state_mut().session.state.selected_props = vec![(LayerId(ids.plate), ids.crypto)]; + h.run_steps(2); + let side = comp_to_screen(&h.ctx, [440.0, 40.0]).unwrap(); + let a = on_plate(&h, 50.0, 40.0); + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1")); + click(&mut h, side, Modifiers::NONE); + assert_eq!(selected_layers(&h), vec![ids.side], "the click beside the layer selected the other layer"); + assert_eq!(selection(&h, &ids), ""); + // The Hand tool with the effect selected: no pick. + h.state_mut().session.state.selected_layers = vec![LayerId(ids.plate)]; + h.state_mut().session.state.selected_props = vec![(LayerId(ids.plate), ids.crypto)]; + h.state_mut().ui.tool = Tool::Hand; + h.run_steps(2); + let a = on_plate(&h, 50.0, 40.0); + click(&mut h, a, Modifiers::NONE); + assert_eq!(selection(&h, &ids), "", "the Hand tool picks nothing"); + // Back to the Selection tool: it picks. + h.state_mut().ui.tool = Tool::Selection; + h.run_steps(2); + let a = on_plate(&h, 50.0, 40.0); + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1")); + click(&mut h, a, Modifiers::NONE); + assert_eq!(selection(&h, &ids), "c1_1"); + let _ = std::fs::remove_dir_all(&ids.dir); +} + +/// A press in the very pass the pointer leaves the layer for another one (the overlay sensed it +/// the pass before, so egui hands it the press): passed on to the viewer, which selects that layer. +#[test] +fn a_press_as_the_pointer_leaves_the_layer_still_selects_the_other_layer() { + let (s, ids) = session("leave"); + let mut h = harness(s); + let a = on_plate(&h, 50.0, 40.0); + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1")); + let side = comp_to_screen(&h.ctx, [440.0, 40.0]).unwrap(); + let ev = &mut h.input_mut().events; + ev.push(Event::PointerMoved(side)); + ev.push(Event::PointerButton { pos: side, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.step(); + h.event(Event::PointerButton { pos: side, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(3); + assert_eq!(selected_layers(&h), vec![ids.side], "the press wasn't swallowed"); + assert_eq!(selection(&h, &ids), ""); + let _ = std::fs::remove_dir_all(&ids.dir); +} + +/// The viewer's own handles on the layer keep working with the effect selected: a drag on a +/// corner of the bounding box scales the layer and picks nothing. +#[test] +fn a_corner_handle_still_scales_with_the_effect_selected() { + let (s, ids) = session("handle"); + let mut h = harness(s); + h.state_mut().ui.viewer.show_layer_controls = true; + h.run_steps(2); + let a = on_plate(&h, 50.0, 40.0); + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1"), "inside the layer it picks"); + let scale = |h: &Harness<'_, EffectcraftApp>| { + let l = h.state().session.active_comp().unwrap().layer(LayerId(ids.plate)).unwrap().clone(); + format!("{:?}", l.transform().and_then(|t| t.get("scale")).map(|p| p.value.clone())) + }; + let before = scale(&h); + // The bottom-right corner of the plate (comp 400, 180). + let corner = comp_to_screen(&h.ctx, [400.0, 180.0]).unwrap(); + drag(&mut h, corner, vec2(-40.0, -20.0)); + assert_ne!(scale(&h), before, "the handle scaled the layer"); + assert_eq!(selection(&h, &ids), ""); + let _ = std::fs::remove_dir_all(&ids.dir); +} + +/// From the effect selected to the eyedropper and back: its button selects the Selection +/// property (the effect itself no longer), so after Esc a click on the layer picks nothing and +/// no Cryptomatte IDs are kept read. +#[test] +fn after_the_eyedropper_and_esc_the_selection_tool_does_not_pick() { + let (s, ids) = session("handoff"); + let mut h = harness(s); + let a = on_plate(&h, 50.0, 40.0); + assert_eq!(hover(&mut h, a).as_deref(), Some("c1_1"), "the effect selected: it picks"); + let eye = crate::ui_crypto_pick::at(&h, &format!("effectControls.prop.{}.eyedropper", ids.sel)); + click(&mut h, eye, Modifiers::NONE); + assert!(h.state().ui.fx_pick.as_ref().is_some_and(|p| p.kind == "crypto" && p.prop == ids.sel), "armed"); + assert_eq!(h.state().session.state.selected_props, vec![(LayerId(ids.plate), ids.sel)], "the row is selected"); + h.key_press(egui::Key::Escape); + h.run_steps(2); + assert!(h.state().ui.fx_pick.is_none(), "Esc ends the eyedropper"); + click(&mut h, a, Modifiers::NONE); + h.run_steps(2); + assert_eq!(selection(&h, &ids), "", "no pick"); + assert!(h.state().auto.find("viewer.cryptoSelect").is_none()); + assert!(!h.state().session.crypto_probe_stats().0, "no IDs kept read"); + let _ = std::fs::remove_dir_all(&ids.dir); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9f3d69ea5..c28d009ce 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -152,7 +152,9 @@ revolutions are `timeline.prop..revolutions`, `effectControls.prop..re `viewer.motionPath..[.in|.out]`, `viewer.puppetPin.[.rotate|.scale]`, `viewer.freeTransform.handle.`, `viewer.cryptoPick` (the Cryptomatte eyedropper's hover chip; its label is the chip's text), `viewer.cryptoPick.error` (the chip when it shows an error; label `whole`, or -`cut` when the message is cut off), `viewer.cryptoPick.highlight` (the hovered ID's highlighted area) and +`cut` when the message is cut off), `viewer.cryptoPick.highlight` (the hovered ID's highlighted area), +`viewer.cryptoSelect` (the Selection tool picking Cryptomatte IDs over the layer while its Cryptomatte +effect is selected in Effect Controls, no eyedropper armed; label: the Selection's name) and `viewer.regionOfInterest`; the Graph Editor adds `timeline.graph.transformBox[.]`, `timeline.graph.snap` and `timeline.graph.reference`. Use `ui.elements` to see what is on screen. diff --git a/docs/effects-parity.md b/docs/effects-parity.md index c7f850834..2f51c4cc8 100644 --- a/docs/effects-parity.md +++ b/docs/effects-parity.md @@ -26,7 +26,7 @@ corpus compares renders, each effect is assumed to differ somewhere, and users k | Category | Effects | On the GPU | Known differences or bugs | |---|---:|---:|---| -| 3D Channel | 8 | 8 | Cryptomatte's GPU path is slow (#473; the Selection is picked in the viewer with an eyedropper, quoted names with commas fixed); multi-layer EXR: one decode per frame for the picture and the channels, cache softly bounded at 1 GB (#474; holding channels as f16 is still to do) | +| 3D Channel | 8 | 8 | Cryptomatte's GPU path is slow (#473; the Selection is picked in the viewer with an eyedropper, or with the Selection tool while the effect is selected as in After Effects (#2839), quoted names with commas fixed); multi-layer EXR: one decode per frame for the picture and the channels, cache softly bounded at 1 GB (#474; holding channels as f16 is still to do) | | Audio | 13 | 0 | CPU by design (applied in the mixdown) | | Blur & Sharpen | 16 | 15 | Radial Blur keeps the layer edges as After Effects 2026 (713 now pixel-identical); Gaussian Blur interaction and output to be measured first ([fidelity/ae-2026-acceptance.md](fidelity/ae-2026-acceptance.md)) | | Channel | 13 | 13 | — |