From 7cb16a8bec135bd7f63ed9bd8658e8eb6e6cf55c Mon Sep 17 00:00:00 2001 From: Utsav Singh Date: Thu, 17 Apr 2025 15:27:56 +0530 Subject: [PATCH] Fix Path tool behavior with Shift-dragging an already selected point, where it wrongly got deselected (#2395) * Send path tool FSM to Dragging state on MouseDown if clicking selected point * Cleanup * Store selected point state before new selection is made and setup deselect logic * update previously saved point data on every point selection * Decide whether to deselect or select on extended_select if node not already selected, when DragStop state is reached instead of inside the mouse_down function. * Fix broken merge and remove leftover debug statements * Code review --------- Co-authored-by: Keavon Chambers --- .../tool/common_functionality/shape_editor.rs | 31 ++++++++++++++ .../messages/tool/tool_messages/path_tool.rs | 41 +++++++++++++++---- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index 22060280..e363f3db 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/editor/src/messages/tool/common_functionality/shape_editor.rs @@ -388,6 +388,37 @@ impl ShapeState { None } + pub fn get_point_selection_state(&mut self, network_interface: &NodeNetworkInterface, mouse_position: DVec2, select_threshold: f64) -> Option<(bool, Option)> { + if self.selected_shape_state.is_empty() { + return None; + } + + if let Some((layer, manipulator_point_id)) = self.find_nearest_point_indices(network_interface, mouse_position, select_threshold) { + let vector_data = network_interface.compute_modified_vector(layer)?; + let point_position = manipulator_point_id.get_position(&vector_data)?; + + let selected_shape_state = self.selected_shape_state.get(&layer)?; + let already_selected = selected_shape_state.is_selected(manipulator_point_id); + + // Offset to snap the selected point to the cursor + let offset = mouse_position - network_interface.document_metadata().transform_to_viewport(layer).transform_point2(point_position); + + // Gather current selection information + let points = self + .selected_shape_state + .iter() + .flat_map(|(layer, state)| state.selected_points.iter().map(|&point_id| ManipulatorPointInfo { layer: *layer, point_id })) + .collect(); + + let selection_info = SelectedPointsInfo { points, offset, vector_data }; + + // Return the current selection state and info + return Some((already_selected, Some(selection_info))); + } + + None + } + pub fn select_anchor_point_by_id(&mut self, layer: LayerNodeIdentifier, id: PointId, extend_selection: bool) { if !extend_selection { self.deselect_all_points(); diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index 6b348cb4..4918c1ff 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -375,6 +375,7 @@ struct PathToolData { current_selected_handle_id: Option, angle: f64, opposite_handle_position: Option, + last_clicked_point_was_selected: bool, snapping_axis: Option, alt_clicked_on_anchor: bool, alt_dragging_from_anchor: bool, @@ -501,11 +502,21 @@ impl PathToolData { let old_selection = shape_editor.selected_points().cloned().collect::>(); - // Select the first point within the threshold (in pixels) - if let Some(selected_points) = shape_editor.change_point_selection(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD, extend_selection) { + // Check if the point is already selected; if not, select the first point within the threshold (in pixels) + if let Some((already_selected, mut selection_info)) = shape_editor.get_point_selection_state(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD) { responses.add(DocumentMessage::StartTransaction); - if let Some(selected_points) = selected_points { + self.last_clicked_point_was_selected = already_selected; + + // If the point is already selected and shift (`extend_selection`) is used, keep the selection unchanged. + // Otherwise, select the first point within the threshold. + if !(already_selected && extend_selection) { + if let Some(updated_selection_info) = shape_editor.change_point_selection(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD, extend_selection) { + selection_info = updated_selection_info; + } + } + + if let Some(selected_points) = selection_info { self.drag_start_pos = input.mouse.position; // If selected points contain only handles and there was some selection before, then it is stored and becomes restored upon release @@ -1386,7 +1397,23 @@ impl Fsm for PathToolFsmState { PathToolFsmState::Ready } (_, PathToolMessage::DragStop { extend_selection, .. }) => { - if tool_data.handle_drag_toggle && tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD { + let extend_selection = input.keyboard.get(extend_selection as usize); + let drag_occurred = tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD; + let nearest_point = shape_editor.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD); + + if let Some((layer, nearest_point)) = nearest_point { + if !drag_occurred && extend_selection { + let clicked_selected = shape_editor.selected_points().any(|&point| nearest_point == point); + if clicked_selected && tool_data.last_clicked_point_was_selected { + shape_editor.selected_shape_state.entry(layer).or_default().deselect_point(nearest_point); + } else { + shape_editor.selected_shape_state.entry(layer).or_default().select_point(nearest_point); + } + responses.add(OverlaysMessage::Draw); + } + } + + if tool_data.handle_drag_toggle && drag_occurred { shape_editor.deselect_all_points(); shape_editor.select_points_by_manipulator_id(&tool_data.saved_points_before_handle_drag); @@ -1404,12 +1431,8 @@ impl Fsm for PathToolFsmState { tool_data.select_anchor_toggled = false; } - let extend_selection = input.keyboard.get(extend_selection as usize); - - let nearest_point = shape_editor.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD); - if let Some((layer, nearest_point)) = nearest_point { - if tool_data.drag_start_pos.distance(input.mouse.position) <= DRAG_THRESHOLD && !extend_selection { + if !drag_occurred && !extend_selection { let clicked_selected = shape_editor.selected_points().any(|&point| nearest_point == point); if clicked_selected { shape_editor.deselect_all_points();