Insert point on segment by clicking once (no more sliding) and Alt+click to delete a segment (#2495)

* segment overlay change

* Segment split and delete

* Cleanup

* graceful handling of edge cases

* Moved constants to conts.rs and tuned the threshold

* Remove going into another state

* Insert point mode cleanup

* Linting fix

* Code review

* Added hints

* Added field for delete segment

* Change controls and fix too far logic

* Fixes

* Code review

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
Adesh Gupta 2025-04-30 06:33:24 +05:30 committed by GitHub
parent 478ace3349
commit da38f672ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 138 additions and 127 deletions

View file

@ -99,8 +99,9 @@ pub const MIN_LENGTH_FOR_SKEW_TRIANGLE_VISIBILITY: f64 = 48.;
pub const MANIPULATOR_GROUP_MARKER_SIZE: f64 = 6.;
pub const SELECTION_THRESHOLD: f64 = 10.;
pub const HIDE_HANDLE_DISTANCE: f64 = 3.;
pub const INSERT_POINT_ON_SEGMENT_TOO_FAR_DISTANCE: f64 = 50.;
pub const HANDLE_ROTATE_SNAP_ANGLE: f64 = 15.;
pub const SEGMENT_INSERTION_DISTANCE: f64 = 7.5;
pub const SEGMENT_OVERLAY_SIZE: f64 = 10.;
// PEN TOOL
pub const CREATE_CURVE_THRESHOLD: f64 = 5.;

View file

@ -212,13 +212,13 @@ pub fn input_mappings() -> Mapping {
entry!(KeyDown(Delete); modifiers=[Shift], action_dispatch=PathToolMessage::BreakPath),
entry!(KeyDown(Backspace); modifiers=[Shift], action_dispatch=PathToolMessage::BreakPath),
entry!(KeyDownNoRepeat(Tab); action_dispatch=PathToolMessage::SwapSelectedHandles),
entry!(KeyDown(MouseLeft); action_dispatch=PathToolMessage::MouseDown { direct_insert_without_sliding: Control, extend_selection: Shift, lasso_select: Control, handle_drag_from_anchor: Alt }),
entry!(KeyDown(MouseLeft); action_dispatch=PathToolMessage::MouseDown { extend_selection: Shift, lasso_select: Control, handle_drag_from_anchor: Alt }),
entry!(KeyDown(MouseRight); action_dispatch=PathToolMessage::RightClick),
entry!(KeyDown(Escape); action_dispatch=PathToolMessage::Escape),
entry!(KeyDown(KeyG); action_dispatch=PathToolMessage::GRS { key: KeyG }),
entry!(KeyDown(KeyR); action_dispatch=PathToolMessage::GRS { key: KeyR }),
entry!(KeyDown(KeyS); action_dispatch=PathToolMessage::GRS { key: KeyS }),
entry!(PointerMove; refresh_keys=[KeyC, Space, Control, Shift, Alt], action_dispatch=PathToolMessage::PointerMove { toggle_colinear: KeyC, equidistant: Alt, move_anchor_with_handles: Space, snap_angle: Shift, lock_angle: Control }),
entry!(PointerMove; refresh_keys=[KeyC, Space, Control, Shift, Alt], action_dispatch=PathToolMessage::PointerMove { toggle_colinear: KeyC, equidistant: Alt, move_anchor_with_handles: Space, snap_angle: Shift, lock_angle: Control, delete_segment: Alt }),
entry!(KeyDown(Delete); action_dispatch=PathToolMessage::Delete),
entry!(KeyDown(KeyA); modifiers=[Accel], action_dispatch=PathToolMessage::SelectAllAnchors),
entry!(KeyDown(KeyA); modifiers=[Accel, Shift], action_dispatch=PathToolMessage::DeselectAllPoints),

View file

@ -104,6 +104,14 @@ impl ClosestSegment {
self.layer
}
pub fn segment(&self) -> SegmentId {
self.segment
}
pub fn points(&self) -> [PointId; 2] {
self.points
}
pub fn closest_point_to_viewport(&self) -> DVec2 {
self.bezier_point_to_viewport
}
@ -128,9 +136,7 @@ impl ClosestSegment {
pub fn too_far(&self, mouse_position: DVec2, tolerance: f64, document_metadata: &DocumentMetadata) -> bool {
let dist_sq = self.distance_squared(mouse_position);
let stroke_width = document_metadata.document_to_viewport.decompose_scale().x.max(1.) * self.stroke_width;
let stroke_width_sq = stroke_width * stroke_width;
let tolerance_sq = tolerance * tolerance;
(stroke_width_sq + tolerance_sq) < dist_sq
(stroke_width + tolerance).powi(2) < dist_sq
}
pub fn handle_positions(&self, document_metadata: &DocumentMetadata) -> (Option<DVec2>, Option<DVec2>) {
@ -199,6 +205,28 @@ impl ClosestSegment {
let id = self.adjusted_insert(responses);
shape_editor.select_anchor_point_by_id(self.layer, id, extend_selection)
}
pub fn calculate_perp(&self, document: &DocumentMessageHandler) -> DVec2 {
let tangent = if let (Some(handle1), Some(handle2)) = self.handle_positions(document.metadata()) {
(handle1 - handle2).try_normalize()
} else {
let [first_point, last_point] = self.points();
if let Some(vector_data) = document.network_interface.compute_modified_vector(self.layer()) {
if let (Some(pos1), Some(pos2)) = (
ManipulatorPointId::Anchor(first_point).get_position(&vector_data),
ManipulatorPointId::Anchor(last_point).get_position(&vector_data),
) {
(pos1 - pos2).try_normalize()
} else {
None
}
} else {
None
}
}
.unwrap_or(DVec2::ZERO);
tangent.perp()
}
}
// TODO Consider keeping a list of selected manipulators to minimize traversals of the layers
@ -900,6 +928,29 @@ impl ShapeState {
.collect::<HashMap<_, _>>()
}
pub fn dissolve_segment(&self, responses: &mut VecDeque<Message>, layer: LayerNodeIdentifier, vector_data: &VectorData, segment: SegmentId, points: [PointId; 2]) {
// Checking which point is terminal point
let is_point1_terminal = vector_data.connected_count(points[0]) == 1;
let is_point2_terminal = vector_data.connected_count(points[1]) == 1;
// Delete the segment and terminal points
let modification_type = VectorModificationType::RemoveSegment { id: segment };
responses.add(GraphOperationMessage::Vector { layer, modification_type });
for &handles in vector_data.colinear_manipulators.iter().filter(|handles| handles.iter().any(|handle| handle.segment == segment)) {
let modification_type = VectorModificationType::SetG1Continuous { handles, enabled: false };
responses.add(GraphOperationMessage::Vector { layer, modification_type });
}
if is_point1_terminal {
let modification_type = VectorModificationType::RemovePoint { id: points[0] };
responses.add(GraphOperationMessage::Vector { layer, modification_type });
}
if is_point2_terminal {
let modification_type = VectorModificationType::RemovePoint { id: points[1] };
responses.add(GraphOperationMessage::Vector { layer, modification_type });
}
}
fn dissolve_anchor(anchor: PointId, responses: &mut VecDeque<Message>, layer: LayerNodeIdentifier, vector_data: &VectorData) -> Option<[(HandleId, PointId); 2]> {
// Delete point
let modification_type = VectorModificationType::RemovePoint { id: anchor };

View file

@ -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, INSERT_POINT_ON_SEGMENT_TOO_FAR_DISTANCE,
SELECTION_THRESHOLD, SELECTION_TOLERANCE,
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,
};
use crate::messages::portfolio::document::overlays::utility_functions::{path_overlays, selected_segments};
use crate::messages::portfolio::document::overlays::utility_types::{DrawHandles, OverlayContext};
@ -64,7 +64,6 @@ pub enum PathToolMessage {
ManipulatorMakeHandlesFree,
ManipulatorMakeHandlesColinear,
MouseDown {
direct_insert_without_sliding: Key,
extend_selection: Key,
lasso_select: Key,
handle_drag_from_anchor: Key,
@ -79,6 +78,7 @@ pub enum PathToolMessage {
move_anchor_with_handles: Key,
snap_angle: Key,
lock_angle: Key,
delete_segment: Key,
},
PointerOutsideViewport {
equidistant: Key,
@ -86,6 +86,7 @@ pub enum PathToolMessage {
move_anchor_with_handles: Key,
snap_angle: Key,
lock_angle: Key,
delete_segment: Key,
},
RightClick,
SelectAllAnchors,
@ -274,6 +275,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionHandlerData<'a>> for PathToo
BreakPath,
DeleteAndBreakPath,
ClosePath,
PointerMove,
),
PathToolFsmState::Dragging(_) => actions!(PathToolMessageDiscriminant;
Escape,
@ -297,15 +299,6 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionHandlerData<'a>> for PathToo
Escape,
RightClick,
),
PathToolFsmState::InsertPoint => actions!(PathToolMessageDiscriminant;
Enter,
MouseDown,
PointerMove,
Escape,
Delete,
RightClick,
GRS,
),
}
}
}
@ -342,12 +335,6 @@ enum PathToolFsmState {
Drawing {
selection_shape: SelectionShapeType,
},
InsertPoint,
}
enum InsertEndKind {
Abort,
Add { extend_selection: bool },
}
#[derive(Default)]
@ -368,6 +355,7 @@ struct PathToolData {
segment: Option<ClosestSegment>,
snap_cache: SnapCache,
double_click_handled: bool,
delete_segment_pressed: bool,
auto_panning: AutoPanning,
saved_points_before_anchor_select_toggle: Vec<ManipulatorPointId>,
select_anchor_toggled: bool,
@ -440,53 +428,6 @@ impl PathToolData {
self.selection_status = selection_status;
}
fn start_insertion(&mut self, responses: &mut VecDeque<Message>, segment: ClosestSegment) -> PathToolFsmState {
if self.segment.is_some() {
warn!("Segment was `Some(..)` before `start_insertion`")
}
self.segment = Some(segment);
responses.add(OverlaysMessage::Draw);
PathToolFsmState::InsertPoint
}
fn update_insertion(&mut self, shape_editor: &mut ShapeState, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>, input: &InputPreprocessorMessageHandler) -> PathToolFsmState {
if let Some(closed_segment) = &mut self.segment {
closed_segment.update_closest_point(document.metadata(), input.mouse.position);
if closed_segment.too_far(input.mouse.position, INSERT_POINT_ON_SEGMENT_TOO_FAR_DISTANCE, document.metadata()) {
self.end_insertion(shape_editor, responses, InsertEndKind::Abort)
} else {
PathToolFsmState::InsertPoint
}
} else {
warn!("Segment was `None` on `update_insertion`");
PathToolFsmState::Ready
}
}
fn end_insertion(&mut self, shape_editor: &mut ShapeState, responses: &mut VecDeque<Message>, kind: InsertEndKind) -> PathToolFsmState {
let mut commit_transaction = false;
match self.segment.as_mut() {
None => {
warn!("Segment was `None` before `end_insertion`")
}
Some(closed_segment) => {
if let InsertEndKind::Add { extend_selection } = kind {
closed_segment.adjusted_insert_and_select(shape_editor, responses, extend_selection);
commit_transaction = true;
}
}
}
self.segment = None;
if commit_transaction {
responses.add(DocumentMessage::EndTransaction);
} else {
responses.add(DocumentMessage::AbortTransaction);
}
responses.add(OverlaysMessage::Draw);
PathToolFsmState::Ready
}
#[allow(clippy::too_many_arguments)]
fn mouse_down(
&mut self,
@ -495,7 +436,6 @@ impl PathToolData {
input: &InputPreprocessorMessageHandler,
responses: &mut VecDeque<Message>,
extend_selection: bool,
direct_insert_without_sliding: bool,
lasso_select: bool,
handle_drag_from_anchor: bool,
) -> PathToolFsmState {
@ -565,17 +505,25 @@ impl PathToolData {
}
PathToolFsmState::Dragging(self.dragging_state)
}
// We didn't find a point nearby, so now we'll try to add a point into the closest path segment
else if let Some(closed_segment) = shape_editor.upper_closest_segment(&document.network_interface, input.mouse.position, SELECTION_TOLERANCE) {
// We didn't find a point nearby, so we will see if there is a segment to insert a point on
else if let Some(closed_segment) = &mut self.segment {
responses.add(DocumentMessage::StartTransaction);
if direct_insert_without_sliding {
self.start_insertion(responses, closed_segment);
self.end_insertion(shape_editor, responses, InsertEndKind::Add { extend_selection })
if self.delete_segment_pressed {
if let Some(vector_data) = document.network_interface.compute_modified_vector(closed_segment.layer()) {
shape_editor.dissolve_segment(responses, closed_segment.layer(), &vector_data, closed_segment.segment(), closed_segment.points());
responses.add(DocumentMessage::EndTransaction);
}
} else {
self.start_insertion(responses, closed_segment)
closed_segment.adjusted_insert_and_select(shape_editor, responses, extend_selection);
responses.add(DocumentMessage::EndTransaction);
}
self.segment = None;
PathToolFsmState::Ready
}
// We didn't find a segment path, so consider selecting the nearest shape instead
// We didn't find a segment, so consider selecting the nearest shape instead
else if let Some(layer) = document.click(input) {
shape_editor.deselect_all_points();
if extend_selection {
@ -1066,6 +1014,26 @@ impl Fsm for PathToolFsmState {
}
match self {
Self::Ready => {
if let Some(closest_segment) = &tool_data.segment {
let perp = closest_segment.calculate_perp(document);
let point = closest_segment.closest_point_to_viewport();
// Draw an X on the segment
if tool_data.delete_segment_pressed {
let angle = 45_f64.to_radians();
let tilted_line = DVec2::from_angle(angle).rotate(perp);
let tilted_perp = tilted_line.perp();
overlay_context.line(point - tilted_line * SEGMENT_OVERLAY_SIZE, point + tilted_line * SEGMENT_OVERLAY_SIZE, Some(COLOR_OVERLAY_BLUE), None);
overlay_context.line(point - tilted_perp * SEGMENT_OVERLAY_SIZE, point + tilted_perp * SEGMENT_OVERLAY_SIZE, Some(COLOR_OVERLAY_BLUE), None);
}
// Draw a line on the segment
else {
overlay_context.line(point - perp * SEGMENT_OVERLAY_SIZE, point + perp * SEGMENT_OVERLAY_SIZE, Some(COLOR_OVERLAY_BLUE), None);
}
}
}
Self::Drawing { selection_shape } => {
let mut fill_color = graphene_std::Color::from_rgb_str(COLOR_OVERLAY_BLUE.strip_prefix('#').unwrap())
.unwrap()
@ -1115,71 +1083,30 @@ impl Fsm for PathToolFsmState {
}
}
}
Self::InsertPoint => {
let state = tool_data.update_insertion(shape_editor, document, responses, input);
if let Some(closest_segment) = &tool_data.segment {
overlay_context.manipulator_anchor(closest_segment.closest_point_to_viewport(), false, Some(COLOR_OVERLAY_BLUE));
if let (Some(handle1), Some(handle2)) = closest_segment.handle_positions(document.metadata()) {
overlay_context.line(closest_segment.closest_point_to_viewport(), handle1, Some(COLOR_OVERLAY_BLUE), None);
overlay_context.line(closest_segment.closest_point_to_viewport(), handle2, Some(COLOR_OVERLAY_BLUE), None);
overlay_context.manipulator_handle(handle1, false, Some(COLOR_OVERLAY_BLUE));
overlay_context.manipulator_handle(handle2, false, Some(COLOR_OVERLAY_BLUE));
}
}
responses.add(PathToolMessage::SelectedPointUpdated);
return state;
}
_ => {}
}
responses.add(PathToolMessage::SelectedPointUpdated);
self
}
// `Self::InsertPoint` case:
(Self::InsertPoint, PathToolMessage::MouseDown { extend_selection, .. } | PathToolMessage::Enter { extend_selection, .. }) => {
tool_data.double_click_handled = true;
let extend_selection = input.keyboard.get(extend_selection as usize);
tool_data.end_insertion(shape_editor, responses, InsertEndKind::Add { extend_selection })
}
(Self::InsertPoint, PathToolMessage::PointerMove { .. }) => {
responses.add(OverlaysMessage::Draw);
// `tool_data.update_insertion` would be called on `OverlaysMessage::Draw`
// we anyway should to call it on `::Draw` because we can change scale by ctrl+scroll without `::PointerMove`
self
}
(Self::InsertPoint, PathToolMessage::Escape | PathToolMessage::Delete | PathToolMessage::RightClick) => tool_data.end_insertion(shape_editor, responses, InsertEndKind::Abort),
(Self::InsertPoint, PathToolMessage::GRS { key: _ }) => PathToolFsmState::InsertPoint,
// Mouse down
(
_,
PathToolMessage::MouseDown {
direct_insert_without_sliding,
extend_selection,
lasso_select,
handle_drag_from_anchor,
..
},
) => {
let extend_selection = input.keyboard.get(extend_selection as usize);
let lasso_select = input.keyboard.get(lasso_select as usize);
let direct_insert_without_sliding = input.keyboard.get(direct_insert_without_sliding as usize);
let handle_drag_from_anchor = input.keyboard.get(handle_drag_from_anchor as usize);
tool_data.selection_mode = None;
tool_data.lasso_polygon.clear();
tool_data.mouse_down(
shape_editor,
document,
input,
responses,
extend_selection,
direct_insert_without_sliding,
lasso_select,
handle_drag_from_anchor,
)
tool_data.mouse_down(shape_editor, document, input, responses, extend_selection, lasso_select, handle_drag_from_anchor)
}
(
PathToolFsmState::Drawing { selection_shape },
@ -1189,6 +1116,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
},
) => {
tool_data.previous_mouse_position = input.mouse.position;
@ -1207,6 +1135,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
PathToolMessage::PointerMove {
@ -1215,6 +1144,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
];
@ -1230,6 +1160,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
},
) => {
let mut selected_only_handles = true;
@ -1299,6 +1230,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
PathToolMessage::PointerMove {
@ -1307,6 +1239,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
];
@ -1314,6 +1247,33 @@ impl Fsm for PathToolFsmState {
PathToolFsmState::Dragging(tool_data.dragging_state)
}
(PathToolFsmState::Ready, PathToolMessage::PointerMove { delete_segment, .. }) => {
tool_data.delete_segment_pressed = input.keyboard.get(delete_segment as usize);
// If there is a point nearby, then remove the overlay
if shape_editor
.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD)
.is_some()
{
tool_data.segment = None;
responses.add(OverlaysMessage::Draw)
}
// If already hovering on a segment, then recalculate its closest point
else if let Some(closest_segment) = &mut tool_data.segment {
closest_segment.update_closest_point(document.metadata(), input.mouse.position);
if closest_segment.too_far(input.mouse.position, SEGMENT_INSERTION_DISTANCE, document.metadata()) {
tool_data.segment = None;
}
responses.add(OverlaysMessage::Draw)
}
// If not, check that if there is some closest segment or not
else if let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, input.mouse.position, SEGMENT_INSERTION_DISTANCE) {
tool_data.segment = Some(closest_segment);
responses.add(OverlaysMessage::Draw)
}
self
}
(PathToolFsmState::Drawing { selection_shape: selection_type }, PathToolMessage::PointerOutsideViewport { .. }) => {
// Auto-panning
if let Some(offset) = tool_data.auto_panning.shift_viewport(input, responses) {
@ -1338,6 +1298,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
},
) => {
// Auto-panning
@ -1348,6 +1309,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
PathToolMessage::PointerMove {
@ -1356,6 +1318,7 @@ impl Fsm for PathToolFsmState {
move_anchor_with_handles,
snap_angle,
lock_angle,
delete_segment,
}
.into(),
];
@ -1546,7 +1509,6 @@ impl Fsm for PathToolFsmState {
responses.add(OverlaysMessage::Draw);
PathToolFsmState::Ready
}
(_, PathToolMessage::PointerMove { .. }) => self,
(_, PathToolMessage::NudgeSelectedPoints { delta_x, delta_y }) => {
shape_editor.move_selected_points(
tool_data.opposing_handle_lengths.take(),
@ -1616,6 +1578,7 @@ impl Fsm for PathToolFsmState {
HintGroup(vec![HintInfo::mouse(MouseMotion::Lmb, "Select Point"), HintInfo::keys([Key::Shift], "Extend").prepend_plus()]),
HintGroup(vec![HintInfo::mouse(MouseMotion::LmbDrag, "Select Area"), HintInfo::keys([Key::Control], "Lasso").prepend_plus()]),
HintGroup(vec![HintInfo::mouse(MouseMotion::Lmb, "Insert Point on Segment")]),
HintGroup(vec![HintInfo::keys_and_mouse([Key::Alt], MouseMotion::Lmb, "Delete Segment")]),
// TODO: Only show if at least one anchor is selected, and dynamically show either "Smooth" or "Sharp" based on the current state
HintGroup(vec![
HintInfo::mouse(MouseMotion::LmbDouble, "Convert Anchor Point"),
@ -1698,10 +1661,6 @@ impl Fsm for PathToolFsmState {
HintInfo::keys([Key::Alt], "Subtract").prepend_plus(),
]),
]),
PathToolFsmState::InsertPoint => HintData(vec![
HintGroup(vec![HintInfo::mouse(MouseMotion::Rmb, ""), HintInfo::keys([Key::Escape], "Cancel").prepend_slash()]),
HintGroup(vec![HintInfo::mouse(MouseMotion::Lmb, "Insert Point")]),
]),
};
responses.add(FrontendMessage::UpdateInputHints { hint_data });