Refactor the 'Round Corners' and 'Auto-Tangents' nodes and vector node unit tests to use Kurbo (#2964)

* add todo

* impl function to convert a bezpath to manipulator groups

* refactor few node impls

* refactor vector nodes test and few methods on VectorData struct

* refactor tests

* remove unused import

* simplify and fix morph node test

* rename vars and comment

* refactor bezpath_to_parametric function

* Code review

* fix bezpath_to_manipulator_groups function

---------

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

Author: indierusty

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

@@ -56,25 +56,26 @@ pub fn split_bezpath_at_segment(bezpath: &BezPath, segment_index: usize, t: f64)
 }
 
 /// Splits the [`BezPath`] at a `t` value which lies in the range of [0, 1].
-/// Returns [`None`] if the given [`BezPath`] has no segments or `t` is within f64::EPSILON of 0 or 1.
-pub fn split_bezpath(bezpath: &BezPath, t: f64, euclidian: bool) -> Option<(BezPath, BezPath)> {
-	if t <= f64::EPSILON || (1. - t) <= f64::EPSILON || bezpath.segments().count() == 0 {
+/// Returns [`None`] if the given [`BezPath`] has no segments.
+pub fn split_bezpath(bezpath: &BezPath, t_value: TValue) -> Option<(BezPath, BezPath)> {
+	if bezpath.segments().count() == 0 {
 		return None;
 	}
 
 	// Get the segment which lies at the split.
-	let (segment_index, t) = t_value_to_parametric(bezpath, t, euclidian, None);
+	let (segment_index, t) = eval_bezpath(bezpath, t_value, None);
 	split_bezpath_at_segment(bezpath, segment_index, t)
 }
 
-pub fn evaluate_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);
+pub fn evaluate_bezpath(bezpath: &BezPath, t_value: TValue, segments_length: Option<&[f64]>) -> Point {
+	let (segment_index, t) = eval_bezpath(bezpath, t_value, segments_length);
 	bezpath.get_seg(segment_index + 1).unwrap().eval(t)
 }
 
-pub fn tangent_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);
+pub fn tangent_on_bezpath(bezpath: &BezPath, t_value: TValue, segments_length: Option<&[f64]>) -> Point {
+	let (segment_index, t) = eval_bezpath(bezpath, t_value, segments_length);
 	let segment = bezpath.get_seg(segment_index + 1).unwrap();
+
 	match segment {
 		PathSeg::Line(line) => line.deriv().eval(t),
 		PathSeg::Quad(quad_bez) => quad_bez.deriv().eval(t),
@@ -180,23 +181,35 @@ pub fn sample_polyline_on_bezpath(
 	Some(sample_bezpath)
 }
 
-pub fn t_value_to_parametric(bezpath: &BezPath, t: f64, euclidian: bool, segments_length: Option<&[f64]>) -> (usize, f64) {
-	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, DEFAULT_ACCURACY));
+#[derive(Debug, Clone, Copy)]
+pub enum TValue {
+	Parametric(f64),
+	Euclidean(f64),
+}
+
+/// Return the subsegment for the given [TValue] range. Returns None if parametric value of `t1` is greater than `t2`.
+pub fn trim_pathseg(segment: PathSeg, t1: TValue, t2: TValue) -> Option<PathSeg> {
+	let t1 = eval_pathseg(segment, t1);
+	let t2 = eval_pathseg(segment, t2);
+
+	if t1 > t2 { None } else { Some(segment.subsegment(t1..t2)) }
+}
+
+pub fn eval_pathseg(segment: PathSeg, t_value: TValue) -> f64 {
+	match t_value {
+		TValue::Parametric(t) => t,
+		TValue::Euclidean(t) => eval_pathseg_euclidean(segment, t, DEFAULT_ACCURACY),
 	}
-	bezpath_t_value_to_parametric(bezpath, BezPathTValue::GlobalParametric(t), segments_length)
 }
 
 /// Finds the t value of point on the given path segment i.e fractional distance along the segment's total length.
 /// It uses a binary search to find the value `t` such that the ratio `length_up_to_t / total_length` approximates the input `distance`.
-pub fn eval_pathseg_euclidean(path_segment: PathSeg, distance: f64, accuracy: f64) -> f64 {
+pub fn eval_pathseg_euclidean(segment: PathSeg, distance: f64, accuracy: f64) -> f64 {
 	let mut low_t = 0.;
 	let mut mid_t = 0.5;
 	let mut high_t = 1.;
 
-	let total_length = path_segment.perimeter(accuracy);
+	let total_length = segment.perimeter(accuracy);
 
 	if !total_length.is_finite() || total_length <= f64::EPSILON {
 		return 0.;
@@ -205,7 +218,7 @@ pub fn eval_pathseg_euclidean(path_segment: PathSeg, distance: f64, accuracy: f6
 	let distance = distance.clamp(0., 1.);
 
 	while high_t - low_t > accuracy {
-		let current_length = path_segment.subsegment(0.0..mid_t).perimeter(accuracy);
+		let current_length = segment.subsegment(0.0..mid_t).perimeter(accuracy);
 		let current_distance = current_length / total_length;
 
 		if current_distance > distance {
@@ -222,7 +235,7 @@ pub fn eval_pathseg_euclidean(path_segment: PathSeg, distance: f64, accuracy: f6
 /// Converts from a bezpath (composed of multiple segments) to a point along a certain segment represented.
 /// The returned tuple represents the segment index and the `t` value along that segment.
 /// Both the input global `t` value and the output `t` value are in euclidean space, meaning there is a constant rate of change along the arc length.
-fn global_euclidean_to_local_euclidean(bezpath: &BezPath, global_t: f64, lengths: &[f64], total_length: f64) -> (usize, f64) {
+fn eval_bazpath_to_euclidean(bezpath: &BezPath, global_t: f64, lengths: &[f64], total_length: f64) -> (usize, f64) {
 	let mut accumulator = 0.;
 	for (index, length) in lengths.iter().enumerate() {
 		let length_ratio = length / total_length;
@@ -234,19 +247,14 @@ fn global_euclidean_to_local_euclidean(bezpath: &BezPath, global_t: f64, lengths
 	(bezpath.segments().count() - 1, 1.)
 }
 
-enum BezPathTValue {
-	GlobalEuclidean(f64),
-	GlobalParametric(f64),
-}
-
-/// Convert a [BezPathTValue] to a parametric `(segment_index, t)` tuple.
-/// - Asserts that `t` values contained within the `SubpathTValue` argument lie in the range [0, 1].
-fn bezpath_t_value_to_parametric(bezpath: &BezPath, t: BezPathTValue, precomputed_segments_length: Option<&[f64]>) -> (usize, f64) {
+/// Convert a [TValue] to a parametric `(segment_index, t)` tuple.
+/// - Asserts that `t` values contained within the `TValue` argument lie in the range [0, 1].
+fn eval_bezpath(bezpath: &BezPath, t: TValue, precomputed_segments_length: Option<&[f64]>) -> (usize, f64) {
 	let segment_count = bezpath.segments().count();
 	assert!(segment_count >= 1);
 
 	match t {
-		BezPathTValue::GlobalEuclidean(t) => {
+		TValue::Euclidean(t) => {
 			let computed_segments_length;
 
 			let segments_length = if let Some(segments_length) = precomputed_segments_length {
@@ -258,16 +266,18 @@ fn bezpath_t_value_to_parametric(bezpath: &BezPath, t: BezPathTValue, precompute
 
 			let total_length = segments_length.iter().sum();
 
-			global_euclidean_to_local_euclidean(bezpath, t, segments_length, total_length)
+			let (segment_index, t) = eval_bazpath_to_euclidean(bezpath, t, segments_length, total_length);
+			let segment = bezpath.get_seg(segment_index + 1).unwrap();
+			(segment_index, eval_pathseg_euclidean(segment, t, DEFAULT_ACCURACY))
 		}
-		BezPathTValue::GlobalParametric(global_t) => {
-			assert!((0.0..=1.).contains(&global_t));
+		TValue::Parametric(t) => {
+			assert!((0.0..=1.).contains(&t));
 
-			if global_t == 1. {
+			if t == 1. {
 				return (segment_count - 1, 1.);
 			}
 
-			let scaled_t = global_t * segment_count as f64;
+			let scaled_t = t * segment_count as f64;
 			let segment_index = scaled_t.floor() as usize;
 			let t = scaled_t - segment_index as f64;
 

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

@@ -1,9 +1,10 @@
+use super::PointId;
+use super::algorithms::offset_subpath::MAX_ABSOLUTE_DIFFERENCE;
 use bezier_rs::{BezierHandles, ManipulatorGroup, Subpath};
 use dyn_any::DynAny;
 use glam::DVec2;
-use kurbo::{BezPath, CubicBez, Line, PathSeg, Point, QuadBez};
-
-use super::PointId;
+use kurbo::{BezPath, CubicBez, Line, ParamCurve, PathSeg, Point, QuadBez};
+use std::ops::Sub;
 
 /// Represents different ways of calculating the centroid.
 #[derive(Default, Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize, Hash, DynAny, specta::Type, node_macro::ChoiceType)]
@@ -169,3 +170,70 @@ pub fn bezpath_from_manipulator_groups(manipulator_groups: &[ManipulatorGroup<Po
 	}
 	bezpath
 }
+
+pub fn bezpath_to_manipulator_groups(bezpath: &BezPath) -> (Vec<ManipulatorGroup<PointId>>, bool) {
+	let mut manipulator_groups = Vec::<ManipulatorGroup<PointId>>::new();
+	let mut is_closed = false;
+
+	for element in bezpath.elements() {
+		let manipulator_group = match *element {
+			kurbo::PathEl::MoveTo(point) => ManipulatorGroup::new(point_to_dvec2(point), None, None),
+			kurbo::PathEl::LineTo(point) => ManipulatorGroup::new(point_to_dvec2(point), None, None),
+			kurbo::PathEl::QuadTo(point, point1) => ManipulatorGroup::new(point_to_dvec2(point1), Some(point_to_dvec2(point)), None),
+			kurbo::PathEl::CurveTo(point, point1, point2) => {
+				if let Some(last_maipulator_group) = manipulator_groups.last_mut() {
+					last_maipulator_group.out_handle = Some(point_to_dvec2(point));
+				}
+				ManipulatorGroup::new(point_to_dvec2(point2), Some(point_to_dvec2(point1)), None)
+			}
+			kurbo::PathEl::ClosePath => {
+				if let Some(last_group) = manipulator_groups.pop() {
+					if let Some(first_group) = manipulator_groups.first_mut() {
+						first_group.out_handle = last_group.in_handle;
+					}
+				}
+				is_closed = true;
+				break;
+			}
+		};
+
+		manipulator_groups.push(manipulator_group);
+	}
+
+	(manipulator_groups, is_closed)
+}
+
+/// Returns true if the [`PathSeg`] is equivalent to a line.
+///
+/// This is different from simply checking if the segment is [`PathSeg::Line`] or [`PathSeg::Quad`] or [`PathSeg::Cubic`]. Bezier curve can also be a line if the control points are colinear to the start and end points. Therefore if the handles exceed the start and end point, it will still be considered as a line.
+pub fn is_linear(segment: PathSeg) -> bool {
+	let is_colinear = |a: Point, b: Point, c: Point| -> bool { ((b.x - a.x) * (c.y - a.y) - (b.y - a.y) * (c.x - a.x)).abs() < MAX_ABSOLUTE_DIFFERENCE };
+
+	match segment {
+		PathSeg::Line(_) => true,
+		PathSeg::Quad(QuadBez { p0, p1, p2 }) => is_colinear(p0, p1, p2),
+		PathSeg::Cubic(CubicBez { p0, p1, p2, p3 }) => is_colinear(p0, p1, p3) && is_colinear(p0, p2, p3),
+	}
+}
+
+/// Get an iterator over the coordinates of all points in a path segment.
+pub fn get_segment_points(segment: PathSeg) -> Vec<Point> {
+	match segment {
+		PathSeg::Line(line) => [line.p0, line.p1].to_vec(),
+		PathSeg::Quad(quad_bez) => [quad_bez.p0, quad_bez.p1, quad_bez.p2].to_vec(),
+		PathSeg::Cubic(cubic_bez) => [cubic_bez.p0, cubic_bez.p1, cubic_bez.p2, cubic_bez.p3].to_vec(),
+	}
+}
+
+/// Returns true if the corresponding points of the two [`PathSeg`]s are within the provided absolute value difference from each other.
+pub fn pathseg_abs_diff_eq(seg1: PathSeg, seg2: PathSeg, max_abs_diff: f64) -> bool {
+	let seg1 = if is_linear(seg1) { PathSeg::Line(Line::new(seg1.start(), seg1.end())) } else { seg1 };
+	let seg2 = if is_linear(seg2) { PathSeg::Line(Line::new(seg2.start(), seg2.end())) } else { seg2 };
+
+	let seg1_points = get_segment_points(seg1);
+	let seg2_points = get_segment_points(seg2);
+
+	let cmp = |a: f64, b: f64| a.sub(b).abs() < max_abs_diff;
+
+	seg1_points.len() == seg2_points.len() && seg1_points.into_iter().zip(seg2_points).all(|(a, b)| cmp(a.x, b.x) && cmp(a.y, b.y))
+}

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

@@ -17,7 +17,7 @@ use core::hash::Hash;
 use dyn_any::DynAny;
 use glam::{DAffine2, DVec2};
 pub use indexed::VectorDataIndex;
-use kurbo::{Affine, Rect, Shape};
+use kurbo::{Affine, BezPath, Rect, Shape};
 pub use modification::*;
 use std::collections::HashMap;
 
@@ -195,6 +195,13 @@ impl VectorData {
 		Self::from_subpaths([subpath], false)
 	}
 
+	/// Construct some new vector data from a single [`BezPath`] with an identity transform and black fill.
+	pub fn from_bezpath(bezpath: BezPath) -> Self {
+		let mut vector_data = Self::default();
+		vector_data.append_bezpath(bezpath);
+		vector_data
+	}
+
 	/// Construct some new vector data from subpaths with an identity transform and black fill.
 	pub fn from_subpaths(subpaths: impl IntoIterator<Item = impl Borrow<bezier_rs::Subpath<PointId>>>, preserve_id: bool) -> Self {
 		let mut vector_data = Self::default();

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

@@ -3,6 +3,7 @@ use crate::vector::vector_data::{HandleId, VectorData};
 use bezier_rs::{BezierHandles, ManipulatorGroup};
 use dyn_any::DynAny;
 use glam::{DAffine2, DVec2};
+use kurbo::{CubicBez, Line, PathSeg, QuadBez};
 use std::collections::HashMap;
 use std::hash::{Hash, Hasher};
 use std::iter::zip;
@@ -673,6 +674,18 @@ impl FoundSubpath {
 }
 
 impl VectorData {
+	/// Construct a [`kurbo::PathSeg`] by resolving the points from their ids.
+	fn path_segment_from_index(&self, start: usize, end: usize, handles: BezierHandles) -> PathSeg {
+		let start = dvec2_to_point(self.point_domain.positions()[start]);
+		let end = dvec2_to_point(self.point_domain.positions()[end]);
+
+		match handles {
+			BezierHandles::Linear => PathSeg::Line(Line::new(start, end)),
+			BezierHandles::Quadratic { handle } => PathSeg::Quad(QuadBez::new(start, dvec2_to_point(handle), end)),
+			BezierHandles::Cubic { handle_start, handle_end } => PathSeg::Cubic(CubicBez::new(start, dvec2_to_point(handle_start), dvec2_to_point(handle_end), end)),
+		}
+	}
+
 	/// Construct a [`bezier_rs::Bezier`] curve spanning from the resolved position of the start and end points with the specified handles.
 	fn segment_to_bezier_with_index(&self, start: usize, end: usize, handles: BezierHandles) -> bezier_rs::Bezier {
 		let start = self.point_domain.positions()[start];
@@ -699,6 +712,19 @@ impl VectorData {
 		(start_id, end_id, self.segment_to_bezier_with_index(start, end, self.segment_domain.handles[index]))
 	}
 
+	/// Iterator over all of the [`bezier_rs::Bezier`] following the order that they are stored in the segment domain, skipping invalid segments.
+	pub fn segment_iter(&self) -> impl Iterator<Item = (SegmentId, PathSeg, PointId, PointId)> {
+		let to_segment = |(((&handles, &id), &start), &end)| (id, self.path_segment_from_index(start, end, handles), self.point_domain.ids()[start], self.point_domain.ids()[end]);
+
+		self.segment_domain
+			.handles
+			.iter()
+			.zip(&self.segment_domain.id)
+			.zip(self.segment_domain.start_point())
+			.zip(self.segment_domain.end_point())
+			.map(to_segment)
+	}
+
 	/// Iterator over all of the [`bezier_rs::Bezier`] following the order that they are stored in the segment domain, skipping invalid segments.
 	pub fn segment_bezier_iter(&self) -> impl Iterator<Item = (SegmentId, bezier_rs::Bezier, PointId, PointId)> + '_ {
 		let to_bezier = |(((&handles, &id), &start), &end)| (id, self.segment_to_bezier_with_index(start, end, handles), self.point_domain.ids()[start], self.point_domain.ids()[end]);
@@ -819,48 +845,8 @@ impl VectorData {
 		Some(bezier_rs::Subpath::new(groups, closed))
 	}
 
-	/// Construct a [`bezier_rs::Bezier`] curve from an iterator of segments with (handles, start point, end point). Returns None if any ids are invalid or if the segments are not continuous.
-	fn subpath_from_segments(&self, segments: impl Iterator<Item = (BezierHandles, usize, usize)>) -> Option<bezier_rs::Subpath<PointId>> {
-		let mut first_point = None;
-		let mut groups = Vec::new();
-		let mut last: Option<(usize, BezierHandles)> = None;
-
-		for (handle, start, end) in segments {
-			if last.is_some_and(|(previous_end, _)| previous_end != start) {
-				warn!("subpath_from_segments that were not continuous");
-				return None;
-			}
-			first_point = Some(first_point.unwrap_or(start));
-
-			groups.push(ManipulatorGroup {
-				anchor: self.point_domain.positions()[start],
-				in_handle: last.and_then(|(_, handle)| handle.end()),
-				out_handle: handle.start(),
-				id: self.point_domain.ids()[start],
-			});
-
-			last = Some((end, handle));
-		}
-
-		let closed = groups.len() > 1 && last.map(|(point, _)| point) == first_point;
-
-		if let Some((end, last_handle)) = last {
-			if closed {
-				groups[0].in_handle = last_handle.end();
-			} else {
-				groups.push(ManipulatorGroup {
-					anchor: self.point_domain.positions()[end],
-					in_handle: last_handle.end(),
-					out_handle: None,
-					id: self.point_domain.ids()[end],
-				});
-			}
-		}
-		Some(bezier_rs::Subpath::new(groups, closed))
-	}
-
 	/// Construct a [`bezier_rs::Bezier`] curve for each region, skipping invalid regions.
-	pub fn region_bezier_paths(&self) -> impl Iterator<Item = (RegionId, bezier_rs::Subpath<PointId>)> + '_ {
+	pub fn region_manipulator_groups(&self) -> impl Iterator<Item = (RegionId, Vec<ManipulatorGroup<PointId>>)> + '_ {
 		self.region_domain
 			.id
 			.iter()
@@ -876,7 +862,29 @@ impl VectorData {
 					.zip(self.segment_domain.end_point.get(range)?)
 					.map(|((&handles, &start), &end)| (handles, start, end));
 
-				self.subpath_from_segments(segments_iter).map(|subpath| (id, subpath))
+				let mut manipulator_groups = Vec::new();
+				let mut in_handle = None;
+
+				for segment in segments_iter {
+					let (handles, start_point_index, _end_point_index) = segment;
+					let start_point_id = self.point_domain.id[start_point_index];
+					let start_point = self.point_domain.position[start_point_index];
+
+					let (manipulator_group, next_in_handle) = match handles {
+						BezierHandles::Linear => (ManipulatorGroup::new_with_id(start_point, in_handle, None, start_point_id), None),
+						BezierHandles::Quadratic { handle } => (ManipulatorGroup::new_with_id(start_point, in_handle, Some(handle), start_point_id), None),
+						BezierHandles::Cubic { handle_start, handle_end } => (ManipulatorGroup::new_with_id(start_point, in_handle, Some(handle_start), start_point_id), Some(handle_end)),
+					};
+
+					in_handle = next_in_handle;
+					manipulator_groups.push(manipulator_group);
+				}
+
+				if let Some(first) = manipulator_groups.first_mut() {
+					first.in_handle = in_handle;
+				}
+
+				Some((id, manipulator_groups))
 			})
 	}