Fix the 'Repeat', 'Circular Repeat', and 'Mirror' nodes to work on point cloud vector data (#2553)

* Include points in bounding box calculations

* Fix unrelated crash from debug assert when reordering root-level folders

* Fix another unrelated crash from debug assert when GRS scaling to size 0

* Fix several vector nodes to respect and propagate local transform space

editor/src/messages/portfolio/document/graph_operation/utility_types.rs

@@ -413,22 +413,14 @@ impl<'a> ModifyInputsContext<'a> {
 	pub fn transform_set_direct(&mut self, transform: DAffine2, skip_rerender: bool, transform_node_id: Option<NodeId>) {
 		// If the Transform node didn't exist yet, create it now
 		let Some(transform_node_id) = transform_node_id.or_else(|| {
+			// Check if the transform is the identity transform and if so, don't create a new Transform node
+			if let Some((scale, angle, translation)) = (transform.matrix2.determinant() != 0.).then(|| transform.to_scale_angle_translation()) {
 				// Check if the transform is the identity transform within an epsilon
-			let is_identity = {
+				if scale.x.abs() < 1e-6 && scale.y.abs() < 1e-6 && angle.abs() < 1e-6 && translation.x.abs() < 1e-6 && translation.y.abs() < 1e-6 {
-				let transform = transform.to_scale_angle_translation();
-				let identity = DAffine2::IDENTITY.to_scale_angle_translation();
-
-				(transform.0.x - identity.0.x).abs() < 1e-6
-					&& (transform.0.y - identity.0.y).abs() < 1e-6
-					&& (transform.1 - identity.1).abs() < 1e-6
-					&& (transform.2.x - identity.2.x).abs() < 1e-6
-					&& (transform.2.y - identity.2.y).abs() < 1e-6
-			};
-
 					// We don't want to pollute the graph with an unnecessary Transform node, so we avoid creating and setting it by returning None
-			if is_identity {
 					return None;
 				}
+			}
 
 			// Create the Transform node
 			self.existing_node_id("Transform", true)

editor/src/messages/portfolio/document/utility_types/document_metadata.rs

@@ -80,6 +80,11 @@ impl DocumentMetadata {
 	}
 
 	pub fn transform_to_viewport(&self, layer: LayerNodeIdentifier) -> DAffine2 {
+		// We're not allowed to convert the root parent to a node id
+		if layer == LayerNodeIdentifier::ROOT_PARENT {
+			return self.document_to_viewport;
+		}
+
 		let footprint = self.upstream_footprints.get(&layer.to_node()).map(|footprint| footprint.transform).unwrap_or(self.document_to_viewport);
 		let local_transform = self.local_transforms.get(&layer.to_node()).copied().unwrap_or_default();
 

node-graph/gcore/src/graphic_element/renderer.rs

@@ -1,7 +1,6 @@
 mod quad;
 mod rect;
 
-use crate::consts::{LAYER_OUTLINE_STROKE_COLOR, LAYER_OUTLINE_STROKE_WEIGHT};
 use crate::raster::image::ImageFrameTable;
 use crate::raster::{BlendMode, Image};
 use crate::transform::{Footprint, Transform};
@@ -465,6 +464,7 @@ impl GraphicElementRendered for VectorDataTable {
 
 	#[cfg(feature = "vello")]
 	fn render_to_vello(&self, scene: &mut Scene, parent_transform: DAffine2, _: &mut RenderContext, render_params: &RenderParams) {
+		use crate::consts::{LAYER_OUTLINE_STROKE_COLOR, LAYER_OUTLINE_STROKE_WEIGHT};
 		use crate::vector::style::{GradientType, LineCap, LineJoin};
 		use vello::kurbo::{Cap, Join};
 		use vello::peniko;

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

@@ -194,9 +194,22 @@ impl VectorData {
 
 	/// Compute the bounding boxes of the subpaths with the specified transform
 	pub fn bounding_box_with_transform(&self, transform: DAffine2) -> Option<[DVec2; 2]> {
-		self.segment_bezier_iter()
+		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(|b1, b2| [b1[0].min(b2[0]), b1[1].max(b2[1])])
+			.reduce(combine);
+
+		anchor_bounds.iter().chain(segment_bounds.iter()).copied().reduce(combine)
 	}
 
 	/// Calculate the corners of the bounding box but with a nonzero size.

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

@@ -6,7 +6,7 @@ use crate::registry::types::{Angle, Fraction, IntegerCount, Length, Percentage,
 use crate::renderer::GraphicElementRendered;
 use crate::transform::{Footprint, Transform, TransformMut};
 use crate::vector::PointDomain;
-use crate::vector::style::LineJoin;
+use crate::vector::style::{LineCap, LineJoin};
 use crate::{CloneVarArgs, Color, Context, Ctx, ExtractAll, GraphicElement, GraphicGroupTable, OwnedContextImpl};
 use bezier_rs::{Cap, Join, ManipulatorGroup, Subpath, SubpathTValue, TValue};
 use core::f64::consts::PI;
@@ -943,13 +943,15 @@ async fn bounding_box(_: impl Ctx, vector_data: VectorDataTable) -> VectorDataTa
 	let vector_data = vector_data.one_instance().instance;
 
 	let mut result = vector_data
-		.bounding_box_with_transform(vector_data_transform)
+		.bounding_box()
 		.map(|bounding_box| VectorData::from_subpath(Subpath::new_rect(bounding_box[0], bounding_box[1])))
 		.unwrap_or_default();
 	result.style = vector_data.style.clone();
 	result.style.set_stroke_transform(DAffine2::IDENTITY);
 
-	VectorDataTable::new(result)
+	let mut result = VectorDataTable::new(result);
+	*result.transform_mut() = vector_data_transform;
+	result
 }
 
 #[node_macro::node(category("Vector"), path(graphene_core::vector), properties("offset_path_properties"))]
@@ -967,7 +969,7 @@ async fn offset_path(_: impl Ctx, vector_data: VectorDataTable, distance: f64, l
 		subpath.apply_transform(vector_data_transform);
 
 		// Taking the existing stroke data and passing it to Bezier-rs to generate new paths.
-		let subpath_out = subpath.offset(
+		let mut subpath_out = subpath.offset(
 			-distance,
 			match line_join {
 				LineJoin::Miter => Join::Miter(Some(miter_limit)),
@@ -976,11 +978,15 @@ async fn offset_path(_: impl Ctx, vector_data: VectorDataTable, distance: f64, l
 			},
 		);
 
+		subpath_out.apply_transform(vector_data_transform.inverse());
+
 		// One closed subpath, open path.
 		result.append_subpath(subpath_out, false);
 	}
 
-	VectorDataTable::new(result)
+	let mut result = VectorDataTable::new(result);
+	*result.transform_mut() = vector_data_transform;
+	result
 }
 
 #[node_macro::node(category("Vector"), path(graphene_core::vector))]
@@ -988,39 +994,34 @@ async fn solidify_stroke(_: impl Ctx, vector_data: VectorDataTable) -> VectorDat
 	let vector_data_transform = vector_data.transform();
 	let vector_data = vector_data.one_instance().instance;
 
-	let style = &vector_data.style;
+	let stroke = vector_data.style.stroke().clone().unwrap_or_default();
-
 	let subpaths = vector_data.stroke_bezier_paths();
 	let mut result = VectorData::empty();
 
 	// Perform operation on all subpaths in this shape.
-	for mut subpath in subpaths {
+	for subpath in subpaths {
-		let stroke = style.stroke().unwrap();
+		// Taking the existing stroke data and passing it to Bezier-rs to generate new fill paths.
-		subpath.apply_transform(vector_data_transform);
+		let stroke_radius = stroke.weight / 2.;
-
+		let join = match stroke.line_join {
-		// Taking the existing stroke data and passing it to Bezier-rs to generate new paths.
-		let subpath_out = subpath.outline(
-			stroke.weight / 2., // Diameter to radius.
-			match stroke.line_join {
 			LineJoin::Miter => Join::Miter(Some(stroke.line_join_miter_limit)),
 			LineJoin::Bevel => Join::Bevel,
 			LineJoin::Round => Join::Round,
-			},
+		};
-			match stroke.line_cap {
+		let cap = match stroke.line_cap {
-				crate::vector::style::LineCap::Butt => Cap::Butt,
+			LineCap::Butt => Cap::Butt,
-				crate::vector::style::LineCap::Round => Cap::Round,
+			LineCap::Round => Cap::Round,
-				crate::vector::style::LineCap::Square => Cap::Square,
+			LineCap::Square => Cap::Square,
-			},
+		};
-		);
+		let solidified = subpath.outline(stroke_radius, join, cap);
 
 		// This is where we determine whether we have a closed or open path. Ex: Oval vs line segment.
-		if subpath_out.1.is_some() {
+		if solidified.1.is_some() {
 			// Two closed subpaths, closed shape. Add both subpaths.
-			result.append_subpath(subpath_out.0, false);
+			result.append_subpath(solidified.0, false);
-			result.append_subpath(subpath_out.1.unwrap(), false);
+			result.append_subpath(solidified.1.unwrap(), false);
 		} else {
 			// One closed subpath, open path.
-			result.append_subpath(subpath_out.0, false);
+			result.append_subpath(solidified.0, false);
 		}
 	}
 
@@ -1030,7 +1031,9 @@ async fn solidify_stroke(_: impl Ctx, vector_data: VectorDataTable) -> VectorDat
 		result.style.set_stroke(Stroke::default());
 	}
 
-	VectorDataTable::new(result)
+	let mut result = VectorDataTable::new(result);
+	*result.transform_mut() = vector_data_transform;
+	result
 }
 
 #[node_macro::node(category("Vector"), path(graphene_core::vector))]
@@ -1294,7 +1297,7 @@ async fn spline(_: impl Ctx, mut vector_data: VectorDataTable) -> VectorDataTabl
 
 	// Exit early if there are no points to generate splines from.
 	if vector_data.point_domain.positions().is_empty() {
-		return VectorDataTable::new(vector_data.clone());
+		return VectorDataTable::new(VectorData::empty());
 	}
 
 	let mut segment_domain = SegmentDomain::default();
@@ -1337,12 +1340,15 @@ async fn jitter_points(_: impl Ctx, vector_data: VectorDataTable, #[default(5.)]
 	let vector_data_transform = vector_data.transform();
 	let mut vector_data = vector_data.one_instance().instance.clone();
 
+	let inverse_transform = (vector_data_transform.matrix2.determinant() != 0.).then(|| vector_data_transform.inverse()).unwrap_or_default();
+
 	let mut rng = rand::rngs::StdRng::seed_from_u64(seed.into());
 
 	let deltas = (0..vector_data.point_domain.positions().len())
 		.map(|_| {
 			let angle = rng.random::<f64>() * std::f64::consts::TAU;
-			DVec2::from_angle(angle) * rng.random::<f64>() * amount
+
+			inverse_transform.transform_vector2(DVec2::from_angle(angle) * rng.random::<f64>() * amount)
 		})
 		.collect::<Vec<_>>();
 	let mut already_applied = vec![false; vector_data.point_domain.positions().len()];
@@ -1353,21 +1359,19 @@ async fn jitter_points(_: impl Ctx, vector_data: VectorDataTable, #[default(5.)]
 
 		if !already_applied[*start] {
 			let start_position = vector_data.point_domain.positions()[*start];
-			let start_position = vector_data_transform.transform_point2(start_position);
 			vector_data.point_domain.set_position(*start, start_position + start_delta);
 			already_applied[*start] = true;
 		}
 		if !already_applied[*end] {
 			let end_position = vector_data.point_domain.positions()[*end];
-			let end_position = vector_data_transform.transform_point2(end_position);
 			vector_data.point_domain.set_position(*end, end_position + end_delta);
 			already_applied[*end] = true;
 		}
 
 		match handles {
 			bezier_rs::BezierHandles::Cubic { handle_start, handle_end } => {
-				*handle_start = vector_data_transform.transform_point2(*handle_start) + start_delta;
+				*handle_start += start_delta;
-				*handle_end = vector_data_transform.transform_point2(*handle_end) + end_delta;
+				*handle_end += end_delta;
 			}
 			bezier_rs::BezierHandles::Quadratic { handle } => {
 				*handle = vector_data_transform.transform_point2(*handle) + (start_delta + end_delta) / 2.;
@@ -1378,7 +1382,9 @@ async fn jitter_points(_: impl Ctx, vector_data: VectorDataTable, #[default(5.)]
 
 	vector_data.style.set_stroke_transform(DAffine2::IDENTITY);
 
-	VectorDataTable::new(vector_data)
+	let mut result = VectorDataTable::new(vector_data.clone());
+	*result.transform_mut() = vector_data_transform;
+	result
 }
 
 #[node_macro::node(category("Vector"), path(graphene_core::vector))]
@@ -1761,9 +1767,10 @@ mod test {
 		let bounding_box = bounding_box.instances().next().unwrap().instance;
 		assert_eq!(bounding_box.region_bezier_paths().count(), 1);
 		let subpath = bounding_box.region_bezier_paths().next().unwrap().1;
-		let sqrt2 = core::f64::consts::SQRT_2;
+		let expected_bounding_box = [DVec2::NEG_ONE, DVec2::new(1., -1.), DVec2::ONE, DVec2::new(-1., 1.)];
-		let sqrt2_bounding_box = [DVec2::new(-sqrt2, -sqrt2), DVec2::new(sqrt2, -sqrt2), DVec2::new(sqrt2, sqrt2), DVec2::new(-sqrt2, sqrt2)];
+		for i in 0..4 {
-		assert!(subpath.anchors()[..4].iter().zip(sqrt2_bounding_box).all(|(p1, p2)| p1.abs_diff_eq(p2, f64::EPSILON)));
+			assert_eq!(subpath.anchors()[i], expected_bounding_box[i]);
+		}
 	}
 	#[tokio::test]
 	async fn copy_to_points() {