Fix the Bevel node so it applies a consistent bevel size regardless of angle (GraphiteEditor#2293)

* feat: update the bevel algorithm for same bevel length

* Fix: Resolves issue in bevel algorithm

* fix bevel algorithm

* Feat : update the bevel algorithm for bezier curve

* Feat: Add support for rounded and inward-rounded Bevel options

* fix bevel algo for curves

* Nits

* refactor area node

* refactor close path node

* small refactor of 'check_point_inside_shape' method

* copy is_linear function from bezier-rs lib

* refactor bevel node implementation

* cleanup

* refactor bevel implementation

* fix transformation

* cleanup

* update the bevel algorithm

* Code review

* Clean up setup logic for failing test

* update tests to reflect new algorithm

* Decimal integer nits

---------

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

Author: 3 people

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

@@ -1,7 +1,8 @@
 use super::poisson_disk::poisson_disk_sample;
+use crate::vector::algorithms::offset_subpath::MAX_ABSOLUTE_DIFFERENCE;
 use crate::vector::misc::{PointSpacingType, dvec2_to_point};
 use glam::DVec2;
-use kurbo::{BezPath, DEFAULT_ACCURACY, Line, ParamCurve, ParamCurveDeriv, PathEl, PathSeg, Point, Rect, Shape};
+use kurbo::{BezPath, CubicBez, DEFAULT_ACCURACY, Line, ParamCurve, ParamCurveDeriv, PathEl, PathSeg, Point, QuadBez, Rect, Shape};
 
 /// Splits the [`BezPath`] at `t` value which lie 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.
@@ -314,3 +315,16 @@ pub fn poisson_disk_points(bezpath_index: usize, bezpaths: &[(BezPath, Rect)], s
 
 	poisson_disk_sample(offset, width, height, separation_disk_diameter, point_in_shape_checker, line_intersect_shape_checker, rng)
 }
+
+/// Returns true if the Bezier curve is equivalent to a line.
+///
+/// **NOTE**: 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),
+	}
+}

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

@@ -1,6 +1,7 @@
+use bezier_rs::BezierHandles;
 use dyn_any::DynAny;
 use glam::DVec2;
-use kurbo::Point;
+use kurbo::{CubicBez, Line, PathSeg, Point, QuadBez};
 
 /// 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)]
@@ -96,3 +97,37 @@ pub fn point_to_dvec2(point: Point) -> DVec2 {
 pub fn dvec2_to_point(value: DVec2) -> Point {
 	Point { x: value.x, y: value.y }
 }
+
+pub fn segment_to_handles(segment: &PathSeg) -> BezierHandles {
+	match *segment {
+		PathSeg::Line(_) => BezierHandles::Linear,
+		PathSeg::Quad(QuadBez { p0: _, p1, p2: _ }) => BezierHandles::Quadratic { handle: point_to_dvec2(p1) },
+		PathSeg::Cubic(CubicBez { p0: _, p1, p2, p3: _ }) => BezierHandles::Cubic {
+			handle_start: point_to_dvec2(p1),
+			handle_end: point_to_dvec2(p2),
+		},
+	}
+}
+
+pub fn handles_to_segment(start: DVec2, handles: BezierHandles, end: DVec2) -> PathSeg {
+	match handles {
+		bezier_rs::BezierHandles::Linear => {
+			let p0 = dvec2_to_point(start);
+			let p1 = dvec2_to_point(end);
+			PathSeg::Line(Line::new(p0, p1))
+		}
+		bezier_rs::BezierHandles::Quadratic { handle } => {
+			let p0 = dvec2_to_point(start);
+			let p1 = dvec2_to_point(handle);
+			let p2 = dvec2_to_point(end);
+			PathSeg::Quad(QuadBez::new(p0, p1, p2))
+		}
+		bezier_rs::BezierHandles::Cubic { handle_start, handle_end } => {
+			let p0 = dvec2_to_point(start);
+			let p1 = dvec2_to_point(handle_start);
+			let p2 = dvec2_to_point(handle_end);
+			let p3 = dvec2_to_point(end);
+			PathSeg::Cubic(CubicBez::new(p0, p1, p2, p3))
+		}
+	}
+}

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

@@ -226,10 +226,10 @@ impl VectorData {
 
 	pub fn close_subpaths(&mut self) {
 		let segments_to_add: Vec<_> = self
-			.stroke_bezier_paths()
-			.filter(|subpath| !subpath.closed)
-			.filter_map(|subpath| {
-				let (first, last) = subpath.manipulator_groups().first().zip(subpath.manipulator_groups().last())?;
+			.build_stroke_path_iter()
+			.filter(|(_, closed)| !closed)
+			.filter_map(|(manipulator_groups, _)| {
+				let (first, last) = manipulator_groups.first().zip(manipulator_groups.last())?;
 				let (start, end) = self.point_domain.resolve_id(first.id).zip(self.point_domain.resolve_id(last.id))?;
 				Some((start, end))
 			})
@@ -370,7 +370,7 @@ impl VectorData {
 	}
 
 	pub fn check_point_inside_shape(&self, vector_data_transform: DAffine2, point: DVec2) -> bool {
-		let bez_paths: Vec<_> = self
+		let number = self
 			.stroke_bezpath_iter()
 			.map(|mut bezpath| {
 				// TODO: apply transform to points instead of modifying the paths
@@ -379,19 +379,9 @@ impl VectorData {
 				let bbox = bezpath.bounding_box();
 				(bezpath, bbox)
 			})
-			.collect();
-
-		// Check against all paths the point is contained in to compute the correct winding number
-		let mut number = 0;
-
-		for (shape, bbox) in bez_paths {
-			if bbox.x0 > point.x || bbox.y0 > point.y || bbox.x1 < point.x || bbox.y1 < point.y {
-				continue;
-			}
-
-			let winding = shape.winding(dvec2_to_point(point));
-			number += winding;
-		}
+			.filter(|(_, bbox)| bbox.contains(dvec2_to_point(point)))
+			.map(|(bezpath, _)| bezpath.winding(dvec2_to_point(point)))
+			.sum::<i32>();
 
 		// Non-zero fill rule
 		number != 0

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

@@ -440,6 +440,35 @@ impl SegmentDomain {
 		let handles = self.handles.iter_mut();
 		zip(ids, zip(start_point, zip(end_point, handles))).map(|(id, (start_point, (end_point, handles)))| (id, start_point, end_point, handles))
 	}
+
+	pub(crate) fn pair_handles_and_points_mut_by_index(
+		&mut self,
+		index1: usize,
+		index2: usize,
+	) -> (&mut bezier_rs::BezierHandles, &mut usize, &mut usize, &mut bezier_rs::BezierHandles, &mut usize, &mut usize) {
+		// Use split_at_mut to avoid multiple mutable borrows of the same slice
+		let (handles_first, handles_second) = self.handles.split_at_mut(index2.max(index1));
+		let (start_first, start_second) = self.start_point.split_at_mut(index2.max(index1));
+		let (end_first, end_second) = self.end_point.split_at_mut(index2.max(index1));
+
+		let (h1, h2) = if index1 < index2 {
+			(&mut handles_first[index1], &mut handles_second[0])
+		} else {
+			(&mut handles_second[0], &mut handles_first[index2])
+		};
+		let (sp1, sp2) = if index1 < index2 {
+			(&mut start_first[index1], &mut start_second[0])
+		} else {
+			(&mut start_second[0], &mut start_first[index2])
+		};
+		let (ep1, ep2) = if index1 < index2 {
+			(&mut end_first[index1], &mut end_second[0])
+		} else {
+			(&mut end_second[0], &mut end_first[index2])
+		};
+
+		(h1, sp1, ep1, h2, sp2, ep2)
+	}
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Hash, DynAny, serde::Serialize, serde::Deserialize)]