From 54745e210a8b7d0a53c10e6a8154f0ad6747c9b7 Mon Sep 17 00:00:00 2001 From: 0HyperCube <78500760+0HyperCube@users.noreply.github.com> Date: Wed, 25 Oct 2023 09:26:24 +0100 Subject: [PATCH] Fix the Path tool's smooth/sharp buttons (#1439) * Fix select tool smooth button * Nit * Fix behavior when zero points are selected but the shape is active --------- Co-authored-by: Keavon Chambers --- .../common_functionality/overlay_renderer.rs | 2 +- .../tool/common_functionality/shape_editor.rs | 43 ++++++------- .../messages/tool/tool_messages/path_tool.rs | 61 +++++++++++-------- node-graph/graph-craft/src/document.rs | 4 +- 4 files changed, 58 insertions(+), 52 deletions(-) diff --git a/editor/src/messages/tool/common_functionality/overlay_renderer.rs b/editor/src/messages/tool/common_functionality/overlay_renderer.rs index d4540092..f3d44410 100644 --- a/editor/src/messages/tool/common_functionality/overlay_renderer.rs +++ b/editor/src/messages/tool/common_functionality/overlay_renderer.rs @@ -343,7 +343,7 @@ impl OverlayRenderer { let deselected_style = style::PathStyle::new(Some(Stroke::new(Some(COLOR_ACCENT), POINT_STROKE_WEIGHT)), Fill::solid(Color::WHITE)); let selected_shape_state = state.get(&layer); // Update if the manipulator points are shown as selected - // Here the index is important, even though overlays[..] has five elements we only care about the first three + // Here the index is important, even though overlays has five elements we only care about the first three for (index, overlay) in [&overlays.in_handle, &overlays.out_handle, &overlays.anchor].into_iter().enumerate() { let selected_type = [SelectedType::InHandle, SelectedType::OutHandle, SelectedType::Anchor][index]; if let Some(overlay_path) = overlay { diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index 32473525..3941449d 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/editor/src/messages/tool/common_functionality/shape_editor.rs @@ -1,3 +1,4 @@ +use super::graph_modification_utils; use crate::consts::DRAG_THRESHOLD; use crate::messages::portfolio::document::node_graph::VectorDataModification; use crate::messages::prelude::*; @@ -167,17 +168,16 @@ impl ShapeState { /// Moves a control point to a `new_position` in document space. /// Returns `Some(())` if successful and `None` otherwise. - pub fn reposition_control_point(&self, point: &ManipulatorPointId, responses: &mut VecDeque, document: &Document, new_position: DVec2, layer_path: &[u64]) -> Option<()> { - let layer = document.layer(layer_path).ok()?; - let vector_data = layer.as_vector_data()?; - let transform = layer.transform.inverse(); - let position = transform.transform_point2(new_position - layer.pivot); - let group = vector_data.manipulator_from_id(point.group)?; + pub fn reposition_control_point(&self, point: &ManipulatorPointId, responses: &mut VecDeque, document: &Document, new_position: DVec2, layer: LayerNodeIdentifier) -> Option<()> { + let subpaths = get_subpaths(layer, document)?; + let transform = document.metadata.transform_to_viewport(layer).inverse(); + let position = transform.transform_point2(new_position); + let group = graph_modification_utils::get_manipulator_from_id(subpaths, point.group)?; let delta = position - point.manipulator_type.get_position(group)?; if point.manipulator_type.is_handle() { responses.add(GraphOperationMessage::Vector { - layer: layer_path.to_vec(), + layer: layer.to_path(), modification: VectorDataModification::SetManipulatorHandleMirroring { id: group.id, mirror_angle: false }, }); } @@ -187,7 +187,7 @@ impl ShapeState { return; }; responses.add(GraphOperationMessage::Vector { - layer: layer_path.to_vec(), + layer: layer.to_path(), modification: VectorDataModification::SetManipulatorPosition { point, position: (position + delta) }, }); }; @@ -208,12 +208,8 @@ impl ShapeState { let mut point_smoothness_status = self .selected_shape_state .iter() - .filter_map(|(layer_id, selection_state)| { - let layer = document.layer(&layer_id.to_path()).ok()?; - let vector_data = layer.as_vector_data()?; - Some((vector_data, selection_state)) - }) - .flat_map(|(vector_data, selection_state)| selection_state.selected_points.iter().map(|selected_point| vector_data.mirror_angle.contains(&selected_point.group))); + .filter_map(|(&layer, selection_state)| Some((graph_modification_utils::get_mirror_handles(layer, document)?, selection_state))) + .flat_map(|(mirror, selection_state)| selection_state.selected_points.iter().map(|selected_point| mirror.contains(&selected_point.group))); let Some(first_is_smooth) = point_smoothness_status.next() else { return ManipulatorAngle::Mixed }; @@ -226,7 +222,7 @@ impl ShapeState { } } - pub fn smooth_manipulator_group(&self, subpath: &bezier_rs::Subpath, index: usize, responses: &mut VecDeque, layer: &LayerNodeIdentifier) { + pub fn smooth_manipulator_group(&self, subpath: &bezier_rs::Subpath, index: usize, responses: &mut VecDeque, layer: LayerNodeIdentifier) { let manipulator_groups = subpath.manipulator_groups(); let manipulator = manipulator_groups[index]; @@ -298,9 +294,8 @@ impl ShapeState { pub fn smooth_selected_groups(&self, responses: &mut VecDeque, document: &Document) -> Option<()> { let mut skip_set = HashSet::new(); - for (layer_id, layer_state) in self.selected_shape_state.iter() { - let layer = document.layer(&layer_id.to_path()).ok()?; - let vector_data = layer.as_vector_data()?; + for (&layer, layer_state) in self.selected_shape_state.iter() { + let subpaths = get_subpaths(layer, document)?; for point in layer_state.selected_points.iter() { if skip_set.contains(&point.group) { @@ -321,14 +316,14 @@ impl ShapeState { group: point.group, manipulator_type: SelectedType::InHandle, }); - let group = vector_data.manipulator_from_id(point.group)?; + let group = graph_modification_utils::get_manipulator_from_id(subpaths, point.group)?; match (anchor_selected, out_selected, in_selected) { (_, true, false) => { let out_handle = ManipulatorPointId::new(point.group, SelectedType::OutHandle); if let Some(position) = group.out_handle { responses.add(GraphOperationMessage::Vector { - layer: layer_id.to_path(), + layer: layer.to_path(), modification: VectorDataModification::SetManipulatorPosition { point: out_handle, position }, }); } @@ -337,13 +332,13 @@ impl ShapeState { let in_handle = ManipulatorPointId::new(point.group, SelectedType::InHandle); if let Some(position) = group.in_handle { responses.add(GraphOperationMessage::Vector { - layer: layer_id.to_path(), + layer: layer.to_path(), modification: VectorDataModification::SetManipulatorPosition { point: in_handle, position }, }); } } (_, _, _) => { - let found = vector_data.subpaths.iter().find_map(|subpath| { + let found = subpaths.iter().find_map(|subpath| { let group_slice = subpath.manipulator_groups(); let index = group_slice.iter().position(|manipulator| manipulator.id == group.id)?; // TODO: try subpath closed? wrapping @@ -351,7 +346,7 @@ impl ShapeState { }); if let Some((subpath, index)) = found { - self.smooth_manipulator_group(subpath, index, responses, layer_id); + self.smooth_manipulator_group(subpath, index, responses, layer); } } } @@ -764,7 +759,7 @@ impl ShapeState { }; if already_sharp { - self.smooth_manipulator_group(subpath, index, responses, &layer); + self.smooth_manipulator_group(subpath, index, responses, layer); } else { let point = ManipulatorPointId::new(manipulator.id, SelectedType::InHandle); responses.add(GraphOperationMessage::Vector { diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index f807f3dd..8b8d23c4 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -2,12 +2,14 @@ use std::vec; use super::tool_prelude::*; use crate::consts::{DRAG_THRESHOLD, SELECTION_THRESHOLD, SELECTION_TOLERANCE}; +use crate::messages::tool::common_functionality::graph_modification_utils::{self, get_manipulator_from_id, get_mirror_handles, get_subpaths}; use crate::messages::tool::common_functionality::overlay_renderer::OverlayRenderer; use crate::messages::tool::common_functionality::shape_editor::{ManipulatorAngle, ManipulatorPointInfo, OpposingHandleLengths, SelectedPointsInfo, ShapeState}; use crate::messages::tool::common_functionality::snapping::SnapManager; use crate::messages::tool::common_functionality::transformation_cage::{add_bounding_box, remove_bounding_box, update_bounding_box}; use document_legacy::document::Document; +use document_legacy::document_metadata::LayerNodeIdentifier; use document_legacy::LayerId; use graphene_core::vector::{ManipulatorPointId, SelectedType}; @@ -204,6 +206,8 @@ struct PathToolData { alt_debounce: bool, opposing_handle_lengths: Option, drag_box_overlay_layer: Option>, + /// Describes information about the selected point(s), if any, across one or multiple shapes and manipulator point types (anchor or handle). + /// The available information varies depending on whether `None`, `One`, or `Multiple` points are currently selected. selection_status: SelectionStatus, } @@ -469,14 +473,14 @@ impl Fsm for PathToolFsmState { PathToolFsmState::Ready } (_, PathToolMessage::SelectedPointXChanged { new_x }) => { - if let Some(&SingleSelectedPoint { coordinates, id, ref layer_path, .. }) = tool_data.selection_status.as_one() { - shape_editor.reposition_control_point(&id, responses, &document.document_legacy, DVec2::new(new_x, coordinates.y), layer_path); + if let Some(&SingleSelectedPoint { coordinates, id, layer, .. }) = tool_data.selection_status.as_one() { + shape_editor.reposition_control_point(&id, responses, &document.document_legacy, DVec2::new(new_x, coordinates.y), layer); } PathToolFsmState::Ready } (_, PathToolMessage::SelectedPointYChanged { new_y }) => { - if let Some(&SingleSelectedPoint { coordinates, id, ref layer_path, .. }) = tool_data.selection_status.as_one() { - shape_editor.reposition_control_point(&id, responses, &document.document_legacy, DVec2::new(coordinates.x, new_y), layer_path); + if let Some(&SingleSelectedPoint { coordinates, id, layer, .. }) = tool_data.selection_status.as_one() { + shape_editor.reposition_control_point(&id, responses, &document.document_legacy, DVec2::new(coordinates.x, new_y), layer); } PathToolFsmState::Ready } @@ -538,6 +542,10 @@ enum SelectionStatus { } impl SelectionStatus { + fn is_none(&self) -> bool { + self == &SelectionStatus::None + } + fn as_one(&self) -> Option<&SingleSelectedPoint> { match self { SelectionStatus::One(one) => Some(one), @@ -551,10 +559,6 @@ impl SelectionStatus { _ => None, } } - - fn is_none(&self) -> bool { - self == &SelectionStatus::None - } } #[derive(Debug, PartialEq)] @@ -566,44 +570,51 @@ struct MultipleSelectedPoints { struct SingleSelectedPoint { coordinates: DVec2, id: ManipulatorPointId, - layer_path: Vec, + layer: LayerNodeIdentifier, manipulator_angle: ManipulatorAngle, } -// If there is one selected and only one manipulator group this yields the selected control point, -// if only one handle is selected it will yield that handle, otherwise it will yield the group's anchor. +/// Sets the cumulative description of the selected points: if `None` are selected, if `One` is selected, or if `Multiple` are selected. +/// Applies to any selected points, whether they are anchors or handles; and whether they are from a single shape or across multiple shapes. fn get_selection_status(document: &Document, shape_state: &mut ShapeState) -> SelectionStatus { - // Check to see if only one manipulator group is selected - let selection_layers: Vec<_> = shape_state.selected_shape_state.iter().take(2).map(|(k, v)| (k, v.selected_points_count())).collect(); - if let [(layer, 1)] = selection_layers[..] { - let Some(layer_data) = document.layer(&layer.to_path()).ok() else { return SelectionStatus::None }; - let Some(vector_data) = layer_data.as_vector_data() else { return SelectionStatus::None }; + let mut selection_layers = shape_state.selected_shape_state.iter().map(|(k, v)| (*k, v.selected_points_count())); + let total_selected_points = selection_layers.clone().map(|(_, v)| v).sum::(); + + // Check to see if only one manipulator group in a single shape is selected + if total_selected_points == 1 { + let Some(layer) = selection_layers.find(|(_, v)| *v > 0).map(|(k, _)| k) else { + return SelectionStatus::None; + }; + + let Some(subpaths) = get_subpaths(layer, document) else { + return SelectionStatus::None; + }; + let Some(mirror) = get_mirror_handles(layer, document) else { + return SelectionStatus::None; + }; let Some(point) = shape_state.selected_points().next() else { return SelectionStatus::None; }; - let Some(group) = vector_data.manipulator_from_id(point.group) else { + let Some(group) = get_manipulator_from_id(subpaths, point.group) else { return SelectionStatus::None; }; let Some(local_position) = point.manipulator_type.get_position(group) else { return SelectionStatus::None; }; - let manipulator_angle = if vector_data.mirror_angle.contains(&point.group) { - ManipulatorAngle::Smooth - } else { - ManipulatorAngle::Sharp - }; + let manipulator_angle = if mirror.contains(&point.group) { ManipulatorAngle::Smooth } else { ManipulatorAngle::Sharp }; return SelectionStatus::One(SingleSelectedPoint { - coordinates: layer_data.transform.transform_point2(local_position) + layer_data.pivot, - layer_path: layer.to_path(), + coordinates: document.metadata.transform_to_document(layer).transform_point2(local_position), + layer, id: *point, manipulator_angle, }); }; - if !selection_layers.is_empty() { + // Check to see if multiple manipulator groups are selected + if total_selected_points > 1 { return SelectionStatus::Multiple(MultipleSelectedPoints { manipulator_angle: shape_state.selected_manipulator_angles(document), }); diff --git a/node-graph/graph-craft/src/document.rs b/node-graph/graph-craft/src/document.rs index 987cb844..85cbd633 100644 --- a/node-graph/graph-craft/src/document.rs +++ b/node-graph/graph-craft/src/document.rs @@ -106,7 +106,7 @@ impl DocumentNode { NodeInput::Inline(inline) => (ProtoNodeInput::None, ConstructionArgs::Inline(inline)), } }; - assert!(!self.inputs.iter().any(|input| matches!(input, NodeInput::Network(_))), "recieved non resolved parameter"); + assert!(!self.inputs.iter().any(|input| matches!(input, NodeInput::Network(_))), "received non resolved parameter"); assert!( !self.inputs.iter().any(|input| matches!(input, NodeInput::Value { .. })), "received value as parameter. inputs: {:#?}, construction_args: {:#?}", @@ -115,7 +115,7 @@ impl DocumentNode { ); // If we have one parameter of the type inline, set it as the construction args - if let &[NodeInput::Inline(ref inline)] = &self.inputs[..] { + if let &[NodeInput::Inline(ref inline)] = self.inputs.as_slice() { args = ConstructionArgs::Inline(inline.clone()); } if let ConstructionArgs::Nodes(nodes) = &mut args {