From f74fe812928289908eeeb7431a93357ddb7bade2 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 17:19:33 -0800 Subject: [PATCH] Live Paint: the swatch preview follows the release point; nudges read the key press's modifiers - After a fill drag, the Live Paint Bucket stores the release point, so the cursor swatch preview shows there instead of where the pointer last moved before the press. - Arrow nudges and Alt+arrow type spacing read Shift and Alt from the key press, as the tool keys do, so a modifier released within the same frame still makes a 10 pt nudge, a copy, or a spacing step. - tool.setOption accepts cursorSwatchPreview for the Live Paint Bucket and refuses a value that is not true or false. --- crates/engine/src/tests/build.rs | 7 +++-- crates/tools/src/builder/live_paint.rs | 4 ++- crates/tools/src/builder/tests.rs | 21 ++++++++++++++ crates/ui-egui/src/shortcuts.rs | 39 +++++++++++++++----------- crates/ui-egui/src/shortcuts/tests.rs | 37 ++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 21 deletions(-) diff --git a/crates/engine/src/tests/build.rs b/crates/engine/src/tests/build.rs index d958ae7cfa..f3681c5991 100644 --- a/crates/engine/src/tests/build.rs +++ b/crates/engine/src/tests/build.rs @@ -3294,9 +3294,10 @@ fn live_paint_tool_options_refuse_invalid_values_and_change_nothing() { let colors = [json!(3), json!("nope"), json!("#12345"), json!("#00ff0080"), json!("00ff0080"), json!("#0f08"), json!(null)]; let widths = [json!("wide"), json!(null), json!(true)]; let switches = [json!("true"), json!(null), json!(1)]; - for (tool, keys) in - [("livePaintBucket", &["paintFills", "paintStrokes", "highlight"][..]), ("livePaintSelection", &["selectFills", "selectStrokes"][..])] - { + for (tool, keys) in [ + ("livePaintBucket", &["paintFills", "paintStrokes", "cursorSwatchPreview", "highlight"][..]), + ("livePaintSelection", &["selectFills", "selectStrokes"][..]), + ] { let mut bad: Vec<(&str, Value)> = keys.iter().flat_map(|k| switches.iter().map(move |v| (*k, v.clone()))).collect(); bad.extend(colors.iter().map(|v| ("color", v.clone()))); bad.extend(widths.iter().map(|v| ("width", v.clone()))); diff --git a/crates/tools/src/builder/live_paint.rs b/crates/tools/src/builder/live_paint.rs index f629174e6a..a907a319bb 100644 --- a/crates/tools/src/builder/live_paint.rs +++ b/crates/tools/src/builder/live_paint.rs @@ -403,6 +403,8 @@ impl Tool for LivePaintBucketTool { } PointerKind::Drag => self.drag_fill(p), PointerKind::Up => { + // The swatch preview shows again at the release point. + self.at = Some((p, ev.mods.shift, ev.mods.alt)); let mut acts = self.drag_fill(p); if self.fill_drag.take().is_some() { acts.push(Action::Commit); @@ -673,7 +675,7 @@ pub(super) fn lp_rgb_color(v: &Value) -> bool { /// value it cannot read. pub fn check_live_paint_options(tool: &str, values: &serde_json::Map) -> Result<(), String> { let switches: &[&str] = match tool { - "livePaintBucket" => &["paintFills", "paintStrokes", "highlight"], + "livePaintBucket" => &["paintFills", "paintStrokes", "cursorSwatchPreview", "highlight"], "livePaintSelection" => &["selectFills", "selectStrokes"], _ => return Ok(()), }; diff --git a/crates/tools/src/builder/tests.rs b/crates/tools/src/builder/tests.rs index 14976f03c9..07a703be2d 100644 --- a/crates/tools/src/builder/tests.rs +++ b/crates/tools/src/builder/tests.rs @@ -1087,6 +1087,27 @@ fn bucket_click_fills_face_and_shift_click_strokes_edge() { assert_eq!(id, "livePaint.strokeEdge"); } +/// After a fill drag, the cursor swatch preview shows at the release point, not where the pointer +/// last moved before the press. +#[test] +fn bucket_swatch_preview_follows_the_release_point() { + let (d, ..) = live_group_doc(); + let s = Selection::default(); + let p = paint(); + let cx = cx(&d, &s, &p); + let mut t = LivePaintBucketTool::default(); + let chip_x = |t: &LivePaintBucketTool| { + t.overlays(&cx).iter().find_map(|o| if let Overlay::Swatch { p, selected: true, .. } = o { Some(p.x) } else { None }) + }; + t.pointer(&cx, &ev(PointerKind::Move, 25.0, 25.0)); + assert_eq!(chip_x(&t), Some(25.0)); + t.pointer(&cx, &ev(PointerKind::Down, 25.0, 25.0)); + t.pointer(&cx, &ev(PointerKind::Drag, 50.0, 25.0)); + assert_eq!(chip_x(&t), None, "no chips during the drag"); + t.pointer(&cx, &ev(PointerKind::Up, 75.0, 25.0)); + assert_eq!(chip_x(&t), Some(75.0)); +} + #[test] fn bucket_options_choose_what_a_click_paints() { let (d, gid, ..) = live_group_doc(); diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 1dd2ef1d96..ea32f6c078 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -625,8 +625,10 @@ fn dialog_or_palette_open(app: &VectorcraftApp) -> bool { /// Route Alt+arrows to spacing controls when text objects are selected. fn alt_type_arrows(app: &mut VectorcraftApp, ctx: &egui::Context) -> bool { - let m = ctx.input(|i| i.modifiers); - if !m.alt { + let keys = [(Key::ArrowLeft, ToolKey::Left), (Key::ArrowRight, ToolKey::Right), (Key::ArrowUp, ToolKey::Up), (Key::ArrowDown, ToolKey::Down)]; + // The modifiers held when each arrow went down, as `nudge_and_clear` reads them. + let pressed = keys.map(|(key, tool_key)| (key, tool_key, ctx.input(|i| press_modifiers(i, key)))); + if !pressed.iter().any(|(.., m)| m.alt) { return false; } let orientation = app.session.active().and_then(|st| { @@ -644,9 +646,8 @@ fn alt_type_arrows(app: &mut VectorcraftApp, ctx: &egui::Context) -> bool { let Some(vertical) = orientation else { return false; }; - let keys = [(Key::ArrowLeft, ToolKey::Left), (Key::ArrowRight, ToolKey::Right), (Key::ArrowUp, ToolKey::Up), (Key::ArrowDown, ToolKey::Down)]; - for (key, tool_key) in keys { - if ctx.input_mut(|i| i.consume_key(m, key)) { + for (key, tool_key, m) in pressed { + if m.alt && ctx.input_mut(|i| i.consume_key(m, key)) { if let Some((attribute, by)) = vectorcraft_tools::text::step_key(tool_key, m.shift, false, vertical) { let by = if m.command { by * 5.0 } else { by }; let _ = app.run("type.step", json!({"attribute": attribute, "by": by})); @@ -685,13 +686,27 @@ fn type_tool_input(app: &mut VectorcraftApp, ctx: &egui::Context, composing: boo settings_on_a_mac(app, ctx, mac(ctx)); } +/// The modifiers held when `key` last went down in this frame, which can differ from the frame's +/// at its end; the frame's when the key did not go down. +fn press_modifiers(i: &egui::InputState, key: Key) -> Modifiers { + i.events + .iter() + .rev() + .find_map(|e| match e { + egui::Event::Key { key: k, pressed: true, modifiers, .. } if *k == key => Some(*modifiers), + _ => None, + }) + .unwrap_or(i.modifiers) +} + /// Apply arrow-key movement and clear selected art when no text editor owns input, unless a Layers /// or Appearance row has Delete/Backspace (`rows_keys`). fn nudge_and_clear(app: &mut VectorcraftApp, ctx: &egui::Context, rows_keys: bool) { // Arrow nudges (Shift = ×10, Alt = copy). let arrows = [(Key::ArrowLeft, -1.0, 0.0), (Key::ArrowRight, 1.0, 0.0), (Key::ArrowUp, 0.0, -1.0), (Key::ArrowDown, 0.0, 1.0)]; for (k, dx, dy) in arrows { - let m = ctx.input(|i| i.modifiers); + // The modifiers held when the key went down, as `claimed_tool_keys` reads them. + let m = ctx.input(|i| press_modifiers(i, k)); if ctx.input_mut(|i| i.consume_key(m, k)) && app.session.active().is_some_and(|d| d.selection.has_objects_or_guides()) { let _ = app.run("object.nudge", json!({"dx": dx, "dy": dy, "big": m.shift, "copy": m.alt})); } @@ -878,17 +893,7 @@ fn claimed_tool_keys(app: &mut VectorcraftApp, ctx: &egui::Context, view: ViewIn // The Smart Pen Tool's Tab while it draws a path (the key otherwise hides the panels). (Key::Tab, ToolKey::Tab), ] { - // The modifiers held when the key went down, which can differ from the frame's at its end. - let pm = ctx.input(|i| { - i.events - .iter() - .rev() - .find_map(|e| match e { - egui::Event::Key { key, pressed: true, modifiers, .. } if *key == k => Some(*modifiers), - _ => None, - }) - .unwrap_or(m) - }); + let pm = ctx.input(|i| press_modifiers(i, k)); if !rows_take(k) && ctx.input(|i| i.key_pressed(k)) && app.session.tool_claims_key_with(tk, crate::canvas::mods(pm, false), view) diff --git a/crates/ui-egui/src/shortcuts/tests.rs b/crates/ui-egui/src/shortcuts/tests.rs index ae1f966696..92a1ab9768 100644 --- a/crates/ui-egui/src/shortcuts/tests.rs +++ b/crates/ui-egui/src/shortcuts/tests.rs @@ -2220,6 +2220,15 @@ fn alt_arrows_step_selected_type_object() { assert_eq!(r["runs"][0]["style"]["tracking"], json!(20.0)); assert_eq!(app.session.active().unwrap().selection.objects, [vectorcraft_doc::NodeId(id)]); + // Alt released within the frame of the press: the press's Alt still steps the spacing. + frame( + &mut app, + vec![egui::Event::ModifiersChanged(Modifiers::ALT), press(Key::ArrowRight, Modifiers::ALT), egui::Event::ModifiersChanged(Modifiers::NONE)], + ); + let r = app.session.execute("text.getRange", &json!({"id": id})).unwrap(); + assert_eq!(r["runs"][0]["style"]["tracking"], json!(40.0)); + assert_eq!(app.session.active().unwrap().selection.objects, [vectorcraft_doc::NodeId(id)], "the text is not copied"); + frame(&mut app, vec![egui::Event::ModifiersChanged(Modifiers::ALT), press(Key::ArrowDown, Modifiers::ALT)]); let r = app.session.execute("text.getRange", &json!({"id": id})).unwrap(); assert_eq!(r["runs"][0]["style"]["leading"], json!(26.0)); @@ -2276,6 +2285,34 @@ fn live_paint_bucket_takes_only_plain_arrows_for_swatches() { assert_ne!(app.session.paint.fill, fill, "a plain Right picks the next swatch"); } +/// An arrow nudge takes Shift and Alt from the key press, also when they are released within the +/// same frame: Shift+Right moves 10 points, and Alt+Right copies. +#[test] +fn arrow_nudge_takes_the_modifiers_of_the_key_press() { + let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); + app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + let rect = app.session.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap(); + let rect = vectorcraft_doc::NodeId(rect); + app.select_tool("selection"); + app.session.execute("select.set", &json!({"ids": [rect.0]})).unwrap(); + let released = |modifiers| { + vec![ + egui::Event::ModifiersChanged(modifiers), + egui::Event::Key { key: Key::ArrowRight, physical_key: Some(Key::ArrowRight), pressed: true, repeat: false, modifiers }, + egui::Event::ModifiersChanged(Modifiers::NONE), + ] + }; + let left = |app: &VectorcraftApp, id| app.session.active().unwrap().doc.bounds_of(&[id], false).unwrap().x0; + + frame(&mut app, released(Modifiers::SHIFT)); + assert_eq!(left(&app, rect), 10.0, "Shift+Right moves 10 points"); + + frame(&mut app, released(Modifiers::ALT)); + let selected = app.session.active().unwrap().selection.objects.clone(); + assert_ne!(selected.as_slice(), [rect], "Alt+Right copies"); + assert_eq!(left(&app, rect), 10.0, "the original stays"); +} + /// #793: with a layout without Latin letters the one-key shortcuts go by the key's position: /// the P key typing Persian `ح` chooses the Pen, the V key typing Russian `м` the Selection /// tool. A Latin layout keeps the letter typed: AZERTY's A (the Q key's place) is A.