Pen tool fixes (GraphiteEditor#563)

Resolves 3 known bugs with the pen tool.

* Fixed crash pointed out by @caleb-ad

* Fixed issue with final path segment losing handle data

* Replace curves with lines when under a drag threshold, improves usability.

* Readability improvements, improved comments

Author: otdavies

editor/src/consts.rs

@@ -38,6 +38,9 @@ pub const BOUNDS_ROTATE_THRESHOLD: f64 = 20.;
 pub const VECTOR_MANIPULATOR_ANCHOR_MARKER_SIZE: f64 = 5.;
 pub const SELECTION_THRESHOLD: f64 = 10.;
 
+// Pen tool
+pub const CREATE_CURVE_THRESHOLD: f64 = 5.;
+
 // Line tool
 pub const LINE_ROTATE_SNAP_ANGLE: f64 = 15.;
 

editor/src/viewport_tools/tools/pen_tool.rs

@@ -1,3 +1,4 @@
+use crate::consts::CREATE_CURVE_THRESHOLD;
 use crate::document::DocumentMessageHandler;
 use crate::frontend::utility_types::MouseCursorIcon;
 use crate::input::keyboard::{Key, MouseMotion};
@@ -7,6 +8,7 @@ use crate::message_prelude::*;
 use crate::misc::{HintData, HintGroup, HintInfo, KeysGroup};
 use crate::viewport_tools::snapping::SnapHandler;
 use crate::viewport_tools::tool::{DocumentToolData, Fsm, ToolActionHandlerData};
+use crate::viewport_tools::vector_editor::constants::ControlPointType;
 use crate::viewport_tools::vector_editor::shape_editor::ShapeEditor;
 use crate::viewport_tools::vector_editor::vector_shape::VectorShape;
 
@@ -133,6 +135,7 @@ struct PenToolData {
 	bez_path: Vec<PathEl>,
 	snap_handler: SnapHandler,
 	shape_editor: ShapeEditor,
+	drag_start_position: DVec2,
 }
 
 impl Fsm for PenToolFsmState {
@@ -173,7 +176,7 @@ impl Fsm for PenToolFsmState {
 					let start_position = transform.inverse().transform_point2(snapped_position);
 					data.weight = tool_options.line_weight;
 
-					// Create the initial shape with a bez_path (only contains a moveto initially)
+					// Create the initial shape with a `bez_path` (only contains a moveto initially)
 					if let Some(layer_path) = &data.path {
 						data.bez_path = start_bez_path(start_position);
 						responses.push_back(
@@ -193,36 +196,49 @@ impl Fsm for PenToolFsmState {
 					Drawing
 				}
 				(Drawing, DragStart) => {
+					data.drag_start_position = input.mouse.position;
 					add_to_curve(data, input, transform, document, responses);
 					Drawing
 				}
 				(Drawing, DragStop) => {
 					// Deselect everything (this means we are no longer dragging the handle)
 					data.shape_editor.deselect_all(responses);
 
+					// If the drag does not exceed the threshold, then replace the curve with a line
+					if data.drag_start_position.distance(input.mouse.position) < CREATE_CURVE_THRESHOLD {
+						// Modify the second to last element (as we have an unplaced element tracing to the cursor as the last element)
+						let replace_index = data.bez_path.len() - 2;
+						let line_from_curve = convert_curve_to_line(data.bez_path[replace_index]);
+						replace_path_element(data, transform, replace_index, line_from_curve, responses);
+					}
+
 					// Reselect the last point
 					if let Some(last_anchor) = data.shape_editor.select_last_anchor() {
-						last_anchor.select_point(0, true, responses);
+						last_anchor.select_point(ControlPointType::Anchor as usize, true, responses);
 					}
 
+					// Move the newly selected points to the cursor
+					let snapped_position = data.snap_handler.snap_position(responses, input.viewport_bounds.size(), document, input.mouse.position);
+					data.shape_editor.move_selected_points(snapped_position, false, responses);
+
 					Drawing
 				}
 				(Drawing, PointerMove) => {
+					// Move selected points
 					let snapped_position = data.snap_handler.snap_position(responses, input.viewport_bounds.size(), document, input.mouse.position);
-					//data.shape_editor.update_shapes(document, responses);
 					data.shape_editor.move_selected_points(snapped_position, false, responses);
 
 					Drawing
 				}
 				(Drawing, Confirm) | (Drawing, Abort) => {
-					// Add a curve to the path
-					if let Some(layer_path) = &data.path {
-						remove_curve_from_end(&mut data.bez_path);
-						responses.push_back(apply_bez_path(layer_path.clone(), data.bez_path.clone(), transform));
-					}
-
 					// Cleanup, we are either canceling or finished drawing
 					if data.bez_path.len() >= 2 {
+						// Remove the last segment
+						remove_from_curve(data);
+						if let Some(layer_path) = &data.path {
+							responses.push_back(apply_bez_path(layer_path.clone(), data.bez_path.clone(), transform));
+						}
+
 						responses.push_back(DocumentMessage::DeselectAllLayers.into());
 						responses.push_back(DocumentMessage::CommitTransaction.into());
 					} else {
@@ -281,65 +297,86 @@ impl Fsm for PenToolFsmState {
 	}
 }
 
-// Add to the curve and select the second anchor of the last point and the newly added anchor point
+/// Add to the curve and select the second anchor of the last point and the newly added anchor point
 fn add_to_curve(data: &mut PenToolData, input: &InputPreprocessorMessageHandler, transform: DAffine2, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
-	// We need to make sure we have the most up-to-date bez_path
-	// Would like to remove this hack eventually
-	if !data.shape_editor.shapes_to_modify.is_empty() {
-		// Hacky way of saving the curve changes
-		data.bez_path = data.shape_editor.shapes_to_modify[0].bez_path.elements().to_vec();
-	}
+	// Refresh data's representation of the path
+	update_path_representation(data);
 
 	// Setup our position params
 	let snapped_position = data.snap_handler.snap_position(responses, input.viewport_bounds.size(), document, input.mouse.position);
 	let position = transform.inverse().transform_point2(snapped_position);
 
 	// Add a curve to the path
 	if let Some(layer_path) = &data.path {
-		add_curve_to_end(position, &mut data.bez_path);
+		// Push curve onto path
+		let point = Point { x: position.x, y: position.y };
+		data.bez_path.push(PathEl::CurveTo(point, point, point));
+
 		responses.push_back(apply_bez_path(layer_path.clone(), data.bez_path.clone(), transform));
 
 		// Clear previous overlays
 		data.shape_editor.remove_overlays(responses);
 
-		// Create a new shape from the updated bez_path
+		// Create a new `shape` from the updated `bez_path`
 		let bez_path = data.bez_path.clone().into_iter().collect();
 		data.curve_shape = VectorShape::new(layer_path.to_vec(), transform, &bez_path, false, responses);
 		data.shape_editor.set_shapes_to_modify(vec![data.curve_shape.clone()]);
 
-		// Select the second to last segment's handle
+		// Select the second to last `PathEl`'s handle
 		data.shape_editor.set_shape_selected(0);
 		let handle_element = data.shape_editor.select_nth_anchor(0, -2);
-		handle_element.select_point(2, true, responses);
+		handle_element.select_point(ControlPointType::Handle2 as usize, true, responses);
 
-		// Select the last segment's anchor point
+		// Select the last `PathEl`'s anchor point
 		if let Some(last_anchor) = data.shape_editor.select_last_anchor() {
-			last_anchor.select_point(0, true, responses);
+			last_anchor.select_point(ControlPointType::Anchor as usize, true, responses);
 		}
 		data.shape_editor.set_selected_mirror_options(true, true);
 	}
 }
 
-// Create the initial moveto for the bez_path
+/// Replace a `PathEl` with another inside of `bez_path` by index
+fn replace_path_element(data: &mut PenToolData, transform: DAffine2, replace_index: usize, replacement: PathEl, responses: &mut VecDeque<Message>) {
+	data.bez_path[replace_index] = replacement;
+	if let Some(layer_path) = &data.path {
+		responses.push_back(apply_bez_path(layer_path.clone(), data.bez_path.clone(), transform));
+	}
+}
+
+/// Remove a curve from the end of the `bez_path`
+fn remove_from_curve(data: &mut PenToolData) {
+	// Refresh data's representation of the path
+	update_path_representation(data);
+	data.bez_path.pop();
+}
+
+/// Create the initial moveto for the `bez_path`
 fn start_bez_path(start_position: DVec2) -> Vec<PathEl> {
 	vec![PathEl::MoveTo(Point {
 		x: start_position.x,
 		y: start_position.y,
 	})]
 }
 
-// Add a curve to the bez_path
-fn add_curve_to_end(point: DVec2, bez_path: &mut Vec<PathEl>) {
-	let point = Point { x: point.x, y: point.y };
-	bez_path.push(PathEl::CurveTo(point, point, point));
+/// Convert curve `PathEl` into a line `PathEl`
+fn convert_curve_to_line(curve: PathEl) -> PathEl {
+	match curve {
+		PathEl::CurveTo(_, _, p) => PathEl::LineTo(p),
+		_ => PathEl::MoveTo(Point::ZERO),
+	}
 }
 
-// Add a curve to the bez_path
-fn remove_curve_from_end(bez_path: &mut Vec<PathEl>) {
-	bez_path.pop();
+/// Update data's version of `bez_path` to match `ShapeEditor`'s version
+fn update_path_representation(data: &mut PenToolData) {
+	// TODO Update ShapeEditor to provide similar functionality
+	// We need to make sure we have the most up-to-date bez_path
+	if !data.shape_editor.shapes_to_modify.is_empty() {
+		// Hacky way of saving the curve changes
+		data.bez_path = data.shape_editor.shapes_to_modify[0].bez_path.elements().to_vec();
+	}
 }
 
-// Apply the bez_path to the shape in the viewport
+/// Apply the `bez_path` to the `shape` in the viewport
 fn apply_bez_path(layer_path: Vec<LayerId>, bez_path: Vec<PathEl>, transform: DAffine2) -> Message {
 	Operation::SetShapePathInViewport {
 		path: layer_path,