From ecf94258fa0fc82ac5a9a7f8c27603e510f4adfb Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Wed, 6 May 2026 03:00:38 -0700 Subject: [PATCH] Fix tool overlays and snap targets to use upstream Path node geometry, not downstream final geometry (#4114) * Fix tool overlays and snap targets to use upstream Path node geometry not downstream final geometry * Snap to upstream Path free points, dedupe the lookup via a helper --- .../utility_types/network_interface.rs | 39 ++++++++++++- .../tool/common_functionality/snapping.rs | 17 ++++-- .../snapping/layer_snapper.rs | 56 +++++++++++++++++-- .../messages/tool/tool_messages/path_tool.rs | 36 ++++++++---- .../tool/tool_messages/select_tool.rs | 22 ++++++-- .../transform_layer_message_handler.rs | 13 ++++- 6 files changed, 151 insertions(+), 32 deletions(-) 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 fe4c3c9d..60b4ce9c 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -26,7 +26,7 @@ use graphene_std::ContextDependencies; use graphene_std::math::quad::Quad; use graphene_std::subpath::Subpath; use graphene_std::transform::Footprint; -use graphene_std::vector::click_target::{ClickTarget, ClickTargetType}; +use graphene_std::vector::click_target::{ClickTarget, ClickTargetType, FreePoint}; use graphene_std::vector::{PointId, Vector, VectorModificationType}; use kurbo::BezPath; use memo_network::MemoNetwork; @@ -3191,8 +3191,8 @@ impl NodeNetworkInterface { let nodes = network_metadata .persistent_metadata .node_metadata - .iter() - .filter_map(|(node_id, _)| if self.is_layer(node_id, network_path) { Some(*node_id) } else { None }) + .keys() + .filter_map(|node_id| if self.is_layer(node_id, network_path) { Some(*node_id) } else { None }) .collect::>(); let layer_widths = nodes .iter() @@ -3234,6 +3234,39 @@ impl NodeNetworkInterface { self.document_metadata.layer_vector_data.get(&layer).map(|arc| arc.as_ref().clone()) } + /// The vector geometry an upstream Path node would surface for editing. + /// This is the result of `compute_modified_vector`, but only if a visible 'Path' node is actually upstream. + /// Useful for tool overlays and snap target collection usages that want to match the Path tool's view + /// (e.g. the pre-solidified centerline for a Solidify Stroke layer) and otherwise do nothing. + pub fn upstream_path_node_vector(&self, layer: LayerNodeIdentifier) -> Option { + let graph_layer = graph_modification_utils::NodeGraphLayer::new(layer, self); + graph_layer.upstream_visible_node_id_from_name_in_layer(&DefinitionIdentifier::Network("Path".into()))?; + self.compute_modified_vector(layer) + } + + /// Outline targets for the Select tool's hover/selection overlay, mirroring the Path tool's view. + /// Returns `Some` when an upstream Path node exists so the outline matches what the Path tool edits + /// (e.g. the pre-solidified centerline for a Solidify Stroke layer); returns `None` otherwise so the + /// caller can fall back to the layer's recorded `outlines`/`click_targets`. + pub fn path_aware_outline_targets(&self, layer: LayerNodeIdentifier) -> Option> { + let vector = self.upstream_path_node_vector(layer)?; + + let mut targets = Vec::new(); + let subpaths: Vec> = vector.stroke_bezier_paths().collect(); + if !subpaths.is_empty() { + targets.push(ClickTargetType::CompoundPath(subpaths)); + } + + for &point_id in vector.point_domain.ids() { + if !vector.any_connected(point_id) { + let position = vector.point_domain.position_from_id(point_id).unwrap_or_default(); + targets.push(ClickTargetType::FreePoint(FreePoint::new(point_id, position))); + } + } + + Some(targets) + } + /// Loads the structure of layer nodes from a node graph. pub fn load_structure(&mut self) { self.document_metadata.structure = HashMap::from_iter([(LayerNodeIdentifier::ROOT_PARENT, NodeRelations::default())]); diff --git a/editor/src/messages/tool/common_functionality/snapping.rs b/editor/src/messages/tool/common_functionality/snapping.rs index ad732978..00cbe7e2 100644 --- a/editor/src/messages/tool/common_functionality/snapping.rs +++ b/editor/src/messages/tool/common_functionality/snapping.rs @@ -472,11 +472,18 @@ impl SnapManager { if let Some(ind) = &self.indicator { for layer in &ind.outline_layers { let &Some(layer) = layer else { continue }; - overlay_context.outline( - snap_data.document.metadata().layer_with_free_points_outline(layer), - snap_data.document.metadata().transform_to_viewport(layer), - None, - ); + // Mirror the Path tool's view (e.g. pre-solidified centerline) when an upstream Path node exists, + // otherwise fall back to the layer's recorded outline geometry + if let Some(targets) = snap_data.document.network_interface.path_aware_outline_targets(layer) { + let transform = snap_data.document.metadata().transform_to_viewport_if_feeds(layer, &snap_data.document.network_interface); + overlay_context.outline(targets.iter(), transform, None); + } else { + overlay_context.outline( + snap_data.document.metadata().layer_with_free_points_outline(layer), + snap_data.document.metadata().transform_to_viewport(layer), + None, + ); + } } if let Some(quad) = ind.target_bounds { overlay_context.quad(to_viewport * quad, None, None); diff --git a/editor/src/messages/tool/common_functionality/snapping/layer_snapper.rs b/editor/src/messages/tool/common_functionality/snapping/layer_snapper.rs index 18a190ac..5d7546c7 100644 --- a/editor/src/messages/tool/common_functionality/snapping/layer_snapper.rs +++ b/editor/src/messages/tool/common_functionality/snapping/layer_snapper.rs @@ -74,7 +74,7 @@ impl LayerSnapper { } if document.snapping_state.target_enabled(SnapTarget::Path(PathSnapTarget::IntersectionPoint)) || document.snapping_state.target_enabled(SnapTarget::Path(PathSnapTarget::AlongPath)) { - for subpath in document.metadata().layer_outline(layer) { + let mut push_candidates = |subpath: &Subpath, transform: DAffine2| { for (start_index, curve) in subpath.iter().enumerate() { let document_curve = Affine::new(transform.to_cols_array()) * curve; let start = subpath.manipulator_groups()[start_index].id; @@ -89,6 +89,22 @@ impl LayerSnapper { bounds: None, }); } + }; + + // Post-solidified outline (the layer's recorded geometry) + for subpath in document.metadata().layer_outline(layer) { + push_candidates(subpath, transform); + } + + // Pre-solidified centerline (the Path tool's view) when an upstream Path node exists, + // so a drag can snap to either the visible solidified shape or the editable centerline + if let Some(vector) = document.network_interface.upstream_path_node_vector(layer) { + let path_aware_transform = document.metadata().transform_to_document_if_feeds(layer, &document.network_interface); + if path_aware_transform.is_finite() { + for subpath in vector.stroke_bezier_paths() { + push_candidates(&subpath, path_aware_transform); + } + } } } } @@ -616,10 +632,40 @@ pub fn get_layer_snap_points(layer: LayerNodeIdentifier, snap_data: &SnapData, p for child in layer.descendants(document.metadata()) { get_layer_snap_points(child, snap_data, points); } - } else if document.metadata().layer_outline(layer).next().is_some() { - let to_document = document.metadata().transform_to_document(layer); - for subpath in document.metadata().layer_outline(layer) { - subpath_anchor_snap_points(layer, subpath, snap_data, points, to_document); + } else { + // Post-solidified outline (the layer's recorded geometry) + if document.metadata().layer_outline(layer).next().is_some() { + let to_document = document.metadata().transform_to_document(layer); + for subpath in document.metadata().layer_outline(layer) { + subpath_anchor_snap_points(layer, subpath, snap_data, points, to_document); + } + } + + // Pre-solidified centerline (the Path tool's view) when an upstream Path node exists, + // so anchor snaps can also target the editable anchor positions and isolated free points, + // matching what the Path tool's overlay shows + if let Some(vector) = document.network_interface.upstream_path_node_vector(layer) { + let to_document = document.metadata().transform_to_document_if_feeds(layer, &document.network_interface); + for subpath in vector.stroke_bezier_paths() { + subpath_anchor_snap_points(layer, &subpath, snap_data, points, to_document); + } + + if document.snapping_state.target_enabled(SnapTarget::Path(PathSnapTarget::AnchorPointWithFreeHandles)) { + for &point_id in vector.point_domain.ids() { + if points.len() >= crate::consts::MAX_LAYER_SNAP_POINTS { + break; + } + if !vector.any_connected(point_id) { + let position = vector.point_domain.position_from_id(point_id).unwrap_or_default(); + points.push(SnapCandidatePoint::new( + to_document.transform_point2(position), + SnapSource::Path(PathSnapSource::AnchorPointWithFreeHandles), + SnapTarget::Path(PathSnapTarget::AnchorPointWithFreeHandles), + Some(layer), + )); + } + } + } } } } diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index 40b201a4..713ea258 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -609,7 +609,10 @@ struct PathToolData { drill_through_cycle_index: usize, drill_through_cycle_count: usize, hovered_layers: Vec, - ghost_outline: Vec<(Vec, LayerNodeIdentifier)>, + /// Snapshot of each selected layer's outline geometry at drag start, drawn in gray as a "before" reference. + /// The `bool` flags whether the snapshot came from `path_aware_outline_targets`, in which case the draw + /// site uses `transform_to_viewport_if_feeds` to match the Path tool's coordinate space. + ghost_outline: Vec<(Vec, LayerNodeIdentifier, bool)>, make_path_editable_is_allowed: bool, } @@ -708,10 +711,15 @@ impl PathToolData { fn set_ghost_outline(&mut self, shape_editor: &ShapeState, document: &DocumentMessageHandler) { self.ghost_outline.clear(); for &layer in shape_editor.selected_shape_state.keys() { - // We probably need to collect here - let outline: Vec = document.metadata().layer_with_free_points_outline(layer).cloned().collect(); + // Mirror the Path tool's view (e.g. pre-solidified centerline) when an upstream Path node exists, + // otherwise fall back to the layer's recorded outline geometry + let (outline, path_aware) = if let Some(targets) = document.network_interface.path_aware_outline_targets(layer) { + (targets, true) + } else { + (document.metadata().layer_with_free_points_outline(layer).cloned().collect(), false) + }; - self.ghost_outline.push((outline, layer)); + self.ghost_outline.push((outline, layer, path_aware)); } } @@ -1712,8 +1720,12 @@ 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 { - for (outline, layer) in &tool_data.ghost_outline { - let transform = document.metadata().transform_to_viewport(*layer); + for (outline, layer, path_aware) in &tool_data.ghost_outline { + let transform = if *path_aware { + document.metadata().transform_to_viewport_if_feeds(*layer, &document.network_interface) + } else { + document.metadata().transform_to_viewport(*layer) + }; overlay_context.outline(outline.iter(), transform, Some(COLOR_OVERLAY_GRAY)); } } @@ -1877,9 +1889,6 @@ impl Fsm for PathToolFsmState { continue; } - let layer_to_viewport = document.metadata().transform_to_viewport(hovered_layer); - let outline = document.metadata().layer_with_free_points_outline(hovered_layer); - // Determine highlight color based on drill-through state let color = match (index, mouse_has_moved) { // If the layer is the next selected one and mouse has not moved, highlight it blue @@ -1891,7 +1900,14 @@ impl Fsm for PathToolFsmState { }; // TODO: Make this draw underneath all other overlays - overlay_context.outline(outline, layer_to_viewport, Some(color)); + // Mirror the Path tool's view (e.g. pre-solidified centerline) when an upstream Path node exists + if let Some(targets) = document.network_interface.path_aware_outline_targets(hovered_layer) { + let layer_to_viewport = document.metadata().transform_to_viewport_if_feeds(hovered_layer, &document.network_interface); + overlay_context.outline(targets.iter(), layer_to_viewport, Some(color)); + } else { + let layer_to_viewport = document.metadata().transform_to_viewport(hovered_layer); + overlay_context.outline(document.metadata().layer_with_free_points_outline(hovered_layer), layer_to_viewport, Some(color)); + } } } Self::Drawing { selection_shape } => { diff --git a/editor/src/messages/tool/tool_messages/select_tool.rs b/editor/src/messages/tool/tool_messages/select_tool.rs index 5baf8138..0169bbdc 100644 --- a/editor/src/messages/tool/tool_messages/select_tool.rs +++ b/editor/src/messages/tool/tool_messages/select_tool.rs @@ -581,6 +581,18 @@ impl SelectToolData { } } +/// Draws the hover/selection outline for a layer. When a Path node is upstream, mirrors the Path tool's view +/// (e.g. the pre-solidified centerline for a Solidify Stroke layer); otherwise uses the layer's recorded outlines. +fn draw_layer_outline(overlay_context: &mut OverlayContext, document: &DocumentMessageHandler, layer: LayerNodeIdentifier, color: Option<&str>) { + if let Some(targets) = document.network_interface.path_aware_outline_targets(layer) { + let layer_to_viewport = document.metadata().transform_to_viewport_if_feeds(layer, &document.network_interface); + overlay_context.outline(targets.iter(), layer_to_viewport, color); + } else { + let layer_to_viewport = document.metadata().transform_to_viewport(layer); + overlay_context.outline(document.metadata().layer_with_free_points_outline(layer), layer_to_viewport, color); + } +} + /// Bounding boxes are unfortunately not axis aligned. The bounding boxes are found after a transformation is applied to all of the layers. /// This uses some rather confusing logic to determine what transform that should be. pub fn create_bounding_box_transform(document: &DocumentMessageHandler) -> DAffine2 { @@ -626,10 +638,10 @@ impl Fsm for SelectToolFsmState { .selected_visible_and_unlocked_layers(&document.network_interface) .filter(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[])) { - let layer_to_viewport = document.metadata().transform_to_viewport(layer); - overlay_context.outline(document.metadata().layer_with_free_points_outline(layer), layer_to_viewport, None); + draw_layer_outline(&mut overlay_context, document, layer, None); if document.metadata().is_text_layer(layer) { + let layer_to_viewport = document.metadata().transform_to_viewport(layer); let transformed_quad = layer_to_viewport * text_bounding_box(layer, document, &cached_data.font_cache); overlay_context.dashed_quad(transformed_quad, None, None, Some(7.), Some(5.), None); } @@ -667,14 +679,13 @@ impl Fsm for SelectToolFsmState { let not_selected_click = click.filter(|&hovered_layer| !document.network_interface.selected_nodes().selected_layers_contains(hovered_layer, document.metadata())); if let Some(layer) = not_selected_click { if overlay_context.visibility_settings.hover_outline() && !document.network_interface.is_artboard(&layer.to_node(), &[]) { - let layer_to_viewport = document.metadata().transform_to_viewport(layer); let mut hover_overlay_draw = |layer: LayerNodeIdentifier, color: Option<&str>| { if layer.has_children(document.metadata()) { if let Some(bounds) = document.metadata().bounding_box_viewport(layer) { overlay_context.quad(Quad::from_box(bounds), color, None); } } else { - overlay_context.outline(document.metadata().layer_with_free_points_outline(layer), layer_to_viewport, color); + draw_layer_outline(&mut overlay_context, document, layer, color); } }; let layer = match tool_data.nested_selection_behavior { @@ -954,8 +965,7 @@ impl Fsm for SelectToolFsmState { if overlay_context.visibility_settings.selection_outline() { // Draws a temporary outline on the layers that will be selected by the current box/lasso area for layer in layers_to_outline { - let layer_to_viewport = document.metadata().transform_to_viewport(layer); - overlay_context.outline(document.metadata().layer_with_free_points_outline(layer), layer_to_viewport, None); + draw_layer_outline(&mut overlay_context, document, layer, None); } } diff --git a/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs b/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs index 74981681..d489c953 100644 --- a/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs +++ b/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs @@ -653,9 +653,16 @@ impl TransformLayerMessageHandler { fn set_ghost_outline(ghost_outline: &mut Vec<(Vec, DAffine2)>, shape_editor: &ShapeState, document: &DocumentMessageHandler) { ghost_outline.clear(); for &layer in shape_editor.selected_shape_state.keys() { - // We probably need to collect here - let outline = document.metadata().layer_with_free_points_outline(layer).cloned().collect(); - let transform = document.metadata().transform_to_viewport(layer); + // Mirror the Path tool's view (e.g. pre-solidified centerline) when an upstream Path node exists, + // otherwise fall back to the layer's recorded outline geometry + let (outline, transform) = if let Some(targets) = document.network_interface.path_aware_outline_targets(layer) { + (targets, document.metadata().transform_to_viewport_if_feeds(layer, &document.network_interface)) + } else { + ( + document.metadata().layer_with_free_points_outline(layer).cloned().collect(), + document.metadata().transform_to_viewport(layer), + ) + }; ghost_outline.push((outline, transform)); } }