Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,52 @@ pub fn snap_drag(start: DVec2, current: DVec2, snap_to_axis: bool, axis: Axis, s
document.metadata().document_to_viewport.transform_vector2(offset)
}

/// Snaps a dragging event using document-space drag state so PTZ changes do not invalidate the drag anchor.
pub fn snap_drag_from_document(start: DVec2, current: DVec2, snap_to_axis: bool, axis: Axis, snap_data: SnapData, snap_manager: &mut SnapManager, candidates: &[SnapCandidatePoint]) -> DVec2 {
let document = snap_data.document;
let document_to_viewport = document.metadata().document_to_viewport;
let start_viewport = document_to_viewport.transform_point2(start);
let mouse_position = axis_align_drag(snap_to_axis, axis, snap_data.input.mouse.position, start_viewport);
let aligned_document = document_to_viewport.inverse().transform_point2(mouse_position);
let total_mouse_delta_document = aligned_document - start;
let mut offset = aligned_document - current;
let mut best_snap = SnappedPoint::infinite_snap(aligned_document);

let bbox = Rect::point_iter(candidates.iter().map(|candidate| candidate.document_point + total_mouse_delta_document));

for (index, point) in candidates.iter().enumerate() {
let config = SnapTypeConfiguration {
bbox,
accept_distribution: true,
use_existing_candidates: index != 0,
..Default::default()
};

let mut point = point.clone();
point.document_point += total_mouse_delta_document;

let constrained_along_axis = snap_to_axis || axis.is_constraint();
let snapped = if constrained_along_axis {
let constraint = SnapConstraint::Line {
origin: point.document_point,
direction: total_mouse_delta_document.try_normalize().unwrap_or(DVec2::X),
};
snap_manager.constrained_snap(&snap_data, &point, constraint, config)
} else {
snap_manager.free_snap(&snap_data, &point, config)
};

if best_snap.other_snap_better(&snapped) {
offset = snapped.snapped_point_document - point.document_point + (aligned_document - current);
best_snap = snapped;
}
}

snap_manager.update_indicator(best_snap);

offset
}

/// Contains info on the overlays for the bounding box and transform handles
#[derive(Clone, Debug, Default)]
pub struct BoundingBoxManager {
Expand All @@ -379,10 +425,12 @@ pub struct BoundingBoxManager {
pub transform_tampered: bool,
/// The transform to viewport space for the bounds co-ordinates when the transformation was started.
pub original_bound_transform: DAffine2,
pub original_bounds_to_document: DAffine2,
pub selected_edges: Option<SelectedEdges>,
pub original_transforms: OriginalTransforms,
pub opposite_pivot: DVec2,
pub center_of_transformation: DVec2,
pub center_of_transformation_doc: DVec2,
}

impl BoundingBoxManager {
Expand Down
2 changes: 1 addition & 1 deletion editor/src/messages/tool/tool_messages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod tool_prelude {
pub use crate::messages::input_mapper::utility_types::input_keyboard::{Key, MouseMotion};
pub use crate::messages::layout::utility_types::widget_prelude::*;
pub use crate::messages::prelude::*;
pub use crate::messages::tool::utility_types::{EventToMessageMap, Fsm, ToolActionMessageContext, ToolMetadata, ToolTransition, ToolType};
pub use crate::messages::tool::utility_types::{DragState, EventToMessageMap, Fsm, ToolActionMessageContext, ToolMetadata, ToolTransition, ToolType};
pub use crate::messages::tool::utility_types::{HintData, HintGroup, HintInfo};
pub use glam::{DAffine2, DVec2};
}
104 changes: 75 additions & 29 deletions editor/src/messages/tool/tool_messages/path_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct PathToolOptions {
pub enum PathToolMessage {
// Standard messages
Abort,
CanvasTransformed,
SelectionChanged,
Overlays {
context: OverlayContext,
Expand Down Expand Up @@ -511,6 +512,7 @@ impl ToolTransition for PathTool {
fn event_to_message_map(&self) -> EventToMessageMap {
EventToMessageMap {
tool_abort: Some(PathToolMessage::Abort.into()),
canvas_transformed: Some(PathToolMessage::CanvasTransformed.into()),
selection_changed: Some(PathToolMessage::SelectionChanged.into()),
overlay_provider: Some(|context| PathToolMessage::Overlays { context }.into()),
..Default::default()
Expand Down Expand Up @@ -561,7 +563,7 @@ struct PathToolData {
snap_manager: SnapManager,
lasso_polygon: Vec<DVec2>,
selection_mode: Option<SelectionMode>,
drag_start_pos: DVec2,
drag_start_doc: DVec2,
previous_mouse_position: DVec2,
toggle_colinear_debounce: bool,
opposing_handle_lengths: Option<OpposingHandleLengths>,
Expand Down Expand Up @@ -645,14 +647,19 @@ impl PathToolData {
// Convert previous mouse position to viewport space first
let document_to_viewport = metadata.document_to_viewport;
let previous_mouse = document_to_viewport.transform_point2(self.previous_mouse_position);
if previous_mouse == self.drag_start_pos {
let drag_start_vp = document_to_viewport.transform_point2(self.drag_start_doc);
if previous_mouse == drag_start_vp {
let tolerance = DVec2::splat(SELECTION_TOLERANCE);
[self.drag_start_pos - tolerance, self.drag_start_pos + tolerance]
[drag_start_vp - tolerance, drag_start_vp + tolerance]
} else {
[self.drag_start_pos, previous_mouse]
[drag_start_vp, previous_mouse]
}
}

fn drag_start_viewport(&self, document: &DocumentMessageHandler) -> DVec2 {
document.metadata().document_to_viewport.transform_point2(self.drag_start_doc)
}

fn update_selection_status(&mut self, shape_editor: &mut ShapeState, document: &DocumentMessageHandler) {
let selection_status = get_selection_status(&document.network_interface, shape_editor);

Expand Down Expand Up @@ -736,7 +743,7 @@ impl PathToolData {
self.double_click_handled = false;
self.opposing_handle_lengths = None;

self.drag_start_pos = input.mouse.position;
self.drag_start_doc = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);

if input.time - self.last_click_time > DOUBLE_CLICK_MILLISECONDS {
self.saved_points_before_anchor_convert_smooth_sharp.clear();
Expand Down Expand Up @@ -784,7 +791,7 @@ impl PathToolData {
}

if let Some(selected_points) = selection_info {
self.drag_start_pos = input.mouse.position;
self.drag_start_doc = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);

// If selected points contain only handles and there was some selection before, then it is stored and becomes restored upon release
let mut dragging_only_handles = true;
Expand Down Expand Up @@ -871,7 +878,7 @@ impl PathToolData {
// TODO: If the segment connected to one of the endpoints is also selected then select that point
}

self.drag_start_pos = input.mouse.position;
self.drag_start_doc = document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position);
let viewport_to_document = document.metadata().document_to_viewport.inverse();
self.previous_mouse_position = viewport_to_document.transform_point2(input.mouse.position);

Expand Down Expand Up @@ -900,15 +907,15 @@ impl PathToolData {

self.started_drawing_from_inside = true;

self.drag_start_pos = input.mouse.position;
self.drag_start_doc = document.metadata().document_to_viewport.inverse().transform_point2(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 }
}
// Start drawing
else {
self.drag_start_pos = input.mouse.position;
self.drag_start_doc = document.metadata().document_to_viewport.inverse().transform_point2(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 };
Expand Down Expand Up @@ -1153,7 +1160,7 @@ impl PathToolData {
fn start_snap_along_axis(&mut self, shape_editor: &mut ShapeState, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, responses: &mut VecDeque<Message>) {
// Find the negative delta to take the point to the drag start position
let current_mouse = input.mouse.position;
let drag_start = self.drag_start_pos;
let drag_start = self.drag_start_viewport(document);
let opposite_delta = drag_start - current_mouse;

shape_editor.move_selected_points_and_segments(None, document, opposite_delta, false, true, false, None, false, responses);
Expand All @@ -1174,7 +1181,7 @@ impl PathToolData {
fn stop_snap_along_axis(&mut self, shape_editor: &mut ShapeState, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, responses: &mut VecDeque<Message>) {
// Calculate the negative delta of the selection and move it back to the drag start
let current_mouse = input.mouse.position;
let drag_start = self.drag_start_pos;
let drag_start = self.drag_start_viewport(document);

let opposite_delta = drag_start - current_mouse;
let Some(axis) = self.snapping_axis else { return };
Expand Down Expand Up @@ -1463,7 +1470,7 @@ impl PathToolData {
let mut was_alt_dragging = false;

if self.snapping_axis.is_none() {
if self.alt_clicked_on_anchor && !self.alt_dragging_from_anchor && self.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD {
if self.alt_clicked_on_anchor && !self.alt_dragging_from_anchor && self.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD {
// Checking which direction the dragging begins
self.alt_dragging_from_anchor = true;
let Some(layer) = document.network_interface.selected_nodes().selected_layers(document.metadata()).next() else {
Expand All @@ -1485,7 +1492,7 @@ impl PathToolData {
return;
};

let delta = input.mouse.position - self.drag_start_pos;
let delta = input.mouse.position - self.drag_start_viewport(document);
let handle = if delta.dot(tangent1) >= delta.dot(tangent2) {
segment1.to_manipulator_point()
} else {
Expand Down Expand Up @@ -1528,7 +1535,7 @@ impl PathToolData {
// Constantly checking and changing the snapping axis based on current mouse position
if snap_axis && self.snapping_axis.is_some() {
let Some(current_axis) = self.snapping_axis else { return };
let total_delta = self.drag_start_pos - input.mouse.position;
let total_delta = self.drag_start_viewport(document) - input.mouse.position;

if (total_delta.x.abs() > total_delta.y.abs() && current_axis == Axis::Y) || (total_delta.y.abs() > total_delta.x.abs() && current_axis == Axis::X) {
self.stop_snap_along_axis(shape_editor, document, input, responses);
Expand Down Expand Up @@ -1706,7 +1713,7 @@ impl Fsm for PathToolFsmState {
}
(_, PathToolMessage::Overlays { context: mut overlay_context }) => {
// Set this to show ghost line only if drag actually happened
if matches!(self, Self::Dragging(_)) && tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD {
if matches!(self, Self::Dragging(_)) && tool_data.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD {
for (outline, layer) in &tool_data.ghost_outline {
let transform = document.metadata().transform_to_viewport(*layer);
overlay_context.outline(outline.iter(), transform, Some(COLOR_OVERLAY_GRAY));
Expand Down Expand Up @@ -1904,7 +1911,8 @@ impl Fsm for PathToolFsmState {
let (points_inside, segments_inside) = match selection_shape {
SelectionShapeType::Box => {
let previous_mouse = document.metadata().document_to_viewport.transform_point2(tool_data.previous_mouse_position);
let bbox = Rect::new(tool_data.drag_start_pos.x, tool_data.drag_start_pos.y, previous_mouse.x, previous_mouse.y).abs();
let drag_start_vp = tool_data.drag_start_viewport(document);
let bbox = Rect::new(drag_start_vp.x, drag_start_vp.y, previous_mouse.x, previous_mouse.y).abs();
shape_editor.get_inside_points_and_segments(
&document.network_interface,
SelectionShape::Box(bbox),
Expand Down Expand Up @@ -1971,7 +1979,7 @@ impl Fsm for PathToolFsmState {
// Draw the snapping axis lines
if tool_data.snapping_axis.is_some() {
let Some(axis) = tool_data.snapping_axis else { return self };
let origin = tool_data.drag_start_pos;
let origin = tool_data.drag_start_viewport(document);
let viewport_diagonal = viewport.size().into_dvec2().length();

match axis {
Expand Down Expand Up @@ -2099,11 +2107,11 @@ impl Fsm for PathToolFsmState {
let selected_only_handles = !shape_editor.selected_points().any(|point| matches!(point, ManipulatorPointId::Anchor(_)));
tool_data.stored_selection = None;

if !tool_data.saved_selection_before_handle_drag.is_empty() && (tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD) && (selected_only_handles) {
if !tool_data.saved_selection_before_handle_drag.is_empty() && (tool_data.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD) && (selected_only_handles) {
tool_data.handle_drag_toggle = true;
}

if tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD {
if tool_data.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD {
tool_data.molding_segment = true;
}

Expand Down Expand Up @@ -2245,15 +2253,15 @@ impl Fsm for PathToolFsmState {
(PathToolFsmState::Drawing { selection_shape: selection_type }, PathToolMessage::PointerOutsideViewport { .. }) => {
// Auto-panning
if let Some(offset) = tool_data.auto_panning.shift_viewport(input, viewport, responses) {
tool_data.drag_start_pos += offset;
tool_data.drag_start_doc += document.metadata().document_to_viewport.inverse().transform_vector2(offset);
}
Comment on lines 2255 to 2257
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Updating drag_start_doc during auto-panning is incorrect. Since drag_start_doc is in document space, it should remain invariant to viewport transformations. Updating it here effectively cancels out the panning movement for the drag delta, which defeats the purpose of auto-panning (where the object should move with the mouse relative to the document).

				let _ = tool_data.auto_panning.shift_viewport(input, viewport, responses);


PathToolFsmState::Drawing { selection_shape: selection_type }
}
(PathToolFsmState::Dragging(dragging_state), PathToolMessage::PointerOutsideViewport { .. }) => {
// Auto-panning
if let Some(offset) = tool_data.auto_panning.shift_viewport(input, viewport, responses) {
tool_data.drag_start_pos += offset;
tool_data.drag_start_doc += document.metadata().document_to_viewport.inverse().transform_vector2(offset);
}
Comment on lines 2263 to 2265
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the drawing state, drag_start_doc should not be updated during auto-panning while dragging. Document-space coordinates are independent of viewport shifts.

				let _ = tool_data.auto_panning.shift_viewport(input, viewport, responses);


PathToolFsmState::Dragging(dragging_state)
Expand Down Expand Up @@ -2314,7 +2322,7 @@ impl Fsm for PathToolFsmState {

let document_to_viewport = document.metadata().document_to_viewport;
let previous_mouse = document_to_viewport.transform_point2(tool_data.previous_mouse_position);
if tool_data.drag_start_pos == previous_mouse {
if tool_data.drag_start_viewport(document) == previous_mouse {
responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![] });
} else {
let selection_mode = match tool_action_data.preferences.get_selection_mode() {
Expand All @@ -2324,7 +2332,8 @@ impl Fsm for PathToolFsmState {

match selection_shape {
SelectionShapeType::Box => {
let bbox = Rect::new(tool_data.drag_start_pos.x, tool_data.drag_start_pos.y, previous_mouse.x, previous_mouse.y).abs();
let drag_start_vp = tool_data.drag_start_viewport(document);
let bbox = Rect::new(drag_start_vp.x, drag_start_vp.y, previous_mouse.x, previous_mouse.y).abs();

shape_editor.select_all_in_shape(
&document.network_interface,
Expand Down Expand Up @@ -2355,7 +2364,7 @@ impl Fsm for PathToolFsmState {
PathToolFsmState::Ready
}
(PathToolFsmState::Dragging { .. }, PathToolMessage::Escape | PathToolMessage::RightClick) => {
if tool_data.handle_drag_toggle && tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD {
if tool_data.handle_drag_toggle && tool_data.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD {
shape_editor.deselect_all_points();
shape_editor.deselect_all_segments();

Expand Down Expand Up @@ -2410,7 +2419,7 @@ impl Fsm for PathToolFsmState {
};
tool_data.started_drawing_from_inside = false;

if tool_data.drag_start_pos.distance(previous_mouse) < 1e-8 {
if tool_data.drag_start_viewport(document).distance(previous_mouse) < 1e-8 {
// Clicked inside or outside the shape then deselect all of the points/segments
if document.click(input, viewport).is_some() && tool_data.stored_selection.is_none() {
tool_data.stored_selection = Some(shape_editor.selected_shape_state.clone());
Expand All @@ -2421,7 +2430,8 @@ impl Fsm for PathToolFsmState {
} else {
match selection_shape {
SelectionShapeType::Box => {
let bbox = Rect::new(tool_data.drag_start_pos.x, tool_data.drag_start_pos.y, previous_mouse.x, previous_mouse.y).abs();
let drag_start_vp = tool_data.drag_start_viewport(document);
let bbox = Rect::new(drag_start_vp.x, drag_start_vp.y, previous_mouse.x, previous_mouse.y).abs();

shape_editor.select_all_in_shape(
&document.network_interface,
Expand Down Expand Up @@ -2454,7 +2464,7 @@ impl Fsm for PathToolFsmState {
(_, PathToolMessage::DragStop { extend_selection, .. }) => {
tool_data.ghost_outline.clear();
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 drag_occurred = tool_data.drag_start_viewport(document).distance(input.mouse.position) > DRAG_THRESHOLD;
let mut segment_dissolved = false;
let mut point_inserted = false;

Expand Down Expand Up @@ -2585,7 +2595,7 @@ impl Fsm for PathToolFsmState {
}
}
// Deselect all points if the user clicks the filled region of the shape
else if tool_data.drag_start_pos.distance(input.mouse.position) <= DRAG_THRESHOLD {
else if tool_data.drag_start_viewport(document).distance(input.mouse.position) <= DRAG_THRESHOLD {
shape_editor.deselect_all_points();
shape_editor.deselect_all_segments();
}
Expand Down Expand Up @@ -3037,7 +3047,7 @@ impl Fsm for PathToolFsmState {

if nearest_point.is_some() {
// Flip the selected point between smooth and sharp
if !tool_data.double_click_handled && tool_data.drag_start_pos.distance(input.mouse.position) <= DRAG_THRESHOLD {
if !tool_data.double_click_handled && tool_data.drag_start_viewport(document).distance(input.mouse.position) <= DRAG_THRESHOLD {
responses.add(DocumentMessage::StartTransaction);

shape_editor.select_points_by_layer_and_id(&tool_data.saved_points_before_anchor_convert_smooth_sharp);
Expand Down Expand Up @@ -3119,6 +3129,42 @@ impl Fsm for PathToolFsmState {

PathToolFsmState::Ready
}
// PTZ handling: re-emit PointerMove to recompute positions with updated document_to_viewport
(PathToolFsmState::Dragging(dragging_state), PathToolMessage::CanvasTransformed) => {
let modifier_keys = PathToolMessage::PointerMove {
equidistant: Key::Alt,
toggle_colinear: Key::KeyC,
move_anchor_with_handles: Key::Space,
snap_angle: Key::Control,
lock_angle: Key::Shift,
delete_segment: Key::Backspace,
break_colinear_molding: Key::Tab,
segment_editing_modifier: Key::Alt,
};
Comment on lines +3134 to +3143
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding modifier keys like Key::Alt and Key::Control here bypasses the input mapper's configuration. If a user has remapped these actions, the tool will check the wrong physical keys during viewport transformations. It would be better to store the keys received in the last PointerMove message and reuse them here.

responses.add(modifier_keys);
responses.add(OverlaysMessage::Draw);
PathToolFsmState::Dragging(dragging_state)
}
(PathToolFsmState::Drawing { selection_shape }, PathToolMessage::CanvasTransformed) => {
let modifier_keys = PathToolMessage::PointerMove {
equidistant: Key::Alt,
toggle_colinear: Key::KeyC,
move_anchor_with_handles: Key::Space,
snap_angle: Key::Control,
lock_angle: Key::Shift,
delete_segment: Key::Backspace,
break_colinear_molding: Key::Tab,
segment_editing_modifier: Key::Alt,
};
responses.add(modifier_keys);
responses.add(OverlaysMessage::Draw);
PathToolFsmState::Drawing { selection_shape }
}
(PathToolFsmState::SlidingPoint, PathToolMessage::CanvasTransformed) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: During SlidingPoint, CanvasTransformed only redraws overlays and does not recompute the sliding anchor position. If the viewport is panned/zoomed without a pointer move, slide_point isn’t called, so the anchor can desync until the next mouse move. Dragging/Drawing explicitly re-emit PointerMove on CanvasTransformed to avoid this drift.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/tool/tool_messages/path_tool.rs, line 3163:

<comment>During SlidingPoint, CanvasTransformed only redraws overlays and does not recompute the sliding anchor position. If the viewport is panned/zoomed without a pointer move, slide_point isn’t called, so the anchor can desync until the next mouse move. Dragging/Drawing explicitly re-emit PointerMove on CanvasTransformed to avoid this drift.</comment>

<file context>
@@ -3119,6 +3129,42 @@ impl Fsm for PathToolFsmState {
+				responses.add(OverlaysMessage::Draw);
+				PathToolFsmState::Drawing { selection_shape }
+			}
+			(PathToolFsmState::SlidingPoint, PathToolMessage::CanvasTransformed) => {
+				responses.add(OverlaysMessage::Draw);
+				PathToolFsmState::SlidingPoint
</file context>

responses.add(OverlaysMessage::Draw);
PathToolFsmState::SlidingPoint
}
(_, PathToolMessage::CanvasTransformed) => self,
(_, PathToolMessage::Abort) => {
responses.add(OverlaysMessage::Draw);
PathToolFsmState::Ready
Expand Down
Loading
Loading