From 20bba9ebaef3103f5bb19bad4affd6e24e41cb41 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 02:05:26 +0000 Subject: [PATCH] Security: linear work for shortcut review, gradients, recovery names, include cycles, batch references, DOCX style ids, CAD layouts and long strokes CodeRabbit findings #2, #16, #31, #103, #114, #124, #162, #179, #182, #183, #185: - Shortcut import review: example names for each code in one pass over the rows. - Gradients: a midpoint stop is mixed from its two stops; freeform recolor samples its increasing positions with one advancing search (`Gradient::sample_increasing_with`). - Recovery: image entry names are looked up by address, and dead ones swept when the map doubles, not on every image. - Script checker: include cycles from strongly connected components (iterative Tarjan), not a reachability row per file. - Batch step references copy at most 256 MB of values for one step. - DOCX: style ids over 256 bytes are ignored where defined and where used. - DXF: layouts keep at most 4 MB of names; a hatch's loops count against the node limit. - Freehand and Liquify: past 1000 samples a drag previews every n/1000 samples, strokes keep at most 100,000 samples, and the stroke kept is previewed in full before it is committed. Co-Authored-By: Claude Opus 5.5 --- crates/cad/src/import/build.rs | 9 ++- crates/cad/src/import/reader.rs | 19 ++++- crates/color/src/freeform.rs | 5 +- crates/color/src/gradient.rs | 64 +++++++++++++-- crates/engine/src/cmd/place/docx.rs | 21 ++++- crates/engine/src/cmd/recovery.rs | 30 ++++++-- crates/engine/src/steps.rs | 68 ++++++++++++++-- crates/scriptcheck/src/includes.rs | 77 ++++++++++++++----- crates/tools/src/distort/liquify.rs | 51 +++++++++--- crates/tools/src/draw2/gesture.rs | 16 +++- crates/tools/src/lib.rs | 14 ++++ crates/ui-egui/src/dialogs/shortcut_import.rs | 13 +++- 12 files changed, 322 insertions(+), 65 deletions(-) diff --git a/crates/cad/src/import/build.rs b/crates/cad/src/import/build.rs index c65862e67..1e43c45dc 100644 --- a/crates/cad/src/import/build.rs +++ b/crates/cad/src/import/build.rs @@ -495,7 +495,14 @@ impl<'d, 'a> Build<'d, 'a> { let path = PathData::from_bezpath(&bp); let kind = |path: PathData| NodeKind::Path { path, rule, live: None, clipping: false, guide: false }; let mut n = if path.subpaths.len() > 1 { - // Several loops (a hatch with islands) paint as one compound path. + // Several loops (a hatch with islands) paint as one compound path. Each loop is an + // object of its own, so it counts against the node limit like any other (`item` + // counts the compound path itself). + if self.nodes.saturating_add(path.subpaths.len()) >= MAX_NODES { + self.warn(TOO_MANY); + return None; + } + self.nodes += path.subpaths.len(); let children = path .subpaths .into_iter() diff --git a/crates/cad/src/import/reader.rs b/crates/cad/src/import/reader.rs index a2d1dc11d..139f35771 100644 --- a/crates/cad/src/import/reader.rs +++ b/crates/cad/src/import/reader.rs @@ -421,21 +421,32 @@ fn font_family(g: &[Pair<'_>]) -> String { /// Model space, then each paper layout (its LAYOUT object, in tab order); a file without layout /// objects but with paper-space art has one paper layout. +/// The most bytes of layout and block names the layouts keep together. Each layout keeps its own +/// copy of its block's name, so many layouts naming one long block name multiplied it; real +/// drawings have a few dozen layouts with short names. +const MAX_LAYOUT_NAME_BYTES: usize = 4 << 20; + fn layouts(objects: &[Record<'_>], block_records: &HashMap, entities: &[Record<'_>]) -> Vec { let mut paper: Vec<(i64, LayoutDef)> = vec![]; let mut seen = std::collections::HashSet::new(); + let mut name_bytes = 0usize; for r in objects.iter().filter(|r| r.kind == "LAYOUT") { // The layout's own groups follow its subclass marker. let own = r.g.iter().position(|p| p.code == 100 && p.value.trim() == "AcDbLayout").and_then(|i| r.g.get(i + 1..)).unwrap_or(&[]); let Some(name) = own.name(1) else { continue }; - let block = own.name(330).and_then(|h| block_records.get(&h.to_uppercase())).cloned(); - if name.eq_ignore_ascii_case(MODEL) || block.as_deref() == Some(MODEL_BLOCK) { + let block = own.name(330).and_then(|h| block_records.get(&h.to_uppercase())); + if name.eq_ignore_ascii_case(MODEL) || block.map(String::as_str) == Some(MODEL_BLOCK) { continue; } let Some(block) = block else { continue }; - if seen.insert(name.to_ascii_lowercase()) { - paper.push((own.int_or(71, i64::MAX), LayoutDef { name: name.to_string(), block: Some(block) })); + if !seen.insert(name.to_ascii_lowercase()) { + continue; } + name_bytes = name_bytes.saturating_add(name.len() + block.len()); + if name_bytes > MAX_LAYOUT_NAME_BYTES { + break; + } + paper.push((own.int_or(71, i64::MAX), LayoutDef { name: name.to_string(), block: Some(block.clone()) })); } paper.sort_by_key(|(order, _)| *order); if paper.is_empty() && groups(entities).any(in_paper_space) { diff --git a/crates/color/src/freeform.rs b/crates/color/src/freeform.rs index 751e23896..798367ba4 100644 --- a/crates/color/src/freeform.rs +++ b/crates/color/src/freeform.rs @@ -162,8 +162,9 @@ impl Freeform { /// ([`Color::lerp_keep_model`]). pub fn recolor(&mut self, g: &Gradient) { let n = self.points.len(); - for (k, p) in self.points.iter_mut().enumerate() { - (p.color, p.opacity) = g.sample_with(k as f32 / (n - 1).max(1) as f32, Color::lerp_keep_model); + let at = (0..n).map(|k| k as f32 / (n - 1).max(1) as f32); + for (p, (color, opacity)) in self.points.iter_mut().zip(g.sample_increasing_with(at, Color::lerp_keep_model)) { + (p.color, p.opacity) = (color, opacity); } } diff --git a/crates/color/src/gradient.rs b/crates/color/src/gradient.rs index e97ca634a..4af32cf85 100644 --- a/crates/color/src/gradient.rs +++ b/crates/color/src/gradient.rs @@ -133,6 +133,16 @@ impl GradientStop { } } +/// The colour and opacity at `t` between stops `a` and `b`, through `a`'s midpoint (`u` = midpoint +/// maps to `v` = ½), mixed by `mix`. +fn between(a: &GradientStop, b: &GradientStop, t: f32, mix: &impl Fn(&Color, &Color, f32) -> Color) -> (Color, f32) { + let span = (b.offset - a.offset).max(1e-6); + let u = (t - a.offset) / span; + let m = a.midpoint.clamp(0.01, 0.99); + let v = if u < m { 0.5 * u / m } else { 0.5 + 0.5 * (u - m) / (1.0 - m) }; + (mix(&a.color, &b.color, v), a.opacity + (b.opacity - a.opacity) * v) +} + fn one() -> f32 { 1.0 } @@ -191,12 +201,7 @@ impl Gradient { for w in stops.windows(2) { let (a, b) = (&w[0], &w[1]); if t <= b.offset { - let span = (b.offset - a.offset).max(1e-6); - let u = (t - a.offset) / span; - // Map through the midpoint: u=mid → 0.5. - let m = a.midpoint.clamp(0.01, 0.99); - let v = if u < m { 0.5 * u / m } else { 0.5 + 0.5 * (u - m) / (1.0 - m) }; - return (mix(&a.color, &b.color, v), a.opacity + (b.opacity - a.opacity) * v); + return between(a, b, t, &mix); } } match stops.last() { @@ -204,6 +209,29 @@ impl Gradient { None => (Color::BLACK, 1.0), } } + /// [`Self::sample_with`] at each of `ts`, which must not decrease. Each search starts where + /// the one before it stopped, so the work is the samples plus the stops, not their product. + pub fn sample_increasing_with(&self, ts: impl IntoIterator, mix: impl Fn(&Color, &Color, f32) -> Color) -> Vec<(Color, f32)> { + let stops = &self.stops; + let mut j = 0; + ts.into_iter() + .map(|t| { + let Some(first) = stops.first() else { return (Color::BLACK, 1.0) }; + if t <= first.offset { + return (first.color, first.opacity); + } + // The first pair whose end `t` doesn't pass, as `sample_with` finds it: one that a + // smaller `t` passed, this one passes too. + while j + 1 < stops.len() && t > stops[j + 1].offset { + j += 1; + } + match (stops.get(j), stops.get(j + 1)) { + (Some(a), Some(b)) => between(a, b, t, &mix), + _ => stops.last().map_or((Color::BLACK, 1.0), |l| (l.color, l.opacity)), + } + }) + .collect() + } /// Stops expanded so that midpoints are represented as explicit stops (for renderers without /// midpoints), and a Perceptual gradient's segments as stops on its curve (for renderers that /// mix linearly; within [`PERCEPTUAL_TOLERANCE`] of it). @@ -219,7 +247,9 @@ impl Gradient { // Classic: a stop at the midpoint, when it is off-centre. let classic = next.filter(|_| off_centre && self.method.is_classic()).map(|n| { let t = s.offset + (n.offset - s.offset) * s.midpoint; - let (c, o) = self.sample(t); + // Mixed from the two stops it lies between, as `sample` would: searching all the + // stops again for each midpoint made many stops cost their square. + let (c, o) = between(s, n, t, &|a: &Color, b: &Color, v| self.method.mix(a, b, v)); (t, c, o, Some(s.midpoint)) }); // Perceptual: stops along the mix (`v`) where a straight line would stray from it, the @@ -776,6 +806,26 @@ mod tests { assert!((mid - 0.5).abs() < 1e-5); } + /// Sampling increasing positions in one pass gives what sampling each alone does, out-of-order + /// and repeated stop offsets included. + #[test] + fn increasing_samples_match_single_samples() { + let stop = |offset: f32, hex: &str, midpoint: f32| GradientStop { midpoint, ..GradientStop::new(offset, Color::from_hex(hex).unwrap()) }; + let g = Gradient::new( + GradientKind::Linear, + vec![ + stop(0.1, "#ff0000", 0.3), + stop(0.4, "#00ff00", 0.7), + stop(0.4, "#0000ff", 0.5), + stop(0.2, "#ffff00", 0.5), + stop(0.9, "#000000", 0.5), + ], + ); + let ts: Vec = (0..=40).map(|k| k as f32 / 40.0).collect(); + let one_by_one: Vec<_> = ts.iter().map(|&t| g.sample_with(t, Color::lerp_keep_model)).collect(); + assert_eq!(g.sample_increasing_with(ts, Color::lerp_keep_model), one_by_one); + } + #[test] fn midpoint_shifts() { let mut g = Gradient::default(); diff --git a/crates/engine/src/cmd/place/docx.rs b/crates/engine/src/cmd/place/docx.rs index bf394f63c..c67c77ca4 100644 --- a/crates/engine/src/cmd/place/docx.rs +++ b/crates/engine/src/cmd/place/docx.rs @@ -77,6 +77,10 @@ const MAX_BASED_ON: usize = 32; const MAX_FONTS: usize = 4096; /// The most bytes of a font name kept. const MAX_NAME: usize = 256; +/// The most bytes of a style id: a longer one (Word's are under 256 characters) is ignored, both +/// where it is defined and where it is used, since every paragraph and run looks its style up by +/// it. +const MAX_STYLE_ID: usize = 256; /// The most paragraphs placed type takes from one Word document. const MAX_PARAGRAPHS: usize = 100_000; /// The most runs of one character formatting placed type takes from one Word document. @@ -484,7 +488,7 @@ impl Reader { } } Some("style") if self.styles.len() < MAX_STYLES => { - let Some(id) = attr(n, "styleId") else { continue }; + let Some(id) = attr(n, "styleId").filter(|id| id.len() <= MAX_STYLE_ID) else { continue }; let default = match attr(n, "type") { Some("paragraph") => Some(&mut self.default_style), Some("character") => Some(&mut self.default_char_style), @@ -495,7 +499,7 @@ impl Reader { } let run = child(n, "rPr").map(|r| self.run_props(r)).unwrap_or_default(); let para = child(n, "pPr").map(para_props).unwrap_or_default(); - let based_on = child(n, "basedOn").and_then(|b| attr(b, "val")).map(str::to_string); + let based_on = child(n, "basedOn").and_then(|b| attr(b, "val")).filter(|v| v.len() <= MAX_STYLE_ID).map(str::to_string); self.styles.entry(id.to_string()).or_insert(Style { based_on, run, para }); } _ => {} @@ -637,7 +641,12 @@ impl Reader { fn paragraph(&mut self, p: Node, depth: usize) { self.flush(); let ppr = child(p, "pPr"); - let style = ppr.and_then(|n| child(n, "pStyle")).and_then(|n| attr(n, "val")).map(str::to_string).or_else(|| self.default_style.clone()); + let style = ppr + .and_then(|n| child(n, "pStyle")) + .and_then(|n| attr(n, "val")) + .filter(|v| v.len() <= MAX_STYLE_ID) + .map(str::to_string) + .or_else(|| self.default_style.clone()); let mut para = Para::default(); para.apply(&self.default_para); let mut base = Chars::default(); @@ -688,7 +697,11 @@ impl Reader { fn run(&mut self, r: Node) { let rpr = child(r, "rPr"); let mut chars = self.base; - let style = rpr.and_then(|n| child(n, "rStyle")).and_then(|n| attr(n, "val")).or(self.default_char_style.as_deref()); + let style = rpr + .and_then(|n| child(n, "rStyle")) + .and_then(|n| attr(n, "val")) + .filter(|v| v.len() <= MAX_STYLE_ID) + .or(self.default_char_style.as_deref()); for s in self.chain(style) { chars.apply(&s.run, true); } diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index 44314a143..02c84ae2f 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -589,17 +589,35 @@ static WRITES: Mutex<()> = Mutex::new(()); type Named = (Weak>, String); /// The entry names of image bytes already hashed ([`blob_name`]), so an image is hashed once, -/// not at every interval. Bytes behind a live `Weak` never change (`Arc::make_mut` copies them). -static NAMES: Mutex> = Mutex::new(Vec::new()); +/// not at every interval, keyed by the bytes' address. Bytes behind a live `Weak` never change +/// (`Arc::make_mut` copies them). +static NAMES: Mutex = Mutex::new(Names { by_address: None, live_after_sweep: 0 }); + +struct Names { + by_address: Option>, + /// How many entries were live at the last sweep of dead ones: the next sweep waits until there + /// are twice as many, so sweeping costs a constant per name, not a pass over all of them. + live_after_sweep: usize, +} /// The name of the entry holding `bytes`: from their content (128 bits of hash and the length), so /// identical images share one entry and one written before isn't written again. fn blob_name(bytes: &Arc>) -> String { - let mut names = NAMES.lock().unwrap_or_else(|e| e.into_inner()); - names.retain(|(w, _)| w.strong_count() > 0); - if let Some((_, name)) = names.iter().find(|(w, _)| std::ptr::eq(w.as_ptr(), Arc::as_ptr(bytes))) { + let mut guard = NAMES.lock().unwrap_or_else(|e| e.into_inner()); + let Names { by_address, live_after_sweep } = &mut *guard; + let names = by_address.get_or_insert_with(Default::default); + let address = Arc::as_ptr(bytes) as usize; + // A dead entry at this address named other bytes that were freed there before. + if let Some((w, name)) = names.get(&address) + && w.strong_count() > 0 + && std::ptr::eq(w.as_ptr(), Arc::as_ptr(bytes)) + { return name.clone(); } + if names.len() >= (*live_after_sweep * 2).max(64) { + names.retain(|_, (w, _)| w.strong_count() > 0); + *live_after_sweep = names.len(); + } let hash = |seed: u8| { let mut h = std::collections::hash_map::DefaultHasher::new(); seed.hash(&mut h); @@ -607,7 +625,7 @@ fn blob_name(bytes: &Arc>) -> String { h.finish() }; let name = format!("img-{:016x}{:016x}-{}{BLOB_EXT}", hash(1), hash(2), bytes.len()); - names.push((Arc::downgrade(bytes), name.clone())); + names.insert(address, (Arc::downgrade(bytes), name.clone())); name } diff --git a/crates/engine/src/steps.rs b/crates/engine/src/steps.rs index f29cdacda..b767aa4e9 100644 --- a/crates/engine/src/steps.rs +++ b/crates/engine/src/steps.rs @@ -11,13 +11,20 @@ use serde_json::Value; /// Most nesting [`resolve`] walks into: deeper parameters are left as they are. const MAX_DEPTH: usize = 64; +/// The most the values references copy in may weigh together ([`weight`]: about their JSON size), +/// for one step's parameters. Each reference copies its value, so a short list of references to +/// one large result multiplied it. +const MAX_COPIED: usize = 256 << 20; + /// `params` with every step reference replaced from `results` (the results of the steps run so -/// far, in order); an error names a reference that resolves to nothing. +/// far, in order); an error names a reference that resolves to nothing, or the reference past +/// which the copied values weigh more than [`MAX_COPIED`]. pub fn resolve(params: &Value, results: &[Value]) -> Result { - resolve_at(params, results, 0) + let mut allowance = MAX_COPIED; + resolve_at(params, results, 0, &mut allowance) } -fn resolve_at(v: &Value, results: &[Value], depth: usize) -> Result { +fn resolve_at(v: &Value, results: &[Value], depth: usize, allowance: &mut usize) -> Result { if depth > MAX_DEPTH { return Ok(v.clone()); } @@ -25,16 +32,49 @@ fn resolve_at(v: &Value, results: &[Value], depth: usize) -> Result match s.strip_prefix("$$") { Some(rest) => Value::String(format!("${rest}")), None => match parse(s) { - Some((step, path)) => lookup(s, step, &path, results)?, + Some((step, path)) => { + let found = lookup(s, step, &path, results)?; + *allowance = weight(found, *allowance) + .and_then(|w| allowance.checked_sub(w)) + .ok_or_else(|| format!("`{s}`: the step's references copy in more than {} MB of values", MAX_COPIED >> 20))?; + found.clone() + } None => v.clone(), }, }, - Value::Array(a) => Value::Array(a.iter().map(|x| resolve_at(x, results, depth + 1)).collect::>()?), - Value::Object(o) => Value::Object(o.iter().map(|(k, x)| Ok((k.clone(), resolve_at(x, results, depth + 1)?))).collect::>()?), + Value::Array(a) => Value::Array(a.iter().map(|x| resolve_at(x, results, depth + 1, allowance)).collect::>()?), + Value::Object(o) => { + Value::Object(o.iter().map(|(k, x)| Ok((k.clone(), resolve_at(x, results, depth + 1, allowance)?))).collect::>()?) + } _ => v.clone(), }) } +/// About the bytes `v` takes as JSON (8 per value, plus its strings and keys); `None` once that +/// passes `limit`, without looking further. +fn weight(v: &Value, limit: usize) -> Option { + let mut total = 0usize; + let mut todo = vec![v]; + while let Some(v) = todo.pop() { + total = total.checked_add(8)?; + match v { + Value::String(s) => total = total.checked_add(s.len())?, + Value::Array(a) => todo.extend(a), + Value::Object(o) => { + for (k, x) in o { + total = total.checked_add(k.len())?; + todo.push(x); + } + } + _ => {} + } + if total > limit { + return None; + } + } + Some(total) +} + /// Which step a reference names. #[derive(Debug, PartialEq)] enum Step { @@ -78,7 +118,7 @@ fn parse(s: &str) -> Option<(Step, Vec)> { Some((step, path)) } -fn lookup(text: &str, step: Step, path: &[Seg], results: &[Value]) -> Result { +fn lookup<'a>(text: &str, step: Step, path: &[Seg], results: &'a [Value]) -> Result<&'a Value, String> { let (i, found) = match step { Step::Index(i) => (i, results.get(i)), Step::Last => (results.len().saturating_sub(1), results.last()), @@ -93,7 +133,7 @@ fn lookup(text: &str, step: Step, path: &[Seg], results: &[Value]) -> Result 0).map(String::as_str); acc.include(&r.value, r.line, file, status, r.path.as_deref()); @@ -173,30 +175,67 @@ pub(crate) fn follow( checked } -/// For each of `n` files, the files it includes directly or through other files (itself too), -/// with `edges` giving the files each one includes. -fn reachable(n: usize, edges: &BTreeSet<(usize, usize)>, acc: &mut Acc) -> Vec> { +/// For each of `n` files, the number of its strongly connected component (files that include each +/// other, directly or through others, share one), with `edges` giving the files each one includes. +/// Tarjan's algorithm without recursion: linear in files and edges, where a row of reached files +/// for every file took their square. +fn components(n: usize, edges: &BTreeSet<(usize, usize)>, acc: &mut Acc) -> Vec { let mut next: Vec> = vec![vec![]; n]; for &(from, to) in edges { - if let Some(v) = next.get_mut(from) { + if let (Some(v), true) = (next.get_mut(from), to < n) { v.push(to); } } - (0..n) - .map(|start| { - let mut seen = vec![false; n]; - let mut stack = vec![start]; - while let Some(f) = stack.pop() { - acc.steps = acc.steps.saturating_add(1); - match seen.get_mut(f) { - Some(s) if !*s => *s = true, - _ => continue, + const UNSEEN: usize = usize::MAX; + let (mut index, mut low, mut component) = (vec![UNSEEN; n], vec![0; n], vec![UNSEEN; n]); + let (mut on_stack, mut stack) = (vec![false; n], vec![]); + let (mut counter, mut components) = (0, 0); + for root in 0..n { + if index[root] != UNSEEN { + continue; + } + // Each frame: a file and how many of its includes have been followed. + let mut frames = vec![(root, 0)]; + index[root] = counter; + low[root] = counter; + counter += 1; + stack.push(root); + on_stack[root] = true; + while let Some(&(f, i)) = frames.last() { + acc.steps = acc.steps.saturating_add(1); + if let Some(&to) = next[f].get(i) { + if let Some(top) = frames.last_mut() { + top.1 += 1; } - stack.extend(next.get(f).into_iter().flatten().copied()); + if index[to] == UNSEEN { + index[to] = counter; + low[to] = counter; + counter += 1; + stack.push(to); + on_stack[to] = true; + frames.push((to, 0)); + } else if on_stack[to] { + low[f] = low[f].min(index[to]); + } + continue; } - seen - }) - .collect() + frames.pop(); + if let Some(&(parent, _)) = frames.last() { + low[parent] = low[parent].min(low[f]); + } + if low[f] == index[f] { + while let Some(g) = stack.pop() { + on_stack[g] = false; + component[g] = components; + if g == f { + break; + } + } + components += 1; + } + } + } + component } /// List the script's `includes` as not read. diff --git a/crates/tools/src/distort/liquify.rs b/crates/tools/src/distort/liquify.rs index 1e917ffc4..b69c2e348 100644 --- a/crates/tools/src/distort/liquify.rs +++ b/crates/tools/src/distort/liquify.rs @@ -667,12 +667,14 @@ pub struct LiquifyTool { hover: Option, /// Time held still since the last sample or repeat (seconds). held: f64, + /// How many samples the last preview of the stroke had. + previewed: usize, } impl LiquifyTool { pub fn new(id: &str) -> Self { let kind = LiquifyKind::parse(id).unwrap_or(LiquifyKind::Warp); - Self { params: LiquifyParams::new(kind), show_brush: true, points: vec![], active: false, sizing: None, hover: None, held: 0.0 } + Self { params: LiquifyParams::new(kind), show_brush: true, points: vec![], active: false, sizing: None, hover: None, held: 0.0, previewed: 0 } } /// The command + params for the stroke so far (with each sample's pressure while Use Pressure @@ -687,6 +689,26 @@ impl LiquifyTool { ("object.liquify".into(), v) } + /// The preview of the stroke so far, when [`crate::previews_at`] its length. + fn preview(&mut self, cx: &ToolContext) -> Vec { + if !crate::previews_at(self.points.len()) { + return vec![]; + } + self.previewed = self.points.len(); + let (c, v) = self.command(cx); + vec![Action::Preview(c, v)] + } + + /// The preview of the whole stroke, when samples came after the last one shown: what is + /// committed is the last preview. + fn stale_preview(&mut self, cx: &ToolContext) -> Option { + (self.previewed != self.points.len() && !self.points.is_empty()).then(|| { + self.previewed = self.points.len(); + let (c, v) = self.command(cx); + Action::Preview(c, v) + }) + } + /// Alt-drag from `s` to `p`: the brush grows or shrinks from its size at the press (its edge /// follows the pointer); with Shift it keeps its proportions. fn resize(&mut self, s: Sizing, p: Point, proportional: bool) { @@ -720,6 +742,7 @@ impl Tool for LiquifyTool { self.points = vec![sample]; self.active = true; self.held = 0.0; + self.previewed = self.points.len(); let (c, v) = self.command(cx); vec![Action::Begin(self.params.kind.label().into()), Action::Preview(c, v)] } @@ -729,13 +752,15 @@ impl Tool for LiquifyTool { self.hover = Some(s.start); return vec![]; } - if !self.active || self.points.last().is_some_and(|(l, _)| l.distance(p) < cx.tol(2.0)) { + if !self.active + || self.points.last().is_some_and(|(l, _)| l.distance(p) < cx.tol(2.0)) + || self.points.len() >= crate::MAX_STROKE_SAMPLES + { return vec![]; } self.points.push(sample); self.held = 0.0; - let (c, v) = self.command(cx); - vec![Action::Preview(c, v)] + self.preview(cx) } PointerKind::Up => { if self.sizing.take().is_some() { @@ -745,8 +770,9 @@ impl Tool for LiquifyTool { return vec![]; } self.active = false; + let out = self.stale_preview(cx).into_iter().chain([Action::Commit]).collect(); self.points.clear(); - vec![Action::Commit] + out } _ => vec![], } @@ -790,10 +816,15 @@ impl Tool for LiquifyTool { fn busy(&self) -> bool { self.active } - fn deactivate(&mut self, _cx: &ToolContext) -> Vec { + fn deactivate(&mut self, cx: &ToolContext) -> Vec { self.hover = None; self.sizing = None; - if std::mem::take(&mut self.active) { vec![Action::Commit] } else { vec![] } + if !std::mem::take(&mut self.active) { + return vec![]; + } + let out = self.stale_preview(cx).into_iter().chain([Action::Commit]).collect(); + self.points.clear(); + out } /// Held still: every [`HOLD_EVERY`] the last sample repeats (one more dab there). fn tick(&mut self, cx: &ToolContext, dt: f64) -> Vec { @@ -809,9 +840,9 @@ impl Tool for LiquifyTool { } self.held = self.held.clamp(0.0, HOLD_EVERY); let Some(&last) = self.points.last().filter(|_| n > 0) else { return vec![] }; - self.points.extend(std::iter::repeat_n(last, n as usize)); - let (c, v) = self.command(cx); - vec![Action::Preview(c, v)] + let room = crate::MAX_STROKE_SAMPLES.saturating_sub(self.points.len()); + self.points.extend(std::iter::repeat_n(last, (n as usize).min(room))); + self.preview(cx) } fn wants_ticks(&self) -> bool { self.active && self.params.kind.holds() diff --git a/crates/tools/src/draw2/gesture.rs b/crates/tools/src/draw2/gesture.rs index 03f4c8779..efd9c7ab8 100644 --- a/crates/tools/src/draw2/gesture.rs +++ b/crates/tools/src/draw2/gesture.rs @@ -21,6 +21,8 @@ use crate::guides::DrawSnap; use crate::{Action, Cursor, Mods, Overlay, PointerEvent, PointerKind, Tool, ToolContext, ToolKey}; pub struct GestureTool { + /// How many points the last preview of the stroke had ([`crate::previews_at`]). + previewed: usize, id: &'static str, points: Vec, /// The pen pressure of each of `points` (the Paintbrush only). @@ -79,6 +81,7 @@ impl GestureTool { let fidelity = if id == "smooth" { 2.5 } else { 1.5 }; Self { id, + previewed: 0, points: vec![], pressures: vec![], active: false, @@ -161,6 +164,7 @@ impl GestureTool { fn clear(&mut self) { self.points.clear(); self.pressures.clear(); + self.previewed = 0; } /// The stroke's points, each `[x, y, pressure]` when a pen pressed less than fully anywhere @@ -340,7 +344,7 @@ impl Tool for GestureTool { return if std::mem::take(&mut self.began) { vec![Action::Cancel] } else { vec![] }; } self.points.truncate(1); - } else if self.points.last().is_some_and(|l| l.distance(p) < cx.tol(1.0)) { + } else if self.points.last().is_some_and(|l| l.distance(p) < cx.tol(1.0)) || self.points.len() >= crate::MAX_STROKE_SAMPLES { return vec![]; } self.push(p, ev.pressure); @@ -352,7 +356,10 @@ impl Tool for GestureTool { self.began = true; out.push(Action::Begin(self.label().into())); } - out.push(Action::Preview("path.freehand".into(), self.freehand_params(false))); + if crate::previews_at(self.points.len()) { + self.previewed = self.points.len(); + out.push(Action::Preview("path.freehand".into(), self.freehand_params(false))); + } out } PointerKind::Up => { @@ -422,12 +429,15 @@ impl Tool for GestureTool { } fn deactivate(&mut self, _cx: &ToolContext) -> Vec { let began = self.began; + // Drags past the last preview are in the stroke kept, too. + let stale = (began && self.previewed != self.points.len() && self.draws_path() && !self.smoothing) + .then(|| Action::Preview("path.freehand".into(), self.freehand_params(false))); self.snap.clear(); self.active = false; self.began = false; self.straight = false; self.clear(); - if began { vec![Action::Commit] } else { vec![] } + if began { stale.into_iter().chain([Action::Commit]).collect() } else { vec![] } } fn overlays(&self, cx: &ToolContext) -> Vec { let mut o = self.snap.guides().to_vec(); diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index c9b0700a7..1553d5c41 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -887,6 +887,20 @@ impl Tool for NoopTool { } } +/// The most samples a drawn stroke (freehand, Liquify) keeps: later drags add nothing to it. +pub(crate) const MAX_STROKE_SAMPLES: usize = 100_000; + +/// Samples up to which every drag previews the whole stroke; past it, a drag previews it every +/// `n / PREVIEW_ALL_UNTIL` samples. Each preview carries every sample so far, so previewing at +/// every sample cost the square of the stroke's length (a gesture of many drags sent at once +/// spent seconds); this way it is about `PREVIEW_ALL_UNTIL` per sample. +const PREVIEW_ALL_UNTIL: usize = 1000; + +/// Whether a drag that made the stroke `n` samples long previews it ([`PREVIEW_ALL_UNTIL`]). +pub(crate) fn previews_at(n: usize) -> bool { + n <= PREVIEW_ALL_UNTIL || n.is_multiple_of(n / PREVIEW_ALL_UNTIL) +} + pub(crate) fn json_ids(ids: &[NodeId]) -> Value { Value::Array(ids.iter().map(|i| Value::from(i.0)).collect()) } diff --git a/crates/ui-egui/src/dialogs/shortcut_import.rs b/crates/ui-egui/src/dialogs/shortcut_import.rs index 6330bd7ed..4845a7102 100644 --- a/crates/ui-egui/src/dialogs/shortcut_import.rs +++ b/crates/ui-egui/src/dialogs/shortcut_import.rs @@ -188,10 +188,21 @@ impl Review { }); } let codes = |list: &Value, field: &str, described: &[(i64, &str)], of: fn(&Row) -> Option| -> BTreeMap { + // The first few names for each code, in one pass over the rows: a scan per code made a + // file of many distinct codes cost codes × rows. + let mut examples: BTreeMap> = BTreeMap::new(); + for r in &rows { + if let Some(value) = of(r) { + let names = examples.entry(value).or_default(); + if names.len() < NAMES_SHOWN { + names.push(r.name.clone()); + } + } + } let mut out = BTreeMap::new(); for c in list.as_array().into_iter().flatten() { let Some(value) = c["value"].as_i64() else { continue }; - let names = rows.iter().filter(|r| of(r) == Some(value)).take(NAMES_SHOWN).map(|r| r.name.clone()).collect(); + let names = examples.get(&value).cloned().unwrap_or_default(); let (text, from) = match (c[field].as_str(), described.iter().find(|(v, _)| *v == value)) { (Some(t), _) => (Some(t.to_string()), "saved"), (None, Some((_, t))) => (Some(t.to_string()), "described"),