From d005ff6472cb6ab1ea0f5a2bb9c94b028fdfb6ff Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Fri, 13 Nov 2026 13:28:42 -0800 Subject: [PATCH] Type: thread checks look up the threads once per command Dynamic Text, Dynamic Text Presets, Center on Top and Center on Bottom, and Area Type Options with Block on checked each object for a thread with its own search of the document's threads. A selection of many objects in a document with many threads repeated that search for each object, in the commands' enabled checks as well as when they ran. The checks now build one lookup from frame to thread for the whole selection. An object counts as threaded by the first thread that lists it, as before, so the commands refuse the same frames, list them in the same order and leave the document and the undo history as they were. --- crates/engine/src/cmd/threads.rs | 47 ++++++++++ crates/engine/src/cmd/threads/tests.rs | 22 +++++ crates/engine/src/cmd/typecmd.rs | 4 +- crates/engine/src/cmd/typemenu.rs | 23 +++-- crates/engine/src/tests/dynamic_text.rs | 113 ++++++++++++++++++++++++ 5 files changed, 194 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/cmd/threads.rs b/crates/engine/src/cmd/threads.rs index c1c3801e40..b364e877f1 100644 --- a/crates/engine/src/cmd/threads.rs +++ b/crates/engine/src/cmd/threads.rs @@ -361,14 +361,61 @@ pub(crate) fn mark_continued(doc: &mut Document) { } } +#[cfg(test)] +thread_local! { + /// Tests: how many times [`story_of`] and [`ThreadIndex::of`] went through the document's + /// threads on this thread. + static THREAD_SCANS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Tests: how many times [`story_of`] and [`ThreadIndex::of`] have gone through the document's +/// threads on this thread. +#[cfg(test)] +pub(crate) fn thread_scans() -> usize { + THREAD_SCANS.with(std::cell::Cell::get) +} + /// The frames of the thread that `id` is a frame of, in thread order, each area type or type on a /// path; empty when `id` is in no thread or the thread has fewer than two such frames. pub(crate) fn story_of(d: &Document, id: NodeId) -> Vec { + #[cfg(test)] + THREAD_SCANS.with(|c| c.set(c.get() + 1)); let Some(thread) = d.text_threads.iter().find(|t| t.contains(&id)) else { return vec![] }; let frames: Vec = thread.iter().copied().filter(|f| text_of(d, *f).is_some()).collect(); if frames.len() >= 2 && frames.contains(&id) { frames } else { vec![] } } +/// A document's text threads looked up by frame, built in one pass over them, for a command that +/// asks [`ThreadIndex::threaded`] of each of many objects. +pub(crate) struct ThreadIndex<'a> { + doc: &'a Document, + /// Each object the threads list, with the index of the first thread that lists it. + first: std::collections::HashMap, + /// Each thread's count of entries that are area type or type on a path. + live: Vec, +} + +impl<'a> ThreadIndex<'a> { + pub(crate) fn of(doc: &'a Document) -> Self { + #[cfg(test)] + THREAD_SCANS.with(|c| c.set(c.get() + 1)); + let mut first = std::collections::HashMap::new(); + let mut live = Vec::with_capacity(doc.text_threads.len()); + for (i, thread) in doc.text_threads.iter().enumerate() { + for id in thread { + first.entry(*id).or_insert(i); + } + live.push(thread.iter().filter(|id| text_of(doc, **id).is_some()).count()); + } + Self { doc, first, live } + } + + /// Whether `id` is a frame of a thread: [`story_of`] is not empty for it. + pub(crate) fn threaded(&self, id: NodeId) -> bool { + self.first.get(&id).and_then(|i| self.live.get(*i)).is_some_and(|n| *n >= 2) && text_of(self.doc, id).is_some() + } +} + /// `ids` followed by the other frames of the threads they are in, each once, in thread order. pub(crate) fn with_threads(d: &Document, ids: &[NodeId]) -> Vec { let mut out = ids.to_vec(); diff --git a/crates/engine/src/cmd/threads/tests.rs b/crates/engine/src/cmd/threads/tests.rs index a622901c01..c490977814 100644 --- a/crates/engine/src/cmd/threads/tests.rs +++ b/crates/engine/src/cmd/threads/tests.rs @@ -245,6 +245,28 @@ fn a_frame_left_alone_by_a_dropped_repeat_stops_continuing() { assert_eq!(t.plain_text(), "Tail"); } +/// [`ThreadIndex::threaded`] answers as [`story_of`] does, by the first thread that lists an +/// object, also for threads listed more than once, with point type, other art and missing ids. +#[test] +fn thread_index_matches_story_of() { + let (mut s, a, b) = setup(); + s.execute("text.thread.create", &json!({})).unwrap(); + let c = s.execute("text.create", &json!({"x": 400, "y": 10, "text": "C", "area": {"width": 150, "height": 60}})).unwrap()["id"].as_u64().unwrap(); + let p = s.execute("text.create", &json!({"x": 10, "y": 250, "text": "Point"})).unwrap()["id"].as_u64().unwrap(); + let r = s.execute("shape.rectangle", &json!({"x": 400, "y": 200, "width": 50, "height": 50})).unwrap()["id"].as_u64().unwrap(); + let mut d = (*s.doc().unwrap().doc).clone(); + let [fa, fb, fc, fp, fr, fx] = [a, b, c, p, r, 9_999_999].map(NodeId); + // a's first thread has one frame beside point type, c's has one beside a rectangle, and b's + // lists a twice. + d.text_threads = vec![vec![fp, fa], vec![fa, fb, fa], vec![fc, fr], vec![fc, fb], vec![fx, fb]]; + let index = ThreadIndex::of(&d); + let got: Vec<(bool, bool)> = [fa, fb, fc, fp, fr, fx].iter().map(|id| (index.threaded(*id), !story_of(&d, *id).is_empty())).collect(); + let want: Vec<(bool, bool)> = [false, true, false, false, false, false].iter().map(|t| (*t, *t)).collect(); + assert_eq!(got, want); + d.text_threads.clear(); + assert!(!ThreadIndex::of(&d).threaded(fb)); +} + #[test] fn edits_copy_only_the_stories_of_threads_whose_frames_changed() { let (mut s, a, b) = setup(); diff --git a/crates/engine/src/cmd/typecmd.rs b/crates/engine/src/cmd/typecmd.rs index e996591f91..94ed16ebd8 100644 --- a/crates/engine/src/cmd/typecmd.rs +++ b/crates/engine/src/cmd/typecmd.rs @@ -1225,8 +1225,8 @@ fn area_options(s: &mut Session, p: &Value) -> Result { .map(|(id, t)| merged(&t.area, t.dynamic_text).map(|o| (id, o))) .collect::>()?; // Dynamic Text's Block is refused for threaded frames, as `type.dynamicText` refuses it. - let frames: Vec = - planned.iter().filter(|(id, (_, block))| *block && !super::threads::story_of(d, *id).is_empty()).map(|(id, _)| id.0.to_string()).collect(); + let threads = super::threads::ThreadIndex::of(d); + let frames: Vec = planned.iter().filter(|(id, (_, block))| *block && threads.threaded(*id)).map(|(id, _)| id.0.to_string()).collect(); if !frames.is_empty() { let frames = frames.join(", "); return Err(bad(C, format!("area type in a thread cannot use Dynamic Text: {frames}"))); diff --git a/crates/engine/src/cmd/typemenu.rs b/crates/engine/src/cmd/typemenu.rs index faada86d5f..87c16bb9b9 100644 --- a/crates/engine/src/cmd/typemenu.rs +++ b/crates/engine/src/cmd/typemenu.rs @@ -7,6 +7,7 @@ use vectorcraft_doc::{AreaFit, CharStyle, Document, Justify, LockedArtboards, No use vectorcraft_geom::{Affine, Rect, shapes}; use super::object::choice; +use super::threads::ThreadIndex; use super::typecmd::{area_text, block_gaps, needed_room, refresh_bounds}; use super::{ CommandSpec, EngineError, Result, Session, bad, checked_ids_param, cmd, find_command, has_clipboard, has_doc, has_selection, number_or, @@ -556,7 +557,7 @@ fn path_center_refusal(d: &Document, ids: &[NodeId]) -> Option { let open = open.join(", "); return Some(format!("type on an open or compound path cannot be centered on its top or bottom: {open}")); } - let frames = threaded_ids(d, ids); + let frames = threaded_ids(&ThreadIndex::of(d), ids); (!frames.is_empty()).then(|| format!("type on a path in a thread cannot be centered on its top or bottom: {frames}")) } @@ -783,11 +784,6 @@ fn path_options(s: &mut Session, p: &Value) -> Result { options(s) } -/// Whether type `id` is a frame of a thread. -fn threaded(d: &Document, id: NodeId) -> bool { - !super::threads::story_of(d, id).is_empty() -} - /// Type on a path or area type: the type Dynamic Text works on. fn dynamic_type(n: Option<&Node>) -> Option<&TextObject> { path_text(n).or_else(|| area_text(n)) @@ -820,17 +816,18 @@ fn has_dynamic_type(s: &Session) -> std::result::Result<(), String> { /// path first; None when none is. fn threaded_refusal(d: &Document, ids: &[NodeId]) -> Option { let (paths, areas): (Vec, Vec) = ids.iter().copied().partition(|id| path_text(d.node(*id)).is_some()); - let frames = threaded_ids(d, &paths); + let threads = ThreadIndex::of(d); + let frames = threaded_ids(&threads, &paths); if !frames.is_empty() { return Some(format!("type on a path in a thread cannot use Dynamic Text: {frames}")); } - let frames = threaded_ids(d, &areas); + let frames = threaded_ids(&threads, &areas); (!frames.is_empty()).then(|| format!("area type in a thread cannot use Dynamic Text: {frames}")) } -/// The ids among `ids` of frames of threads, as a list for a message; empty when there are none. -fn threaded_ids(d: &Document, ids: &[NodeId]) -> String { - ids.iter().filter(|id| threaded(d, **id)).map(|id| id.0.to_string()).collect::>().join(", ") +/// The ids among `ids` of frames of `threads`, as a list for a message; empty when there are none. +fn threaded_ids(threads: &ThreadIndex, ids: &[NodeId]) -> String { + ids.iter().filter(|id| threads.threaded(**id)).map(|id| id.0.to_string()).collect::>().join(", ") } /// Type › Dynamic Text: set `on` (without it, the opposite of the first object's setting, or on @@ -969,7 +966,7 @@ fn has_preset_type(s: &Session) -> std::result::Result<(), String> { if ids.is_empty() { return Err("select point type or type on a path".into()); } - let frames = threaded_ids(d, &ids); + let frames = threaded_ids(&ThreadIndex::of(d), &ids); if !frames.is_empty() { return Err(format!("type on a path in a thread cannot use Dynamic Text: {frames}")); } @@ -1108,7 +1105,7 @@ fn dynamic_text_preset(s: &mut Session, p: &Value) -> Result { if ids.is_empty() { return Err(bad(C, "select point type or type on a path")); } - let frames = threaded_ids(d, &ids); + let frames = threaded_ids(&ThreadIndex::of(d), &ids); if !frames.is_empty() { return Err(bad(C, format!("type on a path in a thread cannot use Dynamic Text: {frames}"))); } diff --git a/crates/engine/src/tests/dynamic_text.rs b/crates/engine/src/tests/dynamic_text.rs index 8605a7e052..0bc13dba7d 100644 --- a/crates/engine/src/tests/dynamic_text.rs +++ b/crates/engine/src/tests/dynamic_text.rs @@ -456,3 +456,116 @@ fn first_text(s: &Session) -> NodeId { }); found.unwrap() } + +/// Whether command `id` is enabled for `s`'s selection, with the reason when it is not. +fn enabled(s: &Session, id: &str) -> std::result::Result<(), String> { + command_specs().iter().find(|c| c.id == id).map_or_else(|| Err(format!("no command {id}")), |c| (c.enabled)(s)) +} + +/// What `f` returned, and how many times it went through the document's text threads to tell +/// which objects are frames of one. +fn scans(f: impl FnOnce() -> T) -> (T, usize) { + let before = crate::cmd::threads::thread_scans(); + let out = f(); + (out, crate::cmd::threads::thread_scans() - before) +} + +/// Threads `a` with a new 100 pt square frame at (`x`, `y`); the thread's frames. +fn thread_with_frame(s: &mut Session, a: NodeId, x: f64, y: f64) -> Vec { + let frame = s.execute("shape.rectangle", &json!({"x": x, "y": y, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.set", &json!({"ids": [a.0, frame]})).unwrap(); + let r = s.execute("text.thread.create", &json!({})).unwrap(); + r["ids"].as_array().unwrap().iter().map(|v| v.as_u64().unwrap()).collect() +} + +/// `ids` as a type command's refusal lists them. +fn listed(ids: &[u64]) -> String { + ids.iter().map(u64::to_string).collect::>().join(", ") +} + +/// Dynamic Text, its presets, Center on Top and Bottom and Area Type Options look up the threads +/// once for a whole selection, not once for each object, and refuse the same frames in the same +/// order. +#[test] +fn type_commands_look_up_threads_once_for_the_whole_selection() { + const N: usize = 24; + let mut s = session(); + let area = |s: &mut Session, i: usize| { + let x = 10.0 + 30.0 * i as f64; + let r = s.execute("text.create", &json!({"x": x, "y": 10, "text": format!("Area {i}"), "area": {"width": 25, "height": 40}})).unwrap(); + NodeId(r["id"].as_u64().unwrap()) + }; + // Area type: every other one is the first frame of a thread of two. + let (mut areas, mut threaded) = (vec![], vec![]); + for i in 0..N { + let a = area(&mut s, i); + areas.push(a.0); + if i % 2 == 0 { + let frames = thread_with_frame(&mut s, a, 10.0 + 30.0 * i as f64, 500.0); + assert_eq!(frames.len(), 2, "{frames:?}"); + areas.push(frames[1]); + threaded.extend(frames); + } + } + // The refusal lists the threaded frames in the order the objects are given. + areas.reverse(); + let refused: Vec = areas.iter().copied().filter(|id| threaded.contains(id)).collect(); + let before = undo_len(&s); + s.execute("select.set", &json!({ "ids": areas })).unwrap(); + let why = format!("area type in a thread cannot use Dynamic Text: {}", listed(&refused)); + let (e, n) = scans(|| enabled(&s, "type.dynamicText")); + assert_eq!((e, n), (Err(why.clone()), 1)); + let (e, n) = scans(|| err(s.execute("type.dynamicText", &json!({"on": true, "ids": areas})))); + assert!(e.ends_with(&why) && n == 1, "{n}: {e}"); + let (e, n) = scans(|| err(s.execute("text.areaOptions", &json!({"fit": "block", "ids": areas})))); + assert!(e.ends_with(&why) && n == 1, "{n}: {e}"); + assert_eq!(undo_len(&s), before); + + // Type on a path in a thread comes first, before the area type. + let path = on_line(&mut s, "Threaded path"); + let path_frames = thread_with_frame(&mut s, path, 700.0, 500.0); + let mut mixed = areas.clone(); + mixed.push(path.0); + s.execute("select.set", &json!({ "ids": mixed })).unwrap(); + let why = format!("type on a path in a thread cannot use Dynamic Text: {}", path.0); + let (e, n) = scans(|| enabled(&s, "type.dynamicText")); + assert_eq!((e, n), (Err(why.clone()), 1)); + let (e, n) = scans(|| err(s.execute("type.dynamicText", &json!({"on": true, "ids": mixed})))); + assert!(e.ends_with(&why) && n == 1, "{n}: {e}"); + + // Presets: point type beside type on a path in a thread. + let mut points: Vec = (0..N) + .map(|i| s.execute("text.create", &json!({"x": 10.0 + 30.0 * i as f64, "y": 300, "text": "P"})).unwrap()["id"].as_u64().unwrap()) + .collect(); + points.insert(N / 2, path.0); + s.execute("select.set", &json!({ "ids": points })).unwrap(); + let (e, n) = scans(|| enabled(&s, "type.dynamicTextPreset")); + assert_eq!((e, n), (Err(why.clone()), 1)); + let (e, n) = scans(|| err(s.execute("type.dynamicTextPreset", &json!({"preset": "arch", "ids": points})))); + assert!(e.ends_with(&why) && n == 1, "{n}: {e}"); + + // Center on Top and Bottom: type on closed paths, some of it in threads. + let mut circles = vec![]; + let mut in_threads = vec![]; + for i in 0..N { + let (x, y) = (60.0 + 30.0 * i as f64, 200.0); + let d = format!("M{} {y} A12 12 0 1 1 {} {y} A12 12 0 1 1 {} {y} Z", x - 12.0, x + 12.0, x - 12.0); + let pid = s.execute("path.create", &json!({ "d": d })).unwrap()["id"].as_u64().unwrap(); + let r = s.execute("text.createInPath", &json!({"path": pid, "mode": "onPath", "text": "O", "size": 6, "at": [x - 12.0, y]})).unwrap(); + let id = NodeId(r["id"].as_u64().unwrap()); + if i % 3 == 0 { + thread_with_frame(&mut s, id, x, 650.0); + in_threads.push(id.0); + } + circles.push(id.0); + } + s.execute("select.set", &json!({ "ids": circles })).unwrap(); + let why = format!("type on a path in a thread cannot be centered on its top or bottom: {}", listed(&in_threads)); + for c in ["type.pathCenter.top", "type.pathCenter.bottom"] { + let (e, n) = scans(|| enabled(&s, c)); + assert_eq!((e, n), (Err(why.clone()), 1), "{c}"); + let (e, n) = scans(|| err(s.execute(c, &json!({ "ids": circles })))); + assert!(e.ends_with(&why) && n == 1, "{c} {n}: {e}"); + } + assert_eq!(path_frames.len(), 2); +}