Add point insertion to the Path tool (#754)

* Messaging cleanup

* Add bezier iter

* Add splitting

* Use bezier_rs bounding box

* Cleanup

* Fix comments

* Fix typo

* Code review tweaks

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

Cargo.lock

@@ -207,6 +207,7 @@ dependencies = [
 name = "graphite-editor"
 version = "0.0.0"
 dependencies = [
+ "bezier-rs",
  "bitflags",
  "derivative",
  "env_logger",
@@ -230,6 +231,7 @@ name = "graphite-graphene"
 version = "0.0.0"
 dependencies = [
  "base64",
+ "bezier-rs",
  "glam",
  "kurbo",
  "log",

editor/Cargo.toml

@@ -17,6 +17,7 @@ thiserror = "1.0.24"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0" }
 graphite-proc-macros = { path = "../proc-macros" }
+bezier-rs = { path = "../libraries/bezier-rs" }
 glam = { version="0.17", features = ["serde"] }
 rand_chacha = "0.3.1"
 spin = "0.9.2"

editor/src/messages/input_mapper/default_mapping.rs

@@ -139,6 +139,7 @@ pub fn default_mapping() -> Mapping {
 		entry!(KeyDown(Delete); action_dispatch=PathToolMessage::Delete),
 		entry!(KeyDown(Backspace); action_dispatch=PathToolMessage::Delete),
 		entry!(KeyUp(Lmb); action_dispatch=PathToolMessage::DragStop),
+		entry!(DoubleClick; action_dispatch=PathToolMessage::InsertPoint),
 		//
 		// PenToolMessage
 		entry!(PointerMove; refresh_keys=[Shift, Control], action_dispatch=PenToolMessage::PointerMove { snap_angle: Control, break_handle: Shift }),

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

@@ -3,7 +3,7 @@ use crate::messages::prelude::*;
 use graphene::layers::vector::consts::ManipulatorType;
 use graphene::layers::vector::manipulator_group::ManipulatorGroup;
 use graphene::layers::vector::manipulator_point::ManipulatorPoint;
-use graphene::layers::vector::subpath::Subpath;
+use graphene::layers::vector::subpath::{BezierId, Subpath};
 use graphene::{LayerId, Operation};
 
 use glam::DVec2;
@@ -259,6 +259,69 @@ impl ShapeEditor {
 		result
 	}
 
+	/// Find the `t` value along the path segment we have clicked upon, together with that segment ID.
+	///
+	/// Returns a tuple of [`BezierId`] and `t` as an f64.
+	fn closest_segment(&self, document: &Document, layer_path: &[LayerId], position: glam::DVec2, tolerance: f64) -> Option<(BezierId, f64)> {
+		let transform = document.generate_transform_relative_to_viewport(layer_path).ok()?;
+		let layer_pos = transform.inverse().transform_point2(position);
+		let projection_options = bezier_rs::ProjectionOptions { lut_size: 5, ..Default::default() };
+
+		let mut result: Option<(BezierId, f64)> = None;
+		let mut closest_distance_squared: f64 = tolerance * tolerance;
+
+		for bezier_id in document.layer(layer_path).ok()?.as_subpath()?.bezier_iter() {
+			let bezier = bezier_id.internal;
+			let t = bezier.project(layer_pos, projection_options);
+			let layerspace = bezier.evaluate(t);
+
+			let screenspace = transform.transform_point2(layerspace);
+			let distance_squared = screenspace.distance_squared(position);
+
+			if distance_squared < closest_distance_squared {
+				closest_distance_squared = distance_squared;
+				result = Some((bezier_id, t));
+			}
+		}
+
+		result
+	}
+
+	/// Handles the splitting of a curve to insert new points (which can be activated by double clicking on a curve with the Path tool).
+	pub fn split(&self, document: &Document, position: glam::DVec2, tolerance: f64, responses: &mut VecDeque<Message>) {
+		for layer_path in &self.selected_layers {
+			if let Some((bezier_id, t)) = self.closest_segment(document, layer_path, position, tolerance) {
+				let [first, second] = bezier_id.internal.split(t);
+
+				// Adjust the first manipulator group's out handle
+				let out_handle = Operation::SetManipulatorPoints {
+					layer_path: layer_path.clone(),
+					id: bezier_id.start,
+					manipulator_type: ManipulatorType::OutHandle,
+					position: first.handle_start().map(|p| p.into()),
+				};
+
+				// Insert a new manipulator group between the existing ones
+				let insert = Operation::InsertManipulatorGroup {
+					layer_path: layer_path.clone(),
+					manipulator_group: ManipulatorGroup::new_with_handles(first.end(), first.handle_end(), second.handle_start()),
+					after_id: bezier_id.end,
+				};
+
+				// Adjust the last manipulator group's in handle
+				let in_handle = Operation::SetManipulatorPoints {
+					layer_path: layer_path.clone(),
+					id: bezier_id.end,
+					manipulator_type: ManipulatorType::InHandle,
+					position: second.handle_end().map(|p| p.into()),
+				};
+
+				responses.extend([out_handle.into(), insert.into(), in_handle.into()]);
+				return;
+			}
+		}
+	}
+
 	fn shape<'a>(&'a self, document: &'a Document, layer_id: &[u64]) -> Option<&'a Subpath> {
 		document.layer(layer_id).ok()?.as_subpath()
 	}

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

@@ -1,4 +1,4 @@
-use crate::consts::SELECTION_THRESHOLD;
+use crate::consts::{SELECTION_THRESHOLD, SELECTION_TOLERANCE};
 use crate::messages::frontend::utility_types::MouseCursorIcon;
 use crate::messages::input_mapper::utility_types::input_keyboard::{Key, KeysGroup, MouseMotion};
 use crate::messages::layout::utility_types::layout_widget::PropertyHolder;
@@ -39,6 +39,7 @@ pub enum PathToolMessage {
 		add_to_selection: Key,
 	},
 	DragStop,
+	InsertPoint,
 	PointerMove {
 		alt_mirror_angle: Key,
 		shift_mirror_distance: Key,
@@ -86,10 +87,12 @@ impl<'a> MessageHandler<ToolMessage, ToolActionHandlerData<'a>> for PathTool {
 
 		match self.fsm_state {
 			Ready => actions!(PathToolMessageDiscriminant;
+				InsertPoint,
 				DragStart,
 				Delete,
 			),
 			Dragging => actions!(PathToolMessageDiscriminant;
+				InsertPoint,
 				DragStop,
 				PointerMove,
 				Delete,
@@ -144,11 +147,8 @@ impl Fsm for PathToolFsmState {
 		responses: &mut VecDeque<Message>,
 	) -> Self {
 		if let ToolMessage::Path(event) = event {
-			use PathToolFsmState::*;
-			use PathToolMessage::*;
-
 			match (self, event) {
-				(_, SelectionChanged) => {
+				(_, PathToolMessage::SelectionChanged) => {
 					// Set the previously selected layers to invisible
 					for layer_path in document.all_layers() {
 						tool_data.overlay_renderer.layer_overlay_visibility(&document.graphene_document, layer_path.to_vec(), false, responses);
@@ -165,7 +165,7 @@ impl Fsm for PathToolFsmState {
 					// This can happen in any state (which is why we return self)
 					self
 				}
-				(_, DocumentIsDirty) => {
+				(_, PathToolMessage::DocumentIsDirty) => {
 					// When the document has moved / needs to be redraw, re-render the overlays
 					// TODO the overlay system should probably receive this message instead of the tool
 					for layer_path in document.selected_visible_layers() {
@@ -175,7 +175,7 @@ impl Fsm for PathToolFsmState {
 					self
 				}
 				// Mouse down
-				(_, DragStart { add_to_selection }) => {
+				(_, PathToolMessage::DragStart { add_to_selection }) => {
 					let toggle_add_to_selection = input.keyboard.get(add_to_selection as usize);
 
 					// Select the first point within the threshold (in pixels)
@@ -204,7 +204,7 @@ impl Fsm for PathToolFsmState {
 						tool_data.snap_manager.add_all_document_handles(document, &include_handles, &[], &new_selected);
 
 						tool_data.drag_start_pos = input.mouse.position;
-						Dragging
+						PathToolFsmState::Dragging
 					}
 					// We didn't find a point nearby, so consider selecting the nearest shape instead
 					else {
@@ -230,13 +230,13 @@ impl Fsm for PathToolFsmState {
 								responses.push_back(DocumentMessage::DeselectAllLayers.into());
 							}
 						}
-						Ready
+						PathToolFsmState::Ready
 					}
 				}
 				// Dragging
 				(
-					Dragging,
+					PathToolFsmState::Dragging,
-					PointerMove {
+					PathToolMessage::PointerMove {
 						alt_mirror_angle,
 						shift_mirror_distance,
 					},
@@ -262,34 +262,39 @@ impl Fsm for PathToolFsmState {
 					let snapped_position = tool_data.snap_manager.snap_position(responses, document, input.mouse.position);
 					tool_data.shape_editor.move_selected_points(snapped_position - tool_data.drag_start_pos, snapped_position, responses);
 					tool_data.drag_start_pos = snapped_position;
-					Dragging
+					PathToolFsmState::Dragging
 				}
 				// Mouse up
-				(_, DragStop) => {
+				(_, PathToolMessage::DragStop) => {
 					tool_data.snap_manager.cleanup(responses);
-					Ready
+					PathToolFsmState::Ready
 				}
 				// Delete key
-				(_, Delete) => {
+				(_, PathToolMessage::Delete) => {
 					// Delete the selected points and clean up overlays
 					responses.push_back(DocumentMessage::StartTransaction.into());
 					tool_data.shape_editor.delete_selected_points(responses);
-					responses.push_back(SelectionChanged.into());
+					responses.push_back(PathToolMessage::SelectionChanged.into());
 					for layer_path in document.all_layers() {
 						tool_data.overlay_renderer.clear_subpath_overlays(&document.graphene_document, layer_path.to_vec(), responses);
 					}
-					Ready
+					PathToolFsmState::Ready
 				}
-				(_, Abort) => {
+				(_, PathToolMessage::InsertPoint) => {
+					tool_data.shape_editor.split(&document.graphene_document, input.mouse.position, SELECTION_TOLERANCE, responses);
+
+					self
+				}
+				(_, PathToolMessage::Abort) => {
 					// TODO Tell overlay manager to remove the overlays
 					for layer_path in document.all_layers() {
 						tool_data.overlay_renderer.clear_subpath_overlays(&document.graphene_document, layer_path.to_vec(), responses);
 					}
-					Ready
+					PathToolFsmState::Ready
 				}
 				(
 					_,
-					PointerMove {
+					PathToolMessage::PointerMove {
 						alt_mirror_angle: _,
 						shift_mirror_distance: _,
 					},

graphene/Cargo.toml

@@ -13,6 +13,7 @@ license = "Apache-2.0"
 [dependencies]
 log = "0.4"
 
+bezier-rs = { path = "../libraries/bezier-rs" }
 kurbo = { git = "https://github.com/linebender/kurbo.git", features = [
 	"serde",
 ] }

graphene/src/document.rs

@@ -815,6 +815,24 @@ impl Document {
 				}
 				Some([update_thumbnails_upstream(&layer_path), vec![DocumentChanged, LayerChanged { path: layer_path }]].concat())
 			}
+			Operation::SetManipulatorPoints {
+				layer_path,
+				id,
+				manipulator_type,
+				position,
+			} => {
+				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) {
+						if let Some(position) = position {
+							manipulator_group.set_point_position(manipulator_type as usize, position.into());
+						} else {
+							manipulator_group.points[manipulator_type] = None;
+						}
+						self.mark_as_dirty(&layer_path)?;
+					}
+				}
+				Some([update_thumbnails_upstream(&layer_path), vec![DocumentChanged, LayerChanged { path: layer_path }]].concat())
+			}
 			Operation::RemoveManipulatorPoint {
 				layer_path,
 				id,

graphene/src/layers/shape_layer.rs

@@ -29,8 +29,7 @@ impl LayerData for ShapeLayer {
 	fn render(&mut self, svg: &mut String, svg_defs: &mut String, transforms: &mut Vec<DAffine2>, render_data: RenderData) {
 		let mut subpath = self.shape.clone();
 
-		let kurbo::Rect { x0, y0, x1, y1 } = subpath.bounding_box();
+		let layer_bounds = subpath.bounding_box().unwrap_or_default();
-		let layer_bounds = [(x0, y0).into(), (x1, y1).into()];
 
 		let transform = self.transform(transforms, render_data.view_mode);
 		let inverse = transform.inverse();
@@ -40,8 +39,7 @@ impl LayerData for ShapeLayer {
 		}
 		subpath.apply_affine(transform);
 
-		let kurbo::Rect { x0, y0, x1, y1 } = subpath.bounding_box();
+		let transformed_bounds = subpath.bounding_box().unwrap_or_default();
-		let transformed_bounds = [(x0, y0).into(), (x1, y1).into()];
 
 		let _ = writeln!(svg, r#"<g transform="matrix("#);
 		inverse.to_cols_array().iter().enumerate().for_each(|(i, entry)| {
@@ -64,8 +62,7 @@ impl LayerData for ShapeLayer {
 		}
 		subpath.apply_affine(transform);
 
-		let kurbo::Rect { x0, y0, x1, y1 } = subpath.bounding_box();
+		subpath.bounding_box()
-		Some([(x0, y0).into(), (x1, y1).into()])
 	}
 
 	fn intersects_quad(&self, quad: Quad, path: &mut Vec<LayerId>, intersections: &mut Vec<Vec<LayerId>>, _font_cache: &FontCache) {

graphene/src/layers/text_layer.rs

@@ -70,13 +70,11 @@ impl LayerData for TextLayer {
 
 			let mut path = self.to_subpath(buzz_face);
 
-			let kurbo::Rect { x0, y0, x1, y1 } = path.bounding_box();
+			let bounds = path.bounding_box().unwrap_or_default();
-			let bounds = [(x0, y0).into(), (x1, y1).into()];
 
 			path.apply_affine(transform);
 
-			let kurbo::Rect { x0, y0, x1, y1 } = path.bounding_box();
+			let transformed_bounds = path.bounding_box().unwrap_or_default();
-			let transformed_bounds = [(x0, y0).into(), (x1, y1).into()];
 
 			let _ = write!(
 				svg,

graphene/src/layers/vector/subpath.rs

@@ -5,7 +5,7 @@ use crate::layers::id_vec::IdBackedVec;
 use crate::layers::layer_info::{Layer, LayerDataType};
 
 use glam::{DAffine2, DVec2};
-use kurbo::{BezPath, PathEl, Rect, Shape};
+use kurbo::{BezPath, PathEl, Shape};
 use serde::{Deserialize, Serialize};
 
 /// [Subpath] represents a single vector path, containing many [ManipulatorGroups].
@@ -334,15 +334,14 @@ impl Subpath {
 		&mut self.0
 	}
 
-	// ** INTERFACE WITH KURBO **
+	/// Return the bounding box of the shape.
-
+	pub fn bounding_box(&self) -> Option<[DVec2; 2]> {
-	// TODO Implement our own a local bounding box calculation
+		self.bezier_iter()
-	/// Return the bounding box of the shape
+			.map(|bezier| bezier.internal.bounding_box())
-	pub fn bounding_box(&self) -> Rect {
+			.reduce(|[a_min, a_max], [b_min, b_max]| [a_min.min(b_min), a_max.max(b_max)])
-		<&Self as Into<BezPath>>::into(self).bounding_box()
 	}
 
-	/// Use kurbo to convert this shape into an SVG path
+	/// Generate an SVG `path` elements's `d` attribute: `<path d="...">`.
 	pub fn to_svg(&mut self) -> String {
 		fn write_positions(result: &mut String, values: [Option<DVec2>; 3]) {
 			use std::fmt::Write;
@@ -404,6 +403,20 @@ impl Subpath {
 		}
 		result
 	}
+
+	/// Convert to an iter over [`bezier_rs::Bezier`] segments.
+	pub fn bezier_iter(&self) -> PathIter {
+		PathIter {
+			path: self.manipulator_groups().enumerate(),
+			last_anchor: None,
+			last_out_handle: None,
+			last_id: None,
+			first_in_handle: None,
+			first_anchor: None,
+			first_id: None,
+			start_new_contour: true,
+		}
+	}
 }
 
 // ** CONVERSIONS **
@@ -434,6 +447,101 @@ impl<'a> TryFrom<&'a Layer> for &'a Subpath {
 	}
 }
 
+/// A wrapper around [`bezier_rs::Bezier`] containing also the IDs for the [`ManipulatorGroup`]s where the points are from.
+pub struct BezierId {
+	/// The internal [`bezier_rs::Bezier`].
+	pub internal: bezier_rs::Bezier,
+	/// The ID of the [ManipulatorGroup] of the start point and, if cubic, the start handle.
+	pub start: u64,
+	/// The ID of the [ManipulatorGroup] of the end point and, if cubic, the end handle.
+	pub end: u64,
+	/// The ID of the [ManipulatorGroup] of the handle on a quadratic (if applicable).
+	pub mid: Option<u64>,
+}
+
+impl BezierId {
+	/// Construct a `BezierId` by encapsulating a [`bezier_rs::Bezier`] and providing the IDs of the [`ManipulatorGroup`]s the constituent points belong to.
+	fn new(internal: bezier_rs::Bezier, start: u64, end: u64, mid: Option<u64>) -> Self {
+		Self { internal, start, end, mid }
+	}
+}
+
+/// An iterator over [`bezier_rs::Bezier`] segments constructable via [`Subpath::bezier_iter`].
+pub struct PathIter<'a> {
+	path: std::iter::Zip<core::slice::Iter<'a, u64>, core::slice::Iter<'a, ManipulatorGroup>>,
+
+	last_anchor: Option<DVec2>,
+	last_out_handle: Option<DVec2>,
+	last_id: Option<u64>,
+
+	first_in_handle: Option<DVec2>,
+	first_anchor: Option<DVec2>,
+	first_id: Option<u64>,
+
+	start_new_contour: bool,
+}
+
+impl<'a> Iterator for PathIter<'a> {
+	type Item = BezierId;
+
+	fn next(&mut self) -> Option<Self::Item> {
+		use bezier_rs::Bezier;
+
+		let mut result = None;
+
+		while result.is_none() {
+			let (&id, manipulator_group) = self.path.next()?;
+
+			let in_handle = manipulator_group.points[ManipulatorType::InHandle].as_ref().map(|point| point.position);
+			let anchor = manipulator_group.points[ManipulatorType::Anchor].as_ref().map(|point| point.position);
+			let out_handle = manipulator_group.points[ManipulatorType::OutHandle].as_ref().map(|point| point.position);
+
+			let mut start_new_contour = false;
+
+			// Move to
+			if anchor.is_some() && self.start_new_contour {
+				// Update the last moveto position
+				(self.first_in_handle, self.first_anchor) = (in_handle, anchor);
+				self.first_id = Some(id);
+			}
+			// Cubic to
+			else if let (Some(p1), Some(p2), Some(p3), Some(p4), Some(last_id)) = (self.last_anchor, self.last_out_handle, in_handle, anchor, self.last_id) {
+				result = Some(BezierId::new(Bezier::from_cubic_dvec2(p1, p2, p3, p4), last_id, id, None));
+			}
+			// Quadratic to
+			else if let (Some(p1), Some(p2), Some(p3), Some(last_id)) = (self.last_anchor, self.last_out_handle.or(in_handle), anchor, self.last_id) {
+				let mid = if self.last_out_handle.is_some() { last_id } else { id };
+				result = Some(BezierId::new(Bezier::from_quadratic_dvec2(p1, p2, p3), last_id, id, Some(mid)));
+			}
+			// Line to
+			else if let (Some(p1), Some(p2), Some(last_id)) = (self.last_anchor, anchor, self.last_id) {
+				result = Some(BezierId::new(Bezier::from_linear_dvec2(p1, p2), last_id, id, None));
+			}
+			// Close path
+			else if in_handle.is_none() && anchor.is_none() {
+				start_new_contour = true;
+				if let (Some(last_id), Some(first_id)) = (self.last_id, self.first_id) {
+					// Complete the last curve
+					if let (Some(p1), Some(p2), Some(p3), Some(p4)) = (self.last_anchor, self.last_out_handle, self.first_in_handle, self.first_anchor) {
+						result = Some(BezierId::new(Bezier::from_cubic_dvec2(p1, p2, p3, p4), last_id, first_id, None));
+					} else if let (Some(p1), Some(p2), Some(p3)) = (self.last_anchor, self.last_out_handle.or(self.first_in_handle), self.first_anchor) {
+						let mid = if self.last_out_handle.is_some() { last_id } else { first_id };
+						result = Some(BezierId::new(Bezier::from_quadratic_dvec2(p1, p2, p3), last_id, first_id, Some(mid)));
+					} else if let (Some(p1), Some(p2)) = (self.last_anchor, self.first_anchor) {
+						result = Some(BezierId::new(Bezier::from_linear_dvec2(p1, p2), last_id, first_id, None));
+					}
+				}
+			}
+
+			self.start_new_contour = start_new_contour;
+			self.last_out_handle = out_handle;
+			self.last_anchor = anchor;
+			self.last_id = Some(id);
+		}
+		result
+	}
+}
+
 impl From<&Subpath> for BezPath {
 	/// Create a [BezPath] from a [Subpath].
 	fn from(subpath: &Subpath) -> Self {

graphene/src/operation.rs

@@ -130,6 +130,12 @@ pub enum Operation {
 		manipulator_type: ManipulatorType,
 		position: (f64, f64),
 	},
+	SetManipulatorPoints {
+		layer_path: Vec<LayerId>,
+		id: u64,
+		manipulator_type: ManipulatorType,
+		position: Option<(f64, f64)>,
+	},
 	RenameLayer {
 		layer_path: Vec<LayerId>,
 		new_name: String,