diff --git a/crates/doc/src/canvas_range.rs b/crates/doc/src/canvas_range.rs new file mode 100644 index 000000000..68658b58d --- /dev/null +++ b/crates/doc/src/canvas_range.rs @@ -0,0 +1,185 @@ +//! A read document brought within the canvas: what lies beyond [`MAX_COORD`] is removed (#3575). +//! Edits refuse to leave anything there (`doc_sane` in the engine checks the artboards, the guides, +//! the ruler origin and the objects an edit touches), so a file holding something beyond it could be +//! opened but not edited: one object, artboard or guide out there made every move fail. + +use std::sync::Arc; + +use vectorcraft_geom::{Point, Rect}; + +use crate::{Document, MAX_COORD, Node, NodeKind}; + +/// What [`Document::drop_beyond_canvas`] removed. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct BeyondCanvas { + /// Objects removed whole (a layer or group keeps its children within the canvas). + pub objects: usize, + pub artboards: usize, + /// Guides beyond the canvas, and the artboard guides of the artboards removed. + pub guides: usize, + /// The ruler origin was beyond the canvas and is back at the origin. + pub ruler_origin: bool, +} + +impl BeyondCanvas { + /// The warning for what was removed, or `None` when nothing was. + pub fn note(&self) -> Option { + let count = |n: usize, one: &str, many: &str| match n { + 0 => None, + 1 => Some(format!("1 {one}")), + n => Some(format!("{n} {many}")), + }; + let parts: Vec = + [count(self.objects, "object", "objects"), count(self.artboards, "artboard", "artboards"), count(self.guides, "guide", "guides")] + .into_iter() + .flatten() + .collect(); + let limit = format!("more than {MAX_COORD} pt from the origin"); + match (parts.is_empty(), self.ruler_origin) { + (true, false) => None, + (true, true) => Some(format!("the ruler origin lay beyond the canvas ({limit}) and was reset")), + (false, origin) => Some(format!( + "{} beyond the canvas ({limit}) {} removed{}", + parts.join(", "), + if self.objects + self.artboards + self.guides == 1 { "was" } else { "were" }, + if origin { "; the ruler origin, beyond it too, was reset" } else { "" } + )), + } + } +} + +/// Is every side of `r` finite and within [`MAX_COORD`] of the origin? +fn rect_in_range(r: Rect) -> bool { + [r.x0, r.y0, r.x1, r.y1].iter().all(|v| in_range(*v)) +} + +fn in_range(v: f64) -> bool { + v.is_finite() && v.abs() <= MAX_COORD +} + +/// Does `n` reach beyond the canvas? An object without geometry doesn't. +fn beyond(n: &Node) -> bool { + n.geometric_bounds().is_some_and(|b| !rect_in_range(b)) +} + +/// Remove the children of `node` that reach beyond the canvas: a layer or group that isn't +/// clipping keeps those of its own children within it, anything else goes whole. Only a node +/// with such a child is copied. Recursion follows the nesting, which [`Document::limit_depth`] +/// bounds. +fn drop_children_beyond(node: &mut Arc, dropped: &mut usize) { + if !node.children().is_some_and(|c| c.iter().any(|c| beyond(c))) { + return; + } + let Some(children) = Arc::make_mut(node).children_mut() else { return }; + children.retain_mut(|c| { + if !beyond(c) { + return true; + } + if matches!(c.kind, NodeKind::Layer { clip: false, .. } | NodeKind::Group { clip: false, .. }) { + drop_children_beyond(c, dropped); + return true; + } + *dropped += 1; + false + }); +} + +impl Document { + /// Remove what a read file has beyond the canvas ([`MAX_COORD`], or not finite): objects + /// (see [`BeyondCanvas::objects`]), artboards, and guides with the artboard guides of those + /// artboards, and put a ruler origin beyond it back at the origin. Run after + /// [`Document::limit_depth`]. A document whose artboards were all removed has none left. + pub fn drop_beyond_canvas(&mut self) -> BeyondCanvas { + let mut gone = BeyondCanvas::default(); + for layer in &mut self.layers { + drop_children_beyond(layer, &mut gone.objects); + } + let removed: Vec = self.artboards.iter().filter(|a| !rect_in_range(a.rect)).map(|a| a.id).collect(); + self.artboards.retain(|a| rect_in_range(a.rect)); + gone.artboards = removed.len(); + let guides = self.guides.len(); + self.guides.retain(|g| in_range(g.pos) && g.artboard.is_none_or(|a| !removed.contains(&a))); + gone.guides = guides - self.guides.len(); + if !(in_range(self.ruler_origin.x) && in_range(self.ruler_origin.y)) { + self.ruler_origin = Point::ZERO; + gone.ruler_origin = true; + } + gone + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Appearance, Artboard, Guide, NodeId}; + use vectorcraft_geom::shapes; + + fn rect(id: u64, y: f64) -> Arc { + Arc::new(Node::path(NodeId(id), shapes::rectangle(Rect::new(0.0, y, 10.0, y + 10.0)), Appearance::default_art())) + } + + fn doc(children: Vec>) -> Document { + let mut d = Document::new(100.0, 100.0); + if let Some(ch) = d.layers.first_mut().and_then(|l| Arc::make_mut(l).children_mut()) { + ch.extend(children); + } + d + } + + fn ids(d: &Document) -> Vec { + let mut out = vec![]; + fn walk(n: &Node, out: &mut Vec) { + out.push(n.id.0); + for c in n.children().into_iter().flatten() { + walk(c, out); + } + } + for l in &d.layers { + walk(l, &mut out); + } + out + } + + #[test] + fn a_document_within_the_canvas_is_left_alone() { + let mut d = doc(vec![rect(10, 0.0), rect(11, MAX_COORD - 10.0), rect(12, -MAX_COORD)]); + let before = d.clone(); + assert_eq!(d.drop_beyond_canvas(), BeyondCanvas::default()); + assert_eq!(d, before); + assert_eq!(BeyondCanvas::default().note(), None); + } + + #[test] + fn objects_beyond_the_canvas_are_removed_and_groups_keep_the_rest() { + let group = Arc::new(Node::group(NodeId(20), vec![rect(21, 0.0), rect(22, 1e17)])); + let clip = Arc::new(Node::new(NodeId(30), NodeKind::Group { children: vec![rect(31, f64::NAN), rect(32, 0.0)], clip: true })); + let mut d = doc(vec![rect(10, 0.0), rect(11, 1e17), group, clip, rect(12, -MAX_COORD - 1.0)]); + let layer = d.layers[0].id.0; + let gone = d.drop_beyond_canvas(); + assert_eq!(gone, BeyondCanvas { objects: 4, ..Default::default() }); + assert_eq!(ids(&d), [layer, 10, 20, 21], "the group keeps its object within the canvas; the clip group goes with its clipping path"); + assert!(d.layers.iter().all(|l| !beyond(l))); + assert_eq!(gone.note().unwrap(), format!("4 objects beyond the canvas (more than {MAX_COORD} pt from the origin) were removed")); + } + + #[test] + fn artboards_guides_and_the_ruler_origin_beyond_the_canvas_are_removed_or_reset() { + let mut d = doc(vec![rect(10, 0.0)]); + let far = Artboard { id: 9, rect: Rect::new(0.0, 1e17, 100.0, 1e17 + 100.0), ..d.artboards[0].clone() }; + d.artboards.push(far); + let guide = |pos: f64, artboard| Guide { artboard, ..Guide::new(false, pos) }; + d.guides = vec![guide(50.0, None), guide(1e17, None), guide(f64::INFINITY, None), guide(20.0, Some(9))]; + d.ruler_origin = Point::new(1e17, 0.0); + let gone = d.drop_beyond_canvas(); + assert_eq!(gone, BeyondCanvas { objects: 0, artboards: 1, guides: 3, ruler_origin: true }); + assert_eq!(d.artboards.len(), 1); + assert_eq!(d.guides, [guide(50.0, None)], "the far artboard's own guide goes with it"); + assert_eq!(d.ruler_origin, Point::ZERO); + assert_eq!( + gone.note().unwrap(), + format!( + "1 artboard, 3 guides beyond the canvas (more than {MAX_COORD} pt from the origin) were removed; the ruler origin, beyond it too, was reset" + ) + ); + } +} diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index 18873d6d9..91ede3df9 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -117,6 +117,7 @@ use vectorcraft_color::{Swatch, SwatchGroup}; use vectorcraft_geom::{Affine, Point, Rect, Vec2}; mod artboard; +mod canvas_range; mod document; mod document_ext; mod document_guides; @@ -127,6 +128,7 @@ mod records; mod units; pub use artboard::{Artboard, LockedArtboards, RetiredArtboardId, VIDEO_PIXEL_ASPECT_RANGE, VIDEO_SAFE_INSETS}; +pub use canvas_range::BeyondCanvas; #[cfg(test)] use document_ext::free_id; pub use graphic_style::{DEFAULT_GRAPHIC_STYLE, GraphicStyle}; diff --git a/crates/engine/tests/it/beyond_canvas.rs b/crates/engine/tests/it/beyond_canvas.rs new file mode 100644 index 000000000..9dee868d5 --- /dev/null +++ b/crates/engine/tests/it/beyond_canvas.rs @@ -0,0 +1,69 @@ +//! #3575: a native file holding an object, artboard, guide or ruler origin beyond the canvas +//! (`MAX_COORD`) opened, but every move then failed with "result would exceed the canvas": edits +//! check those. Opening it now removes them (or resets the ruler origin) and says so. + +use serde_json::{Value, json}; +use vectorcraft_engine::Session; + +/// A saved two-rectangle document as JSON, with its first rectangle 1e17 pt down when `far_object`. +fn document(far_object: bool) -> Value { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 30})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 20, "height": 20})).unwrap(); + let mut v: Value = serde_json::from_slice(&vectorcraft_format::save(&s.doc().unwrap().doc, false)).unwrap(); + if far_object { + let far = &mut v["document"]["layers"][0]["kind"]["children"][0]["kind"]; + far["path"]["subpaths"][0]["anchors"].as_array_mut().unwrap().iter_mut().for_each(|a| a["p"][1] = json!(a["p"][1].as_f64().unwrap() + 1e17)); + far["live"]["xf"][5] = json!(far["live"]["xf"][5].as_f64().unwrap() + 1e17); + } + v +} + +/// Open `v` from a file, select everything and move it: → the open reply's warnings. +fn open_and_move(v: &Value, tag: &str) -> (Session, Value) { + let path = std::env::temp_dir().join(format!("vectorcraft-beyond-canvas-{tag}-{}.vectorcraft", std::process::id())); + std::fs::write(&path, serde_json::to_vec(v).unwrap()).unwrap(); + let mut s = Session::new(); + let opened = s.execute("document.open", &json!({"path": path})); + let _ = std::fs::remove_file(&path); + let opened = opened.unwrap(); + s.execute("select.all", &json!({})).unwrap(); + s.execute("object.move", &json!({"dx": 5, "dy": 5})).expect("everything left can be moved"); + (s, opened["warnings"].clone()) +} + +#[test] +fn an_object_beyond_the_canvas_is_removed_on_open() { + let (s, warnings) = open_and_move(&document(true), "object"); + assert_eq!(s.doc().unwrap().doc.layers[0].children().unwrap().len(), 1, "the far rectangle is gone, the other stays"); + assert!(warnings.to_string().contains("1 object beyond the canvas"), "{warnings}"); +} + +#[test] +fn an_artboard_a_guide_or_a_ruler_origin_beyond_the_canvas_no_longer_blocks_edits() { + let mut v = document(false); + let mut far = v["document"]["artboards"][0].clone(); + far["id"] = json!(9); + far["rect"] = json!({"x0": 0.0, "y0": 1e17, "x1": 100.0, "y1": 1e17 + 100.0}); + v["document"]["artboards"].as_array_mut().unwrap().push(far); + v["document"]["guides"] = json!([{"vertical": false, "pos": 1e17}, {"vertical": true, "pos": 50.0}]); + v["document"]["ruler_origin"] = json!([1e17, 0.0]); + let (s, warnings) = open_and_move(&v, "setup"); + let d = &s.doc().unwrap().doc; + assert_eq!((d.artboards.len(), d.guides.len(), d.ruler_origin.x), (1, 1, 0.0)); + assert!(warnings.to_string().contains("1 artboard, 1 guide beyond the canvas"), "{warnings}"); + assert!(warnings.to_string().contains("ruler origin"), "{warnings}"); +} + +#[test] +fn a_file_whose_artboards_all_lie_beyond_the_canvas_is_refused() { + let mut v = document(false); + v["document"]["artboards"][0]["rect"] = json!({"x0": 0.0, "y0": 1e17, "x1": 100.0, "y1": 1e17 + 100.0}); + let path = std::env::temp_dir().join(format!("vectorcraft-beyond-canvas-pages-{}.vectorcraft", std::process::id())); + std::fs::write(&path, serde_json::to_vec(&v).unwrap()).unwrap(); + let r = Session::new().execute("document.open", &json!({"path": path})); + let _ = std::fs::remove_file(&path); + let e = r.unwrap_err().to_string(); + assert!(e.contains("artboards all lie beyond the canvas"), "{e}"); +} diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index b0409cffd..5b9dc39c6 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -10,6 +10,7 @@ mod text_fixture; mod affinity_features; mod affinity_local; mod artboard_click; +mod beyond_canvas; mod clear_commands; mod compound_attributes; mod dimensions; diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 0e65fcb66..ad9df39ff 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -522,7 +522,8 @@ pub fn load(bytes: &[u8]) -> Result { pub struct NativeFile { pub doc: Document, /// What was done to bring the document within the nesting caps - /// ([`Document::limit_depth_noting`]: a file saved before them, or a damaged one). + /// ([`Document::limit_depth_noting`]: a file saved before them, or a damaged one) and within + /// the canvas ([`Document::drop_beyond_canvas`]: a damaged or hand-made file). pub depth_notes: Vec, pub info: FileInfo, /// ICC files by profile name (saved with Embed ICC Profiles; undecodable ones are left out). @@ -617,6 +618,17 @@ fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result doc.raster_effects.add_around = doc.raster_effects.add_around.clamp(0.0, vectorcraft_doc::rastersettings::MAX_ADD_AROUND); let mut depth_notes = vec![]; doc.limit_depth_noting(&mut depth_notes); + // Untrusted: nothing beyond the canvas, where no edit may leave anything (after the depth + // limit: the walk recurses). A file whose artboards all lie there has no page to show (#3575). + let had_artboards = !doc.artboards.is_empty(); + let beyond = doc.drop_beyond_canvas(); + if had_artboards && doc.artboards.is_empty() { + return Err(FormatError::NotVectorcraft(format!( + "its artboards all lie beyond the canvas (more than {} pt from the origin)", + vectorcraft_doc::MAX_COORD + ))); + } + depth_notes.extend(beyond.note()); // Untrusted: the type's sizes, leading, baseline shifts and scales, its indents and paragraph // spacing, and its tab stops, each within its range (after the depth limit: the walk recurses). doc.keep_type_values_in_range();