Add path closing and segment extension to the Pen tool (#753)

* Close paths with pen tool

* Issue Z command always at end of path

* Small code review style changes

* Extending paths

* Fix mirror on extend path

* Code review tweaks

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

editor/src/messages/tool/tool_messages/pen_tool.rs

@@ -12,7 +12,6 @@ use crate::messages::tool::utility_types::{HintData, HintGroup, HintInfo};
 use graphene::layers::style;
 use graphene::layers::vector::consts::ManipulatorType;
 use graphene::layers::vector::manipulator_group::ManipulatorGroup;
-use graphene::layers::vector::subpath::Subpath;
 use graphene::LayerId;
 use graphene::Operation;
 
@@ -170,6 +169,9 @@ struct PenToolData {
 	path: Option<Vec<LayerId>>,
 	overlay_renderer: OverlayRenderer,
 	snap_manager: SnapManager,
+	should_mirror: bool,
+	// Indicates that curve extension is occurring from the first point, rather than (more commonly) the last point
+	from_start: bool,
 }
 
 impl Fsm for PenToolFsmState {
@@ -205,13 +207,45 @@ impl Fsm for PenToolFsmState {
 				}
 				(PenToolFsmState::Ready, PenToolMessage::DragStart) => {
 					responses.push_back(DocumentMessage::StartTransaction.into());
-					responses.push_back(DocumentMessage::DeselectAllLayers.into());
 
-					// Create a new layer and prep snap system
-					tool_data.path = Some(document.get_path_for_new_layer());
+					// Initialize snapping
 					tool_data.snap_manager.start_snap(document, document.bounding_boxes(None, None, font_cache), true, true);
 					tool_data.snap_manager.add_all_document_handles(document, &[], &[], &[]);
-					let snapped_position = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
+
+					// Disable this tool's mirroring
+					tool_data.should_mirror = false;
+
+					// Perform extension of an existing path
+					if let Some((layer, from_start)) = should_extend(document, input.mouse.position, crate::consts::SNAP_POINT_TOLERANCE) {
+						tool_data.path = Some(layer.to_vec());
+						tool_data.from_start = from_start;
+
+						// Stop the handles on the first point from mirroring
+						let mut stop_mirror = || {
+							let subpath = document.graphene_document.layer(layer).ok().and_then(|layer| layer.as_subpath())?;
+							let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+							let (&id, _) = if from_start { manipulator_groups.next()? } else { manipulator_groups.next_back()? };
+
+							let op = Operation::SetManipulatorHandleMirroring {
+								layer_path: layer.to_vec(),
+								id,
+								mirror_distance: false,
+								mirror_angle: false,
+							};
+							responses.push_back(op.into());
+							Some(())
+						};
+						stop_mirror();
+
+						return PenToolFsmState::DraggingHandle;
+					}
+
+					// Deselect layers because we are now creating a new layer
+					responses.push_back(DocumentMessage::DeselectAllLayers.into());
+
+					// Create a new layer
+					tool_data.path = Some(document.get_path_for_new_layer());
+					tool_data.from_start = false;
 
 					// Get the position and set properties
 					let transform = tool_data
@@ -219,6 +253,7 @@ impl Fsm for PenToolFsmState {
 						.as_ref()
 						.and_then(|path| document.graphene_document.multiply_transforms(&path[..path.len() - 1]).ok())
 						.unwrap_or_default();
+					let snapped_position = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
 					let start_position = transform.inverse().transform_point2(snapped_position);
 					tool_data.weight = tool_options.line_weight;
 
@@ -236,114 +271,245 @@ impl Fsm for PenToolFsmState {
 						);
 						responses.push_back(add_manipulator_group(
 							&tool_data.path,
+							tool_data.from_start,
 							ManipulatorGroup::new_with_handles(start_position, Some(start_position), Some(start_position)),
 						));
 					}
 
+					// Enter the dragging handle state while the mouse is held down, allowing the user to move the mouse and position the handle
 					PenToolFsmState::DraggingHandle
 				}
 				(PenToolFsmState::PlacingAnchor, PenToolMessage::DragStart) => PenToolFsmState::DraggingHandle,
 				(PenToolFsmState::DraggingHandle, PenToolMessage::DragStop) => {
-					// Add new point onto path
-					if let Some(layer_path) = &tool_data.path {
-						if let Some(manipulator_group) = get_subpath(layer_path, document).and_then(|subpath| subpath.manipulator_groups().last()) {
-							if let Some(out_handle) = &manipulator_group.points[ManipulatorType::OutHandle] {
-								responses.push_back(add_manipulator_group(&tool_data.path, ManipulatorGroup::new_with_anchor(out_handle.position)));
-							}
-						}
-					}
+					let mut process = || {
+						// Get subpath
+						let layer_path = tool_data.path.as_ref()?;
+						let subpath = document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())?;
 
-					PenToolFsmState::PlacingAnchor
+						// Get the last manipulator group and the one previous to that
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (&last_id, last_manipulator_group) = if tool_data.from_start { manipulator_groups.next()? } else { manipulator_groups.next_back()? };
+						let previous = if tool_data.from_start { manipulator_groups.next() } else { manipulator_groups.next_back() };
+
+						// Get the first manipulator group
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (&first_id, first_manipulator_group) = if tool_data.from_start { manipulator_groups.next_back()? } else { manipulator_groups.next()? };
+
+						// Get correct handle types
+						let inwards_handle = if tool_data.from_start { ManipulatorType::OutHandle } else { ManipulatorType::InHandle };
+						let outwards_handle = if tool_data.from_start { ManipulatorType::InHandle } else { ManipulatorType::OutHandle };
+
+						// Get manipulator points
+						let last_anchor = last_manipulator_group.points[ManipulatorType::Anchor].as_ref()?;
+						let first_anchor = first_manipulator_group.points[ManipulatorType::Anchor].as_ref()?;
+						let last_in = last_manipulator_group.points[inwards_handle].as_ref()?;
+
+						// Close path
+						let transformed_distance_between_squared = transform.transform_point2(last_anchor.position).distance_squared(transform.transform_point2(first_anchor.position));
+						let snap_point_tolerance_squared = crate::consts::SNAP_POINT_TOLERANCE.powi(2);
+						if transformed_distance_between_squared < snap_point_tolerance_squared && previous.is_some() {
+							// Move the in handle of the first point to where the user has placed it
+							let op = Operation::MoveManipulatorPoint {
+								layer_path: layer_path.clone(),
+								id: first_id,
+								manipulator_type: inwards_handle,
+								position: last_in.position.into(),
+							};
+							responses.push_back(op.into());
+
+							// Stop the handles on the first point from mirroring
+							let op = Operation::SetManipulatorHandleMirroring {
+								layer_path: layer_path.clone(),
+								id: first_id,
+								mirror_distance: false,
+								mirror_angle: false,
+							};
+							responses.push_back(op.into());
+
+							// Remove the point that has just been placed
+							let op = Operation::RemoveManipulatorGroup {
+								layer_path: layer_path.clone(),
+								id: last_id,
+							};
+							responses.push_back(op.into());
+
+							// Push a close path node
+							responses.push_back(add_manipulator_group(&tool_data.path, tool_data.from_start, ManipulatorGroup::closed()));
+
+							responses.push_back(DocumentMessage::CommitTransaction.into());
+
+							// Clean up overlays
+							for layer_path in document.all_layers() {
+								tool_data.overlay_renderer.clear_subpath_overlays(&document.graphene_document, layer_path.to_vec(), responses);
+							}
+
+							// Clean up tool data
+							tool_data.path = None;
+							tool_data.snap_manager.cleanup(responses);
+
+							// Return the new tool state, wrapped in `Some()` because this closure returns an Option used by the `?` operation various times above
+							return Some(PenToolFsmState::Ready);
+						}
+						// Add a new manipulator for the next anchor that we will place
+						if let Some(out_handle) = &last_manipulator_group.points[outwards_handle] {
+							responses.push_back(add_manipulator_group(&tool_data.path, tool_data.from_start, ManipulatorGroup::new_with_anchor(out_handle.position)));
+						}
+
+						// Returning `None` means the `unwrap_or` clause below returns the state `PlacingAnchor`
+						None
+					};
+					tool_data.should_mirror = true;
+					process().unwrap_or(PenToolFsmState::PlacingAnchor)
 				}
 				(PenToolFsmState::DraggingHandle, PenToolMessage::PointerMove { snap_angle, break_handle }) => {
-					if let Some(layer_path) = &tool_data.path {
-						let mouse = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
-						let mut pos = transform.inverse().transform_point2(mouse);
-						if let Some(((&id, manipulator_group), _previous)) = get_subpath(layer_path, document).and_then(last_2_manipulator_groups) {
-							if let Some(anchor) = manipulator_group.points[ManipulatorType::Anchor].as_ref() {
-								pos = compute_snapped_angle(input, snap_angle, pos, anchor.position);
-							}
+					let mut process = || {
+						// Get subpath
+						let layer_path = tool_data.path.as_ref()?;
+						let subpath = document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())?;
+
+						// Get the last manipulator group
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (&last_id, last_manipulator_group) = if tool_data.from_start { manipulator_groups.next()? } else { manipulator_groups.next_back()? };
+
+						// Get correct handle types
+						let inwards_handle = if tool_data.from_start { ManipulatorType::OutHandle } else { ManipulatorType::InHandle };
+						let outwards_handle = if tool_data.from_start { ManipulatorType::InHandle } else { ManipulatorType::OutHandle };
+
+						// Get manipulator points
+						let last_anchor = last_manipulator_group.points[ManipulatorType::Anchor].as_ref()?;
+
+						let mouse = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
+						let pos = transform.inverse().transform_point2(mouse);
+						let pos = compute_snapped_angle(input, snap_angle, pos, last_anchor.position);
+
+						// Update points on current segment (to show preview of new handle)
+						let msg = Operation::MoveManipulatorPoint {
+							layer_path: layer_path.clone(),
+							id: last_id,
+							manipulator_type: outwards_handle,
+							position: pos.into(),
+						};
+						responses.push_back(msg.into());
+
+						// Mirror handle of last segment
+						if !input.keyboard.get(break_handle as usize) && tool_data.should_mirror {
+							// Could also be written as `last_anchor.position * 2 - pos` but this way avoids overflow/underflow better
+							let pos = last_anchor.position - (pos - last_anchor.position);
 
-							// Update points on current segment (to show preview of new handle)
 							let msg = Operation::MoveManipulatorPoint {
 								layer_path: layer_path.clone(),
-								id,
-								manipulator_type: ManipulatorType::OutHandle,
+								id: last_id,
+								manipulator_type: inwards_handle,
 								position: pos.into(),
 							};
 							responses.push_back(msg.into());
-
-							// Mirror handle of last segment
-							if !input.keyboard.get(break_handle as usize) && get_subpath(layer_path, document).map(|shape| shape.manipulator_groups().len() > 1).unwrap_or_default() {
-								if let Some(anchor) = manipulator_group.points[ManipulatorType::Anchor].as_ref() {
-									pos = anchor.position - (pos - anchor.position);
-								}
-								let msg = Operation::MoveManipulatorPoint {
-									layer_path: layer_path.clone(),
-									id,
-									manipulator_type: ManipulatorType::InHandle,
-									position: pos.into(),
-								};
-								responses.push_back(msg.into());
-							}
 						}
-					}
 
-					self
+						Some(())
+					};
+					if process().is_none() {
+						PenToolFsmState::Ready
+					} else {
+						self
+					}
 				}
 				(PenToolFsmState::PlacingAnchor, PenToolMessage::PointerMove { snap_angle, .. }) => {
-					if let Some(layer_path) = &tool_data.path {
+					let mut process = || {
+						// Get subpath
+						let layer_path = tool_data.path.as_ref()?;
+						let subpath = document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())?;
+
+						// Get the last manipulator group and the one previous to that
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (&last_id, _last_manipulator_group) = if tool_data.from_start { manipulator_groups.next()? } else { manipulator_groups.next_back()? };
+						let previous = if tool_data.from_start { manipulator_groups.next() } else { manipulator_groups.next_back() };
+
+						// Get the first manipulator group
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (_first_id, first_manipulator_group) = if tool_data.from_start { manipulator_groups.next_back()? } else { manipulator_groups.next()? };
+
+						// Get manipulator points
+						let first_anchor = first_manipulator_group.points[ManipulatorType::Anchor].as_ref()?;
+
 						let mouse = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
 						let mut pos = transform.inverse().transform_point2(mouse);
 
-						if let Some(((&id, _), previous)) = get_subpath(layer_path, document).and_then(last_2_manipulator_groups) {
-							if let Some(relative) = previous.as_ref().and_then(|(_, manipulator_group)| manipulator_group.points[ManipulatorType::Anchor].as_ref()) {
-								pos = compute_snapped_angle(input, snap_angle, pos, relative.position);
-							}
-
-							for manipulator_type in [ManipulatorType::Anchor, ManipulatorType::InHandle, ManipulatorType::OutHandle] {
-								let msg = Operation::MoveManipulatorPoint {
-									layer_path: layer_path.clone(),
-									id,
-									manipulator_type,
-									position: pos.into(),
-								};
-								responses.push_back(msg.into());
-							}
+						// Snap to the first point (to show close path)
+						if mouse.distance_squared(transform.transform_point2(first_anchor.position)) < crate::consts::SNAP_POINT_TOLERANCE.powi(2) {
+							pos = first_anchor.position;
 						}
-					}
 
-					self
+						if let Some(relative) = previous.as_ref().and_then(|(_, manipulator_group)| manipulator_group.points[ManipulatorType::Anchor].as_ref()) {
+							pos = compute_snapped_angle(input, snap_angle, pos, relative.position);
+						}
+
+						for manipulator_type in [ManipulatorType::Anchor, ManipulatorType::InHandle, ManipulatorType::OutHandle] {
+							let msg = Operation::MoveManipulatorPoint {
+								layer_path: layer_path.clone(),
+								id: last_id,
+								manipulator_type,
+								position: pos.into(),
+							};
+							responses.push_back(msg.into());
+						}
+
+						Some(())
+					};
+					if process().is_none() {
+						PenToolFsmState::Ready
+					} else {
+						self
+					}
 				}
 				(PenToolFsmState::DraggingHandle | PenToolFsmState::PlacingAnchor, PenToolMessage::Abort | PenToolMessage::Confirm) => {
 					// Abort or commit the transaction to the undo history
-					if let Some(layer_path) = tool_data.path.as_ref() {
-						if let Some(subpath) = (get_subpath(layer_path, document)).filter(|subpath| subpath.manipulator_groups().len() > 1) {
-							if let Some(((&(mut id), mut manipulator_group), previous)) = last_2_manipulator_groups(subpath) {
-								// Remove the unplaced anchor if in anchor placing mode
-								if self == PenToolFsmState::PlacingAnchor {
-									let layer_path = layer_path.clone();
-									let op = Operation::RemoveManipulatorGroup { layer_path, id };
-									responses.push_back(op.into());
-									if let Some((&new_id, new_manipulator_group)) = previous {
-										id = new_id;
-										manipulator_group = new_manipulator_group;
-									}
-								}
+					let mut commit = || {
+						// Get subpath
+						let layer_path = tool_data.path.as_ref()?;
+						let subpath = document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())?;
 
-								// Remove the out handle if in dragging handle mode
-								let op = Operation::MoveManipulatorPoint {
-									layer_path: layer_path.clone(),
-									id,
-									manipulator_type: ManipulatorType::OutHandle,
-									position: manipulator_group.points[ManipulatorType::Anchor].as_ref().unwrap().position.into(),
-								};
-								responses.push_back(op.into());
-							}
+						// If placing anchor we should abort if there are less than three manipulators (as the last one gets deleted)
+						if self == PenToolFsmState::PlacingAnchor && subpath.manipulator_groups().len() < 3 {
+							return None;
 						}
 
-						responses.push_back(DocumentMessage::CommitTransaction.into());
-					} else {
+						// Get the last manipulator group and the one previous to that
+						let mut manipulator_groups = subpath.manipulator_groups().enumerate();
+						let (&(mut last_id), mut last_manipulator_group) = if tool_data.from_start { manipulator_groups.next()? } else { manipulator_groups.next_back()? };
+						let previous = if tool_data.from_start { manipulator_groups.next() } else { manipulator_groups.next_back() };
+
+						// Get correct handle types
+						let outwards_handle = if tool_data.from_start { ManipulatorType::InHandle } else { ManipulatorType::OutHandle };
+
+						// Clean up if there are two or more manipulators
+						if let Some((&previous_id, previous_manipulator_group)) = previous {
+							// Remove the unplaced anchor if in anchor placing mode
+							if self == PenToolFsmState::PlacingAnchor {
+								let layer_path = layer_path.clone();
+								let op = Operation::RemoveManipulatorGroup { layer_path, id: last_id };
+								responses.push_back(op.into());
+								last_id = previous_id;
+								last_manipulator_group = previous_manipulator_group;
+							}
+
+							// Remove the out handle
+							let op = Operation::MoveManipulatorPoint {
+								layer_path: layer_path.clone(),
+								id: last_id,
+								manipulator_type: outwards_handle,
+								position: last_manipulator_group.points[ManipulatorType::Anchor].as_ref()?.position.into(),
+							};
+							responses.push_back(op.into());
+
+							responses.push_back(DocumentMessage::CommitTransaction.into());
+
+							return Some(());
+						}
+
+						// Abort if only one manipulator group has been placed
+						None
+					};
+					if commit().is_none() {
 						responses.push_back(DocumentMessage::AbortTransaction.into());
 					}
 
@@ -446,33 +612,51 @@ fn compute_snapped_angle(input: &InputPreprocessorMessageHandler, key: Key, pos:
 }
 
 /// Pushes a [ManipulatorGroup] to the current layer via an [Operation].
-fn add_manipulator_group(layer_path: &Option<Vec<LayerId>>, manipulator_group: ManipulatorGroup) -> Message {
-	if let Some(layer_path) = layer_path {
-		Operation::PushManipulatorGroup {
+fn add_manipulator_group(layer_path: &Option<Vec<LayerId>>, from_start: bool, manipulator_group: ManipulatorGroup) -> Message {
+	match (layer_path, from_start) {
+		(Some(layer_path), true) => Operation::PushFrontManipulatorGroup {
 			layer_path: layer_path.clone(),
 			manipulator_group,
 		}
-		.into()
-	} else {
-		Message::NoOp
+		.into(),
+		(Some(layer_path), false) => Operation::PushManipulatorGroup {
+			layer_path: layer_path.clone(),
+			manipulator_group,
+		}
+		.into(),
+		(None, _) => Message::NoOp,
 	}
 }
 
-/// Gets the currently editing [Subpath].
-fn get_subpath<'a>(layer_path: &'a [LayerId], document: &'a DocumentMessageHandler) -> Option<&'a Subpath> {
-	document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())
-}
+/// Determines if a path should be extended. Returns the path and if it is extending from the start, if applicable.
+fn should_extend(document: &DocumentMessageHandler, pos: DVec2, tolerance: f64) -> Option<(&[LayerId], bool)> {
+	let mut best = None;
+	let mut best_distance_squared = tolerance * tolerance;
 
-type ManipulatorGroupRef<'a> = (&'a u64, &'a ManipulatorGroup);
+	for layer_path in document.selected_layers() {
+		(|| {
+			let viewspace = document.graphene_document.generate_transform_relative_to_viewport(layer_path).ok()?;
 
-/// Gets the last 2 [ManipulatorGroup]s on the currently editing layer along with its ID.
-fn last_2_manipulator_groups(subpath: &Subpath) -> Option<(ManipulatorGroupRef, Option<ManipulatorGroupRef>)> {
-	subpath.manipulator_groups().enumerate().last().map(|last| {
-		(
-			last,
-			(subpath.manipulator_groups().len() > 1)
-				.then(|| subpath.manipulator_groups().enumerate().nth(subpath.manipulator_groups().len() - 2))
-				.flatten(),
-		)
-	})
+			let subpath = document.graphene_document.layer(layer_path).ok().and_then(|layer| layer.as_subpath())?;
+			let (_first_id, first) = subpath.manipulator_groups().enumerate().next()?;
+			let (_last_id, last) = subpath.manipulator_groups().enumerate().next_back()?;
+
+			if !last.is_close() {
+				for (manipulator_group, from_start) in [(first, true), (last, false)] {
+					if let Some(point) = &manipulator_group.points[ManipulatorType::Anchor] {
+						let distance_squared = viewspace.transform_point2(point.position).distance_squared(pos);
+
+						if distance_squared < best_distance_squared {
+							best = Some((layer_path, from_start));
+							best_distance_squared = distance_squared;
+						}
+					}
+				}
+			}
+
+			None::<()>
+		})();
+	}
+
+	best
 }

graphene/src/document.rs

@@ -787,6 +787,13 @@ impl Document {
 				}
 				Some([update_thumbnails_upstream(&layer_path), vec![DocumentChanged, LayerChanged { path: layer_path }]].concat())
 			}
+			Operation::PushFrontManipulatorGroup { layer_path, manipulator_group } => {
+				if let Ok(Some(shape)) = self.layer_mut(&layer_path).map(|layer| layer.as_subpath_mut()) {
+					shape.manipulator_groups_mut().push_front(manipulator_group);
+					self.mark_as_dirty(&layer_path)?;
+				}
+				Some([update_thumbnails_upstream(&layer_path), vec![DocumentChanged, LayerChanged { path: layer_path }]].concat())
+			}
 			Operation::RemoveManipulatorGroup { layer_path, id } => {
 				if let Ok(Some(shape)) = self.layer_mut(&layer_path).map(|layer| layer.as_subpath_mut()) {
 					shape.manipulator_groups_mut().remove(id);
@@ -959,6 +966,21 @@ impl Document {
 				self.mark_as_dirty(&layer_path)?;
 				Some([vec![DocumentChanged, LayerChanged { path: layer_path.clone() }], update_thumbnails_upstream(&layer_path)].concat())
 			}
+			Operation::SetManipulatorHandleMirroring {
+				layer_path,
+				id,
+				mirror_distance,
+				mirror_angle,
+			} => {
+				if let Ok(Some(shape)) = self.layer_mut(&layer_path).map(|layer| layer.as_subpath_mut()) {
+					if let Some(manipulator_group) = shape.manipulator_groups_mut().by_id_mut(id) {
+						manipulator_group.editor_state.mirror_distance_between_handles = mirror_distance;
+						manipulator_group.editor_state.mirror_angle_between_handles = mirror_angle;
+						self.mark_as_dirty(&layer_path)?;
+					}
+				}
+				Some([update_thumbnails_upstream(&layer_path), vec![DocumentChanged, LayerChanged { path: layer_path }]].concat())
+			}
 			Operation::SetSelectedHandleMirroring {
 				layer_path,
 				toggle_distance,

graphene/src/layers/id_vec.rs

@@ -34,7 +34,7 @@ impl<T> IdBackedVec<T> {
 		Some(self.next_id)
 	}
 
-	// Push an element to the end of the vector
+	/// Push an element to the end of the vector
 	pub fn push_end(&mut self, element: T) -> Option<ElementId> {
 		self.next_id += 1;
 		self.elements.push(element);
@@ -113,7 +113,7 @@ impl<T> IdBackedVec<T> {
 	}
 
 	/// Enumerate the ids and elements in this container `(&ElementId, &T)`
-	pub fn enumerate(&self) -> impl Iterator<Item = (&ElementId, &T)> {
+	pub fn enumerate(&self) -> std::iter::Zip<core::slice::Iter<u64>, core::slice::Iter<T>> {
 		self.element_ids.iter().zip(self.elements.iter())
 	}
 

graphene/src/layers/vector/subpath.rs

@@ -384,18 +384,21 @@ impl Subpath {
 				} else if last_out_handle.is_some() || first_in_handle.is_some() {
 					result.push('Q');
 					write_positions(&mut result, [last_out_handle, first_in_handle, first_in_anchor]);
-				} else {
-					result.push('Z');
 				}
-			} else if command == 'M' {
-				// Update the last moveto position
+				result.push('Z');
+			}
+			// Update the last moveto position
+			else if command == 'M' {
 				(first_in_handle, first_in_anchor) = (in_handle, anchor);
 				result.push(command);
 				write_positions(&mut result, [None, None, anchor]);
-			} else {
+			}
+			// Write other path commands (line to/quadratic to/cubic to)
+			else {
 				result.push(command);
 				write_positions(&mut result, [last_out_handle, in_handle, anchor]);
 			}
+
 			start_new_contour = command == 'Z';
 			last_out_handle = out_handle;
 		}

graphene/src/operation.rs

@@ -172,6 +172,10 @@ pub enum Operation {
 		layer_path: Vec<LayerId>,
 		manipulator_group: ManipulatorGroup,
 	},
+	PushFrontManipulatorGroup {
+		layer_path: Vec<LayerId>,
+		manipulator_group: ManipulatorGroup,
+	},
 	RemoveManipulatorGroup {
 		layer_path: Vec<LayerId>,
 		id: u64,
@@ -226,6 +230,12 @@ pub enum Operation {
 		path: Vec<LayerId>,
 		stroke: Stroke,
 	},
+	SetManipulatorHandleMirroring {
+		layer_path: Vec<LayerId>,
+		id: u64,
+		mirror_distance: bool,
+		mirror_angle: bool,
+	},
 	SetSelectedHandleMirroring {
 		layer_path: Vec<LayerId>,
 		toggle_distance: bool,