collective
Browse / github.com/storytold/vectorcraft / patch
Effects: optimize Transform copies before Offset Path and cap segment pairs (#4145)
[Bug Report] [VectorCraft] RAM usage spikes when opening a VectorCraft project file ### Description When I applied more than one effect to a text layer (Transform and Offset Path effect applied on additional Fill layer), the second effect wasn't showing up at first. When I tried closing and reopening the project file, VectorCraft's RAM usage suddenly spiked to over 50% of available RAM, and the application displayed a “Not Responding” message and failed to reopen the file. ### Operating system and version Windows 11 <!-- ticket-relay:files --> ### Attachments 
- Base
443b31b13df3a7ab611c2fe1b95e73898ed74fdd- Size
- +183 −4 in 6 files
- Review
- pending
- Shared
- 2026-10-28
- Id
9364958d4156b16b0b6e198d
How it went
No other operator has reported using it yet.
Diff
raw · contributors/people.toml crates/effects/src/geometry.rs crates/effects/src/reshape.rs crates/effects/src/tests.rs crates/pathops/src/budget.rs crates/pathops/src/offset.rs
@@ -17,6 +17,11 @@ # To remove your names, delete your entry (or set fields empty) in a PR. The next # contributors.py run, and the next build, drop them. +[people.el-j]+real_name = "Fabian Althaus"+display_name = "faff (el-j)"+sync_github_name = false+ [people.lollinoo] sync_github_name = true @@ -111,16 +111,69 @@ pub fn apply_geometry_with(effects: &[Effect], path: &PathData, bounds: Rect, ct apply_geometry_within(effects, path, bounds, ctx, Budget::EFFECTS) } +/// Does effect `effects[i + 1]` safely commute with pure translation when it is terminal?+fn is_terminal_offset_eligible(effects: &[&Effect], i: usize) -> bool {+ if i + 1 != effects.len() - 1 {+ return false;+ }+ let e = effects[i + 1];+ if e.id != "path.offsetPath" {+ return false;+ }+ let p = merged_params(&e.id, &e.params);+ util::num(&p, "offset", 10.0) >= 0.0+}++/// Optimize a list of visible geometry effects: move pure-translation Transform effects with+/// copies after a terminal Offset Path effect when the effective style is safe (NonZero fill and+/// no stroke), so that the offset path is evaluated once on the base path rather than on dozens+/// of overlapping copies, avoiding combinatorial explosions in the planar sweep (#4145).+pub(crate) fn optimize_geometry_effects<'a>(effects: &[&'a Effect], style_safe: bool) -> Vec<&'a Effect> {+ let mut out: Vec<&'a Effect> = effects.to_vec();+ if !style_safe || out.len() < 2 {+ return out;+ }+ let mut changed = true;+ while changed {+ changed = false;+ for i in 0..out.len().saturating_sub(1) {+ let e1 = out[i];+ if e1.id == "distort.transform" {+ let p1 = merged_params(&e1.id, &e1.params);+ if let Some((_, copies)) = crate::distort::transform_move(&p1)+ && copies > 0+ && is_terminal_offset_eligible(&out, i)+ {+ out.swap(i, i + 1);+ changed = true;+ }+ }+ }+ }+ out+}+ /// [`apply_geometry_with`] whose effects keep to `budget`, raised to what `path` costs where that /// is more. pub(crate) fn apply_geometry_within(effects: &[Effect], path: &PathData, bounds: Rect, ctx: &GeomContext, budget: Budget) -> PathData { // `budget` raised to what `path` costs, measured once, and only for a path that has geometry // effects.+ let has_stroke = match (ctx.item, ctx.appearance) {+ (Some(idx), Some(app)) => match app.items.get(idx) {+ Some(AppearanceItem::Stroke(s)) => !s.paint.is_none() && s.width > 0.0,+ Some(AppearanceItem::Fill(_)) => false,+ None => ctx.stroke.is_some_and(|s| !s.paint.is_none() && s.width > 0.0),+ },+ _ => ctx.stroke.is_some_and(|s| !s.paint.is_none() && s.width > 0.0),+ };+ let style_safe = ctx.rule == FillRule::NonZero && !has_stroke;+ let visible_fx: Vec<&Effect> = effects.iter().filter(|e| e.visible && is_geometry(&e.id)).collect();+ let fx = optimize_geometry_effects(&visible_fx, style_safe); let mut limit = None; let mut cur = path.clone(); let mut b = bounds; let mut first = true;- for e in effects.iter().filter(|e| e.visible && is_geometry(&e.id)) {+ for e in fx { if !first { b = cur.bounds().unwrap_or(b); }@@ -431,6 +431,22 @@ pub(crate) fn reshape_leaves_within(art: &mut Node, fx: &[&Effect], budget: Budg if fx.is_empty() { return; }+ let mut leaves_safe = true;+ for_each_leaf(art, &mut |leaf| {+ if !leaves_safe {+ return;+ }+ if let Some((_, rule)) = leaf_geometry(leaf) {+ let has_stroke = leaf.appearance.stroke().is_some_and(|s| !s.paint.is_none() && s.width > 0.0);+ if rule != FillRule::NonZero || has_stroke {+ leaves_safe = false;+ }+ }+ });+ let art_stroke = art.appearance.stroke().is_some_and(|s| !s.paint.is_none() && s.width > 0.0);+ let style_safe = leaves_safe && !art_stroke;+ let opt_fx = crate::geometry::optimize_geometry_effects(fx, style_safe);+ let fx = opt_fx.as_slice(); let seed = art.id.0; // The placed gradients of type's outlines in `art` (`outline_text`) move and scale with the // pieces, from the outlines' bounds before the effects to their bounds after them.@@ -74,6 +74,109 @@ fn geometry_cases() -> Vec<(&'static str, Value)> { v } +#[test]+fn transform_with_copies_then_offset_path_does_not_hang_or_explode_memory() {+ // Issue #4145: applying Transform (with copies) and Offset Path caused Offset Path to evaluate+ // over dozens of overlapping copies at once, making millions of crossings in linesweeper,+ // spiking memory by gigabytes and hanging the app.+ // 1. Synthetic multi-subpath compound shape (e.g. 10 disconnected squares/shapes).+ let mut subs = Vec::new();+ for i in 0..10 {+ let x = (i as f64) * 40.0;+ let y = if i % 2 == 0 { 0.0 } else { 30.0 };+ subs.push(shapes::rectangle(Rect::new(x, y, x + 20.0, y + 20.0)).subpaths[0].clone());+ }+ let p = PathData::new(subs);+ assert_eq!(p.subpaths.len(), 10);++ let effects = [fx("distort.transform", json!({"moveH": -1.0, "moveV": 1.0, "copies": 30})), fx("path.offsetPath", json!({"offset": 5.0}))];+ let start = std::time::Instant::now();+ let out = apply_geometry(&effects, &p, p.bounds().unwrap());+ let elapsed = start.elapsed();+ assert!(elapsed < std::time::Duration::from_secs(2), "elapsed: {:?}", elapsed);+ assert!(!out.is_empty());+ // 31 copies of the 10 offset subpaths.+ assert_eq!(out.subpaths.len(), 10 * 31);++ // 2. Real glyph outlines if Source Sans 3 is available.+ let font_path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../assets/fonts/SourceSans3-Regular.ttf");+ if let Ok(font_bytes) = std::fs::read(&font_path).or_else(|_| std::fs::read("assets/fonts/SourceSans3-Regular.ttf")) {+ let db = vectorcraft_text::FontDb::global();+ db.add_font(font_bytes);+ let text = "SOMETHING\nHERE";+ let style =+ vectorcraft_doc::CharStyle { font_family: "Source Sans 3".into(), font_style: "Regular".into(), size: 48.0, ..Default::default() };+ let to = vectorcraft_doc::TextObject::point(vectorcraft_geom::Point::new(50.0, 100.0), text, style);+ let layout = vectorcraft_text::layout(db, &to);+ let mut all = kurbo::BezPath::new();+ for g in &layout.glyphs {+ all.extend(g.outline.iter());+ }+ let text_path = PathData::from_bezpath(&all);+ if !text_path.is_empty() {+ let text_effects =+ [fx("distort.transform", json!({"moveH": -1.0, "moveV": 1.0, "copies": 25})), fx("path.offsetPath", json!({"offset": 10.0}))];+ let start = std::time::Instant::now();+ let text_out = apply_geometry(&text_effects, &text_path, text_path.bounds().unwrap());+ let text_elapsed = start.elapsed();+ assert!(text_elapsed < std::time::Duration::from_secs(2), "text elapsed: {:?}", text_elapsed);+ assert!(!text_out.is_empty());+ }+ }+}++#[test]+fn reshape_leaves_within_optimizes_transform_before_terminal_offset_path() {+ use std::sync::Arc;+ use vectorcraft_doc::{Appearance, Node, NodeId};++ let id = NodeId(10);+ let leaf = |x: f64| Arc::new(Node::path(id, shapes::rectangle(Rect::new(x, 0.0, x + 20.0, 20.0)), Appearance::default()));+ let mut art = Node::group(id, (0..5).map(|k| leaf(k as f64 * 30.0)).collect());+ let transform = fx("distort.transform", json!({"moveH": 2.0, "moveV": 2.0, "copies": 10}));+ let offset = fx("path.offsetPath", json!({"offset": 4.0}));++ let start = std::time::Instant::now();+ crate::reshape::reshape_leaves_within(&mut art, &[&transform, &offset], Budget::EFFECTS);+ let elapsed = start.elapsed();+ assert!(elapsed < std::time::Duration::from_secs(2));++ let children = art.children().unwrap();+ assert_eq!(children.len(), 5);+ for c in children {+ let (p, _) = crate::reshape::leaf_geometry(c).unwrap();+ // Base rectangle offset once, then copied 10 times -> 11 subpaths+ assert_eq!(p.subpaths.len(), 11);+ }+}++#[test]+fn reshape_leaves_within_preserves_order_when_not_terminal_or_not_style_safe() {+ use std::sync::Arc;+ use vectorcraft_doc::{Appearance, Node, NodeId, NodeKind};++ let id = NodeId(11);+ // 1. EvenOdd fill rule: preserving order causes Transform copies to union in Offset Path into 1 subpath.+ let mut leaf = Node::path(id, shapes::rectangle(Rect::new(0.0, 0.0, 20.0, 20.0)), Appearance::default());+ if let NodeKind::Path { rule, .. } = &mut leaf.kind {+ *rule = FillRule::EvenOdd;+ }+ let mut art_evenodd = Node::group(id, vec![Arc::new(leaf)]);+ let transform = fx("distort.transform", json!({"moveH": 2.0, "moveV": 2.0, "copies": 2}));+ let offset = fx("path.offsetPath", json!({"offset": 4.0}));+ crate::reshape::reshape_leaves_within(&mut art_evenodd, &[&transform, &offset], Budget::EFFECTS);+ let p = art_evenodd.children().unwrap()[0].path_data().unwrap();+ assert_eq!(p.subpaths.len(), 1, "EvenOdd does not swap; copies union into 1 outer subpath");++ // 2. Non-terminal Offset Path: followed by Round Corners, must preserve order.+ let leaf_round = Arc::new(Node::path(id, shapes::rectangle(Rect::new(0.0, 0.0, 20.0, 20.0)), Appearance::default()));+ let mut art_round = Node::group(id, vec![leaf_round]);+ let round_corners = fx("stylize.roundCorners", json!({"radius": 2.0}));+ crate::reshape::reshape_leaves_within(&mut art_round, &[&transform, &offset, &round_corners], Budget::EFFECTS);+ let p = art_round.children().unwrap()[0].path_data().unwrap();+ assert_eq!(p.subpaths.len(), 1, "Non-terminal Offset Path does not swap; copies union into 1 outer subpath");+}+ #[test] fn catalog_complete_and_unique() { let cat = effect_catalog();@@ -10,7 +10,7 @@ use kurbo::{BezPath, ParamCurveExtrema, Rect}; /// Most pairs of segments whose bounds overlap that a planar map is built from. Neighbouring /// segments of a path always overlap at their shared end; real art of 65,536 segments (the live /// Pathfinder's cap) overlaps a few times per segment, far below this.-pub const MAX_SEGMENT_PAIRS: usize = 1 << 22;+pub const MAX_SEGMENT_PAIRS: usize = 1 << 19; /// Fenwick tree of counts over its slots. struct Counts(Vec<isize>);@@ -109,8 +109,10 @@ pub fn offset_path(path: &PathData, delta: f64, join: Join, miter_limit: f64) -> } else { BezPath::new() };- let Ok(arr) = Arrangement::new(vec![(fill, FillRule::NonZero), (ring, FillRule::NonZero), (open_bp, FillRule::NonZero)]) else {- return PathData::default();+ let arr = match Arrangement::new(vec![(fill, FillRule::NonZero), (ring, FillRule::NonZero), (open_bp, FillRule::NonZero)]) {+ Ok(arr) => arr,+ Err(crate::PathOpsError::NonFinite) => return PathData::default(),+ Err(_) => return path.clone(), }; let c = if delta >= 0.0 { arr.contours(|m| m[0] || m[1] || m[2]) } else { arr.contours(|m| m[0] && !m[1]) }; // When `d` exceeds a curvature radius the stroker's inner offset inverts and its loops cancel