mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-29 12:17:14 -06:00
fix(format): remove what a native file has beyond the canvas (#3575)
Edits refuse to leave anything beyond MAX_COORD (doc_sane checks the artboards, the guides, the ruler origin and the objects an edit touches), but the native loader didn't check them. A file with one object, artboard, guide or ruler origin out there opened without a warning, and then every move failed with "result would exceed the canvas", in-range objects included; saving kept it that way. Document::drop_beyond_canvas removes objects whose geometric bounds aren't finite or reach past MAX_COORD (a layer or non-clipping group keeps its children within it), artboards with their artboard guides, and guides out there, and puts the ruler origin back at the origin. load_file runs it after the depth limit and reports what it did with the other load notes; a file whose artboards all lie beyond the canvas is refused.
This commit is contained in:
@@ -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<String> {
|
||||
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<String> =
|
||||
[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<Node>, 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<u32> = 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<Node> {
|
||||
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<Arc<Node>>) -> 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<u64> {
|
||||
let mut out = vec![];
|
||||
fn walk(n: &Node, out: &mut Vec<u64>) {
|
||||
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"
|
||||
)
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -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};
|
||||
|
||||
@@ -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}");
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -522,7 +522,8 @@ pub fn load(bytes: &[u8]) -> Result<Document, FormatError> {
|
||||
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<String>,
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user