From faa96926f4bf396795b79c375ae9271f461c436d Mon Sep 17 00:00:00 2001 From: Skyfuller1303 Date: Wed, 28 Oct 2026 11:59:53 +0100 Subject: [PATCH] M4: keep stroked whole-shape merges and bound region paints per file Merging every region of one stroked shape is a no-op again: only fill parts count, so a vector with several painted regions still merges. The region paint import budget now covers the whole file instead of resetting for each layer, so instances of a main with large region paint stacks cannot multiply the work. --- crates/engine/src/canvas/shapebuilder.rs | 5 ++- .../shapebuilder_region_paints_tests.rs | 20 ++++++++++ .../engine/src/shapebuilder_strokes_tests.rs | 14 +++++++ crates/format/src/fig/map.rs | 4 ++ crates/format/src/fig/map/layer.rs | 7 ++-- crates/format/src/fig/tail_tests.rs | 37 +++++++++++++++++++ 6 files changed, 81 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/canvas/shapebuilder.rs b/crates/engine/src/canvas/shapebuilder.rs index ea02ccc8..19e2ec59 100644 --- a/crates/engine/src/canvas/shapebuilder.rs +++ b/crates/engine/src/canvas/shapebuilder.rs @@ -415,8 +415,9 @@ fn apply(s: &mut Session, cmd: &str, p: &Value, merge: bool) -> Result { // Merging every region of a single shape would only replace it with a copy of itself. let whole_shape = touched.len() == 1 && a.regions.iter().enumerate().all(|(i, r)| pick.contains(&i) || !r.sources.iter().any(|x| touched.contains(x))); - let single_fill_part = touched_parts.len() == 1 && touched_parts.iter().all(|part| !part.stroke); - if merge && whole_shape && single_fill_part { + // A vector's distinct region paints are separate fill parts that merging does unify. + let fill_parts = touched_parts.iter().filter(|part| !part.stroke).count(); + if merge && whole_shape && fill_parts <= 1 { return Ok(json!({"id": null, "changed": [], "removed": [], "outlines": [], "regions": pick})); } let doc = s.doc_mut()?; diff --git a/crates/engine/src/canvas/shapebuilder_region_paints_tests.rs b/crates/engine/src/canvas/shapebuilder_region_paints_tests.rs index d1b747c3..74e45093 100644 --- a/crates/engine/src/canvas/shapebuilder_region_paints_tests.rs +++ b/crates/engine/src/canvas/shapebuilder_region_paints_tests.rs @@ -285,3 +285,23 @@ fn removal_preserves_holes_and_transformed_gradient_frames() { assert!(net.region_contains(0, Point::new(90.0, 50.0)), "the unselected ring remains filled"); assert_same_transform(before, normalized_paint_world(&s, vector, 0)); } + +#[test] +fn removing_the_left_region_keeps_the_right_region_paint_frame() { + let mut s = Session::new(); + let vector = split_vector(&mut s); + { + let NodeKind::Vector { network } = &mut s.doc_mut().unwrap().get_mut(vector).unwrap().kind else { panic!("expected vector") }; + network.regions[1].fills = Some(vec![Paint::gradient(Gradient::default_for(GradientKind::Linear, Color::from_hex("#0000ff").unwrap()))]); + } + let before = normalized_paint_world(&s, vector, 1); + s.set_selection(vec![vector]); + ex(&mut s, "shapeBuilder.remove", json!({"points": [[25, 50]]})); + let node = s.doc().unwrap().node(vector).unwrap(); + assert!((node.width - 50.0).abs() < 1e-6, "the layer is reboxed to the surviving region: {}", node.width); + let net = network(&s, vector); + assert_eq!(net.regions.len(), 1); + assert!(matches!(&net.regions[0].fills.as_ref().unwrap()[0].kind, figcraft_color::PaintKind::Gradient(_))); + assert!((world_region_area(&s, vector, 0) - 5000.0).abs() < 1e-6); + assert_same_transform(before, normalized_paint_world(&s, vector, 0)); +} diff --git a/crates/engine/src/shapebuilder_strokes_tests.rs b/crates/engine/src/shapebuilder_strokes_tests.rs index 19aa2c9c..8bebfb0f 100644 --- a/crates/engine/src/shapebuilder_strokes_tests.rs +++ b/crates/engine/src/shapebuilder_strokes_tests.rs @@ -166,6 +166,20 @@ fn a_centred_stroke_on_a_sharp_star_makes_three_regions() { assert_eq!(covers, vec![(false, true), (true, false), (true, true)], "fill only, stroke only, both"); } +#[test] +fn merging_every_region_of_one_stroked_shape_is_a_no_op() { + let mut s = Session::new(); + let r = id(&ex(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 100, "height": 100, "props": {"fill": "#ff0000"}}))); + ex(&mut s, "node.set", json!({"id": r, "props": {"strokes": ["#000000"], "strokeWeight": 10, "strokeAlign": "center"}})); + s.set_selection(vec![r]); + let count = ex(&mut s, "shapeBuilder.regions", json!({}))["regions"].as_array().unwrap().len(); + assert!(count > 1, "fill and stroke make several regions"); + let before = s.doc().unwrap().clone(); + let out = ex(&mut s, "shapeBuilder.merge", json!({"regions": (0..count).collect::>()})); + assert!(out["id"].is_null(), "{out}"); + assert_eq!(*s.doc().unwrap(), before); +} + #[test] fn stroke_regions_survive_junk() { for j in [ diff --git a/crates/format/src/fig/map.rs b/crates/format/src/fig/map.rs index 06cfdb8b..546f6eb3 100644 --- a/crates/format/src/fig/map.rs +++ b/crates/format/src/fig/map.rs @@ -248,6 +248,9 @@ struct Mapper<'a> { networks: Vec>, /// Index in `networks` by what a network is made from: its blob, its scale and its style table. network_ids: HashMap<(usize, u64, u64, usize), usize>, + /// Region paint elements (paints and gradient stops) read so far: one budget for the whole + /// file, since layers with region paints don't share their network. + region_paint_elements: usize, } impl<'a> Mapper<'a> { @@ -322,6 +325,7 @@ impl<'a> Mapper<'a> { reserved: 0, networks: Vec::new(), network_ids: HashMap::new(), + region_paint_elements: 0, } } diff --git a/crates/format/src/fig/map/layer.rs b/crates/format/src/fig/map/layer.rs index 555907aa..6178a238 100644 --- a/crates/format/src/fig/map/layer.rs +++ b/crates/format/src/fig/map/layer.rs @@ -526,7 +526,6 @@ impl<'a> Mapper<'a> { .collect(); let segments: Vec = net.segments.iter().map(|s| json!({"a": s.a, "b": s.b, "ta": [s.ta.0 * sx, s.ta.1 * sy], "tb": [s.tb.0 * sx, s.tb.1 * sy]})).collect(); - let mut region_paint_elements = 0usize; let regions: Vec = net .regions .iter() @@ -554,12 +553,12 @@ impl<'a> Mapper<'a> { }); let oversized_stops = source_paints.iter().any(|paint| paint.get("stops").map(Kv::list).unwrap_or_default().len() > 256); - if source_paints.len() > 256 || oversized_stops || region_paint_elements.saturating_add(paint_cost) > 200_000 { + if source_paints.len() > 256 || oversized_stops || self.region_paint_elements.saturating_add(paint_cost) > 200_000 + { self.warn("vector region paint data exceeded import limits and was skipped"); - region_paint_elements = 200_000; return region; } - region_paint_elements += paint_cost; + self.region_paint_elements += paint_cost; let paints: Vec = source_paints.iter().filter_map(|p| paint(p, &mut |w| self.warn(w))).collect(); if paints.len() != source_paints.len() { self.warn("invalid vector region paints were skipped"); diff --git a/crates/format/src/fig/tail_tests.rs b/crates/format/src/fig/tail_tests.rs index 19b0b871..0e6d32bc 100644 --- a/crates/format/src/fig/tail_tests.rs +++ b/crates/format/src/fig/tail_tests.rs @@ -407,6 +407,43 @@ fn vector_region_link_to_non_fill_style_keeps_own_fill() { assert_eq!((color.r, color.g, color.b), (0.1, 0.2, 0.9)); } +#[test] +fn vector_region_paint_budget_covers_the_whole_file() { + // 190 gradients of 255 stops: 48,640 elements per layer, so four layers fit and a fifth doesn't. + let stop = o(vec![("position", f(0.0)), ("color", o(vec![("r", f(1.0)), ("g", f(0.0)), ("b", f(0.0)), ("a", f(1.0))]))]); + let gradient = o(vec![("type", e("GRADIENT_LINEAR")), ("stops", arr(vec![stop; 255]))]); + let mut message = tail_message(); + let Kv::Array(nodes) = field_mut(&mut message, "nodeChanges").unwrap() else { panic!() }; + let wave = nodes.iter_mut().find(|node| node.get("name").and_then(Kv::str) == Some("Wave")).unwrap(); + let vector_data = field_mut(wave, "vectorData").unwrap(); + let Kv::Array(styles) = field_mut(vector_data, "styleOverrideTable").unwrap() else { panic!() }; + let Kv::Object(style_fields) = styles.first_mut().unwrap() else { panic!() }; + style_fields.push(("fillPaints", arr(vec![gradient; 190]))); + let wave = wave.clone(); + for (index, position) in ["w", "x", "y", "z"].into_iter().enumerate() { + let mut copy = wave.clone(); + let Kv::Object(fields) = &mut copy else { panic!() }; + for (key, value) in fields.iter_mut() { + match *key { + "guid" => *value = g(60 + index as u64), + "name" => *value = Kv::Str(format!("Wave {index}")), + "parentIndex" => *value = o(vec![("guid", g(1)), ("position", s(position))]), + _ => {} + } + } + nodes.push(copy); + } + let mut network = network::decode(&square_blob()).unwrap(); + network.regions[0].style = 1; + let Kv::Array(blobs) = field_mut(&mut message, "blobs").unwrap() else { panic!() }; + *field_mut(blobs.get_mut(1).unwrap(), "bytes").unwrap() = Kv::Bytes(network::encode(&network)); + + let (doc, warnings) = import(&canvas(&schema(), &message), "Region paint budget").unwrap(); + let painted = ["Wave", "Wave 0", "Wave 1", "Wave 2", "Wave 3"].iter().filter(|name| network_of(&doc, name).regions[0].fills.is_some()).count(); + assert_eq!(painted, 4); + assert!(warnings.iter().any(|warning| warning.contains("region paint data exceeded import limits")), "{warnings:?}"); +} + #[test] fn vector_regions_keep_distinct_solid_paints() { let mut message = tail_message();