From df01e1d75c369b5ec52bbe60873b9ed12a0c07f5 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 18:26:04 +0100 Subject: [PATCH] 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); +}