diff --git a/crates/engine/src/cmd/addons/cleanup/checks.rs b/crates/engine/src/cmd/addons/cleanup/checks.rs index c3abd029b..4b03f0e43 100644 --- a/crates/engine/src/cmd/addons/cleanup/checks.rs +++ b/crates/engine/src/cmd/addons/cleanup/checks.rs @@ -120,12 +120,12 @@ impl Kind { let removable = !place.clips && !is_guide_or_clip(n); let gone = |is: bool| is.then(Vec::new); match self { - Kind::EmptyText => gone(matches!(&n.kind, NodeKind::Text(t) if t.plain_text().trim().is_empty())), + Kind::EmptyText => gone(removable && matches!(&n.kind, NodeKind::Text(t) if t.plain_text().trim().is_empty())), Kind::StrayPoints => gone(removable && matches!(&n.kind, NodeKind::Path { path, .. } if path.anchor_count() <= 1)), Kind::Unpainted => gone(removable && !place.in_compound && is_shape(n) && !is_painted(&n.appearance)), Kind::ZeroOpacity => gone(removable && !place.in_compound && n.opacity <= 0.0), Kind::DuplicateObjects => gone(removable && duplicates_the_one_below(cx.d, n)), - Kind::OffArtboard => gone(place.top && !is_guide_or_clip(n) && off_artboards(cx, n)), + Kind::OffArtboard => gone(place.top && removable && off_artboards(cx, n)), Kind::NeedlessGroups => needless_group(n), Kind::NeedlessCompounds => needless_compound(n), Kind::NeedlessClipGroups => needless_clip_group(cx, n), @@ -175,7 +175,9 @@ fn objects<'a>(d: &'a Document, scope: &Scope) -> Vec<(&'a Node, Place)> { let top = Place { top: true, ..Place::default() }; match scope { Scope::Document => d.layers.iter().for_each(|l| visit(l, top, &mut out)), - Scope::Objects(ids) => ids.iter().filter_map(|id| d.node(*id)).for_each(|n| visit(n, top, &mut out)), + Scope::Objects(ids) => { + ids.iter().filter_map(|id| d.node(*id)).for_each(|n| visit(n, Place { clips: is_clipping_path(d, n.id), ..top }, &mut out)) + } } let editable = d.editable(); out.retain(|(n, _)| editable(n.id)); @@ -202,6 +204,11 @@ fn visit<'a>(n: &'a Node, place: Place, out: &mut Vec<(&'a Node, Place)>) { } } +/// Is `id` the clipping path of its group or layer? +fn is_clipping_path(d: &Document, id: NodeId) -> bool { + d.position(id).is_some_and(|(parent, i, _)| i == 0 && parent.and_then(|p| d.node(p)).is_some_and(Node::clips)) +} + /// A group that stands for live or generated art: its members aren't loose art to clean. fn is_generated(n: &Node) -> bool { let e = n.extras(); diff --git a/crates/engine/src/cmd/addons/cleanup/tests.rs b/crates/engine/src/cmd/addons/cleanup/tests.rs index d02e1e4c0..2860d39e7 100644 --- a/crates/engine/src/cmd/addons/cleanup/tests.rs +++ b/crates/engine/src/cmd/addons/cleanup/tests.rs @@ -226,6 +226,49 @@ fn each_check_finds_and_fixes_its_issue_and_leaves_clean_art_alone() { } } +/// The pixel at (100, 100) of the document drawn on white. +fn pixel(s: &Session) -> [u8; 4] { + let opts = vectorcraft_render::RenderOptions { background: Some([255, 255, 255, 255]), ..Default::default() }; + vectorcraft_render::Renderer::new().render(&s.doc().unwrap().doc, 400, 300, vectorcraft_geom::Affine::IDENTITY, &opts).pixel(100, 100) +} + +/// A clipping path hides the art outside it: empty type that clips a group, and type off every +/// artboard that clips a layer, stay, and the art stays hidden. +#[test] +fn clipping_type_stays_with_the_art_it_hides() { + const WHITE: [u8; 4] = [255, 255, 255, 255]; + let mut s = session(); + let art = rect(&mut s, 50.0, 50.0, 100.0, 100.0); + s.execute("paint.setFill", &json!({"color": "#ff0000"})).unwrap(); + let t = id_of(&s.execute("text.create", &json!({"x": 60, "y": 80, "text": " ", "size": 40})).unwrap()); + select(&mut s, &[art, t]); + s.execute("object.clippingMask.make", &json!({})).unwrap(); + select(&mut s, &[]); + assert_eq!(pixel(&s), WHITE); + assert_eq!(found(&mut s, "emptyText", "document"), 0); + assert_eq!(s.execute("addon.cleanup.fix", &json!({"scope": "document"})).unwrap()["total"], 0); + select(&mut s, &[t]); + assert_eq!(found(&mut s, "emptyText", "selection"), 0, "the clipping path selected"); + assert_eq!(s.execute("addon.cleanup.fix", &json!({"scope": "selection"})).unwrap()["total"], 0); + assert!(node(&s, t).is_some()); + assert_eq!(pixel(&s), WHITE); + + let mut s = session(); + rect(&mut s, 50.0, 50.0, 100.0, 100.0); + s.execute("paint.setFill", &json!({"color": "#ff0000"})).unwrap(); + let t = id_of(&s.execute("text.create", &json!({"x": -300, "y": -300, "text": "Clip", "size": 40})).unwrap()); + s.execute("layer.clippingMask.toggle", &json!({})).unwrap(); + select(&mut s, &[]); + assert_eq!(pixel(&s), WHITE); + assert_eq!(found(&mut s, "offArtboard", "document"), 0); + assert_eq!(s.execute("addon.cleanup.fix", &json!({"scope": "document"})).unwrap()["total"], 0); + select(&mut s, &[t]); + assert_eq!(found(&mut s, "offArtboard", "selection"), 0, "the clipping path selected"); + assert_eq!(s.execute("addon.cleanup.fix", &json!({"scope": "selection"})).unwrap()["total"], 0); + assert!(node(&s, t).is_some()); + assert_eq!(pixel(&s), WHITE); +} + /// Type set twice in the same place (from a PDF, or for a fake bold) is a duplicate within a point: /// Show selects the upper copy and Fix removes it. Other text, or the same text further off, stays. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index f1e55b9fd..20a3f2720 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -5936,12 +5936,12 @@ envelopes, graphs, symbols), are left out. | Kind | Finds | Fix | |---|---|---| -| `emptyText` | Text with no characters, or only spaces | Removes it | +| `emptyText` | Text with no characters, or only spaces (not clipping paths) | Removes it | | `strayPoints` | Paths of one anchor (not clipping paths) | Removes them | | `unpainted` | Paths and compound paths with no fill and no stroke | Removes them | | `zeroOpacity` | Objects at 0 % opacity | Removes them | | `duplicateObjects` | A path stacked directly on an identical one (geometry, paint, opacity), or type stacked directly on the same type (text, styles, paint) within 1 pt of it | Removes the upper one | -| `offArtboard` | Top-level objects that touch no artboard | Removes them | +| `offArtboard` | Top-level objects that touch no artboard (not clipping paths) | Removes them | | `duplicateAnchors` | Neighbouring anchors within 0.01 pt | Merges them | | `tinyHandles` | Handles shorter than 0.01 pt | Retracts them | | `collinearAnchors` | Corner anchors on a straight run (turning by a sine of 0.005 or less) | Removes them |