From 726d7e47a161bcef50bc8e5fe12599ef2d12ba4a Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Tue, 6 Oct 2026 15:16:02 -0400 Subject: [PATCH] Control: ui.click / ui.drag hold their modifiers for the whole gesture egui takes i.modifiers from ModifiersChanged events, not from the modifiers on a pointer event, so a control-channel Shift-click arrived as a plain click (e.g. Shift-click in the Timeline replaced the selection instead of adding). ui.click and ui.drag now queue ModifiersChanged before the gesture and clear it after. The synthetic-input feeding moves into EffectcraftApp::feed_synthetic so it can be tested; the new tests fail without the fix. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/control.rs | 6 ++ crates/ui-egui/src/lib.rs | 33 +++++++---- crates/ui-egui/tests/ui_control_modifiers.rs | 62 ++++++++++++++++++++ 3 files changed, 88 insertions(+), 13 deletions(-) create mode 100644 crates/ui-egui/tests/ui_control_modifiers.rs diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 617f0096..20a44953 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -308,6 +308,9 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques Err(e) => return err(e), }; let m = modifiers(p); + // egui keeps the modifiers from `ModifiersChanged` (not the ones on a pointer event) + // and the synthetic events arrive one per frame: hold them for the whole gesture. + app.synthetic.push(egui::Event::ModifiersChanged(m)); app.synthetic.push(egui::Event::PointerMoved(pos)); if req.method == "ui.click" { let button = match s("button") { @@ -321,6 +324,7 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques app.synthetic.push(egui::Event::PointerButton { pos, button, pressed: false, modifiers: m }); } } + app.synthetic.push(egui::Event::ModifiersChanged(egui::Modifiers::NONE)); Outcome::AfterInput } "ui.drag" => { @@ -332,6 +336,7 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques }; let steps = p.get("steps").and_then(Value::as_u64).unwrap_or(12).max(2); let m = modifiers(p); + app.synthetic.push(egui::Event::ModifiersChanged(m)); app.synthetic.push(egui::Event::PointerMoved(a)); app.synthetic.push(egui::Event::PointerButton { pos: a, button: egui::PointerButton::Primary, pressed: true, modifiers: m }); for i in 1..=steps { @@ -339,6 +344,7 @@ pub fn handle(app: &mut EffectcraftApp, ctx: &egui::Context, req: &ControlReques app.synthetic.push(egui::Event::PointerMoved(a + (b - a) * f)); } app.synthetic.push(egui::Event::PointerButton { pos: b, button: egui::PointerButton::Primary, pressed: false, modifiers: m }); + app.synthetic.push(egui::Event::ModifiersChanged(egui::Modifiers::NONE)); Outcome::AfterInput } "ui.scroll" => { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 764c6b69..3cd1d031 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -381,6 +381,25 @@ impl EffectcraftApp { v } + /// Feed this frame's share of the queued synthetic input (control channel `ui.click`, + /// `ui.drag`…) into `raw_input`. Pointer events go one per frame so egui sees press → moves + /// → release as a real gesture. + pub fn feed_synthetic(&mut self, raw_input: &mut egui::RawInput) { + if !self.synthetic.is_empty() { + // Pointer events go one per frame so egui sees press → moves → release as a real drag. + let pointer = |e: &egui::Event| matches!(e, egui::Event::PointerMoved(_) | egui::Event::PointerButton { .. } | egui::Event::MouseWheel { .. }); + let n = if pointer(&self.synthetic[0]) { + 1 + } else { + self.synthetic + .iter() + .position(|e| pointer(e) || matches!(e, egui::Event::Key { pressed: false, .. })) + .map_or(self.synthetic.len(), |i| if pointer(&self.synthetic[i]) { i.max(1) } else { i + 1 }) + }; + raw_input.events.extend(self.synthetic.drain(..n)); + } + } + /// Show panel `p` (opening it in its usual place if it is closed), bring it to the front and /// give it the focus: Window ▸ . pub fn show_panel(&mut self, p: PanelKind) { @@ -1393,19 +1412,7 @@ impl eframe::App for EffectcraftApp { } fn raw_input_hook(&mut self, _ctx: &egui::Context, raw_input: &mut egui::RawInput) { - if !self.synthetic.is_empty() { - // Pointer events go one per frame so egui sees press → moves → release as a real drag. - let pointer = |e: &egui::Event| matches!(e, egui::Event::PointerMoved(_) | egui::Event::PointerButton { .. } | egui::Event::MouseWheel { .. }); - let n = if pointer(&self.synthetic[0]) { - 1 - } else { - self.synthetic - .iter() - .position(|e| pointer(e) || matches!(e, egui::Event::Key { pressed: false, .. })) - .map_or(self.synthetic.len(), |i| if pointer(&self.synthetic[i]) { i.max(1) } else { i + 1 }) - }; - raw_input.events.extend(self.synthetic.drain(..n)); - } + self.feed_synthetic(raw_input); } fn logic(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { diff --git a/crates/ui-egui/tests/ui_control_modifiers.rs b/crates/ui-egui/tests/ui_control_modifiers.rs new file mode 100644 index 00000000..46567aed --- /dev/null +++ b/crates/ui-egui/tests/ui_control_modifiers.rs @@ -0,0 +1,62 @@ +//! Control-channel clicks and drags hold their modifiers for the whole gesture: egui takes +//! `i.modifiers` from `ModifiersChanged` events, not from the pointer events, and synthetic +//! events arrive one per frame, so a Shift-click must hold Shift from its press to its release. + +use effectcraft_engine::Session; +use effectcraft_ui_egui::EffectcraftApp; +use effectcraft_ui_egui::control::{self, ControlRequest}; +use serde_json::{Value, json}; + +/// Feed the queued synthetic input frame by frame; per frame: (has a press/release, the +/// modifiers egui holds in that frame, which only `ModifiersChanged` events change). +fn frames(app: &mut EffectcraftApp) -> Vec<(bool, egui::Modifiers)> { + let mut out = vec![]; + let mut held = egui::Modifiers::NONE; + for _ in 0..64 { + let mut raw = egui::RawInput::default(); + app.feed_synthetic(&mut raw); + if raw.events.is_empty() { + break; + } + let mut button = false; + for e in &raw.events { + match e { + egui::Event::ModifiersChanged(m) => held = *m, + egui::Event::PointerButton { .. } => button = true, + _ => {} + } + } + out.push((button, held)); + } + out +} + +fn send(app: &mut EffectcraftApp, method: &str, params: Value) { + let ctx = egui::Context::default(); + let (req, _rx) = ControlRequest::new(method, params); + let _ = control::handle(app, &ctx, &req); +} + +#[test] +fn shift_click_holds_shift_from_press_to_release() { + let mut app = EffectcraftApp::new(Session::default()); + send(&mut app, "ui.click", json!({"x": 10, "y": 10, "modifiers": {"shift": true}})); + let f = frames(&mut app); + let buttons: Vec<_> = f.iter().filter(|(b, _)| *b).collect(); + assert_eq!(buttons.len(), 2, "press and release: {f:?}"); + assert!(buttons.iter().all(|(_, m)| m.shift), "Shift on the press and the release: {f:?}"); + // Afterwards the modifiers are released again. + send(&mut app, "ui.click", json!({"x": 10, "y": 10})); + assert!(frames(&mut app).iter().all(|(_, m)| !m.shift), "a plain click after it has no Shift"); +} + +#[test] +fn shift_drag_holds_shift_through_the_moves() { + let mut app = EffectcraftApp::new(Session::default()); + send(&mut app, "ui.drag", json!({"from": {"x": 10, "y": 10}, "to": {"x": 90, "y": 40}, "steps": 4, "modifiers": {"shift": true}})); + let f = frames(&mut app); + let first = f.iter().position(|(b, _)| *b).expect("press"); + let last = f.iter().rposition(|(b, _)| *b).expect("release"); + assert!(last > first + 1, "moves between press and release: {f:?}"); + assert!(f[first..=last].iter().all(|(_, m)| m.shift), "Shift held for the whole drag: {f:?}"); +}