From 35db931810cff174e09bc4a28d207b01061481a4 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 9 Nov 2026 16:24:13 -0800 Subject: [PATCH] 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 | 2 +- crates/effects/src/reshape.rs | 86 +++++++++--- crates/effects/src/tests/transformfx.rs | 167 ++++++++++++++++++++++++ crates/pdf/src/tests.rs | 4 +- crates/svg/tests/it/container_export.rs | 2 +- docs/effects-parity.md | 2 +- 6 files changed, 241 insertions(+), 22 deletions(-) diff --git a/crates/effects/src/catalog.rs b/crates/effects/src/catalog.rs index 95111b5fb..e9bbaf4bb 100644 --- a/crates/effects/src/catalog.rs +++ b/crates/effects/src/catalog.rs @@ -247,7 +247,7 @@ pub fn effect_catalog() -> Vec { "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( diff --git a/crates/effects/src/reshape.rs b/crates/effects/src/reshape.rs index 4db4bb3f5..f6c5cf2ae 100644 --- a/crates/effects/src/reshape.rs +++ b/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> = 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(©); + let cost = copy_cost(©); 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> = 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(©, &moved)); + if !budget.fits(made.plus(cost)) || !copy.geometric_bounds().is_some_and(crate::distort::within_reach) { break; } placements.push(Arc::new(copy)); diff --git a/crates/effects/src/tests/transformfx.rs b/crates/effects/src/tests/transformfx.rs index e424a8293..de69116b7 100644 --- a/crates/effects/src/tests/transformfx.rs +++ b/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 = 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" + ); +} diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index a8a22b00f..365b65389 100644 --- a/crates/pdf/src/tests.rs +++ b/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 = 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. diff --git a/crates/svg/tests/it/container_export.rs b/crates/svg/tests/it/container_export.rs index 5f34b6cc3..9518d31d2 100644 --- a/crates/svg/tests/it/container_export.rs +++ b/crates/svg/tests/it/container_export.rs @@ -92,5 +92,5 @@ fn a_group_transform_copies_its_images() { .filter(|l| l.starts_with("