Improve Path tool layer selection behavior using double-click instead of single-click (#2794)

* Improve path tool layer selection behaviour

* Fix layer selection behaviour

* Fix layer double click selection behaviour

* Code review

---------

Co-authored-by: Keavon Chambers <[email protected]>

Author: 4adex

editor/src/messages/input_mapper/input_mappings.rs

@@ -225,7 +225,7 @@ pub fn input_mappings() -> Mapping {
 		entry!(KeyDown(Backspace); action_dispatch=PathToolMessage::Delete),
 		entry!(KeyUp(MouseLeft); action_dispatch=PathToolMessage::DragStop { extend_selection: Shift, shrink_selection: Alt }),
 		entry!(KeyDown(Enter); action_dispatch=PathToolMessage::Enter { extend_selection: Shift, shrink_selection: Alt }),
-		entry!(DoubleClick(MouseButton::Left); action_dispatch=PathToolMessage::FlipSmoothSharp),
+		entry!(DoubleClick(MouseButton::Left); action_dispatch=PathToolMessage::DoubleClick { extend_selection: Shift, shrink_selection: Alt }),
 		entry!(KeyDown(ArrowRight); action_dispatch=PathToolMessage::NudgeSelectedPoints { delta_x: NUDGE_AMOUNT, delta_y: 0. }),
 		entry!(KeyDown(ArrowRight); modifiers=[Shift], action_dispatch=PathToolMessage::NudgeSelectedPoints { delta_x: BIG_NUDGE_AMOUNT, delta_y: 0. }),
 		entry!(KeyDown(ArrowRight); modifiers=[ArrowUp], action_dispatch=PathToolMessage::NudgeSelectedPoints { delta_x: NUDGE_AMOUNT, delta_y: -NUDGE_AMOUNT }),

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

@@ -96,6 +96,14 @@ impl SelectedLayerState {
 		self.selected_segments.remove(&segment);
 	}
 
+	pub fn deselect_all_points_in_layer(&mut self) {
+		self.selected_points.clear();
+	}
+
+	pub fn deselect_all_segments_in_layer(&mut self) {
+		self.selected_segments.clear();
+	}
+
 	pub fn clear_points(&mut self) {
 		self.selected_points.clear();
 	}
@@ -388,6 +396,10 @@ impl ClosestSegment {
 
 // TODO Consider keeping a list of selected manipulators to minimize traversals of the layers
 impl ShapeState {
+	pub fn is_selected_layer(&self, layer: LayerNodeIdentifier) -> bool {
+		self.selected_shape_state.contains_key(&layer)
+	}
+
 	pub fn is_point_ignored(&self, point: &ManipulatorPointId) -> bool {
 		(point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors)
 	}
@@ -637,7 +649,7 @@ impl ShapeState {
 	}
 
 	/// Selects all anchors connected to the selected subpath, and deselects all handles, for the given layer.
-	pub fn select_connected_anchors(&mut self, document: &DocumentMessageHandler, layer: LayerNodeIdentifier, mouse: DVec2) {
+	pub fn select_connected(&mut self, document: &DocumentMessageHandler, layer: LayerNodeIdentifier, mouse: DVec2, points: bool, segments: bool) {
 		let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else {
 			return;
 		};
@@ -655,18 +667,39 @@ impl ShapeState {
 			}
 		}
 		state.clear_points();
+
 		if selected_stack.is_empty() {
-			// Fall back on just selecting all points in the layer
-			for &point in vector_data.point_domain.ids() {
-				state.select_point(ManipulatorPointId::Anchor(point))
+			// Fall back on just selecting all points/segments in the layer
+			if points {
+				for &point in vector_data.point_domain.ids() {
+					state.select_point(ManipulatorPointId::Anchor(point));
+				}
 			}
-		} else {
-			// Select all connected points
-			while let Some(point) = selected_stack.pop() {
-				let anchor_point = ManipulatorPointId::Anchor(point);
-				if !state.is_point_selected(anchor_point) {
-					state.select_point(anchor_point);
-					selected_stack.extend(vector_data.connected_points(point));
+			if segments {
+				for &segment in vector_data.segment_domain.ids() {
+					state.select_segment(segment);
+				}
+			}
+			return;
+		}
+
+		let mut connected_points = HashSet::new();
+
+		while let Some(point) = selected_stack.pop() {
+			if !connected_points.contains(&point) {
+				connected_points.insert(point);
+				selected_stack.extend(vector_data.connected_points(point));
+			}
+		}
+
+		if points {
+			connected_points.iter().for_each(|point| state.select_point(ManipulatorPointId::Anchor(*point)));
+		}
+
+		if segments {
+			for (id, _, start, end) in vector_data.segment_bezier_iter() {
+				if connected_points.contains(&start) || connected_points.contains(&end) {
+					state.select_segment(id);
 				}
 			}
 		}

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

@@ -1,8 +1,8 @@
 use super::select_tool::extend_lasso;
 use super::tool_prelude::*;
 use crate::consts::{
-	COLOR_OVERLAY_BLUE, COLOR_OVERLAY_GREEN, COLOR_OVERLAY_RED, DRAG_DIRECTION_MODE_DETERMINATION_THRESHOLD, DRAG_THRESHOLD, HANDLE_ROTATE_SNAP_ANGLE, SEGMENT_INSERTION_DISTANCE,
-	SEGMENT_OVERLAY_SIZE, SELECTION_THRESHOLD, SELECTION_TOLERANCE,
+	COLOR_OVERLAY_BLUE, COLOR_OVERLAY_GREEN, COLOR_OVERLAY_RED, DOUBLE_CLICK_MILLISECONDS, DRAG_DIRECTION_MODE_DETERMINATION_THRESHOLD, DRAG_THRESHOLD, HANDLE_ROTATE_SNAP_ANGLE,
+	SEGMENT_INSERTION_DISTANCE, SEGMENT_OVERLAY_SIZE, SELECTION_THRESHOLD, SELECTION_TOLERANCE,
 };
 use crate::messages::portfolio::document::overlays::utility_functions::{path_overlays, selected_segments};
 use crate::messages::portfolio::document::overlays::utility_types::{DrawHandles, OverlayContext};
@@ -12,7 +12,7 @@ use crate::messages::portfolio::document::utility_types::transformation::Axis;
 use crate::messages::preferences::SelectionMode;
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
 use crate::messages::tool::common_functionality::shape_editor::{
-	ClosestSegment, ManipulatorAngle, OpposingHandleLengths, SelectedPointsInfo, SelectionChange, SelectionShape, SelectionShapeType, ShapeState,
+	ClosestSegment, ManipulatorAngle, OpposingHandleLengths, SelectedLayerState, SelectedPointsInfo, SelectionChange, SelectionShape, SelectionShapeType, ShapeState,
 };
 use crate::messages::tool::common_functionality::snapping::{SnapCache, SnapCandidatePoint, SnapConstraint, SnapData, SnapManager};
 use crate::messages::tool::common_functionality::utility_functions::{calculate_segment_angle, find_two_param_best_approximate};
@@ -58,7 +58,10 @@ pub enum PathToolMessage {
 	},
 	Escape,
 	ClosePath,
-	FlipSmoothSharp,
+	DoubleClick {
+		extend_selection: Key,
+		shrink_selection: Key,
+	},
 	GRS {
 		// Should be `Key::KeyG` (Grab), `Key::KeyR` (Rotate), or `Key::KeyS` (Scale)
 		key: Key,
@@ -319,7 +322,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionHandlerData<'a>> for PathToo
 	fn actions(&self) -> ActionList {
 		match self.fsm_state {
 			PathToolFsmState::Ready => actions!(PathToolMessageDiscriminant;
-				FlipSmoothSharp,
+				DoubleClick,
 				MouseDown,
 				Delete,
 				NudgeSelectedPoints,
@@ -334,7 +337,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionHandlerData<'a>> for PathToo
 			PathToolFsmState::Dragging(_) => actions!(PathToolMessageDiscriminant;
 				Escape,
 				RightClick,
-				FlipSmoothSharp,
+				DoubleClick,
 				DragStop,
 				PointerMove,
 				Delete,
@@ -343,7 +346,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionHandlerData<'a>> for PathToo
 				SwapSelectedHandles,
 			),
 			PathToolFsmState::Drawing { .. } => actions!(PathToolMessageDiscriminant;
-				FlipSmoothSharp,
+				DoubleClick,
 				DragStop,
 				PointerMove,
 				Delete,
@@ -462,6 +465,8 @@ struct PathToolData {
 	adjacent_anchor_offset: Option<DVec2>,
 	sliding_point_info: Option<SlidingPointInfo>,
 	started_drawing_from_inside: bool,
+	first_selected_with_single_click: bool,
+	stored_selection: Option<HashMap<LayerNodeIdentifier, SelectedLayerState>>,
 }
 
 impl PathToolData {
@@ -544,8 +549,9 @@ impl PathToolData {
 
 		self.drag_start_pos = input.mouse.position;
 
-		if !self.saved_points_before_anchor_convert_smooth_sharp.is_empty() && (input.time - self.last_click_time > 500) {
+		if input.time - self.last_click_time > DOUBLE_CLICK_MILLISECONDS {
 			self.saved_points_before_anchor_convert_smooth_sharp.clear();
+			self.stored_selection = None;
 		}
 
 		self.last_click_time = input.time;
@@ -685,20 +691,18 @@ impl PathToolData {
 				PathToolFsmState::MoldingSegment
 			}
 		}
-		// We didn't find a segment, so consider selecting the nearest shape instead and start drawing
+		// If no other layers are selected and this is a single-click, then also select the layer (exception)
 		else if let Some(layer) = document.click(input) {
-			shape_editor.deselect_all_points();
-			shape_editor.deselect_all_segments();
-			if extend_selection {
-				responses.add(NodeGraphMessage::SelectedNodesAdd { nodes: vec![layer.to_node()] });
-			} else {
+			if shape_editor.selected_shape_state.is_empty() {
+				self.first_selected_with_single_click = true;
 				responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![layer.to_node()] });
 			}
-			self.drag_start_pos = input.mouse.position;
-			self.previous_mouse_position = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);
 
 			self.started_drawing_from_inside = true;
 
+			self.drag_start_pos = input.mouse.position;
+			self.previous_mouse_position = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);
+
 			let selection_shape = if lasso_select { SelectionShapeType::Lasso } else { SelectionShapeType::Box };
 			PathToolFsmState::Drawing { selection_shape }
 		}
@@ -1557,7 +1561,9 @@ impl Fsm for PathToolFsmState {
 				},
 			) => {
 				tool_data.previous_mouse_position = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);
+
 				tool_data.started_drawing_from_inside = false;
+				tool_data.stored_selection = None;
 
 				if selection_shape == SelectionShapeType::Lasso {
 					extend_lasso(&mut tool_data.lasso_polygon, input.mouse.position);
@@ -1604,6 +1610,7 @@ impl Fsm for PathToolFsmState {
 					break_colinear_molding,
 				},
 			) => {
+				tool_data.stored_selection = None;
 				let mut selected_only_handles = true;
 
 				let selected_points = shape_editor.selected_points();
@@ -1727,6 +1734,7 @@ impl Fsm for PathToolFsmState {
 				if tool_data.adjacent_anchor_offset.is_some() {
 					tool_data.adjacent_anchor_offset = None;
 				}
+				tool_data.stored_selection = None;
 
 				responses.add(OverlaysMessage::Draw);
 
@@ -1895,12 +1903,16 @@ impl Fsm for PathToolFsmState {
 					SelectionMode::Directional => tool_data.calculate_selection_mode_from_direction(document.metadata()),
 					selection_mode => selection_mode,
 				};
+				tool_data.started_drawing_from_inside = false;
 
 				if tool_data.drag_start_pos.distance(previous_mouse) < 1e-8 {
-					// If click happens inside of a shape then don't set selected nodes to empty
-					if document.click(input).is_none() {
-						responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![] });
+					// Clicked inside or outside the shape then deselect all of the points/segments
+					if document.click(input).is_some() && tool_data.stored_selection.is_none() {
+						tool_data.stored_selection = Some(shape_editor.selected_shape_state.clone());
 					}
+
+					shape_editor.deselect_all_points();
+					shape_editor.deselect_all_segments();
 				} else {
 					match selection_shape {
 						SelectionShapeType::Box => {
@@ -2072,8 +2084,8 @@ impl Fsm for PathToolFsmState {
 				shape_editor.delete_point_and_break_path(document, responses);
 				PathToolFsmState::Ready
 			}
-			(_, PathToolMessage::FlipSmoothSharp) => {
-				// Double-clicked on a point
+			(_, PathToolMessage::DoubleClick { extend_selection, shrink_selection }) => {
+				// Double-clicked on a point (flip smooth/sharp behavior)
 				let nearest_point = shape_editor.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD);
 				if nearest_point.is_some() {
 					// Flip the selected point between smooth and sharp
@@ -2090,13 +2102,70 @@ impl Fsm for PathToolFsmState {
 
 					return PathToolFsmState::Ready;
 				}
-
 				// Double-clicked on a filled region
-				if let Some(layer) = document.click(input) {
-					// Select all points in the layer
-					shape_editor.select_connected_anchors(document, layer, input.mouse.position);
+				else if let Some(layer) = document.click(input) {
+					let extend_selection = input.keyboard.get(extend_selection as usize);
+					let shrink_selection = input.keyboard.get(shrink_selection as usize);
+
+					if shape_editor.is_selected_layer(layer) {
+						if extend_selection && !tool_data.first_selected_with_single_click {
+							responses.add(NodeGraphMessage::SelectedNodesRemove { nodes: vec![layer.to_node()] });
+
+							if let Some(selection) = &tool_data.stored_selection {
+								let mut selection = selection.clone();
+								selection.remove(&layer);
+								shape_editor.selected_shape_state = selection;
+								tool_data.stored_selection = None;
+							}
+						} else if shrink_selection && !tool_data.first_selected_with_single_click {
+							// Only deselect all the points of the double clicked layer
+							if let Some(selection) = &tool_data.stored_selection {
+								let selection = selection.clone();
+								shape_editor.selected_shape_state = selection;
+								tool_data.stored_selection = None;
+							}
+
+							let state = shape_editor.selected_shape_state.get_mut(&layer).expect("No state for selected layer");
+							state.deselect_all_points_in_layer();
+							state.deselect_all_segments_in_layer();
+						} else if !tool_data.first_selected_with_single_click {
+							// Select according to the selected editing mode
+							let point_editing_mode = tool_options.path_editing_mode.point_editing_mode;
+							let segment_editing_mode = tool_options.path_editing_mode.segment_editing_mode;
+							shape_editor.select_connected(document, layer, input.mouse.position, point_editing_mode, segment_editing_mode);
+
+							// Select all the other layers back again
+							if let Some(selection) = &tool_data.stored_selection {
+								let mut selection = selection.clone();
+								selection.remove(&layer);
+
+								for (layer, state) in selection {
+									shape_editor.selected_shape_state.insert(layer, state);
+								}
+								tool_data.stored_selection = None;
+							}
+						}
+
+						// If it was the very first click without there being an existing selection,
+						// then the single-click behavior and double-click behavior should not collide
+						tool_data.first_selected_with_single_click = false;
+					} else if extend_selection {
+						responses.add(NodeGraphMessage::SelectedNodesAdd { nodes: vec![layer.to_node()] });
+
+						if let Some(selection) = &tool_data.stored_selection {
+							shape_editor.selected_shape_state = selection.clone();
+							tool_data.stored_selection = None;
+						}
+					} else {
+						responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![layer.to_node()] });
+					}
+
 					responses.add(OverlaysMessage::Draw);
 				}
+				// Double clicked on the background
+				else {
+					responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![] });
+				}
 
 				PathToolFsmState::Ready
 			}