Merge pull request #151 from storytold/fix/issue-86

Fix #86: modifiers sent with ui.key, ui.click and ui.drag reach every handler

crates/ui-egui/src/control.rs

@@ -160,10 +160,11 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context, req: &ControlReques
                 command: b("cmd"),
             };
             app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers: m });
-            app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers: m });
+            // Typed text comes with the press, so it shares the key's frame and modifiers.
             if let Some(t) = s("text") {
                 app.synthetic.push(egui::Event::Text(t.to_string()));
             }
+            app.synthetic.push(egui::Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers: m });
             ctx.request_repaint();
             ok(Value::Null)
         }

crates/ui-egui/src/lib.rs

@@ -76,6 +76,8 @@ mod tests_svg;
 #[cfg(test)]
 mod tests_svgsave;
 #[cfg(test)]
+mod tests_synthetic;
+#[cfg(test)]
 mod tests_sysclip;
 #[cfg(test)]
 mod tests_sysclip_emf;
@@ -269,6 +271,10 @@ pub struct VectorcraftApp {
     pub background: background::Background,
     /// Data Recovery's timer and startup question ([`recovery`]).
     pub recovery: recovery::Timer,
+    /// Modifiers the keyboard holds (from the host's input), given back after synthetic input.
+    host_modifiers: egui::Modifiers,
+    /// Synthetic input set the modifiers egui holds (see [`Self::raw_input_hook`]).
+    synthetic_modifiers: bool,
 }
 
 /// Seconds between two looks at the system clipboard for [`VectorcraftApp::system_paste`].
@@ -322,6 +328,8 @@ impl VectorcraftApp {
             paste_chord: Default::default(),
             background: Default::default(),
             recovery: Default::default(),
+            host_modifiers: Default::default(),
+            synthetic_modifiers: false,
         }
     }
 
@@ -654,20 +662,52 @@ impl VectorcraftApp {
         place::drop_files(self, files, target);
     }
 
-    /// Inject synthetic events (one press/release step per frame).
+    /// Inject synthetic events (one press/release step per frame). Handlers read the modifiers
+    /// egui holds (`i.modifiers`), so a synthetic key or button holds its own for the frames it
+    /// spans (a drag's moves included); the keyboard's come back after.
     pub fn raw_input_hook(&mut self, raw: &mut egui::RawInput) {
-        if self.synthetic.is_empty() {
+        for e in &raw.events {
-            return;
+            match e {
+                egui::Event::ModifiersChanged(m) => self.host_modifiers = *m,
+                egui::Event::WindowFocused(false) => self.host_modifiers = egui::Modifiers::NONE,
+                _ => {}
             }
+        }
+        let Some(first) = self.synthetic.first() else {
+            if std::mem::take(&mut self.synthetic_modifiers) {
+                raw.events.push(egui::Event::ModifiersChanged(self.host_modifiers));
+            }
+            return;
+        };
         // Pointer events go one per frame so egui sees presses, drags and releases as real input;
         // keyboard sequences go up to the key release.
-        let n = match self.synthetic[0] {
+        let n = match first {
             egui::Event::PointerMoved(_) | egui::Event::PointerButton { .. } => 1,
             _ => self.synthetic.iter().position(|e| matches!(e, egui::Event::Key { pressed: false, .. })).map_or(self.synthetic.len(), |i| i + 1),
         };
-        if let Some(egui::Event::PointerMoved(p) | egui::Event::PointerButton { pos: p, .. }) = self.synthetic.first() {
+        if let egui::Event::PointerMoved(p) | egui::Event::PointerButton { pos: p, .. } = first {
             raw.events.push(egui::Event::PointerMoved(*p));
         }
+        let (now, later) = self.synthetic.split_at(n.min(self.synthetic.len()));
+        // This frame's key or button, else the button a drag holds down (released later).
+        let held = now
+            .iter()
+            .find_map(|e| match e {
+                egui::Event::Key { modifiers, .. } | egui::Event::PointerButton { modifiers, .. } => Some(*modifiers),
+                _ => None,
+            })
+            .or_else(|| match later.iter().find(|e| matches!(e, egui::Event::PointerButton { .. })) {
+                Some(egui::Event::PointerButton { pressed: false, modifiers, .. }) => Some(*modifiers),
+                _ => None,
+            });
+        match held {
+            Some(m) => {
+                raw.events.push(egui::Event::ModifiersChanged(m));
+                self.synthetic_modifiers = true;
+            }
+            None if std::mem::take(&mut self.synthetic_modifiers) => raw.events.push(egui::Event::ModifiersChanged(self.host_modifiers)),
+            None => {}
+        }
         raw.events.extend(self.synthetic.drain(..n));
     }
 

crates/ui-egui/src/tests_synthetic.rs

@@ -0,0 +1,118 @@
+//! Control-channel input (`ui.key`, `ui.click`, `ui.drag`) holds its modifiers in egui's input
+//! state for the frames it spans, so handlers reading `i.modifiers` see them as they see the
+//! keyboard's.
+
+use egui::Modifiers;
+use serde_json::{Value, json};
+use vectorcraft_doc::NodeId;
+use vectorcraft_engine::Session;
+
+use crate::VectorcraftApp;
+use crate::control::{ControlRequest, Outcome};
+
+/// Send one control request (it must succeed).
+pub(crate) fn control(app: &mut VectorcraftApp, ctx: &egui::Context, method: &str, params: Value) {
+    let (req, _rx) = ControlRequest::new(method, params);
+    let Outcome::Done(r) = crate::control::handle(app, ctx, &req) else { panic!("not done") };
+    assert_eq!(r["ok"], true, "{r}");
+}
+
+/// Frames through the host's input hook until the synthetic queue is drained, and one more (the
+/// keyboard's modifiers come back); `draw` lays out what the input reaches. → the modifiers
+/// held in each frame.
+pub(crate) fn run_synthetic(app: &mut VectorcraftApp, ctx: &egui::Context, draw: fn(&mut VectorcraftApp, &mut egui::Ui)) -> Vec<Modifiers> {
+    let mut held = vec![];
+    loop {
+        let last = app.synthetic.is_empty();
+        let mut raw = egui::RawInput::default();
+        app.raw_input_hook(&mut raw);
+        let mut out = ctx.run_ui(raw, |ui| {
+            held.push(ui.input(|i| i.modifiers));
+            draw(app, ui);
+        });
+        out.textures_delta.clear();
+        if last {
+            return held;
+        }
+    }
+}
+
+fn keys(app: &mut VectorcraftApp, ui: &mut egui::Ui) {
+    crate::shortcuts::handle(app, ui.ctx());
+}
+
+/// A document with one selected 10 × 10 square at (10, 10) → (app, its id).
+fn one_square() -> (VectorcraftApp, NodeId) {
+    let mut app = VectorcraftApp::new(Session::new(), Default::default());
+    app.run("file.new", json!({"width": 200, "height": 200})).unwrap();
+    let id = app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap();
+    app.run("select.set", json!({"ids": [id]})).unwrap();
+    (app, NodeId(id))
+}
+
+fn top_left(app: &VectorcraftApp, id: NodeId) -> (f64, f64) {
+    let b = app.session.active().unwrap().doc.node(id).unwrap().geometric_bounds().unwrap();
+    (b.x0, b.y0)
+}
+
+fn object_count(app: &VectorcraftApp) -> usize {
+    app.session.active().unwrap().doc.layers[0].children().unwrap().len()
+}
+
+#[test]
+fn shift_arrow_nudges_ten_points_and_alt_arrow_copies() {
+    let (mut app, id) = one_square();
+    let ctx = egui::Context::default();
+    control(&mut app, &ctx, "ui.key", json!({"key": "ArrowDown", "shift": true}));
+    let held = run_synthetic(&mut app, &ctx, keys);
+    assert_eq!(top_left(&app, id), (10.0, 20.0), "Shift+↓ nudges by 10 pt");
+    assert!(held.first().is_some_and(|m| m.shift));
+    assert_eq!(held.last(), Some(&Modifiers::NONE), "the keyboard's modifiers come back");
+    control(&mut app, &ctx, "ui.key", json!({"key": "ArrowRight", "alt": true}));
+    run_synthetic(&mut app, &ctx, keys);
+    assert_eq!(object_count(&app), 2, "Alt+→ nudges a copy");
+}
+
+#[test]
+fn a_typed_key_holds_its_modifiers_for_its_text() {
+    let (mut app, _) = one_square();
+    let ctx = egui::Context::default();
+    control(&mut app, &ctx, "ui.key", json!({"key": "B", "shift": true, "text": "B"}));
+    run_synthetic(&mut app, &ctx, keys);
+    assert_eq!(app.session.tool_id(), "blobBrush", "Shift+B, not B");
+}
+
+#[test]
+fn a_drag_holds_its_modifiers_from_the_press_to_the_release() {
+    let (mut app, _) = one_square();
+    let ctx = egui::Context::default();
+    control(&mut app, &ctx, "ui.drag", json!({"x": 10, "y": 10, "toX": 50, "toY": 50, "steps": 3, "shift": true}));
+    let held = run_synthetic(&mut app, &ctx, |_, _| {});
+    let shift: Vec<bool> = held.iter().map(|m| m.shift).collect();
+    // Move to the start, press, three moves, release, then the keyboard's modifiers again.
+    assert_eq!(shift, [false, true, true, true, true, true, false]);
+}
+
+#[test]
+fn a_shift_click_on_a_layers_selection_square_adds_to_the_selection() {
+    let (mut app, a) = one_square();
+    let b = NodeId(app.run("shape.rectangle", json!({"x": 40, "y": 10, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap());
+    app.run("select.set", json!({"ids": [a.0]})).unwrap();
+    let ctx = egui::Context::default();
+    // Rows top down: the layer, then `b`; a row's selection square is 17 pt right of its target circle.
+    let mut out = ctx.run_ui(egui::RawInput::default(), |ui| crate::panels::layers::show(&mut app, ui));
+    out.textures_delta.clear();
+    let mut circles: Vec<egui::Pos2> = out
+        .shapes
+        .iter()
+        .filter_map(|c| match &c.shape {
+            egui::Shape::Circle(cs) if cs.radius == 5.0 => Some(cs.center),
+            _ => None,
+        })
+        .collect();
+    circles.sort_by(|p, q| p.y.total_cmp(&q.y));
+    let square = circles[1] + egui::vec2(17.0, 0.0);
+    control(&mut app, &ctx, "ui.click", json!({"x": square.x, "y": square.y, "shift": true}));
+    run_synthetic(&mut app, &ctx, crate::panels::layers::show);
+    assert_eq!(app.session.active().unwrap().selection.objects, vec![a, b], "added, not replaced");
+}

docs/control-protocol.md

@@ -416,3 +416,9 @@ from)}` opens too: dialog `printPreset`, the Print dialog's settings fields plus
 `print.presets.save` and returns to Print Presets (a new preset can't take a name in use). Delete, Import… and
 Export… are `print.presets.delete`, `print.presets.import` and `print.presets.export`; `ui.dialog.confirm` closes.
 Opening a `.vcprintpresets` file with `app.open` imports its presets.
+
+Modifiers on synthetic input: `ui.key`, `ui.click` and `ui.drag` take `shift`, `alt`, `ctrl` and `cmd` (Command on
+macOS, Ctrl elsewhere), and the app holds them for the frames the input spans, as if the keys were down: a key's
+press, its `text` and its release; a click's press and release; a drag from the press through every move to the
+release. Every handler sees them (Shift+arrow nudges by the big increment, Alt+arrow nudges a copy, Shift-clicking a
+Layers selection square adds to the selection); the keyboard's own modifiers apply again afterwards.