From e33906976d91ad615625831633d304264a0e2aa7 Mon Sep 17 00:00:00 2001 From: HassanKhan Date: Tue, 10 Nov 2026 10:00:03 +0500 Subject: [PATCH 1/2] Spelling right-click: only the topmost type's word; dictionary tests one at a time (part of #4936) --- crates/engine/src/tests/spelling.rs | 17 ++++++++ crates/ui-egui/src/canvas/tool_overlays.rs | 49 ++++++++++++---------- crates/ui-egui/src/tests/contextmenu.rs | 22 ++++++++++ 3 files changed, 67 insertions(+), 21 deletions(-) diff --git a/crates/engine/src/tests/spelling.rs b/crates/engine/src/tests/spelling.rs index 0dfc9fab5..eca38d8e6 100644 --- a/crates/engine/src/tests/spelling.rs +++ b/crates/engine/src/tests/spelling.rs @@ -6,6 +6,14 @@ use serde_json::{Value, json}; use crate::cmd::spelling::MAX_DICTIONARY_FILE_BYTES; use crate::*; +/// Held by each test that changes the custom dictionary, which the whole process shares: a save +/// writes every word in it to the test's file and a reload takes the file's words back, so another +/// test's word could come back after that test deleted it. +fn dictionary_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + /// A session with a new 400 × 400 document. fn session() -> Session { let mut s = Session::new(); @@ -115,6 +123,7 @@ fn marked(s: &mut Session) -> Vec { #[test] fn added_words_are_right_in_every_check_until_deleted() { + let _one_at_a_time = dictionary_lock(); let mut s = session(); text(&mut s, 50.0, "drawing Zqvwkx tools"); assert_eq!(marked(&mut s), ["Zqvwkx"]); @@ -137,6 +146,7 @@ fn added_words_are_right_in_every_check_until_deleted() { #[test] fn a_changed_word_replaces_the_old_one() { + let _one_at_a_time = dictionary_lock(); let mut s = session(); text(&mut s, 50.0, "xkvzqa xkvzqb"); run(&mut s, "text.spelling.addWord", json!({"word": "xkvzqa"})); @@ -150,6 +160,7 @@ fn a_changed_word_replaces_the_old_one() { #[test] fn the_custom_dictionary_file_is_read_and_kept_up_to_date() { + let _one_at_a_time = dictionary_lock(); let dir = vectorcraft_testkit::temp_dir("spelling-dictionary"); let file = dir.join("Custom Dictionary.txt"); std::fs::write(&file, "kqzvwx\nnot one word\n\n").unwrap(); @@ -170,6 +181,7 @@ fn the_custom_dictionary_file_is_read_and_kept_up_to_date() { #[test] fn ignore_all_skips_a_word_in_the_active_document_only() { + let _one_at_a_time = dictionary_lock(); let mut s = session(); text(&mut s, 50.0, "Wqzxkv drawing wqzxkv vqzxkw"); run(&mut s, "text.spelling.ignoreAll", json!({"word": "WQZXKV"})); @@ -183,6 +195,7 @@ fn ignore_all_skips_a_word_in_the_active_document_only() { #[test] fn custom_dictionary_commands_refuse_what_isnt_one_word() { + let _one_at_a_time = dictionary_lock(); let mut s = session(); let long = "q".repeat(65); for (id, p) in [ @@ -210,6 +223,7 @@ fn in_dictionary(s: &mut Session, word: &str) -> bool { #[test] fn a_dictionary_file_that_cant_be_read_whole_is_never_written_over() { + let _one_at_a_time = dictionary_lock(); let dir = vectorcraft_testkit::temp_dir("spelling-unreadable"); // A folder where the file should be can't be read as one; a file past what craft-proof reads // would lose its other words if written back. @@ -231,6 +245,7 @@ fn a_dictionary_file_that_cant_be_read_whole_is_never_written_over() { #[test] fn a_change_that_cant_be_saved_is_taken_back() { + let _one_at_a_time = dictionary_lock(); let mut s = session(); run(&mut s, "text.spelling.addWord", json!({"word": "wkqzvx"})); let dir = vectorcraft_testkit::temp_dir("spelling-rollback"); @@ -248,6 +263,7 @@ fn a_change_that_cant_be_saved_is_taken_back() { #[cfg(unix)] #[test] fn a_word_another_window_saved_stays_when_this_save_fails() { + let _one_at_a_time = dictionary_lock(); use std::os::unix::fs::PermissionsExt; let dir = vectorcraft_testkit::temp_dir("spelling-readonly"); let file = dir.join("Custom Dictionary.txt"); @@ -296,6 +312,7 @@ fn misspellings_of_one_object_are_what_the_check_lists() { /// what Auto Spell Check found before is checked again. #[test] fn custom_dictionary_changes_move_its_generation_on() { + let _one_at_a_time = dictionary_lock(); use crate::cmd::spelling::dictionary_generation; let mut s = session(); let mut last = dictionary_generation(); diff --git a/crates/ui-egui/src/canvas/tool_overlays.rs b/crates/ui-egui/src/canvas/tool_overlays.rs index 5e23522f7..0151b3558 100644 --- a/crates/ui-egui/src/canvas/tool_overlays.rs +++ b/crates/ui-egui/src/canvas/tool_overlays.rs @@ -166,41 +166,48 @@ fn auto_spell_checks(n: &Node) -> bool { !shown(&n.appearance.effects) && !n.appearance.items.iter().any(|i| shown(i.effects())) } -/// The word with Auto Spell Check's red line at `p` (document space), on top of the others: in -/// the topmost visible, unlocked type object whose unknown word's glyphs `p` is on, with what its -/// language's dictionary suggests instead. For the context menu's suggestions, Ignore All and Add +/// The word with Auto Spell Check's red line at `p` (document space): in the topmost visible type +/// object whose glyphs `p` is on, when that object isn't locked and Auto Spell Check marks it and +/// its word at `p` is one it underlines, with what its language's dictionary suggests instead. For the context menu's suggestions, Ignore All and Add /// to Dictionary ([Check and correct spelling](https://helpx.adobe.com/illustrator/desktop/design-with-text/edit-format-text/check-and-correct-spelling.html)). pub(crate) fn misspelled_word_at(app: &mut VectorcraftApp, p: Point) -> Option { let st = app.session.active()?; - let mut hit = None; - fn walk(nodes: &[std::sync::Arc], f: &mut impl FnMut(&Node)) { - for n in nodes.iter().filter(|n| n.visible && !n.locked) { - f(n); + // The topmost type whose glyphs `p` is on, and whether it or a layer or group holding it is + // locked: a word under other type isn't the one right-clicked, whether that type's word there + // is right or not. + let mut top: Option<(&Node, bool)> = None; + fn walk<'a>(nodes: &'a [std::sync::Arc], locked: bool, f: &mut impl FnMut(&'a Node, bool)) { + for n in nodes.iter().filter(|n| n.visible) { + let locked = locked || n.locked; + f(n, locked); if let Some(children) = n.children() { - walk(children, f); + walk(children, locked, f); } } } // Paint order: a later hit is on top. - walk(&st.doc.layers, &mut |n| { + walk(&st.doc.layers, false, &mut |n, locked| { let NodeKind::Text(tx) = &n.kind else { return }; - if !auto_spell_checks(n) || n.geometric_bounds().is_none_or(|b| !b.contains(p)) { - return; - } - let (words, _) = vectorcraft_engine::cmd::spelling::misspellings(tx, &st.spelling_ignored, MAX_UNDERLINED); - if words.is_empty() { + if n.geometric_bounds().is_none_or(|b| !b.contains(p)) { return; } let lay = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), tx); - for w in words { - let on = vectorcraft_text::selection_quads(&lay, w.range.start, w.range.end).iter().any(|q| in_quad(q.map(|c| tx.xf * c), p)); - if on { - hit = Some((n.id, w)); - } + let on = vectorcraft_text::selection_quads(&lay, 0, tx.plain_text().len()).iter().any(|q| in_quad(q.map(|c| tx.xf * c), p)); + if on { + top = Some((n, locked)); } }); - let (id, w) = hit?; - let Some(NodeKind::Text(tx)) = st.doc.node(id).map(|n| &n.kind) else { return None }; + let (n, locked) = top?; + let NodeKind::Text(tx) = &n.kind else { return None }; + if locked || !auto_spell_checks(n) { + return None; + } + let (words, _) = vectorcraft_engine::cmd::spelling::misspellings(tx, &st.spelling_ignored, MAX_UNDERLINED); + let lay = vectorcraft_text::layout(vectorcraft_text::FontDb::global(), tx); + let w = words + .into_iter() + .find(|w| vectorcraft_text::selection_quads(&lay, w.range.start, w.range.end).iter().any(|q| in_quad(q.map(|c| tx.xf * c), p)))?; + let id = n.id; let word = tx.plain_text().get(w.range.clone())?.to_string(); let uid = st.uid; let suggestions = app diff --git a/crates/ui-egui/src/tests/contextmenu.rs b/crates/ui-egui/src/tests/contextmenu.rs index 0812f410e..830da939d 100644 --- a/crates/ui-egui/src/tests/contextmenu.rs +++ b/crates/ui-egui/src/tests/contextmenu.rs @@ -506,3 +506,25 @@ fn no_spelling_items_off_a_misspelled_word_or_with_auto_spell_check_off() { let texts = click(&mut app, &ctx, p, PointerButton::Secondary); assert!(!has(&texts, "Ignore All"), "off: {texts:?}"); } + +/// A right word in type on top of an underlined word in other type: the right-click is on the +/// top type, so it lists no spelling items for the word under it. +#[test] +fn a_word_under_other_type_is_not_the_one_right_clicked() { + let mut app = app(); + let under = spelled(&mut app, "Qzvxtrwk"); + let ctx = egui::Context::default(); + frame(&mut app, &ctx, vec![]); + let p = word_at(&app, under, 0, 8); + let texts = click(&mut app, &ctx, p, PointerButton::Secondary); + assert!(has(&texts, "Ignore All"), "alone, the word is offered: {texts:?}"); + frame( + &mut app, + &ctx, + vec![Event::Key { key: egui::Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Default::default() }], + ); + app.run("text.create", json!({"x": 20, "y": 60, "text": "the cat sat on the mat", "size": 24})).unwrap(); + app.run("select.none", json!({})).unwrap(); + let texts = click(&mut app, &ctx, p, PointerButton::Secondary); + assert!(!has(&texts, "Ignore All"), "under right type: {texts:?}"); +} From 0d7c6dd6e21359389c780e06c5662954a016fcc3 Mon Sep 17 00:00:00 2001 From: HassanKhan Date: Tue, 10 Nov 2026 10:48:52 +0500 Subject: [PATCH 2/2] Spelling right-click: Intertwine members by their tier at the point (part of #4936) --- crates/ui-egui/src/canvas/tool_overlays.rs | 25 +++++++++----- crates/ui-egui/src/tests/contextmenu.rs | 40 ++++++++++++++++++++++ 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/canvas/tool_overlays.rs b/crates/ui-egui/src/canvas/tool_overlays.rs index 0151b3558..67f6ac861 100644 --- a/crates/ui-egui/src/canvas/tool_overlays.rs +++ b/crates/ui-egui/src/canvas/tool_overlays.rs @@ -175,18 +175,25 @@ pub(crate) fn misspelled_word_at(app: &mut VectorcraftApp, p: Point) -> Option = None; - fn walk<'a>(nodes: &'a [std::sync::Arc], locked: bool, f: &mut impl FnMut(&'a Node, bool)) { + let mut top: Option<(vectorcraft_doc::NodeId, bool)> = None; + // Back to front as drawn at `p`: an Intertwine group's members by their tier there, those of + // one tier in their stacking order ([`vectorcraft_doc::intertwine`]). + fn walk(nodes: &[std::sync::Arc], p: Point, locked: bool, f: &mut impl FnMut(&Node, bool)) { for n in nodes.iter().filter(|n| n.visible) { let locked = locked || n.locked; f(n, locked); - if let Some(children) = n.children() { - walk(children, locked, f); + let Some(children) = n.children() else { continue }; + if matches!(n.kind, NodeKind::Intertwine { .. }) { + let mut drawn = children.to_vec(); + drawn.sort_by_key(|c| vectorcraft_doc::intertwine::tier_at(c, p)); + walk(&drawn, p, locked, f); + } else { + walk(children, p, locked, f); } } } - // Paint order: a later hit is on top. - walk(&st.doc.layers, false, &mut |n, locked| { + // A later hit is on top. + walk(&st.doc.layers, p, false, &mut |n, locked| { let NodeKind::Text(tx) = &n.kind else { return }; if n.geometric_bounds().is_none_or(|b| !b.contains(p)) { return; @@ -194,10 +201,11 @@ pub(crate) fn misspelled_word_at(app: &mut VectorcraftApp, p: Point) -> Option Option