Fix #6316: a group copies each member in place when a member makes more art

A group with a visible member that carries a geometry, Pathfinder or 3D effect, also in an opacity mask, keeps its previous copies. Copies stop at the budget with what each copy's own effects make, and a group's copies count the art of opacity masks.

The PDF and SVG tests for a group's image copies now expect the original on top.

crates/effects/src/catalog.rs

@@ -247,7 +247,7 @@ pub fn effect_catalog() -> Vec<EffectInfo> {
             "distort.transform",
             "Transform…",
             DT,
-            "{scaleH: % (100), scaleV: % (100), moveH: pt (0; right = +), moveV: pt (0; down = +), rotate: deg (0; counter-clockwise; when the effect turns, reflects or scales the object unevenly, a gradient fitted to the object is fixed to its fit and turns with the object and with each copy), copies: 0..1000 (0; each copy transforms the last again and is painted below it, the object on top; a group's copies each hold all its members; placed gradients transform with the object and with each copy), reflectX: bool (false; flips left to right), reflectY: bool (false; flips top to bottom), reference: 0..8 (4; the point of the bounds' 9-point grid it scales, rotates and reflects about: 0 top left, 4 centre, 8 bottom right), random: bool (false; each scale goes a random share of the way from 100 % to its value, each move and the angle a random share of theirs, differently for each object and the same on every redraw), objects: bool (true; Transform Objects: off, the geometry stays where it is and makes no copies, and only the two options below apply, once), patterns: bool (false; Transform Patterns: pattern fills and strokes transform along; with copies the original keeps its pattern and copy k takes the transformation k times), strokes: bool (false; Scale Strokes & Effects: stroke weights and dashes, and the distances of the effects on the fills and strokes, scale by the mean scale √|scaleH·scaleV|, copy k by its k-th power)}",
+            "{scaleH: % (100), scaleV: % (100), moveH: pt (0; right = +), moveV: pt (0; down = +), rotate: deg (0; counter-clockwise; when the effect turns, reflects or scales the object unevenly, a gradient fitted to the object is fixed to its fit and turns with the object and with each copy), copies: 0..1000 (0; each copy transforms the last again and is painted below it, the object on top; a group's copies each hold all its members, and a group with a member that carries a geometry, Pathfinder or 3D effect copies each member in its place; placed gradients transform with the object and with each copy), reflectX: bool (false; flips left to right), reflectY: bool (false; flips top to bottom), reference: 0..8 (4; the point of the bounds' 9-point grid it scales, rotates and reflects about: 0 top left, 4 centre, 8 bottom right), random: bool (false; each scale goes a random share of the way from 100 % to its value, each move and the angle a random share of theirs, differently for each object and the same on every redraw), objects: bool (true; Transform Objects: off, the geometry stays where it is and makes no copies, and only the two options below apply, once), patterns: bool (false; Transform Patterns: pattern fills and strokes transform along; with copies the original keeps its pattern and copy k takes the transformation k times), strokes: bool (false; Scale Strokes & Effects: stroke weights and dashes, and the distances of the effects on the fills and strokes, scale by the mean scale √|scaleH·scaleV|, copy k by its k-th power)}",
             json!({"scaleH": 100.0, "scaleV": 100.0, "moveH": 0.0, "moveV": 0.0, "rotate": 0.0, "copies": 0, "reflectX": false, "reflectY": false, "reference": 4, "random": false, "objects": true, "patterns": false, "strokes": false}),
         ),
         g(

crates/effects/src/reshape.rs

@@ -881,7 +881,8 @@ fn transform_leaf(
 /// group or layer of several objects, at least one of which paints, with no type left in it and no
 /// fill or stroke of its own that paints. Type that stays type is copied in its place by the baking
 /// that keeps it, a clipping group's first member is its clip, and the art's own fills and strokes
-/// paint the shape of everything under it, copies included.
+/// paint the shape of everything under it, copies included. A member that [`makes_art`] is copied
+/// in its place as well: each copy would make that art again, past what [`copy_art`] counts.
 fn copies_whole(art: &Node) -> bool {
     fn parts(n: &Node, count: &mut usize, painted: &mut bool, text: &mut bool) {
         match &n.kind {
@@ -906,7 +907,59 @@ fn copies_whole(art: &Node) -> bool {
     }
     let (mut count, mut painted, mut text) = (0, false, false);
     parts(art, &mut count, &mut painted, &mut text);
-    count > 1 && painted && !text
+    count > 1 && painted && !text && !art.children().into_iter().flatten().any(|c| makes_art(c))
+}
+
+/// Does `n` or anything visible under it, the art of its opacity masks included, carry a visible
+/// effect that makes art when it is drawn (its own or that of a fill or stroke that paints), or is
+/// any of them a compound shape? Raster effects, color adjustments and unknown effects make none.
+fn makes_art(n: &Node) -> bool {
+    let makes = |e: &Effect| {
+        e.visible
+            && (is_geometry(&e.id)
+                || crate::is_pathfinder(&e.id)
+                || crate::is_three_d(&e.id)
+                || e.id == crate::CROP_MARKS
+                || e.id == crate::addons::halftone::HALFTONE)
+    };
+    n.visible
+        && (n.is_compound_shape()
+            || n.appearance.effects.iter().any(makes)
+            || n.appearance.items.iter().any(|i| crate::paints(i) && i.effects().iter().any(makes))
+            || n.extras().mask.as_deref().is_some_and(|m| makes_art(&m.art))
+            || n.children().into_iter().flatten().any(|c| makes_art(c)))
+}
+
+/// What the paths under `n` cost together, those of its opacity masks included.
+fn copy_cost(n: &Node) -> Budget {
+    let mut cost = Budget { anchors: 0, lines: 0 };
+    vectorcraft_doc::swatches::walk_with_masks(n, &mut |c| {
+        if let NodeKind::Path { guide: false, path, .. } = &c.kind {
+            cost = cost.plus(Budget::of(path));
+        }
+    });
+    cost
+}
+
+/// What the geometry effects of `leaf` (with geometry `path`) and of its fills and strokes add to
+/// what `path` costs when it is drawn.
+fn effects_cost(leaf: &Node, path: &PathData) -> Budget {
+    let none = Budget { anchors: 0, lines: 0 };
+    let items: Vec<&AppearanceItem> = leaf.appearance.items.iter().filter(|i| crate::paints(i) && crate::has_geometry(i.effects())).collect();
+    if items.is_empty() && !crate::has_geometry(&leaf.appearance.effects) {
+        return none;
+    }
+    let Some(bounds) = path.bounds() else { return none };
+    let ctx = GeomContext { seed: leaf.id.0, ..GeomContext::of(leaf) };
+    let own = crate::apply_geometry_with(&leaf.appearance.effects, path, bounds, &ctx);
+    let item_cost = |i: &&AppearanceItem| {
+        let b = own.bounds()?;
+        Some(Budget::of(&crate::apply_geometry_with(i.effects(), &own, b, &ctx.item(i))))
+    };
+    // The object's geometry is drawn as it is only by what has no geometry effect of its own.
+    let plain = items.is_empty() || leaf.appearance.items.iter().any(|i| crate::paints(i) && !crate::has_geometry(i.effects()));
+    let base = if plain { Budget::of(&own) } else { none };
+    base.plus(items.iter().filter_map(item_cost).fold(none, Budget::plus)).minus(Budget::of(path))
 }
 
 /// Remove the guides anywhere under `n`.
@@ -919,17 +972,16 @@ fn without_guides(n: &mut Node) {
 /// Replace the members of `art` by groups of them: the original on top and `copies` more below it,
 /// each mapped once more by `m` than the last (with `scaling`), as a Transform effect copies a
 /// group, without its guides. The copies stop where [`copy_leaf`]'s would, before they pass `budget` or
-/// [`crate::distort::MAX_COPY_OBJECTS`] nodes in all. Returns what the art costs after.
+/// [`crate::distort::MAX_COPY_OBJECTS`] nodes in all, the art of opacity masks counted. Returns what
+/// the art costs after.
 fn copy_art(art: &mut Node, m: Affine, copies: usize, scaling: vectorcraft_doc::Scaling, budget: Budget) -> Budget {
     let Some(members) = art.children().cloned() else { return art_cost(art) };
     let original = Node::group(art.id, members);
-    let mut made = art_cost(&original);
+    let mut made = copy_cost(&original);
     let mut drawn = original.clone();
     without_guides(&mut drawn);
-    let mut nodes = 0usize;
-    original.walk(&mut |_| nodes += 1);
-    let mut one = 0usize;
-    drawn.walk(&mut |_| one += 1);
+    let mut nodes = original.count_with_masks();
+    let one = drawn.count_with_masks();
     let mut placements: Vec<Arc<Node>> = vec![];
     let mut xf = Affine::IDENTITY;
     for _ in 0..copies {
@@ -943,7 +995,7 @@ fn copy_art(art: &mut Node, m: Affine, copies: usize, scaling: vectorcraft_doc::
         }
         let mut copy = drawn.clone();
         copy.transform(xf, scaling);
-        let cost = art_cost(&copy);
+        let cost = copy_cost(&copy);
         if !budget.fits(made.plus(cost)) || !copy.geometric_bounds().is_some_and(crate::distort::within_reach) {
             break;
         }
@@ -964,25 +1016,25 @@ fn copy_art(art: &mut Node, m: Affine, copies: usize, scaling: vectorcraft_doc::
 /// copies an object. Each copy is an object of its own, so its gradients fit it (a placed gradient
 /// moves with it) rather than spanning every copy, and its strokes and patterns can differ from the
 /// others'. The copies stop where [`crate::distort::transform_within`]'s would, before they pass
-/// `budget`, and before one's strokes would scale past [`MAX_STROKE_SCALE`]. Returns what the
-/// copies cost, the original included.
+/// `budget` with what each copy's own geometry effects make ([`effects_cost`]), and before one's
+/// strokes would scale past [`MAX_STROKE_SCALE`]. Returns what the copies cost, the original
+/// included.
 fn copy_leaf(leaf: &mut Node, path: &PathData, m: Affine, copies: usize, scaling: vectorcraft_doc::Scaling, budget: Budget) -> Budget {
-    let mut made = Budget::of(path);
+    let mut made = Budget::of(path).plus(effects_cost(leaf, path));
     let mut placements: Vec<Arc<Node>> = vec![Arc::new(leaf.clone())];
     let mut xf = Affine::IDENTITY;
     for _ in 0..copies {
         xf *= m;
         // A copy scaled up is drawn with more lines.
-        let cost = Budget::of(&path.transformed(xf));
-        if !budget.fits(made.plus(cost)) {
-            break;
-        }
+        let moved = path.transformed(xf);
         if scaling.strokes && vectorcraft_doc::Scaling::factor(xf).is_some_and(|k| !(k.is_finite() && k <= MAX_STROKE_SCALE)) {
             break;
         }
         let mut copy = leaf.clone();
         copy.transform(xf, scaling);
-        if !copy.geometric_bounds().is_some_and(crate::distort::within_reach) {
+        // Each copy's own geometry effects make a different amount, as the copy and its effects scale.
+        let cost = Budget::of(&moved).plus(effects_cost(&copy, &moved));
+        if !budget.fits(made.plus(cost)) || !copy.geometric_bounds().is_some_and(crate::distort::within_reach) {
             break;
         }
         placements.push(Arc::new(copy));

crates/effects/src/tests/transformfx.rs

@@ -509,3 +509,170 @@ fn a_groups_copies_stop_at_the_node_limit() {
     assert!(nodes <= crate::distort::MAX_COPY_OBJECTS + 1, "{nodes}");
     assert!(g.children().unwrap().len() < 1001, "the limit stopped the copies");
 }
+
+#[test]
+fn a_groups_copies_count_the_art_of_opacity_masks() {
+    let empties = (0..5_000).map(|i| std::sync::Arc::new(Node::group(NodeId(100 + i), vec![]))).collect();
+    let mut g = two_squares(json!({}));
+    if let Some(members) = g.children_mut() {
+        let mask = vectorcraft_doc::OpacityMask::new(Node::group(NodeId(99), empties), true);
+        std::sync::Arc::make_mut(&mut members[0]).extras_mut().mask = Some(Box::new(mask));
+    }
+    g.appearance.effects.clear();
+    let copies = transform(json!({"moveH": 1, "copies": 1000}));
+    crate::reshape::reshape_leaves_within(&mut g, &[&copies], crate::geometry::Budget::EFFECTS, None);
+    let nodes = g.count_with_masks();
+    assert!(nodes <= crate::distort::MAX_COPY_OBJECTS + 1, "{nodes}");
+}
+
+/// The anchors `n` draws with every effect inside it evaluated, group by group and member by
+/// member as drawing and baking evaluate them. The count stops once it passes `limit`.
+fn drawn_anchors(n: &Node, limit: usize) -> usize {
+    fn count(n: &Node, limit: usize, total: &mut usize) {
+        if *total > limit {
+            return;
+        }
+        if n.children().is_some() && !matches!(n.kind, NodeKind::Compound { .. }) {
+            let art = crate::evaluate_container(None, n).unwrap_or_else(|| n.clone());
+            art.children().into_iter().flatten().for_each(|c| count(c, limit, total));
+            return;
+        }
+        if let Some(art) = reshape(None, n, None) {
+            count(&art, limit, total);
+            return;
+        }
+        let Some((path, _)) = crate::reshape::leaf_geometry(n) else { return };
+        let Some(bounds) = path.bounds() else { return };
+        let own = crate::apply_geometry(&n.appearance.effects, &path, bounds);
+        let items: Vec<usize> = n
+            .appearance
+            .items
+            .iter()
+            .filter(|i| crate::has_geometry(i.effects()))
+            .filter_map(|i| Some(crate::apply_geometry(i.effects(), &own, own.bounds()?).anchor_count()))
+            .collect();
+        *total += if items.is_empty() { own.anchor_count() } else { items.iter().sum() };
+    }
+    let mut total = 0;
+    count(n, limit, &mut total);
+    total
+}
+
+#[test]
+fn a_groups_copies_keep_what_the_effects_inside_them_make_to_the_budget() {
+    let most = crate::geometry::MAX_EFFECT_ANCHORS;
+    let copies = |key: &str| transform(json!({key: 1, "copies": 1000}));
+
+    // Copies of a group whose member group makes copies of its own.
+    let inner = two_squares(json!({"moveH": 1, "copies": 1000}));
+    let mut outer = Node::group(NodeId(30), vec![std::sync::Arc::new(inner)]);
+    outer.appearance.effects.push(copies("moveV"));
+    let drawn = drawn_anchors(&outer, 2 * most);
+    assert!(drawn <= most, "a group inside: {drawn}");
+
+    // Copies of a group whose member path makes copies of its own.
+    let mut own = two_squares(json!({"moveH": 1, "copies": 1000}));
+    if let Some(members) = own.children_mut() {
+        std::sync::Arc::make_mut(&mut members[0]).appearance.effects.push(copies("moveV"));
+    }
+    let drawn = drawn_anchors(&own, 2 * most);
+    assert!(drawn <= most, "a path's own effect: {drawn}");
+
+    // Copies of a group whose member's fill makes copies of its own.
+    let mut item = two_squares(json!({"moveH": 1, "copies": 1000}));
+    if let Some(members) = item.children_mut() {
+        std::sync::Arc::make_mut(&mut members[0]).appearance.items[0].effects_mut().push(copies("moveV"));
+    }
+    let drawn = drawn_anchors(&item, 2 * most);
+    assert!(drawn <= most, "a fill's own effect: {drawn}");
+}
+
+#[test]
+fn copies_with_scaled_effects_keep_what_each_one_makes_to_the_budget() {
+    // Each copy's Roughen is twice as large as the last's, and the rough edges take more lines.
+    let roughen = Effect { id: "distort.roughen".into(), params: json!({"size": 40, "relative": false, "detail": 20}), visible: true };
+    let lines = |budget: crate::geometry::Budget| {
+        let mut g = two_squares(json!({}));
+        if let Some(members) = g.children_mut() {
+            let m = std::sync::Arc::make_mut(&mut members[0]);
+            m.appearance.items.retain(AppearanceItem::is_fill);
+            m.appearance.effects.push(roughen.clone());
+            members.truncate(1);
+        }
+        g.appearance.effects.clear();
+        let copies = transform(json!({"scaleH": 200, "scaleV": 200, "copies": 5, "strokes": true}));
+        crate::reshape::reshape_leaves_within(&mut g, &[&copies], budget, None);
+        let (mut count, mut lines) = (0, 0);
+        for leaf in leaves(&g) {
+            count += 1;
+            let Some((path, _)) = crate::reshape::leaf_geometry(leaf) else { continue };
+            let drawn = crate::apply_geometry(&leaf.appearance.effects, &path, path.bounds().unwrap());
+            lines += crate::geometry::Budget::of(&drawn).lines;
+        }
+        (count, lines)
+    };
+    let (all, most) = lines(crate::geometry::Budget::EFFECTS);
+    assert_eq!(all, 6, "the original and 5 copies");
+    let cap = most / 2;
+    let budget = crate::geometry::Budget { anchors: crate::geometry::MAX_EFFECT_ANCHORS, lines: cap };
+    let (count, drawn) = lines(budget);
+    assert!(count < all, "the budget stopped the copies");
+    assert!(drawn <= cap, "{drawn} > {cap}");
+}
+
+#[test]
+fn a_groups_copies_are_whole_only_when_nothing_inside_makes_more_art() {
+    let copies = transform(json!({"moveH": 300, "copies": 2}));
+    let effect = |id: &str| Effect { id: id.into(), params: json!({}), visible: true };
+    let roughen = effect("distort.roughen");
+    // Two squares under a group with `change` made to the first member.
+    let whole = |change: &dyn Fn(&mut Node)| {
+        let mut g = two_squares(json!({}));
+        if let Some(members) = g.children_mut() {
+            change(std::sync::Arc::make_mut(&mut members[0]));
+        }
+        g.appearance.effects.clear();
+        crate::reshape::reshape_leaves_within(&mut g, &[&copies], crate::geometry::Budget::EFFECTS, None);
+        g.children().unwrap().len() == 3
+    };
+    // A group of one square with effect `e`, in place of the member.
+    let in_group = |m: &mut Node, e: &Effect| {
+        let square = Node::path(NodeId(40), shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default());
+        let mut g = Node::group(NodeId(41), vec![std::sync::Arc::new(square), std::sync::Arc::new(m.clone())]);
+        g.appearance.effects.push(e.clone());
+        *m = g;
+    };
+    assert!(whole(&|_| {}));
+    // Effects that only change pixels, and effects turned off, are copied with the art.
+    assert!(whole(&|m| m.appearance.effects.push(effect("stylize.dropShadow"))));
+    assert!(whole(&|m| m.appearance.effects.push(Effect { visible: false, ..roughen.clone() })));
+    // Effects that draw nothing are copied with the art as well.
+    assert!(
+        whole(&|m| {
+            m.visible = false;
+            m.appearance.effects.push(roughen.clone());
+        }),
+        "a hidden member's effect"
+    );
+    assert!(
+        whole(&|m| {
+            *m.appearance.items[0].paint_mut() = Paint::None;
+            m.appearance.items[0].effects_mut().push(roughen.clone());
+        }),
+        "the effect of a fill that paints nothing"
+    );
+    assert!(whole(&|m| m.appearance.effects.push(effect("no.such.effect"))), "an unknown effect");
+    // Effects that make more art than a copy holds keep each member's copies in its place.
+    assert!(!whole(&|m| m.appearance.effects.push(roughen.clone())), "a member's own effect");
+    assert!(!whole(&|m| m.appearance.items[0].effects_mut().push(roughen.clone())), "a fill's own effect");
+    assert!(!whole(&|m| in_group(m, &roughen)), "a group's own effect");
+    assert!(!whole(&|m| in_group(m, &effect("pathfinder.add"))), "a Pathfinder effect");
+    assert!(
+        !whole(&|m| {
+            let mut art = Node::group(NodeId(43), vec![]);
+            in_group(&mut art, &roughen);
+            m.extras_mut().mask = Some(Box::new(vectorcraft_doc::OpacityMask::new(art, true)));
+        }),
+        "an effect in an opacity mask"
+    );
+}

crates/pdf/src/tests.rs

@@ -749,7 +749,7 @@ fn an_image_whose_pixels_cant_be_decoded_is_left_out_with_one_note() {
 }
 
 /// #2023: a group's Transform effect copies an image among its members as it does the paths: the
-/// PDF draws the image three times, where the effect put it.
+/// PDF draws the image three times, where the effect put it, the original on top.
 #[test]
 fn a_group_transform_effect_exports_its_image_copies() {
     let mut d = doc(200.0, 100.0);
@@ -776,7 +776,7 @@ fn a_group_transform_effect_exports_its_image_copies() {
     let out = roundtrip(&d);
     let lefts: Vec<f64> =
         leaves(&out).iter().filter(|n| matches!(n.kind, NodeKind::Image(_))).filter_map(|n| n.geometric_bounds()).map(|b| b.x0.round()).collect();
-    assert_eq!(lefts, [20.0, 70.0, 120.0]);
+    assert_eq!(lefts, [120.0, 70.0, 20.0]);
 }
 
 /// Build a PDF with a correct xref table from object bodies.

crates/svg/tests/it/container_export.rs

@@ -92,5 +92,5 @@ fn a_group_transform_copies_its_images() {
         .filter(|l| l.starts_with("<image") || l.starts_with("<use"))
         .filter_map(|l| l.split("transform=\"").nth(1)?.split('"').next())
         .collect();
-    assert_eq!(placed, ["matrix(1 0 0 1 20 20)", "matrix(1 0 0 1 60 20)", "matrix(1 0 0 1 100 20)"], "{svg}");
+    assert_eq!(placed, ["matrix(1 0 0 1 100 20)", "matrix(1 0 0 1 60 20)", "matrix(1 0 0 1 20 20)"], "the original on top: {svg}");
 }

docs/effects-parity.md

@@ -22,7 +22,7 @@ Part of [target-app-parity.md](target-app-parity.md); open work is in [gaps.md](
 |---|---|---:|---|
 | Convert to Shape | Rectangle, Rounded Rectangle, Ellipse | 3/3 | — |
 | Crop Marks | Crop Marks | 1/1 | — |
-| Distort & Transform | Free Distort, Pucker & Bloat, Roughen, Transform, Tweak, Twist, Zig Zag | 7/7 | Free Distort's dialog has a handle on each corner of the object's bounds, moved by dragging or with the arrow keys; Zig Zag's ridges are evenly spaced along each segment (#1558), and Smooth points make a continuous wave whose peaks and valleys have handles along the path (#2343); Roughen with Smooth keeps turn-backs smooth instead of cusps that draw as spikes (#1646); Expand Appearance of a path whose Transform effect makes copies gives a group of one object per copy, each painted as the path, and a second Transform effect's copies nest in a group per copy of the first (#5612); drawn, each copy of a path with a fill and a stroke (or several, an even-odd fill or a partly transparent one) paints below the one before, the fills above covering the strokes of the copies below, instead of one path of every copy (#5920), and a group's copies each hold all its members, the original group on top (#6316) |
+| Distort & Transform | Free Distort, Pucker & Bloat, Roughen, Transform, Tweak, Twist, Zig Zag | 7/7 | Free Distort's dialog has a handle on each corner of the object's bounds, moved by dragging or with the arrow keys; Zig Zag's ridges are evenly spaced along each segment (#1558), and Smooth points make a continuous wave whose peaks and valleys have handles along the path (#2343); Roughen with Smooth keeps turn-backs smooth instead of cusps that draw as spikes (#1646); Expand Appearance of a path whose Transform effect makes copies gives a group of one object per copy, each painted as the path, and a second Transform effect's copies nest in a group per copy of the first (#5612); drawn, each copy of a path with a fill and a stroke (or several, an even-odd fill or a partly transparent one) paints below the one before, the fills above covering the strokes of the copies below, instead of one path of every copy (#5920), and a group's copies each hold all its members, the original group on top (#6316), except that a group with a member that carries a geometry, Pathfinder or 3D effect copies each member in its place |
 | Path | Offset Path, Outline Object, Outline Stroke | 3/3 | With Use Preview Bounds on, Align, Distribute and the Transform panel still measure type that carries Outline Object by its layout box (preview bounds leave out effects) |
 | Pathfinder | Add, Intersect, Exclude, Subtract, Minus Back, Divide, Trim, Merge, Crop, Outline, Hard Mix, Soft Mix, Trap | 13/13 | Trap traps only solid fills (gradients and patterns hide what is below them but don't trap) and spreads each colour's visible area as a whole; the public page gives no default values, so Trap's 0.25 pt, 100% and 40% are VectorCraft's own |
 | Rasterize | Rasterize… (as a live effect) | 1/1 | — |