From c2730bd3a6b0d76e29919dfcde7871ad0a96295f Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 21 Dec 2025 15:30:04 -0800 Subject: [PATCH 1/3] Promote vector meshes from experimental by removing it from preferences --- .../preferences_dialog_message_handler.rs | 25 +---- .../document/overlays/utility_functions.rs | 6 +- .../preferences/preferences_message.rs | 1 - .../preferences_message_handler.rs | 5 - .../tool/common_functionality/shape_editor.rs | 10 +- .../common_functionality/utility_functions.rs | 13 +-- .../tool/tool_messages/freehand_tool.rs | 5 +- .../messages/tool/tool_messages/path_tool.rs | 4 +- .../messages/tool/tool_messages/pen_tool.rs | 98 +++++++------------ .../messages/tool/tool_messages/shape_tool.rs | 2 - .../tool/tool_messages/spline_tool.rs | 13 ++- .../vector-types/src/vector/vector_types.rs | 18 +++- 12 files changed, 69 insertions(+), 131 deletions(-) diff --git a/editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs b/editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs index 5078d4beb2..c17f2c497c 100644 --- a/editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs +++ b/editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs @@ -256,29 +256,6 @@ impl PreferencesDialogMessageHandler { .widget_instance(), ]; - let checkbox_id = CheckboxId::new(); - let vector_mesh_description = " - Allow the Pen tool to produce branching geometry, where more than two segments may be connected to one anchor point.\n\ - \n\ - Currently, vector meshes do not properly render strokes (branching joins) and fills (multiple regions). - " - .trim(); - let vector_meshes = vec![ - Separator::new(SeparatorType::Unrelated).widget_instance(), - Separator::new(SeparatorType::Unrelated).widget_instance(), - CheckboxInput::new(preferences.vector_meshes) - .tooltip_label("Vector Meshes") - .tooltip_description(vector_mesh_description) - .on_update(|checkbox_input: &CheckboxInput| PreferencesMessage::VectorMeshes { enabled: checkbox_input.checked }.into()) - .for_label(checkbox_id) - .widget_instance(), - TextLabel::new("Vector Meshes") - .tooltip_label("Vector Meshes") - .tooltip_description(vector_mesh_description) - .for_checkbox(checkbox_id) - .widget_instance(), - ]; - let checkbox_id = CheckboxId::new(); let brush_tool_description = " Enable the Brush tool to support basic raster-based layer painting.\n\ @@ -304,7 +281,7 @@ impl PreferencesDialogMessageHandler { .widget_instance(), ]; - rows.extend_from_slice(&[header, node_graph_wires_label, graph_wire_style, use_vello, vector_meshes, brush_tool]); + rows.extend_from_slice(&[header, node_graph_wires_label, graph_wire_style, use_vello, brush_tool]); } Layout(rows.into_iter().map(|r| LayoutGroup::Row { widgets: r }).collect()) diff --git a/editor/src/messages/portfolio/document/overlays/utility_functions.rs b/editor/src/messages/portfolio/document/overlays/utility_functions.rs index eb2877b5d7..da4586bafc 100644 --- a/editor/src/messages/portfolio/document/overlays/utility_functions.rs +++ b/editor/src/messages/portfolio/document/overlays/utility_functions.rs @@ -3,7 +3,7 @@ use crate::consts::HIDE_HANDLE_DISTANCE; use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier; use crate::messages::portfolio::document::utility_types::network_interface::NodeNetworkInterface; use crate::messages::tool::common_functionality::shape_editor::{SelectedLayerState, ShapeState}; -use crate::messages::tool::tool_messages::tool_prelude::{DocumentMessageHandler, PreferencesMessageHandler}; +use crate::messages::tool::tool_messages::tool_prelude::DocumentMessageHandler; use glam::{DAffine2, DVec2}; use graphene_std::subpath::{Bezier, BezierHandles}; use graphene_std::text::{Font, FontCache, TextAlign, TextContext, TypesettingConfig}; @@ -200,7 +200,7 @@ pub fn path_overlays(document: &DocumentMessageHandler, draw_handles: DrawHandle } } -pub fn path_endpoint_overlays(document: &DocumentMessageHandler, shape_editor: &mut ShapeState, overlay_context: &mut OverlayContext, preferences: &PreferencesMessageHandler) { +pub fn path_endpoint_overlays(document: &DocumentMessageHandler, shape_editor: &mut ShapeState, overlay_context: &mut OverlayContext) { if !overlay_context.visibility_settings.anchors() { return; } @@ -213,7 +213,7 @@ pub fn path_endpoint_overlays(document: &DocumentMessageHandler, shape_editor: & let selected = shape_editor.selected_shape_state.get(&layer); let is_selected = |selected: Option<&SelectedLayerState>, point: ManipulatorPointId| selected.is_some_and(|selected| selected.is_point_selected(point)); - for point in vector.extendable_points(preferences.vector_meshes) { + for point in vector.extendable_points() { let Some(position) = vector.point_domain.position_from_id(point) else { continue }; let position = transform.transform_point2(position); overlay_context.manipulator_anchor(position, is_selected(selected, ManipulatorPointId::Anchor(point)), None); diff --git a/editor/src/messages/preferences/preferences_message.rs b/editor/src/messages/preferences/preferences_message.rs index 027bdd406a..6408cd5d2f 100644 --- a/editor/src/messages/preferences/preferences_message.rs +++ b/editor/src/messages/preferences/preferences_message.rs @@ -12,7 +12,6 @@ pub enum PreferencesMessage { // Per-preference messages UseVello { use_vello: bool }, SelectionMode { selection_mode: SelectionMode }, - VectorMeshes { enabled: bool }, BrushTool { enabled: bool }, ModifyLayout { zoom_with_scroll: bool }, GraphWireStyle { style: GraphWireStyle }, diff --git a/editor/src/messages/preferences/preferences_message_handler.rs b/editor/src/messages/preferences/preferences_message_handler.rs index a85e384b72..6321119c5a 100644 --- a/editor/src/messages/preferences/preferences_message_handler.rs +++ b/editor/src/messages/preferences/preferences_message_handler.rs @@ -17,7 +17,6 @@ pub struct PreferencesMessageHandler { pub selection_mode: SelectionMode, pub zoom_with_scroll: bool, pub use_vello: bool, - pub vector_meshes: bool, pub brush_tool: bool, pub graph_wire_style: GraphWireStyle, pub viewport_zoom_wheel_rate: f64, @@ -46,7 +45,6 @@ impl Default for PreferencesMessageHandler { selection_mode: SelectionMode::Touched, zoom_with_scroll: matches!(MappingVariant::default(), MappingVariant::ZoomWithScroll), use_vello: EditorPreferences::default().use_vello, - vector_meshes: false, brush_tool: false, graph_wire_style: GraphWireStyle::default(), viewport_zoom_wheel_rate: VIEWPORT_ZOOM_WHEEL_RATE, @@ -87,9 +85,6 @@ impl MessageHandler> for Prefe responses.add(PortfolioMessage::UpdateVelloPreference); responses.add(PortfolioMessage::EditorPreferences); } - PreferencesMessage::VectorMeshes { enabled } => { - self.vector_meshes = enabled; - } PreferencesMessage::BrushTool { enabled } => { self.brush_tool = enabled; diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index 5f00d97111..6723db92e9 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/editor/src/messages/tool/common_functionality/shape_editor.rs @@ -421,7 +421,7 @@ impl ShapeState { (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, vector_meshes: bool) { + 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 .selected_shape_state @@ -446,14 +446,6 @@ impl ShapeState { let (layer1, start_point) = all_selected_points[0]; let (layer2, end_point) = all_selected_points[1]; - let Some(vector1) = document.network_interface.compute_modified_vector(layer1) else { return }; - let Some(vector2) = document.network_interface.compute_modified_vector(layer2) else { return }; - - // If vector meshes is not selected then only for endpoints, otherwise normally applicable - if !vector_meshes && (vector1.all_connected(start_point).count() != 1 || vector2.all_connected(end_point).count() != 1) { - return; - } - if layer1 == layer2 { if start_point == end_point { return; diff --git a/editor/src/messages/tool/common_functionality/utility_functions.rs b/editor/src/messages/tool/common_functionality/utility_functions.rs index 3c457aa605..baf6c067c2 100644 --- a/editor/src/messages/tool/common_functionality/utility_functions.rs +++ b/editor/src/messages/tool/common_functionality/utility_functions.rs @@ -22,14 +22,8 @@ use graphene_std::vector::{HandleExt, PointId, SegmentId, Vector, VectorModifica use kurbo::{CubicBez, DEFAULT_ACCURACY, Line, ParamCurve, PathSeg, Point, QuadBez, Shape}; /// Determines if a path should be extended. Goal in viewport space. Returns the path and if it is extending from the start, if applicable. -pub fn should_extend( - document: &DocumentMessageHandler, - goal: DVec2, - tolerance: f64, - layers: impl Iterator, - preferences: &PreferencesMessageHandler, -) -> Option<(LayerNodeIdentifier, PointId, DVec2)> { - closest_point(document, goal, tolerance, layers, |_| false, preferences) +pub fn should_extend(document: &DocumentMessageHandler, goal: DVec2, tolerance: f64, layers: impl Iterator) -> Option<(LayerNodeIdentifier, PointId, DVec2)> { + closest_point(document, goal, tolerance, layers, |_| false) } /// Determine the closest point to the goal point under max_distance. @@ -40,7 +34,6 @@ pub fn closest_point( max_distance: f64, layers: impl Iterator, exclude: T, - preferences: &PreferencesMessageHandler, ) -> Option<(LayerNodeIdentifier, PointId, DVec2)> where T: Fn(PointId) -> bool, @@ -50,7 +43,7 @@ where for layer in layers { let viewspace = document.metadata().transform_to_viewport(layer); let Some(vector) = document.network_interface.compute_modified_vector(layer) else { continue }; - for id in vector.extendable_points(preferences.vector_meshes) { + for id in vector.extendable_points() { if exclude(id) { continue; } diff --git a/editor/src/messages/tool/tool_messages/freehand_tool.rs b/editor/src/messages/tool/tool_messages/freehand_tool.rs index a74635f391..f41b5a5fc9 100644 --- a/editor/src/messages/tool/tool_messages/freehand_tool.rs +++ b/editor/src/messages/tool/tool_messages/freehand_tool.rs @@ -236,7 +236,6 @@ impl Fsm for FreehandToolFsmState { global_tool_data, input, shape_editor, - preferences, viewport, .. } = tool_action_data; @@ -244,7 +243,7 @@ impl Fsm for FreehandToolFsmState { let ToolMessage::Freehand(event) = event else { return self }; match (self, event) { (_, FreehandToolMessage::Overlays { context: mut overlay_context }) => { - path_endpoint_overlays(document, shape_editor, &mut overlay_context, tool_action_data.preferences); + path_endpoint_overlays(document, shape_editor, &mut overlay_context); self } @@ -258,7 +257,7 @@ impl Fsm for FreehandToolFsmState { // Extend an endpoint of the selected path let selected_nodes = document.network_interface.selected_nodes(); let tolerance = crate::consts::SNAP_POINT_TOLERANCE; - if let Some((layer, point, position)) = should_extend(document, input.mouse.position, tolerance, selected_nodes.selected_layers(document.metadata()), preferences) { + if let Some((layer, point, position)) = should_extend(document, input.mouse.position, tolerance, selected_nodes.selected_layers(document.metadata())) { tool_data.layer = Some(layer); tool_data.end_point = Some((position, point)); diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index dd0d2865d1..09a4bd4b63 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -823,7 +823,7 @@ impl PathToolData { .filter(|handle| handle.length(&vector) < 1e-6) .map(|handle| handle.to_manipulator_point()) .collect::>(); - let endpoint = vector.extendable_points(false).any(|anchor| point == anchor); + let endpoint = vector.extendable_points_no_vector_meshes().any(|anchor| point == anchor); if drag_zero_handle && (handles.len() == 1 && !endpoint) { shape_editor.deselect_all_points(); @@ -2662,7 +2662,7 @@ impl Fsm for PathToolFsmState { } (_, PathToolMessage::ClosePath) => { responses.add(DocumentMessage::AddTransaction); - shape_editor.close_selected_path(document, responses, tool_action_data.preferences.vector_meshes); + shape_editor.close_selected_path(document, responses); responses.add(DocumentMessage::EndTransaction); responses.add(OverlaysMessage::Draw); diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs index 8cb1bfeecb..d97e5ec510 100644 --- a/editor/src/messages/tool/tool_messages/pen_tool.rs +++ b/editor/src/messages/tool/tool_messages/pen_tool.rs @@ -589,15 +589,7 @@ impl PenToolData { } /// If the user places the anchor on top of the previous anchor, it becomes sharp and the outgoing handle may be dragged. - fn bend_from_previous_point( - &mut self, - snap_data: SnapData, - transform: DAffine2, - layer: LayerNodeIdentifier, - preferences: &PreferencesMessageHandler, - shape_editor: &mut ShapeState, - responses: &mut VecDeque, - ) { + fn bend_from_previous_point(&mut self, snap_data: SnapData, transform: DAffine2, layer: LayerNodeIdentifier, shape_editor: &mut ShapeState, responses: &mut VecDeque) { self.g1_continuous = true; let document = snap_data.document; self.next_handle_start = self.next_point; @@ -614,7 +606,7 @@ impl PenToolData { self.handle_end = None; self.handle_mode = HandleMode::Free; - self.store_clicked_endpoint(document, &transform, snap_data.input, snap_data.viewport, preferences); + self.store_clicked_endpoint(document, &transform, snap_data.input, snap_data.viewport); if self.modifiers.lock_angle { self.set_lock_angle(&vector, id, self.prior_segment); @@ -631,8 +623,8 @@ impl PenToolData { } // Closing path - let closing_path_on_point = self.close_path_on_point(snap_data, &vector, document, preferences, id, &transform); - if !closing_path_on_point && preferences.vector_meshes { + let closing_path_on_point = self.close_path_on_point(snap_data, &vector, document, id, &transform); + if !closing_path_on_point { // Attempt to find nearest segment and close path on segment by creating an anchor point on it let tolerance = crate::consts::SNAP_POINT_TOLERANCE; if let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, transform.transform_point2(self.next_point), tolerance) { @@ -659,8 +651,8 @@ impl PenToolData { } } - fn close_path_on_point(&mut self, snap_data: SnapData, vector: &Vector, document: &DocumentMessageHandler, preferences: &PreferencesMessageHandler, id: PointId, transform: &DAffine2) -> bool { - for id in vector.extendable_points(preferences.vector_meshes).filter(|&point| point != id) { + fn close_path_on_point(&mut self, snap_data: SnapData, vector: &Vector, document: &DocumentMessageHandler, id: PointId, transform: &DAffine2) -> bool { + for id in vector.extendable_points().filter(|&point| point != id) { let Some(pos) = vector.point_domain.position_from_id(id) else { continue }; let transformed_distance_between_squared = transform.transform_point2(pos).distance_squared(transform.transform_point2(self.next_point)); let snap_point_tolerance_squared = crate::consts::SNAP_POINT_TOLERANCE.powi(2); @@ -670,7 +662,7 @@ impl PenToolData { self.handle_end_offset = None; self.path_closed = true; self.next_handle_start = self.next_point; - self.store_clicked_endpoint(document, transform, snap_data.input, snap_data.viewport, preferences); + self.store_clicked_endpoint(document, transform, snap_data.input, snap_data.viewport); self.handle_mode = HandleMode::Free; if let (true, Some(prior_endpoint)) = (self.modifiers.lock_angle, self.prior_segment_endpoint) { self.set_lock_angle(vector, prior_endpoint, self.prior_segment); @@ -682,7 +674,7 @@ impl PenToolData { false } - fn finish_placing_handle(&mut self, snap_data: SnapData, transform: DAffine2, preferences: &PreferencesMessageHandler, responses: &mut VecDeque) -> Option { + fn finish_placing_handle(&mut self, snap_data: SnapData, transform: DAffine2, responses: &mut VecDeque) -> Option { let document = snap_data.document; let next_handle_start = self.next_handle_start; let handle_start = self.latest_point()?.handle_start; @@ -693,12 +685,12 @@ impl PenToolData { let Some(handle_end) = self.handle_end else { responses.add(DocumentMessage::EndTransaction); self.handle_end = Some(next_handle_start); - self.place_anchor(snap_data, transform, mouse, preferences, responses); + self.place_anchor(snap_data, transform, mouse, responses); self.latest_point_mut()?.handle_start = next_handle_start; return None; }; let next_point = self.next_point; - self.place_anchor(snap_data, transform, mouse, preferences, responses); + self.place_anchor(snap_data, transform, mouse, responses); let handles = [handle_start - self.latest_point()?.pos, handle_end - next_point].map(Some); // Get close path @@ -709,7 +701,7 @@ impl PenToolData { let vector = document.network_interface.compute_modified_vector(layer)?; let start = self.latest_point()?.id; let transform = document.metadata().document_to_viewport * transform; - for id in vector.extendable_points(preferences.vector_meshes).filter(|&point| point != start) { + for id in vector.extendable_points().filter(|&point| point != start) { let Some(pos) = vector.point_domain.position_from_id(id) else { continue }; let transformed_distance_between_squared = transform.transform_point2(pos).distance_squared(transform.transform_point2(next_point)); let snap_point_tolerance_squared = crate::consts::SNAP_POINT_TOLERANCE.powi(2); @@ -1123,7 +1115,7 @@ impl PenToolData { self.update_target_handle_pos(opposite_handle, self.next_point, responses, new_handle_position, layer); } - fn place_anchor(&mut self, snap_data: SnapData, transform: DAffine2, mouse: DVec2, preferences: &PreferencesMessageHandler, responses: &mut VecDeque) -> Option { + fn place_anchor(&mut self, snap_data: SnapData, transform: DAffine2, mouse: DVec2, responses: &mut VecDeque) -> Option { let document = snap_data.document; let relative = if self.path_closed { None } else { self.latest_point().map(|point| point.pos) }; @@ -1134,7 +1126,7 @@ impl PenToolData { let layer = selected_layers.next().filter(|_| selected_layers.next().is_none()).or(self.current_layer)?; let vector = document.network_interface.compute_modified_vector(layer)?; let transform = document.metadata().document_to_viewport * transform; - for point in vector.extendable_points(preferences.vector_meshes) { + for point in vector.extendable_points() { let Some(pos) = vector.point_domain.position_from_id(point) else { continue }; let transformed_distance_between_squared = transform.transform_point2(pos).distance_squared(transform.transform_point2(self.next_point)); let snap_point_tolerance_squared = crate::consts::SNAP_POINT_TOLERANCE.powi(2); @@ -1230,7 +1222,6 @@ impl PenToolData { responses: &mut VecDeque, tool_options: &PenOptions, append: bool, - preferences: &PreferencesMessageHandler, shape_editor: &mut ShapeState, ) { let point = SnapCandidatePoint::handle(document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position)); @@ -1242,14 +1233,12 @@ impl PenToolData { self.handle_end = None; let tolerance = crate::consts::SNAP_POINT_TOLERANCE; - let extension_choice = should_extend(document, viewport_vec, tolerance, selected_nodes.selected_layers(document.metadata()), preferences); + let extension_choice = should_extend(document, viewport_vec, tolerance, selected_nodes.selected_layers(document.metadata())); if let Some((layer, point, position)) = extension_choice { self.current_layer = Some(layer); self.extend_existing_path(document, layer, point, position); return; - } else if preferences.vector_meshes - && let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, viewport_vec, tolerance) - { + } else if let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, viewport_vec, tolerance) { let (point, segments) = closest_segment.adjusted_insert(responses); let layer = closest_segment.layer(); let position = closest_segment.closest_point_document(); @@ -1264,7 +1253,7 @@ impl PenToolData { } if append { - if let Some((layer, point, _)) = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false, preferences) { + if let Some((layer, point, _)) = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false) { let vector = document.network_interface.compute_modified_vector(layer).unwrap(); let segment = vector.all_connected(point).collect::>().first().map(|s| s.segment); @@ -1282,7 +1271,7 @@ impl PenToolData { } } - if let Some((layer, point, _position)) = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false, preferences) { + if let Some((layer, point, _position)) = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false) { let vector = document.network_interface.compute_modified_vector(layer).unwrap(); let segment = vector.all_connected(point).collect::>().first().map(|s| s.segment); self.handle_mode = HandleMode::Free; @@ -1366,14 +1355,7 @@ impl PenToolData { } // Stores the segment and point ID of the clicked endpoint - fn store_clicked_endpoint( - &mut self, - document: &DocumentMessageHandler, - transform: &DAffine2, - input: &InputPreprocessorMessageHandler, - viewport: &ViewportMessageHandler, - preferences: &PreferencesMessageHandler, - ) { + fn store_clicked_endpoint(&mut self, document: &DocumentMessageHandler, transform: &DAffine2, input: &InputPreprocessorMessageHandler, viewport: &ViewportMessageHandler) { let mut manipulators = HashMap::with_hasher(NoHashBuilder); let mut unselected = Vec::new(); let mut layer_manipulators = HashSet::with_hasher(NoHashBuilder); @@ -1389,7 +1371,7 @@ impl PenToolData { self.prior_segment_layer = None; self.prior_segments = None; - if let Some((layer, point, _position)) = closest_point(document, viewport, tolerance, document.metadata().all_layers(), |_| false, preferences) { + if let Some((layer, point, _position)) = closest_point(document, viewport, tolerance, document.metadata().all_layers(), |_| false) { self.prior_segment_endpoint = Some(point); self.prior_segment_layer = Some(layer); let vector = document.network_interface.compute_modified_vector(layer).unwrap(); @@ -1468,7 +1450,6 @@ impl Fsm for PenToolFsmState { global_tool_data, input, shape_editor, - preferences, viewport, .. } = tool_action_data; @@ -1630,11 +1611,8 @@ impl Fsm for PenToolFsmState { let snapped = tool_data.snap_manager.free_snap(&SnapData::new(document, input, viewport), &point, SnapTypeConfiguration::default()); let viewport_vec = document.metadata().document_to_viewport.transform_point2(snapped.snapped_point_document); - let close_to_point = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false, preferences).is_some(); - if preferences.vector_meshes - && !close_to_point - && let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, viewport_vec, tolerance) - { + let close_to_point = closest_point(document, viewport_vec, tolerance, document.metadata().all_layers(), |_| false).is_some(); + if !close_to_point && let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, viewport_vec, tolerance) { let pos = closest_segment.closest_point_to_viewport(); let perp = closest_segment.calculate_perp(document); overlay_context.manipulator_anchor(pos, true, None); @@ -1689,10 +1667,8 @@ impl Fsm for PenToolFsmState { } // If a vector mesh then there can be more than one prior segments else if let Some(segments) = tool_data.prior_segments.clone() { - if preferences.vector_meshes { - let selected_anchors_data = HashMap::from([(layer, segments)]); - path_overlays(document, DrawHandles::SelectedAnchors(selected_anchors_data), shape_editor, &mut overlay_context); - } + let selected_anchors_data = HashMap::from([(layer, segments)]); + path_overlays(document, DrawHandles::SelectedAnchors(selected_anchors_data), shape_editor, &mut overlay_context); } else { path_overlays(document, DrawHandles::None, shape_editor, &mut overlay_context); }; @@ -1755,12 +1731,12 @@ impl Fsm for PenToolFsmState { overlay_context.manipulator_anchor(next_anchor, false, None); } - if self == PenToolFsmState::PlacingAnchor && preferences.vector_meshes { + if self == PenToolFsmState::PlacingAnchor { let tolerance = crate::consts::SNAP_POINT_TOLERANCE; let point = SnapCandidatePoint::handle(document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position)); let snapped = tool_data.snap_manager.free_snap(&SnapData::new(document, input, viewport), &point, SnapTypeConfiguration::default()); let viewport = document.metadata().document_to_viewport.transform_point2(snapped.snapped_point_document); - let close_to_point = closest_point(document, viewport, tolerance, document.metadata().all_layers(), |_| false, preferences).is_some(); + let close_to_point = closest_point(document, viewport, tolerance, document.metadata().all_layers(), |_| false).is_some(); if !close_to_point && let Some(closest_segment) = shape_editor.upper_closest_segment(&document.network_interface, viewport, tolerance) { let pos = closest_segment.closest_point_to_viewport(); let perp = closest_segment.calculate_perp(document); @@ -1779,7 +1755,7 @@ impl Fsm for PenToolFsmState { if let Some(layer) = layer && let Some(mut vector) = document.network_interface.compute_modified_vector(layer) { - let closest_point = vector.extendable_points(preferences.vector_meshes).filter(|&id| id != start).find(|&id| { + let closest_point = vector.extendable_points().filter(|&id| id != start).find(|&id| { vector.point_domain.position_from_id(id).is_some_and(|pos| { let dist_sq = transform.transform_point2(pos).distance_squared(transform.transform_point2(next_point)); dist_sq < crate::consts::SNAP_POINT_TOLERANCE.powi(2) @@ -1835,8 +1811,8 @@ impl Fsm for PenToolFsmState { // Get the closest point and the segment it is on let append = input.keyboard.key(append_to_selected); - tool_data.store_clicked_endpoint(document, &transform, input, viewport, preferences); - tool_data.create_initial_point(document, input, viewport, responses, tool_options, append, preferences, shape_editor); + tool_data.store_clicked_endpoint(document, &transform, input, viewport); + tool_data.create_initial_point(document, input, viewport, responses, tool_options, append, shape_editor); // Enter the dragging handle state while the mouse is held down, allowing the user to move the mouse and position the handle PenToolFsmState::DraggingHandle(tool_data.handle_mode) @@ -1860,8 +1836,8 @@ impl Fsm for PenToolFsmState { if let Some(layer) = layer { tool_data.buffering_merged_vector = false; tool_data.handle_mode = HandleMode::ColinearLocked; - tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, preferences, shape_editor, responses); - tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, preferences, responses); + tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, shape_editor, responses); + tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses); } tool_data.buffering_merged_vector = false; PenToolFsmState::DraggingHandle(tool_data.handle_mode) @@ -1876,7 +1852,7 @@ impl Fsm for PenToolFsmState { let layers = LayerNodeIdentifier::ROOT_PARENT .descendants(document.metadata()) .filter(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[])); - if let Some((other_layer, _, _)) = should_extend(document, viewport_vec, crate::consts::SNAP_POINT_TOLERANCE, layers, preferences) { + if let Some((other_layer, _, _)) = should_extend(document, viewport_vec, crate::consts::SNAP_POINT_TOLERANCE, layers) { let selected_nodes = document.network_interface.selected_nodes(); let mut selected_layers = selected_nodes.selected_layers(document.metadata()); if let Some(current_layer) = selected_layers @@ -1906,7 +1882,7 @@ impl Fsm for PenToolFsmState { (PenToolFsmState::DraggingHandle(_), PenToolMessage::DragStop) => { tool_data.cleanup_target_selections(shape_editor, layer, document, responses); tool_data - .finish_placing_handle(SnapData::new(document, input, viewport), transform, preferences, responses) + .finish_placing_handle(SnapData::new(document, input, viewport), transform, responses) .unwrap_or(PenToolFsmState::PlacingAnchor) } ( @@ -2025,7 +2001,7 @@ impl Fsm for PenToolFsmState { move_anchor_with_handles: input.keyboard.key(move_anchor_with_handles), }; let state = tool_data - .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, preferences, responses) + .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses) .unwrap_or(PenToolFsmState::Ready); // Auto-panning @@ -2132,7 +2108,7 @@ impl Fsm for PenToolFsmState { // Confirm to end path if let Some((vector, layer)) = layer.and_then(|layer| document.network_interface.compute_modified_vector(layer)).zip(layer) { let single_point_in_layer = vector.point_domain.ids().len() == 1; - tool_data.finish_placing_handle(SnapData::new(document, input, viewport), transform, preferences, responses); + tool_data.finish_placing_handle(SnapData::new(document, input, viewport), transform, responses); let latest_points = tool_data.latest_points.len() == 1; if latest_points && single_point_in_layer { @@ -2179,7 +2155,7 @@ impl Fsm for PenToolFsmState { PenToolFsmState::Ready } else { tool_data - .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, preferences, responses) + .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses) .unwrap_or(PenToolFsmState::Ready) } } @@ -2210,7 +2186,7 @@ impl Fsm for PenToolFsmState { if tool_data.point_index > 0 { tool_data.point_index -= 1; tool_data - .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, preferences, responses) + .place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses) .unwrap_or(PenToolFsmState::PlacingAnchor) } else { responses.add(PenToolMessage::Abort); @@ -2219,7 +2195,7 @@ impl Fsm for PenToolFsmState { } (_, PenToolMessage::Redo) => { tool_data.point_index = (tool_data.point_index + 1).min(tool_data.latest_points.len().saturating_sub(1)); - tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, preferences, responses); + tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses); match tool_data.point_index { 0 => PenToolFsmState::Ready, _ => PenToolFsmState::PlacingAnchor, diff --git a/editor/src/messages/tool/tool_messages/shape_tool.rs b/editor/src/messages/tool/tool_messages/shape_tool.rs index 7edbb73a33..24798333e7 100644 --- a/editor/src/messages/tool/tool_messages/shape_tool.rs +++ b/editor/src/messages/tool/tool_messages/shape_tool.rs @@ -556,7 +556,6 @@ impl Fsm for ShapeToolFsmState { document, global_tool_data, input, - preferences, shape_editor, viewport, .. @@ -762,7 +761,6 @@ impl Fsm for ShapeToolFsmState { SNAP_POINT_TOLERANCE, document.network_interface.selected_nodes().selected_visible_and_unlocked_layers(&document.network_interface), |_| false, - preferences, ) && clicked_on_line_endpoints(layer, document, input, tool_data) && !input.keyboard.key(Key::Control) { diff --git a/editor/src/messages/tool/tool_messages/spline_tool.rs b/editor/src/messages/tool/tool_messages/spline_tool.rs index 55c8352abb..b364b9a77d 100644 --- a/editor/src/messages/tool/tool_messages/spline_tool.rs +++ b/editor/src/messages/tool/tool_messages/spline_tool.rs @@ -294,7 +294,6 @@ impl Fsm for SplineToolFsmState { global_tool_data, input, shape_editor, - preferences, viewport, .. } = tool_action_data; @@ -303,7 +302,7 @@ impl Fsm for SplineToolFsmState { match (self, event) { (_, SplineToolMessage::CanvasTransformed) => self, (_, SplineToolMessage::Overlays { context: mut overlay_context }) => { - path_endpoint_overlays(document, shape_editor, &mut overlay_context, preferences); + path_endpoint_overlays(document, shape_editor, &mut overlay_context); tool_data.snap_manager.draw_overlays(SnapData::new(document, input, viewport), &mut overlay_context); self } @@ -351,7 +350,7 @@ impl Fsm for SplineToolFsmState { .filter(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[])); // Extend an endpoint of the selected path - if let Some((layer, point, position)) = should_extend(document, viewport_vec, SNAP_POINT_TOLERANCE, layers, preferences) { + if let Some((layer, point, position)) = should_extend(document, viewport_vec, SNAP_POINT_TOLERANCE, layers) { if find_spline(document, layer).is_some() { // If the point is the part of Spline then we extend it. tool_data.current_layer = Some(layer); @@ -417,7 +416,7 @@ impl Fsm for SplineToolFsmState { extend_spline(tool_data, false, responses); tool_data.preview_point = preview_point; - if try_merging_lastest_endpoint(document, tool_data, preferences).is_some() { + if try_merging_lastest_endpoint(document, tool_data).is_some() { responses.add(SplineToolMessage::Confirm); } } @@ -427,7 +426,7 @@ impl Fsm for SplineToolFsmState { (SplineToolFsmState::Drawing, SplineToolMessage::PointerMove) => { let Some(layer) = tool_data.current_layer else { return SplineToolFsmState::Ready }; let ignore = |cp: PointId| tool_data.preview_point.is_some_and(|pp| pp == cp) || tool_data.points.last().is_some_and(|(ep, _)| *ep == cp); - let join_point = closest_point(document, input.mouse.position, PATH_JOIN_THRESHOLD, vec![layer].into_iter(), ignore, preferences); + let join_point = closest_point(document, input.mouse.position, PATH_JOIN_THRESHOLD, vec![layer].into_iter(), ignore); // Endpoints snapping if let Some((_, _, point)) = join_point { @@ -511,7 +510,7 @@ impl Fsm for SplineToolFsmState { } } -fn try_merging_lastest_endpoint(document: &DocumentMessageHandler, tool_data: &mut SplineToolData, preferences: &PreferencesMessageHandler) -> Option<()> { +fn try_merging_lastest_endpoint(document: &DocumentMessageHandler, tool_data: &mut SplineToolData) -> Option<()> { if tool_data.points.len() < 2 { return None; }; @@ -526,7 +525,7 @@ fn try_merging_lastest_endpoint(document: &DocumentMessageHandler, tool_data: &m let exclude = |p: PointId| preview_point.is_some_and(|pp| pp == p) || *last_endpoint == p; let position = document.metadata().transform_to_viewport(current_layer).transform_point2(*last_endpoint_position); - let (layer, endpoint, _) = closest_point(document, position, PATH_JOIN_THRESHOLD, layers, exclude, preferences)?; + let (layer, endpoint, _) = closest_point(document, position, PATH_JOIN_THRESHOLD, layers, exclude)?; tool_data.merge_layers.insert(layer); tool_data.merge_endpoints.push((EndpointPosition::End, endpoint)); diff --git a/node-graph/libraries/vector-types/src/vector/vector_types.rs b/node-graph/libraries/vector-types/src/vector/vector_types.rs index 953adebf72..600743bbd8 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_types.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_types.rs @@ -366,10 +366,20 @@ impl Vector { /// Points that can be extended from. /// - /// This is usually only points with exactly one connection unless vector meshes are enabled. - pub fn extendable_points(&self, vector_meshes: bool) -> impl Iterator + '_ { - let point_ids = self.point_domain.ids().iter().enumerate(); - point_ids.filter(move |(index, _)| vector_meshes || self.segment_domain.connected_count(*index) == 1).map(|(_, &id)| id) + /// This may be points with more than one connection because of vector meshes. + pub fn extendable_points(&self) -> impl Iterator + '_ { + self.point_domain.ids().iter().copied() + } + + // TODO: Avoid needing this special function that's used in only one place. See: + /// Points that can be extended from. + /// + /// This includes only points with exactly one connection because vector meshes are ignored. + pub fn extendable_points_no_vector_meshes(&self) -> impl Iterator + '_ { + self.extendable_points() + .enumerate() + .filter(|&(index, _)| self.segment_domain.connected_count(index) == 1) + .map(|(_, id)| id) } /// Computes if all the connected handles are colinear for an anchor, or if that handle is colinear for a handle. From 40228369f78ff3f3125c84cad8cbd5756896b712 Mon Sep 17 00:00:00 2001 From: Adam Date: Sun, 14 Dec 2025 23:53:22 -0800 Subject: [PATCH 2/3] Improve expose input and remove cancel/commit transaction messages --- .../portfolio/document/document_message.rs | 2 - .../document/document_message_handler.rs | 54 ++++++++++--------- .../document/node_graph/node_graph_message.rs | 6 +-- .../node_graph/node_graph_message_handler.rs | 39 +++----------- .../document/node_graph/node_properties.rs | 8 +-- .../utility_types/network_interface.rs | 6 +-- .../tool/tool_messages/freehand_tool.rs | 9 +--- 7 files changed, 45 insertions(+), 79 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message.rs b/editor/src/messages/portfolio/document/document_message.rs index b6bc9b63ae..cce55e7560 100644 --- a/editor/src/messages/portfolio/document/document_message.rs +++ b/editor/src/messages/portfolio/document/document_message.rs @@ -182,8 +182,6 @@ pub enum DocumentMessage { AddTransaction, StartTransaction, EndTransaction, - CommitTransaction, - CancelTransaction, AbortTransaction, RepeatedAbortTransaction { undo_count: usize, diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index a0b7332c25..3f52bdbf85 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -1292,46 +1292,28 @@ impl MessageHandler> for DocumentMes responses.add_front(NodeGraphMessage::RunDocumentGraph); } DocumentMessage::AddTransaction => { - // Reverse order since they are added to the front - responses.add_front(DocumentMessage::CommitTransaction); - responses.add_front(DocumentMessage::StartTransaction); + self.start_transaction(responses); + self.commit_transaction(responses); } // Note: A transaction should never be started in a scope that mutates the network interface, since it will only be run after that scope ends. DocumentMessage::StartTransaction => { - self.network_interface.start_transaction(); - let network_interface_clone = self.network_interface.clone(); - self.document_undo_history.push_back(network_interface_clone); - if self.document_undo_history.len() > crate::consts::MAX_UNDO_HISTORY_LEN { - self.document_undo_history.pop_front(); - } - // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents - responses.add(PortfolioMessage::UpdateOpenDocumentsList); + self.start_transaction(responses); } // Commits the transaction if the network was mutated since the transaction started, otherwise it cancels the transaction DocumentMessage::EndTransaction => match self.network_interface.transaction_status() { TransactionStatus::Started => { - responses.add_front(DocumentMessage::CancelTransaction); + self.network_interface.finish_transaction(); + self.document_undo_history.pop_back(); } TransactionStatus::Modified => { - responses.add_front(DocumentMessage::CommitTransaction); + self.commit_transaction(responses); } TransactionStatus::Finished => {} }, - DocumentMessage::CancelTransaction => { - self.network_interface.finish_transaction(); - self.document_undo_history.pop_back(); - } - DocumentMessage::CommitTransaction => { - if self.network_interface.transaction_status() == TransactionStatus::Finished { - return; - } - self.network_interface.finish_transaction(); - self.document_redo_history.clear(); - responses.add(PortfolioMessage::UpdateOpenDocumentsList); - } DocumentMessage::AbortTransaction => match self.network_interface.transaction_status() { TransactionStatus::Started => { - responses.add_front(DocumentMessage::CancelTransaction); + self.network_interface.finish_transaction(); + self.document_undo_history.pop_back(); } TransactionStatus::Modified => { responses.add(DocumentMessage::RepeatedAbortTransaction { undo_count: 1 }); @@ -1829,6 +1811,26 @@ impl DocumentMessageHandler { val.unwrap() } + pub fn start_transaction(&mut self, responses: &mut VecDeque) { + self.network_interface.start_transaction(); + let network_interface_clone = self.network_interface.clone(); + self.document_undo_history.push_back(network_interface_clone); + if self.document_undo_history.len() > crate::consts::MAX_UNDO_HISTORY_LEN { + self.document_undo_history.pop_front(); + } + // Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents + responses.add(PortfolioMessage::UpdateOpenDocumentsList); + } + + pub fn commit_transaction(&mut self, responses: &mut VecDeque) { + if self.network_interface.transaction_status() == TransactionStatus::Finished { + return; + } + self.network_interface.finish_transaction(); + self.document_redo_history.clear(); + responses.add(PortfolioMessage::UpdateOpenDocumentsList); + } + pub fn deserialize_document(serialized_content: &str) -> Result { let document_message_handler = serde_json::from_str::(serialized_content) .or_else(|e| { diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs index 330d123efe..c68a68cc7c 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message.rs @@ -63,9 +63,9 @@ pub enum NodeGraphMessage { EnterNestedNetwork, DuplicateSelectedNodes, ExposeInput { - input_connector: InputConnector, - set_to_exposed: bool, - start_transaction: bool, + node_id: NodeId, + input_index: usize, + exposed: bool, }, ExposeEncapsulatingPrimaryInput { exposed: bool, diff --git a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs index 699e898de5..4b5dc8eaad 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs @@ -403,15 +403,7 @@ impl<'a> MessageHandler> for NodeG responses.add(DocumentMessage::EnterNestedNetwork { node_id }); } } - NodeGraphMessage::ExposeInput { - input_connector, - set_to_exposed, - start_transaction, - } => { - let InputConnector::Node { node_id, input_index } = input_connector else { - log::error!("Cannot expose/hide export"); - return; - }; + NodeGraphMessage::ExposeInput { node_id, input_index, exposed } => { let Some(node) = network_interface.document_node(&node_id, selection_network_path) else { log::error!("Could not find node {node_id} in NodeGraphMessage::ExposeInput"); return; @@ -421,38 +413,19 @@ impl<'a> MessageHandler> for NodeG return; }; - // If we're un-exposing an input that is not a value, then disconnect it. This will convert it to a value input, - // so we can come back to handle this message again to set the exposed value in the second run-through. - if !set_to_exposed && node_input.as_value().is_none() { - // Reversed order because we are pushing front - responses.add_front(NodeGraphMessage::ExposeInput { - input_connector, - set_to_exposed, - start_transaction: false, - }); - responses.add_front(NodeGraphMessage::DisconnectInput { input_connector }); - responses.add_front(DocumentMessage::StartTransaction); - return; - } - - // Add a history step, but only do so if we didn't already start a transaction in the first run-through of this message in the above code - if start_transaction { - responses.add_front(DocumentMessage::StartTransaction); - } + responses.add(DocumentMessage::AddTransaction); - // If this node's input is a value type, we set its chosen exposed state + let new_exposed = exposed; if let NodeInput::Value { exposed, .. } = &mut node_input { - *exposed = set_to_exposed; + *exposed = new_exposed; } + responses.add(NodeGraphMessage::SetInput { input_connector: InputConnector::node(node_id, input_index), input: node_input, }); - // Finish the history step - responses.add(DocumentMessage::CommitTransaction); - - // Update the graph UI and re-render + // Update the graph UI and re-render if the graph is open, if the graph is closed then open the graph and zoom in on the input if graph_view_overlay_open { responses.add(PropertiesPanelMessage::Refresh); responses.add(NodeGraphMessage::SendGraph); diff --git a/editor/src/messages/portfolio/document/node_graph/node_properties.rs b/editor/src/messages/portfolio/document/node_graph/node_properties.rs index 2369dc7847..4127e7e066 100644 --- a/editor/src/messages/portfolio/document/node_graph/node_properties.rs +++ b/editor/src/messages/portfolio/document/node_graph/node_properties.rs @@ -47,7 +47,7 @@ pub fn commit_value(_: &T) -> Message { DocumentMessage::AddTransaction.into() } -pub fn expose_widget(node_id: NodeId, index: usize, data_type: FrontendGraphDataType, exposed: bool) -> WidgetInstance { +pub fn expose_widget(node_id: NodeId, input_index: usize, data_type: FrontendGraphDataType, exposed: bool) -> WidgetInstance { ParameterExposeButton::new() .exposed(exposed) .data_type(data_type) @@ -58,9 +58,9 @@ pub fn expose_widget(node_id: NodeId, index: usize, data_type: FrontendGraphData }) .on_update(move |_parameter| Message::Batched { messages: Box::new([NodeGraphMessage::ExposeInput { - input_connector: InputConnector::node(node_id, index), - set_to_exposed: !exposed, - start_transaction: true, + node_id, + input_index, + exposed: !exposed, } .into()]), }) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index ed485a1f06..825a44c617 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -3800,8 +3800,8 @@ impl NodeNetworkInterface { return; }; - // When changing a NodeInput::Node to a NodeInput::Node, the input should first be disconnected to ensure proper side effects - if (matches!(previous_input, NodeInput::Node { .. }) && matches!(new_input, NodeInput::Node { .. })) { + // When changing a NodeInput::Node to another input, the input should first be disconnected to ensure proper side effects + if matches!(previous_input, NodeInput::Node { .. }) { self.disconnect_input(input_connector, network_path); self.set_input(input_connector, new_input, network_path); return; @@ -3856,7 +3856,7 @@ impl NodeNetworkInterface { return; } - // It is necessary to ensure the grpah is acyclic before calling `self.position` as it sometimes crashes with cyclic graphs #3227 + // It is necessary to ensure the graph is acyclic before calling `self.position` as it sometimes crashes with cyclic graphs #3227 let previous_metadata = match &previous_input { NodeInput::Node { node_id, .. } => self.position(node_id, network_path).map(|position| (*node_id, position)), _ => None, diff --git a/editor/src/messages/tool/tool_messages/freehand_tool.rs b/editor/src/messages/tool/tool_messages/freehand_tool.rs index f41b5a5fc9..1faca85c24 100644 --- a/editor/src/messages/tool/tool_messages/freehand_tool.rs +++ b/editor/src/messages/tool/tool_messages/freehand_tool.rs @@ -214,7 +214,6 @@ impl ToolTransition for FreehandTool { #[derive(Clone, Debug, Default)] struct FreehandToolData { end_point: Option<(DVec2, PointId)>, - dragged: bool, weight: f64, layer: Option, } @@ -250,7 +249,6 @@ impl Fsm for FreehandToolFsmState { (FreehandToolFsmState::Ready, FreehandToolMessage::DragStart { append_to_selected }) => { responses.add(DocumentMessage::StartTransaction); - tool_data.dragged = false; tool_data.end_point = None; tool_data.weight = tool_options.line_weight; @@ -307,11 +305,7 @@ impl Fsm for FreehandToolFsmState { FreehandToolFsmState::Drawing } (FreehandToolFsmState::Drawing, FreehandToolMessage::DragStop) => { - if tool_data.dragged { - responses.add(DocumentMessage::CommitTransaction); - } else { - responses.add(DocumentMessage::EndTransaction); - } + responses.add(DocumentMessage::EndTransaction); tool_data.end_point = None; tool_data.layer = None; @@ -380,7 +374,6 @@ fn extend_path_with_next_segment(tool_data: &mut FreehandToolData, position: DVe }); } - tool_data.dragged = true; tool_data.end_point = Some((position, id)); } From 0206c4e15abad39061c6880a67e1c2ced8472cc1 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 21 Dec 2025 15:43:14 -0800 Subject: [PATCH 3/3] Add comments to history-related messages and handlers --- .../portfolio/document/document_message.rs | 17 ++++-- .../document/document_message_handler.rs | 56 ++++++++++++------- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message.rs b/editor/src/messages/portfolio/document/document_message.rs index cce55e7560..8a94907d2c 100644 --- a/editor/src/messages/portfolio/document/document_message.rs +++ b/editor/src/messages/portfolio/document/document_message.rs @@ -49,8 +49,6 @@ pub enum DocumentMessage { }, DeleteSelectedLayers, DeselectAllLayers, - DocumentHistoryBackward, - DocumentHistoryForward, DocumentStructureChanged, DrawArtboardOverlays { context: OverlayContext, @@ -110,7 +108,6 @@ pub enum DocumentMessage { mouse: Option<(f64, f64)>, parent_and_insert_index: Option<(LayerNodeIdentifier, usize)>, }, - Redo, RenameDocument { new_name: String, }, @@ -179,10 +176,23 @@ pub enum DocumentMessage { SetRenderMode { render_mode: RenderMode, }, + Undo, + Redo, + DocumentHistoryBackward, + DocumentHistoryForward, + // TODO: Rename to HistoryStepPush + /// Create a snapshot of the document at this point in time, by immediately starting and committing a transaction. AddTransaction, + // TODO: Rename to HistoryTransactionStart + /// Take a snapshot of the document to an intermediate state, and then depending on what we do next, we might either commit or abort it. StartTransaction, + // TODO: Rename to HistoryTransactionEnd + /// Either commit (creating a new history step) or cancel (removing the last history step, as if it never happened) the last transaction started with `StartTransaction`. EndTransaction, + /// Cause the document to revert back to the state when the transaction was started. For example, the user may be dragging + /// something around and hits Escape to abort the drag. This jumps the document back to the point before the drag began. AbortTransaction, + /// The same as `AbortTransaction` with one step back, but it can also be called with multiple steps back in the history of undos. RepeatedAbortTransaction { undo_count: usize, }, @@ -206,7 +216,6 @@ pub enum DocumentMessage { UpdateClipTargets { clip_targets: HashSet, }, - Undo, UngroupSelectedLayers, UngroupLayer { layer: LayerNodeIdentifier, diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index 3f52bdbf85..1bd1666eef 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -372,8 +372,6 @@ impl MessageHandler> for DocumentMes responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![] }); self.layer_range_selection_reference = None; } - DocumentMessage::DocumentHistoryBackward => self.undo_with_history(viewport, responses), - DocumentMessage::DocumentHistoryForward => self.redo_with_history(viewport, responses), DocumentMessage::DocumentStructureChanged => { if layers_panel_open { self.network_interface.load_structure(); @@ -953,15 +951,6 @@ impl MessageHandler> for DocumentMes responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![layer.to_node()] }); responses.add(ToolMessage::ActivateTool { tool_type: ToolType::Select }); } - DocumentMessage::Redo => { - if self.network_interface.transaction_status() != TransactionStatus::Finished { - return; - } - responses.add(SelectToolMessage::Abort); - responses.add(DocumentMessage::DocumentHistoryForward); - responses.add(ToolMessage::Redo); - responses.add(OverlaysMessage::Draw); - } DocumentMessage::RenameDocument { new_name } => { self.name = new_name.clone(); @@ -1291,6 +1280,27 @@ impl MessageHandler> for DocumentMes self.render_mode = render_mode; responses.add_front(NodeGraphMessage::RunDocumentGraph); } + DocumentMessage::Undo => { + if self.network_interface.transaction_status() != TransactionStatus::Finished { + return; + } + responses.add(ToolMessage::PreUndo); + responses.add(DocumentMessage::DocumentHistoryBackward); + responses.add(OverlaysMessage::Draw); + responses.add(ToolMessage::Undo); + } + DocumentMessage::Redo => { + if self.network_interface.transaction_status() != TransactionStatus::Finished { + return; + } + responses.add(SelectToolMessage::Abort); + responses.add(DocumentMessage::DocumentHistoryForward); + responses.add(ToolMessage::Redo); + responses.add(OverlaysMessage::Draw); + } + DocumentMessage::DocumentHistoryBackward => self.undo_with_history(viewport, responses), + DocumentMessage::DocumentHistoryForward => self.redo_with_history(viewport, responses), + // Create a snapshot of the document at this point in time, by immediately starting and committing a transaction. DocumentMessage::AddTransaction => { self.start_transaction(responses); self.commit_transaction(responses); @@ -1299,37 +1309,50 @@ impl MessageHandler> for DocumentMes DocumentMessage::StartTransaction => { self.start_transaction(responses); } - // Commits the transaction if the network was mutated since the transaction started, otherwise it cancels the transaction + // Either commit (creating a new history step) or cancel (removing the last history step, as if it never happened) the last transaction started with `StartTransaction`. DocumentMessage::EndTransaction => match self.network_interface.transaction_status() { + // This is used if, between the start and end of the transaction, the changes were undone by the user. + // For example, dragging something around and then dropping it back at its exact original position. + // So we cancel the transaction to return to the point before the transaction was started. TransactionStatus::Started => { self.network_interface.finish_transaction(); self.document_undo_history.pop_back(); } + // This is used if, between the start and end of the transaction, actual changes did occur and we want to keep them as part of a history step that the user can undo/redo. TransactionStatus::Modified => { self.commit_transaction(responses); } TransactionStatus::Finished => {} }, DocumentMessage::AbortTransaction => match self.network_interface.transaction_status() { + // If we abort a transaction without any changes having been made, we simply remove the transaction as if it never occurred. TransactionStatus::Started => { self.network_interface.finish_transaction(); self.document_undo_history.pop_back(); } + // If we abort a transaction after changes have been made, we need to undo those changes. TransactionStatus::Modified => { responses.add(DocumentMessage::RepeatedAbortTransaction { undo_count: 1 }); } + // This is an erroneous state indicating that a transaction is being aborted without having ever been started. TransactionStatus::Finished => {} }, + // The same as `AbortTransaction` with one step back, but it can also be called with multiple steps back in the history of undos. DocumentMessage::RepeatedAbortTransaction { undo_count } => { + // This prevents us from aborting a transaction multiple times in a row, which would be erroneous. if self.network_interface.transaction_status() == TransactionStatus::Finished { return; } + // Sometimes (like successive G/R/S transformations) we may need to undo multiple steps to fully abort the transaction, before we finish. for _ in 0..undo_count { self.undo(viewport, responses); } + // Finally finish the transaction, ensuring that any future operations are not erroneously redone as part of this aborted transaction. self.network_interface.finish_transaction(); + + // Refresh state responses.add(OverlaysMessage::Draw); responses.add(PortfolioMessage::UpdateOpenDocumentsList); } @@ -1401,15 +1424,6 @@ impl MessageHandler> for DocumentMes DocumentMessage::UpdateClipTargets { clip_targets } => { self.network_interface.update_clip_targets(clip_targets); } - DocumentMessage::Undo => { - if self.network_interface.transaction_status() != TransactionStatus::Finished { - return; - } - responses.add(ToolMessage::PreUndo); - responses.add(DocumentMessage::DocumentHistoryBackward); - responses.add(OverlaysMessage::Draw); - responses.add(ToolMessage::Undo); - } DocumentMessage::UngroupSelectedLayers => { if !self.selection_network_path.is_empty() { log::error!("Ungrouping selected layers is only supported for the Document Network");