From 57e455dfa78944e2aae9821366cd9fc507293c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E6=9E=97=E6=A9=A6?= <108113065+yelintong@users.noreply.github.com> Date: Sat, 17 Oct 2026 19:14:54 +0800 Subject: [PATCH 1/2] engine: fill rules as vector.fillRule and vector.reverseLoop Two vector commands behind the fill-rule panel, split out so they can land on their own (the panel is held for a maintainer design decision): - vector.fillRule {ids? | id?, region?, rule: nonZero|evenOdd} sets the winding rule a filled area fills by (the whole layer when region is left out). - vector.reverseLoop {ids? | id?, region, loop} turns one loop round, which is how a hole is made and unmade. reverseLoop no longer swaps the stored ends of a segment a loop shares with a neighbour: reversing the loop's list already flips this loop's traversal of the shared segment, so flipping its segment would turn the neighbour's area round too (bug called out in review). A new test builds two areas that share a segment and checks the shared segment and the neighbour's fill are left alone. Adds the commands to the Vector menu (catalog/parity), their locale labels, and vector_fill_rule_tests (rule per area or whole layer, junk refused, shared segment). --- crates/engine/src/catalog.rs | 2 + crates/engine/src/cmd/structure.rs | 132 ++++++++++++ crates/engine/src/lib.rs | 2 + crates/engine/src/vector_fill_rule_tests.rs | 228 ++++++++++++++++++++ crates/ui-egui/locales/cs.toml | 2 + crates/ui-egui/locales/de.toml | 2 + crates/ui-egui/locales/es.toml | 2 + crates/ui-egui/locales/fr.toml | 2 + crates/ui-egui/locales/id.toml | 2 + crates/ui-egui/locales/ja.toml | 2 + crates/ui-egui/locales/ko.toml | 2 + crates/ui-egui/locales/pt-BR.toml | 2 + crates/ui-egui/locales/ru.toml | 2 + crates/ui-egui/locales/sv.toml | 2 + crates/ui-egui/locales/tr.toml | 2 + crates/ui-egui/locales/zh-Hans.toml | 2 + 16 files changed, 388 insertions(+) create mode 100644 crates/engine/src/vector_fill_rule_tests.rs diff --git a/crates/engine/src/catalog.rs b/crates/engine/src/catalog.rs index c8f34fc1..6bee5048 100644 --- a/crates/engine/src/catalog.rs +++ b/crates/engine/src/catalog.rs @@ -235,6 +235,8 @@ pub static MENU: &[MenuItem] = &[ m!("Vector", "Delete and heal selection", "vector.deleteHeal"), m!("Vector", "Simplify vector", "vector.simplify"), m!("Vector", "Reverse path direction", "vector.reverse"), + m!("Vector", "Set fill rule", "vector.fillRule"), + m!("Vector", "Reverse loop", "vector.reverseLoop"), m!("Vector", "Offset path", "vector.offset"), // Plugins m!("Plugins", "Run last plugin", "plugins.runLast"), diff --git a/crates/engine/src/cmd/structure.rs b/crates/engine/src/cmd/structure.rs index 500fa021..c38052ec 100644 --- a/crates/engine/src/cmd/structure.rs +++ b/crates/engine/src/cmd/structure.rs @@ -103,6 +103,20 @@ pub fn specs() -> Vec { } ok() }), + spec!( + "vector.fillRule", + "Set fill rule", + "{ids? | id?, region? (a filled area of a vector; every area when left out), rule: nonZero|evenOdd}", + true, + fill_rule + ), + spec!( + "vector.reverseLoop", + "Reverse loop", + "{ids? | id?, region, loop} (a loop of a filled area; its direction flips, which changes how the area fills)", + true, + reverse_loop + ), spec!("vector.simplify", "Simplify vector", "{ids?, tolerance?}", true, simplify), spec!("vector.offset", "Offset path", "{ids?, offset}", true, offset_path), spec!("vector.join", "Join selection", "{ids?} (in vector edit mode: joins the selected points)", true, |s, p| { @@ -719,3 +733,121 @@ fn resize_to_fit(s: &mut Session, p: &Value) -> Result { } Ok(json!({"ids": fitted})) } + +/// The vector network of `id`, when it is a vector layer. +fn network_mut(doc: &mut Document, id: NodeId) -> Option<&mut VectorNetwork> { + match &mut doc.get_mut(id).ok()?.kind { + NodeKind::Vector { network } => Some(network), + _ => None, + } +} + +/// The id a fill rule is written as. +fn rule_id(rule: WindingRule) -> &'static str { + match rule { + WindingRule::NonZero => "nonZero", + WindingRule::EvenOdd => "evenOdd", + } +} + +/// `region` as an index: absent (or null) means every area. +fn area_index(cmd: &str, p: &Value) -> Result> { + match get(p, "region") { + None | Some(Value::Null) => Ok(None), + Some(v) => { + let k = f64_of(v).ok_or_else(|| bad(cmd, "`region` must be a number"))?; + if !(0.0..1e9).contains(&k) || k.fract() != 0.0 { + return Err(bad(cmd, "`region` out of range")); + } + Ok(Some(k as usize)) + } + } +} + +/// Segments listed by more than one loop (of any region): reversing a loop must leave them as they +/// are, or a neighbouring area that shares the segment would turn round with it — flipping the +/// neighbour's fill or stroke. Reversing the loop's list already flips this loop's traversal of the +/// shared segment, so its stored direction is left alone. +fn shared_segments(net: &VectorNetwork) -> std::collections::HashSet { + let mut count = std::collections::HashMap::::new(); + for region in &net.regions { + for lp in ®ion.loops { + for &s in lp { + *count.entry(s).or_insert(0) += 1; + } + } + } + count.into_iter().filter(|(_, c)| *c > 1).map(|(s, _)| s).collect() +} + +fn fill_rule(s: &mut Session, p: &Value) -> Result { + let ids = req_targets("vector.fillRule", s, p)?; + let rule = match string(p, "rule").as_deref().map(str::trim) { + Some("evenOdd" | "evenodd" | "EVENODD") => WindingRule::EvenOdd, + Some("nonZero" | "nonzero" | "NONZERO") => WindingRule::NonZero, + _ => return Err(bad("vector.fillRule", "`rule` is nonZero or evenOdd")), + }; + let only = area_index("vector.fillRule", p)?; + let doc = s.doc_mut()?; + let mut count = 0usize; + for id in ids { + let Some(net) = network_mut(doc, id) else { continue }; + match only { + Some(r) => { + if let Some(region) = net.regions.get_mut(r) { + region.winding = rule; + count += 1; + } + } + None => { + for region in &mut net.regions { + region.winding = rule; + count += 1; + } + } + } + } + if count == 0 { + return Err(EngineError::Disabled("vector.fillRule".into(), "no filled area to set (select a vector)".into())); + } + Ok(json!({"regions": count, "rule": rule_id(rule)})) +} + +/// Turn one loop of a filled area round: its segments are walked the other way, so the area it +/// bounds counts the other way in the winding (which turns a hole into a filled part and back). +fn reverse_loop(s: &mut Session, p: &Value) -> Result { + let ids = req_targets("vector.reverseLoop", s, p)?; + let region = req_index("vector.reverseLoop", p, "region")?; + let index = req_index("vector.reverseLoop", p, "loop")?; + let doc = s.doc_mut()?; + let mut reversed = false; + for id in ids { + let Some(net) = network_mut(doc, id) else { continue }; + let Some(loop_) = net.regions.get(region).and_then(|r| r.loops.get(index)).cloned() else { continue }; + // A loop is walked in the order its segments are listed, so the list turned round walks the + // ring the other way. A loop of one or two segments only turns round when its segments are + // stored the other way too (list reversal alone is a no-op for them). + // A segment shared with another loop must NOT be swapped: it belongs to a neighbouring area + // whose winding we leave alone, and reversing the list already flips this loop's traversal + // of it. + let shared = shared_segments(net); + let turned: Vec = loop_.iter().rev().copied().collect(); + for &seg in &turned { + if shared.contains(&seg) { + continue; + } + if let Some(seg) = net.segments.get_mut(seg) { + std::mem::swap(&mut seg.a, &mut seg.b); + std::mem::swap(&mut seg.ta, &mut seg.tb); + } + } + if let Some(l) = net.regions.get_mut(region).and_then(|r| r.loops.get_mut(index)) { + *l = turned; + reversed = true; + } + } + if !reversed { + return Err(EngineError::Disabled("vector.reverseLoop".into(), "no such loop (select a vector with filled areas)".into())); + } + Ok(json!({"reversed": true})) +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 8796dc9c..5e29ae44 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1790,6 +1790,8 @@ mod variables_io_tests; #[cfg(test)] mod vector_behaviour_tests; #[cfg(test)] +mod vector_fill_rule_tests; +#[cfg(test)] mod vector_multi_behaviour_tests; #[cfg(test)] mod vector_offset_tests; diff --git a/crates/engine/src/vector_fill_rule_tests.rs b/crates/engine/src/vector_fill_rule_tests.rs new file mode 100644 index 00000000..db96b669 --- /dev/null +++ b/crates/engine/src/vector_fill_rule_tests.rs @@ -0,0 +1,228 @@ +//! Fill rules on vectors: `vector.fillRule` sets the rule a filled area fills by and +//! `vector.reverseLoop` turns one of its loops round — which is how a hole is made and unmade. +//! The numbers come from the two rules' definitions: non-zero fills where the winding count is not +//! zero (so a hole is a loop running against its outer loop), even-odd where it is odd (so the +//! direction of a loop doesn't matter). + +use figcraft_doc::{NodeId, NodeKind, Region, Segment, VectorNetwork, Vertex, WindingRule}; +use figcraft_geom::{BezPath, Point}; +use serde_json::{Value, json}; + +use crate::Session; + +fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")) +} + +fn net(s: &Session, id: NodeId) -> VectorNetwork { + match &s.doc().unwrap().node(id).unwrap().kind { + NodeKind::Vector { network } => network.clone(), + other => panic!("not a vector: {other:?}"), + } +} + +/// A 100 × 100 square with a 25…75 hole, both loops in one area: the outer one clockwise, the inner +/// one the same way round (so non-zero fills the hole too until it is turned round). +fn ring() -> VectorNetwork { + let mut path = BezPath::new(); + for (x0, y0, x1, y1) in [(0.0, 0.0, 100.0, 100.0), (25.0, 25.0, 75.0, 75.0)] { + path.move_to((x0, y0)); + path.line_to((x1, y0)); + path.line_to((x1, y1)); + path.line_to((x0, y1)); + path.close_path(); + } + let mut net = VectorNetwork::from_path(&path); + net.regions[0].winding = WindingRule::NonZero; + net +} + +/// A selected vector layer holding `ring()`. +fn ringed(s: &mut Session) -> NodeId { + let id = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap(); + let id = NodeId(id); + ex(s, "vector.edit.begin", json!({"id": id})); + ex(s, "vector.edit.end", json!({})); + { + let doc = s.doc_mut().unwrap(); + match &mut doc.get_mut(id).unwrap().kind { + NodeKind::Vector { network } => *network = ring(), + other => panic!("not a vector: {other:?}"), + } + } + ex(s, "select.set", json!({"ids": [id]})); + id +} + +/// Two 50 × 100 squares side by side share the vertical edge between them: one segment listed by +/// both areas' single loops. A segment shared between areas is the case `vector.reverseLoop` must +/// not flip, or a neighbour's fill or stroke would turn round with it. +fn two_squares() -> VectorNetwork { + // 0:A(0,0) 1:B(50,0) 2:C(100,0) 3:D(100,100) 4:E(50,100) 5:F(0,100) + // s0 A→B (left bottom), s1 B→E (the shared vertical edge), s2 E→F (left top), + // s3 F→A (left left), s4 B→C (right bottom), s5 C→D (right right), s6 D→E (right top); + // the right area's left edge is s1 walked the other way, so the squares share one segment. + // region 0: left square A→B→E→F→A = [s0, s1, s2, s3]. + // region 1: right square B→C→D→E→B = [s4, s5, s6, s1] (s1 walked backwards). + VectorNetwork { + vertices: vec![ + Vertex::new(0.0, 0.0), + Vertex::new(50.0, 0.0), + Vertex::new(100.0, 0.0), + Vertex::new(100.0, 100.0), + Vertex::new(50.0, 100.0), + Vertex::new(0.0, 100.0), + ], + segments: vec![ + Segment::line(0, 1), + Segment::line(1, 4), + Segment::line(4, 5), + Segment::line(5, 0), + Segment::line(1, 2), + Segment::line(2, 3), + Segment::line(3, 4), + ], + regions: vec![ + Region { loops: vec![vec![0, 1, 2, 3]], winding: WindingRule::NonZero }, + Region { loops: vec![vec![4, 5, 6, 1]], winding: WindingRule::NonZero }, + ], + } +} + +/// A selected vector layer holding [`two_squares`]. +fn squared(s: &mut Session) -> NodeId { + let id = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap(); + let id = NodeId(id); + ex(s, "vector.edit.begin", json!({"id": id})); + ex(s, "vector.edit.end", json!({})); + { + let doc = s.doc_mut().unwrap(); + match &mut doc.get_mut(id).unwrap().kind { + NodeKind::Vector { network } => *network = two_squares(), + other => panic!("not a vector: {other:?}"), + } + } + ex(s, "select.set", json!({"ids": [id]})); + id +} + +/// The point in the middle of the hole, and one on the square's left band. +const HOLE: Point = Point::new(50.0, 50.0); +const BAND: Point = Point::new(10.0, 50.0); + +#[test] +fn a_loop_running_with_its_outer_loop_fills_the_hole_until_it_is_turned_round() { + let mut s = Session::new(); + let id = ringed(&mut s); + let n = net(&s, id); + assert_eq!(n.regions.len(), 1); + assert_eq!(n.regions[0].loops.len(), 2, "an outer loop and the hole's"); + assert!(n.region_contains(0, BAND), "the band fills"); + assert!(n.region_contains(0, HOLE), "the hole fills while its loop runs the same way"); + // Turning the inner loop round makes it a hole under non-zero. + let r = ex(&mut s, "vector.reverseLoop", json!({"id": id, "region": 0, "loop": 1})); + assert_eq!(r["reversed"], json!(true)); + let n = net(&s, id); + assert!(n.region_contains(0, BAND), "the band still fills"); + assert!(!n.region_contains(0, HOLE), "the hole is empty"); + assert_eq!(n.region_winding(0, HOLE), Some(0), "its loops cancel out there"); + // Undo turns it back. + s.undo().unwrap(); + assert!(net(&s, id).region_contains(0, HOLE), "one undo step restores the loop"); +} + +#[test] +fn the_rule_switches_every_area_or_only_the_one_asked_for() { + let mut s = Session::new(); + let id = ringed(&mut s); + // Even-odd leaves the hole empty whatever way its loop runs. + let r = ex(&mut s, "vector.fillRule", json!({"id": id, "rule": "evenOdd"})); + assert_eq!((r["regions"].clone(), r["rule"].clone()), (json!(1), json!("evenOdd"))); + let n = net(&s, id); + assert_eq!(n.regions[0].winding, WindingRule::EvenOdd); + assert!(!n.region_contains(0, HOLE), "even-odd counts the hole's loop as a second crossing"); + // And back. + ex(&mut s, "vector.fillRule", json!({"id": id, "rule": "nonZero"})); + assert!(net(&s, id).region_contains(0, HOLE), "non-zero fills it again"); + // One area of several, by index. + let n = net(&s, id); + let mut two = n.clone(); + two.regions.push(n.regions[0].clone()); + { + let doc = s.doc_mut().unwrap(); + match &mut doc.get_mut(id).unwrap().kind { + NodeKind::Vector { network } => *network = two, + other => panic!("not a vector: {other:?}"), + } + } + ex(&mut s, "vector.fillRule", json!({"id": id, "region": 1, "rule": "evenOdd"})); + let n = net(&s, id); + assert_eq!(n.regions[0].winding, WindingRule::NonZero, "the other area keeps its rule"); + assert_eq!(n.regions[1].winding, WindingRule::EvenOdd); + // Every area at once. + ex(&mut s, "vector.fillRule", json!({"id": id, "rule": "evenOdd"})); + assert!(net(&s, id).regions.iter().all(|r| r.winding == WindingRule::EvenOdd)); +} + +#[test] +fn junk_parameters_are_refused_and_change_nothing() { + let mut s = Session::new(); + let id = ringed(&mut s); + let before = net(&s, id); + for p in [ + json!({"id": id, "rule": "nope"}), + json!({"id": id}), + json!({"id": id, "rule": 7}), + json!({"id": id, "rule": "evenOdd", "region": -1}), + json!({"id": id, "rule": "evenOdd", "region": 1.5}), + json!({"id": id, "rule": "evenOdd", "region": "x"}), + json!({"id": id, "rule": "evenOdd", "region": 99}), + ] { + assert!(s.execute("vector.fillRule", &p).is_err(), "{p} is refused"); + } + for p in [ + json!({"id": id}), + json!({"id": id, "region": 0}), + json!({"id": id, "region": 0, "loop": 99}), + json!({"id": id, "region": 99, "loop": 0}), + json!({"id": id, "region": -2, "loop": 0}), + ] { + assert!(s.execute("vector.reverseLoop", &p).is_err(), "{p} is refused"); + } + assert_eq!(net(&s, id), before, "nothing changed"); + // A rectangle that was never a vector is refused too. + let rect = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()["id"].as_u64().unwrap(); + assert!(s.execute("vector.fillRule", &json!({"id": rect, "rule": "evenOdd"})).is_err()); + assert!(s.execute("vector.reverseLoop", &json!({"id": rect, "region": 0, "loop": 0})).is_err()); +} + +#[test] +fn reversing_a_loop_shared_with_a_neighbour_leaves_its_segment_and_fill_alone() { + let mut s = Session::new(); + let id = squared(&mut s); + let n = net(&s, id); + assert_eq!(n.regions.len(), 2, "two areas, side by side"); + // Both areas fill under non-zero: a point in each. + assert!(n.region_contains(0, Point::new(25.0, 50.0)), "the left area fills"); + assert!(n.region_contains(1, Point::new(75.0, 50.0)), "the right area fills"); + // The vertical edge between them is one segment listed by both areas' loops. + const SHARED: usize = 1; + assert!(n.regions[0].loops[0].contains(&SHARED) && n.regions[1].loops[0].contains(&SHARED), "one segment is shared"); + let before = n.segments[SHARED]; + let left_winding = n.region_winding(0, Point::new(25.0, 50.0)).unwrap(); + assert_ne!(left_winding, 0, "the left area winds once"); + // Reverse the left area's only loop. + let r = ex(&mut s, "vector.reverseLoop", json!({"id": id, "region": 0, "loop": 0})); + assert_eq!(r["reversed"], json!(true)); + let n = net(&s, id); + // The shared segment's stored ends are untouched: reversing the loop must not flip a neighbour's + // segment (reversing the list already turns this loop's traversal of it the other way). + assert_eq!(n.segments[SHARED], before, "the shared segment is left as it was"); + // The neighbour's fill is unchanged: it still fills its interior. + assert!(n.region_contains(1, Point::new(75.0, 50.0)), "the neighbour still fills"); + // The reversed loop really turned round: its winding at a left-area point flipped sign. + assert_eq!(n.region_winding(0, Point::new(25.0, 50.0)), Some(-left_winding), "the left loop reversed"); + // Undo restores the shared segment and the loop. + s.undo().unwrap(); + assert_eq!(net(&s, id).segments[SHARED], before, "undo restores the shared segment too"); +} diff --git a/crates/ui-egui/locales/cs.toml b/crates/ui-egui/locales/cs.toml index 2459d104..7d176d1b 100644 --- a/crates/ui-egui/locales/cs.toml +++ b/crates/ui-egui/locales/cs.toml @@ -2151,11 +2151,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Nástroj úprav vektoru" "vector.edit.transform" = "Transformovat body" "vector.edit.widthPoint" = "Bod šířky" +"vector.fillRule" = "Nastavit pravidlo výplně" "vector.flatten" = "Sloučit do vektoru" "vector.join" = "Spojit výběr" "vector.offset" = "Odsadit cestu" "vector.outlineStroke" = "Převést tah na obrysy" "vector.reverse" = "Obrátit směr cesty" +"vector.reverseLoop" = "Obrátit smyčku" "vector.simplify" = "Zjednodušit vektor" "vector.smoothJoin" = "Hladce spojit výběr" "version.autosave" = "Automaticky uložit verzi" diff --git a/crates/ui-egui/locales/de.toml b/crates/ui-egui/locales/de.toml index b679feb7..bd609450 100644 --- a/crates/ui-egui/locales/de.toml +++ b/crates/ui-egui/locales/de.toml @@ -2093,11 +2093,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Vektorbearbeitungswerkzeug" "vector.edit.transform" = "Punkte transformieren" "vector.edit.widthPoint" = "Breitenpunkt" +"vector.fillRule" = "Füllregel festlegen" "vector.flatten" = "Zusammenfügen" "vector.join" = "Auswahl verbinden" "vector.offset" = "Pfad versetzen" "vector.outlineStroke" = "Kontur in Pfad umwandeln" "vector.reverse" = "Pfadrichtung umkehren" +"vector.reverseLoop" = "Schleife umdrehen" "vector.simplify" = "Vektor vereinfachen" "vector.smoothJoin" = "Auswahl glatt verbinden" "version.autosave" = "Version automatisch speichern" diff --git a/crates/ui-egui/locales/es.toml b/crates/ui-egui/locales/es.toml index 2849f4ec..f05c5bdd 100644 --- a/crates/ui-egui/locales/es.toml +++ b/crates/ui-egui/locales/es.toml @@ -2183,11 +2183,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Herramienta de edición vectorial" "vector.edit.transform" = "Transformar puntos" "vector.edit.widthPoint" = "Punto de ancho" +"vector.fillRule" = "Establecer regla de relleno" "vector.flatten" = "Acoplar" "vector.join" = "Unir selección" "vector.offset" = "Desplazar trazado" "vector.outlineStroke" = "Contornear trazo" "vector.reverse" = "Invertir dirección del trazado" +"vector.reverseLoop" = "Invertir bucle" "vector.simplify" = "Simplificar vector" "vector.smoothJoin" = "Unir selección con suavizado" "version.autosave" = "Guardar versión automáticamente" diff --git a/crates/ui-egui/locales/fr.toml b/crates/ui-egui/locales/fr.toml index 1c561f15..621eb725 100644 --- a/crates/ui-egui/locales/fr.toml +++ b/crates/ui-egui/locales/fr.toml @@ -2084,11 +2084,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Outil de modification vectorielle" "vector.edit.transform" = "Transformer les points" "vector.edit.widthPoint" = "Point de largeur" +"vector.fillRule" = "Définir la règle de remplissage" "vector.flatten" = "Aplatir" "vector.join" = "Joindre la sélection" "vector.offset" = "Décaler le tracé" "vector.outlineStroke" = "Convertir le contour en tracé" "vector.reverse" = "Inverser le sens du tracé" +"vector.reverseLoop" = "Inverser la boucle" "vector.simplify" = "Simplifier le vecteur" "vector.smoothJoin" = "Joindre la sélection en lissant" "version.autosave" = "Enregistrer automatiquement une version" diff --git a/crates/ui-egui/locales/id.toml b/crates/ui-egui/locales/id.toml index 7a0a3c3c..e196f88b 100644 --- a/crates/ui-egui/locales/id.toml +++ b/crates/ui-egui/locales/id.toml @@ -2155,11 +2155,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Alat edit vector" "vector.edit.transform" = "Transformasi titik" "vector.edit.widthPoint" = "Titik lebar" +"vector.fillRule" = "Atur aturan isian" "vector.flatten" = "Satukan" "vector.join" = "Gabungkan pilihan" "vector.offset" = "Offset path" "vector.outlineStroke" = "Outline stroke" "vector.reverse" = "Balik arah path" +"vector.reverseLoop" = "Balik loop" "vector.simplify" = "Sederhanakan vector" "vector.smoothJoin" = "Gabungkan pilihan dengan halus" "version.autosave" = "Simpan versi otomatis" diff --git a/crates/ui-egui/locales/ja.toml b/crates/ui-egui/locales/ja.toml index b09db3bd..533170af 100644 --- a/crates/ui-egui/locales/ja.toml +++ b/crates/ui-egui/locales/ja.toml @@ -2168,11 +2168,13 @@ status = "draft — needs native review" "vector.edit.tool" = "ベクター編集ツール" "vector.edit.transform" = "ポイントを変形" "vector.edit.widthPoint" = "幅のポイント" +"vector.fillRule" = "塗りつぶし規則を設定" "vector.flatten" = "平坦化" "vector.join" = "選択範囲を連結" "vector.offset" = "パスのオフセット" "vector.outlineStroke" = "線のアウトライン化" "vector.reverse" = "パスの方向を反転" +"vector.reverseLoop" = "ループを反転" "vector.simplify" = "ベクターを単純化" "vector.smoothJoin" = "選択範囲を滑らかに連結" "version.autosave" = "自動保存バージョン" diff --git a/crates/ui-egui/locales/ko.toml b/crates/ui-egui/locales/ko.toml index f2cfcfff..09ebef0d 100644 --- a/crates/ui-egui/locales/ko.toml +++ b/crates/ui-egui/locales/ko.toml @@ -2084,11 +2084,13 @@ status = "draft — needs native review" "vector.edit.tool" = "벡터 편집 도구" "vector.edit.transform" = "점 변형" "vector.edit.widthPoint" = "너비 점" +"vector.fillRule" = "칠 규칙 설정" "vector.flatten" = "평면화" "vector.join" = "선택 항목 연결" "vector.offset" = "패스 오프셋" "vector.outlineStroke" = "선 윤곽선 만들기" "vector.reverse" = "패스 방향 반전" +"vector.reverseLoop" = "루프 반전" "vector.simplify" = "벡터 단순화" "vector.smoothJoin" = "선택 항목 부드럽게 연결" "version.autosave" = "자동 저장 버전" diff --git a/crates/ui-egui/locales/pt-BR.toml b/crates/ui-egui/locales/pt-BR.toml index e9e1300c..59802d73 100644 --- a/crates/ui-egui/locales/pt-BR.toml +++ b/crates/ui-egui/locales/pt-BR.toml @@ -2083,11 +2083,13 @@ status = "draft — needs native review" "vector.edit.tool" = "Ferramenta de edição vetorial" "vector.edit.transform" = "Transformar pontos" "vector.edit.widthPoint" = "Ponto de largura" +"vector.fillRule" = "Definir regra de preenchimento" "vector.flatten" = "Nivelar" "vector.join" = "Unir seleção" "vector.offset" = "Deslocar caminho" "vector.outlineStroke" = "Contornar traço" "vector.reverse" = "Inverter direção do caminho" +"vector.reverseLoop" = "Inverter laço" "vector.simplify" = "Simplificar vetor" "vector.smoothJoin" = "Unir seleção com suavização" "version.autosave" = "Salvar versão automaticamente" diff --git a/crates/ui-egui/locales/ru.toml b/crates/ui-egui/locales/ru.toml index 2023b692..bf8e29c7 100644 --- a/crates/ui-egui/locales/ru.toml +++ b/crates/ui-egui/locales/ru.toml @@ -2153,11 +2153,13 @@ name = "Русский" "vector.edit.tool" = "Инструмент редактирования вектора" "vector.edit.transform" = "Трансформировать точки" "vector.edit.widthPoint" = "Точка ширины" +"vector.fillRule" = "Задать правило заливки" "vector.flatten" = "Свести" "vector.join" = "Соединить выделение" "vector.offset" = "Сместить путь" "vector.outlineStroke" = "Преобразовать обводку в контур" "vector.reverse" = "Обратить направление пути" +"vector.reverseLoop" = "Развернуть контур" "vector.simplify" = "Упростить вектор" "vector.smoothJoin" = "Плавно соединить выделение" "version.autosave" = "Автосохранить версию" diff --git a/crates/ui-egui/locales/sv.toml b/crates/ui-egui/locales/sv.toml index 51f16508..96e8b83a 100644 --- a/crates/ui-egui/locales/sv.toml +++ b/crates/ui-egui/locales/sv.toml @@ -2176,11 +2176,13 @@ name = "Svenska" "vector.edit.tool" = "Vektorredigeringsverktyg" "vector.edit.transform" = "Transformera punkter" "vector.edit.widthPoint" = "Breddpunkt" +"vector.fillRule" = "Ange fyllnadsregel" "vector.flatten" = "Platta till" "vector.join" = "Sammanfoga markering" "vector.offset" = "Förskjut bana" "vector.outlineStroke" = "Konvertera kontur till form" "vector.reverse" = "Vänd banans riktning" +"vector.reverseLoop" = "Vänd slinga" "vector.simplify" = "Förenkla vektor" "vector.smoothJoin" = "Sammanfoga markering mjukt" "version.autosave" = "Autospara version" diff --git a/crates/ui-egui/locales/tr.toml b/crates/ui-egui/locales/tr.toml index cc002654..68daa4b3 100644 --- a/crates/ui-egui/locales/tr.toml +++ b/crates/ui-egui/locales/tr.toml @@ -2155,11 +2155,13 @@ name = "Türkçe" "vector.edit.tool" = "Vektör düzenleme aracı" "vector.edit.transform" = "Noktaları dönüştür" "vector.edit.widthPoint" = "Genişlik noktası" +"vector.fillRule" = "Dolgu kuralını ayarla" "vector.flatten" = "Düzleştir" "vector.join" = "Seçimi birleştir" "vector.offset" = "Yolu ötele" "vector.outlineStroke" = "Konturu ana hatlara dönüştür" "vector.reverse" = "Yol yönünü ters çevir" +"vector.reverseLoop" = "Döngüyü çevir" "vector.simplify" = "Vektörü sadeleştir" "vector.smoothJoin" = "Seçimi yumuşak birleştir" "version.autosave" = "Otomatik kayıt sürümü" diff --git a/crates/ui-egui/locales/zh-Hans.toml b/crates/ui-egui/locales/zh-Hans.toml index 702cae7b..7a8d9b96 100644 --- a/crates/ui-egui/locales/zh-Hans.toml +++ b/crates/ui-egui/locales/zh-Hans.toml @@ -2085,11 +2085,13 @@ status = "draft — needs native review" "vector.edit.tool" = "矢量编辑工具" "vector.edit.transform" = "变换点" "vector.edit.widthPoint" = "宽度点" +"vector.fillRule" = "设置填充规则" "vector.flatten" = "拼合" "vector.join" = "连接所选项" "vector.offset" = "偏移路径" "vector.outlineStroke" = "轮廓化描边" "vector.reverse" = "反转路径方向" +"vector.reverseLoop" = "反转环" "vector.simplify" = "简化矢量" "vector.smoothJoin" = "平滑连接所选项" "version.autosave" = "自动保存版本" From d489235d2c9ef689cb2321e7ddb28aaf57383a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E6=9E=97=E6=A9=A6?= <108113065+yelintong@users.noreply.github.com> Date: Sat, 17 Oct 2026 22:35:50 +0800 Subject: [PATCH 2/2] engine: short loops in vector.reverseLoop really turn round reverseLoop reversed the list and skipped shared segments, which only turns a loop of three or more round: a one-segment loop has no order at all, and a two-segment ring reads the same whichever way its list is turned, so both answered reversed: true without moving. Three or more segments: reverse the list and leave every stored end alone (a shared one belongs to the neighbour too). One or two: store one segment the other way round, skipping any segment another short loop lists; when none is left to swap, refuse with a clear error and leave the document as it was. Tests: two 2-segment areas over one line that share it, the shared segment listed first in one area and last in the other; each reversal flips that area's winding sign and leaves the shared segment and the neighbour alone. A shared one-segment loop is refused. The 4-segment squares stay as the long-loop case. Drop the two catalog.rs menu lines: they aren't in the reference app's menus, and the Vector menu runs commands without parameters, so both would fail there. --- crates/engine/src/catalog.rs | 2 - crates/engine/src/cmd/structure.rs | 97 +++++++++++------- crates/engine/src/vector_fill_rule_tests.rs | 108 +++++++++++++++++--- 3 files changed, 154 insertions(+), 53 deletions(-) diff --git a/crates/engine/src/catalog.rs b/crates/engine/src/catalog.rs index 6bee5048..c8f34fc1 100644 --- a/crates/engine/src/catalog.rs +++ b/crates/engine/src/catalog.rs @@ -235,8 +235,6 @@ pub static MENU: &[MenuItem] = &[ m!("Vector", "Delete and heal selection", "vector.deleteHeal"), m!("Vector", "Simplify vector", "vector.simplify"), m!("Vector", "Reverse path direction", "vector.reverse"), - m!("Vector", "Set fill rule", "vector.fillRule"), - m!("Vector", "Reverse loop", "vector.reverseLoop"), m!("Vector", "Offset path", "vector.offset"), // Plugins m!("Plugins", "Run last plugin", "plugins.runLast"), diff --git a/crates/engine/src/cmd/structure.rs b/crates/engine/src/cmd/structure.rs index c38052ec..d38b3dfc 100644 --- a/crates/engine/src/cmd/structure.rs +++ b/crates/engine/src/cmd/structure.rs @@ -764,20 +764,20 @@ fn area_index(cmd: &str, p: &Value) -> Result> { } } -/// Segments listed by more than one loop (of any region): reversing a loop must leave them as they -/// are, or a neighbouring area that shares the segment would turn round with it — flipping the -/// neighbour's fill or stroke. Reversing the loop's list already flips this loop's traversal of the -/// shared segment, so its stored direction is left alone. -fn shared_segments(net: &VectorNetwork) -> std::collections::HashSet { - let mut count = std::collections::HashMap::::new(); - for region in &net.regions { - for lp in ®ion.loops { - for &s in lp { - *count.entry(s).or_insert(0) += 1; +/// Segments another one- or two-segment loop lists: a short loop takes its direction from the ends +/// its segments are stored with, so re-storing one of those would turn that loop round too. A loop +/// of three or more segments is left out: its ring is walked in list order, which the stored ends +/// don't decide. +fn short_loop_segments(net: &VectorNetwork, skip: (usize, usize)) -> std::collections::HashSet { + let mut out = std::collections::HashSet::new(); + for (r, region) in net.regions.iter().enumerate() { + for (l, lp) in region.loops.iter().enumerate() { + if (r, l) != skip && lp.len() <= 2 { + out.extend(lp.iter().copied()); } } } - count.into_iter().filter(|(_, c)| *c > 1).map(|(s, _)| s).collect() + out } fn fill_rule(s: &mut Session, p: &Value) -> Result { @@ -813,41 +813,66 @@ fn fill_rule(s: &mut Session, p: &Value) -> Result { Ok(json!({"regions": count, "rule": rule_id(rule)})) } +/// One loop of a vector, worked out but not yet written back: its segments in the new order and, +/// for a loop of one or two segments, the one to store the other way. +struct LoopFlip { + id: NodeId, + turned: Vec, + store: Option, +} + /// Turn one loop of a filled area round: its segments are walked the other way, so the area it /// bounds counts the other way in the winding (which turns a hole into a filled part and back). +/// +/// How a loop is walked depends on how many segments it has: +/// - three or more: the order they are listed in, each segment turned to continue from the point +/// reached. Reversing the list is therefore enough, and the stored ends are left alone (a shared +/// segment's ends belong to its neighbour too); +/// - two: the same ring either way round, so one of the two has to be stored the other way instead; +/// - one: only its stored ends say which way it runs, so they are swapped. +/// +/// A short loop can only turn round by re-storing a segment, which is refused when another short +/// loop lists that segment too: it would turn that area round as well. fn reverse_loop(s: &mut Session, p: &Value) -> Result { - let ids = req_targets("vector.reverseLoop", s, p)?; - let region = req_index("vector.reverseLoop", p, "region")?; - let index = req_index("vector.reverseLoop", p, "loop")?; + let cmd = "vector.reverseLoop"; + let ids = req_targets(cmd, s, p)?; + let region = req_index(cmd, p, "region")?; + let index = req_index(cmd, p, "loop")?; let doc = s.doc_mut()?; - let mut reversed = false; + // Work every loop out before touching the document: one that can't turn round refuses the call + // and the others stay as they were. + let mut flips: Vec = Vec::new(); for id in ids { + let Some(NodeKind::Vector { network }) = doc.node(id).map(|n| &n.kind) else { continue }; + let Some(loop_) = network.regions.get(region).and_then(|r| r.loops.get(index)) else { continue }; + if loop_.is_empty() { + continue; + } + let mut store = None; + if loop_.len() <= 2 { + let busy = short_loop_segments(network, (region, index)); + let Some(pick) = loop_.iter().copied().filter(|s| !busy.contains(s)).min() else { + return Err(EngineError::Disabled( + cmd.into(), + "the loop shares its segments with another one- or two-segment loop, so turning it round would turn that one round too".into(), + )); + }; + store = Some(pick); + } + flips.push(LoopFlip { id, turned: loop_.iter().rev().copied().collect(), store }); + } + if flips.is_empty() { + return Err(EngineError::Disabled(cmd.into(), "no such loop (select a vector with filled areas)".into())); + } + for LoopFlip { id, turned, store } in flips { let Some(net) = network_mut(doc, id) else { continue }; - let Some(loop_) = net.regions.get(region).and_then(|r| r.loops.get(index)).cloned() else { continue }; - // A loop is walked in the order its segments are listed, so the list turned round walks the - // ring the other way. A loop of one or two segments only turns round when its segments are - // stored the other way too (list reversal alone is a no-op for them). - // A segment shared with another loop must NOT be swapped: it belongs to a neighbouring area - // whose winding we leave alone, and reversing the list already flips this loop's traversal - // of it. - let shared = shared_segments(net); - let turned: Vec = loop_.iter().rev().copied().collect(); - for &seg in &turned { - if shared.contains(&seg) { - continue; - } - if let Some(seg) = net.segments.get_mut(seg) { - std::mem::swap(&mut seg.a, &mut seg.b); - std::mem::swap(&mut seg.ta, &mut seg.tb); - } + if let Some(seg) = store.and_then(|i| net.segments.get_mut(i)) { + std::mem::swap(&mut seg.a, &mut seg.b); + std::mem::swap(&mut seg.ta, &mut seg.tb); } if let Some(l) = net.regions.get_mut(region).and_then(|r| r.loops.get_mut(index)) { *l = turned; - reversed = true; } } - if !reversed { - return Err(EngineError::Disabled("vector.reverseLoop".into(), "no such loop (select a vector with filled areas)".into())); - } Ok(json!({"reversed": true})) } diff --git a/crates/engine/src/vector_fill_rule_tests.rs b/crates/engine/src/vector_fill_rule_tests.rs index db96b669..8a8fe850 100644 --- a/crates/engine/src/vector_fill_rule_tests.rs +++ b/crates/engine/src/vector_fill_rule_tests.rs @@ -37,8 +37,8 @@ fn ring() -> VectorNetwork { net } -/// A selected vector layer holding `ring()`. -fn ringed(s: &mut Session) -> NodeId { +/// A selected vector layer holding `network`. +fn vector_layer(s: &mut Session, network: VectorNetwork) -> NodeId { let id = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap(); let id = NodeId(id); ex(s, "vector.edit.begin", json!({"id": id})); @@ -46,7 +46,7 @@ fn ringed(s: &mut Session) -> NodeId { { let doc = s.doc_mut().unwrap(); match &mut doc.get_mut(id).unwrap().kind { - NodeKind::Vector { network } => *network = ring(), + NodeKind::Vector { network: net } => *net = network, other => panic!("not a vector: {other:?}"), } } @@ -54,6 +54,11 @@ fn ringed(s: &mut Session) -> NodeId { id } +/// A selected vector layer holding `ring()`. +fn ringed(s: &mut Session) -> NodeId { + vector_layer(s, ring()) +} + /// Two 50 × 100 squares side by side share the vertical edge between them: one segment listed by /// both areas' single loops. A segment shared between areas is the case `vector.reverseLoop` must /// not flip, or a neighbour's fill or stroke would turn round with it. @@ -91,19 +96,43 @@ fn two_squares() -> VectorNetwork { /// A selected vector layer holding [`two_squares`]. fn squared(s: &mut Session) -> NodeId { - let id = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 100, "height": 100})).unwrap()["id"].as_u64().unwrap(); - let id = NodeId(id); - ex(s, "vector.edit.begin", json!({"id": id})); - ex(s, "vector.edit.end", json!({})); - { - let doc = s.doc_mut().unwrap(); - match &mut doc.get_mut(id).unwrap().kind { - NodeKind::Vector { network } => *network = two_squares(), - other => panic!("not a vector: {other:?}"), - } + vector_layer(s, two_squares()) +} + +/// Two areas of two segments each over the same line: one curve above it and one below. Both areas +/// list the line's segment — the first area first (`[line, above]`), the second area last +/// (`[below, line]`) — and a two-segment ring reads the same whichever way its list is turned, so +/// these only turn round when a segment is stored the other way. +fn two_lenses() -> VectorNetwork { + // 0:A(0,0) 1:B(100,0) + // s0 A→B the straight line both areas use; s1 B→A curving above it; s2 A→B curving below it. + VectorNetwork { + vertices: vec![Vertex::new(0.0, 0.0), Vertex::new(100.0, 0.0)], + segments: vec![ + Segment::line(0, 1), + Segment { a: 1, b: 0, ta: (0.0, -50.0), tb: (0.0, -50.0) }, + Segment { a: 0, b: 1, ta: (0.0, 50.0), tb: (0.0, 50.0) }, + ], + regions: vec![ + Region { loops: vec![vec![0, 1]], winding: WindingRule::NonZero }, + Region { loops: vec![vec![2, 0]], winding: WindingRule::NonZero }, + ], + } +} + +/// A selected vector layer holding [`two_lenses`]. +fn lensed(s: &mut Session) -> NodeId { + vector_layer(s, two_lenses()) +} + +/// Two areas whose only loop is the same closed segment (a curve the path closes with a straight +/// line): a one-segment loop's direction is its segment's stored ends, and both areas read them. +fn one_shared_segment() -> VectorNetwork { + VectorNetwork { + vertices: vec![Vertex::new(0.0, 0.0), Vertex::new(100.0, 0.0)], + segments: vec![Segment { a: 0, b: 1, ta: (0.0, -60.0), tb: (0.0, -60.0) }], + regions: vec![Region { loops: vec![vec![0]], winding: WindingRule::NonZero }, Region { loops: vec![vec![0]], winding: WindingRule::NonZero }], } - ex(s, "select.set", json!({"ids": [id]})); - id } /// The point in the middle of the hole, and one on the square's left band. @@ -226,3 +255,52 @@ fn reversing_a_loop_shared_with_a_neighbour_leaves_its_segment_and_fill_alone() s.undo().unwrap(); assert_eq!(net(&s, id).segments[SHARED], before, "undo restores the shared segment too"); } + +#[test] +fn a_two_segment_loop_turns_round_with_its_shared_segment_listed_first_or_last() { + let mut s = Session::new(); + let id = lensed(&mut s); + const SHARED: usize = 0; + const ABOVE: Point = Point::new(50.0, -25.0); + const BELOW: Point = Point::new(50.0, 25.0); + let n = net(&s, id); + assert_eq!(n.regions.len(), 2, "two areas over the same line"); + assert_eq!((n.regions[0].loops[0][0], n.regions[0].loops[0].len()), (SHARED, 2), "the shared segment is listed first"); + assert_eq!((n.regions[1].loops[0][1], n.regions[1].loops[0].len()), (SHARED, 2), "and last"); + assert!(n.region_contains(0, ABOVE), "the area above the line fills"); + assert!(n.region_contains(1, BELOW), "the area below it fills"); + let (above, below) = (n.region_winding(0, ABOVE).unwrap(), n.region_winding(1, BELOW).unwrap()); + assert_ne!(above, 0, "the area above winds once"); + assert_ne!(below, 0, "the area below winds once"); + let shared = n.segments[SHARED]; + // Turn the area that lists the shared segment first round: a two-segment ring reads the same + // either way, so this only works by storing one of its segments the other way. + assert_eq!(ex(&mut s, "vector.reverseLoop", json!({"id": id, "region": 0, "loop": 0}))["reversed"], json!(true)); + let n = net(&s, id); + assert_eq!(n.segments[SHARED], shared, "the shared segment keeps its ends"); + assert!(n.region_contains(0, ABOVE), "the area still fills"); + assert_eq!(n.region_winding(0, ABOVE), Some(-above), "it really turned round"); + assert_eq!(n.region_winding(1, BELOW), Some(below), "the neighbour did not"); + // And the area that lists it last. + ex(&mut s, "vector.reverseLoop", json!({"id": id, "region": 1, "loop": 0})); + let n = net(&s, id); + assert_eq!(n.segments[SHARED], shared, "still untouched"); + assert_eq!(n.region_winding(1, BELOW), Some(-below), "the second area really turned round"); + assert_eq!(n.region_winding(0, ABOVE), Some(-above), "the first stays as it was"); + // Undo turns each back. + s.undo().unwrap(); + assert_eq!(net(&s, id).region_winding(1, BELOW), Some(below), "undo turns the second back"); + s.undo().unwrap(); + assert_eq!(net(&s, id).region_winding(0, ABOVE), Some(above), "and the first"); +} + +#[test] +fn a_one_segment_loop_shared_with_another_is_refused_instead_of_claiming_a_turn() { + let mut s = Session::new(); + let id = vector_layer(&mut s, one_shared_segment()); + let before = net(&s, id); + let err = s.execute("vector.reverseLoop", &json!({"id": id, "region": 0, "loop": 0})).unwrap_err(); + let message = err.to_string(); + assert!(message.contains("shares its segments"), "the error says why: {message}"); + assert_eq!(net(&s, id), before, "nothing changed"); +}