diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index f2ab877275b..be21f9e96b6 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/editor/src/messages/tool/common_functionality/shape_editor.rs @@ -863,20 +863,6 @@ impl ShapeState { self.selected_shape_state.get(&layer).map(|state| &state.selected_segments) } - pub fn move_primary(&self, segment: SegmentId, delta: DVec2, layer: LayerNodeIdentifier, responses: &mut VecDeque) { - responses.add(GraphOperationMessage::Vector { - layer, - modification_type: VectorModificationType::ApplyPrimaryDelta { segment, delta }, - }); - } - - pub fn move_end(&self, segment: SegmentId, delta: DVec2, layer: LayerNodeIdentifier, responses: &mut VecDeque) { - responses.add(GraphOperationMessage::Vector { - layer, - modification_type: VectorModificationType::ApplyEndDelta { segment, delta }, - }); - } - pub fn move_anchor(&self, point: PointId, vector: &VectorGeometry, delta: DVec2, layer: LayerNodeIdentifier, selected: Option<&SelectedLayerState>, responses: &mut VecDeque) { // Move anchor responses.add(GraphOperationMessage::Vector { @@ -894,7 +880,10 @@ impl ShapeState { } let relative_position = handle - point_to_dvec2(path_segment.start()) + delta; - let modification_type = VectorModificationType::SetPrimaryHandle { segment, relative_position }; + let modification_type = VectorModificationType::SetHandlePosition { + handle: HandleId::primary(segment), + relative_position, + }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } @@ -919,24 +908,20 @@ impl ShapeState { let transform = network_interface.document_metadata().transform_to_document_if_feeds(layer, network_interface).inverse(); let position = transform.transform_point2(new_position); let current_position = vector.position_of(*point)?; - let delta = position - current_position; match *point { - ManipulatorPointId::Anchor(point) => self.move_anchor(point, &vector, delta, layer, None, responses), - ManipulatorPointId::PrimaryHandle(segment) => { - self.move_primary(segment, delta, layer, responses); - if let Some(handle) = point.as_handle() - && let Some(handles) = vector.colinear_pair_containing(handle) - { - let modification_type = VectorModificationType::SetG1Continuous { handles, enabled: false }; - responses.add(GraphOperationMessage::Vector { layer, modification_type }); - } - } - ManipulatorPointId::EndHandle(segment) => { - self.move_end(segment, delta, layer, responses); - if let Some(handle) = point.as_handle() - && let Some(handles) = vector.colinear_pair_containing(handle) - { + ManipulatorPointId::Anchor(anchor) => self.move_anchor(anchor, &vector, position - current_position, layer, None, responses), + ManipulatorPointId::PrimaryHandle(_) | ManipulatorPointId::EndHandle(_) => { + let handle = point.as_handle()?; + + // Set by its offset from the anchor, since a delta has no stored offset to add onto until the Path node edits this handle + let relative_position = position - vector.anchor_position_of(*point)?; + responses.add(GraphOperationMessage::Vector { + layer, + modification_type: handle.set_relative_position(relative_position), + }); + + if let Some(handles) = vector.colinear_pair_containing(handle) { let modification_type = VectorModificationType::SetG1Continuous { handles, enabled: false }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } @@ -1587,11 +1572,7 @@ impl ShapeState { responses.add(GraphOperationMessage::Vector { layer, modification_type }); // Update segment - let HandleId { ty, segment } = handle; - let modification_type = match ty { - graphene_std::vector::misc::HandleType::Primary => VectorModificationType::SetStartPoint { segment, id }, - graphene_std::vector::misc::HandleType::End => VectorModificationType::SetEndPoint { segment, id }, - }; + let modification_type = VectorModificationType::SetEndpoint { handle, id }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index 493f5c61b08..7378fe3aae8 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -4113,4 +4113,36 @@ mod test_path_tool { assert_eq!(pasted.colinear_pairs().len(), 2, "the pasted copy of the smooth anchor should be smooth too"); } + + // The ellipse's handles come from its generator, so the first typed position is the Path node's first edit to the handle + #[tokio::test] + async fn a_typed_handle_position_moves_the_handle_there() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + editor.draw_ellipse(100., 100., 300., 200.).await; + let layer = editor.get_selected_layer().await.expect("the drawn ellipse should be selected"); + + let handle_in_document = |editor: &EditorTestUtils, handle: ManipulatorPointId| { + let document = editor.active_document(); + let vector = document.network_interface.compute_modified_vector(layer).expect("the ellipse should have a vector"); + let to_document = document.metadata().transform_to_document_if_feeds(layer, &document.network_interface); + to_document.transform_point2(vector.position_of(handle).expect("the handle should exist")) + }; + + let segment = { + let document = editor.active_document(); + let vector = document.network_interface.compute_modified_vector(layer).expect("the ellipse should have a vector"); + vector.segment_ids()[0] + }; + let handle = ManipulatorPointId::PrimaryHandle(segment); + let before = handle_in_document(&editor, handle); + + editor.select_tool(ToolType::Path).await; + editor.editor.dispatcher.message_handlers.tool_message_handler.shape_editor.select_point_by_layer_and_id(handle, layer); + editor.handle_message(PathToolMessage::SelectedPointUpdated).await; + editor.handle_message(PathToolMessage::SelectedPointXChanged { new_x: before.x + 5. }).await; + + let after = handle_in_document(&editor, handle); + assert!(after.distance(before + DVec2::new(5., 0.)) < 1e-6, "the handle moved from {before} to {after}"); + } } diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs index 9c7d3facc83..2aadd7e8523 100644 --- a/editor/src/messages/tool/tool_messages/pen_tool.rs +++ b/editor/src/messages/tool/tool_messages/pen_tool.rs @@ -596,12 +596,18 @@ impl PenToolData { } TargetHandle::PriorInHandle(segment) => { let relative_position = delta - anchor_pos; - let modification_type = VectorModificationType::SetEndHandle { segment, relative_position }; + let modification_type = VectorModificationType::SetHandlePosition { + handle: HandleId::end(segment), + relative_position, + }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } TargetHandle::PriorOutHandle(segment) => { let relative_position = delta - anchor_pos; - let modification_type = VectorModificationType::SetPrimaryHandle { segment, relative_position }; + let modification_type = VectorModificationType::SetHandlePosition { + handle: HandleId::primary(segment), + relative_position, + }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } TargetHandle::None => {} diff --git a/node-graph/libraries/vector-types/src/vector/flatten.rs b/node-graph/libraries/vector-types/src/vector/flatten.rs index be20579b229..75cb09556be 100644 --- a/node-graph/libraries/vector-types/src/vector/flatten.rs +++ b/node-graph/libraries/vector-types/src/vector/flatten.rs @@ -124,10 +124,6 @@ impl WorkingVector { vector.set_smooth_at_indices(paths, smooth_anchors, true); } - - pub fn resolve_id(&self, id: PointId) -> Option { - self.point_ids.iter().position(|&check_id| check_id == id) - } } // TODO: Eventually remove this document upgrade code diff --git a/node-graph/libraries/vector-types/src/vector/vector_modification.rs b/node-graph/libraries/vector-types/src/vector/vector_modification.rs index b90388f9015..743831087f1 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_modification.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_modification.rs @@ -52,6 +52,8 @@ impl PointModification { working.point_positions.retain(|_| keep.next().unwrap_or_default()); working.point_ids.retain(|id| !self.remove.contains(id)); + // Gathering the moves first keeps the segment pass below off the common path where nothing moves + let mut moved: HashMap = HashMap::new(); for (index, id) in working.point_ids.iter().enumerate() { let Some(&delta) = self.delta.get(id) else { continue }; if !delta.is_finite() { @@ -60,17 +62,22 @@ impl PointModification { } working.point_positions[index] += delta; + moved.insert(index, delta); + } - for segment_index in 0..working.segment_ids.len() { - if working.segment_start[segment_index] == index { - working.segment_handles[segment_index].move_start(delta); + // Handles travel with their anchor, carried in one pass over the segment ends + if !moved.is_empty() { + for index in 0..working.segment_ids.len() { + if let Some(&delta) = moved.get(&working.segment_start[index]) { + working.segment_handles[index].move_start(delta); } - if working.segment_end[segment_index] == index { - working.segment_handles[segment_index].move_end(delta); + if let Some(&delta) = moved.get(&working.segment_end[index]) { + working.segment_handles[index].move_end(delta); } } } + let mut named: HashSet = if self.add.is_empty() { HashSet::new() } else { working.point_ids.iter().copied().collect() }; for &add_id in &self.add { let Some(&position) = self.delta.get(&add_id) else { continue }; if !position.is_finite() { @@ -78,8 +85,7 @@ impl PointModification { continue; } - #[cfg(debug_assertions)] - if working.point_ids.contains(&add_id) { + if !named.insert(add_id) { warn!("Tried to push a duplicate point to the working form"); continue; } @@ -144,16 +150,26 @@ impl SegmentModification { let mut keep_iter = keep.iter(); working.segment_ids.retain(|_| *keep_iter.next().unwrap_or(&false)); + // The points no longer change here, so one index serves every endpoint lookup below. + // Nothing looks an endpoint up in the common pure-handle case, so it skips the build. + let mut point_index: HashMap = HashMap::new(); + if !self.start_point.is_empty() || !self.end_point.is_empty() || !self.add.is_empty() { + point_index.reserve(working.point_ids.len()); + for (index, &id) in working.point_ids.iter().enumerate() { + point_index.entry(id).or_insert(index); + } + } + for (index, id) in working.segment_ids.iter().enumerate() { - if let Some(&new) = self.start_point.get(id) { - match working.resolve_id(new) { - Some(point) => working.segment_start[index] = point, + if let Some(new) = self.start_point.get(id) { + match point_index.get(new) { + Some(&point) => working.segment_start[index] = point, None => warn!("Invalid start ID when applying a segment modification"), } } - if let Some(&new) = self.end_point.get(id) { - match working.resolve_id(new) { - Some(point) => working.segment_end[index] = point, + if let Some(new) = self.end_point.get(id) { + match point_index.get(new) { + Some(&point) => working.segment_end[index] = point, None => warn!("Invalid end ID when applying a segment modification"), } } @@ -203,18 +219,24 @@ impl SegmentModification { }; } + let mut named: HashSet = if self.add.is_empty() { HashSet::new() } else { working.segment_ids.iter().copied().collect() }; for &add_id in &self.add { let Some(&start) = self.start_point.get(&add_id) else { continue }; let Some(&end) = self.end_point.get(&add_id) else { continue }; let Some(&handle_start) = self.handle_primary.get(&add_id) else { continue }; let Some(&handle_end) = self.handle_end.get(&add_id) else { continue }; - let Some(start_index) = working.resolve_id(start) else { - warn!("invalid start id: {start:#?}"); + if !named.insert(add_id) { + warn!("Tried to push a duplicate segment to the working form"); + continue; + } + + let Some(&start_index) = point_index.get(&start) else { + warn!("Invalid start ID: {start:#?}"); continue; }; - let Some(end_index) = working.resolve_id(end) else { - warn!("invalid end id: {end:#?}"); + let Some(&end_index) = point_index.get(&end) else { + warn!("Invalid end ID: {end:#?}"); continue; }; @@ -230,7 +252,7 @@ impl SegmentModification { }; if !handles.is_finite() { - warn!("invalid handles"); + warn!("Invalid handles"); continue; } @@ -267,6 +289,22 @@ impl SegmentModification { } } + /// The handle map for the side of its segment that the given handle sits on. + fn handle_side_of(&mut self, handle: HandleId) -> &mut HashMap> { + match handle.ty { + HandleType::Primary => &mut self.handle_primary, + HandleType::End => &mut self.handle_end, + } + } + + /// The endpoint map for the side of its segment that the given handle sits on. + fn endpoint_side_of(&mut self, handle: HandleId) -> &mut HashMap { + match handle.ty { + HandleType::Primary => &mut self.start_point, + HandleType::End => &mut self.end_point, + } + } + fn push(&mut self, id: SegmentId, points: [PointId; 2], handles: [Option; 2]) { self.remove.remove(&id); self.add.push(id); @@ -302,22 +340,45 @@ pub struct VectorModification { #[derive(PartialEq, Clone, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum VectorModificationType { - InsertSegment { id: SegmentId, points: [PointId; 2], handles: [Option; 2] }, - InsertPoint { id: PointId, position: DVec2 }, - - RemoveSegment { id: SegmentId }, - RemovePoint { id: PointId }, - - SetG1Continuous { handles: [HandleId; 2], enabled: bool }, - SetHandles { segment: SegmentId, handles: [Option; 2] }, - SetPrimaryHandle { segment: SegmentId, relative_position: DVec2 }, - SetEndHandle { segment: SegmentId, relative_position: DVec2 }, - SetStartPoint { segment: SegmentId, id: PointId }, - SetEndPoint { segment: SegmentId, id: PointId }, - - ApplyPointDelta { point: PointId, delta: DVec2 }, - ApplyPrimaryDelta { segment: SegmentId, delta: DVec2 }, - ApplyEndDelta { segment: SegmentId, delta: DVec2 }, + InsertSegment { + id: SegmentId, + points: [PointId; 2], + handles: [Option; 2], + }, + InsertPoint { + id: PointId, + position: DVec2, + }, + + RemoveSegment { + id: SegmentId, + }, + RemovePoint { + id: PointId, + }, + + SetG1Continuous { + handles: [HandleId; 2], + enabled: bool, + }, + SetHandles { + segment: SegmentId, + handles: [Option; 2], + }, + SetHandlePosition { + handle: HandleId, + relative_position: DVec2, + }, + /// Reconnects one end of the handle's segment to another point: the start for a primary handle, the end for an end handle. + SetEndpoint { + handle: HandleId, + id: PointId, + }, + + ApplyPointDelta { + point: PointId, + delta: DVec2, + }, } impl VectorModificationType { @@ -325,13 +386,8 @@ impl VectorModificationType { fn is_finite(&self) -> bool { match self { Self::InsertSegment { handles, .. } | Self::SetHandles { handles, .. } => handles.iter().flatten().all(|handle| handle.is_finite()), - Self::InsertPoint { position: value, .. } - | Self::SetPrimaryHandle { relative_position: value, .. } - | Self::SetEndHandle { relative_position: value, .. } - | Self::ApplyPointDelta { delta: value, .. } - | Self::ApplyPrimaryDelta { delta: value, .. } - | Self::ApplyEndDelta { delta: value, .. } => value.is_finite(), - Self::RemoveSegment { .. } | Self::RemovePoint { .. } | Self::SetG1Continuous { .. } | Self::SetStartPoint { .. } | Self::SetEndPoint { .. } => true, + Self::InsertPoint { position: value, .. } | Self::SetHandlePosition { relative_position: value, .. } | Self::ApplyPointDelta { delta: value, .. } => value.is_finite(), + Self::RemoveSegment { .. } | Self::RemovePoint { .. } | Self::SetG1Continuous { .. } | Self::SetEndpoint { .. } => true, } } } @@ -444,14 +500,18 @@ impl VectorModification { self.points.apply(&mut working); self.segments.apply(&mut working); - let valid = |val: &[HandleId; 2]| working.segment_ids.contains(&val[0].segment) && working.segment_ids.contains(&val[1].segment); + let surviving_segments: HashSet = working.segment_ids.iter().copied().collect(); + let valid = |val: &[HandleId; 2]| surviving_segments.contains(&val[0].segment) && surviving_segments.contains(&val[1].segment); working .colinear .retain(|val| !self.remove_g1_continuous.contains(val) && !self.remove_g1_continuous.contains(&[val[1], val[0]]) && valid(val)); + // Both orientations go in, since a pair and its reverse name the same tangent + let mut present: HashSet<[HandleId; 2]> = working.colinear.iter().flat_map(|&[first, second]| [[first, second], [second, first]]).collect(); for handles in &self.add_g1_continuous { - if !working.colinear.iter().any(|test| test == handles || test == &[handles[1], handles[0]]) && valid(handles) { + if !present.contains(handles) && valid(handles) { working.colinear.push(*handles); + present.extend([*handles, [handles[1], handles[0]]]); } } @@ -492,30 +552,16 @@ impl VectorModification { self.segments.handle_primary.insert(*segment, handles[0]); self.segments.handle_end.insert(*segment, handles[1]); } - VectorModificationType::SetPrimaryHandle { segment, relative_position } => { - self.segments.handle_primary.insert(*segment, Some(*relative_position)); - } - VectorModificationType::SetEndHandle { segment, relative_position } => { - self.segments.handle_end.insert(*segment, Some(*relative_position)); - } - VectorModificationType::SetStartPoint { segment, id } => { - self.segments.start_point.insert(*segment, *id); + VectorModificationType::SetHandlePosition { handle, relative_position } => { + self.segments.handle_side_of(*handle).insert(handle.segment, Some(*relative_position)); } - VectorModificationType::SetEndPoint { segment, id } => { - self.segments.end_point.insert(*segment, *id); + VectorModificationType::SetEndpoint { handle, id } => { + self.segments.endpoint_side_of(*handle).insert(handle.segment, *id); } VectorModificationType::ApplyPointDelta { point, delta } => { *self.points.delta.entry(*point).or_default() += *delta; } - VectorModificationType::ApplyPrimaryDelta { segment, delta } => { - let position = self.segments.handle_primary.entry(*segment).or_default(); - *position = Some(position.unwrap_or_default() + *delta); - } - VectorModificationType::ApplyEndDelta { segment, delta } => { - let position = self.segments.handle_end.entry(*segment).or_default(); - *position = Some(position.unwrap_or_default() + *delta); - } } } @@ -648,11 +694,7 @@ pub trait HandleExt { impl HandleExt for HandleId { fn set_relative_position(self, relative_position: DVec2) -> VectorModificationType { - let Self { ty, segment } = self; - match ty { - HandleType::Primary => VectorModificationType::SetPrimaryHandle { segment, relative_position }, - HandleType::End => VectorModificationType::SetEndHandle { segment, relative_position }, - } + VectorModificationType::SetHandlePosition { handle: self, relative_position } } } @@ -672,6 +714,88 @@ mod tests { vector.position(vector.index_of_point_id(id).unwrap() as usize).unwrap() } + // Re-adding a segment or point under an ID the geometry already holds adds no second copy under that ID + #[test] + fn duplicate_adds_are_skipped() { + use kurbo::{Rect, Shape}; + + let (mut vector, mut paths) = Vector::from_bezpath(&Rect::new(0., 0., 10., 10.).to_path(0.)); + let segment = vector.segment_iter(&paths).next().map(|(id, ..)| id).unwrap(); + let [start, end] = [anchor_id(&vector, &paths, 0), anchor_id(&vector, &paths, 2)]; + + let mut modification = VectorModification::default(); + modification.modify(&VectorModificationType::InsertSegment { + id: segment, + points: [start, end], + handles: [None, None], + }); + modification.modify(&VectorModificationType::InsertPoint { + id: start, + position: DVec2::new(99., 99.), + }); + modification.apply(&mut vector, &mut paths); + + assert_eq!(vector.segment_iter(&paths).count(), 4); + assert_eq!(vector.anchor_points(&paths).count(), 4); + let ids: HashSet<_> = vector.segment_iter(&paths).map(|(id, ..)| id).collect(); + assert_eq!(ids.len(), 4); + } + + // An end handle addresses the end of its segment, both where its handle sits and which point that end connects to + #[test] + fn end_handles_address_the_end_of_their_segment() { + let mut open = BezPath::new(); + open.move_to(Point::new(0., 0.)); + open.curve_to(Point::new(1., 1.), Point::new(2., 1.), Point::new(3., 0.)); + + let (vector, paths) = Vector::from_bezpath(&open); + let segment = vector.segment_iter(&paths).next().map(|(id, ..)| id).unwrap(); + let new_end = PointId::generate(); + + let mut modification = VectorModification::create_from_vector(&vector, &paths); + modification.modify(&VectorModificationType::InsertPoint { + id: new_end, + position: DVec2::new(10., 5.), + }); + modification.modify(&VectorModificationType::SetEndpoint { + handle: HandleId::end(segment), + id: new_end, + }); + modification.modify(&HandleId::end(segment).set_relative_position(DVec2::new(1., 2.))); + + let (mut applied, mut applied_paths) = (Vector::default(), List::new()); + modification.apply(&mut applied, &mut applied_paths); + + let (_, curve, start, end) = applied.segment_iter(&applied_paths).find(|&(id, ..)| id == segment).unwrap(); + assert_eq!(curve, PathSeg::Cubic(kurbo::CubicBez::new((0., 0.), (1., 1.), (11., 7.), (10., 5.)))); + assert_eq!(start, anchor_id(&vector, &paths, 0)); + assert_eq!(end, new_end); + } + + // A pair and its reverse name the same tangent, so removing either order clears a pair the incoming geometry holds + #[test] + fn g1_pair_removal_ignores_the_order_of_its_handles() { + let mut open = BezPath::new(); + open.move_to(Point::new(0., 0.)); + open.curve_to(Point::new(1., 1.), Point::new(2., 1.), Point::new(3., 0.)); + open.curve_to(Point::new(4., -1.), Point::new(5., -1.), Point::new(6., 0.)); + + let (mut smooth, mut smooth_paths) = Vector::from_bezpath(&open); + let middle = anchor_id(&smooth, &smooth_paths, 1); + smooth.set_smooth(&mut smooth_paths, middle, true); + let pairs = smooth.colinear_pairs(&smooth_paths); + let [pair] = pairs[..] else { panic!("the middle anchor should be smooth, not {pairs:?}") }; + + for removed in [pair, [pair[1], pair[0]]] { + let (mut vector, mut paths) = (smooth.clone(), smooth_paths.clone()); + let mut modification = VectorModification::default(); + modification.remove_g1_continuous.insert(removed); + modification.apply(&mut vector, &mut paths); + + assert!(vector.colinear_pairs(&paths).is_empty(), "removing {removed:?} should clear the tangent"); + } + } + #[test] fn modify_new() { let (mut vector, mut paths) = Vector::from_bezpath(&ellipse_bezpath(DVec2::ZERO, DVec2::ONE)); @@ -732,8 +856,8 @@ mod tests { #[test] fn non_finite_modifications_are_ignored() { let mut modification = VectorModification::default(); - modification.modify(&VectorModificationType::SetPrimaryHandle { - segment: SegmentId::generate(), + modification.modify(&VectorModificationType::SetHandlePosition { + handle: HandleId::primary(SegmentId::generate()), relative_position: DVec2::NAN, }); modification.modify(&VectorModificationType::ApplyPointDelta { 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 86f51aa7b99..4b10f762911 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_types.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_types.rs @@ -384,6 +384,16 @@ impl Vector { .collect() } + /// Drops the paths and every point used only as a handle, leaving the anchors and free points as a bare cloud. + /// A point serving as an anchor in any path is kept even where another path uses it as a handle. + pub fn keep_only_anchors(&mut self, paths: &mut List) { + let anchors = self.anchor_indices(paths); + let pool = anchors.into_iter().filter_map(|index| self.0.clone_item(index)).collect(); + + self.0 = pool; + *paths = List::new(); + } + /// The positions of all anchor points and free points in pool order, skipping handles. pub fn anchor_positions(&self, paths: &List) -> Vec { self.anchor_indices(paths).into_iter().filter_map(|index| self.position(index)).collect() diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index a7bc75d1f71..6a26f286ed2 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -33,7 +33,10 @@ use vector_types::vector::misc::{ bezpath_from_manipulator_groups, dvec2_to_point, handles_to_segment, is_linear, point_to_dvec2, segment_to_handles, }; use vector_types::vector::style::{DashPattern, Gradient, GradientSettings, Stroke, StrokeAlign, StrokeCap, StrokeJoin}; -use vector_types::vector::{Path, PointId, SegmentId, WorkingVector, curve_type_at, edit_item_geometry, no_paths, paths_from_item_attributes, paths_from_list_row_attributes}; +use vector_types::vector::{ + CurveType, Path, PointId, SegmentId, WorkingVector, curve_type_at, edit_item_geometry, handle_point, no_paths, paths_from_item_attributes, paths_from_list_row_attributes, push_path, + span_endpoints, spans, +}; /// Appends one point to the working form, returning the index that names it. pub(crate) fn push_working_point(working: &mut WorkingVector, id: PointId, position: DVec2) -> usize { @@ -1415,27 +1418,23 @@ async fn points_to_polyline(_: impl Ctx, #[implem let mut points = points; edit_item_geometry(&mut points, |vector, paths| { - let flattened = WorkingVector::flatten(vector, paths); - let mut working = WorkingVector { - point_ids: flattened.point_ids, - point_positions: flattened.point_positions, - ..Default::default() - }; - let mut next_id = SegmentId::ZERO; - - let points_count = working.point_ids.len(); + vector.keep_only_anchors(paths); - if points_count >= 2 { - (0..points_count - 1).for_each(|i| { - push_working_segment(&mut working, next_id.next_id(), i, i + 1, BezierHandles::Linear); - }); - - if closed && points_count != 2 { - push_working_segment(&mut working, next_id.next_id(), points_count - 1, 0, BezierHandles::Linear); - } + let points_count = vector.point_count(); + if points_count < 2 { + return; } - working.reweave(vector, paths); + // A pair has nowhere to close back to, so its two anchors draw one segment either way + let closing = closed && points_count != 2; + let mut next_id = SegmentId::ZERO; + + push_path( + paths, + CurveType::Linear, + (0..points_count as u32).chain(closing.then_some(0)), + std::iter::repeat_with(|| next_id.next_id()), + ); }); points @@ -2400,7 +2399,7 @@ async fn scatter_points( let mut rng = rand::rngs::StdRng::seed_from_u64(seed as u64); - let mut result = WorkingVector::default(); + let mut cloud = List::new(); let content_paths = paths_from_item_attributes(&content).cloned().unwrap_or_default(); let path_with_bounding_boxes: Vec<_> = content @@ -2420,11 +2419,16 @@ async fn scatter_points( } for point in bezpath_algorithms::poisson_disk_points(i, &path_with_bounding_boxes, separation, || rng.random::()) { - push_working_point(&mut result, PointId::generate(), point); + cloud.push(Item::new_from_element(point).with_attribute(ATTR_ID, PointId::generate())); } } - edit_item_geometry(&mut content, |vector, paths| result.reweave(vector, paths)); + // The scattered points replace the source shape entirely, leaving no paths + edit_item_geometry(&mut content, |vector, paths| { + *vector = Vector(cloud); + *paths = List::new(); + }); + content }) } @@ -2440,20 +2444,30 @@ async fn spline(_: impl Ctx, #[implementations(Gr edit_item_geometry(&mut content, |vector, paths| { let manipulator_runs = vector.stroke_manipulator_groups(paths).collect::>(); + vector.keep_only_anchors(paths); - let flattened = WorkingVector::flatten(vector, paths); - let mut working = WorkingVector { - point_ids: flattened.point_ids, - point_positions: flattened.point_positions, - ..Default::default() - }; + // An ID shared by several points resolves to the first of them + let mut anchor_index: HashMap = HashMap::new(); + for index in 0..vector.point_count() as u32 { + if let Some(id) = vector.point_id_at(index) { + anchor_index.entry(id).or_insert(index); + } + } let mut next_id = SegmentId::ZERO; let mut smooth_anchors = Vec::new(); for (manipulator_groups, closed) in manipulator_runs { let positions = manipulator_groups.iter().map(|manipulators| manipulators.anchor).collect::>(); + if positions.len() < 2 { + continue; + } let closed = closed && positions.len() > 2; + let Some(anchors) = manipulator_groups.iter().map(|group| anchor_index.get(&group.id).copied()).collect::>>() else { + log::error!("Spline skipped a run with anchor IDs that name no point"); + continue; + }; + // The spline runs smoothly through every anchor with a segment on each side let interior = if closed { &manipulator_groups[..] @@ -2469,22 +2483,27 @@ async fn spline(_: impl Ctx, #[implementations(Gr solve_spline_first_handle_open(&positions) }; - // Create segments with computed Bezier handles and add them to the output vector element's segments. - for i in 0..(positions.len() - if closed { 0 } else { 1 }) { - let next_index = (i + 1) % positions.len(); - - let start_index = working.resolve_id(manipulator_groups[i].id).unwrap(); - let end_index = working.resolve_id(manipulator_groups[next_index].id).unwrap(); + // Weave the run as one cubic path, each span's two handles appended to the pool and referenced between its anchors + let span_count = positions.len() - if closed { 0 } else { 1 }; + let (mut references, mut ids) = (Vec::with_capacity(span_count * 3 + 1), Vec::with_capacity(span_count)); + for i in 0..span_count { + let next_index = (i + 1) % positions.len(); let handle_start = first_handles[i]; let handle_end = positions[next_index] * 2. - first_handles[next_index]; - let handles = BezierHandles::Cubic { handle_start, handle_end }; - push_working_segment(&mut working, next_id.next_id(), start_index, end_index, handles); + references.push(anchors[i]); + for handle in [handle_start, handle_end] { + references.push(vector.point_count() as u32); + vector.0.push(Item::new_from_element(handle).with_attribute(ATTR_ID, PointId::UNNAMED)); + } + ids.push(next_id.next_id()); } + references.push(anchors[span_count % positions.len()]); + + push_path(paths, CurveType::Cubic, references, ids); } - working.reweave(vector, paths); vector.set_smooth_points(paths, smooth_anchors, true); }); @@ -2513,38 +2532,40 @@ fn inverse_linear_or_repair(linear: DMat2) -> DMat2 { repaired.inverse() } -/// Applies per-anchor displacement deltas to the anchor and handle positions of a vector element. -fn apply_point_deltas(vector: &mut Vector, paths: &mut List, deltas: &[DVec2], transform: DAffine2) { - let mut working = WorkingVector::flatten(vector, paths); - let mut already_applied = vec![false; working.point_positions.len()]; - - for index in 0..working.segment_ids.len() { - let (start, end) = (working.segment_start[index], working.segment_end[index]); - let start_delta = deltas[start]; - let end_delta = deltas[end]; - - if !already_applied[start] { - working.point_positions[start] += start_delta; - already_applied[start] = true; - } - if !already_applied[end] { - working.point_positions[end] += end_delta; - already_applied[end] = true; - } +/// Applies displacement deltas to the anchors at the given pool indices, moving the handles of their segments along with them. +fn apply_point_deltas(vector: &mut Vector, paths: &List, anchors: &[usize], deltas: &[DVec2]) { + let mut anchor_delta = vec![None; vector.point_count()]; + for (&anchor, &delta) in anchors.iter().zip(deltas) { + anchor_delta[anchor] = Some(delta); + } + let mut delta_at: Vec = anchor_delta.iter().map(|delta| delta.unwrap_or_default()).collect(); + let delta_of = |anchor: u32| anchor_delta.get(anchor as usize).copied().flatten().unwrap_or_default(); + + // A handle travels with the anchor on its own side of the segment, unless it is also an anchor with a delta of its own + for span in spans(paths) { + let Some((start, end)) = span_endpoints(paths, span) else { continue }; + let (start_delta, end_delta) = (delta_of(start), delta_of(end)); + + let carried = match curve_type_at(paths, span.path as usize) { + CurveType::Linear => [(None, DVec2::ZERO); 2], + // A quadratic's lone handle answers to both anchors, so it splits the difference between them + CurveType::Quadratic => [(handle_point(paths, span, true), (start_delta + end_delta) / 2.), (None, DVec2::ZERO)], + CurveType::Cubic => [(handle_point(paths, span, true), start_delta), (handle_point(paths, span, false), end_delta)], + }; - match &mut working.segment_handles[index] { - BezierHandles::Cubic { handle_start, handle_end } => { - *handle_start += start_delta; - *handle_end += end_delta; - } - BezierHandles::Quadratic { handle } => { - *handle = transform.transform_point2(*handle) + (start_delta + end_delta) / 2.; + for (handle, delta) in carried { + if let Some(handle) = handle + && anchor_delta.get(handle as usize).is_some_and(Option::is_none) + && let Some(slot) = delta_at.get_mut(handle as usize) + { + *slot = delta; } - BezierHandles::Linear => {} } } - working.reweave(vector, paths); + for (position, delta) in vector.0.iter_element_values_mut().zip(delta_at) { + *position += delta; + } } /// Perturbs the positions of anchor points in vector geometry by random amounts and directions. @@ -2573,12 +2594,11 @@ async fn jitter_points( let transform_attribute: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); let inverse_linear = inverse_linear_or_repair(transform_attribute.matrix2); - let transform: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); edit_item_geometry(&mut content, |vector, paths| { - let deltas: Vec<_> = vector - .anchor_indices(paths) - .into_iter() - .map(|point_index| { + let anchors = vector.anchor_indices(paths); + let deltas: Vec<_> = anchors + .iter() + .map(|&point_index| { let normal = if along_normals { vector.point_tangent(paths, point_index).map(|t| -t.perp()) } else { None }; let offset = if let Some(normal) = normal { @@ -2591,7 +2611,7 @@ async fn jitter_points( }) .collect(); - apply_point_deltas(vector, paths, &deltas, transform); + apply_point_deltas(vector, paths, &anchors, &deltas); }); content @@ -2617,12 +2637,11 @@ async fn offset_points( let transform_attribute: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); let inverse_linear = inverse_linear_or_repair(transform_attribute.matrix2); - let transform: DAffine2 = content.attribute_cloned_or_default(ATTR_TRANSFORM); edit_item_geometry(&mut content, |vector, paths| { - let deltas: Vec<_> = vector - .anchor_indices(paths) - .into_iter() - .map(|point_index| { + let anchors = vector.anchor_indices(paths); + let deltas: Vec<_> = anchors + .iter() + .map(|&point_index| { let Some(normal) = vector.point_tangent(paths, point_index).map(|t| -t.perp()) else { return DVec2::ZERO; }; @@ -2631,7 +2650,7 @@ async fn offset_points( }) .collect(); - apply_point_deltas(vector, paths, &deltas, transform); + apply_point_deltas(vector, paths, &anchors, &deltas); }); content @@ -3818,6 +3837,57 @@ mod test { assert_eq!(result.element().segment_iter(&paths).count(), 0); } + // The lone handle of a quadratic answers to both anchors, so it moves by the average of their deltas + #[test] + fn point_deltas_carry_a_quadratic_handle_with_both_anchors() { + let mut quadratic = BezPath::new(); + quadratic.move_to((0., 0.)); + quadratic.quad_to((5., 10.), (10., 0.)); + + let (mut vector, paths) = Vector::from_bezpath(&quadratic); + let anchors = vector.anchor_indices(&paths); + super::apply_point_deltas(&mut vector, &paths, &anchors, &[DVec2::X, DVec2::X * 3.]); + + assert_eq!(vector.position(0), Some(DVec2::new(1., 0.))); + assert_eq!(vector.position(1), Some(DVec2::new(7., 10.))); + assert_eq!(vector.position(2), Some(DVec2::new(13., 0.))); + } + + // A point no segment touches still takes its delta, so jitter can shift a bare point cloud + #[test] + fn point_deltas_move_free_floating_points() { + let cloud = vector_item_from_points(&[DVec2::ZERO, DVec2::new(10., 0.)]); + let paths = item_paths(&cloud).clone(); + let mut vector = cloud.element().clone(); + + let anchors = vector.anchor_indices(&paths); + super::apply_point_deltas(&mut vector, &paths, &anchors, &[DVec2::Y, DVec2::Y * 2.]); + + assert_eq!(vector.position(0), Some(DVec2::new(0., 1.))); + assert_eq!(vector.position(1), Some(DVec2::new(10., 2.))); + } + + // Point 1 is a handle of the first path and an anchor of the second, so it keeps its own delta rather than its neighbor's + #[test] + fn point_deltas_keep_an_anchor_that_is_also_a_handle_on_its_own_delta() { + let mut vector = Vector::default(); + for x in 0..7 { + vector.0.push(Item::new_from_element(DVec2::new(x as f64 * 10., 0.))); + } + let mut paths = List::new(); + vector_types::vector::push_path(&mut paths, CurveType::Cubic, [0, 1, 2, 3], []); + vector_types::vector::push_path(&mut paths, CurveType::Cubic, [1, 4, 5, 6], []); + + let anchors = vector.anchor_indices(&paths); + assert_eq!(anchors, [0, 1, 3, 6]); + super::apply_point_deltas(&mut vector, &paths, &anchors, &[DVec2::X, DVec2::Y, DVec2::X * 2., DVec2::X * 3.]); + + assert_eq!(vector.position(1), Some(DVec2::new(10., 1.))); + assert_eq!(vector.position(2), Some(DVec2::new(22., 0.))); + assert_eq!(vector.position(4), Some(DVec2::new(40., 1.))); + assert_eq!(vector.position(5), Some(DVec2::new(53., 0.))); + } + #[tokio::test] async fn offset_path_does_not_duplicate_closing_anchors() { // Offsetting closed triangles must not leave each subpath with a redundant start/end anchor (a Kurbo offset @@ -4121,6 +4191,45 @@ mod test { assert_eq!(smooth_anchors_by_contour(&spline), [[false, true, true, false]]); } + // A curved source's handles are replaced by the spline's own rather than kept alongside them + #[tokio::test] + async fn spline_replaces_the_handles_of_a_curved_source() { + let ellipse = vector_item_from_bezpath(ellipse_bezpath(DVec2::splat(-50.), DVec2::splat(50.))); + let anchors = ellipse.element().anchor_positions(item_paths(&ellipse)); + assert_eq!(anchors.len(), 4); + + let spline = super::spline(Footprint::default(), ellipse).await; + let paths = item_paths(&spline); + let spline = spline.element(); + + assert_eq!(spline.anchor_positions(paths), anchors); + assert_eq!(spline.segment_iter(paths).count(), 4); + assert_eq!(spline.point_count(), 4 + 4 * 2, "each of the four spans should hold two new handles and no old ones"); + } + + // Only the source's anchors become corners, joined by lines in their stored order + #[tokio::test] + async fn points_to_polyline_joins_the_anchors_with_lines() { + let ellipse = vector_item_from_bezpath(ellipse_bezpath(DVec2::splat(-50.), DVec2::splat(50.))); + let anchors = ellipse.element().anchor_positions(item_paths(&ellipse)); + + let closed = super::points_to_polyline(Footprint::default(), ellipse.clone(), item(true)).await; + let paths = item_paths(&closed); + let segments: Vec = closed.element().segment_iter(paths).map(|(_, segment, ..)| segment).collect(); + assert_eq!(closed.element().point_count(), anchors.len(), "the source's handles should be dropped"); + assert_eq!(closed.element().anchor_positions(paths), anchors); + assert_eq!(segments.len(), 4); + assert!(segments.iter().all(|segment| matches!(segment, PathSeg::Line(_)))); + assert!(closed.element().stroke_bezpath_iter(paths).all(|bezpath| bezpath.elements().last() == Some(&PathEl::ClosePath))); + + let open = super::points_to_polyline(Footprint::default(), ellipse, item(false)).await; + assert_eq!(open.element().segment_iter(item_paths(&open)).count(), 3); + + // Two points have nowhere to close back to, so they draw one segment either way + let pair = super::points_to_polyline(Footprint::default(), vector_item_from_points(&[DVec2::ZERO, DVec2::X]), item(true)).await; + assert_eq!(pair.element().segment_iter(item_paths(&pair)).count(), 1); + } + /// Whether each anchor keeps a smooth tangent pair, contour by contour. fn smooth_anchors_by_contour(item: &Item) -> Vec> { use vector_types::vector::misc::ManipulatorPointId;