macOS: the app no longer reads the menus at every step of a move, scale or shape drag

On macOS, native_menu::sync reads every row of the menus (each
command's enabled and checked state, and each label) whenever
state_hash changes, and the macOS backend updates the AppKit items
whose label, enabled or checked state changed. The hash included the
active document's revision, and Session::preview makes a new revision
at each step of a drag, so the app read every row again at each step
of a move, scale or shape drag. In a release build on an M2 Max, that
took about 1.5 ms a step with one object selected, and 13.8 ms a step
with 5,001 objects selected.

While an interaction is open, state_hash leaves the revision out.
Move, scale and shape drags, dialog previews and the font menu's
preview are interactions. The menus are read when an interaction
begins and again on the frame it ends: when a drag's button is
released, when Escape cancels a drag, when a move is dragged onto a
panel, when a dialog's preview is applied or canceled, when the font
menu's preview ends, or when a control request such as
ui.dialog.cancel ends it. They are still read on a click or a key
press during an interaction.

docs/development.md says when the menus are updated during an
interaction.

crates/ui-egui/src/native_menu.rs

@@ -560,7 +560,9 @@ pub fn run(app: &mut VectorcraftApp, ctx: &egui::Context) {
 
 /// Bring the native menu up to date when anything it shows may have changed: a command ran, the
 /// document, its history or selection changed, the user clicked or pressed a key. Reading the rows
-/// asks every command whether it is enabled, so frames that only animate or scroll skip it.
+/// asks every command whether it is enabled, so frames that only animate or scroll skip it, and so
+/// do the previews of an open interaction (the steps of a move, scale or shape drag, a dialog's
+/// preview), whose new revisions [`state_hash`] leaves out.
 pub fn sync(app: &mut VectorcraftApp, ctx: &egui::Context) {
     let Some(menu) = app.services.native_menu.as_ref() else { return };
     let input = ctx
@@ -585,7 +587,10 @@ pub fn sync(app: &mut VectorcraftApp, ctx: &egui::Context) {
 /// What the menus' rows depend on, cheaply (no allocation): commands run, the active document,
 /// its revision (edits, selection and view changes), history and clipboard, the language,
 /// shortcuts, plug-ins and an open file dialog. `input` forces a change on frames with a click or
-/// a key press, which covers what the hash doesn't list (panels shown, a tool picked).
+/// a key press, which covers what the hash doesn't list (panels shown, a tool picked). While an
+/// interaction is open (for example a move, scale or shape drag, a dialog's or the font menu's
+/// preview, or a typing session), the revision, which each of its previews changes, is left out:
+/// the hash changes as the interaction begins and as it ends.
 fn state_hash(app: &VectorcraftApp, input: Option<u64>) -> u64 {
     let mut h = DefaultHasher::new();
     input.hash(&mut h);
@@ -593,7 +598,8 @@ fn state_hash(app: &VectorcraftApp, input: Option<u64>) -> u64 {
     let s = &app.session;
     (s.journal.len(), s.active_index(), s.clipboard.is_empty(), s.tool_composing()).hash(&mut h);
     if let Some(d) = s.active() {
-        (d.uid, d.revision, d.history.undo.len(), d.history.redo.len(), d.transparency_grid, d.print_tiling).hash(&mut h);
+        let revision = d.interaction.is_none().then_some(d.revision);
+        (d.uid, revision, d.history.undo.len(), d.history.redo.len(), d.transparency_grid, d.print_tiling).hash(&mut h);
     }
     (app.system_paste, app.last_effect.is_some(), app.ui.recent_files.len(), app.ui.recent_files.first(), app.recent_fonts().len()).hash(&mut h);
     app.file_dialog_open().hash(&mut h);

crates/ui-egui/src/tests_nativemenu.rs

@@ -334,6 +334,71 @@ impl Mac {
     fn selected(&self) -> usize {
         self.app.session.active().unwrap().selection.objects.len()
     }
+
+    /// Where the document's point (`x`, `y`) is on the screen.
+    fn on_canvas(&self, x: f64, y: f64) -> Pos2 {
+        crate::canvas::Xf::new(self.app.canvas_rect.unwrap(), self.app.view().unwrap()).to_screen(vectorcraft_geom::Point::new(x, y))
+    }
+
+    /// Run `n` frames, each with the events that `step` returns (`step` may act on the app first),
+    /// and return the number of frames in which the app read the menus.
+    fn reads(&mut self, n: usize, mut step: impl FnMut(&mut Mac, usize) -> Vec<Event>) -> usize {
+        let synced = self.fake.synced();
+        for i in 0..n {
+            let events = step(self, i);
+            self.frame(events);
+        }
+        self.fake.synced() - synced
+    }
+
+    /// Press on the selected rectangle and move the pointer 8 points to the right: the Selection
+    /// tool's move begins. Returns where the press was.
+    fn start_move(&mut self) -> Pos2 {
+        let start = self.on_canvas(38.5, 40.0);
+        self.frame(vec![Event::PointerMoved(start), click(start, true)]);
+        self.frame(vec![Event::PointerMoved(start + vec2(8.0, 0.0))]);
+        assert!(self.app.session.in_interaction(), "the move began");
+        start
+    }
+
+    /// Open the Rotate dialog with Preview on: the rotation previews on the canvas.
+    fn rotate_preview(&mut self) {
+        menus::invoke(&mut self.app, "object.rotate", json!({}));
+        self.frame(vec![]);
+        self.app.ui.dialog.as_mut().unwrap().fields.insert("preview".into(), json!(true));
+        self.frame(vec![]);
+        assert!(self.app.session.in_interaction(), "the rotation previews");
+    }
+
+    /// Set `n` new angles in the open dialog through the control port, one a frame, and return the
+    /// number of frames in which the app read the menus.
+    fn angles(&mut self, n: usize) -> usize {
+        self.reads(n, |m, i| {
+            crate::tests_synthetic::control(&mut m.app, &m.ctx, "ui.dialog.set", json!({"field": "angle", "value": 10 * (i + 1)}));
+            vec![]
+        })
+    }
+}
+
+fn click(at: Pos2, pressed: bool) -> Event {
+    Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }
+}
+
+/// Run `drag` on a new app with the fake native menu, and run it again on another new app until
+/// the shortcut and plug-in generations hold still through a run (at most 20 runs). Return that app
+/// and what `drag` returned. The generations are process-wide and only grow. A test running
+/// alongside can bump them, and the app then reads the menus again.
+fn undisturbed<R>(drag: impl Fn(&mut Mac) -> R) -> (Mac, R) {
+    let generations = || (crate::shortcut_editor::GENERATION.load(std::sync::atomic::Ordering::Relaxed), crate::menus::plugin_revision());
+    for _ in 0..20 {
+        let mut m = Mac::new(true);
+        let before = generations();
+        let r = drag(&mut m);
+        if generations() == before {
+            return (m, r);
+        }
+    }
+    panic!("tests running alongside kept bumping the generations");
 }
 
 /// A key equivalent runs through the app's own key handling, once: ⌘A selects every object, ⌘Z
@@ -365,6 +430,122 @@ fn key_equivalents_run_through_the_normal_shortcut_path_once() {
     assert_eq!(crate::control::inspect(&m.app, &m.ctx)["nativeMenuBar"], json!(true));
 }
 
+/// Each step of a drag previews the move as a new revision: the menus aren't read again for it
+/// while the button is held, and are read when the button is released, with Undo naming the move.
+#[test]
+fn a_drag_reads_the_menus_when_it_is_released() {
+    let (mut m, (start, read)) = undisturbed(|m| {
+        let start = m.start_move();
+        (start, m.reads(6, |_, i| vec![Event::PointerMoved(start + vec2(8.0 * (i + 2) as f32, 0.0))]))
+    });
+    assert_eq!(read, 0, "the menus weren't read at any step of the drag");
+    let synced = m.fake.synced();
+    m.frame(vec![click(start + vec2(56.0, 0.0), false)]);
+    assert!(!m.app.session.in_interaction());
+    assert_eq!(m.fake.synced(), synced + 1, "the menus were read on the release");
+    assert_eq!(find(&m.fake.last(), "edit.undo").label, "Undo Move");
+}
+
+/// Escape ends a Rectangle tool drag while its button is still held: the menus are read then, with
+/// the preview's rectangle gone (nothing selected, so Cut is off), and not at the steps before.
+#[test]
+fn escape_during_a_drag_reads_the_menus() {
+    let (mut m, read) = undisturbed(|m| {
+        m.app.run("select.none", json!({})).unwrap();
+        m.app.run("tool.select", json!({"tool": "rectangle"})).unwrap();
+        let start = m.on_canvas(200.0, 150.0);
+        m.frame(vec![Event::PointerMoved(start), click(start, true)]);
+        m.frame(vec![Event::PointerMoved(start + vec2(10.0, 10.0))]);
+        assert!(m.app.session.in_interaction(), "the rectangle is being drawn");
+        m.reads(6, |_, i| vec![Event::PointerMoved(start + vec2(10.0, 10.0) * (i + 2) as f32)])
+    });
+    assert_eq!(read, 0, "the menus weren't read at any step of the drag");
+    assert!(find(&m.fake.last(), "edit.cut").enabled, "the preview's rectangle is selected");
+    let synced = m.fake.synced();
+    m.frame(vec![Event::Key { key: Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE }]);
+    assert!(!m.app.session.in_interaction(), "Escape dropped the rectangle");
+    assert_eq!(m.fake.synced(), synced + 1, "the menus were read");
+    assert!(!find(&m.fake.last(), "edit.cut").enabled, "nothing is selected");
+}
+
+/// A move dragged off the canvas onto a panel stops being a move with its button still held
+/// (`canvas::drag_art_out`): the menus are read then, without a release or a key, and Undo still
+/// names the step before the drag.
+#[test]
+fn a_move_dragged_onto_a_panel_reads_the_menus() {
+    let (mut m, (start, undo, read)) = undisturbed(|m| {
+        let undo = find(&m.fake.last(), "edit.undo").label.clone();
+        let start = m.start_move();
+        (start, undo, m.reads(4, |_, i| vec![Event::PointerMoved(start + vec2(8.0 * (i + 2) as f32, 0.0))]))
+    });
+    assert_eq!(read, 0, "the menus weren't read at any step of the drag");
+    let synced = m.fake.synced();
+    let panel = Pos2::new(m.app.canvas_rect.unwrap().right() + 40.0, start.y);
+    m.frame(vec![Event::PointerMoved(panel)]);
+    assert!(!m.app.session.in_interaction(), "the move was dropped");
+    assert_eq!(m.fake.synced(), synced + 1, "the menus were read");
+    assert_eq!(find(&m.fake.last(), "edit.undo").label, undo);
+}
+
+/// A dialog previews each angle that the control port sets, and the menus aren't read for the
+/// angles: `ui.dialog.cancel` on the control port ends the preview, and the menus are read then,
+/// without a release or a key.
+#[test]
+fn a_control_port_cancel_of_a_dialog_preview_reads_the_menus() {
+    let (mut m, read) = undisturbed(|m| {
+        m.rotate_preview();
+        m.angles(4)
+    });
+    assert_eq!(read, 0, "the menus weren't read while the dialog previewed the angles");
+    let synced = m.fake.synced();
+    crate::tests_synthetic::control(&mut m.app, &m.ctx, "ui.dialog.cancel", json!({}));
+    m.frame(vec![]);
+    assert!(!m.app.session.in_interaction() && m.app.ui.dialog.is_none());
+    assert_eq!(m.fake.synced(), synced + 1, "the menus were read");
+}
+
+/// OK applies the rotation a dialog previews as one undo step: the menus are read then, with Undo
+/// naming it, and not for the angles previewed before.
+#[test]
+fn applying_a_dialog_preview_reads_the_menus() {
+    let (mut m, read) = undisturbed(|m| {
+        m.rotate_preview();
+        m.angles(4)
+    });
+    assert_eq!(read, 0, "the menus weren't read while the dialog previewed the angles");
+    let synced = m.fake.synced();
+    crate::tests_synthetic::control(&mut m.app, &m.ctx, "ui.dialog.confirm", json!({}));
+    m.frame(vec![]);
+    assert!(!m.app.session.in_interaction() && m.app.ui.dialog.is_none());
+    assert_eq!(m.fake.synced(), synced + 1, "the menus were read");
+    assert_eq!(find(&m.fake.last(), "edit.undo").label, "Undo Rotate");
+}
+
+/// The font menu previews each font it highlights on the selected type, in one interaction: the
+/// menus are read as the preview begins and as it ends, and not for the fonts between.
+#[test]
+fn the_font_menu_preview_reads_the_menus_when_it_ends() {
+    use crate::font_menu::FontPick;
+    // The app reads the menus after each pick as at the end of a frame. A whole frame would start
+    // with `font_menu::end_stale_preview`, which ends a preview whose menu isn't drawn.
+    let apply = |m: &mut Mac, pick: FontPick| {
+        let synced = m.fake.synced();
+        crate::font_menu::apply(&mut m.app, &m.ctx, pick);
+        native_menu::sync(&mut m.app, &m.ctx);
+        m.fake.synced() - synced
+    };
+    let (mut m, read) = undisturbed(|m| {
+        m.app.run("select.none", json!({})).unwrap();
+        m.app.run("text.create", json!({"x": 50, "y": 50, "text": "Hi"})).unwrap();
+        m.frame(vec![]);
+        assert_eq!(apply(m, FontPick::Preview("Source Serif 4".into(), None)), 1, "the menus were read as the preview began");
+        apply(m, FontPick::Preview("Source Sans 3".into(), None)) + apply(m, FontPick::Preview("Inter".into(), None))
+    });
+    assert_eq!(read, 0, "the menus weren't read for the fonts after the first");
+    assert_eq!(apply(&mut m, FontPick::EndPreview), 1, "the menus were read as the preview ended");
+    assert!(!m.app.session.in_interaction());
+}
+
 /// A click runs its command like an in-window click, and the menus follow: View › Show Rulers
 /// reads Hide Rulers afterwards; the app menu's Appearance items run with their params.
 #[test]

docs/development.md

@@ -92,7 +92,7 @@ By default VectorCraft's own crates log at `info` and everything else at `warn`.
 On macOS the menus live in the system menu bar, not in the window's app bar (which keeps the brand mark, Home, the search icon and the workspace switcher next to the traffic lights, centred on the bar by `apps/vectorcraft/src/mac_window.rs`). `--in-window-menus` or `VECTORCRAFT_IN_WINDOW_MENUS=1` keeps them in the window, as on Windows, Linux and the web; so does a menu bar AppKit refuses to build (logged). `ui.inspect` says which with `nativeMenuBar`.
 
 - **One menu model.** `crates/ui-egui/src/native_menu.rs` (platform-free, tested on every platform) builds the menu bar from the in-window menus (`menus::menu_tree`, each item through `menus::entry`, which the in-window menus draw too) and lays it out the Mac way: the VectorCraft menu holds About, Settings ▸ (one item per Preferences page, General… ⌘K), Language ▸, Appearance ▸ (UI brightness), Services, Hide VectorCraft ⌃⌘H (⌘H stays View › Hide Edges), Hide Others ⌥⌘H, Show All and Quit ⌘Q (the app's own `app.quit`, so unsaved documents are asked about); those leave Edit and Help. The in-window bar itself never shows the VectorCraft menu (it starts at File, as in Illustrator, on every platform): Language and Appearance have no in-window menu and are synthesized in `mac_layout`, and Quit is File › Exit in the window, relabelled to Quit in the App menu. Window starts with Minimize ⌃⌘M and Zoom and ends with Bring All to Front, and Help is the system's Help menu (its search field). A system key a command (or a user's shortcut) already has stays the command's, and the log says so.
-- **The native side** is `apps/vectorcraft/src/mac_menu.rs`: muda (`=0.21.1`, default features off, macOS only, no `unsafe`) builds the `NSMenu`s. Labels, enabled and checked are updated in place when anything they show may have changed (a command ran, the document, its history or selection changed, a click or a key press); a change of structure (a recent file, the language, a shortcut) rebuilds the menus.
+- **The native side** is `apps/vectorcraft/src/mac_menu.rs`: muda (`=0.21.1`, default features off, macOS only, no `unsafe`) builds the `NSMenu`s. Labels, enabled and checked are updated in place when anything they show may have changed (a command ran, the document, its history or selection changed, a click or a key press); a change of structure (a recent file, the language, a shortcut) rebuilds the menus. While an interaction is open (for example a move, scale or shape drag, a dialog's preview or the font menu's preview), the app updates them for the document's changes only when the interaction begins and when it ends.
 - **Keys stay with egui.** AppKit runs a menu's key equivalents before the window sees the key, so the item's key goes back to egui as the key press it was (⌘X/⌘C/⌘V as egui's Cut/Copy/Paste events), through `raw_input_hook`: `shortcuts::handle` runs it once, user shortcuts apply, and a focused text field keeps ⌘A, ⌘C, ⌘V and ⌘Z. A click runs the item like an in-window click. Bare keys and ⌥/⇧ chords are never key equivalents (they type).
 - Built and clippy-checked for `aarch64-apple-darwin` and `x86_64-apple-darwin` from any host: `cargo clippy -p vectorcraft --target aarch64-apple-darwin --all-targets -- -D warnings`.