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 <keavon@keavon.com>

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 {

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<Message>, 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<Message>, 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<ManipulatorGroupId>, index: usize, responses: &mut VecDeque<Message>, layer: &LayerNodeIdentifier) {
+	pub fn smooth_manipulator_group(&self, subpath: &bezier_rs::Subpath<ManipulatorGroupId>, index: usize, responses: &mut VecDeque<Message>, 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<Message>, 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 {

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<OpposingHandleLengths>,
 	drag_box_overlay_layer: Option<Vec<LayerId>>,
+	/// 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<u64>,
+	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::<usize>();
+
+	// 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),
 		});

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 {