From ea59f10b507cbc66c3bc494993295ec842c00356 Mon Sep 17 00:00:00 2001 From: seam0s <153828136+seam0s-dev@users.noreply.github.com> Date: Mon, 19 May 2025 00:09:58 +0300 Subject: [PATCH] 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 --- .../tool/common_functionality/shape_editor.rs | 104 +++++++++++------- .../messages/tool/tool_messages/path_tool.rs | 26 ++--- 2 files changed, 77 insertions(+), 53 deletions(-) diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index 62e7eb45e..39053e93a 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/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, + /// 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, + ignored_anchor_points: HashSet, } impl SelectedLayerState { pub fn selected(&self) -> impl Iterator + '_ { 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; #[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) { // 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, ) -> 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); diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index 766a55467..5d9f62286 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/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 }