Merge pull request #571 from mo0kid/fix/macos-native-menu-dismissal

Fix macOS native menu dismissal

ROADMAP.md

@@ -102,7 +102,7 @@ grade by behaviour against `plan/illustrator/`, not by whether a menu item exist
   - Panels undock (#495): a dock tab, a panel icon or a popped-out panel's title dragged out of the dock floats that panel inside the window, and the strip right of the dock's tabs floats the whole tabbed group. A floating group moves by its title bar, a tab dragged out of it floats on its own, and dropped on another group's title bar or tabs it stacks with that group; dropped on the dock, which lights up under the pointer, its panels go back where they live (the tabbed group or the icon column), as its × and the panel menus' Float Panel / Dock Panel do (`window.panel.float`, `window.panel.dock`). The Tools panel floats by its title bar and docks again on the window's left edge. Floating panels and the Tools panel's position are kept with the preferences and in user workspaces, and clamped into a smaller window. Not done: panels in separate OS windows, docking a panel into a dock group it doesn't belong to, reordering tabs by dragging, collapsing a floating group to icons, resizing floating panels.
   - The dock's double arrow collapses Properties, Layers and Libraries to icons at the top of the icon column, popping out like the other icon panels, and expands them again (`window.collapseDock`, kept with the preferences and in saved workspaces). The icon column's own arrow (expanding the icon panels into a full column) is not done.
   - Localised UI: menus (in-window and native), panels, dialogs, toolbar and Preferences read from per-language catalogs (`crates/ui-egui/src/i18n`, see `docs/development.md`); Traditional Chinese (Taiwan), Simplified Chinese (mainland), Japanese, Spanish and Italian ship complete, and Czech and Brazilian Portuguese cover every menu label (and more), with system-locale detection on macOS, Windows, Linux and the web (the browser's languages, or `?lang=`), a VectorCraft ▸ Language menu and a Language preference. Names are never translated (fonts and their styles, layers, artboards, saved presets, user libraries, custom workspaces, recent files, plug-ins).
-  - Native macOS menu bar, vector tool cursors, a ⌘K command palette, middle-button panning with any tool.
+  - Native macOS menu bar, vector tool cursors, a ⌘K command palette, middle-button panning with any tool. Native menus explicitly end tracking on command selection, outside window presses and Escape; menu updates wait until tracking ends, and a dismissing press never starts a canvas gesture (M14.8).
   - The macOS menu bar is built from the in-window menus (one model, `native_menu`) and laid out as a Mac app's: the VectorCraft menu holds About, Settings ▸ (each Preferences page, General… ⌘K), Language ▸, Appearance ▸ (UI brightness), Services, Hide VectorCraft ⌃⌘H (⌘H stays Hide Edges), Hide Others, Show All and Quit; Window gains Minimize ⌃⌘M, Zoom and Bring All to Front, and Help the system's search field. Its key equivalents go back to egui as key presses, so user shortcuts, focused text fields and the Type tool's input method behave as in the window, and nothing fires twice; enabled, check marks and labels update in place. `--in-window-menus` or `VECTORCRAFT_IN_WINDOW_MENUS=1` keeps the menus in the window; `ui.inspect` reports `nativeMenuBar`.
   - macOS: Finder opens every format File › Open reads with VectorCraft (double-click, Open With, a drop on the Dock icon, `open -a`), at launch or while it runs; the release and `cargo xtask bundle` apps share one `Info.plist` that declares those document types, owning only VectorCraft's own formats. The DMG opens on a designed Finder window, VectorCraft.app beside the `Applications` link over a background with the app icon (#493).
   - A canvas context menu: right-click selects the object under the pointer and lists what applies to the selection (Undo/Redo, clipboard, group, isolation, join, Remove Anchor Points when anchors are selected, masks, compound paths, guides, Transform, Arrange, Select, Export Selection), or the view commands on empty canvas.
@@ -182,7 +182,7 @@ grade by behaviour against `plan/illustrator/`, not by whether a menu item exist
 | M11 | Artboards & views (artboard panel/tool, Trim View, middle-button pan, print tiling and the status bar's artboard navigator done; multiple windows, presentation polish) | 🟡 | 15–25 |
 | M12 | Advanced color & art (CMYK/ICC, separations, Gradient Mesh, Live Paint, Image Trace, Graphs) | 🟡 Gradient Mesh, Live Paint, Image Trace (12 presets, 18 ms/1k² image), Recolor Artwork, colour management (ICC, soft proofing, separations preview), Graphs (all 9 tools, Graph Data/Type, regenerate in place) done; graph Design/Column/Marker designs pending | 6–10 |
 | M13 | Automation (Actions ✅ record/playback, persisted; variables, scripting, batch) | 🟡 | 18–27 |
-| M14 | 1.0 polish (preferences, shortcut editor, workspaces, accessibility, packaging for all OSes) | 🟡 Preferences, shortcut editor (chords with Shift and a punctuation key fire as typed), workspaces, a custom title bar on Windows/Linux with a Home button, content-sized dialogs and menus that scroll when longer than the window done; an experimental, unsupported Windows 7 x64 portable OpenGL build (not verified by maintainers); accessibility and broader Windows/Linux packaging/QA pending | 18–28 |
+| M14 | 1.0 polish (preferences, shortcut editor, workspaces, accessibility, packaging for all OSes) | 🟡 Preferences, shortcut editor (chords with Shift and a punctuation key fire as typed), workspaces, a custom title bar on Windows/Linux with a Home button, content-sized dialogs, menus that scroll when longer than the window and macOS native menu dismissal done (M14.8); an experimental, unsupported Windows 7 x64 portable OpenGL build (not verified by maintainers); accessibility and broader Windows/Linux packaging/QA pending | 18–28 |
 | — | Interaction fidelity pass (every tool's modifiers, Properties panel per context, isolation, nuance) | ⬜ | 60–90 |
 | — | Hardening at scale (big-file corpus, fuzzing, cross-platform + browser QA) | 🟡 | 50–80 |
 | | **Total to full parity** (one agent; re-derived from the [Parity estimate](#parity-estimate) table) | | **~320–500** |
@@ -221,7 +221,7 @@ about 1–1.5 agent-hours each, so the other rows (estimated on the older scale)
 | Export for Screens, Asset Export, slices, Save for Web | 3 | 85% | polish only (all four are done) | 2–4 |
 | Print, colour management, separations, flattener | 3 | 80% | print preview fidelity (flattener presets in PDF and print, overprint in PDF and composite print, Print dialog, presets, PostScript, marks, separations, Print Tiling, print overprint/flattener/bitmap options and printer profile are done) | 1–2 |
 | Automation | 3 | 60% | Variables (data merge), scripting surface, batch | 8–12 |
-| UI chrome (panels, contextual Properties, workspaces, prefs) | 6 | 78% | Variables, SVG Interactivity, Properties per context, Consolidate All Windows, floating panels outside the app window and docked into any group, context menus beyond the canvas (Links, Asset Export, CSS Properties, Attributes and the canvas context menu are done), about 30 Preferences options stored but not yet acted on (#394: Spacing Guides, Auto Size New Area Type, Show Font Names in English, Highlight Alternate Glyphs, performance, devices…) | 14–22 |
+| UI chrome (panels, contextual Properties, workspaces, prefs) | 6 | 78% | macOS native menu dismissal fixed (M14.8); remaining: Variables, SVG Interactivity, Properties per context, Consolidate All Windows, floating panels outside the app window and docked into any group, context menus beyond the canvas (Links, Asset Export, CSS Properties, Attributes and the canvas context menu are done), about 30 Preferences options stored but not yet acted on (#394: Spacing Guides, Auto Size New Area Type, Show Font Names in English, Highlight Alternate Glyphs, performance, devices…) | 14–22 |
 | Libraries, Links, Package | 2 | 50% | a local Libraries panel (no cloud by design); Links and Package are done; placed VectorCraft documents are done (linked by default, vectors in every output, previews, Break Link, Expand, Edit Original, refresh on save) | 5–8 |
 | **Feature parity** | **103** | **~75%** | | **~210–330** |
 | Interaction-fidelity pass (side by side with Illustrator: every tool modifier, cursor, dialog, Properties context) | | | | 60–90 |

apps/vectorcraft/Cargo.toml

@@ -41,7 +41,7 @@ clipboard-win = { workspace = true }
 # with default features off). Pinned exactly: review the diff before bumping.
 muda = { version = "=0.21.1", default-features = false }
 objc2 = "0.6"
-objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSApplication", "NSEvent", "NSPasteboard", "NSResponder", "NSTextInputContext"] }
+objc2-app-kit = { version = "0.3", default-features = false, features = ["std", "NSApplication", "NSMenu", "NSEvent", "NSPasteboard", "NSResponder", "NSTextInputContext"] }
 # Files opened from Finder and the Dock (src/open_documents.rs) and pasted bitmaps (src/clipboard.rs); both already come with winit, muda and rfd.
 objc2-foundation = { version = "0.3", default-features = false, features = ["std", "block2", "NSArray", "NSData", "NSNotification", "NSOperation", "NSString", "NSURL"] }
 block2 = "0.6"

apps/vectorcraft/src/mac_menu.rs

@@ -12,16 +12,83 @@
 //! - Hide and Hide Others are items of ours calling `NSApplication`: AppKit's own always take ⌘H
 //!   (View › Hide Edges) and ⌥⌘H.
 
+use std::sync::Once;
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::mpsc::{Receiver, channel};
 
+use block2::RcBlock;
+
 use egui::{Key, KeyboardShortcut};
 use muda::accelerator::{Accelerator, Code, Modifiers};
 use muda::{CheckMenuItem, Menu, MenuEvent, MenuItem, PredefinedMenuItem, Submenu};
 use objc2::MainThreadMarker;
-use objc2_app_kit::{NSApplication, NSEventModifierFlags, NSEventType};
+use objc2_app_kit::{NSApplication, NSEventModifierFlags, NSEventType, NSMenuDidBeginTrackingNotification, NSMenuDidEndTrackingNotification};
+use objc2_foundation::{NSNotification, NSNotificationCenter};
 use vectorcraft_ui_egui::VectorcraftApp;
 use vectorcraft_ui_egui::native_menu::{self, Backend, Event, MenuBar, MenuRole, Node, Standard};
 
+// AppKit can deliver window input while its system menu is still tracking (especially with
+// asynchronous menu presentation). Keep the native tracking state separate from egui popups.
+static TRACKING: AtomicBool = AtomicBool::new(false);
+
+fn observe_tracking(ctx: &egui::Context) {
+    static INSTALL: Once = Once::new();
+    INSTALL.call_once(|| {
+        // SAFETY: AppKit initializes these immutable notification-name constants.
+        #[allow(unsafe_code)]
+        let notifications = unsafe { [(NSMenuDidBeginTrackingNotification, true), (NSMenuDidEndTrackingNotification, false)] };
+        for (name, tracking) in notifications {
+            let repaint = ctx.clone();
+            let block = RcBlock::new(move |_: std::ptr::NonNull<NSNotification>| {
+                TRACKING.store(tracking, Ordering::Relaxed);
+                // Apply any deferred menu updates as soon as the tracking session ends.
+                if !tracking {
+                    repaint.request_repaint();
+                }
+            });
+            // SAFETY: these are AppKit's notification names. The notification center retains
+            // the observer and block for the application's lifetime; callbacks update an atomic and request repaint.
+            #[allow(unsafe_code)]
+            unsafe {
+                NSNotificationCenter::defaultCenter().addObserverForName_object_queue_usingBlock(Some(name), None, None, &block);
+            }
+        }
+    });
+}
+
+fn cancel_tracking() {
+    if let Some(mtm) = MainThreadMarker::new()
+        && let Some(menu) = NSApplication::sharedApplication(mtm).mainMenu()
+    {
+        menu.cancelTracking();
+    }
+    TRACKING.store(false, Ordering::Relaxed);
+}
+
+/// A dismissing click belongs to the menu, so the canvas and panel controls must not see it.
+fn dismiss_input(raw: &mut egui::RawInput, tracking: bool) -> bool {
+    if !raw
+        .events
+        .iter()
+        .any(|e| matches!(e, egui::Event::PointerButton { pressed: true, .. } | egui::Event::Key { key: egui::Key::Escape, pressed: true, .. }))
+    {
+        return false;
+    }
+    if tracking {
+        raw.events.retain(|e| !matches!(e, egui::Event::PointerButton { .. } | egui::Event::Key { key: egui::Key::Escape, .. }));
+    }
+    true
+}
+
+/// Inspect real window input before native key equivalents and control events are added.
+pub fn raw_input_hook(raw: &mut egui::RawInput) {
+    // Asynchronous menu presentation may leave a displayed menu after its tracking notification.
+    // Still cancel it, but consume the dismissing input only during a reported tracking session.
+    if dismiss_input(raw, TRACKING.load(Ordering::Relaxed)) {
+        cancel_tracking();
+    }
+}
+
 enum Handle {
     Plain(MenuItem),
     Check(CheckMenuItem),
@@ -48,10 +115,13 @@ pub struct MacMenu {
     entries: Vec<Entry>,
     rx: Receiver<Chosen>,
     structure: u64,
+    /// Latest menu state to apply after AppKit finishes tracking; item ids stay stable meanwhile.
+    pending: Option<MenuBar>,
 }
 
 impl MacMenu {
     fn new(ctx: &egui::Context) -> MacMenu {
+        observe_tracking(ctx);
         let (tx, rx) = channel();
         let repaint = ctx.clone();
         MenuEvent::set_event_handler(Some(move |e: MenuEvent| {
@@ -67,7 +137,7 @@ impl MacMenu {
             // Wake egui, so the item runs now rather than on the next mouse move.
             repaint.request_repaint();
         }));
-        MacMenu { menu: None, entries: Vec::new(), rx, structure: 0 }
+        MacMenu { menu: None, entries: Vec::new(), rx, structure: 0, pending: None }
     }
 
     /// Build the menus of `bar` and make them the app's, in place of the ones before.
@@ -146,6 +216,13 @@ fn append(sub: &Submenu, nodes: &[Node], entries: &mut Vec<Entry>) -> muda::Resu
 
 impl Backend for MacMenu {
     fn sync(&mut self, bar: &MenuBar) {
+        // Replacing or mutating an NSMenu during tracking can strand its displayed menu. Keep
+        // the latest state: the shared model considers this sync complete and may not resend it.
+        if TRACKING.load(Ordering::Relaxed) {
+            self.pending = Some(bar.clone());
+            return;
+        }
+        self.pending = None;
         let items = bar.items();
         if self.menu.is_none() || native_menu::structure_key(bar) != self.structure || items.len() != self.entries.len() {
             if let Err(e) = self.build(bar) {
@@ -183,6 +260,7 @@ impl Backend for MacMenu {
     fn drain(&mut self) -> Vec<Event> {
         let mut out = Vec::new();
         while let Ok(chosen) = self.rx.try_recv() {
+            cancel_tracking();
             // Ids that aren't ours are AppKit's own items (Services, Zoom…), already handled.
             let Some(e) = chosen.id.strip_prefix("vc").and_then(|n| n.parse::<usize>().ok()).and_then(|i| self.entries.get(i)) else { continue };
             if let Some(command @ (native_menu::HIDE | native_menu::HIDE_OTHERS)) = e.item.command {
@@ -207,6 +285,12 @@ impl Backend for MacMenu {
                 None => Event::Click(e.item.clone()),
             });
         }
+        // Drain against the old item ids before a deferred structure change replaces them.
+        if !TRACKING.load(Ordering::Relaxed)
+            && let Some(bar) = self.pending.take()
+        {
+            self.sync(&bar);
+        }
         out
     }
 }
@@ -364,4 +448,40 @@ mod tests {
         assert_ne!(parse("Cmd+M"), parse("Ctrl+Cmd+M"));
         assert!(parse("Cmd+M").is_some());
     }
+
+    fn press() -> egui::Event {
+        egui::Event::PointerButton {
+            pos: egui::pos2(300.0, 200.0),
+            button: egui::PointerButton::Primary,
+            pressed: true,
+            modifiers: Default::default(),
+        }
+    }
+
+    #[test]
+    fn outside_press_dismisses_without_starting_a_canvas_gesture() {
+        let moved = egui::Event::PointerMoved(egui::pos2(300.0, 200.0));
+        let mut raw = egui::RawInput { events: vec![moved.clone(), press()], ..Default::default() };
+        assert!(dismiss_input(&mut raw, true));
+        assert_eq!(raw.events, [moved]);
+        assert!(!dismiss_input(&mut raw, true));
+        raw.events.push(press());
+        assert!(dismiss_input(&mut raw, false));
+        assert_eq!(raw.events.len(), 2, "ordinary canvas presses still reach egui");
+    }
+
+    #[test]
+    fn escape_dismisses_without_reaching_the_active_tool() {
+        let escape = |pressed| egui::Event::Key { key: egui::Key::Escape, physical_key: None, pressed, repeat: false, modifiers: Default::default() };
+        let mut raw = egui::RawInput { events: vec![escape(true), escape(false)], ..Default::default() };
+        assert!(dismiss_input(&mut raw, true));
+        assert!(raw.events.is_empty());
+    }
+
+    #[test]
+    fn moving_the_pointer_does_not_dismiss_a_tracking_menu() {
+        let mut raw = egui::RawInput { events: vec![egui::Event::PointerMoved(egui::pos2(300.0, 200.0))], ..Default::default() };
+        assert!(!dismiss_input(&mut raw, true));
+        assert_eq!(raw.events.len(), 1);
+    }
 }

apps/vectorcraft/src/main.rs

@@ -68,6 +68,10 @@ impl eframe::App for App {
         }
     }
     fn raw_input_hook(&mut self, _ctx: &egui::Context, raw: &mut egui::RawInput) {
+        #[cfg(target_os = "macos")]
+        if self.app.services.native_menu.is_some() {
+            mac_menu::raw_input_hook(raw);
+        }
         self.app.raw_input_hook(raw);
     }
     fn ui(&mut self, ui: &mut egui::Ui, _frame: &mut eframe::Frame) {

contributors/people.toml

@@ -25,3 +25,6 @@ sync_github_name = false
 [people.callmeBalloch]
 real_name = "Riccardo Balloch"
 sync_github_name = false
+
+[people.mo0kid]
+sync_github_name = true

crates/engine/src/cmd/typecmd.rs

@@ -530,7 +530,10 @@ mod area_tests {
         s.execute("text.setFormat", &json!({"leadingModel": "emBoxTop"})).unwrap();
         let t = text(&s);
         assert_eq!(t.para.leading_model, LeadingModel::EmBoxTop);
-        assert_ne!(t.cached_bounds, before, "the first line moves up to the frame's top");
+        // The fallback used without craft-fonts has no glyph metrics to move the first line.
+        if !vectorcraft_text::CRAFT_FONTS.is_empty() {
+            assert_ne!(t.cached_bounds, before, "the first line moves up to the frame's top");
+        }
         assert_eq!(serde_json::to_value(&t.para).unwrap()["leading_model"], "emBoxTop");
         s.execute("edit.undo", &json!({})).unwrap();
         assert!(serde_json::to_value(&text(&s).para).unwrap().get("leading_model").is_none());

crates/plugins/src/manifest.rs

@@ -139,10 +139,8 @@ impl Manifest {
                 return Err(Error::Manifest(format!("invalid parameter name {k:?}")));
             }
             match spec {
-                ParamSpec::Number { min, max, default } => {
-                    if !(min.is_finite() && max.is_finite() && default.is_finite()) || min > max {
-                        return Err(Error::Manifest(format!("parameter `{k}`: bad range")));
-                    }
+                ParamSpec::Number { min, max, default } if !(min.is_finite() && max.is_finite() && default.is_finite()) || min > max => {
+                    return Err(Error::Manifest(format!("parameter `{k}`: bad range")));
                 }
                 ParamSpec::Int { min, max, .. } if min > max => return Err(Error::Manifest(format!("parameter `{k}`: bad range"))),
                 ParamSpec::Choice { options, default } => {