From 804d7cee57fc524166f328b2907d4eff67c3ff3d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 5 Nov 2026 07:12:27 -0800 Subject: [PATCH] fix: preserve queued key order before tool selection --- crates/ui-egui/src/app/control.rs | 17 +++---- crates/ui-egui/src/tests/synthetic.rs | 64 +++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/app/control.rs b/crates/ui-egui/src/app/control.rs index 039012f35..2f6443282 100644 --- a/crates/ui-egui/src/app/control.rs +++ b/crates/ui-egui/src/app/control.rs @@ -10,24 +10,25 @@ impl VectorcraftApp { let mut handled = 0; while handled < control::MAX_CONTROL_REQUESTS_PER_FRAME { let Some(req) = self.deferred_control.take().or_else(|| rx.try_recv().ok()) else { break }; - // Synthetic input sent before this pointer request was dropped (see - // `discard_stale_injected_batch`): its events would reach the tool without that input. - if req.method == "ui.pointer" && req.made_before(self.synthetic_dropped_before) { + // Synthetic input sent before this tool action was dropped (see + // `discard_stale_injected_batch`): it would reach the tool without that input. + let tool_action = matches!(req.method.as_str(), "ui.pointer" | "ui.tool.select"); + if tool_action && req.made_before(self.synthetic_dropped_before) { // One its caller gave up is skipped without counting toward this frame's limit. if req.claim() { handled += 1; let _ = req.reply.send(json!({ "ok": false, - "error": "input sent before this pointer request was dropped because the document, dialog or focus changed" + "error": "input sent before this tool action was dropped because the document, dialog or focus changed" })); } continue; } - // `ui.pointer` drives the tool at once, while `ui.key` and the other synthetic input - // reach it in a later UI pass: a pointer request waits until the input sent before it - // was applied, so a key pressed during a drag changes the drag before its release. + // `ui.pointer` and `ui.tool.select` act at once, while `ui.key` and other synthetic + // input reach the tool in a later UI pass. Wait for that earlier input, so a key + // pressed during a drag reaches its tool before a pointer event or tool switch. // One its caller gave up meanwhile is dropped below instead of holding back the rest. - if req.method == "ui.pointer" && !req.given_up() && (!self.synthetic.is_empty() || self.synthetic_injected) { + if tool_action && !req.given_up() && (!self.synthetic.is_empty() || self.synthetic_injected) { self.deferred_control = Some(req); ctx.request_repaint(); break; diff --git a/crates/ui-egui/src/tests/synthetic.rs b/crates/ui-egui/src/tests/synthetic.rs index 1afa4da1e..e61cb8fa1 100644 --- a/crates/ui-egui/src/tests/synthetic.rs +++ b/crates/ui-egui/src/tests/synthetic.rs @@ -117,6 +117,70 @@ fn a_pointer_release_after_a_key_waits_for_the_key() { assert!(closed, "C reached the Arc tool before the release"); } +/// An MCP pointer gesture can select a tool just before sending its pointer events. A key queued +/// by the previous call must reach the old tool before that selection changes it. +#[test] +fn tool_selection_after_a_key_waits_for_the_key() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut app = VectorcraftApp::new(Session::new(), Default::default()).with_control(rx); + app.run("file.new", json!({"width": 300, "height": 300})).unwrap(); + app.run("tool.select", json!({"tool": "pen"})).unwrap(); + let ctx = egui::Context::default(); + let (key, _key_reply) = ControlRequest::new("ui.key", json!({"key": "B", "shift": true, "text": "B"})); + let (select, select_reply) = ControlRequest::new("ui.tool.select", json!({"tool": "arc"})); + tx.send(key).unwrap(); + tx.send(select).unwrap(); + + app.drain_control(&ctx); + assert!(select_reply.try_recv().is_err(), "selection waits while Shift+B is queued"); + assert_eq!(app.session.tool_id(), "pen"); + let mut key_reached_tool = false; + let mut selected = None; + for _ in 0..8 { + let mut raw = egui::RawInput::default(); + app.raw_input_hook(&ctx, &mut raw); + app.drain_control(&ctx); + if let Ok(reply) = select_reply.try_recv() { + selected = Some(reply); + break; + } + let mut out = ctx.run_ui(raw, |ui| crate::shortcuts::handle(&mut app, ui.ctx())); + out.textures_delta.clear(); + key_reached_tool |= app.session.tool_id() == "blobBrush"; + } + assert!(key_reached_tool, "Shift+B reaches the old tool before the explicit selection"); + let selected = selected.expect("the later tool selection runs"); + assert_eq!(selected["ok"], true, "{selected}"); + assert_eq!(app.session.tool_id(), "arc"); +} + +#[test] +fn tool_selection_after_a_discarded_key_does_not_run() { + let (tx, rx) = std::sync::mpsc::channel(); + let mut app = VectorcraftApp::new(Session::new(), Default::default()).with_control(rx); + app.run("file.new", json!({"width": 300, "height": 300})).unwrap(); + app.run("tool.select", json!({"tool": "pen"})).unwrap(); + let ctx = egui::Context::default(); + let (key, _key_reply) = ControlRequest::new("ui.key", json!({"key": "B", "shift": true, "text": "B"})); + let (select, select_reply) = ControlRequest::new("ui.tool.select", json!({"tool": "arc"})); + tx.send(key).unwrap(); + tx.send(select).unwrap(); + for _ in 0..2 { + let mut raw = egui::RawInput::default(); + app.raw_input_hook(&ctx, &mut raw); + app.drain_control(&ctx); + assert!(select_reply.try_recv().is_err(), "selection waits for the key"); + } + assert!(app.synthetic_injected); + app.ui.dialog = Some(crate::state::Dialog::new(crate::dialogs::save_options::KIND, json!({}))); + let mut raw = egui::RawInput::default(); + app.raw_input_hook(&ctx, &mut raw); + app.drain_control(&ctx); + let selected = select_reply.try_recv().expect("the waiting selection is answered"); + assert_eq!(selected["ok"], false, "{selected}"); + assert_eq!(app.session.tool_id(), "pen", "the discarded key cannot precede a tool switch"); +} + /// A waiting `ui.pointer` request whose caller gave it up is dropped at once: the requests sent /// after it run without waiting for the input sent before it to be applied. #[test]