Refactor the 'Bounding Box' node to use Kurbo instead of Bezier-rs (#2662)

* use kurbo's default accuracy constant

* fix append_bezpath() method

* refactor bounding box node

* fix append bezpath implementation.

* comments

---------

Co-authored-by: Keavon Chambers <[email protected]>

Author: indierusty

node-graph/gcore/src/vector/algorithms/bezpath_algorithms.rs

@@ -1,12 +1,7 @@
 use super::poisson_disk::poisson_disk_sample;
 use crate::vector::misc::dvec2_to_point;
 use glam::DVec2;
-use kurbo::{BezPath, Line, ParamCurve, ParamCurveDeriv, PathSeg, Point, Rect, Shape};
-
-/// Accuracy to find the position on [kurbo::Bezpath].
-const POSITION_ACCURACY: f64 = 1e-5;
-/// Accuracy to find the length of the [kurbo::PathSeg].
-pub const PERIMETER_ACCURACY: f64 = 1e-5;
+use kurbo::{BezPath, DEFAULT_ACCURACY, Line, ParamCurve, ParamCurveDeriv, PathSeg, Point, Rect, Shape};
 
 pub fn position_on_bezpath(bezpath: &BezPath, t: f64, euclidian: bool, segments_length: Option<&[f64]>) -> Point {
 	let (segment_index, t) = t_value_to_parametric(bezpath, t, euclidian, segments_length);
@@ -79,7 +74,7 @@ pub fn sample_points_on_bezpath(bezpath: BezPath, spacing: f64, start_offset: f6
 		let t = (next_length / next_segment_length).clamp(0., 1.);
 
 		let segment = bezpath.get_seg(next_segment_index + 1).unwrap();
-		let t = eval_pathseg_euclidean(segment, t, POSITION_ACCURACY);
+		let t = eval_pathseg_euclidean(segment, t, DEFAULT_ACCURACY);
 		let point = segment.eval(t);
 
 		if sample_bezpath.elements().is_empty() {
@@ -96,7 +91,7 @@ pub fn t_value_to_parametric(bezpath: &BezPath, t: f64, euclidian: bool, segment
 	if euclidian {
 		let (segment_index, t) = bezpath_t_value_to_parametric(bezpath, BezPathTValue::GlobalEuclidean(t), segments_length);
 		let segment = bezpath.get_seg(segment_index + 1).unwrap();
-		return (segment_index, eval_pathseg_euclidean(segment, t, POSITION_ACCURACY));
+		return (segment_index, eval_pathseg_euclidean(segment, t, DEFAULT_ACCURACY));
 	}
 	bezpath_t_value_to_parametric(bezpath, BezPathTValue::GlobalParametric(t), segments_length)
 }
@@ -164,7 +159,7 @@ fn bezpath_t_value_to_parametric(bezpath: &kurbo::BezPath, t: BezPathTValue, pre
 			let segments_length = if let Some(segments_length) = precomputed_segments_length {
 				segments_length
 			} else {
-				computed_segments_length = bezpath.segments().map(|segment| segment.perimeter(PERIMETER_ACCURACY)).collect::<Vec<f64>>();
+				computed_segments_length = bezpath.segments().map(|segment| segment.perimeter(DEFAULT_ACCURACY)).collect::<Vec<f64>>();
 				computed_segments_length.as_slice()
 			};
 

node-graph/gcore/src/vector/vector_data.rs

@@ -12,7 +12,7 @@ use core::borrow::Borrow;
 use dyn_any::DynAny;
 use glam::{DAffine2, DVec2};
 pub use indexed::VectorDataIndex;
-use kurbo::{Affine, Shape};
+use kurbo::{Affine, Rect, Shape};
 pub use modification::*;
 use std::collections::HashMap;
 
@@ -194,6 +194,11 @@ impl VectorData {
 		vector_data
 	}
 
+	/// Compute the bounding boxes of the bezpaths without any transform
+	pub fn bounding_box_rect(&self) -> Option<Rect> {
+		self.bounding_box_with_transform_rect(DAffine2::IDENTITY)
+	}
+
 	pub fn close_subpaths(&mut self) {
 		let segments_to_add: Vec<_> = self
 			.stroke_bezier_paths()
@@ -213,27 +218,25 @@ impl VectorData {
 
 	/// Compute the bounding boxes of the subpaths without any transform
 	pub fn bounding_box(&self) -> Option<[DVec2; 2]> {
-		self.bounding_box_with_transform(DAffine2::IDENTITY)
+		self.bounding_box_with_transform_rect(DAffine2::IDENTITY)
+			.map(|rect| [DVec2::new(rect.x0, rect.y0), DVec2::new(rect.x1, rect.y1)])
 	}
 
 	/// Compute the bounding boxes of the subpaths with the specified transform
 	pub fn bounding_box_with_transform(&self, transform: DAffine2) -> Option<[DVec2; 2]> {
-		let combine = |[a_min, a_max]: [DVec2; 2], [b_min, b_max]: [DVec2; 2]| [a_min.min(b_min), a_max.max(b_max)];
-
-		let anchor_bounds = self
-			.point_domain
-			.positions()
-			.iter()
-			.map(|&point| transform.transform_point2(point))
-			.map(|point| [point, point])
-			.reduce(combine);
-
-		let segment_bounds = self
-			.segment_bezier_iter()
-			.map(|(_, bezier, _, _)| bezier.apply_transformation(|point| transform.transform_point2(point)).bounding_box())
-			.reduce(combine);
+		self.bounding_box_with_transform_rect(transform)
+			.map(|rect| [DVec2::new(rect.x0, rect.y0), DVec2::new(rect.x1, rect.y1)])
+	}
 
-		anchor_bounds.iter().chain(segment_bounds.iter()).copied().reduce(combine)
+	/// Compute the bounding boxes of the bezpaths with the specified transform
+	pub fn bounding_box_with_transform_rect(&self, transform: DAffine2) -> Option<Rect> {
+		let combine = |r1: Rect, r2: Rect| r1.union(r2);
+		self.stroke_bezpath_iter()
+			.map(|mut bezpath| {
+				bezpath.apply_affine(Affine::new(transform.to_cols_array()));
+				bezpath.bounding_box()
+			})
+			.reduce(combine)
 	}
 
 	/// Calculate the corners of the bounding box but with a nonzero size.

node-graph/gcore/src/vector/vector_data/modification.rs

@@ -5,7 +5,7 @@ use crate::uuid::generate_uuid;
 use bezier_rs::BezierHandles;
 use core::hash::BuildHasher;
 use dyn_any::DynAny;
-use kurbo::{BezPath, PathEl};
+use kurbo::{BezPath, PathEl, Point};
 use std::collections::{HashMap, HashSet};
 
 /// Represents a procedural change to the [`PointDomain`] in [`VectorData`].
@@ -556,11 +556,12 @@ where
 }
 
 pub struct AppendBezpath<'a> {
+	first_point: Option<Point>,
+	last_point: Option<Point>,
 	first_point_index: Option<usize>,
 	last_point_index: Option<usize>,
 	first_segment_id: Option<SegmentId>,
 	last_segment_id: Option<SegmentId>,
-	next_handle: Option<BezierHandles>,
 	point_id: PointId,
 	segment_id: SegmentId,
 	vector_data: &'a mut VectorData,
@@ -569,79 +570,118 @@ pub struct AppendBezpath<'a> {
 impl<'a> AppendBezpath<'a> {
 	fn new(vector_data: &'a mut VectorData) -> Self {
 		Self {
+			first_point: None,
+			last_point: None,
 			first_point_index: None,
 			last_point_index: None,
 			first_segment_id: None,
 			last_segment_id: None,
-			next_handle: None,
 			point_id: vector_data.point_domain.next_id(),
 			segment_id: vector_data.segment_domain.next_id(),
 			vector_data,
 		}
 	}
 
-	fn append_path_element(&mut self, handle: BezierHandles, point: kurbo::Point, next_element: Option<&PathEl>) {
-		if let Some(PathEl::ClosePath) = next_element {
-			self.next_handle = Some(handle);
+	fn append_segment_and_close_path(&mut self, point: Point, handle: BezierHandles) {
+		let handle = if self.first_point.unwrap() != point {
+			// If the first point is not the same as the last point of the path then we append the segment
+			// with given handle and point and then close the path with linear handle.
+			self.append_segment(point, handle);
+			BezierHandles::Linear
 		} else {
-			let next_point_index = self.vector_data.point_domain.ids().len();
-			self.vector_data.point_domain.push(self.point_id.next_id(), point_to_dvec2(point));
+			// if the endpoints are the same then we close the path with given handle.
+			handle
+		};
+
+		// Create a new segment.
+		let next_segment_id = self.segment_id.next_id();
+		self.vector_data
+			.segment_domain
+			.push(next_segment_id, self.last_point_index.unwrap(), self.first_point_index.unwrap(), handle, StrokeId::ZERO);
+
+		// Create a new region.
+		let next_region_id = self.vector_data.region_domain.next_id();
+		let first_segment_id = self.first_segment_id.unwrap_or(next_segment_id);
+		let last_segment_id = next_segment_id;
+
+		self.vector_data.region_domain.push(next_region_id, first_segment_id..=last_segment_id, FillId::ZERO);
+	}
 
-			let next_segment_id = self.segment_id.next_id();
-			self.vector_data
-				.segment_domain
-				.push(self.segment_id.next_id(), self.last_point_index.unwrap(), next_point_index, handle, StrokeId::ZERO);
+	fn append_segment(&mut self, end_point: Point, handle: BezierHandles) {
+		// Append the point.
+		let next_point_index = self.vector_data.point_domain.ids().len();
+		let next_point_id = self.point_id.next_id();
 
-			self.last_point_index = Some(next_point_index);
-			self.first_segment_id = Some(self.first_segment_id.unwrap_or(next_segment_id));
-			self.last_segment_id = Some(next_segment_id);
-		}
+		self.vector_data.point_domain.push(next_point_id, point_to_dvec2(end_point));
+
+		// Append the segment.
+		let next_segment_id = self.segment_id.next_id();
+		self.vector_data
+			.segment_domain
+			.push(next_segment_id, self.last_point_index.unwrap(), next_point_index, handle, StrokeId::ZERO);
+
+		// Update the states.
+		self.last_point = Some(end_point);
+		self.last_point_index = Some(next_point_index);
+
+		self.first_segment_id = Some(self.first_segment_id.unwrap_or(next_segment_id));
+		self.last_segment_id = Some(next_segment_id);
 	}
 
-	pub fn append_bezpath(vector_data: &'a mut VectorData, bezpath: BezPath) {
-		let mut this = Self::new(vector_data);
+	fn append_first_point(&mut self, point: Point) {
+		self.first_point = Some(point);
+		self.last_point = Some(point);
 
-		let stroke_id = StrokeId::ZERO;
-		let fill_id = FillId::ZERO;
+		// Append the first point.
+		let next_point_index = self.vector_data.point_domain.ids().len();
+		self.vector_data.point_domain.push(self.point_id.next_id(), point_to_dvec2(point));
 
-		for i in 0..bezpath.elements().len() {
-			let current_element = bezpath.elements()[i];
-			let next_element = bezpath.elements().get(i + 1);
+		// Update the state.
+		self.first_point_index = Some(next_point_index);
+		self.last_point_index = Some(next_point_index);
+	}
 
-			match current_element {
-				kurbo::PathEl::MoveTo(point) => {
-					let next_point_index = this.vector_data.point_domain.ids().len();
-					this.vector_data.point_domain.push(this.point_id.next_id(), point_to_dvec2(point));
-					this.first_point_index = Some(next_point_index);
-					this.last_point_index = Some(next_point_index);
+	pub fn append_bezpath(vector_data: &'a mut VectorData, bezpath: BezPath) {
+		let mut this = Self::new(vector_data);
+		let mut elements = bezpath.elements().iter().peekable();
+
+		while let Some(element) = elements.next() {
+			let close_path = elements.peek().is_some_and(|elm| **elm == PathEl::ClosePath);
+
+			match *element {
+				PathEl::MoveTo(point) => this.append_first_point(point),
+				PathEl::LineTo(point) => {
+					let handle = BezierHandles::Linear;
+					if close_path {
+						this.append_segment_and_close_path(point, handle);
+					} else {
+						this.append_segment(point, handle);
+					}
 				}
-				kurbo::PathEl::ClosePath => match (this.first_point_index, this.last_point_index) {
-					(Some(first_point_index), Some(last_point_index)) => {
-						let next_segment_id = this.segment_id.next_id();
-						this.vector_data
-							.segment_domain
-							.push(next_segment_id, last_point_index, first_point_index, this.next_handle.unwrap_or(BezierHandles::Linear), stroke_id);
-
-						let next_region_id = this.vector_data.region_domain.next_id();
-						// In case there is only one anchor point.
-						let first_segment_id = this.first_segment_id.unwrap_or(next_segment_id);
-
-						this.vector_data.region_domain.push(next_region_id, first_segment_id..=next_segment_id, fill_id);
+				PathEl::QuadTo(point, point1) => {
+					let handle = BezierHandles::Quadratic { handle: point_to_dvec2(point) };
+					if close_path {
+						this.append_segment_and_close_path(point1, handle);
+					} else {
+						this.append_segment(point1, handle);
 					}
-					_ => {
-						error!("Empty bezpath cannot be closed.")
+				}
+				PathEl::CurveTo(point, point1, point2) => {
+					let handle = BezierHandles::Cubic {
+						handle_start: point_to_dvec2(point),
+						handle_end: point_to_dvec2(point1),
+					};
+
+					if close_path {
+						this.append_segment_and_close_path(point2, handle);
+					} else {
+						this.append_segment(point2, handle);
 					}
-				},
-				kurbo::PathEl::LineTo(point) => this.append_path_element(BezierHandles::Linear, point, next_element),
-				kurbo::PathEl::QuadTo(handle, point) => this.append_path_element(BezierHandles::Quadratic { handle: point_to_dvec2(handle) }, point, next_element),
-				kurbo::PathEl::CurveTo(handle_start, handle_end, point) => this.append_path_element(
-					BezierHandles::Cubic {
-						handle_start: point_to_dvec2(handle_start),
-						handle_end: point_to_dvec2(handle_end),
-					},
-					point,
-					next_element,
-				),
+				}
+				PathEl::ClosePath => {
+					// Already handled using `append_segment_and_close_path()`;
+					break;
+				}
 			}
 		}
 	}

node-graph/gcore/src/vector/vector_nodes.rs

@@ -1,4 +1,4 @@
-use super::algorithms::bezpath_algorithms::{self, PERIMETER_ACCURACY, position_on_bezpath, sample_points_on_bezpath, tangent_on_bezpath};
+use super::algorithms::bezpath_algorithms::{self, position_on_bezpath, sample_points_on_bezpath, tangent_on_bezpath};
 use super::algorithms::offset_subpath::offset_subpath;
 use super::misc::{CentroidType, point_to_dvec2};
 use super::style::{Fill, Gradient, GradientStops, Stroke};
@@ -16,7 +16,7 @@ use bezier_rs::{Join, ManipulatorGroup, Subpath, SubpathTValue};
 use core::f64::consts::PI;
 use core::hash::{Hash, Hasher};
 use glam::{DAffine2, DVec2};
-use kurbo::{Affine, BezPath, Shape};
+use kurbo::{Affine, BezPath, DEFAULT_ACCURACY, Shape};
 use rand::{Rng, SeedableRng};
 use std::collections::hash_map::DefaultHasher;
 use std::f64::consts::TAU;
@@ -1028,9 +1028,14 @@ async fn bounding_box(_: impl Ctx, vector_data: VectorDataTable) -> VectorDataTa
 		let vector_data = vector_data_instance.instance;
 
 		let mut result = vector_data
-			.bounding_box()
-			.map(|bounding_box| VectorData::from_subpath(Subpath::new_rect(bounding_box[0], bounding_box[1])))
+			.bounding_box_rect()
+			.map(|bbox| {
+				let mut vector_data = VectorData::default();
+				vector_data.append_bezpath(bbox.to_path(DEFAULT_ACCURACY));
+				vector_data
+			})
 			.unwrap_or_default();
+
 		result.style = vector_data.style.clone();
 		result.style.set_stroke_transform(DAffine2::IDENTITY);
 
@@ -1413,7 +1418,7 @@ async fn subpath_segment_lengths(_: impl Ctx, vector_data: VectorDataTable) -> V
 				.stroke_bezpath_iter()
 				.flat_map(|mut bezpath| {
 					bezpath.apply_affine(Affine::new(transform.to_cols_array()));
-					bezpath.segments().map(|segment| segment.perimeter(PERIMETER_ACCURACY)).collect::<Vec<f64>>()
+					bezpath.segments().map(|segment| segment.perimeter(DEFAULT_ACCURACY)).collect::<Vec<f64>>()
 				})
 				.collect::<Vec<f64>>()
 		})