From b26dfbcd7c928689ad6b98d2f38709653b578e40 Mon Sep 17 00:00:00 2001 From: Dennis Kobert Date: Thu, 3 Oct 2024 20:20:04 +0200 Subject: [PATCH] Always close subpaths before applying boolean ops (#2014) * Always close subpaths before applying boolean ops * Roundtrip boolean path through svg string * Reverse closing path segment * Sort result of boolean ops * Make face visiting order deterministic * Remove debugging code * Remove unused post processing * Clippy lint --------- Co-authored-by: Keavon Chambers --- libraries/path-bool/src/lib.rs | 37 +++++++++++++++++++++++++ libraries/path-bool/src/path_boolean.rs | 9 ++++-- node-graph/gstd/src/vector.rs | 17 +++++++++--- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/libraries/path-bool/src/lib.rs b/libraries/path-bool/src/lib.rs index 0703a1f0..ba58a94a 100644 --- a/libraries/path-bool/src/lib.rs +++ b/libraries/path-bool/src/lib.rs @@ -61,6 +61,24 @@ mod test { assert!(!union[0].is_empty()); } #[test] + fn semi_circle_join() { + let a = path_from_path_data( + "M 0.000000000000,0.000000000000 C 0.000000000000,0.000000000000 48.487723000000,-68.116546000000 51.950617000000,-96.987654000000 C 53.354187000000,-108.689605000000 5.171145000000,-241.394513000000 -0.000000000000,-254.901235000000 Z M 0.000000000000,0.000000000000 L -0.000000000000,-254.901235000000 Z" + ).unwrap(); + let b = path_from_path_data( + "M -0.000000000000,0.484328000000 C -0.000000000000,0.484328000000 -48.487723000000,-67.632218000000 -51.950617000000,-96.503326000000 C -53.354187000000,-108.205277000000 -5.171145000000,-240.910185000000 -0.000000000000,-254.416907000000 Z M -0.000000000000,0.484328000000 L -0.000000000000,-254.416907000000 Z", + ) + .unwrap(); + + let result = path_boolean(&a, FillRule::NonZero, &b, FillRule::NonZero, PathBooleanOperation::Union).unwrap(); + + // Add assertions here based on expected results + assert_eq!(result.len(), 1, "Expected 1 resulting path for Union operation"); + dbg!(path_to_path_data(&result[0], 0.001)); + // Add more specific assertions about the resulting path if needed + assert!(!result[0].is_empty()); + } + #[test] fn nesting_02() { let a = path_from_path_data("M 0.99999994,31.334457 C 122.61195,71.81859 -79.025816,-5.5803326 47,32.253367 V 46.999996 H 0.99999994 Z").unwrap(); let b = path_from_path_data("m 25.797222,29.08718 c 0,1.292706 -1.047946,2.340652 -2.340652,2.340652 -1.292707,0 -2.340652,-1.047946 -2.340652,-2.340652 0,-1.292707 1.047945,-2.340652 2.340652,-2.340652 1.292706,0 2.340652,1.047945 2.340652,2.340652 z M 7.5851073,28.332212 c 1e-7,1.292706 -1.0479456,2.340652 -2.3406521,2.340652 -1.2927063,-1e-6 -2.3406518,-1.047946 -2.3406517,-2.340652 -10e-8,-1.292707 1.0479454,-2.340652 2.3406517,-2.340652 1.2927065,-1e-6 2.3406522,1.047945 2.3406521,2.340652 z").unwrap(); @@ -274,4 +292,23 @@ mod test { // Add more specific assertions about the resulting path if needed assert!(!result[0].is_empty()); } + #[test] + fn leaf() { + let a = path_from_path_data( + "M 0.000000000000,0.000000000000 C 0.000000000000,0.000000000000 30.204085760000,16.549763300000 72.719386340000,26.932261410000 C 72.719386340000,26.932261410000 70.254313510000,17.656445030000 64.511458520000,11.328808640000 C 64.511458520000,11.328808640000 87.851933730000,7.175523990000 112.835205650000,14.952290640000 C 112.835205650000,14.952290640000 85.410272600000,-0.944114870000 126.097112900000,-8.575894450000 C 118.337466460000,-27.271823700000 151.641256810000,-26.435638610000 157.566270790000,-37.101186050000 C 157.566270790000,-37.101186050000 140.392289790000,-20.685582680000 114.877703390000,-41.337145860000 C 99.535241580000,-45.054042580000 93.348771040000,-46.907137740000 91.251365300000,-51.920903790000 C 88.782358670000,-57.822967180000 120.444444440000,-82.851851850000 141.777777780000,-86.604938270000 C 141.777777780000,-86.604938270000 123.802469140000,-82.654320990000 114.716049380000,-90.160493830000 C 105.629629630000,-97.666666670000 112.345679010000,-115.000000000000 125.382716050000,-124.679012350000 C 125.382716050000,-124.679012350000 112.740740740000,-117.962962960000 115.506172840000,-132.382716050000 C 118.271604940000,-146.802469140000 109.740281350000,-157.864197530000 136.839506170000,-177.617283950000 C 115.506172840000,-165.567901230000 97.333333330000,-150.358024690000 92.395061730000,-161.419753090000 C 81.925925930000,-147.987654320000 66.518518520000,-140.481481480000 66.320987650000,-154.308641980000 C 59.703703700000,-139.000000000000 54.074074070000,-137.518518520000 49.135802470000,-136.333333330000 C 47.555555560000,-141.864197530000 43.456790120000,-172.333333330000 63.802469140000,-190.901234570000 C 48.888888890000,-178.259259260000 20.444444440000,-172.037037040000 28.444444440000,-210.456790120000 C 23.506172840000,-203.938271600000 13.234567900000,-196.432098770000 0.000000000000,-231.000000000000 L 0.000000000000,0.000000000000 Z" + ).unwrap(); + let b = path_from_path_data( + "M 0.100000000000,0.000000000000 C 0.100000000000,0.000000000000 -30.104085760000,16.549763300000 -72.619386340000,26.932261410000 C -72.619386340000,26.932261410000 -70.154313510000,17.656445030000 -64.411458520000,11.328808640000 C -64.411458520000,11.328808640000 -87.751933730000,7.175523990000 -112.735205650000,14.952290640000 C -112.735205650000,14.952290640000 -85.310272600000,-0.944114870000 -125.997112900000,-8.575894450000 C -118.237466460000,-27.271823700000 -151.541256810000,-26.435638610000 -157.466270790000,-37.101186050000 C -157.466270790000,-37.101186050000 -140.292289790000,-20.685582680000 -114.777703390000,-41.337145860000 C -99.435241580000,-45.054042580000 -93.248771040000,-46.907137740000 -91.151365300000,-51.920903790000 C -88.682358670000,-57.822967180000 -120.344444440000,-82.851851850000 -141.677777780000,-86.604938270000 C -141.677777780000,-86.604938270000 -123.702469140000,-82.654320990000 -114.616049380000,-90.160493830000 C -105.529629630000,-97.666666670000 -112.245679010000,-115.000000000000 -125.282716050000,-124.679012350000 C -125.282716050000,-124.679012350000 -112.640740740000,-117.962962960000 -115.406172840000,-132.382716050000 C -118.171604940000,-146.802469140000 -109.640281350000,-157.864197530000 -136.739506170000,-177.617283950000 C -115.406172840000,-165.567901230000 -97.233333330000,-150.358024690000 -92.295061730000,-161.419753090000 C -81.825925930000,-147.987654320000 -66.418518520000,-140.481481480000 -66.220987650000,-154.308641980000 C -59.603703700000,-139.000000000000 -53.974074070000,-137.518518520000 -49.035802470000,-136.333333330000 C -47.455555560000,-141.864197530000 -43.356790120000,-172.333333330000 -63.702469140000,-190.901234570000 C -48.788888890000,-178.259259260000 -20.344444440000,-172.037037040000 -28.344444440000,-210.456790120000 C -23.406172840000,-203.938271600000 -13.134567900000,-196.432098770000 0.100000000000,-231.000000000000 L 0.100000000000,0.000000000000 Z", + ) + .unwrap(); + + let result = path_boolean(&a, FillRule::NonZero, &b, FillRule::NonZero, PathBooleanOperation::Union).unwrap(); + + // Add assertions here based on expected results + assert_eq!(result.len(), 1, "Expected 1 resulting path for Union operation"); + dbg!(path_to_path_data(&result[0], 0.001)); + // Add more specific assertions about the resulting path if needed + assert_eq!(result.len(), 1); + assert!(!result[0].is_empty()); + } } diff --git a/libraries/path-bool/src/path_boolean.rs b/libraries/path-bool/src/path_boolean.rs index 2f513b99..38e5b92f 100644 --- a/libraries/path-bool/src/path_boolean.rs +++ b/libraries/path-bool/src/path_boolean.rs @@ -1338,8 +1338,10 @@ fn get_selected_faces<'a>(predicate: &'a impl Fn(u8) -> bool, flags: &'a HashMap flags.iter().filter_map(|(key, &flag)| predicate(flag).then_some(*key)) } -fn walk_faces<'a>(faces: &'a HashSet, edges: &SlotMap, vertices: &SlotMap) -> impl Iterator + 'a { - let is_removed_edge = |edge: &DualGraphHalfEdge| faces.contains(&edge.incident_vertex) == faces.contains(&edges[edge.twin.unwrap()].incident_vertex); +fn walk_faces<'a>(faces: &'a [DualVertexKey], edges: &SlotMap, vertices: &SlotMap) -> impl Iterator + 'a { + let face_set: HashSet<_> = faces.iter().copied().collect(); + // TODO: Try using a binary search to avioid the hashset construction + let is_removed_edge = |edge: &DualGraphHalfEdge| face_set.contains(&edge.incident_vertex) == face_set.contains(&edges[edge.twin.unwrap()].incident_vertex); let mut edge_to_next = HashMap::new(); for face_key in faces { @@ -1627,7 +1629,8 @@ pub fn path_boolean(a: &Path, a_fill_rule: FillRule, b: &Path, b_fill_rule: Fill match op { PathBooleanOperation::Division | PathBooleanOperation::Fracture => Ok(dump_faces(&nesting_trees, predicate, edges, vertices, &flags)), _ => { - let selected_faces: HashSet = get_selected_faces(&predicate, &flags).collect(); + let mut selected_faces: Vec = get_selected_faces(&predicate, &flags).collect(); + selected_faces.sort_unstable(); Ok(vec![walk_faces(&selected_faces, edges, vertices).collect()]) } } diff --git a/node-graph/gstd/src/vector.rs b/node-graph/gstd/src/vector.rs index 1f7aa58e..760bb9bc 100644 --- a/node-graph/gstd/src/vector.rs +++ b/node-graph/gstd/src/vector.rs @@ -5,10 +5,11 @@ use graphene_core::transform::Transform; use graphene_core::vector::misc::BooleanOperation; pub use graphene_core::vector::*; use graphene_core::{Color, GraphicElement, GraphicGroup}; +use path_bool::FillRule; +use path_bool::PathBooleanOperation; use glam::{DAffine2, DVec2}; -use path_bool::PathBooleanOperation; -use std::ops::{Div, Mul}; +use std::ops::Mul; #[node_macro::node(category(""))] async fn boolean_operation( @@ -205,11 +206,17 @@ fn to_path(vector: &VectorData, transform: DAffine2) -> Vec, subpath: &bezier_rs::Subpath, transform: DAffine2) { use path_bool::PathSegment; + let mut global_start = None; + let mut global_end = DVec2::ZERO; for bezier in subpath.iter() { const EPS: f64 = 1e-8; - let transformed = bezier.apply_transformation(|pos| transform.transform_point2(pos).mul(EPS.recip()).round().div(EPS.recip())); + let transformed = bezier.apply_transformation(|pos| transform.transform_point2(pos).mul(EPS.recip()).round().mul(EPS)); let start = transformed.start; let end = transformed.end; + if global_start.is_none() { + global_start = Some(start); + } + global_end = end; let segment = match transformed.handles { bezier_rs::BezierHandles::Linear => PathSegment::Line(start, end), bezier_rs::BezierHandles::Quadratic { handle } => PathSegment::Quadratic(start, handle, end), @@ -217,6 +224,9 @@ fn to_path_segments(path: &mut Vec, subpath: &bezier_rs: }; path.push(segment); } + if let Some(start) = global_start { + path.push(PathSegment::Line(global_end, start)); + } } fn from_path(path_data: &[Path]) -> VectorData { @@ -317,7 +327,6 @@ fn boolean_union(a: Path, b: Path) -> Vec { } fn path_bool(a: Path, b: Path, op: PathBooleanOperation) -> Vec { - use path_bool::FillRule; match path_bool::path_boolean(&a, FillRule::NonZero, &b, FillRule::NonZero, op) { Ok(results) => results, Err(e) => {