Merge pull request #4611 from el-j/perf/transform-before-round-offset-4145

perf(effects): reorder pure-translation Transform copies after round-join terminal Offset Path (#4145)

contributors/people.toml

@@ -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
 

crates/effects/src/geometry.rs

@@ -114,16 +114,67 @@ pub fn apply_geometry_with(effects: &[Effect], path: &PathData, bounds: Rect, ct
     apply_geometry_within(effects, path, bounds, ctx, Budget::EFFECTS)
 }
 
+/// Does effect `e` safely commute with pure translation when it is terminal?
+/// Only positive offsets with round joins commute exactly with pure translations for non-zero fills (#4145).
+fn is_terminal_offset_eligible(e: &Effect) -> bool {
+    if e.id != "path.offsetPath" {
+        return false;
+    }
+    let p = merged_params(&e.id, &e.params);
+    util::join(&p, "joins") == vectorcraft_pathops::Join::Round && 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 + 1])
+                    && out.get(i + 2..).is_some_and(|tail| tail.iter().all(|e| e.id == "distort.transform"))
+                {
+                    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);
         }

crates/effects/src/reshape.rs

@@ -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.

crates/effects/src/tests.rs

@@ -438,6 +438,93 @@ fn three_max_detail_roughens_stay_within_the_effect_budget() {
     assert_ne!(out, apply_geometry(&[rough], &rect, rect.bounds().unwrap()), "more than one Roughen applies");
 }
 
+#[test]
+fn transform_copies_before_offset_path_evaluates_without_ram_spike() {
+    let effects = [
+        fx("distort.transform", json!({"moveH": -0.66, "moveV": 0.66, "copies": 30})),
+        fx("path.offsetPath", json!({"offset": 10.0, "joins": "round"})),
+    ];
+    let path = shapes::rectangle(Rect::new(0.0, 0.0, 100.0, 100.0));
+    let start = std::time::Instant::now();
+    let out = apply_geometry(&effects, &path, path.bounds().unwrap());
+    let elapsed = start.elapsed();
+    assert!(elapsed < std::time::Duration::from_secs(2), "elapsed: {:?}", elapsed);
+    assert_eq!(out.subpaths.len(), 31);
+}
+
+#[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, "joins": "round"}));
+
+    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");
+
+    // 3. Non-round join (e.g. miter): preserving order is required as corners differ (#4145).
+    let leaf_miter = Arc::new(Node::path(id, shapes::rectangle(Rect::new(0.0, 0.0, 20.0, 20.0)), Appearance::default()));
+    let mut art_miter = Node::group(id, vec![leaf_miter]);
+    let offset_miter = fx("path.offsetPath", json!({"offset": 4.0, "joins": "miter"}));
+    crate::reshape::reshape_leaves_within(&mut art_miter, &[&transform, &offset_miter], Budget::EFFECTS);
+    let p = art_miter.children().unwrap()[0].path_data().unwrap();
+    assert_eq!(p.subpaths.len(), 1, "Miter join does not swap; copies union into 1 outer subpath");
+}
+
+#[test]
+fn consecutive_transforms_before_terminal_offset_path_all_move_after() {
+    let t1 = fx("distort.transform", json!({"moveH": 1.0, "moveV": 0.0, "copies": 2}));
+    let t2 = fx("distort.transform", json!({"moveH": 0.0, "moveV": 1.0, "copies": 3}));
+    let offset = fx("path.offsetPath", json!({"offset": 4.0, "joins": "round"}));
+
+    let opt = crate::geometry::optimize_geometry_effects(&[&t1, &t2, &offset], true);
+    assert_eq!(opt.len(), 3);
+    assert_eq!(opt[0].id, "path.offsetPath");
+    assert_eq!(opt[1].id, "distort.transform");
+    assert_eq!(opt[2].id, "distort.transform");
+}
+
 #[test]
 fn transform_copies_stop_before_they_overflow() {
     let out = run("distort.transform", json!({"scaleH": 100000, "scaleV": 100000, "copies": 1000}), &square());