From fdb1d040b38065dc32cc23758658e05fc9fc78cc Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Sun, 18 Jan 2026 22:03:56 -0600 Subject: [PATCH 1/7] Attempt one-shot n-ary ops --- Cargo.lock | 47 ++++- Cargo.toml | 2 + node-graph/nodes/path-bool/Cargo.toml | 2 + node-graph/nodes/path-bool/src/lib.rs | 282 +++++++++----------------- 4 files changed, 133 insertions(+), 200 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 853d170aa2..abb7d2b1b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -202,6 +202,9 @@ name = "arrayvec" version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" +dependencies = [ + "serde", +] [[package]] name = "as-raw-xcb-connection" @@ -1136,7 +1139,7 @@ dependencies = [ "dyn-any", "glam", "image", - "kurbo", + "kurbo 0.13.0", "log", "lyon_geom", "no-std-types", @@ -2528,7 +2531,7 @@ dependencies = [ "image", "interpreted-executor", "js-sys", - "kurbo", + "kurbo 0.13.0", "log", "num_enum", "once_cell", @@ -3302,6 +3305,18 @@ dependencies = [ "libc", ] +[[package]] +name = "kurbo" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce9729cc38c18d86123ab736fd2e7151763ba226ac2490ec092d1dd148825e32" +dependencies = [ + "arrayvec", + "euclid", + "serde", + "smallvec", +] + [[package]] name = "kurbo" version = "0.13.0" @@ -3391,6 +3406,16 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4a5ff6bcca6c4867b1c4fd4ef63e4db7436ef363e0ad7531d1558856bae64f4" +[[package]] +name = "linesweeper" +version = "0.1.2" +source = "git+https://github.com/jneem/linesweeper?rev=db9809e698c0b7c46666987df33b43f8ec3c2fde#db9809e698c0b7c46666987df33b43f8ec3c2fde" +dependencies = [ + "arrayvec", + "kurbo 0.12.0", + "polycool", +] + [[package]] name = "linux-raw-sys" version = "0.9.4" @@ -4289,10 +4314,12 @@ dependencies = [ "dyn-any", "glam", "graphic-types", + "linesweeper", "log", "node-macro", "path-bool", "serde", + "smallvec", "tsify", "vector-types", "wasm-bindgen", @@ -4311,7 +4338,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a2b6aadb221872732e87d465213e9be5af2849b0e8cc5300a8ba98fffa2e00a" dependencies = [ "color", - "kurbo", + "kurbo 0.13.0", "linebender_resource_handle", "smallvec", ] @@ -4907,7 +4934,7 @@ dependencies = [ "futures", "glam", "image", - "kurbo", + "kurbo 0.13.0", "ndarray", "no-std-types", "node-macro", @@ -5142,7 +5169,7 @@ dependencies = [ "dyn-any", "glam", "graphic-types", - "kurbo", + "kurbo 0.13.0", "log", "num-traits", "serde", @@ -5160,7 +5187,7 @@ dependencies = [ "glam", "graphene-core", "graphic-types", - "kurbo", + "kurbo 0.13.0", "log", "node-macro", "raster-types", @@ -6091,7 +6118,7 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "695b5790b3131dafa99b3bbfd25a216edb3d216dad9ca208d4657bfb8f2abc3d" dependencies = [ - "kurbo", + "kurbo 0.13.0", "siphasher", ] @@ -6863,7 +6890,7 @@ dependencies = [ "flate2", "fontdb", "imagesize", - "kurbo", + "kurbo 0.13.0", "log", "pico-args", "roxmltree 0.21.1", @@ -6925,7 +6952,7 @@ dependencies = [ "glam", "graphene-core", "graphic-types", - "kurbo", + "kurbo 0.13.0", "log", "node-macro", "qrcodegen", @@ -6949,7 +6976,7 @@ dependencies = [ "dyn-any", "fixedbitset", "glam", - "kurbo", + "kurbo 0.13.0", "log", "lyon_geom", "node-macro", diff --git a/Cargo.toml b/Cargo.toml index a7bd74b4f0..1539a816b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -212,6 +212,8 @@ cargo-gpu = { git = "https://github.com/Firestar99/cargo-gpu", rev = "3952a22d16 qrcodegen = "1.8" lzma-rust2 = { version = "0.16", default-features = false, features = ["std", "encoder", "optimization", "xz"] } scraper = "0.25" +linesweeper = { git = "https://github.com/jneem/linesweeper", rev = "db9809e698c0b7c46666987df33b43f8ec3c2fde" } +smallvec = "1.13.2" [workspace.lints.rust] unexpected_cfgs = { level = "allow", check-cfg = ['cfg(target_arch, values("spirv"))'] } diff --git a/node-graph/nodes/path-bool/Cargo.toml b/node-graph/nodes/path-bool/Cargo.toml index d755adcb75..5ee2e6e774 100644 --- a/node-graph/nodes/path-bool/Cargo.toml +++ b/node-graph/nodes/path-bool/Cargo.toml @@ -16,9 +16,11 @@ core-types = { workspace = true } graphic-types = { workspace = true } node-macro = { workspace = true } glam = { workspace = true } +linesweeper = { workspace = true } log = { workspace = true } path-bool = { workspace = true } serde = { workspace = true } +smallvec = { workspace = true } vector-types = { workspace = true } # Optional workspace dependencies diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 49ab3004b4..7b0d14273b 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -2,14 +2,16 @@ use core_types::table::{Table, TableRow, TableRowRef}; use core_types::{Color, Ctx}; use dyn_any::DynAny; use glam::{DAffine2, DVec2}; -use graphic_types::vector_types::subpath::{ManipulatorGroup, PathSegPoints, Subpath, pathseg_points}; +use graphic_types::vector_types::subpath::{ManipulatorGroup, Subpath}; use graphic_types::vector_types::vector::PointId; use graphic_types::vector_types::vector::algorithms::merge_by_distance::MergeByDistanceExt; use graphic_types::vector_types::vector::style::Fill; use graphic_types::{Graphic, Vector}; +use linesweeper::topology::Topology; pub use path_bool as path_bool_lib; use path_bool::{FillRule, PathBooleanOperation}; -use std::ops::Mul; +use smallvec::SmallVec; +use vector_types::kurbo::{Affine, BezPath, CubicBez, ParamCurve, Point}; // TODO: Fix boolean ops to work by removing .transform() and .one_instance_*() calls, // TODO: since before we used a Vec of single-row tables and now we use a single table @@ -68,164 +70,98 @@ async fn boolean_operation(vector: impl DoubleEndedIterator> + Clone, boolean_operation: BooleanOperation) -> Table { - match boolean_operation { - BooleanOperation::Union => union(vector), - BooleanOperation::SubtractFront => subtract(vector), - BooleanOperation::SubtractBack => subtract(vector.rev()), - BooleanOperation::Intersect => intersect(vector), - BooleanOperation::Difference => difference(vector), - } +#[derive(Clone, Debug, Default, PartialEq, Eq)] +struct WindingNumber { + elems: SmallVec<[i16; 8]>, } -fn union<'a>(vector: impl DoubleEndedIterator>) -> Table { - // Reverse the vector table rows so that the result style is the style of the first vector row - let mut vector_reversed = vector.rev(); - - let mut result_vector_table = Table::new_from_row(vector_reversed.next().map(|x| x.into_cloned()).unwrap_or_default()); - let mut first_row = result_vector_table.iter_mut().next().expect("Expected the one row we just pushed"); - - // Loop over all vector table rows and union it with the result - let default = TableRow::default(); - let mut second_vector = Some(vector_reversed.next().unwrap_or(default.as_ref())); - while let Some(lower_vector) = second_vector { - let transform_of_lower_into_space_of_upper = first_row.transform.inverse() * *lower_vector.transform; - - let result = &mut first_row.element; - - let upper_path_string = to_path(result, DAffine2::IDENTITY); - let lower_path_string = to_path(lower_vector.element, transform_of_lower_into_space_of_upper); - - #[allow(unused_unsafe)] - let boolean_operation_string = unsafe { boolean_union(upper_path_string, lower_path_string) }; - let boolean_operation_result = from_path(&boolean_operation_string); - - result.colinear_manipulators = boolean_operation_result.colinear_manipulators; - result.point_domain = boolean_operation_result.point_domain; - result.segment_domain = boolean_operation_result.segment_domain; - result.region_domain = boolean_operation_result.region_domain; +impl linesweeper::topology::WindingNumber for WindingNumber { + type Tag = usize; - second_vector = vector_reversed.next(); + fn single(tag: usize, positive: bool) -> Self { + let mut elems = SmallVec::with_capacity(tag); + let sign = if positive { 1 } else { -1 }; + for _ in 0..tag { + elems.push(0); + } + elems.push(sign); + Self { elems } } - - result_vector_table } -fn subtract<'a>(vector: impl Iterator>) -> Table { - let mut vector = vector.into_iter(); - - let mut result_vector_table = Table::new_from_row(vector.next().map(|x| x.into_cloned()).unwrap_or_default()); - let mut first_row = result_vector_table.iter_mut().next().expect("Expected the one row we just pushed"); - let first_row_transform = if first_row.transform.matrix2.determinant() != 0. { - first_row.transform.inverse() - } else { - DAffine2::IDENTITY - }; - - let mut next_vector = vector.next(); - - while let Some(lower_vector) = next_vector { - let transform_of_lower_into_space_of_upper = first_row_transform * *lower_vector.transform; - - let result = &mut first_row.element; - - let upper_path_string = to_path(result, DAffine2::IDENTITY); - let lower_path_string = to_path(lower_vector.element, transform_of_lower_into_space_of_upper); - - #[allow(unused_unsafe)] - let boolean_operation_string = unsafe { boolean_subtract(upper_path_string, lower_path_string) }; - let boolean_operation_result = from_path(&boolean_operation_string); +impl std::ops::AddAssign for WindingNumber { + fn add_assign(&mut self, rhs: Self) { + if self.elems.len() < rhs.elems.len() { + self.elems.resize(rhs.elems.len(), 0); + } - result.colinear_manipulators = boolean_operation_result.colinear_manipulators; - result.point_domain = boolean_operation_result.point_domain; - result.segment_domain = boolean_operation_result.segment_domain; - result.region_domain = boolean_operation_result.region_domain; + for (me, them) in self.elems.iter_mut().zip(&rhs.elems) { + *me += *them; + } - next_vector = vector.next(); + // Removing trailing zeros normalizes the representation so that the derived + // PartialEq works. (Alternatively, we could write our own PartialEq.) + let trailing_zeros = self.elems.iter().rev().take_while(|w| **w == 0).count(); + self.elems.truncate(self.elems.len() - trailing_zeros); } - - result_vector_table } -fn intersect<'a>(vector: impl DoubleEndedIterator>) -> Table { - let mut vector = vector.rev(); - - let mut result_vector_table = Table::new_from_row(vector.next().map(|x| x.into_cloned()).unwrap_or_default()); - let mut first_row = result_vector_table.iter_mut().next().expect("Expected the one row we just pushed"); +impl std::ops::Add for WindingNumber { + type Output = WindingNumber; - let default = TableRow::default(); - let mut second_vector = Some(vector.next().unwrap_or(default.as_ref())); - - // For each vector table row, set the result to the intersection of that path and the current result - while let Some(lower_vector) = second_vector { - let transform_of_lower_into_space_of_upper = first_row.transform.inverse() * *lower_vector.transform; - - let result = &mut first_row.element; - - let upper_path_string = to_path(result, DAffine2::IDENTITY); - let lower_path_string = to_path(lower_vector.element, transform_of_lower_into_space_of_upper); - - #[allow(unused_unsafe)] - let boolean_operation_string = unsafe { boolean_intersect(upper_path_string, lower_path_string) }; - let boolean_operation_result = from_path(&boolean_operation_string); - - result.colinear_manipulators = boolean_operation_result.colinear_manipulators; - result.point_domain = boolean_operation_result.point_domain; - result.segment_domain = boolean_operation_result.segment_domain; - result.region_domain = boolean_operation_result.region_domain; - second_vector = vector.next(); + fn add(mut self, rhs: Self) -> Self::Output { + self += rhs; + self } - - result_vector_table } -fn difference<'a>(vector: impl DoubleEndedIterator> + Clone) -> Table { - let mut vector_iter = vector.clone().rev(); - let mut any_intersection = TableRow::default(); - let default = TableRow::default(); - let mut second_vector = Some(vector_iter.next().unwrap_or(default.as_ref())); - - // Find where all vector table row paths intersect at least once - while let Some(lower_vector) = second_vector { - let filtered_vector = vector.clone().filter(|v| *v != lower_vector).collect::>().into_iter(); - let unioned = boolean_operation_on_vector_table(filtered_vector, BooleanOperation::Union); - let first_row = unioned.iter().next().expect("Expected at least one row after the boolean union"); - - let transform_of_lower_into_space_of_upper = first_row.transform.inverse() * *lower_vector.transform; - - let upper_path_string = to_path(first_row.element, DAffine2::IDENTITY); - let lower_path_string = to_path(lower_vector.element, transform_of_lower_into_space_of_upper); - - #[allow(unused_unsafe)] - let boolean_intersection_string = unsafe { boolean_intersect(upper_path_string, lower_path_string) }; - let mut element = from_path(&boolean_intersection_string); - element.style = first_row.element.style.clone(); - let boolean_intersection_result = TableRow { - element, - transform: *first_row.transform, - alpha_blending: *first_row.alpha_blending, - source_node_id: *first_row.source_node_id, - }; +impl WindingNumber { + fn is_inside(&self, op: BooleanOperation) -> bool { + match op { + BooleanOperation::Union => self.elems.iter().any(|w| *w != 0), + BooleanOperation::SubtractFront => self.elems.first().is_some_and(|w| *w != 0) && self.elems.iter().skip(1).all(|w| *w == 0), + BooleanOperation::SubtractBack => self.elems.last().is_some_and(|w| *w != 0) && self.elems.iter().rev().skip(1).all(|w| *w == 0), + BooleanOperation::Intersect => self.elems.iter().all(|w| *w != 0), + BooleanOperation::Difference => self.elems.iter().any(|w| *w != 0) && !self.elems.iter().all(|w| *w != 0), + } + } +} - let transform_of_lower_into_space_of_upper = boolean_intersection_result.transform.inverse() * any_intersection.transform; +fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator>, boolean_operation: BooleanOperation) -> Table { + const EPSILON: f64 = 1e-5; + let mut table = Table::new(); + let mut alpha_blending = None; + let mut source_node_id = None; + let mut paths = Vec::new(); + for v in vector { + if alpha_blending.is_none() { + alpha_blending = Some(*v.alpha_blending); + source_node_id = Some(*v.source_node_id); + } - let upper_path_string = to_path(&boolean_intersection_result.element, DAffine2::IDENTITY); - let lower_path_string = to_path(&any_intersection.element, transform_of_lower_into_space_of_upper); + paths.push(to_bez_path(v.element, *v.transform)); + } - #[allow(unused_unsafe)] - let union_result = from_path(&unsafe { boolean_union(upper_path_string, lower_path_string) }); - any_intersection.element = union_result; + log::warn!("boolean op {boolean_operation:?} on paths:"); + for p in &paths { + log::warn!("{}", p.to_svg()); + } - any_intersection.transform = boolean_intersection_result.transform; - any_intersection.element.style = boolean_intersection_result.element.style.clone(); - any_intersection.alpha_blending = boolean_intersection_result.alpha_blending; + // unwrap: Topology::from_paths only errors on a non-closed path, and our paths are closed by construction. + let top = Topology::::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, idx)), EPSILON).unwrap(); + let contours = top.contours(|w| w.is_inside(boolean_operation)); - second_vector = vector_iter.next(); + log::warn!("boolean op output paths:"); + for c in contours.contours() { + log::warn!("{}", c.path.to_svg()); } - - // Subtract the area where they intersect at least once from the union of all vector paths - let union = boolean_operation_on_vector_table(vector, BooleanOperation::Union); - boolean_operation_on_vector_table(union.iter().chain(std::iter::once(any_intersection.as_ref())), BooleanOperation::SubtractFront) + table.push(TableRow { + element: from_bez_paths(contours.contours().map(|c| &c.path)), + transform: DAffine2::IDENTITY, + alpha_blending: alpha_blending.unwrap_or_default(), + source_node_id: source_node_id.unwrap_or_default(), + }); + table } fn flatten_vector(graphic_table: &Table) -> Table { @@ -325,66 +261,40 @@ fn flatten_vector(graphic_table: &Table) -> Table { .collect() } -fn to_path(vector: &Vector, transform: DAffine2) -> Vec { - let mut path = Vec::new(); +fn to_bez_path(vector: &Vector, transform: DAffine2) -> BezPath { + let mut path = BezPath::new(); for subpath in vector.stroke_bezier_paths() { - to_path_segments(&mut path, &subpath, transform); + push_subpath(&mut path, &subpath, transform); } path } -fn to_path_segments(path: &mut Vec, subpath: &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 transform_point = |pos: DVec2| transform.transform_point2(pos).mul(EPS.recip()).round().mul(EPS); +fn push_subpath(path: &mut BezPath, subpath: &Subpath, transform: DAffine2) { + let transform = Affine::new(transform.to_cols_array()); + let mut first = true; - let PathSegPoints { p0, p1, p2, p3 } = pathseg_points(bezier); - - let p0 = transform_point(p0); - let p1 = p1.map(transform_point); - let p2 = p2.map(transform_point); - let p3 = transform_point(p3); - - if global_start.is_none() { - global_start = Some(p0); + for seg in subpath.iter() { + if first { + first = false; + path.move_to(transform * seg.start()); } - global_end = p3; - - let segment = match (p1, p2) { - (None, None) => PathSegment::Line(p0, p3), - (None, Some(p2)) | (Some(p2), None) => PathSegment::Quadratic(p0, p2, p3), - (Some(p1), Some(p2)) => PathSegment::Cubic(p0, p1, p2, p3), - }; - - path.push(segment); - } - if let Some(start) = global_start { - path.push(PathSegment::Line(global_end, start)); + path.push(transform * seg.as_path_el()); } } -fn from_path(path_data: &[Path]) -> Vector { - const EPSILON: f64 = 1e-5; - - fn is_close(a: DVec2, b: DVec2) -> bool { - (a - b).length_squared() < EPSILON * EPSILON - } - +fn from_bez_paths<'a>(paths: impl Iterator) -> Vector { let mut all_subpaths = Vec::new(); - for path in path_data.iter().filter(|path| !path.is_empty()) { - let cubics: Vec<[DVec2; 4]> = path.iter().map(|segment| segment.to_cubic()).collect(); + for path in paths { + let cubics: Vec = path.segments().map(|segment| segment.to_cubic()).collect(); let mut manipulators_list = Vec::new(); let mut current_start = None; for (index, cubic) in cubics.iter().enumerate() { - let [start, handle1, handle2, end] = *cubic; + let d = |p: Point| DVec2::new(p.x, p.y); + let [start, handle1, handle2, end] = [d(cubic.p0), d(cubic.p1), d(cubic.p2), d(cubic.p3)]; - if current_start.is_none() || !is_close(start, current_start.unwrap()) { + if current_start.is_none() { // Start a new subpath if !manipulators_list.is_empty() { all_subpaths.push(Subpath::new(std::mem::take(&mut manipulators_list), true)); @@ -416,10 +326,6 @@ fn from_path(path_data: &[Path]) -> Vector { type Path = Vec; -fn boolean_union(a: Path, b: Path) -> Vec { - path_bool(a, b, PathBooleanOperation::Union) -} - fn path_bool(a: Path, b: Path, op: PathBooleanOperation) -> Vec { match path_bool::path_boolean(&a, FillRule::NonZero, &b, FillRule::NonZero, op) { Ok(results) => results, @@ -432,10 +338,6 @@ fn path_bool(a: Path, b: Path, op: PathBooleanOperation) -> Vec { } } -fn boolean_subtract(a: Path, b: Path) -> Vec { - path_bool(a, b, PathBooleanOperation::Difference) -} - pub fn boolean_intersect(a: Path, b: Path) -> Vec { path_bool(a, b, PathBooleanOperation::Intersection) } From 6fa4b39305a284b39fc2eb37620ddad9406799e9 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Mon, 19 Jan 2026 16:26:03 -0600 Subject: [PATCH 2/7] Make it not crash --- node-graph/nodes/path-bool/src/lib.rs | 100 ++++++++++++++------------ 1 file changed, 54 insertions(+), 46 deletions(-) diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 7b0d14273b..fc015664d0 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -76,33 +76,28 @@ struct WindingNumber { } impl linesweeper::topology::WindingNumber for WindingNumber { - type Tag = usize; + type Tag = (usize, usize); - fn single(tag: usize, positive: bool) -> Self { - let mut elems = SmallVec::with_capacity(tag); - let sign = if positive { 1 } else { -1 }; - for _ in 0..tag { - elems.push(0); - } - elems.push(sign); + fn single((tag, out_of): (usize, usize), positive: bool) -> Self { + let mut elems = SmallVec::with_capacity(out_of); + elems.resize(out_of, 0); + elems[tag] = if positive { 1 } else { -1 }; Self { elems } } } impl std::ops::AddAssign for WindingNumber { fn add_assign(&mut self, rhs: Self) { - if self.elems.len() < rhs.elems.len() { - self.elems.resize(rhs.elems.len(), 0); + if rhs.elems.is_empty() { + return; } - - for (me, them) in self.elems.iter_mut().zip(&rhs.elems) { - *me += *them; + if self.elems.is_empty() { + self.elems = rhs.elems; + } else { + for (me, them) in self.elems.iter_mut().zip(&rhs.elems) { + *me += *them; + } } - - // Removing trailing zeros normalizes the representation so that the derived - // PartialEq works. (Alternatively, we could write our own PartialEq.) - let trailing_zeros = self.elems.iter().rev().take_while(|w| **w == 0).count(); - self.elems.truncate(self.elems.len() - trailing_zeros); } } @@ -117,50 +112,63 @@ impl std::ops::Add for WindingNumber { impl WindingNumber { fn is_inside(&self, op: BooleanOperation) -> bool { + let is_in = |w: &i16| *w != 0; + let is_out = |w: &i16| *w == 0; match op { - BooleanOperation::Union => self.elems.iter().any(|w| *w != 0), - BooleanOperation::SubtractFront => self.elems.first().is_some_and(|w| *w != 0) && self.elems.iter().skip(1).all(|w| *w == 0), - BooleanOperation::SubtractBack => self.elems.last().is_some_and(|w| *w != 0) && self.elems.iter().rev().skip(1).all(|w| *w == 0), - BooleanOperation::Intersect => self.elems.iter().all(|w| *w != 0), - BooleanOperation::Difference => self.elems.iter().any(|w| *w != 0) && !self.elems.iter().all(|w| *w != 0), + BooleanOperation::Union => self.elems.iter().any(is_in), + BooleanOperation::SubtractFront => self.elems.first().is_some_and(is_in) && self.elems.iter().skip(1).all(is_out), + BooleanOperation::SubtractBack => self.elems.last().is_some_and(is_in) && self.elems.iter().rev().skip(1).all(is_out), + BooleanOperation::Intersect => !self.elems.is_empty() && self.elems.iter().all(is_in), + BooleanOperation::Difference => self.elems.iter().any(is_in) && !self.elems.iter().all(is_in), } } } -fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator>, boolean_operation: BooleanOperation) -> Table { +fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator> + Clone, boolean_operation: BooleanOperation) -> Table { const EPSILON: f64 = 1e-5; let mut table = Table::new(); - let mut alpha_blending = None; - let mut source_node_id = None; let mut paths = Vec::new(); - for v in vector { - if alpha_blending.is_none() { - alpha_blending = Some(*v.alpha_blending); - source_node_id = Some(*v.source_node_id); - } + let mut row = TableRow::::default(); + + // How should we style the result? The previous implementation copied it + // from either the first or the last row, depending on the boolean op. + // We copy that behaviour, although I'm not sure what motivated it. + let copy_from = if matches!(boolean_operation, BooleanOperation::SubtractFront) { + vector.clone().next() + } else { + vector.clone().next_back() + }; + if let Some(copy_from) = copy_from { + row.alpha_blending = *copy_from.alpha_blending; + row.source_node_id = *copy_from.source_node_id; + row.element.style = copy_from.element.style.clone(); + row.element.upstream_data = copy_from.element.upstream_data.clone(); + } + for v in vector { paths.push(to_bez_path(v.element, *v.transform)); } - log::warn!("boolean op {boolean_operation:?} on paths:"); + log::debug!("boolean op {boolean_operation:?} on paths:"); for p in &paths { - log::warn!("{}", p.to_svg()); + log::debug!("{}", p.to_svg()); } - // unwrap: Topology::from_paths only errors on a non-closed path, and our paths are closed by construction. - let top = Topology::::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, idx)), EPSILON).unwrap(); + // unwrap: Topology::from_paths only errors on a non-closed path, and our paths are closed because `push_subpath` + // always makes closed subpaths. + let top = Topology::::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, (idx, paths.len()))), EPSILON).unwrap(); let contours = top.contours(|w| w.is_inside(boolean_operation)); - log::warn!("boolean op output paths:"); + log::debug!("boolean op output paths:"); for c in contours.contours() { - log::warn!("{}", c.path.to_svg()); + log::debug!("{}", c.path.to_svg()); } - table.push(TableRow { - element: from_bez_paths(contours.contours().map(|c| &c.path)), - transform: DAffine2::IDENTITY, - alpha_blending: alpha_blending.unwrap_or_default(), - source_node_id: source_node_id.unwrap_or_default(), - }); + + for subpath in from_bez_paths(contours.contours().map(|c| &c.path)) { + row.element.append_subpath(subpath, false); + } + + table.push(row); table } @@ -273,7 +281,7 @@ fn push_subpath(path: &mut BezPath, subpath: &Subpath, transform: DAffi let transform = Affine::new(transform.to_cols_array()); let mut first = true; - for seg in subpath.iter() { + for seg in subpath.iter_closed() { if first { first = false; path.move_to(transform * seg.start()); @@ -282,7 +290,7 @@ fn push_subpath(path: &mut BezPath, subpath: &Subpath, transform: DAffi } } -fn from_bez_paths<'a>(paths: impl Iterator) -> Vector { +fn from_bez_paths<'a>(paths: impl Iterator) -> Vec> { let mut all_subpaths = Vec::new(); for path in paths { @@ -321,7 +329,7 @@ fn from_bez_paths<'a>(paths: impl Iterator) -> Vector { } } - Vector::from_subpaths(all_subpaths, false) + all_subpaths } type Path = Vec; From 7630f09e8e24c568ea5471d1b102fb86c363b884 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Tue, 20 Jan 2026 22:31:50 -0600 Subject: [PATCH 3/7] Try to remove more path_bool --- Cargo.lock | 1 - .../document/document_message_handler.rs | 55 ++++++------------- node-graph/nodes/path-bool/Cargo.toml | 1 - node-graph/nodes/path-bool/src/lib.rs | 19 ++----- 4 files changed, 22 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abb7d2b1b0..8df5902e22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4317,7 +4317,6 @@ dependencies = [ "linesweeper", "log", "node-macro", - "path-bool", "serde", "smallvec", "tsify", diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index cf31a4337a..c234e0a743 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -29,7 +29,7 @@ use glam::{DAffine2, DVec2, IVec2}; use graph_craft::document::value::TaggedValue; use graph_craft::document::{NodeId, NodeInput, NodeNetwork, OldNodeNetwork}; use graphene_std::math::quad::Quad; -use graphene_std::path_bool::{boolean_intersect, path_bool_lib}; +use graphene_std::path_bool::boolean_intersect; use graphene_std::raster::BlendMode; use graphene_std::raster_types::Raster; use graphene_std::render_node::wgpu_available; @@ -37,9 +37,9 @@ use graphene_std::subpath::Subpath; use graphene_std::table::Table; use graphene_std::vector::PointId; use graphene_std::vector::click_target::{ClickTarget, ClickTargetType}; -use graphene_std::vector::misc::{dvec2_to_point, point_to_dvec2}; +use graphene_std::vector::misc::dvec2_to_point; use graphene_std::vector::style::RenderMode; -use kurbo::{Affine, CubicBez, Line, ParamCurve, PathSeg, QuadBez}; +use kurbo::{Affine, BezPath, Line, PathSeg}; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -2982,7 +2982,7 @@ fn default_document_network_interface() -> NodeNetworkInterface { enum XRayTarget { Point(DVec2), Quad(Quad), - Path(Vec), + Path(BezPath), Polygon(Subpath), } @@ -3000,17 +3000,12 @@ pub struct ClickXRayIter<'a> { parent_targets: Vec<(LayerNodeIdentifier, XRayTarget)>, } -fn quad_to_path_lib_segments(quad: Quad) -> Vec { - quad.all_edges().into_iter().map(|[start, end]| path_bool_lib::PathSegment::Line(start, end)).collect() +fn quad_to_kurbo(quad: Quad) -> BezPath { + BezPath::from_path_segments(quad.all_edges().into_iter().map(|[start, end]| PathSeg::Line(Line::new(dvec2_to_point(start), dvec2_to_point(end))))) } -fn click_targets_to_path_lib_segments<'a>(click_targets: impl Iterator, transform: DAffine2) -> Vec { - let segment = |bezier: PathSeg| match bezier { - PathSeg::Line(line) => path_bool_lib::PathSegment::Line(point_to_dvec2(line.p0), point_to_dvec2(line.p1)), - PathSeg::Quad(quad_bez) => path_bool_lib::PathSegment::Quadratic(point_to_dvec2(quad_bez.p0), point_to_dvec2(quad_bez.p1), point_to_dvec2(quad_bez.p2)), - PathSeg::Cubic(cubic_bez) => path_bool_lib::PathSegment::Cubic(point_to_dvec2(cubic_bez.p0), point_to_dvec2(cubic_bez.p1), point_to_dvec2(cubic_bez.p2), point_to_dvec2(cubic_bez.p3)), - }; - click_targets +fn click_targets_to_kurbo<'a>(click_targets: impl Iterator, transform: DAffine2) -> BezPath { + let segments = click_targets .filter_map(|target| { if let ClickTargetType::Subpath(subpath) = target.target_type() { Some(subpath.iter()) @@ -3019,8 +3014,8 @@ fn click_targets_to_path_lib_segments<'a>(click_targets: impl Iterator ClickXRayIter<'a> { @@ -3041,22 +3036,8 @@ impl<'a> ClickXRayIter<'a> { } /// Handles the checking of the layer where the target is a rect or path - fn check_layer_area_target( - &mut self, - click_targets: Option<&[Arc]>, - clip: bool, - layer: LayerNodeIdentifier, - path: Vec, - transform: DAffine2, - ) -> XRayResult { - // Convert back to Kurbo types for intersections - let segment = |bezier: &path_bool_lib::PathSegment| match *bezier { - path_bool_lib::PathSegment::Line(start, end) => PathSeg::Line(Line::new(dvec2_to_point(start), dvec2_to_point(end))), - path_bool_lib::PathSegment::Cubic(start, h1, h2, end) => PathSeg::Cubic(CubicBez::new(dvec2_to_point(start), dvec2_to_point(h1), dvec2_to_point(h2), dvec2_to_point(end))), - path_bool_lib::PathSegment::Quadratic(start, h1, end) => PathSeg::Quad(QuadBez::new(dvec2_to_point(start), dvec2_to_point(h1), dvec2_to_point(end))), - path_bool_lib::PathSegment::Arc(_, _, _, _, _, _, _) => unimplemented!(), - }; - let get_clip = || path.iter().map(segment); + fn check_layer_area_target(&mut self, click_targets: Option<&[Arc]>, clip: bool, layer: LayerNodeIdentifier, path: BezPath, transform: DAffine2) -> XRayResult { + let get_clip = || path.segments(); let intersects = click_targets.is_some_and(|targets| targets.iter().any(|target| target.intersect_path(get_clip, transform))); let clicked = intersects; @@ -3065,8 +3046,9 @@ impl<'a> ClickXRayIter<'a> { // In the case of a clip path where the area partially intersects, it is necessary to do a boolean operation. // We do this on this using the target area to reduce computation (as the target area is usually very simple). if clip && intersects { - let clip_path = click_targets_to_path_lib_segments(click_targets.iter().flat_map(|x| x.iter()).map(|x| x.as_ref()), transform); - let subtracted = boolean_intersect(path, clip_path).into_iter().flatten().collect::>(); + let clip_path: BezPath = click_targets_to_kurbo(click_targets.iter().flat_map(|x| x.iter()).map(|x| x.as_ref()), transform); + let intersection = boolean_intersect(&path, &clip_path); + let subtracted = BezPath::from_path_segments(intersection.iter().flat_map(|p| p.segments())); if subtracted.is_empty() { use_children = false; } else { @@ -3099,13 +3081,10 @@ impl<'a> ClickXRayIter<'a> { use_children: !clip || intersects, } } - XRayTarget::Quad(quad) => self.check_layer_area_target(click_targets, clip, layer, quad_to_path_lib_segments(*quad), transform), + XRayTarget::Quad(quad) => self.check_layer_area_target(click_targets, clip, layer, quad_to_kurbo(*quad), transform), XRayTarget::Path(path) => self.check_layer_area_target(click_targets, clip, layer, path.clone(), transform), XRayTarget::Polygon(polygon) => { - let polygon = polygon - .iter_closed() - .map(|line| path_bool_lib::PathSegment::Line(point_to_dvec2(line.start()), point_to_dvec2(line.end()))) - .collect(); + let polygon = BezPath::from_path_segments(polygon.iter_closed()); self.check_layer_area_target(click_targets, clip, layer, polygon, transform) } } diff --git a/node-graph/nodes/path-bool/Cargo.toml b/node-graph/nodes/path-bool/Cargo.toml index 5ee2e6e774..54010339e2 100644 --- a/node-graph/nodes/path-bool/Cargo.toml +++ b/node-graph/nodes/path-bool/Cargo.toml @@ -18,7 +18,6 @@ node-macro = { workspace = true } glam = { workspace = true } linesweeper = { workspace = true } log = { workspace = true } -path-bool = { workspace = true } serde = { workspace = true } smallvec = { workspace = true } vector-types = { workspace = true } diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index fc015664d0..6dacfe8863 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -8,8 +8,7 @@ use graphic_types::vector_types::vector::algorithms::merge_by_distance::MergeByD use graphic_types::vector_types::vector::style::Fill; use graphic_types::{Graphic, Vector}; use linesweeper::topology::Topology; -pub use path_bool as path_bool_lib; -use path_bool::{FillRule, PathBooleanOperation}; +use linesweeper::{BinaryOp, FillRule, binary_op}; use smallvec::SmallVec; use vector_types::kurbo::{Affine, BezPath, CubicBez, ParamCurve, Point}; @@ -332,20 +331,12 @@ fn from_bez_paths<'a>(paths: impl Iterator) -> Vec; - -fn path_bool(a: Path, b: Path, op: PathBooleanOperation) -> Vec { - match path_bool::path_boolean(&a, FillRule::NonZero, &b, FillRule::NonZero, op) { - Ok(results) => results, +pub fn boolean_intersect(a: &BezPath, b: &BezPath) -> Vec { + match binary_op(a, b, FillRule::NonZero, BinaryOp::Intersection) { + Ok(contours) => contours.contours().map(|c| c.path.clone()).collect(), Err(e) => { - let a_path = path_bool::path_to_path_data(&a, 0.001); - let b_path = path_bool::path_to_path_data(&b, 0.001); - log::error!("Boolean error {e:?} encountered while processing {a_path}\n {op:?}\n {b_path}"); + log::error!("boolean op failed: {e}"); Vec::new() } } } - -pub fn boolean_intersect(a: Path, b: Path) -> Vec { - path_bool(a, b, PathBooleanOperation::Intersection) -} From febd175e9a0848917e79fe9e69ac3d7dba0dbbb2 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Sat, 24 Jan 2026 14:38:42 -0600 Subject: [PATCH 4/7] Add quantization --- node-graph/nodes/path-bool/src/lib.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 6dacfe8863..7656ebf823 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -10,7 +10,7 @@ use graphic_types::{Graphic, Vector}; use linesweeper::topology::Topology; use linesweeper::{BinaryOp, FillRule, binary_op}; use smallvec::SmallVec; -use vector_types::kurbo::{Affine, BezPath, CubicBez, ParamCurve, Point}; +use vector_types::kurbo::{Affine, BezPath, CubicBez, Line, ParamCurve, PathSeg, Point, QuadBez}; // TODO: Fix boolean ops to work by removing .transform() and .one_instance_*() calls, // TODO: since before we used a Vec of single-row tables and now we use a single table @@ -268,6 +268,23 @@ fn flatten_vector(graphic_table: &Table) -> Table { .collect() } +// I don't think this quantization should be necessary, but +// - it imitates the previous behavior, and +// - without it, the oak leaf in changing seasons is funky, since without +// quantization the top and bottom points don't quite line up vertically. +fn quantize_segment(seg: PathSeg) -> PathSeg { + const QUANTIZE_EPS: f64 = 1e-8; + fn q(p: Point) -> Point { + Point::new((p.x / QUANTIZE_EPS).round() * QUANTIZE_EPS, (p.y / QUANTIZE_EPS).round() * QUANTIZE_EPS) + } + + match seg { + PathSeg::Line(s) => PathSeg::Line(Line::new(q(s.p0), q(s.p1))), + PathSeg::Quad(s) => PathSeg::Quad(QuadBez::new(q(s.p0), q(s.p1), q(s.p2))), + PathSeg::Cubic(s) => PathSeg::Cubic(CubicBez::new(q(s.p0), q(s.p1), q(s.p2), q(s.p3))), + } +} + fn to_bez_path(vector: &Vector, transform: DAffine2) -> BezPath { let mut path = BezPath::new(); for subpath in vector.stroke_bezier_paths() { @@ -283,9 +300,9 @@ fn push_subpath(path: &mut BezPath, subpath: &Subpath, transform: DAffi for seg in subpath.iter_closed() { if first { first = false; - path.move_to(transform * seg.start()); + path.move_to(quantize_segment(transform * seg).start()); } - path.push(transform * seg.as_path_el()); + path.push(quantize_segment(transform * seg).as_path_el()); } } From 5468dc740a7b99cc9b94bc1abffe199297e0e0dd Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 24 Feb 2026 03:00:23 -0800 Subject: [PATCH 5/7] Update to latest --- Cargo.lock | 41 ++++++++++++++++------------------------- Cargo.toml | 2 +- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8df5902e22..a23863c93f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1139,7 +1139,7 @@ dependencies = [ "dyn-any", "glam", "image", - "kurbo 0.13.0", + "kurbo", "log", "lyon_geom", "no-std-types", @@ -2531,7 +2531,7 @@ dependencies = [ "image", "interpreted-executor", "js-sys", - "kurbo 0.13.0", + "kurbo", "log", "num_enum", "once_cell", @@ -3305,18 +3305,6 @@ dependencies = [ "libc", ] -[[package]] -name = "kurbo" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce9729cc38c18d86123ab736fd2e7151763ba226ac2490ec092d1dd148825e32" -dependencies = [ - "arrayvec", - "euclid", - "serde", - "smallvec", -] - [[package]] name = "kurbo" version = "0.13.0" @@ -3408,12 +3396,15 @@ checksum = "d4a5ff6bcca6c4867b1c4fd4ef63e4db7436ef363e0ad7531d1558856bae64f4" [[package]] name = "linesweeper" -version = "0.1.2" -source = "git+https://github.com/jneem/linesweeper?rev=db9809e698c0b7c46666987df33b43f8ec3c2fde#db9809e698c0b7c46666987df33b43f8ec3c2fde" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8421b276e96af0ace5f3d8d2d165d0dea07fe764d2fe94ec06bb1acaf8a1e759" dependencies = [ "arrayvec", - "kurbo 0.12.0", + "kurbo", "polycool", + "rustc-hash 2.1.1", + "smallvec", ] [[package]] @@ -4337,7 +4328,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a2b6aadb221872732e87d465213e9be5af2849b0e8cc5300a8ba98fffa2e00a" dependencies = [ "color", - "kurbo 0.13.0", + "kurbo", "linebender_resource_handle", "smallvec", ] @@ -4933,7 +4924,7 @@ dependencies = [ "futures", "glam", "image", - "kurbo 0.13.0", + "kurbo", "ndarray", "no-std-types", "node-macro", @@ -5168,7 +5159,7 @@ dependencies = [ "dyn-any", "glam", "graphic-types", - "kurbo 0.13.0", + "kurbo", "log", "num-traits", "serde", @@ -5186,7 +5177,7 @@ dependencies = [ "glam", "graphene-core", "graphic-types", - "kurbo 0.13.0", + "kurbo", "log", "node-macro", "raster-types", @@ -6117,7 +6108,7 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "695b5790b3131dafa99b3bbfd25a216edb3d216dad9ca208d4657bfb8f2abc3d" dependencies = [ - "kurbo 0.13.0", + "kurbo", "siphasher", ] @@ -6889,7 +6880,7 @@ dependencies = [ "flate2", "fontdb", "imagesize", - "kurbo 0.13.0", + "kurbo", "log", "pico-args", "roxmltree 0.21.1", @@ -6951,7 +6942,7 @@ dependencies = [ "glam", "graphene-core", "graphic-types", - "kurbo 0.13.0", + "kurbo", "log", "node-macro", "qrcodegen", @@ -6975,7 +6966,7 @@ dependencies = [ "dyn-any", "fixedbitset", "glam", - "kurbo 0.13.0", + "kurbo", "log", "lyon_geom", "node-macro", diff --git a/Cargo.toml b/Cargo.toml index 1539a816b0..9c58c7cd19 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -212,7 +212,7 @@ cargo-gpu = { git = "https://github.com/Firestar99/cargo-gpu", rev = "3952a22d16 qrcodegen = "1.8" lzma-rust2 = { version = "0.16", default-features = false, features = ["std", "encoder", "optimization", "xz"] } scraper = "0.25" -linesweeper = { git = "https://github.com/jneem/linesweeper", rev = "db9809e698c0b7c46666987df33b43f8ec3c2fde" } +linesweeper = "0.3" smallvec = "1.13.2" [workspace.lints.rust] From 8139307d1e19d20297b66f7abd2416ac869b5b54 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 10 Mar 2026 19:32:35 -0700 Subject: [PATCH 6/7] Nits --- .../document/document_message_handler.rs | 2 +- node-graph/nodes/path-bool/src/lib.rs | 31 +++++-------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index c234e0a743..e9c8ba2f11 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -3046,7 +3046,7 @@ impl<'a> ClickXRayIter<'a> { // In the case of a clip path where the area partially intersects, it is necessary to do a boolean operation. // We do this on this using the target area to reduce computation (as the target area is usually very simple). if clip && intersects { - let clip_path: BezPath = click_targets_to_kurbo(click_targets.iter().flat_map(|x| x.iter()).map(|x| x.as_ref()), transform); + let clip_path = click_targets_to_kurbo(click_targets.iter().flat_map(|x| x.iter()).map(|x| x.as_ref()), transform); let intersection = boolean_intersect(&path, &clip_path); let subtracted = BezPath::from_path_segments(intersection.iter().flat_map(|p| p.segments())); if subtracted.is_empty() { diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index 7656ebf823..f60cf45e07 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -129,9 +129,6 @@ fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator::default(); - // How should we style the result? The previous implementation copied it - // from either the first or the last row, depending on the boolean op. - // We copy that behaviour, although I'm not sure what motivated it. let copy_from = if matches!(boolean_operation, BooleanOperation::SubtractFront) { vector.clone().next() } else { @@ -144,24 +141,13 @@ fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, (idx, paths.len()))), EPSILON).unwrap(); - let contours = top.contours(|w| w.is_inside(boolean_operation)); - - log::debug!("boolean op output paths:"); - for c in contours.contours() { - log::debug!("{}", c.path.to_svg()); - } + let contours = top.contours(|winding| winding.is_inside(boolean_operation)); for subpath in from_bez_paths(contours.contours().map(|c| &c.path)) { row.element.append_subpath(subpath, false); @@ -268,10 +254,9 @@ fn flatten_vector(graphic_table: &Table) -> Table { .collect() } -// I don't think this quantization should be necessary, but -// - it imitates the previous behavior, and -// - without it, the oak leaf in changing seasons is funky, since without -// quantization the top and bottom points don't quite line up vertically. +// This quantization should potentially be removed since it's not conceptually necessary, +// but without it, the oak leaf in the Changing Seasons demo artwork is funky because +// quantization is needed for the top and bottom points to line up vertically. fn quantize_segment(seg: PathSeg) -> PathSeg { const QUANTIZE_EPS: f64 = 1e-8; fn q(p: Point) -> Point { @@ -352,7 +337,7 @@ pub fn boolean_intersect(a: &BezPath, b: &BezPath) -> Vec { match binary_op(a, b, FillRule::NonZero, BinaryOp::Intersection) { Ok(contours) => contours.contours().map(|c| c.path.clone()).collect(), Err(e) => { - log::error!("boolean op failed: {e}"); + log::error!("Boolean Operation failed: {e}"); Vec::new() } } From 66317947089edee5371ee4592f3cb6783941d694 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 10 Mar 2026 20:16:39 -0700 Subject: [PATCH 7/7] Code review --- node-graph/nodes/path-bool/src/lib.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/node-graph/nodes/path-bool/src/lib.rs b/node-graph/nodes/path-bool/src/lib.rs index f60cf45e07..890a9337a8 100644 --- a/node-graph/nodes/path-bool/src/lib.rs +++ b/node-graph/nodes/path-bool/src/lib.rs @@ -145,8 +145,14 @@ fn boolean_operation_on_vector_table<'a>(vector: impl DoubleEndedIterator::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, (idx, paths.len()))), EPSILON).unwrap(); + let top = match Topology::::from_paths(paths.iter().enumerate().map(|(idx, path)| (path, (idx, paths.len()))), EPSILON) { + Ok(top) => top, + Err(e) => { + log::error!("Boolean operation failed while building topology: {e}"); + table.push(row); + return table; + } + }; let contours = top.contours(|winding| winding.is_inside(boolean_operation)); for subpath in from_bez_paths(contours.contours().map(|c| &c.path)) { @@ -283,12 +289,14 @@ fn push_subpath(path: &mut BezPath, subpath: &Subpath, transform: DAffi let mut first = true; for seg in subpath.iter_closed() { + let quantized = quantize_segment(transform * seg); if first { first = false; - path.move_to(quantize_segment(transform * seg).start()); + path.move_to(quantized.start()); } - path.push(quantize_segment(transform * seg).as_path_el()); + path.push(quantized.as_path_el()); } + path.close_path(); } fn from_bez_paths<'a>(paths: impl Iterator) -> Vec> { @@ -304,10 +312,6 @@ fn from_bez_paths<'a>(paths: impl Iterator) -> Vec Vec { match binary_op(a, b, FillRule::NonZero, BinaryOp::Intersection) { Ok(contours) => contours.contours().map(|c| c.path.clone()).collect(), Err(e) => { - log::error!("Boolean Operation failed: {e}"); + log::error!("Boolean Operation failed (a: {} segments, b: {} segments): {e}", a.segments().count(), b.segments().count()); Vec::new() } }