Merge pull request #801 from phonedu/fix/silent-document-errors

M1: vector.create honours width/height by scaling the path

crates/engine/src/cmd/create.rs

@@ -73,9 +73,13 @@ pub fn specs() -> Vec<CommandSpec> {
         spec!("text.create", "Text", "{text, x, y, parent?, width? (fixed width wraps), props? (fontSize, fontFamily, …)}", true, |s, p| {
             create_kind(s, p, "text")
         }),
-        spec!("vector.create", "Vector", "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, parent?, props?}", true, |s, p| create_kind(
-            s, p, "vector"
-        )),
+        spec!(
+            "vector.create",
+            "Vector",
+            "{path (SVG path data) | points [[x,y],..], closed?, x?, y?, width?, height? (scale the path; one keeps its proportion), parent?, props?}",
+            true,
+            |s, p| create_kind(s, p, "vector")
+        ),
         spec!(
             "image.place",
             "Place image",
@@ -162,6 +166,37 @@ pub fn parse_path_data(d: &str) -> Option<figcraft_geom::BezPath> {
     figcraft_geom::BezPath::from_svg(d).ok()
 }
 
+/// How much a vector's path is scaled to take the size asked for: with both sides, each scales
+/// independently (as resizing a vector does); with one, the other keeps its proportion. A size on
+/// an axis the geometry has none of is an error unless it is 0 (a line's height), and a size of 0
+/// or less on an axis it has is refused rather than flattening it.
+fn vector_scale(width: Option<f64>, height: Option<f64>, bw: f64, bh: f64, what: &str) -> std::result::Result<(f64, f64), String> {
+    let fit = |asked: Option<f64>, have: f64, axis: &str| match asked {
+        None => Ok(1.0),
+        Some(a) if have <= 1e-9 => {
+            if a == 0.0 {
+                Ok(1.0)
+            } else {
+                Err(format!("`{axis}` can't size {what} with no {} extent", if axis == "width" { "horizontal" } else { "vertical" }))
+            }
+        }
+        Some(a) if a > 0.0 && (a / have).is_finite() => Ok(a / have),
+        Some(_) => Err(format!("`{axis}` must be a positive, finite size: 0 or less would flatten {what}")),
+    };
+    Ok(match (width, height) {
+        (Some(_), Some(_)) => (fit(width, bw, "width")?, fit(height, bh, "height")?),
+        (Some(_), None) => {
+            let s = fit(width, bw, "width")?;
+            (s, s)
+        }
+        (None, Some(_)) => {
+            let s = fit(height, bh, "height")?;
+            (s, s)
+        }
+        (None, None) => (1.0, 1.0),
+    })
+}
+
 pub fn create(s: &mut Session, p: &Value) -> Result<Value> {
     let cmd = "node.create";
     let t = string(p, "type").ok_or_else(|| bad(cmd, "`type` is required"))?;
@@ -187,20 +222,32 @@ pub fn create(s: &mut Session, p: &Value) -> Result<Value> {
         "Section" => (num_or(p, "width", 400.0), num_or(p, "height", 300.0)),
         _ => (num_or(p, "width", 100.0), num_or(p, "height", 100.0)),
     };
-    // Vectors: path data or points define the shape and its box.
+    // Vectors: path data or points define the shape and its box. `width`/`height`, when asked
+    // for, scale the geometry: both sides independently, one side proportionally.
     let mut offset = (0.0, 0.0);
+    let asked = (num(p, "width"), num(p, "height"));
     if let NodeKind::Vector { network } = &mut kind {
         if let Some(d) = string(p, "path") {
             let mut path = parse_path_data(&d).ok_or_else(|| bad(cmd, "`path` is not valid SVG path data"))?;
             use figcraft_geom::Shape;
             let bb = path.bounding_box();
+            if bb.width() <= 1e-9 && bb.height() <= 1e-9 {
+                return Err(bad(cmd, "`path` draws nothing: a vector needs a visible line or area"));
+            }
             path.apply_affine(figcraft_geom::Affine::translate((-bb.x0, -bb.y0)));
+            let (sx, sy) = vector_scale(asked.0, asked.1, bb.width(), bb.height(), "a path").map_err(|e| bad(cmd, e))?;
+            if sx != 1.0 || sy != 1.0 {
+                path.apply_affine(figcraft_geom::Affine::scale_non_uniform(sx, sy));
+            }
             *network = VectorNetwork::from_path(&path);
+            if network.segments.is_empty() {
+                return Err(bad(cmd, "`path` draws nothing: a vector needs a visible line or area"));
+            }
             if boolean(p, "closed") == Some(false) {
                 network.regions.clear();
             }
-            w = bb.width();
-            h = bb.height();
+            w = bb.width() * sx;
+            h = bb.height() * sy;
             offset = (bb.x0, bb.y0);
         } else if let Some(Value::Array(pts)) = get(p, "points") {
             let pts: Vec<Point> = pts.iter().filter_map(|v| Some(Point::new(f64_of(v.get(0)?)?, f64_of(v.get(1)?)?))).collect();
@@ -208,10 +255,14 @@ pub fn create(s: &mut Session, p: &Value) -> Result<Value> {
                 return Err(bad(cmd, "`points` needs at least two [x, y] pairs"));
             }
             let bb = figcraft_geom::points_bounds(pts.iter().copied()).unwrap_or_default();
-            let local: Vec<Point> = pts.iter().map(|q| Point::new(q.x - bb.x0, q.y - bb.y0)).collect();
+            if bb.width() <= 1e-9 && bb.height() <= 1e-9 {
+                return Err(bad(cmd, "`points` draws nothing: a vector needs a visible line or area"));
+            }
+            let (sx, sy) = vector_scale(asked.0, asked.1, bb.width(), bb.height(), "a point set").map_err(|e| bad(cmd, e))?;
+            let local: Vec<Point> = pts.iter().map(|q| Point::new((q.x - bb.x0) * sx, (q.y - bb.y0) * sy)).collect();
             *network = VectorNetwork::from_points(&local, boolean(p, "closed").unwrap_or(false));
-            w = bb.width();
-            h = bb.height();
+            w = bb.width() * sx;
+            h = bb.height() * sy;
             offset = (bb.x0, bb.y0);
         }
     }

crates/engine/src/vector_behaviour_tests.rs

@@ -439,3 +439,61 @@ fn bend_paint_bucket_cut_join_and_eraser_tools() {
     assert_eq!(net(&s, id).segments.len(), 2, "the eraser removes what it passes over");
     assert_eq!(s.doc().unwrap().node(page).unwrap().children, vec![id], "and the pieces stay one layer");
 }
+
+#[test]
+fn vector_paths_take_the_requested_size_and_refuse_empty_geometry() {
+    let mut s = Session::new();
+    let path = "M0 0 L24 0 L24 24 L0 24 Z";
+    let intrinsic = id_of(&ex(&mut s, "vector.create", json!({"path": path}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(intrinsic).unwrap();
+    assert_eq!((n.width, n.height), (24.0, 24.0), "a path without a size keeps its intrinsic box");
+    // Both sides: each scales independently, like resizing a vector.
+    let both = id_of(&ex(&mut s, "vector.create", json!({"path": path, "width": 16, "height": 8}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(both).unwrap();
+    assert_eq!((n.width, n.height), (16.0, 8.0));
+    let n = net(&s, both);
+    let max = n.vertices.iter().fold((0.0f64, 0.0f64), |(x, y), v| (x.max(v.x), y.max(v.y)));
+    assert!((max.0 - 16.0).abs() < 1e-9 && (max.1 - 8.0).abs() < 1e-9, "{max:?}: the geometry itself is scaled");
+    // One side: the other keeps its proportion.
+    let width = id_of(&ex(&mut s, "vector.create", json!({"path": path, "width": 16}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(width).unwrap();
+    assert_eq!((n.width, n.height), (16.0, 16.0));
+    // A path or point set that draws nothing is an error, not an invisible zero-size layer.
+    let e = s.execute("vector.create", &json!({"path": "M5 5"})).unwrap_err().to_string();
+    assert!(e.contains("draws nothing"), "{e}");
+    let e = s.execute("vector.create", &json!({"points": [[5, 5], [5, 5]]})).unwrap_err().to_string();
+    assert!(e.contains("draws nothing"), "{e}");
+    // A size on an axis the geometry has none of is refused, not silently ignored.
+    let e = s.execute("vector.create", &json!({"path": "M0 0 L0 24", "width": 16})).unwrap_err().to_string();
+    assert!(e.contains("horizontal extent"), "{e}");
+    let e = s.execute("vector.create", &json!({"path": "M0 0 L24 0", "height": 16})).unwrap_err().to_string();
+    assert!(e.contains("vertical extent"), "{e}");
+    // But 0 on an axis it has none of is what it already is: a horizontal line with height 0.
+    let line = id_of(&ex(&mut s, "vector.create", json!({"path": "M0 0 L24 0", "width": 16, "height": 0}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(line).unwrap();
+    assert_eq!((n.width, n.height), (16.0, 0.0));
+    // A size of 0 or less on an axis it has would flatten it, so it is refused, alone or with the other.
+    for p in [json!({"path": path, "width": 0}), json!({"path": path, "width": 16, "height": 0}), json!({"path": path, "width": -4, "height": 8})] {
+        let e = s.execute("vector.create", &p).unwrap_err().to_string();
+        assert!(e.contains("positive"), "{p}: {e}");
+    }
+}
+
+#[test]
+fn vector_points_take_the_requested_size() {
+    let mut s = Session::new();
+    let points = json!([[10, 20], [34, 20], [34, 32]]);
+    let corners = |s: &Session, id: NodeId| net(s, id).vertices.iter().map(|v| (v.x, v.y)).collect::<Vec<_>>();
+    // Both sides: each axis scales on its own; the vertices move with the box.
+    let both = id_of(&ex(&mut s, "vector.create", json!({"points": points, "x": 10, "y": 20, "width": 48, "height": 6}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(both).unwrap();
+    assert_eq!((n.width, n.height), (48.0, 6.0));
+    assert_eq!(corners(&s, both), vec![(0.0, 0.0), (48.0, 0.0), (48.0, 6.0)]);
+    // One side: the other keeps its proportion.
+    let one = id_of(&ex(&mut s, "vector.create", json!({"points": points, "height": 24}))["id"]).unwrap();
+    let n = s.doc().unwrap().node(one).unwrap();
+    assert_eq!((n.width, n.height), (48.0, 24.0));
+    assert_eq!(corners(&s, one), vec![(0.0, 0.0), (48.0, 0.0), (48.0, 24.0)]);
+    let e = s.execute("vector.create", &json!({"points": points, "width": 16, "height": -1})).unwrap_err().to_string();
+    assert!(e.contains("positive"), "{e}");
+}

crates/mcp/src/tools.rs

@@ -147,7 +147,7 @@ pub fn tool_definitions() -> Vec<Value> {
                     "type": {"type": "string", "enum": ["frame", "rectangle", "ellipse", "polygon", "star", "line", "arrow", "text", "vector", "section", "component", "slice"], "description": "Layer type"},
                     "x": num("Left, relative to the parent frame (default 0)"),
                     "y": num("Top, relative to the parent frame (default 0)"),
-                    "width": num("Width (default 100; text hugs its content unless width is set, which makes it wrap)"),
+                    "width": num("Width (default 100; text hugs its content unless width is set, which makes it wrap; for a vector with path, width/height scale the path, one alone keeping its proportion, and without them the path's own box is used)"),
                     "height": num("Height (default 100; ignored for lines/arrows and auto-height text)"),
                     "parent": id_schema("Parent frame/group/component id (default: the current page)"),
                     "index": int("Z-index among the parent's children (default: on top)"),

docs/mcp.md

@@ -426,6 +426,12 @@ on; `key` resolves keys the way the canvas does. Commands behind keys:
 
 ### Editing vectors
 
+`vector.create {path | points}` makes the layer's box from the geometry; `width`/`height` scale
+the geometry to the size asked for (with both, each side independently; with one, the other keeps
+its proportion). A path or point set that draws nothing is refused instead of making an invisible
+0×0 layer, as is a size on an axis the geometry has none of (except 0, such as a horizontal line's
+height) and a size of 0 or less on an axis it has, which would flatten it.
+
 Vector edit mode is driven with `run_command` (or `pointer`/`key` for gestures):
 `vector.edit.begin {id? | ids?}` (default the selection; converts shapes to vectors; several layers
 are edited at once, the first active; `key` Enter with shapes selected does the same),