From 3e92904cba325f9a451f96aa54401a0f66d0400a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jindr=CC=8Cich=20S=CC=8Cars=CC=8Con?= Date: Thu, 22 Oct 2026 00:03:30 +0200 Subject: [PATCH] M6: Sketch import: review fixes Texts an auto layout places keep the layout's position (the next layout pass would take the baseline move back). A faded layer's group takes the layer's id, so stacks, links and overrides still find it; faded masks stay masks. figcraft-format no longer depends on figcraft-layout. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 - crates/engine/src/cmd/file.rs | 5 +++- crates/engine/src/sketch_import_tests.rs | 35 ++++++++++++++++++++++++ crates/format/Cargo.toml | 1 - crates/format/src/sketch/frames_tests.rs | 15 ++++++++++ crates/format/src/sketch/tests.rs | 27 ++++++++++++++---- crates/format/src/sketch/tree.rs | 13 +++++---- 7 files changed, 83 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d118fda..32e7be59 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1587,7 +1587,6 @@ name = "figcraft-format" version = "0.9.0" dependencies = [ "figcraft-doc", - "figcraft-layout", "figcraft-pathops", "flate2", "image", diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 8db15cee..8f7da824 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -720,6 +720,9 @@ fn sketch_line_heights(doc: &mut Document) { }; let ids: Vec = doc.nodes().filter(|n| matches!(n.kind, NodeKind::Text(_))).map(|n| n.id).collect(); for id in ids { + // An auto layout places its texts itself (its next pass would take a move back). + let laid_out = + doc.node(id).is_some_and(|n| !n.child_layout.absolute && n.parent.and_then(|p| doc.node(p)).is_some_and(|p| p.layout.is_some())); let Some(n) = doc.node_mut(id) else { continue }; let NodeKind::Text(td) = &mut n.kind else { continue }; let down = set(&mut td.style); @@ -727,7 +730,7 @@ fn sketch_line_heights(doc: &mut Document) { set(&mut r.style); } // Down in the layer's own frame (a rotated text moves along its own axis). - if down.is_finite() && down != 0.0 { + if !laid_out && down.is_finite() && down != 0.0 { n.transform *= figcraft_doc::geom::Affine::translate((0.0, down)); } } diff --git a/crates/engine/src/sketch_import_tests.rs b/crates/engine/src/sketch_import_tests.rs index 36b200a5..292cd019 100644 --- a/crates/engine/src/sketch_import_tests.rs +++ b/crates/engine/src/sketch_import_tests.rs @@ -441,3 +441,38 @@ fn sketch_text_gets_sketchs_natural_line() { let figcraft_doc::LineHeight::Percent(p) = td.style.line_height else { panic!("{:?}", td.style.line_height) }; assert!((p - 116.0).abs() < 1e-9, "{p}"); } + +/// A text an auto layout places isn't moved to Sketch's baseline (the next layout pass would take +/// the move back): laying the stack out again leaves it where the import put it. +#[test] +fn sketch_text_in_a_stack_stays_put() { + if !std::path::Path::new("/System/Library/Fonts/Helvetica.ttc").exists() { + return; + } + figcraft_text::fontdb::scan_system_fonts(); + let font = json!({"name": "Helvetica", "size": 40}); + let run = json!({"location": 0, "length": 2, "attributes": {"MSAttributedStringFontAttribute": {"attributes": font}}}); + let label = shape( + "text", + "Label", + [10.0, 10.0, 50.0, 48.0], + json!({"attributedString": {"string": "Hi", "attributes": [run]}, "horizontalSizing": 1, "verticalSizing": 1}), + ); + let layout = json!({"_class": "MSImmutableFlexGroupLayout", "flexDirection": 1, "justifyContent": 0, "alignItems": 0, "allGuttersGap": 0, "stackingOrder": 0}); + let stack = shape( + "group", + "Stack", + [0.0, 0.0, 200.0, 68.0], + json!({"groupBehavior": 1, "groupLayout": layout, "leftPadding": 10, "topPadding": 10, "rightPadding": 10, "bottomPadding": 10, "layers": [label]}), + ); + let data = base64::engine::general_purpose::STANDARD.encode(sketch_with("2026.3.1", &[("A", "Page")], &[stack])); + let mut s = Session::new(); + s.execute("file.open", &json!({"data": data, "name": "x.sketch"})).unwrap(); + let mut doc = s.doc().unwrap().clone(); + let stack = doc.nodes().find(|n| n.name == "Stack").unwrap().id; + let label = doc.nodes().find(|n| n.name == "Label").unwrap().id; + let before = doc.world_bounds(label).unwrap(); + figcraft_layout::layout_frame(&mut doc, stack, &crate::measure::TextMeasure); + let after = doc.world_bounds(label).unwrap(); + assert!((before.y0 - after.y0).abs() < 1e-6, "{before:?} → {after:?}"); +} diff --git a/crates/format/Cargo.toml b/crates/format/Cargo.toml index 80a943e2..3dca3912 100644 --- a/crates/format/Cargo.toml +++ b/crates/format/Cargo.toml @@ -11,7 +11,6 @@ workspace = true [dependencies] figcraft-doc = { workspace = true } figcraft-pathops = { workspace = true } -figcraft-layout = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } zip = { workspace = true } diff --git a/crates/format/src/sketch/frames_tests.rs b/crates/format/src/sketch/frames_tests.rs index 429a884b..5a5cefca 100644 --- a/crates/format/src/sketch/frames_tests.rs +++ b/crates/format/src/sketch/frames_tests.rs @@ -426,3 +426,18 @@ fn nested_tints_are_budgeted() { let (_, warnings) = open(vec![l]); assert!(warnings.iter().any(|w| w.contains("tint")), "{warnings:?}"); } + +/// A faded layer in a stack keeps its place in the stack: the group the fade makes takes the +/// layer's id, and with it its sizing. +#[test] +fn faded_stack_items_keep_their_sizing() { + let fade = json!({"_class": "graphicsContextSettings", "opacity": 1, "isProgressive": true, + "gradient": {"_class": "gradient", "gradientType": 0, "from": "{0.5, 0}", "to": "{0.5, 1}", "stops": [ + {"_class": "gradientStop", "position": 0, "color": color(0.0, 0.0, 0.0, 1.0)}, {"_class": "gradientStop", "position": 1, "color": color(0.0, 0.0, 0.0, 0.0)}]}}); + let mut item = styled(rect("Item", 0.0, 0.0, 50.0, 20.0), "contextSettings", fade); + item["horizontalSizing"] = json!(2); + let (doc, _) = open(vec![stack(frame_group("Row", 0.0, 0.0, 200.0, 20.0, vec![item]), 0, 0, 0, 0.0, [0.0; 4])]); + let row = node(&doc, "Row"); + let wrap = doc.node(row.children[0]).unwrap(); + assert_eq!(format!("{:?}", wrap.child_layout.sizing_h), "Fill", "{:?}", wrap.child_layout); +} diff --git a/crates/format/src/sketch/tests.rs b/crates/format/src/sketch/tests.rs index 8ed30231..b4368de1 100644 --- a/crates/format/src/sketch/tests.rs +++ b/crates/format/src/sketch/tests.rs @@ -101,21 +101,36 @@ fn junk_values_never_panic() { let mut icon = master("MI", "SI", "Icon", 24.0, 24.0, vec![dot.clone()]); icon["groupBehavior"] = json!(2); icon["style"]["fills"] = json!([tint]); - let mut chip = stack(master("MC", "SC", "Chip", 60.0, 24.0, vec![text_layer.clone(), instance("N", "SI", 0.0, 0.0, 24.0, 24.0, &[])]), 0, 1, 1, 6.0, [8.0, 4.0, 10.0, 4.0]); + let mut chip = stack( + master("MC", "SC", "Chip", 60.0, 24.0, vec![text_layer.clone(), instance("N", "SI", 0.0, 0.0, 24.0, 24.0, &[])]), + 0, + 1, + 1, + 6.0, + [8.0, 4.0, 10.0, 4.0], + ); chip["style"]["corners"] = json!({"_class": "MSImmutableStyleCorners", "radii": [12], "style": 2, "smoothing": 0.6}); - let over = [("T_stringValue", json!("Longer")), ("N_fillColor", color(0.0, 1.0, 0.0, 1.0)), ("T_textColor", color(0.0, 0.0, 1.0, 1.0)), ("N_isVisible", json!("0"))]; + let over = [ + ("T_stringValue", json!("Longer")), + ("N_fillColor", color(0.0, 1.0, 0.0, 1.0)), + ("T_textColor", color(0.0, 0.0, 1.0, 1.0)), + ("N_isVisible", json!("0")), + ]; let inst = instance("I", "SC", 0.0, 100.0, 200.0, 24.0, &over); let grad = json!({"_class": "fill", "isEnabled": true, "fillType": 1, "color": color(0.0, 0.0, 0.0, 1.0), "gradient": {"_class": "gradient", "gradientType": 0, "from": "{0, 0}", "to": "{1, 1}", "stops": [ {"_class": "gradientStop", "position": 0, "color": color(0.0, 0.0, 0.0, 1.0)}, {"_class": "gradientStop", "position": 1, "color": color(1.0, 1.0, 1.0, 0.0)}]}}); let mut faded = styled(rect("Faded", 0.0, 0.0, 30.0, 30.0), "fills", json!([grad.clone()])); - faded["style"]["contextSettings"] = json!({"_class": "graphicsContextSettings", "opacity": 1, "isProgressive": true, "gradient": grad["gradient"].clone()}); - let mask = with(with(styled(rect("Mask", 0.0, 0.0, 30.0, 30.0), "fills", json!([grad])), "hasClippingMask", json!(true)), "clippingMaskMode", json!(1)); + faded["style"]["contextSettings"] = + json!({"_class": "graphicsContextSettings", "opacity": 1, "isProgressive": true, "gradient": grad["gradient"].clone()}); + let mask = + with(with(styled(rect("Mask", 0.0, 0.0, 30.0, 30.0), "fills", json!([grad])), "hasClippingMask", json!(true)), "clippingMaskMode", json!(1)); let path = shape_path("P", 0.0, 0.0, 20.0, 20.0, &[(0.0, 0.0), (1.0, 0.0), (0.5, 1.0)], true); let mut g = group("G", 0.0, 0.0, 100.0, 100.0, vec![mask, faded, path, dot, text_layer]); g["style"] = json!({"_class": "style", "fills": [tint_fill()]}); let mut frame = stack(frame_group("F", 0.0, 0.0, 300.0, 300.0, vec![g, inst]), 1, 0, 3, 8.0, [0.0; 4]); - frame["layout"] = json!({"_class": "layoutGrid", "isEnabled": true, "columnWidth": 20, "gutterWidth": 10, "numberOfColumns": 4, "drawVertical": true}); + frame["layout"] = + json!({"_class": "layoutGrid", "isEnabled": true, "columnWidth": 20, "gutterWidth": 10, "numberOfColumns": 4, "drawVertical": true}); let pages = [page("P", "Page", vec![frame]), page("S", "Symbols", vec![icon, chip])]; // Every value's path, with each junk value in turn (a deterministic sample keeps it quick). fn paths(v: &serde_json::Value, at: &mut Vec, out: &mut Vec>) { @@ -149,7 +164,7 @@ fn junk_values_never_panic() { v = match v { serde_json::Value::Object(m) => m.get_mut(k).unwrap(), serde_json::Value::Array(a) => &mut a[k.parse::().unwrap()], - _ => unreachable!(), + _ => panic!("a path only runs through objects and arrays"), }; } *v = junk[n % junk.len()].clone(); diff --git a/crates/format/src/sketch/tree.rs b/crates/format/src/sketch/tree.rs index cccf3934..8bad4c6c 100644 --- a/crates/format/src/sketch/tree.rs +++ b/crates/format/src/sketch/tree.rs @@ -294,7 +294,8 @@ pub(crate) fn layer(cx: &mut Cx, l: &Value, parent: Affine, depth: usize, root: fn fade(cx: &mut Cx, l: &Value, node: JsonMap, world: Affine, w: f64, h: f64) -> Value { let ctx = l.get("style").and_then(|s| s.get("contextSettings")); let gradient = ctx.filter(|c| c.get("isProgressive").and_then(Value::as_bool) == Some(true)).and_then(|c| c.get("gradient")); - let Some(gradient) = gradient else { return Value::Object(node) }; + // A mask stays as it is: its group would stop masking the layers above it. + let Some(gradient) = gradient.filter(|_| node.get("isMask").and_then(Value::as_bool) != Some(true)) else { return Value::Object(node) }; let fill = json!({"_class": "fill", "isEnabled": true, "fillType": 1, "gradient": gradient}); let fills = super::style::fills(cx, &json!({"fills": [fill]}), w, h); if fills.is_empty() { @@ -304,18 +305,20 @@ fn fade(cx: &mut Cx, l: &Value, node: JsonMap, world: Affine, w: let identity = json!([[1.0, 0.0, 0.0], [0.0, 1.0, 0.0]]); let bb = world.transform_rect_bbox(Rect::new(0.0, 0.0, w, h)); let abs = json!({"x": bb.x0, "y": bb.y0, "width": bb.width(), "height": bb.height()}); - let mut inner = node.clone(); + // The group takes the layer's id (stacks, links and overrides find it), the layer a new one. + let mut inner = node; let mut group = JsonMap::new(); for k in ["relativeTransform", "absoluteBoundingBox", "constraints", "visible", "locked"] { - if let Some(v) = node.get(k) { + if let Some(v) = inner.get(k) { group.insert(k.into(), v.clone()); } } group.insert("type".into(), json!("GROUP")); - group.insert("id".into(), json!(format!("{id}-fade"))); - group.insert("name".into(), node.get("name").cloned().unwrap_or_else(|| json!("Fade"))); + group.insert("id".into(), json!(id)); + group.insert("name".into(), inner.get("name").cloned().unwrap_or_else(|| json!("Fade"))); group.insert("size".into(), json!({"x": w, "y": h})); group.insert("absoluteBoundingBox".into(), abs.clone()); + inner.insert("id".into(), json!(format!("{id}-faded"))); inner.insert("relativeTransform".into(), identity.clone()); inner.insert("absoluteBoundingBox".into(), abs.clone()); inner.insert("visible".into(), json!(true));