Switch attribute-based vector data from referencing PointIds to indexes in the points table (#1949)

* Index for points

* Code review

* Add todo comment about a possible perf optimization

---------

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

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

@@ -89,7 +89,7 @@ impl OriginalTransforms {
 
 					let selected_handles = selected_points.iter().filter_map(|point| point.as_handle());
 					let anchor_ids = selected_points.iter().filter_map(|point| point.as_anchor());
-					let connected_handles = anchor_ids.flat_map(|point| vector_data.segment_domain.all_connected(point));
+					let connected_handles = anchor_ids.flat_map(|point| vector_data.all_connected(point));
 					let all_handles = selected_handles.chain(connected_handles);
 
 					let handles = all_handles

editor/src/messages/tool/common_functionality/shape_editor.rs

@@ -199,7 +199,7 @@ impl ShapeState {
 				let mut point = SnapCandidatePoint::new_source(to_document.transform_point2(position) + mouse_delta, source);
 
 				if let Some(id) = selected.as_anchor() {
-					for neighbor in vector_data.segment_domain.connected_points(id) {
+					for neighbor in vector_data.connected_points(id) {
 						if state.is_selected(ManipulatorPointId::Anchor(neighbor)) {
 							continue;
 						}
@@ -305,7 +305,7 @@ impl ShapeState {
 			while let Some(point) = selected_stack.pop() {
 				if !state.is_selected(ManipulatorPointId::Anchor(point)) {
 					state.select_point(ManipulatorPointId::Anchor(point));
-					selected_stack.extend(vector_data.segment_domain.connected_points(point));
+					selected_stack.extend(vector_data.connected_points(point));
 				}
 			}
 		}
@@ -399,7 +399,7 @@ impl ShapeState {
 		});
 
 		// Move the other handle for a quadratic bezier
-		for segment in vector_data.segment_domain.end_connected(point) {
+		for segment in vector_data.end_connected(point) {
 			let Some((start, _end, bezier)) = vector_data.segment_points_from_id(segment) else { continue };
 
 			if let BezierHandles::Quadratic { handle } = bezier.handles {
@@ -476,10 +476,10 @@ impl ShapeState {
 		let Some(anchor_position) = ManipulatorPointId::Anchor(point_id).get_position(vector_data) else {
 			return;
 		};
-		let handles = vector_data.segment_domain.all_connected(point_id).take(2).collect::<Vec<_>>();
+		let handles = vector_data.all_connected(point_id).take(2).collect::<Vec<_>>();
 
 		// Grab the next and previous manipulator groups by simply looking at the next / previous index
-		let points = handles.iter().map(|handle| vector_data.segment_domain.other_point(handle.segment, point_id));
+		let points = handles.iter().map(|handle| vector_data.other_point(handle.segment, point_id));
 		let anchor_positions = points
 			.map(|point| point.and_then(|point| ManipulatorPointId::Anchor(point).get_position(vector_data)))
 			.collect::<Vec<_>>();
@@ -556,7 +556,7 @@ impl ShapeState {
 				let Some(anchor_id) = point.get_anchor(&vector_data) else { continue };
 				let Some(anchor) = vector_data.point_domain.position_from_id(anchor_id) else { continue };
 
-				let anchor_points = handles.map(|handle| vector_data.segment_domain.other_point(handle.segment, anchor_id));
+				let anchor_points = handles.map(|handle| vector_data.other_point(handle.segment, anchor_id));
 				let anchor_positions = anchor_points.map(|point| point.and_then(|point| vector_data.point_domain.position_from_id(point)));
 
 				// If one handle is selected (but both exist), only move the other handle
@@ -715,7 +715,7 @@ impl ShapeState {
 		responses.add(GraphOperationMessage::Vector { layer, modification_type });
 
 		// Delete connected segments
-		for HandleId { segment, .. } in vector_data.segment_domain.all_connected(anchor) {
+		for HandleId { segment, .. } in vector_data.all_connected(anchor) {
 			let modification_type = VectorModificationType::RemoveSegment { id: segment };
 			responses.add(GraphOperationMessage::Vector { layer, modification_type });
 			for &handles in vector_data.colinear_manipulators.iter().filter(|handles| handles.iter().any(|handle| handle.segment == segment)) {
@@ -736,14 +736,14 @@ impl ShapeState {
 	}
 
 	/// Dissolve the selected points.
-	pub fn delete_selected_points(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
-		for (&layer, state) in &self.selected_shape_state {
+	pub fn delete_selected_points(&mut self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
+		for (&layer, state) in &mut self.selected_shape_state {
 			let mut missing_anchors = HashMap::new();
 			let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else {
 				continue;
 			};
 
-			for &point in &state.selected_points {
+			for point in std::mem::take(&mut state.selected_points) {
 				match point {
 					ManipulatorPointId::Anchor(anchor) => {
 						if let Some(handles) = Self::dissolve_anchor(anchor, responses, layer, &vector_data) {
@@ -839,7 +839,7 @@ impl ShapeState {
 				let Some(pos) = vector_data.point_domain.position_from_id(point) else { continue };
 
 				let mut used_initial_point = false;
-				for handle in vector_data.segment_domain.all_connected(point) {
+				for handle in vector_data.all_connected(point) {
 					// Disable the g1 continuous
 					for &handles in &vector_data.colinear_manipulators {
 						if handles.contains(&handle) {
@@ -874,13 +874,13 @@ impl ShapeState {
 	}
 
 	/// Delete point(s) and adjacent segments.
-	pub fn delete_point_and_break_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
-		for (&layer, state) in &self.selected_shape_state {
+	pub fn delete_point_and_break_path(&mut self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
+		for (&layer, state) in &mut self.selected_shape_state {
 			let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else {
 				continue;
 			};
 
-			for &delete in &state.selected_points {
+			for delete in std::mem::take(&mut state.selected_points) {
 				let Some(point) = delete.get_anchor(&vector_data) else { continue };
 
 				// Delete point
@@ -888,7 +888,7 @@ impl ShapeState {
 				responses.add(GraphOperationMessage::Vector { layer, modification_type });
 
 				// Delete connected segments
-				for HandleId { segment, .. } in vector_data.segment_domain.all_connected(point) {
+				for HandleId { segment, .. } in vector_data.all_connected(point) {
 					let modification_type = VectorModificationType::RemoveSegment { id: segment };
 					responses.add(GraphOperationMessage::Vector { layer, modification_type });
 				}
@@ -905,7 +905,7 @@ impl ShapeState {
 
 			for &point in &state.selected_points {
 				if let ManipulatorPointId::Anchor(point) = point {
-					for connected in vector_data.segment_domain.all_connected(point) {
+					for connected in vector_data.all_connected(point) {
 						if let Some(&handles) = vector_data.colinear_manipulators.iter().find(|target| target.iter().any(|&target| target == connected)) {
 							let modification_type = VectorModificationType::SetG1Continuous { handles, enabled: false };
 							responses.add(GraphOperationMessage::Vector { layer, modification_type });
@@ -1072,7 +1072,7 @@ impl ShapeState {
 			}
 
 			let (id, anchor) = result?;
-			let handles = vector_data.segment_domain.all_connected(id);
+			let handles = vector_data.all_connected(id);
 			let mut positions = handles
 				.filter_map(|handle| handle.to_manipulator_point().get_position(&vector_data))
 				.filter(|&handle| !anchor.abs_diff_eq(handle, 1e-5));
@@ -1083,7 +1083,7 @@ impl ShapeState {
 			if already_sharp {
 				self.convert_manipulator_handles_to_colinear(&vector_data, id, responses, layer);
 			} else {
-				for handle in vector_data.segment_domain.all_connected(id) {
+				for handle in vector_data.all_connected(id) {
 					let Some(bezier) = vector_data.segment_from_id(handle.segment) else { continue };
 
 					match bezier.handles {
@@ -1141,13 +1141,13 @@ impl ShapeState {
 			let Some(vector_data) = vector_data else { continue };
 			let transform = network_interface.document_metadata().transform_to_viewport(layer);
 
-			assert_eq!(vector_data.segment_domain.ids().len(), vector_data.segment_domain.start_point().len());
-			assert_eq!(vector_data.segment_domain.ids().len(), vector_data.segment_domain.end_point().len());
-			for start in vector_data.segment_domain.start_point() {
-				assert!(vector_data.point_domain.ids().contains(start));
+			assert_eq!(vector_data.segment_domain.ids().len(), vector_data.start_point().count());
+			assert_eq!(vector_data.segment_domain.ids().len(), vector_data.end_point().count());
+			for start in vector_data.start_point() {
+				assert!(vector_data.point_domain.ids().contains(&start));
 			}
-			for end in vector_data.segment_domain.end_point() {
-				assert!(vector_data.point_domain.ids().contains(end));
+			for end in vector_data.end_point() {
+				assert!(vector_data.point_domain.ids().contains(&end));
 			}
 
 			for (id, bezier, _, _) in vector_data.segment_bezier_iter() {

libraries/bezier-rs/src/subpath/core.rs

@@ -10,7 +10,7 @@ impl<PointId: crate::Identifier> Subpath<PointId> {
 	/// A `Subpath` with less than 2 [ManipulatorGroup]s may not be closed.
 	#[track_caller]
 	pub fn new(manipulator_groups: Vec<ManipulatorGroup<PointId>>, closed: bool) -> Self {
-		assert!(!closed || manipulator_groups.len() > 1, "A closed Subpath must contain more than 1 ManipulatorGroup.");
+		assert!(!closed || !manipulator_groups.is_empty(), "A closed Subpath must contain more than 0 ManipulatorGroups.");
 		Self { manipulator_groups, closed }
 	}
 

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

@@ -364,9 +364,6 @@ impl GraphicElementRendered for VectorData {
 		let transformed_bounds = self.bounding_box_with_transform(multiplied_transform).unwrap_or_default();
 
 		let mut path = String::new();
-		for (_, subpath) in self.region_bezier_paths() {
-			let _ = subpath.subpath_to_svg(&mut path, multiplied_transform);
-		}
 		for subpath in self.stroke_bezier_paths() {
 			let _ = subpath.subpath_to_svg(&mut path, multiplied_transform);
 		}
@@ -431,10 +428,6 @@ impl GraphicElementRendered for VectorData {
 		let kurbo_transform = kurbo::Affine::new(transform.to_cols_array());
 		let to_point = |p: DVec2| kurbo::Point::new(p.x, p.y);
 		let mut path = kurbo::BezPath::new();
-		// TODO: Is this correct and efficient? Deesn't this lead to us potentially rendering a path twice?
-		for (_, subpath) in self.region_bezier_paths() {
-			subpath.to_vello_path(self.transform, &mut path);
-		}
 		for subpath in self.stroke_bezier_paths() {
 			subpath.to_vello_path(self.transform, &mut path);
 		}

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

@@ -83,7 +83,7 @@ impl VectorData {
 					point_id.next_id()
 				};
 				self.point_domain.push(id, pair[0].anchor);
-				id
+				self.point_domain.ids().len() - 1
 			});
 			first_point = Some(first_point.unwrap_or(start));
 			let end = if preserve_id && !self.point_domain.ids().contains(&pair[1].id) {
@@ -91,14 +91,15 @@ impl VectorData {
 			} else {
 				point_id.next_id()
 			};
+			let end_index = self.point_domain.ids().len();
 			self.point_domain.push(end, pair[1].anchor);
 
 			let id = segment_id.next_id();
 			first_seg = Some(first_seg.unwrap_or(id));
 			last_seg = Some(id);
-			self.segment_domain.push(id, start, end, handles(&pair[0], &pair[1]), stroke_id);
+			self.segment_domain.push(id, start, end_index, handles(&pair[0], &pair[1]), stroke_id);
 
-			last_point = Some(end);
+			last_point = Some(end_index);
 		}
 
 		let fill_id = FillId::ZERO;
@@ -169,9 +170,83 @@ impl VectorData {
 		self.transform.transform_point2(self.layerspace_pivot(normalized_pivot))
 	}
 
+	pub fn start_point(&self) -> impl Iterator<Item = PointId> + '_ {
+		self.segment_domain.start_point().iter().map(|&index| self.point_domain.ids()[index])
+	}
+
+	pub fn end_point(&self) -> impl Iterator<Item = PointId> + '_ {
+		self.segment_domain.end_point().iter().map(|&index| self.point_domain.ids()[index])
+	}
+
+	pub fn push(&mut self, id: SegmentId, start: PointId, end: PointId, handles: bezier_rs::BezierHandles, stroke: StrokeId) {
+		let [Some(start), Some(end)] = [start, end].map(|id| self.point_domain.resolve_id(id)) else {
+			return;
+		};
+		self.segment_domain.push(id, start, end, handles, stroke)
+	}
+
+	pub fn handles_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut bezier_rs::BezierHandles, PointId, PointId)> {
+		self.segment_domain
+			.handles_mut()
+			.map(|(id, handles, start, end)| (id, handles, self.point_domain.ids()[start], self.point_domain.ids()[end]))
+	}
+
+	pub fn segment_start_from_id(&self, segment: SegmentId) -> Option<PointId> {
+		self.segment_domain.segment_start_from_id(segment).map(|index| self.point_domain.ids()[index])
+	}
+
+	pub fn segment_end_from_id(&self, segment: SegmentId) -> Option<PointId> {
+		self.segment_domain.segment_end_from_id(segment).map(|index| self.point_domain.ids()[index])
+	}
+
+	/// Returns an array for the start and end points of a segment.
+	pub fn points_from_id(&self, segment: SegmentId) -> Option<[PointId; 2]> {
+		self.segment_domain.points_from_id(segment).map(|val| val.map(|index| self.point_domain.ids()[index]))
+	}
+
+	/// Attempts to find another point in the segment that is not the one passed in.
+	pub fn other_point(&self, segment: SegmentId, current: PointId) -> Option<PointId> {
+		let index = self.point_domain.resolve_id(current);
+		index.and_then(|index| self.segment_domain.other_point(segment, index)).map(|index| self.point_domain.ids()[index])
+	}
+
+	/// Gets all points connected to the current one but not including the current one.
+	pub fn connected_points(&self, current: PointId) -> impl Iterator<Item = PointId> + '_ {
+		let index = [self.point_domain.resolve_id(current)].into_iter().flatten();
+		index.flat_map(|index| self.segment_domain.connected_points(index).map(|index| self.point_domain.ids()[index]))
+	}
+
+	/// Enumerate all segments that start at the point.
+	pub fn start_connected(&self, point: PointId) -> impl Iterator<Item = SegmentId> + '_ {
+		let index = [self.point_domain.resolve_id(point)].into_iter().flatten();
+		index.flat_map(|index| self.segment_domain.start_connected(index))
+	}
+
+	/// Enumerate all segments that end at the point.
+	pub fn end_connected(&self, point: PointId) -> impl Iterator<Item = SegmentId> + '_ {
+		let index = [self.point_domain.resolve_id(point)].into_iter().flatten();
+		index.flat_map(|index| self.segment_domain.end_connected(index))
+	}
+
+	/// Enumerate all segments that start or end at a point, converting them to [`HandleId`s]. Note that the handles may not exist e.g. for a linear segment.
+	pub fn all_connected(&self, point: PointId) -> impl Iterator<Item = HandleId> + '_ {
+		let index = [self.point_domain.resolve_id(point)].into_iter().flatten();
+		index.flat_map(|index| self.segment_domain.all_connected(index))
+	}
+
+	/// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice.
+	pub fn connected_count(&self, point: PointId) -> usize {
+		self.point_domain.resolve_id(point).map_or(0, |point| self.segment_domain.connected_count(point))
+	}
+
 	/// Points connected to a single segment
 	pub fn single_connected_points(&self) -> impl Iterator<Item = PointId> + '_ {
-		self.point_domain.ids().iter().copied().filter(|&point| self.segment_domain.connected_count(point) == 1)
+		self.point_domain
+			.ids()
+			.iter()
+			.enumerate()
+			.filter(|(index, _)| self.segment_domain.connected_count(*index) == 1)
+			.map(|(_, &id)| id)
 	}
 
 	/// Computes if all the connected handles are colinear for an anchor, or if that handle is colinear for a handle.
@@ -179,7 +254,7 @@ impl VectorData {
 		let has_handle = |target| self.colinear_manipulators.iter().flatten().any(|&handle| handle == target);
 		match point {
 			ManipulatorPointId::Anchor(id) => {
-				self.segment_domain.start_connected(id).all(|segment| has_handle(HandleId::primary(segment))) && self.segment_domain.end_connected(id).all(|segment| has_handle(HandleId::end(segment)))
+				self.start_connected(id).all(|segment| has_handle(HandleId::primary(segment))) && self.end_connected(id).all(|segment| has_handle(HandleId::end(segment)))
 			}
 			ManipulatorPointId::PrimaryHandle(segment) => has_handle(HandleId::primary(segment)),
 			ManipulatorPointId::EndHandle(segment) => has_handle(HandleId::end(segment)),
@@ -218,6 +293,7 @@ pub enum ManipulatorPointId {
 impl ManipulatorPointId {
 	/// Attempt to retrieve the manipulator position in layer space (no transformation applied).
 	#[must_use]
+	#[track_caller]
 	pub fn get_position(&self, vector_data: &VectorData) -> Option<DVec2> {
 		match self {
 			ManipulatorPointId::Anchor(id) => vector_data.point_domain.position_from_id(*id),
@@ -230,7 +306,7 @@ impl ManipulatorPointId {
 	#[must_use]
 	pub fn get_handle_pair(self, vector_data: &VectorData) -> Option<[HandleId; 2]> {
 		match self {
-			ManipulatorPointId::Anchor(point) => vector_data.segment_domain.all_connected(point).take(2).collect::<Vec<_>>().try_into().ok(),
+			ManipulatorPointId::Anchor(point) => vector_data.all_connected(point).take(2).collect::<Vec<_>>().try_into().ok(),
 			ManipulatorPointId::PrimaryHandle(segment) => {
 				let point = vector_data.segment_domain.segment_start_from_id(segment)?;
 				let current = HandleId::primary(segment);
@@ -251,8 +327,8 @@ impl ManipulatorPointId {
 	pub fn get_anchor(self, vector_data: &VectorData) -> Option<PointId> {
 		match self {
 			ManipulatorPointId::Anchor(point) => Some(point),
-			ManipulatorPointId::PrimaryHandle(segment) => vector_data.segment_domain.segment_start_from_id(segment),
-			ManipulatorPointId::EndHandle(segment) => vector_data.segment_domain.segment_end_from_id(segment),
+			ManipulatorPointId::PrimaryHandle(segment) => vector_data.segment_start_from_id(segment),
+			ManipulatorPointId::EndHandle(segment) => vector_data.segment_end_from_id(segment),
 		}
 	}
 

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

@@ -95,9 +95,26 @@ impl PointDomain {
 		self.positions.clear();
 	}
 
-	pub fn retain(&mut self, f: impl Fn(&PointId) -> bool) {
+	pub fn retain(&mut self, segment_domain: &mut SegmentDomain, f: impl Fn(&PointId) -> bool) {
 		let mut keep = self.id.iter().map(&f);
 		self.positions.retain(|_| keep.next().unwrap_or_default());
+
+		// TODO(TrueDoctor): Consider using a prefix sum to avoid this Vec allocation (https://github.com/GraphiteEditor/Graphite/pull/1949#discussion_r1741711562)
+		let mut id_map = Vec::with_capacity(self.ids().len());
+		let mut new_index = 0;
+		for id in self.ids() {
+			if f(id) {
+				id_map.push(new_index);
+				new_index += 1;
+			} else {
+				id_map.push(usize::MAX); // A placeholder for invalid ids. This is checked after the segment domain is modified.
+			}
+		}
+
+		let update_index = |index: &mut usize| *index = id_map[*index];
+		segment_domain.start_point.iter_mut().for_each(update_index);
+		segment_domain.end_point.iter_mut().for_each(update_index);
+
 		self.id.retain(f);
 	}
 
@@ -126,6 +143,7 @@ impl PointDomain {
 		self.ids().iter().copied().max_by(|a, b| a.0.cmp(&b.0)).map(|mut id| id.next_id()).unwrap_or(PointId::ZERO)
 	}
 
+	#[track_caller]
 	pub fn position_from_id(&self, id: PointId) -> Option<DVec2> {
 		let pos = self.resolve_id(id).map(|index| self.positions[index]);
 		if pos.is_none() {
@@ -134,7 +152,7 @@ impl PointDomain {
 		pos
 	}
 
-	fn resolve_id(&self, id: PointId) -> Option<usize> {
+	pub(crate) fn resolve_id(&self, id: PointId) -> Option<usize> {
 		self.id.iter().position(|&check_id| check_id == id)
 	}
 
@@ -155,8 +173,8 @@ impl PointDomain {
 /// Stores data which is per-segment. A segment is a bézier curve between two end points with a stroke. In future this will be extendable at runtime with custom attributes.
 pub struct SegmentDomain {
 	ids: Vec<SegmentId>,
-	start_point: Vec<PointId>,
-	end_point: Vec<PointId>,
+	start_point: Vec<usize>,
+	end_point: Vec<usize>,
 	handles: Vec<bezier_rs::BezierHandles>,
 	stroke: Vec<StrokeId>,
 }
@@ -200,11 +218,11 @@ impl SegmentDomain {
 		self.ids().iter().copied().max_by(|a, b| a.0.cmp(&b.0)).map(|mut id| id.next_id()).unwrap_or(SegmentId::ZERO)
 	}
 
-	pub fn start_point(&self) -> &[PointId] {
+	pub(crate) fn start_point(&self) -> &[usize] {
 		&self.start_point
 	}
 
-	pub fn end_point(&self) -> &[PointId] {
+	pub(crate) fn end_point(&self) -> &[usize] {
 		&self.end_point
 	}
 
@@ -216,35 +234,34 @@ impl SegmentDomain {
 		&self.stroke
 	}
 
-	pub fn push(&mut self, id: SegmentId, start: PointId, end: PointId, handles: bezier_rs::BezierHandles, stroke: StrokeId) {
+	pub(crate) fn push(&mut self, id: SegmentId, start: usize, end: usize, handles: bezier_rs::BezierHandles, stroke: StrokeId) {
 		if self.ids.contains(&id) {
-			warn!("Duplicate segment");
 			return;
 		}
 		// Attempt to keep line joins?
-		let after = self.end_point.iter().copied().position(|other_end| other_end == start || other_end == end);
-		let before = self.start_point.iter().copied().position(|other_start| other_start == start || other_start == end);
-		let (index, flip) = match (before, after) {
-			(_, Some(after)) => (after + 1, self.end_point[after] == end),
-			(Some(before), _) => (before, self.start_point[before] == start),
-			(None, None) => (self.ids.len(), false),
+		let after = self.end_point.iter().copied().position(|other_end| other_end == start);
+		let before = self.start_point.iter().copied().position(|other_start| other_start == end);
+		let index = match (before, after) {
+			(_, Some(after)) => after + 1,
+			(Some(before), _) => before,
+			(None, None) => self.ids.len(),
 		};
 		self.ids.insert(index, id);
-		self.start_point.insert(index, if flip { end } else { start });
-		self.end_point.insert(index, if flip { start } else { end });
-		self.handles.insert(index, if flip { handles.flipped() } else { handles });
+		self.start_point.insert(index, start);
+		self.end_point.insert(index, end);
+		self.handles.insert(index, handles);
 		self.stroke.insert(index, stroke);
 	}
 
-	pub fn start_point_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut PointId)> {
+	pub(crate) fn start_point_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut usize)> {
 		self.ids.iter().copied().zip(self.start_point.iter_mut())
 	}
 
-	pub fn end_point_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut PointId)> {
+	pub(crate) fn end_point_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut usize)> {
 		self.ids.iter().copied().zip(self.end_point.iter_mut())
 	}
 
-	pub fn handles_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut bezier_rs::BezierHandles, PointId, PointId)> {
+	pub(crate) fn handles_mut(&mut self) -> impl Iterator<Item = (SegmentId, &mut bezier_rs::BezierHandles, usize, usize)> {
 		let nested = self.ids.iter().zip(&mut self.handles).zip(&self.start_point).zip(&self.end_point);
 		nested.map(|(((&a, b), &c), &d)| (a, b, c, d))
 	}
@@ -253,26 +270,26 @@ impl SegmentDomain {
 		self.ids.iter().copied().zip(self.stroke.iter_mut())
 	}
 
-	pub fn segment_start_from_id(&self, segment: SegmentId) -> Option<PointId> {
+	pub(crate) fn segment_start_from_id(&self, segment: SegmentId) -> Option<usize> {
 		self.id_to_index(segment).and_then(|index| self.start_point.get(index)).copied()
 	}
 
-	pub fn segment_end_from_id(&self, segment: SegmentId) -> Option<PointId> {
+	pub(crate) fn segment_end_from_id(&self, segment: SegmentId) -> Option<usize> {
 		self.id_to_index(segment).and_then(|index| self.end_point.get(index)).copied()
 	}
 
 	/// Returns an array for the start and end points of a segment.
-	pub fn points_from_id(&self, segment: SegmentId) -> Option<[PointId; 2]> {
+	pub(crate) fn points_from_id(&self, segment: SegmentId) -> Option<[usize; 2]> {
 		self.segment_start_from_id(segment).and_then(|start| self.segment_end_from_id(segment).map(|end| [start, end]))
 	}
 
 	/// Attempts to find another point in the segment that is not the one passed in.
-	pub fn other_point(&self, segment: SegmentId, current: PointId) -> Option<PointId> {
+	pub(crate) fn other_point(&self, segment: SegmentId, current: usize) -> Option<usize> {
 		self.points_from_id(segment).and_then(|points| points.into_iter().find(|&point| point != current))
 	}
 
 	/// Gets all points connected to the current one but not including the current one.
-	pub fn connected_points(&self, current: PointId) -> impl Iterator<Item = PointId> + '_ {
+	pub(crate) fn connected_points(&self, current: usize) -> impl Iterator<Item = usize> + '_ {
 		self.start_point.iter().zip(&self.end_point).filter_map(move |(&a, &b)| match (a == current, b == current) {
 			(true, false) => Some(b),
 			(false, true) => Some(a),
@@ -299,8 +316,8 @@ impl SegmentDomain {
 
 	fn concat(&mut self, other: &Self, transform: DAffine2, id_map: &IdMap) {
 		self.ids.extend(other.ids.iter().map(|id| *id_map.segment_map.get(id).unwrap_or(id)));
-		self.start_point.extend(other.start_point.iter().map(|id| *id_map.point_map.get(id).unwrap_or(id)));
-		self.end_point.extend(other.end_point.iter().map(|id| *id_map.point_map.get(id).unwrap_or(id)));
+		self.start_point.extend(other.start_point.iter().map(|&index| id_map.point_offset + index));
+		self.end_point.extend(other.end_point.iter().map(|&index| id_map.point_offset + index));
 		self.handles.extend(other.handles.iter().map(|handles| handles.apply_transformation(|p| transform.transform_point2(p))));
 		self.stroke.extend(&other.stroke);
 	}
@@ -312,22 +329,22 @@ impl SegmentDomain {
 	}
 
 	/// Enumerate all segments that start at the point.
-	pub fn start_connected(&self, point: PointId) -> impl Iterator<Item = SegmentId> + '_ {
+	pub(crate) fn start_connected(&self, point: usize) -> impl Iterator<Item = SegmentId> + '_ {
 		self.start_point.iter().zip(&self.ids).filter(move |&(&found_point, _)| found_point == point).map(|(_, &seg)| seg)
 	}
 
 	/// Enumerate all segments that end at the point.
-	pub fn end_connected(&self, point: PointId) -> impl Iterator<Item = SegmentId> + '_ {
+	pub(crate) fn end_connected(&self, point: usize) -> impl Iterator<Item = SegmentId> + '_ {
 		self.end_point.iter().zip(&self.ids).filter(move |&(&found_point, _)| found_point == point).map(|(_, &seg)| seg)
 	}
 
 	/// Enumerate all segments that start or end at a point, converting them to [`HandleId`s]. Note that the handles may not exist e.g. for a linear segment.
-	pub fn all_connected(&self, point: PointId) -> impl Iterator<Item = HandleId> + '_ {
+	pub(crate) fn all_connected(&self, point: usize) -> impl Iterator<Item = HandleId> + '_ {
 		self.start_connected(point).map(HandleId::primary).chain(self.end_connected(point).map(HandleId::end))
 	}
 
 	/// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice.
-	pub fn connected_count(&self, point: PointId) -> usize {
+	pub(crate) fn connected_count(&self, point: usize) -> usize {
 		self.all_connected(point).count()
 	}
 }
@@ -415,11 +432,11 @@ impl RegionDomain {
 }
 
 impl super::VectorData {
-	/// Construct a [`bezier_rs::Bezier`] curve spanning from the resolved position of the start and end points with the specified handles. Returns [`None`] if either ID is invalid.
-	fn segment_to_bezier(&self, start: PointId, end: PointId, handles: bezier_rs::BezierHandles) -> Option<bezier_rs::Bezier> {
-		let start = self.point_domain.position_from_id(start)?;
-		let end = self.point_domain.position_from_id(end)?;
-		Some(bezier_rs::Bezier { start, end, handles })
+	/// 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: bezier_rs::BezierHandles) -> bezier_rs::Bezier {
+		let start = self.point_domain.positions()[start];
+		let end = self.point_domain.positions()[end];
+		bezier_rs::Bezier { start, end, handles }
 	}
 
 	/// Tries to convert a segment with the specified id to a [`bezier_rs::Bezier`], returning None if the id is invalid.
@@ -432,26 +449,28 @@ impl super::VectorData {
 		let index: usize = self.segment_domain.id_to_index(id)?;
 		let start = self.segment_domain.start_point[index];
 		let end = self.segment_domain.end_point[index];
-		Some((start, end, self.segment_to_bezier(start, end, self.segment_domain.handles[index])?))
+		let start_id = self.point_domain.ids()[start];
+		let end_id = self.point_domain.ids()[end];
+		Some((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_bezier_iter(&self) -> impl Iterator<Item = (SegmentId, bezier_rs::Bezier, PointId, PointId)> + '_ {
-		let to_bezier = |(((&handles, &id), &start), &end)| self.segment_to_bezier(start, end, handles).map(|bezier| (id, bezier, start, end));
+		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]);
 		self.segment_domain
 			.handles
 			.iter()
 			.zip(&self.segment_domain.ids)
-			.zip(&self.segment_domain.start_point)
-			.zip(&self.segment_domain.end_point)
-			.filter_map(to_bezier)
+			.zip(self.segment_domain.start_point())
+			.zip(self.segment_domain.end_point())
+			.map(to_bezier)
 	}
 
 	/// 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 = (bezier_rs::BezierHandles, PointId, PointId)>) -> Option<bezier_rs::Subpath<PointId>> {
+	fn subpath_from_segments(&self, segments: impl Iterator<Item = (bezier_rs::BezierHandles, usize, usize)>) -> Option<bezier_rs::Subpath<PointId>> {
 		let mut first_point = None;
 		let mut groups = Vec::new();
-		let mut last: Option<(PointId, bezier_rs::BezierHandles)> = None;
+		let mut last: Option<(usize, bezier_rs::BezierHandles)> = None;
 
 		for (handle, start, end) in segments {
 			if last.is_some_and(|(previous_end, _)| previous_end != start) {
@@ -461,10 +480,10 @@ impl super::VectorData {
 			first_point = Some(first_point.unwrap_or(start));
 
 			groups.push(bezier_rs::ManipulatorGroup {
-				anchor: self.point_domain.position_from_id(start)?,
+				anchor: self.point_domain.positions()[start],
 				in_handle: last.and_then(|(_, handle)| handle.end()),
 				out_handle: handle.start(),
-				id: start,
+				id: self.point_domain.ids()[start],
 			});
 
 			last = Some((end, handle));
@@ -477,10 +496,10 @@ impl super::VectorData {
 				groups[0].in_handle = last_handle.end();
 			} else {
 				groups.push(bezier_rs::ManipulatorGroup {
-					anchor: self.point_domain.position_from_id(end)?,
+					anchor: self.point_domain.positions()[end],
 					in_handle: last_handle.end(),
 					out_handle: None,
-					id: end,
+					id: self.point_domain.ids()[end],
 				});
 			}
 		}
@@ -510,7 +529,18 @@ impl super::VectorData {
 
 	/// Construct a [`bezier_rs::Bezier`] curve for stroke.
 	pub fn stroke_bezier_paths(&self) -> StrokePathIter<'_> {
-		StrokePathIter { vector_data: self, segment_index: 0 }
+		let mut points = vec![StrokePathIterPointMetadata::default(); self.point_domain.ids().len()];
+		for (segment_index, (&start, &end)) in self.segment_domain.start_point.iter().zip(&self.segment_domain.end_point).enumerate() {
+			points[start].set(StrokePathIterPointSegmentMetadata::new(segment_index, false));
+			points[end].set(StrokePathIterPointSegmentMetadata::new(segment_index, true));
+		}
+
+		StrokePathIter {
+			vector_data: self,
+			points,
+			skip: 0,
+			done_one: false,
+		}
 	}
 
 	/// Construct an iterator [`bezier_rs::ManipulatorGroup`] for stroke.
@@ -531,37 +561,121 @@ impl super::VectorData {
 	}
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+struct StrokePathIterPointSegmentMetadata {
+	segment_index: usize,
+	start_from_end: bool,
+}
+
+impl StrokePathIterPointSegmentMetadata {
+	#[must_use]
+	const fn new(segment_index: usize, start_from_end: bool) -> Self {
+		Self { segment_index, start_from_end }
+	}
+	#[must_use]
+	const fn flipped(&self) -> Self {
+		Self {
+			segment_index: self.segment_index,
+			start_from_end: !self.start_from_end,
+		}
+	}
+}
+
+#[derive(Clone, Default)]
+struct StrokePathIterPointMetadata([Option<StrokePathIterPointSegmentMetadata>; 2]);
+
+impl StrokePathIterPointMetadata {
+	fn set(&mut self, value: StrokePathIterPointSegmentMetadata) {
+		if self.0[0].is_none() {
+			self.0[0] = Some(value)
+		} else if self.0[1].is_none() {
+			self.0[1] = Some(value);
+		} else {
+			panic!("Mesh networks are not supported");
+		}
+	}
+	#[must_use]
+	fn connected(&self) -> usize {
+		self.0.iter().filter(|val| val.is_some()).count()
+	}
+	#[must_use]
+	fn take_first(&mut self) -> Option<StrokePathIterPointSegmentMetadata> {
+		self.0[0].take().or_else(|| self.0[1].take())
+	}
+	fn take_eq(&mut self, target: StrokePathIterPointSegmentMetadata) -> bool {
+		self.0[0].take_if(|&mut value| value == target).or_else(|| self.0[1].take_if(|&mut value| value == target)).is_some()
+	}
+}
+
 #[derive(Clone)]
 pub struct StrokePathIter<'a> {
 	vector_data: &'a super::VectorData,
-	segment_index: usize,
+	points: Vec<StrokePathIterPointMetadata>,
+	skip: usize,
+	done_one: bool,
 }
 
 impl<'a> Iterator for StrokePathIter<'a> {
 	type Item = bezier_rs::Subpath<PointId>;
 
 	fn next(&mut self) -> Option<Self::Item> {
-		let segments = &self.vector_data.segment_domain;
-		if self.segment_index >= segments.end_point.len() {
-			return None;
-		}
-		let mut old_end = None;
-		let mut count = 0;
-		let segments_iter = segments.handles[self.segment_index..]
-			.iter()
-			.zip(&segments.start_point[self.segment_index..])
-			.zip(&segments.end_point[self.segment_index..])
-			.map(|((&handles, &start), &end)| (handles, start, end))
-			.take_while(|&(_, start, end)| {
-				let continuous = old_end.is_none() || old_end.is_some_and(|old_end| old_end == start);
-				old_end = Some(end);
-				continuous
-			})
-			.inspect(|_| count += 1);
+		let current_start = if let Some((index, _)) = self.points.iter().enumerate().skip(self.skip).find(|(_, val)| val.connected() == 1) {
+			index
+		} else {
+			if !self.done_one {
+				self.done_one = true;
+				self.skip = 0;
+			}
+			self.points.iter().enumerate().skip(self.skip).find(|(_, val)| val.connected() > 0)?.0
+		};
+		self.skip = current_start + 1;
 
-		let subpath = self.vector_data.subpath_from_segments(segments_iter);
-		self.segment_index += count;
-		subpath
+		// There will always be one (seeing as we checked above)
+		let mut point_index = current_start;
+		let mut groups = Vec::new();
+		let mut in_handle = None;
+		let mut closed = false;
+		loop {
+			let Some(val) = self.points[point_index].take_first() else {
+				// Dead end
+				groups.push(bezier_rs::ManipulatorGroup {
+					anchor: self.vector_data.point_domain.positions()[point_index],
+					in_handle,
+					out_handle: None,
+					id: self.vector_data.point_domain.ids()[point_index],
+				});
+
+				break;
+			};
+
+			let mut handles = self.vector_data.segment_domain.handles()[val.segment_index];
+			if val.start_from_end {
+				handles = handles.flipped();
+			}
+			let next_point_index = if val.start_from_end {
+				self.vector_data.segment_domain.start_point()[val.segment_index]
+			} else {
+				self.vector_data.segment_domain.end_point()[val.segment_index]
+			};
+			groups.push(bezier_rs::ManipulatorGroup {
+				anchor: self.vector_data.point_domain.positions()[point_index],
+				in_handle,
+				out_handle: handles.start(),
+				id: self.vector_data.point_domain.ids()[point_index],
+			});
+
+			in_handle = handles.end();
+
+			point_index = next_point_index;
+			self.points[next_point_index].take_eq(val.flipped());
+			if next_point_index == current_start {
+				closed = true;
+				groups[0].in_handle = in_handle;
+				break;
+			}
+		}
+
+		Some(bezier_rs::Subpath::new(groups, closed))
 	}
 }
 
@@ -584,7 +698,12 @@ impl crate::vector::ConcatElement for super::VectorData {
 		let segment_map = new_ids.collect::<HashMap<_, _>>();
 		let new_ids = other.region_domain.ids.iter().filter(|id| self.region_domain.ids.contains(id)).map(|&old| (old, RegionId::generate()));
 		let region_map = new_ids.collect::<HashMap<_, _>>();
-		let id_map = IdMap { point_map, segment_map, region_map };
+		let id_map = IdMap {
+			point_offset: self.point_domain.ids().len(),
+			point_map,
+			segment_map,
+			region_map,
+		};
 		self.point_domain.concat(&other.point_domain, transform * other.transform, &id_map);
 		self.segment_domain.concat(&other.segment_domain, transform * other.transform, &id_map);
 		self.region_domain.concat(&other.region_domain, transform * other.transform, &id_map);
@@ -597,6 +716,7 @@ impl crate::vector::ConcatElement for super::VectorData {
 
 /// Represents the conversion of ids used when concatenating vector data with conflicting ids.
 struct IdMap {
+	point_offset: usize,
 	point_map: HashMap<PointId, PointId>,
 	segment_map: HashMap<SegmentId, SegmentId>,
 	region_map: HashMap<RegionId, RegionId>,

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

@@ -27,9 +27,9 @@ impl Hash for PointModification {
 impl PointModification {
 	/// Apply this modification to the specified [`PointDomain`].
 	pub fn apply(&self, point_domain: &mut PointDomain, segment_domain: &mut SegmentDomain) {
-		point_domain.retain(|id| !self.remove.contains(id));
+		point_domain.retain(segment_domain, |id| !self.remove.contains(id));
 
-		for (id, position) in point_domain.positions_mut() {
+		for (index, (id, position)) in point_domain.positions_mut().enumerate() {
 			let Some(&delta) = self.delta.get(&id) else { continue };
 			if !delta.is_finite() {
 				warn!("Invalid delta when applying a point modification");
@@ -39,10 +39,10 @@ impl PointModification {
 			*position += delta;
 
 			for (_, handles, start, end) in segment_domain.handles_mut() {
-				if start == id {
+				if start == index {
 					handles.move_start(delta);
 				}
-				if end == id {
+				if end == index {
 					handles.move_end(delta);
 				}
 			}
@@ -105,27 +105,27 @@ impl SegmentModification {
 
 		for (id, point) in segment_domain.start_point_mut() {
 			let Some(&new) = self.start_point.get(&id) else { continue };
-			if !point_domain.ids().contains(&new) {
+			let Some(index) = point_domain.resolve_id(new) else {
 				warn!("Invalid start ID when applying a segment modification");
 				continue;
-			}
+			};
 
-			*point = new;
+			*point = index;
 		}
 
 		for (id, point) in segment_domain.end_point_mut() {
 			let Some(&new) = self.end_point.get(&id) else { continue };
-			if !point_domain.ids().contains(&new) {
+			let Some(index) = point_domain.resolve_id(new) else {
 				warn!("Invalid end ID when applying a segment modification");
 				continue;
-			}
+			};
 
-			*point = new;
+			*point = index;
 		}
 
 		for (id, handles, start, end) in segment_domain.handles_mut() {
-			let Some(start) = point_domain.position_from_id(start) else { continue };
-			let Some(end) = point_domain.position_from_id(end) else { continue };
+			let Some(&start) = point_domain.positions().get(start) else { continue };
+			let Some(&end) = point_domain.positions().get(end) else { continue };
 
 			// Compute the actual start and end position based on the offset from the anchor
 			let start = self.handle_primary.get(&id).copied().map(|handle| handle.map(|handle| handle + start));
@@ -178,17 +178,17 @@ impl SegmentModification {
 			let Some(&handle_end) = self.handle_end.get(&add_id) else { continue };
 			let Some(&stroke) = self.stroke.get(&add_id) else { continue };
 
-			if !point_domain.ids().contains(&start) {
+			let Some(start_index) = point_domain.resolve_id(start) else {
 				warn!("invalid start id");
 				continue;
-			}
-			if !point_domain.ids().contains(&end) {
+			};
+			let Some(end_index) = point_domain.resolve_id(end) else {
 				warn!("invalid end id");
 				continue;
-			}
+			};
 
-			let Some(start_position) = point_domain.position_from_id(start) else { continue };
-			let Some(end_position) = point_domain.position_from_id(end) else { continue };
+			let start_position = point_domain.positions()[start_index];
+			let end_position = point_domain.positions()[end_index];
 			let handles = match (handle_start, handle_end) {
 				(Some(handle_start), Some(handle_end)) => BezierHandles::Cubic {
 					handle_start: handle_start + start_position,
@@ -203,17 +203,21 @@ impl SegmentModification {
 				continue;
 			}
 
-			segment_domain.push(add_id, start, end, handles, stroke);
+			segment_domain.push(add_id, start_index, end_index, handles, stroke);
 		}
+
+		assert!(segment_domain.start_point().iter().all(|&index| index < point_domain.ids().len()), "index should be in range");
+		assert!(segment_domain.end_point().iter().all(|&index| index < point_domain.ids().len()), "index should be in range");
 	}
 
 	/// Create a new modification that will convert an empty [`VectorData`] into the target [`VectorData`].
 	pub fn create_from_vector(vector_data: &VectorData) -> Self {
+		let point_id = |(&segment, &index)| (segment, vector_data.point_domain.ids()[index]);
 		Self {
 			add: vector_data.segment_domain.ids().to_vec(),
 			remove: HashSet::new(),
-			start_point: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.start_point().iter().cloned()).collect(),
-			end_point: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.end_point().iter().cloned()).collect(),
+			start_point: vector_data.segment_domain.ids().iter().zip(vector_data.segment_domain.start_point()).map(point_id).collect(),
+			end_point: vector_data.segment_domain.ids().iter().zip(vector_data.segment_domain.end_point()).map(point_id).collect(),
 			handle_primary: vector_data.segment_bezier_iter().map(|(id, b, _, _)| (id, b.handle_start().map(|handle| handle - b.start))).collect(),
 			handle_end: vector_data.segment_bezier_iter().map(|(id, b, _, _)| (id, b.handle_end().map(|handle| handle - b.end))).collect(),
 			stroke: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.stroke().iter().cloned()).collect(),

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

@@ -520,9 +520,7 @@ fn splines_from_points(mut vector_data: VectorData) -> VectorData {
 		let handle_end = points.positions()[end_index] * 2. - first_handles[end_index];
 		let handles = bezier_rs::BezierHandles::Cubic { handle_start, handle_end };
 
-		vector_data
-			.segment_domain
-			.push(SegmentId::generate(), points.ids()[start_index], points.ids()[end_index], handles, stroke_id)
+		vector_data.segment_domain.push(SegmentId::generate(), start_index, end_index, handles, stroke_id)
 	}
 
 	vector_data