Refactor the 'Sample Points' node to use Kurbo instead of Bezier-rs (#2629)

* fix naming

* refactor sample_points node/function.

* avoid recalculating segments length to find point on bezpath.

* cleanup

* rename few variables

* fix transformation and use precomputed segment lengths

* write comments

* set POSITION_ACCURACY and PERIMETER_ACCURACY to smaller value to get better approximate euclidean position on a path

* fix segment index when t value is 1.0

* Improve comments

* move sampling points code into a separate function

* it works! finding the segment index is linear now!

* small fix and improve variable names & comment

* Naming

* evaluate segment at t as euclidean distance. fix.

* improve comment & variable name

---------

Co-authored-by: indierusty <priyaayadav@gmail.com>
Co-authored-by: Keavon Chambers <keavon@keavon.com>

Author: 3 people

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

@@ -1,17 +1,17 @@
 /// Accuracy to find the position on [kurbo::Bezpath].
-const POSITION_ACCURACY: f64 = 1e-3;
+const POSITION_ACCURACY: f64 = 1e-5;
 /// Accuracy to find the length of the [kurbo::PathSeg].
-const PERIMETER_ACCURACY: f64 = 1e-3;
+pub const PERIMETER_ACCURACY: f64 = 1e-5;
 
 use kurbo::{BezPath, ParamCurve, ParamCurveDeriv, PathSeg, Point, Shape};
 
-pub fn position_on_bezpath(bezpath: &BezPath, t: f64, euclidian: bool) -> Point {
-	let (segment_index, t) = tvalue_to_parametric(bezpath, t, euclidian);
+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);
 	bezpath.get_seg(segment_index + 1).unwrap().eval(t)
 }
 
-pub fn tangent_on_bezpath(bezpath: &BezPath, t: f64, euclidian: bool) -> Point {
-	let (segment_index, t) = tvalue_to_parametric(bezpath, t, euclidian);
+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);
 	let segment = bezpath.get_seg(segment_index + 1).unwrap();
 	match segment {
 		PathSeg::Line(line) => line.deriv().eval(t),
@@ -20,23 +20,92 @@ pub fn tangent_on_bezpath(bezpath: &BezPath, t: f64, euclidian: bool) -> Point {
 	}
 }
 
-pub fn tvalue_to_parametric(bezpath: &BezPath, t: f64, euclidian: bool) -> (usize, f64) {
+pub fn sample_points_on_bezpath(bezpath: BezPath, spacing: f64, start_offset: f64, stop_offset: f64, adaptive_spacing: bool, segments_length: &[f64]) -> Option<BezPath> {
+	let mut sample_bezpath = BezPath::new();
+
+	// Calculate the total length of the collected segments.
+	let total_length: f64 = segments_length.iter().sum();
+
+	// Adjust the usable length by subtracting start and stop offsets.
+	let mut used_length = total_length - start_offset - stop_offset;
+
+	if used_length <= 0. {
+		return None;
+	}
+
+	// Determine the number of points to generate along the path.
+	let sample_count = if adaptive_spacing {
+		// Calculate point count to evenly distribute points while covering the entire path.
+		// With adaptive spacing, we widen or narrow the points as necessary to ensure the last point is always at the end of the path.
+		(used_length / spacing).round()
+	} else {
+		// Calculate point count based on exact spacing, which may not cover the entire path.
+
+		// Without adaptive spacing, we just evenly space the points at the exact specified spacing, usually falling short before the end of the path.
+		let count = (used_length / spacing + f64::EPSILON).floor();
+		used_length -= used_length % spacing;
+		count
+	};
+
+	// Skip if there are no points to generate.
+	if sample_count < 1. {
+		return None;
+	}
+
+	// Generate points along the path based on calculated intervals.
+	let mut length_up_to_previous_segment = 0.;
+	let mut next_segment_index = 0;
+
+	for count in 0..=sample_count as usize {
+		let fraction = count as f64 / sample_count;
+		let length_up_to_next_sample_point = fraction * used_length + start_offset;
+		let mut next_length = length_up_to_next_sample_point - length_up_to_previous_segment;
+		let mut next_segment_length = segments_length[next_segment_index];
+
+		// Keep moving to the next segment while the length up to the next sample point is less or equals to the length up to the segment.
+		while next_length > next_segment_length {
+			if next_segment_index == segments_length.len() - 1 {
+				break;
+			}
+			length_up_to_previous_segment += next_segment_length;
+			next_length = length_up_to_next_sample_point - length_up_to_previous_segment;
+			next_segment_index += 1;
+			next_segment_length = segments_length[next_segment_index];
+		}
+
+		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 point = segment.eval(t);
+
+		if sample_bezpath.elements().is_empty() {
+			sample_bezpath.move_to(point)
+		} else {
+			sample_bezpath.line_to(point)
+		}
+	}
+
+	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) = t_value_to_parametric(bezpath, BezPathTValue::GlobalEuclidean(t));
+		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_euclidian(segment, t, POSITION_ACCURACY));
+		return (segment_index, eval_pathseg_euclidean(segment, t, POSITION_ACCURACY));
 	}
-	t_value_to_parametric(bezpath, BezPathTValue::GlobalParametric(t))
+	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_upto_t / total_length` approximates the input `distance`.
-fn eval_pathseg_euclidian(path: kurbo::PathSeg, distance: f64, accuracy: f64) -> f64 {
+/// 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: kurbo::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.perimeter(accuracy);
+	let total_length = path_segment.perimeter(accuracy);
 
 	if !total_length.is_finite() || total_length <= f64::EPSILON {
 		return 0.;
@@ -45,7 +114,7 @@ fn eval_pathseg_euclidian(path: kurbo::PathSeg, distance: f64, accuracy: f64) ->
 	let distance = distance.clamp(0., 1.);
 
 	while high_t - low_t > accuracy {
-		let current_length = path.subsegment(0.0..mid_t).perimeter(accuracy);
+		let current_length = path_segment.subsegment(0.0..mid_t).perimeter(accuracy);
 		let current_distance = current_length / total_length;
 
 		if current_distance > distance {
@@ -71,7 +140,7 @@ fn global_euclidean_to_local_euclidean(bezpath: &kurbo::BezPath, global_t: f64,
 		}
 		accumulator += length_ratio;
 	}
-	(bezpath.segments().count() - 2, 1.)
+	(bezpath.segments().count() - 1, 1.)
 }
 
 enum BezPathTValue {
@@ -81,24 +150,28 @@ enum BezPathTValue {
 
 /// 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 t_value_to_parametric(bezpath: &kurbo::BezPath, t: BezPathTValue) -> (usize, f64) {
-	let segment_len = bezpath.segments().count();
-	assert!(segment_len >= 1);
+fn bezpath_t_value_to_parametric(bezpath: &kurbo::BezPath, t: BezPathTValue, segments_length: Option<&[f64]>) -> (usize, f64) {
+	let segment_count = bezpath.segments().count();
+	assert!(segment_count >= 1);
 
 	match t {
 		BezPathTValue::GlobalEuclidean(t) => {
-			let lengths = bezpath.segments().map(|bezier| bezier.perimeter(PERIMETER_ACCURACY)).collect::<Vec<f64>>();
-			let total_length: f64 = lengths.iter().sum();
+			let lengths = segments_length
+				.map(|segments_length| segments_length.to_vec())
+				.unwrap_or(bezpath.segments().map(|segment| segment.perimeter(PERIMETER_ACCURACY)).collect());
+
+			let total_length = lengths.iter().sum();
+
 			global_euclidean_to_local_euclidean(bezpath, t, lengths.as_slice(), total_length)
 		}
 		BezPathTValue::GlobalParametric(global_t) => {
 			assert!((0.0..=1.).contains(&global_t));
 
 			if global_t == 1. {
-				return (segment_len - 1, 1.);
+				return (segment_count - 1, 1.);
 			}
 
-			let scaled_t = global_t * segment_len as f64;
+			let scaled_t = global_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/vector_nodes.rs

@@ -1,4 +1,4 @@
-use super::algorithms::bezpath_algorithms::{position_on_bezpath, tangent_on_bezpath};
+use super::algorithms::bezpath_algorithms::{PERIMETER_ACCURACY, 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};
@@ -11,11 +11,11 @@ use crate::transform::{Footprint, ReferencePoint, Transform, TransformMut};
 use crate::vector::PointDomain;
 use crate::vector::style::{LineCap, LineJoin};
 use crate::{CloneVarArgs, Color, Context, Ctx, ExtractAll, GraphicElement, GraphicGroupTable, OwnedContextImpl};
-use bezier_rs::{Join, ManipulatorGroup, Subpath, SubpathTValue, TValue};
+use bezier_rs::{Join, ManipulatorGroup, Subpath, SubpathTValue};
 use core::f64::consts::PI;
 use core::hash::{Hash, Hasher};
 use glam::{DAffine2, DVec2};
-use kurbo::Affine;
+use kurbo::{Affine, Shape};
 use rand::{Rng, SeedableRng};
 use std::collections::hash_map::DefaultHasher;
 
@@ -1147,144 +1147,43 @@ async fn sample_points(_: impl Ctx, vector_data: VectorDataTable, spacing: f64,
 	let spacing = spacing.max(0.01);
 
 	let vector_data_transform = vector_data.transform();
-	let vector_data = vector_data.one_instance_ref().instance;
 
-	// Create an iterator over the bezier segments with enumeration and peeking capability.
-	let mut bezier = vector_data.segment_bezier_iter().enumerate().peekable();
+	// Using `stroke_bezpath_iter` so that the `subpath_segment_lengths` is aligned to the segments of each bezpath.
+	// So we can index into `subpath_segment_lengths` to get the length of the segments.
+	// NOTE: `subpath_segment_lengths` has precalulated lengths with transformation applied.
+	let bezpaths = vector_data.one_instance_ref().instance.stroke_bezpath_iter();
 
 	// Initialize the result VectorData with the same transformation as the input.
 	let mut result = VectorDataTable::default();
 	*result.transform_mut() = vector_data_transform;
 
-	// Iterate over each segment in the bezier iterator.
-	while let Some((index, (segment_id, _, start_point_index, mut last_end))) = bezier.next() {
-		// Record the start point index of the subpath.
-		let subpath_start_point_index = start_point_index;
-
-		// Collect connected segments that form a continuous path.
-		let mut lengths = vec![(segment_id, subpath_segment_lengths.get(index).copied().unwrap_or_default())];
-
-		// Continue collecting segments as long as they are connected end-to-start.
-		while let Some(&seg) = bezier.peek() {
-			let (_, (_, _, ref start, _)) = seg;
-			if *start == last_end {
-				// Consume the next element since it continues the path.
-				let (index, (next_segment_id, _, _, end)) = bezier.next().unwrap();
-				last_end = end;
-				lengths.push((next_segment_id, subpath_segment_lengths.get(index).copied().unwrap_or_default()));
-			} else {
-				// The next segment does not continue the path.
-				break;
-			}
-		}
-
-		// Determine if the subpath is closed.
-		let subpath_is_closed = last_end == subpath_start_point_index;
+	// Keeps track of the index of the first segment of the next bezpath in order to get lengths of all segments.
+	let mut next_segment_index = 0;
 
-		// Calculate the total length of the collected segments.
-		let total_length: f64 = lengths.iter().map(|(_, len)| *len).sum();
+	for mut bezpath in bezpaths {
+		// Apply the tranformation to the current bezpath to calculate points after transformation.
+		bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
 
-		// Adjust the usable length by subtracting start and stop offsets.
-		let mut used_length = total_length - start_offset - stop_offset;
-		if used_length <= 0. {
-			continue;
-		}
+		let segment_count = bezpath.segments().count();
 
-		// Determine the number of points to generate along the path.
-		let count = if adaptive_spacing {
-			// Calculate point count to evenly distribute points while covering the entire path.
-			// With adaptive spacing, we widen or narrow the points as necessary to ensure the last point is always at the end of the path.
-			(used_length / spacing).round()
-		} else {
-			// Calculate point count based on exact spacing, which may not cover the entire path.
+		// For the current bezpath we get its segment's length by calculating the start index and end index.
+		let current_bezpath_segments_length = &subpath_segment_lengths[next_segment_index..next_segment_index + segment_count];
 
-			// Without adaptive spacing, we just evenly space the points at the exact specified spacing, usually falling short before the end of the path.
-			let c = (used_length / spacing + f64::EPSILON).floor();
-			used_length -= used_length % spacing;
-			c
-		};
+		// Increment the segment index by the number of segments in the current bezpath to calculate the next bezpath segment's length.
+		next_segment_index += segment_count;
 
-		// Skip if there are no points to generate.
-		if count < 1. {
+		let Some(mut sample_bezpath) = sample_points_on_bezpath(bezpath, spacing, start_offset, stop_offset, adaptive_spacing, current_bezpath_segments_length) else {
 			continue;
-		}
-
-		// Initialize a vector to store indices of generated points.
-		let mut point_indices = Vec::new();
-
-		// Generate points along the path based on calculated intervals.
-		let max_c = if subpath_is_closed { count as usize - 1 } else { count as usize };
-		for c in 0..=max_c {
-			let fraction = c as f64 / count;
-			let total_distance = fraction * used_length + start_offset;
-
-			// Find the segment corresponding to the current total_distance.
-			let (mut current_segment_id, mut length) = lengths[0];
-			let mut total_length_before = 0.;
-			for &(next_segment_id, next_length) in lengths.iter().skip(1) {
-				if total_length_before + length > total_distance {
-					break;
-				}
-
-				total_length_before += length;
-				current_segment_id = next_segment_id;
-				length = next_length;
-			}
-
-			// Retrieve the segment and apply transformation.
-			let Some(segment) = vector_data.segment_from_id(current_segment_id) else { continue };
-			let segment = segment.apply_transformation(|point| vector_data_transform.transform_point2(point));
-
-			// Calculate the position on the segment.
-			let parametric_t = segment.euclidean_to_parametric_with_total_length((total_distance - total_length_before) / length, 0.001, length);
-			let point = segment.evaluate(TValue::Parametric(parametric_t));
-
-			// Generate a new PointId and add the point to result.point_domain.
-			let point_id = PointId::generate();
-			result.one_instance_mut().instance.point_domain.push(point_id, vector_data_transform.inverse().transform_point2(point));
-
-			// Store the index of the point.
-			let point_index = result.one_instance_mut().instance.point_domain.ids().len() - 1;
-			point_indices.push(point_index);
-		}
-
-		// After generating points, create segments between consecutive points.
-		for window in point_indices.windows(2) {
-			if let [start_index, end_index] = *window {
-				// Generate a new SegmentId.
-				let segment_id = SegmentId::generate();
-
-				// Use BezierHandles::Linear for linear segments.
-				let handles = bezier_rs::BezierHandles::Linear;
-
-				// Generate a new StrokeId.
-				let stroke_id = StrokeId::generate();
-
-				// Add the segment to result.segment_domain.
-				result.one_instance_mut().instance.segment_domain.push(segment_id, start_index, end_index, handles, stroke_id);
-			}
-		}
-
-		// If the subpath is closed, add a closing segment connecting the last point to the first point.
-		if subpath_is_closed {
-			if let (Some(&first_index), Some(&last_index)) = (point_indices.first(), point_indices.last()) {
-				// Generate a new SegmentId.
-				let segment_id = SegmentId::generate();
-
-				// Use BezierHandles::Linear for linear segments.
-				let handles = bezier_rs::BezierHandles::Linear;
+		};
 
-				// Generate a new StrokeId.
-				let stroke_id = StrokeId::generate();
+		// Reverse the transformation applied to the bezpath as the `result` already has the transformation set.
+		sample_bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()).inverse());
 
-				// Add the closing segment to result.segment_domain.
-				result.one_instance_mut().instance.segment_domain.push(segment_id, last_index, first_index, handles, stroke_id);
-			}
-		}
+		// Append the bezpath (subpath) that connects generated points by lines.
+		result.one_instance_mut().instance.append_bezpath(sample_bezpath);
 	}
-
 	// Transfer the style from the input vector data to the result.
-	result.one_instance_mut().instance.style = vector_data.style.clone();
+	result.one_instance_mut().instance.style = vector_data.one_instance_ref().instance.style.clone();
 	result.one_instance_mut().instance.style.set_stroke_transform(vector_data_transform);
 
 	// Return the resulting vector data with newly generated points and segments.
@@ -1320,7 +1219,7 @@ async fn position_on_path(
 		let t = if progress == bezpath_count { 1. } else { progress.fract() };
 		bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
 
-		point_to_dvec2(position_on_bezpath(bezpath, t, euclidian))
+		point_to_dvec2(position_on_bezpath(bezpath, t, euclidian, None))
 	})
 }
 
@@ -1353,10 +1252,10 @@ async fn tangent_on_path(
 		let t = if progress == bezpath_count { 1. } else { progress.fract() };
 		bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
 
-		let mut tangent = point_to_dvec2(tangent_on_bezpath(bezpath, t, euclidian));
+		let mut tangent = point_to_dvec2(tangent_on_bezpath(bezpath, t, euclidian, None));
 		if tangent == DVec2::ZERO {
 			let t = t + if t > 0.5 { -0.001 } else { 0.001 };
-			tangent = point_to_dvec2(tangent_on_bezpath(bezpath, t, euclidian));
+			tangent = point_to_dvec2(tangent_on_bezpath(bezpath, t, euclidian, None));
 		}
 		if tangent == DVec2::ZERO {
 			return 0.;
@@ -1430,8 +1329,11 @@ async fn subpath_segment_lengths(_: impl Ctx, vector_data: VectorDataTable) -> V
 	let vector_data = vector_data.one_instance_ref().instance;
 
 	vector_data
-		.segment_bezier_iter()
-		.map(|(_id, bezier, _, _)| bezier.apply_transformation(|point| vector_data_transform.transform_point2(point)).length(None))
+		.stroke_bezpath_iter()
+		.flat_map(|mut bezpath| {
+			bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
+			bezpath.segments().map(|segment| segment.perimeter(PERIMETER_ACCURACY)).collect::<Vec<f64>>()
+		})
 		.collect()
 }