From cefb44c0339077e10e14d0bf0299af9569d7ddf6 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 5 Nov 2026 15:34:28 +0700 Subject: [PATCH] MCP: pointer_gesture fails when the input sent before it is dropped Squashed from #5772 onto main. --- crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/app/control.rs | 12 ++++++ crates/ui-egui/src/app/input.rs | 3 ++ crates/ui-egui/src/control.rs | 20 +++++++++- crates/ui-egui/src/lib.rs | 4 ++ crates/ui-egui/src/tests/synthetic.rs | 56 +++++++++++++++++++++++++++ docs/mcp.md | 2 +- 7 files changed, 95 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 7a8e99409..e9a7c6d5c 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -105,6 +105,7 @@ impl VectorcraftApp { pending_url: None, control_rx: None, deferred_control: None, + synthetic_dropped_before: 0, pending_screenshots: vec![], queued_screenshots: vec![], screenshot_token: 0, diff --git a/crates/ui-egui/src/app/control.rs b/crates/ui-egui/src/app/control.rs index f6088892a..5031234b8 100644 --- a/crates/ui-egui/src/app/control.rs +++ b/crates/ui-egui/src/app/control.rs @@ -10,6 +10,18 @@ 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) { + handled += 1; + if req.claim() { + let _ = req.reply.send(json!({ + "ok": false, + "error": "input sent before this pointer request 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. diff --git a/crates/ui-egui/src/app/input.rs b/crates/ui-egui/src/app/input.rs index 2854e11d6..29baa37d2 100644 --- a/crates/ui-egui/src/app/input.rs +++ b/crates/ui-egui/src/app/input.rs @@ -123,6 +123,9 @@ impl VectorcraftApp { self.synthetic_modifiers = false; self.synthetic_injected = false; self.synthetic_frame = false; + // The `ui.pointer` requests made after this input, waiting or still on the channel, would + // run without it: `drain_control` answers the ones made before now with an error. + self.synthetic_dropped_before = crate::control::ControlRequest::next_seq(); } /// Release the pointer buttons egui still holds down when the system reports that no mouse diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 721a452e1..fccfbedab 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -34,7 +34,7 @@ //! Background Save or Background Export on. A `wait` other than true, false or null is refused use std::sync::Arc; -use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::atomic::{AtomicU8, AtomicU64, Ordering}; use std::sync::mpsc::Sender; use serde_json::{Value, json}; @@ -101,8 +101,13 @@ pub struct ControlRequest { pub params: Value, pub reply: Sender, state: Arc, + /// Its place in the order requests were made ([`ControlRequest::next_seq`]). + seq: u64, } +/// The sequence number of the next request made. +static NEXT_SEQ: AtomicU64 = AtomicU64::new(0); + /// A request's state: waiting to run, taken by the app ([`ControlRequest::claim`]), or given up /// by its caller before that ([`Ticket::give_up`]). It leaves `QUEUED` once, one way or the other. const QUEUED: u8 = 0; @@ -124,7 +129,18 @@ impl Ticket { impl ControlRequest { pub fn new(method: impl Into, params: Value) -> (Self, std::sync::mpsc::Receiver) { let (tx, rx) = std::sync::mpsc::channel(); - (Self { method: method.into(), params, reply: tx, state: Arc::new(AtomicU8::new(QUEUED)) }, rx) + let seq = NEXT_SEQ.fetch_add(1, Ordering::SeqCst); + (Self { method: method.into(), params, reply: tx, state: Arc::new(AtomicU8::new(QUEUED)), seq }, rx) + } + + /// The sequence number the next request made gets: every request made so far is before it. + pub fn next_seq() -> u64 { + NEXT_SEQ.load(Ordering::SeqCst) + } + + /// Whether this request was made before the one numbered `seq`. + pub fn made_before(&self, seq: u64) -> bool { + self.seq < seq } /// The caller's handle on this request, to give it up with. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 5b0aa93d8..afb8cbbc4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -154,6 +154,10 @@ pub struct VectorcraftApp { /// A `ui.pointer` request taken from the channel while synthetic input sent before it was not /// yet applied: it runs, ahead of the channel, on the first frame after that input is applied. deferred_control: Option, + /// When synthetic input was last dropped before a UI pass took it, the sequence number of the + /// next control request ([`ControlRequest::next_seq`]): a `ui.pointer` request made before it + /// came after that input and is answered with an error. + synthetic_dropped_before: u64, /// Window captures asked for and not answered yet. pending_screenshots: Vec, /// Admitted captures whose settled frame has not come yet (issued one at a time). diff --git a/crates/ui-egui/src/tests/synthetic.rs b/crates/ui-egui/src/tests/synthetic.rs index 495692a89..acf2c55ca 100644 --- a/crates/ui-egui/src/tests/synthetic.rs +++ b/crates/ui-egui/src/tests/synthetic.rs @@ -142,6 +142,62 @@ fn a_given_up_pointer_does_not_hold_back_later_requests() { assert!(up_reply.try_recv().is_err(), "the given-up pointer request did not run"); } +/// A `ui.pointer` release sent after a `ui.key` fails when the key's batch is discarded because a +/// dialog opened before a UI pass took it, whether the release was waiting for the key +/// (`waiting`) or still on the channel. A release made after the discard runs. +fn a_release_after_a_discarded_key(waiting: bool) { + 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": "arc"})).unwrap(); + let ctx = egui::Context::default(); + control(&mut app, &ctx, "ui.pointer", json!({"events": [{"kind": "down", "x": 100, "y": 100}, {"kind": "drag", "x": 200, "y": 180}]})); + let (key, _key_reply) = ControlRequest::new("ui.key", json!({"key": "c"})); + let (up, up_reply) = ControlRequest::new("ui.pointer", json!({"events": [{"kind": "up", "x": 200, "y": 180}]})); + tx.send(key).unwrap(); + let mut up = Some(up); + if waiting { + tx.send(up.take().unwrap()).unwrap(); + } + // The first frame queues the key; the second injects it. + for _ in 0..2 { + let mut raw = egui::RawInput::default(); + app.raw_input_hook(&ctx, &mut raw); + app.drain_control(&ctx); + assert!(up_reply.try_recv().is_err(), "the release waits for the key"); + } + assert!(app.synthetic_injected, "the key is in the frame's input"); + if let Some(up) = up { + tx.send(up).unwrap(); + } + // A dialog opens before a UI pass takes the key, so the next frame discards it. + 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); + // A release made after the key was discarded, queued behind the first one, runs. + let (later, later_reply) = ControlRequest::new("ui.pointer", json!({"events": [{"kind": "up", "x": 200, "y": 180}]})); + tx.send(later).unwrap(); + app.drain_control(&ctx); + let r = up_reply.try_recv().expect("the release was answered"); + assert_eq!(r["ok"], false, "{r}"); + let r = later_reply.try_recv().expect("the later release was answered"); + assert_eq!(r["ok"], true, "{r}"); + let document = app.session.active().unwrap(); + let arc = *document.selection.objects.last().expect("the arc drawn on the later release"); + let closed = document.doc.node(arc).unwrap().path_data().unwrap().subpaths[0].closed; + assert!(!closed, "the discarded C did not reach the Arc tool"); +} + +#[test] +fn a_waiting_pointer_release_fails_when_the_key_before_it_is_discarded() { + a_release_after_a_discarded_key(true); +} + +#[test] +fn a_queued_pointer_release_fails_when_the_key_before_it_is_discarded() { + a_release_after_a_discarded_key(false); +} + #[test] fn a_drag_holds_its_modifiers_from_the_press_to_the_release() { let (mut app, _) = one_square(); diff --git a/docs/mcp.md b/docs/mcp.md index 5fdb5c53a..df2529a18 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -427,7 +427,7 @@ opens the picker through `run_command` with the remote backend. | `inspect_ui` | `{}` | UI state. Remote mode only. | | `select_tool` | `{tool}` | `selection`, `directSelection`, `pen`, `rectangle`, `ellipse`, `polygon`, `star`, `lineSegment`, … | | `pointer_gesture` | `{events:[{kind,x,y,mods?,backquote?,pressure?,tiltX?,tiltY?,rotation?,holdMs?}], tool?, mods?, backquote?}` | `kind` is one of `down`, `drag`, `up`, `move`, `doubleclick`. `backquote` is the held state for each event; the top-level value supplies its default, and a `backquote` that is not a boolean is an error. Events go through the same path as the mouse. Headless, a `move` replies after the tool's highlight under the pointer is ready, including the Shape Builder's and the Live Paint Bucket's regions of a large selection, which the desktop app finds off the UI thread and highlights after the reply. | -| `pointer_gesture` | `{events:[{kind,x,y,mods?,backquote?,pressure?,tiltX?,tiltY?,rotation?,holdMs?}], tool?, mods?, backquote?}` | `kind` is one of `down`, `drag`, `up`, `move`, `doubleclick`. `backquote` is the held state for each event; the top-level value supplies its default. Events go through the same path as the mouse. Headless, a `move` replies after the tool's highlight under the pointer is ready, including the Shape Builder's and the Live Paint Bucket's regions of a large selection, which the desktop app finds off the UI thread and highlights after the reply. Remote, the events wait until the keys and other input sent before them (`press_key`, `type_text`) have reached the app, so a key pressed between two calls of one drag applies before the release. | +| `pointer_gesture` | `{events:[{kind,x,y,mods?,backquote?,pressure?,tiltX?,tiltY?,rotation?,holdMs?}], tool?, mods?, backquote?}` | `kind` is one of `down`, `drag`, `up`, `move`, `doubleclick`. `backquote` is the held state for each event; the top-level value supplies its default. Events go through the same path as the mouse. Headless, a `move` replies after the tool's highlight under the pointer is ready, including the Shape Builder's and the Live Paint Bucket's regions of a large selection, which the desktop app finds off the UI thread and highlights after the reply. Remote, the events wait until the keys and other input sent before them (`press_key`, `type_text`) have reached the app, so a key pressed between two calls of one drag applies before the release. When that input is dropped because the document, a dialog or the focus changed first, the call fails and its events are not sent. | | `draw_path` | `{points \| d, closed?, fill?, stroke?, strokeWidth?, dash?, cap?, join?, opacity?, name?, effects?}` | `points` is `[[x,y],…]` or `[{x,y,in?,out?,smooth?},…]`. `d` is SVG path data. The options are below. | | `draw_shape` | `{shape, …geometry, fill?, stroke?, strokeWidth?, dash?, cap?, join?, opacity?, name?, effects?}` | `rectangle`/`ellipse`: `x,y,width,height` (plus `radius` for corners). `polygon`: `cx,cy,radius,sides`. `star`: `cx,cy,radius1,radius2,points`. `line`: `x1,y1,x2,y2`. | | `set_paint` | `{fill?, stroke?, strokeWidth?, ids?}` | Applies to the selection (or `ids`) and becomes the default for new art. Returns `{applied, objects}`: `objects` is the number of objects painted, as the `objects` of the `paint.setFill`, `paint.setStroke` and `stroke.set` replies (the largest of them): a selected group counts the objects inside it. When it is 0, a `note` says that only the paint for new art changed. |