From f715f30cccffa9ed29f4d081d0b032803f6f0c47 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 17:31:58 +0100 Subject: [PATCH 1/3] Cryptomatte: the Selection tool picks IDs while the effect is selected As in After Effects, with the Cryptomatte effect selected in Effect Controls and the Selection tool active, a click on its layer in the Composition viewer picks the ID under the cursor without arming the eyedropper: plain replaces, Shift adds, Alt or Ctrl removes, one undo step each, with the same hover name and highlight. A drag there moves nothing. The viewer keeps its clicks off the layer, on its own handles there (bounding box, gizmo, mask vertices, motion-path keys, guides, region of interest), while a region of interest or Free Transform box is drawn, with another effect or a property row selected, and with other tools. A press in the pass the pointer leaves the layer is passed on to the viewer as a click. Reuses the eyedropper's code path (effect.pickCryptomatte); automation id viewer.cryptoSelect. Fixes #2839 --- crates/ui-egui/src/panels/crypto_pick.rs | 155 ++++++++-- .../src/panels/effect_controls/viewer.rs | 22 +- crates/ui-egui/src/panels/viewer.rs | 79 ++++- crates/ui-egui/src/panels/viewer/shapes.rs | 6 + crates/ui-egui/tests/it/main.rs | 1 + crates/ui-egui/tests/it/ui_crypto_pick.rs | 3 + .../ui-egui/tests/it/ui_crypto_pick_undo.rs | 2 + crates/ui-egui/tests/it/ui_crypto_select.rs | 288 ++++++++++++++++++ docs/control-protocol.md | 4 +- docs/effects-parity.md | 2 +- 10 files changed, 531 insertions(+), 31 deletions(-) create mode 100644 crates/ui-egui/tests/it/ui_crypto_select.rs 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 | — | From df01e1d75c369b5ec52bbe60873b9ed12a0c07f5 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 18:26:04 +0100 Subject: [PATCH 2/3] Cryptomatte: a touch that lands on the layer and presses in one pass picks A touch, or a pen without hover, arrives on the layer and presses in the same pass. The overlay wasn't there the pass before, so egui handed the press to the viewer and nothing was picked. The overlay now takes such a press when it is on the layer and off the viewer's handles, and claims it from the viewer (CLAIMED_PRIMARY), which then neither clicks, double-clicks nor drags with it: Shift no longer toggles the layer off, and a drag moves nothing. The claim ends with the button up. --- crates/ui-egui/src/panels/crypto_pick.rs | 22 ++++++++--- crates/ui-egui/src/panels/viewer.rs | 24 +++++++++++- crates/ui-egui/tests/it/ui_crypto_select.rs | 41 +++++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/panels/crypto_pick.rs b/crates/ui-egui/src/panels/crypto_pick.rs index e5e6dd88f..543fa6bdf 100644 --- a/crates/ui-egui/src/panels/crypto_pick.rs +++ b/crates/ui-egui/src/panels/crypto_pick.rs @@ -11,7 +11,8 @@ //! "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 +//! the pointer leaves the layer is passed on to the viewer as a click; one in the pass it arrives +//! on the layer (a touch, a pen without hover) is claimed from the viewer and picks. 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. //! @@ -307,7 +308,7 @@ 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, 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). +/// effect's layer, off the viewer's own handles there). struct PointerNav<'a> { tools: bool, nav: bool, @@ -315,6 +316,10 @@ struct PointerNav<'a> { /// 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, + /// The overlay did not sense clicks the pass before, so egui hands the pass's presses to the + /// viewer: one it owns (a touch, or a pen without hover, arriving on the layer and pressing in + /// one pass) is claimed from the viewer ([`super::viewer::CLAIMED_PRIMARY`]). + claim: bool, } /// Queues Cryptomatte picks from primary presses and releases on the armed eyedropper overlay. @@ -366,6 +371,10 @@ fn viewer_click_to_pick( let pass = ctx.cumulative_pass_nr(); ctx.data_mut(|d| d.insert_temp(egui::Id::unique(super::viewer::FORWARDED_PRIMARY), (pos, keys, pass))); } + // Handed to the viewer but ours: the viewer leaves it alone, so it only picks. + if pointer.claim && ours && !pointer.nav { + ctx.data_mut(|d| d.insert_temp(egui::Id::unique(super::viewer::CLAIMED_PRIMARY), ())); + } st.press = (!pointer.nav && ours).then(|| Press { pos, keys, @@ -637,8 +646,9 @@ pub fn viewer_hook( 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). + // Whether the overlay sensed clicks the pass before: egui handed it the pass's presses, not the + // viewer underneath (which would select or move the layer as well). If it didn't, a press of + // ours is claimed from the viewer; if it did, one not ours is passed on to the viewer. let owned_before = !implicit || st.sensed.is_some_and(|n| n.saturating_add(1) == pass); st.implicit = implicit; let mut reading = false; @@ -655,8 +665,8 @@ pub fn viewer_hook( } 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); + let pointer = PointerNav { tools, nav, owned: &on_layer, forward: implicit && owned_before, claim: implicit && !owned_before }; + viewer_click_to_pick(app, ui, map, ectx, &mut st, pointer, &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); diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index d653dc662..5886f722e 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -72,6 +72,17 @@ pub(crate) const FORWARDED_SECONDARY: &str = "viewer-forwarded-secondary"; /// 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"; +/// The egui data key (`()`) of a primary press an overlay claimed although egui handed it to the +/// viewer (the Selection tool's Cryptomatte picking, #2839, when the pointer arrived on the layer +/// and pressed in one pass, as a touch or a pen without hover does): the viewer neither clicks +/// nor drags with it, and lets go of the claim once the button is up. +pub(crate) const CLAIMED_PRIMARY: &str = "viewer-claimed-primary"; + +/// Whether an overlay claimed the primary press in progress ([`CLAIMED_PRIMARY`]). +fn press_claimed(ctx: &egui::Context) -> bool { + ctx.data(|d| d.get_temp::<()>(egui::Id::unique(CLAIMED_PRIMARY))).is_some() +} + /// What the viewer's own handles reach this pass, for overlays drawn on top of it. #[derive(Clone, Default)] struct Grabs { @@ -1203,6 +1214,7 @@ fn phase_dragstart(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, }; if resp.drag_started() && !text_owns + && !press_claimed(ui.ctx()) && let Some(pos) = resp.interact_pointer_pos() { let cpt = map.to_comp(pos); @@ -1804,7 +1816,9 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response .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 }; + // A press an overlay claimed (CLAIMED_PRIMARY) is not a click here. + let claimed = press_claimed(&ctx); + let clicked = if resp.clicked() && !text_owns && !claimed { 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 { @@ -1914,8 +1928,14 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response fn phase_dblclick(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response, v: &mut Vw<'_>) { let map = v.map; - let text_owns = v.text_owns; let ctx = v.ctx.clone(); + // A press an overlay claimed (CLAIMED_PRIMARY) is no double-click here either; the claim + // ends with the button up (after this pass's click and double-click). + let claimed = press_claimed(&ctx); + if claimed && !ctx.input(|i| i.pointer.primary_down()) { + ctx.data_mut(|d| d.remove::<()>(egui::Id::unique(CLAIMED_PRIMARY))); + } + let text_owns = v.text_owns || claimed; let handle_hits = std::mem::take(&mut v.handle_hits); let vertex_hits = std::mem::take(&mut v.vertex_hits); let paths = std::mem::take(&mut v.paths); diff --git a/crates/ui-egui/tests/it/ui_crypto_select.rs b/crates/ui-egui/tests/it/ui_crypto_select.rs index f4b693556..b1b716e24 100644 --- a/crates/ui-egui/tests/it/ui_crypto_select.rs +++ b/crates/ui-egui/tests/it/ui_crypto_select.rs @@ -286,3 +286,44 @@ fn after_the_eyedropper_and_esc_the_selection_tool_does_not_pick() { assert!(!h.state().session.crypto_probe_stats().0, "no IDs kept read"); let _ = std::fs::remove_dir_all(&ids.dir); } + +/// A touch (or a pen without hover) arrives on the layer and presses in one pass: the overlay +/// wasn't there the pass before, so egui hands the press to the viewer. It is still a pick (and +/// Shift still adds), and the viewer neither selects, toggles nor moves the layer with it. +#[test] +fn a_touch_that_arrives_and_presses_in_one_pass_picks() { + let (s, ids) = session("touch"); + 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)); + let touch = |h: &mut Harness<'_, EffectcraftApp>, pos: Pos2, modifiers: Modifiers, by: egui::Vec2| { + h.event(Event::PointerGone); + h.run_steps(2); + let ev = &mut h.input_mut().events; + ev.push(Event::ModifiersChanged(modifiers)); + ev.push(Event::PointerMoved(pos)); + ev.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed: true, modifiers }); + h.step(); + for k in 1..=4 { + h.event(Event::PointerMoved(pos + by * k as f32 / 4.0)); + h.step(); + } + h.event(Event::PointerButton { pos: pos + by, button: PointerButton::Primary, pressed: false, modifiers }); + h.step(); + h.event(Event::ModifiersChanged(Modifiers::NONE)); + h.event(Event::PointerGone); + h.step(); + wait_probe(h); + h.run_steps(2); + }; + touch(&mut h, a, Modifiers::NONE, vec2(0.0, 0.0)); + assert_eq!(selection(&h, &ids), "c1_1", "the touch picked"); + touch(&mut h, b, Modifiers::SHIFT, vec2(0.0, 0.0)); + assert_eq!(selection(&h, &ids), "c1_1, c3_3", "Shift added"); + assert_eq!(selected_layers(&h), vec![ids.plate], "the viewer didn't toggle the layer"); + // A touch that drags moves nothing (and picks nothing). + touch(&mut h, a, Modifiers::NONE, vec2(120.0, 40.0)); + assert_eq!((position(&h, ids.plate), undo_steps(&h)), (before, steps + 2), "the layer stays put"); + assert_eq!(selection(&h, &ids), "c1_1, c3_3"); + let _ = std::fs::remove_dir_all(&ids.dir); +} From 691befb201660bdacf672d9a8ee3dbfa1cf6b02b Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 19:45:59 +0100 Subject: [PATCH 3/3] Cryptomatte selection: doc comments for the functions this change touches --- crates/ui-egui/src/panels/viewer.rs | 9 +++++++++ crates/ui-egui/tests/it/ui_crypto_pick_undo.rs | 2 ++ crates/ui-egui/tests/it/ui_crypto_select.rs | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/crates/ui-egui/src/panels/viewer.rs b/crates/ui-egui/src/panels/viewer.rs index 5886f722e..05e9b23d7 100644 --- a/crates/ui-egui/src/panels/viewer.rs +++ b/crates/ui-egui/src/panels/viewer.rs @@ -96,6 +96,7 @@ struct Grabs { guides: Vec<(f32, bool)>, } +/// The egui data key of the viewer's [`Grabs`] for the current pass. fn grabs_id() -> egui::Id { egui::Id::unique("viewer-grabs") } @@ -1180,6 +1181,8 @@ fn led_by_a_moved_ancestor(comp: &Comp, layers: &[MoveLayer]) -> Vec { layers.iter().map(|l| led(l.0)).collect() } +/// Starts the gesture a new primary drag begins (pan, move, scale, a handle, a vertex, a marquee), +/// unless an overlay claimed the press. fn phase_dragstart(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, resp: &egui::Response, v: &mut Vw<'_>) { let comp_rect = v.comp_rect; let zoom = v.zoom; @@ -1780,6 +1783,8 @@ fn phase_stopped(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, re v.pin_hits = pin_hits; } +/// Acts on a click in the viewer for the active tool (select, zoom, add or delete vertices), +/// including a click an overlay passed on, and not one an overlay claimed. fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response, v: &mut Vw<'_>) { let area = v.area; let zoom = v.zoom; @@ -1926,6 +1931,8 @@ fn phase_clicked(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response v.puppet_layers = puppet_layers; } +/// Acts on a double-click: a path's points into a Free Transform box, or a precomp opened; lets +/// go of an overlay's claim on the press once the button is up. fn phase_dblclick(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Response, v: &mut Vw<'_>) { let map = v.map; let ctx = v.ctx.clone(); @@ -1986,6 +1993,8 @@ fn phase_dblclick(app: &mut EffectcraftApp, ectx: &EvalCtx, resp: &egui::Respons v.paths = paths; } +/// Runs the overlays drawn on top of the viewer (effect, gradient and text handles), after +/// recording the viewer's own controls for them, and draws the status chips. fn phase_tail(app: &mut EffectcraftApp, ui: &mut egui::Ui, ectx: &EvalCtx, v: &mut Vw<'_>) { let t = v.t; let cid = v.cid; 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 a35743ac0..7780dac0c 100644 --- a/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs +++ b/crates/ui-egui/tests/it/ui_crypto_pick_undo.rs @@ -452,6 +452,8 @@ enum Step { Blur(u64, f64), } +/// Performs one model step (a pick, an edit, undo or redo, applying or removing the effect) and +/// records the state the model expects after it. fn run_step(h: &mut Harness<'_, EffectcraftApp>, f: &Fix, m: &mut Model, size: [f64; 2], step: &Step, n: usize) { let mut o = m.state().clone(); let before = h.state().session.history.undo.len(); diff --git a/crates/ui-egui/tests/it/ui_crypto_select.rs b/crates/ui-egui/tests/it/ui_crypto_select.rs index b1b716e24..9331913d4 100644 --- a/crates/ui-egui/tests/it/ui_crypto_select.rs +++ b/crates/ui-egui/tests/it/ui_crypto_select.rs @@ -73,6 +73,7 @@ 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() } +/// Moves the pointer to `pos`, waits until the Cryptomatte IDs are read, and returns the chip's text. 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. @@ -97,6 +98,7 @@ fn click(h: &mut Harness<'_, EffectcraftApp>, pos: Pos2, modifiers: Modifiers) { h.run_steps(2); } +/// Presses at `from`, drags by `by` over a few passes and releases. fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, by: egui::Vec2) { h.event(Event::PointerMoved(from)); h.step(); @@ -110,6 +112,7 @@ fn drag(h: &mut Harness<'_, EffectcraftApp>, from: Pos2, by: egui::Vec2) { h.run_steps(3); } +/// The plate's Cryptomatte Selection text. 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 { @@ -118,15 +121,18 @@ fn selection(h: &Harness<'_, EffectcraftApp>, ids: &Ids) -> String { } } +/// The layer's Position value, formatted for comparison. 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())) } +/// How many steps the undo history holds. fn undo_steps(h: &Harness<'_, EffectcraftApp>) -> usize { h.state().session.history.undo.len() } +/// The ids of the selected layers. fn selected_layers(h: &Harness<'_, EffectcraftApp>) -> Vec { h.state().session.state.selected_layers.iter().map(|l| l.0).collect() }