Merge main into #1409

# Conflicts:
#	crates/engine/src/session/undo.rs
#	docs/roadmap.md

crates/engine/src/cmd/edit.rs

@@ -8,8 +8,8 @@ use serde_json::{Map, Value, json};
 
 use super::{CommandSpec, ok};
 use crate::params::{
-    blend, boolean, color, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets, string,
-    targets,
+    blend, boolean, color, fraction_of, get, id, id_of, num, num_or, opt_index, paint_of, parse, req_id, req_index, req_num, req_string, req_targets,
+    string, targets,
 };
 use crate::{BatchStep, EngineError, Result, Session, bad, spec};
 

crates/engine/src/cmd/edit/props.rs

@@ -1,6 +1,6 @@
 use super::{
-    BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, get, json, num, ok, opt_index,
-    paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted,
+    BTreeMap, Effect, EngineError, Map, Node, NodeId, NodeKind, Result, Session, Value, bad, blend, boolean, color, fraction_of, get, json, num, ok,
+    opt_index, paint_of, parse, req_id, req_index, req_string, req_targets, roots, string, targets, z_sorted,
 };
 
 pub(super) fn batch_rename(s: &mut Session, p: &Value) -> Result<Value> {
@@ -312,8 +312,8 @@ pub(super) fn paint_set(s: &mut Session, p: &Value, fills: bool) -> Result<Value
                 slot.color_alpha = c.a;
             }
         }
-        if let Some(o) = num(p, "opacity") {
-            slot.opacity = (if o > 1.0 { o / 100.0 } else { o }).clamp(0.0, 1.0) as f32;
+        if let Some(o) = get(p, "opacity").and_then(fraction_of) {
+            slot.opacity = o.clamp(0.0, 1.0) as f32;
             slot.color_alpha = 1.0;
         }
         if let Some(v) = boolean(p, "visible") {

crates/engine/src/cmd/file.rs

@@ -432,5 +432,8 @@ mod rest_import_tests;
 #[cfg(test)]
 mod staging_write_tests;
 
+#[cfg(test)]
+mod thumbnail_tests;
+
 #[cfg(test)]
 mod zip_cap_tests;

crates/engine/src/cmd/file/save.rs

@@ -3,19 +3,38 @@ use super::load::{ext, file_stem, write};
 use super::open::remember;
 use super::*;
 
-/// Thumbnail PNG of the first page's content (for file browsers / the home screen).
+/// Thumbnail PNG of the chosen frame, or the first page's content (file browsers / home screen).
 pub fn thumbnail(doc: &Document) -> Option<Vec<u8>> {
-    let page = doc.first_page()?;
-    let b = doc.page_content_bounds(page)?;
-    let scale = (400.0 / b.width().max(b.height()).max(1.0)).min(2.0);
-    let view = figcraft_render::View {
-        origin: (b.x0, b.y0),
-        scale,
-        width: (b.width() * scale).ceil().max(1.0) as u32,
-        height: (b.height() * scale).ceil().max(1.0) as u32,
+    let render = |page, b: figcraft_geom::Rect, solo| {
+        let scale = (400.0 / b.width().max(b.height()).max(1.0)).min(2.0);
+        let view = figcraft_render::View {
+            origin: (b.x0, b.y0),
+            scale,
+            width: (b.width() * scale).ceil().max(1.0) as u32,
+            height: (b.height() * scale).ceil().max(1.0) as u32,
+        };
+        let opts = figcraft_render::Options { solo, ..Default::default() };
+        let pix = figcraft_render::render_view(doc, page, &view, &opts)?;
+        figcraft_render::encode_png(&pix).ok()
     };
-    let pix = figcraft_render::render_view(doc, page, &view, &Default::default())?;
-    figcraft_render::encode_png(&pix).ok()
+    doc.ids()
+        .find_map(|id| {
+            let n = doc.node(id)?;
+            if !n.kind.is_frame_like() || n.plugin_data.get("figcraft.thumbnail").map(String::as_str) != Some("true") {
+                return None;
+            }
+            // Hidden library-store pages are not document pages.
+            let page = doc.page_of(id).filter(|p| doc.pages.contains(p))?;
+            let b = figcraft_render::export_bounds(doc, id)?;
+            if ![b.x0, b.y0, b.x1, b.y1, b.width(), b.height()].into_iter().all(f64::is_finite) || b.width() <= 0.0 || b.height() <= 0.0 {
+                return None;
+            }
+            render(page, b, Some(id))
+        })
+        .or_else(|| {
+            let page = doc.first_page()?;
+            render(page, doc.page_content_bounds(page)?, None)
+        })
 }
 
 /// The `.figcraft` file for a document: content, thumbnail and version history.

crates/engine/src/cmd/file/thumbnail_tests.rs

@@ -0,0 +1,155 @@
+use super::*;
+use figcraft_color::{Color, Paint};
+use figcraft_geom::Affine;
+
+const RED: Color = Color::from_rgb8_const(255, 0, 0);
+const GREEN: Color = Color::from_rgb8_const(0, 255, 0);
+const BLUE: Color = Color::from_rgb8_const(0, 0, 255);
+
+fn scene() -> (Session, NodeId, NodeId) {
+    let mut doc = Document::new("Thumbnail");
+    let page = doc.first_page().unwrap();
+    let cover = doc.create(page, NodeKind::Frame, "Cover", 0.0, 600.0, 100.0, 100.0).unwrap();
+    let wide = doc.create(page, NodeKind::Frame, "Wide", 700.0, 600.0, 600.0, 100.0).unwrap();
+    doc.get_mut(cover).unwrap().fills = vec![Paint::solid(RED)];
+    doc.get_mut(wide).unwrap().fills = vec![Paint::solid(BLUE)];
+    (Session::with_document(doc), cover, wide)
+}
+
+fn choose(s: &mut Session, id: NodeId) {
+    s.execute("object.setThumbnail", &json!({"id": id})).unwrap();
+}
+
+fn picture(doc: &Document) -> image::RgbaImage {
+    image::load_from_memory(&thumbnail(doc).unwrap()).unwrap().to_rgba8()
+}
+
+#[test]
+fn chosen_frame_draws_its_children_without_siblings_or_document_changes() {
+    let (mut s, cover, _) = scene();
+    let doc = s.doc_mut().unwrap();
+    let page = doc.first_page().unwrap();
+    let child = doc.create(cover, NodeKind::Rectangle, "Child", 20.0, 20.0, 20.0, 20.0).unwrap();
+    doc.get_mut(child).unwrap().fills = vec![Paint::solid(GREEN)];
+    // Cropping the whole page to Cover's bounds would still draw this overlapping sibling.
+    let sibling = doc.create(page, NodeKind::Rectangle, "Overlap", 0.0, 600.0, 100.0, 100.0).unwrap();
+    doc.get_mut(sibling).unwrap().fills = vec![Paint::solid(BLUE)];
+    choose(&mut s, cover);
+    let before = s.doc().unwrap().clone();
+    let png = picture(s.doc().unwrap());
+    assert_eq!(png.dimensions(), (200, 200));
+    assert_eq!(png.get_pixel(10, 10).0, [255, 0, 0, 255]);
+    assert_eq!(png.get_pixel(60, 60).0, [0, 255, 0, 255]);
+    assert!(!png.pixels().any(|p| p.0 == [0, 0, 255, 255]), "neither blue sibling is drawn");
+    assert_eq!(s.doc().unwrap(), &before);
+    s.doc_mut().unwrap().set_world_transform(cover, Affine::translate((4200.0, -3600.0))).unwrap();
+    assert_eq!(picture(s.doc().unwrap()), png, "moving the frame changes neither size nor contents");
+}
+
+#[test]
+fn no_thumbnail_choice_keeps_the_first_page_content_bounds() {
+    let (s, _, _) = scene();
+    let png = picture(s.doc().unwrap());
+    assert_eq!(png.dimensions(), (400, 31));
+    assert_eq!(png.get_pixel(10, 15).0, [255, 0, 0, 255]);
+    assert_eq!(png.get_pixel(310, 15).0, [0, 0, 255, 255]);
+}
+
+#[test]
+fn invalid_flags_and_non_frames_keep_the_default_thumbnail() {
+    let (mut s, cover, wide) = scene();
+    let default = thumbnail(s.doc().unwrap()).unwrap();
+    for value in ["false", "TRUE", ""] {
+        s.doc_mut().unwrap().get_mut(cover).unwrap().plugin_data.insert("figcraft.thumbnail".into(), value.into());
+        assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "flag {value:?}");
+    }
+    choose(&mut s, wide);
+    s.doc_mut().unwrap().get_mut(wide).unwrap().kind = NodeKind::Rectangle;
+    assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "only frames can be thumbnails");
+    s.doc_mut().unwrap().get_mut(wide).unwrap().kind = NodeKind::Component { description: String::new(), props: Default::default() };
+    assert_ne!(thumbnail(s.doc().unwrap()).unwrap(), default, "a component counts as a frame");
+}
+
+#[test]
+fn deleting_the_chosen_frame_falls_back_to_the_remaining_page_content() {
+    let (mut s, cover, _) = scene();
+    choose(&mut s, cover);
+    s.doc_mut().unwrap().remove(cover).unwrap();
+    let png = picture(s.doc().unwrap());
+    assert_eq!(png.dimensions(), (400, 67));
+    assert_eq!(png.get_pixel(200, 30).0, [0, 0, 255, 255]);
+}
+
+#[test]
+fn invalid_thumbnail_ranges_and_pages_fall_back() {
+    let (mut s, _, _) = scene();
+    let default = thumbnail(s.doc().unwrap()).unwrap();
+    let doc = s.doc_mut().unwrap();
+    let page = doc.add_page("Invalid cover");
+    let cover = doc.create(page, NodeKind::Frame, "Cover", 0.0, 0.0, 100.0, 100.0).unwrap();
+    choose(&mut s, cover);
+    for (width, height) in [(0.0, 100.0), (100.0, 0.0), (f64::INFINITY, 100.0), (f64::NAN, 100.0)] {
+        let n = s.doc_mut().unwrap().get_mut(cover).unwrap();
+        n.width = width;
+        n.height = height;
+        assert_eq!(thumbnail(s.doc().unwrap()).unwrap(), default, "invalid range {width} x {height}");
+    }
+    let doc = s.doc_mut().unwrap();
+    let n = doc.get_mut(cover).unwrap();
+    n.width = 100.0;
+    n.height = 100.0;
+    n.parent = Some(NodeId(u64::MAX));
+    assert_eq!(thumbnail(doc).unwrap(), default, "missing parent page");
+    doc.get_mut(cover).unwrap().parent = Some(page);
+    doc.pages.retain(|p| *p != page);
+    assert_eq!(thumbnail(doc).unwrap(), default, "page outside the document's page list");
+}
+
+#[test]
+fn a_nested_frame_on_another_page_uses_its_page_background() {
+    let (mut s, _, _) = scene();
+    let doc = s.doc_mut().unwrap();
+    let page = doc.add_page("Cover page");
+    if let NodeKind::Page { background, .. } = &mut doc.get_mut(page).unwrap().kind {
+        *background = GREEN;
+    }
+    let section = doc.create(page, NodeKind::Section { contents_hidden: false }, "Section", 700.0, 900.0, 300.0, 200.0).unwrap();
+    let cover = doc.create(section, NodeKind::Frame, "Cover", -50.0, 80.0, 80.0, 40.0).unwrap();
+    doc.get_mut(cover).unwrap().fills.clear();
+    let child = doc.create(cover, NodeKind::Rectangle, "Child", 10.0, 10.0, 20.0, 10.0).unwrap();
+    doc.get_mut(child).unwrap().fills = vec![Paint::solid(RED)];
+    choose(&mut s, cover);
+    let png = picture(s.doc().unwrap());
+    assert_eq!(png.dimensions(), (160, 80));
+    assert_eq!(png.get_pixel(10, 10).0, [0, 255, 0, 255]);
+    assert_eq!(png.get_pixel(30, 30).0, [255, 0, 0, 255]);
+}
+
+#[test]
+fn custom_thumbnails_keep_the_size_limit_and_aspect_ratio() {
+    let (mut s, cover, _) = scene();
+    choose(&mut s, cover);
+    for (width, height, want) in [(100.0, 100.0, (200, 200)), (800.0, 200.0, (400, 100)), (200.0, 800.0, (100, 400))] {
+        let n = s.doc_mut().unwrap().get_mut(cover).unwrap();
+        n.width = width;
+        n.height = height;
+        assert_eq!(picture(s.doc().unwrap()).dimensions(), want);
+    }
+}
+
+#[test]
+fn native_save_and_fig_export_embed_the_same_chosen_thumbnail() {
+    let (mut s, cover, _) = scene();
+    choose(&mut s, cover);
+    let before = s.doc().unwrap().clone();
+    let expected = picture(s.doc().unwrap());
+    let native = file_bytes(s.ds().unwrap()).unwrap();
+    let (fig, _) = super::export::fig_bytes(s.doc().unwrap()).unwrap();
+    for bytes in [native, fig] {
+        let png = figcraft_format::thumbnail::thumbnail_png(std::io::Cursor::new(bytes)).unwrap();
+        let actual = image::load_from_memory(&png).unwrap().to_rgba8();
+        assert_eq!(actual.dimensions(), (200, 200));
+        assert_eq!(actual, expected);
+    }
+    assert_eq!(s.doc().unwrap(), &before);
+}

crates/engine/src/cmd/layout.rs

@@ -181,6 +181,19 @@ mod tests {
         assert!(s.execute("node.set", &json!({"id": f, "props": {"paddingLeft": "wide"}})).is_err());
     }
 
+    #[test]
+    fn junk_axis_padding_is_refused_and_keeps_padding() {
+        let mut s = Session::new();
+        let (f, _, _) = al_frame(&mut s);
+        ex(&mut s, "node.set", json!({"id": f, "props": {"padding": 16}}));
+        for key in ["paddingHorizontal", "paddingVertical"] {
+            for junk in [json!("abc"), json!({"a": 1}), json!([1, 2])] {
+                assert!(s.execute("node.set", &json!({"id": f, "props": {key: junk}})).is_err(), "{key}");
+                assert_eq!(node(&s, f).layout.as_ref().unwrap().padding, [16.0; 4], "{key}");
+            }
+        }
+    }
+
     #[test]
     fn baseline_alignment_lines_up_text() {
         let mut s = Session::new();

crates/engine/src/cmd/page.rs

@@ -94,7 +94,7 @@ pub fn specs() -> Vec<CommandSpec> {
             ok()
         }),
         spec!("page.duplicate", "Duplicate page", "{id?}", true, |s, p| {
-            let src = id(p, "id").unwrap_or(s.page()?);
+            let src = page_target(s, "page.duplicate", p)?;
             let doc = s.doc_mut()?;
             let name = format!("Copy of {}", doc.get(src)?.name);
             let np = doc.add_page(&name);

crates/engine/src/cmd/props.rs

@@ -9,14 +9,14 @@ use figcraft_doc::{
 use figcraft_geom::{Affine, Point};
 use serde_json::{Map, Value, json};
 
-use crate::params::{bool_of, color_of, f64_of, get, paints_of};
+use crate::params::{bool_of, color_of, f64_of, fraction_of, get, paints_of};
 use crate::{Result, bad};
 
 /// The per-side stroke weight keys, in `StrokeProps::sides` order (top, right, bottom, left).
 pub const STROKE_SIDES: [&str; 4] = ["strokeTopWeight", "strokeRightWeight", "strokeBottomWeight", "strokeLeftWeight"];
 
 /// Keys `node.set` understands (documented for agents).
-pub const KEYS: &str = "name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \
+pub const KEYS: &str = "name, visible, locked, opacity (0-1 or \"50%\"), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \
 fills (paints or colours; gradients as {type: linearGradient|radialGradient|angularGradient|diamondGradient, stops: [\"#hex\", ...], from?, to?} too), fill (colour, or \"none\"/null: no fill), strokes, stroke (colour, or \"none\"/null: no stroke), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \
 strokeCap / startCap / endCap, strokeJoin, miterAngle (degrees: sharper joins bevel; or miterLimit as a ratio), dashes [..], widthProfile (preset uniform|wedge|taper|eye|rounded|pinch, [[t, scale], ...] or null), brush (preset id, {preset, spacing, sizeJitter, rotationJitter, scatter, seed} or null), cornerRadius (n or [tl,tr,br,bl]), cornerSmoothing (0-1), effects, \
 clip, isMask, layout (null|horizontal|vertical|grid|{...}), gap, crossGap, padding (n or [t,r,b,l]), paddingTop/Right/Bottom/Left, \
@@ -169,7 +169,7 @@ pub(crate) fn letter_spacing(v: &Value) -> Option<LetterSpacing> {
 fn four(v: &Value) -> Option<[f64; 4]> {
     match v {
         Value::Array(a) => {
-            let f: Vec<f64> = a.iter().filter_map(f64_of).collect();
+            let f: Vec<f64> = a.iter().map(f64_of).collect::<Option<_>>()?;
             match f.as_slice() {
                 [x] => Some([*x; 4]),
                 [a, b] => Some([*a, *b, *a, *b]),
@@ -487,8 +487,7 @@ fn apply_look(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) ->
         "visible" => doc.get_mut(id)?.visible = v.as_bool().ok_or_else(|| bad(cmd, "`visible` must be a boolean"))?,
         "locked" => doc.get_mut(id)?.locked = v.as_bool().ok_or_else(|| bad(cmd, "`locked` must be a boolean"))?,
         "opacity" => {
-            let o = f64_of(v).ok_or_else(|| bad(cmd, "`opacity` must be a number"))?;
-            let o = if o > 1.0 { o / 100.0 } else { o };
+            let o = fraction_of(v).ok_or_else(|| bad(cmd, "`opacity` must be a number"))?;
             doc.get_mut(id)?.opacity = o.clamp(0.0, 1.0) as f32;
         }
         "blend" | "blendMode" => {
@@ -650,13 +649,13 @@ fn apply_layout(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) -
             }
         }
         "paddingHorizontal" => {
-            let val = f64_of(v).unwrap_or(0.0).max(0.0);
+            let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.max(0.0);
             let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
             l.padding[1] = val;
             l.padding[3] = val;
         }
         "paddingVertical" => {
-            let val = f64_of(v).unwrap_or(0.0).max(0.0);
+            let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?.max(0.0);
             let l = layout_mut(cmd, k, doc.get_mut(id)?)?;
             l.padding[0] = val;
             l.padding[2] = val;
@@ -888,20 +887,20 @@ fn apply_shape(cmd: &str, doc: &mut Document, id: NodeId, k: &str, v: &Value) ->
             }
         }
         "innerRadius" | "ratio" => {
-            let r = f64_of(v).unwrap_or(0.38);
-            let r = if r > 1.0 { r / 100.0 } else { r };
+            let r = fraction_of(v).unwrap_or(0.38);
             match &mut doc.get_mut(id)?.kind {
                 NodeKind::Star { inner_ratio, .. } => *inner_ratio = r.clamp(0.0, 1.0),
                 _ => return Err(bad(cmd, "innerRadius applies to stars")),
             }
         }
         "arcStart" | "arcSweep" | "arcInner" => {
-            let val = f64_of(v).ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?;
+            let val = if k == "arcInner" { fraction_of(v) } else { f64_of(v) };
+            let val = val.ok_or_else(|| bad(cmd, format!("`{k}` must be a number")))?;
             match &mut doc.get_mut(id)?.kind {
                 NodeKind::Ellipse { start, sweep, inner } => match k {
                     "arcStart" => *start = val.to_radians(),
                     "arcSweep" => *sweep = val.clamp(-360.0, 360.0).to_radians(),
-                    _ => *inner = (if val > 1.0 { val / 100.0 } else { val }).clamp(0.0, 0.99),
+                    _ => *inner = val.clamp(0.0, 0.99),
                 },
                 _ => return Err(bad(cmd, "arc applies to ellipses")),
             }
@@ -1370,4 +1369,13 @@ mod tests {
         }
         assert!(matches!(line_height(&json!("150%")), Some(LineHeight::Percent(p)) if p == 150.0));
     }
+
+    #[test]
+    fn four_rejects_non_numeric_entries() {
+        assert_eq!(four(&json!([10, "abc"])), None);
+        assert_eq!(four(&json!([10, "abc", null, 20])), None);
+        assert_eq!(four(&json!([10, 20, 30])), None);
+        assert_eq!(four(&json!([1, 2, 3, 4])), Some([1.0, 2.0, 3.0, 4.0]));
+        assert_eq!(four(&json!([1, 2])), Some([1.0, 2.0, 1.0, 2.0]));
+    }
 }