Fix editor crash due to mismanaged selected points on layers (#2640)

* Add hash sets to hold ignored points in SelectedLayerState

* Fix non selected anchor dragging

* Update selected points when ignoring handles or anchors

* Refactor selected points status logic

* Refactor ignore_handles and ignore_anchors bools to ShapeState

* Add back in ignore_anchors and ignore_handles in SelectedLayerState

* Code review

---------

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

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

@@ -44,46 +44,67 @@ pub enum ManipulatorAngle {
 #[derive(Clone, Debug, Default)]
 pub struct SelectedLayerState {
 	selected_points: HashSet<ManipulatorPointId>,
+	/// Keeps track of the current state; helps avoid unnecessary computation when called by [`ShapeState`].
 	ignore_handles: bool,
 	ignore_anchors: bool,
+	/// Points that are selected but ignored (when their overlays are disabled) are stored here.
+	ignored_handle_points: HashSet<ManipulatorPointId>,
+	ignored_anchor_points: HashSet<ManipulatorPointId>,
 }
 
 impl SelectedLayerState {
 	pub fn selected(&self) -> impl Iterator<Item = ManipulatorPointId> + '_ {
 		self.selected_points.iter().copied()
 	}
+
 	pub fn is_selected(&self, point: ManipulatorPointId) -> bool {
 		self.selected_points.contains(&point)
 	}
+
 	pub fn select_point(&mut self, point: ManipulatorPointId) {
-		if (point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors) {
-			return;
-		}
 		self.selected_points.insert(point);
 	}
+
 	pub fn deselect_point(&mut self, point: ManipulatorPointId) {
-		if (point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors) {
-			return;
-		}
 		self.selected_points.remove(&point);
 	}
-	pub fn set_handles_status(&mut self, ignore: bool) {
-		self.ignore_handles = ignore;
-	}
-	pub fn set_anchors_status(&mut self, ignore: bool) {
-		self.ignore_anchors = ignore;
-	}
-	pub fn clear_points_force(&mut self) {
-		self.selected_points.clear();
-		self.ignore_handles = false;
-		self.ignore_anchors = false;
-	}
-	pub fn clear_points(&mut self) {
-		if self.ignore_handles || self.ignore_anchors {
+
+	pub fn ignore_handles(&mut self, status: bool) {
+		if self.ignore_handles == !status {
 			return;
 		}
+
+		self.ignore_handles = !status;
+
+		if self.ignore_handles {
+			self.ignored_handle_points.extend(self.selected_points.iter().copied().filter(|point| point.as_handle().is_some()));
+			self.selected_points.retain(|point| !self.ignored_handle_points.contains(point));
+		} else {
+			self.selected_points.extend(self.ignored_handle_points.iter().copied());
+			self.ignored_handle_points.clear();
+		}
+	}
+
+	pub fn ignore_anchors(&mut self, status: bool) {
+		if self.ignore_anchors == !status {
+			return;
+		}
+
+		self.ignore_anchors = !status;
+
+		if self.ignore_anchors {
+			self.ignored_anchor_points.extend(self.selected_points.iter().copied().filter(|point| point.as_anchor().is_some()));
+			self.selected_points.retain(|point| !self.ignored_anchor_points.contains(point));
+		} else {
+			self.selected_points.extend(self.ignored_anchor_points.iter().copied());
+			self.ignored_anchor_points.clear();
+		}
+	}
+
+	pub fn clear_points(&mut self) {
 		self.selected_points.clear();
 	}
+
 	pub fn selected_points_count(&self) -> usize {
 		self.selected_points.len()
 	}
@@ -93,8 +114,10 @@ pub type SelectedShapeState = HashMap<LayerNodeIdentifier, SelectedLayerState>;
 
 #[derive(Debug, Default)]
 pub struct ShapeState {
-	// The layers we can select and edit manipulators (anchors and handles) from
+	/// The layers we can select and edit manipulators (anchors and handles) from.
 	pub selected_shape_state: SelectedShapeState,
+	ignore_handles: bool,
+	ignore_anchors: bool,
 }
 
 #[derive(Debug)]
@@ -255,6 +278,10 @@ impl ClosestSegment {
 
 // TODO Consider keeping a list of selected manipulators to minimize traversals of the layers
 impl ShapeState {
+	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)
+	}
+
 	pub fn close_selected_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
 		// First collect all selected anchor points across all layers
 		let all_selected_points: Vec<(LayerNodeIdentifier, PointId)> = self
@@ -507,8 +534,9 @@ impl ShapeState {
 		} else {
 			// Select all connected points
 			while let Some(point) = selected_stack.pop() {
-				if !state.is_selected(ManipulatorPointId::Anchor(point)) {
-					state.select_point(ManipulatorPointId::Anchor(point));
+				let anchor_point = ManipulatorPointId::Anchor(point);
+				if !state.is_selected(anchor_point) {
+					state.select_point(anchor_point);
 					selected_stack.extend(vector_data.connected_points(point));
 				}
 			}
@@ -548,27 +576,17 @@ impl ShapeState {
 		}
 	}
 
-	pub fn mark_selected_anchors(&mut self) {
+	pub fn update_selected_anchors_status(&mut self, status: bool) {
 		for state in self.selected_shape_state.values_mut() {
-			state.set_anchors_status(false);
+			self.ignore_anchors = !status;
+			state.ignore_anchors(status);
 		}
 	}
 
-	pub fn mark_selected_handles(&mut self) {
+	pub fn update_selected_handles_status(&mut self, status: bool) {
 		for state in self.selected_shape_state.values_mut() {
-			state.set_handles_status(false);
-		}
-	}
-
-	pub fn ignore_selected_anchors(&mut self) {
-		for state in self.selected_shape_state.values_mut() {
-			state.set_anchors_status(true);
-		}
-	}
-
-	pub fn ignore_selected_handles(&mut self) {
-		for state in self.selected_shape_state.values_mut() {
-			state.set_handles_status(true);
+			self.ignore_handles = !status;
+			state.ignore_handles(status);
 		}
 	}
 
@@ -675,6 +693,10 @@ impl ShapeState {
 		layer: LayerNodeIdentifier,
 		responses: &mut VecDeque<Message>,
 	) -> Option<()> {
+		if self.is_point_ignored(point) {
+			return None;
+		}
+
 		let vector_data = network_interface.compute_modified_vector(layer)?;
 		let transform = network_interface.document_metadata().transform_to_document(layer).inverse();
 		let position = transform.transform_point2(new_position);
@@ -924,6 +946,10 @@ impl ShapeState {
 			let delta = delta_transform.inverse().transform_vector2(delta);
 
 			for &point in state.selected_points.iter() {
+				if self.is_point_ignored(&point) {
+					continue;
+				}
+
 				let handle = match point {
 					ManipulatorPointId::Anchor(point) => {
 						self.move_anchor(point, &vector_data, delta, layer, Some(state), responses);
@@ -1596,7 +1622,7 @@ impl ShapeState {
 	pub fn select_all_in_shape(&mut self, network_interface: &NodeNetworkInterface, selection_shape: SelectionShape, selection_change: SelectionChange) {
 		for (&layer, state) in &mut self.selected_shape_state {
 			if selection_change == SelectionChange::Clear {
-				state.clear_points_force()
+				state.clear_points()
 			}
 
 			let vector_data = network_interface.compute_modified_vector(layer);

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

@@ -100,6 +100,9 @@ pub enum PathToolMessage {
 	},
 	SwapSelectedHandles,
 	UpdateOptions(PathOptionsUpdate),
+	UpdateSelectedPointsStatus {
+		overlay_context: OverlayContext,
+	},
 }
 
 #[derive(PartialEq, Eq, Hash, Copy, Clone, Debug, Default, serde::Serialize, serde::Deserialize, specta::Type)]
@@ -989,24 +992,18 @@ impl Fsm for PathToolFsmState {
 				shape_editor.set_selected_layers(target_layers);
 
 				responses.add(OverlaysMessage::Draw);
+				self
+			}
+			(_, PathToolMessage::UpdateSelectedPointsStatus { overlay_context }) => {
+				let display_anchors = overlay_context.visibility_settings.anchors();
+				let display_handles = overlay_context.visibility_settings.handles();
+
+				shape_editor.update_selected_anchors_status(display_anchors);
+				shape_editor.update_selected_handles_status(display_handles);
 
-				responses.add(PathToolMessage::SelectedPointUpdated);
 				self
 			}
 			(_, PathToolMessage::Overlays(mut overlay_context)) => {
-				let display_anchors = overlay_context.visibility_settings.anchors();
-				let display_handles = overlay_context.visibility_settings.handles();
-				if !display_handles {
-					shape_editor.ignore_selected_handles();
-				} else {
-					shape_editor.mark_selected_handles();
-				}
-				if !display_anchors {
-					shape_editor.ignore_selected_anchors();
-				} else {
-					shape_editor.mark_selected_anchors();
-				}
-
 				// TODO: find the segment ids of which the selected points are a part of
 
 				match tool_options.path_overlay_mode {
@@ -1133,6 +1130,7 @@ impl Fsm for PathToolFsmState {
 				}
 
 				responses.add(PathToolMessage::SelectedPointUpdated);
+				responses.add(PathToolMessage::UpdateSelectedPointsStatus { overlay_context });
 				self
 			}