Fix 'Scatter Points' node artifacts (#2657)

* fix

* improve variable names

* fix point offsetting.

* Code review

* Update red dress artwork to preserve its look with new seeds

* Code review

---------

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

Author: indierusty

demo-artwork/red-dress.graphite

@@ -1,7 +1,7 @@
 use super::poisson_disk::poisson_disk_sample;
 use crate::vector::misc::dvec2_to_point;
 use glam::DVec2;
-use kurbo::{Affine, BezPath, Line, ParamCurve, ParamCurveDeriv, PathSeg, Point, Rect, Shape};
+use kurbo::{BezPath, Line, ParamCurve, ParamCurveDeriv, PathSeg, Point, Rect, Shape};
 
 /// Accuracy to find the position on [kurbo::Bezpath].
 const POSITION_ACCURACY: f64 = 1e-5;
@@ -198,77 +198,63 @@ fn bezpath_t_value_to_parametric(bezpath: &kurbo::BezPath, t: BezPathTValue, pre
 ///
 /// While the conceptual process described above asymptotically slows down and is never guaranteed to produce a maximal set in finite time,
 /// this is implemented with an algorithm that produces a maximal set in O(n) time. The slowest part is actually checking if points are inside the subpath shape.
-pub fn poisson_disk_points(bezpath: &BezPath, separation_disk_diameter: f64, rng: impl FnMut() -> f64, subpaths: &[(BezPath, Rect)], subpath_index: usize) -> Vec<DVec2> {
-	if bezpath.elements().is_empty() {
+pub fn poisson_disk_points(bezpath_index: usize, bezpaths: &[(BezPath, Rect)], separation_disk_diameter: f64, rng: impl FnMut() -> f64) -> Vec<DVec2> {
+	let (this_bezpath, this_bbox) = bezpaths[bezpath_index].clone();
+
+	if this_bezpath.elements().is_empty() {
 		return Vec::new();
 	}
-	let bbox = bezpath.bounding_box();
-	let (offset_x, offset_y) = (bbox.x0, bbox.y0);
-	let (width, height) = (bbox.x1 - bbox.x0, bbox.y1 - bbox.y0);
 
-	// TODO: Optimize the following code and make it more robust
+	let offset = DVec2::new(this_bbox.x0, this_bbox.y0);
+	let (width, height) = (this_bbox.width(), this_bbox.height());
 
-	let mut shape = bezpath.clone();
-	shape.close_path();
-	shape.apply_affine(Affine::translate((-offset_x, -offset_y)));
+	// TODO: Optimize the following code and make it more robust
 
 	let point_in_shape_checker = |point: DVec2| {
 		// Check against all paths the point is contained in to compute the correct winding number
 		let mut number = 0;
-		for (i, (shape, bbox)) in subpaths.iter().enumerate() {
-			let point = point + DVec2::new(bbox.x0, bbox.y0);
+
+		for (i, (shape, bbox)) in bezpaths.iter().enumerate() {
+			let point = point + offset;
+
 			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));
 
-			if i == subpath_index && winding == 0 {
+			let winding = shape.winding(dvec2_to_point(point));
+			if winding == 0 && i == bezpath_index {
 				return false;
 			}
 			number += winding;
 		}
+
+		// Non-zero fill rule
 		number != 0
 	};
 
 	let square_edges_intersect_shape_checker = |position: DVec2, size: f64| {
-		let rect = Rect::new(position.x, position.y, position.x + size, position.y + size);
-		bezpath_rectangle_intersections_exist(bezpath, rect)
-	};
+		let min = position + offset;
+		let max = min + DVec2::splat(size);
 
-	let mut points = poisson_disk_sample(width, height, separation_disk_diameter, point_in_shape_checker, square_edges_intersect_shape_checker, rng);
-	for point in &mut points {
-		point.x += offset_x;
-		point.y += offset_y;
-	}
-	points
-}
-
-fn bezpath_rectangle_intersections_exist(bezpath: &BezPath, rect: Rect) -> bool {
-	if !bezpath.bounding_box().overlaps(rect) {
-		return false;
-	}
+		let top_line = Line::new((min.x, min.y), (max.x, min.y));
+		let right_line = Line::new((max.x, min.y), (max.x, max.y));
+		let bottom_line = Line::new((max.x, max.y), (min.x, max.y));
+		let left_line = Line::new((min.x, max.y), (min.x, min.y));
 
-	// Top left
-	let p1 = Point::new(rect.x0, rect.y0);
-	// Top right
-	let p2 = Point::new(rect.x1, rect.y0);
-	// Bottom right
-	let p3 = Point::new(rect.x1, rect.y1);
-	// Bottom left
-	let p4 = Point::new(rect.x0, rect.y1);
-
-	let top_line = Line::new((p1.x, p1.y), (p2.x, p2.y));
-	let right_line = Line::new((p2.x, p2.y), (p3.x, p3.y));
-	let bottom_line = Line::new((p3.x, p3.y), (p4.x, p4.y));
-	let left_line = Line::new((p4.x, p4.y), (p1.x, p1.y));
-
-	for segment in bezpath.segments() {
 		for line in [top_line, right_line, bottom_line, left_line] {
-			if !segment.intersect_line(line).is_empty() {
-				return true;
+			for segment in this_bezpath.segments() {
+				if !segment.intersect_line(line).is_empty() {
+					return true;
+				}
 			}
 		}
-	}
 
-	false
+		false
+	};
+
+	let mut points = poisson_disk_sample(width, height, separation_disk_diameter, point_in_shape_checker, square_edges_intersect_shape_checker, rng);
+	for point in &mut points {
+		*point += offset;
+	}
+	points
 }

frontend/assets/images/demo-artwork/thumbnail-red-dress.png

@@ -223,8 +223,7 @@ impl ActiveListLevel {
 				let point_in_shape = point_in_shape_checker(corner);
 				let square_edges_intersect_shape = square_edges_intersect_shape_checker(corner, square_size);
 				let square_not_outside_shape = point_in_shape || square_edges_intersect_shape;
-				let square_in_shape = point_in_shape_checker(corner + square_size) && !square_edges_intersect_shape;
-				// if !square_edges_intersect_shape { assert_eq!(point_in_shape_checker(corner), point_in_shape_checker(corner + square_size)); }
+				let square_in_shape = !square_edges_intersect_shape && point_in_shape_checker(corner + square_size);
 				// Sometimes this fails so it is necessary to also check the bottom right corner.
 				square_not_outside_shape.then_some(ActiveSquare::new(corner, square_in_shape))
 			})

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

@@ -1366,11 +1366,12 @@ async fn poisson_disk_points(
 	}
 	let path_with_bounding_boxes: Vec<_> = vector_data
 		.stroke_bezpath_iter()
-		.map(|mut subpath| {
+		.map(|mut bezpath| {
 			// TODO: apply transform to points instead of modifying the paths
-			subpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
-			let bbox = subpath.bounding_box();
-			(subpath, bbox)
+			bezpath.apply_affine(Affine::new(vector_data_transform.to_cols_array()));
+			bezpath.close_path();
+			let bbox = bezpath.bounding_box();
+			(bezpath, bbox)
 		})
 		.collect();
 
@@ -1381,7 +1382,7 @@ async fn poisson_disk_points(
 
 		let mut poisson_disk_bezpath = BezPath::new();
 
-		for point in bezpath_algorithms::poisson_disk_points(subpath, separation_disk_diameter, || rng.random::<f64>(), &path_with_bounding_boxes, i) {
+		for point in bezpath_algorithms::poisson_disk_points(i, &path_with_bounding_boxes, separation_disk_diameter, || rng.random::<f64>()) {
 			if poisson_disk_bezpath.elements().is_empty() {
 				poisson_disk_bezpath.move_to(dvec2_to_point(point));
 			} else {