diff --git a/editor/src/messages/portfolio/document/utility_types/transformation.rs b/editor/src/messages/portfolio/document/utility_types/transformation.rs index 34198b29..27c2567d 100644 --- a/editor/src/messages/portfolio/document/utility_types/transformation.rs +++ b/editor/src/messages/portfolio/document/utility_types/transformation.rs @@ -89,7 +89,7 @@ impl OriginalTransforms { let selected_handles = selected_points.iter().filter_map(|point| point.as_handle()); let anchor_ids = selected_points.iter().filter_map(|point| point.as_anchor()); - let connected_handles = anchor_ids.flat_map(|point| vector_data.segment_domain.all_connected(point)); + let connected_handles = anchor_ids.flat_map(|point| vector_data.all_connected(point)); let all_handles = selected_handles.chain(connected_handles); let handles = all_handles diff --git a/editor/src/messages/tool/common_functionality/shape_editor.rs b/editor/src/messages/tool/common_functionality/shape_editor.rs index 52bc3780..24f45a05 100644 --- a/editor/src/messages/tool/common_functionality/shape_editor.rs +++ b/editor/src/messages/tool/common_functionality/shape_editor.rs @@ -199,7 +199,7 @@ impl ShapeState { let mut point = SnapCandidatePoint::new_source(to_document.transform_point2(position) + mouse_delta, source); if let Some(id) = selected.as_anchor() { - for neighbor in vector_data.segment_domain.connected_points(id) { + for neighbor in vector_data.connected_points(id) { if state.is_selected(ManipulatorPointId::Anchor(neighbor)) { continue; } @@ -305,7 +305,7 @@ impl ShapeState { while let Some(point) = selected_stack.pop() { if !state.is_selected(ManipulatorPointId::Anchor(point)) { state.select_point(ManipulatorPointId::Anchor(point)); - selected_stack.extend(vector_data.segment_domain.connected_points(point)); + selected_stack.extend(vector_data.connected_points(point)); } } } @@ -399,7 +399,7 @@ impl ShapeState { }); // Move the other handle for a quadratic bezier - for segment in vector_data.segment_domain.end_connected(point) { + for segment in vector_data.end_connected(point) { let Some((start, _end, bezier)) = vector_data.segment_points_from_id(segment) else { continue }; if let BezierHandles::Quadratic { handle } = bezier.handles { @@ -476,10 +476,10 @@ impl ShapeState { let Some(anchor_position) = ManipulatorPointId::Anchor(point_id).get_position(vector_data) else { return; }; - let handles = vector_data.segment_domain.all_connected(point_id).take(2).collect::>(); + let handles = vector_data.all_connected(point_id).take(2).collect::>(); // Grab the next and previous manipulator groups by simply looking at the next / previous index - let points = handles.iter().map(|handle| vector_data.segment_domain.other_point(handle.segment, point_id)); + let points = handles.iter().map(|handle| vector_data.other_point(handle.segment, point_id)); let anchor_positions = points .map(|point| point.and_then(|point| ManipulatorPointId::Anchor(point).get_position(vector_data))) .collect::>(); @@ -556,7 +556,7 @@ impl ShapeState { let Some(anchor_id) = point.get_anchor(&vector_data) else { continue }; let Some(anchor) = vector_data.point_domain.position_from_id(anchor_id) else { continue }; - let anchor_points = handles.map(|handle| vector_data.segment_domain.other_point(handle.segment, anchor_id)); + let anchor_points = handles.map(|handle| vector_data.other_point(handle.segment, anchor_id)); let anchor_positions = anchor_points.map(|point| point.and_then(|point| vector_data.point_domain.position_from_id(point))); // If one handle is selected (but both exist), only move the other handle @@ -715,7 +715,7 @@ impl ShapeState { responses.add(GraphOperationMessage::Vector { layer, modification_type }); // Delete connected segments - for HandleId { segment, .. } in vector_data.segment_domain.all_connected(anchor) { + for HandleId { segment, .. } in vector_data.all_connected(anchor) { let modification_type = VectorModificationType::RemoveSegment { id: segment }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); for &handles in vector_data.colinear_manipulators.iter().filter(|handles| handles.iter().any(|handle| handle.segment == segment)) { @@ -736,14 +736,14 @@ impl ShapeState { } /// Dissolve the selected points. - pub fn delete_selected_points(&self, document: &DocumentMessageHandler, responses: &mut VecDeque) { - for (&layer, state) in &self.selected_shape_state { + pub fn delete_selected_points(&mut self, document: &DocumentMessageHandler, responses: &mut VecDeque) { + for (&layer, state) in &mut self.selected_shape_state { let mut missing_anchors = HashMap::new(); let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else { continue; }; - for &point in &state.selected_points { + for point in std::mem::take(&mut state.selected_points) { match point { ManipulatorPointId::Anchor(anchor) => { if let Some(handles) = Self::dissolve_anchor(anchor, responses, layer, &vector_data) { @@ -839,7 +839,7 @@ impl ShapeState { let Some(pos) = vector_data.point_domain.position_from_id(point) else { continue }; let mut used_initial_point = false; - for handle in vector_data.segment_domain.all_connected(point) { + for handle in vector_data.all_connected(point) { // Disable the g1 continuous for &handles in &vector_data.colinear_manipulators { if handles.contains(&handle) { @@ -874,13 +874,13 @@ impl ShapeState { } /// Delete point(s) and adjacent segments. - pub fn delete_point_and_break_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque) { - for (&layer, state) in &self.selected_shape_state { + pub fn delete_point_and_break_path(&mut self, document: &DocumentMessageHandler, responses: &mut VecDeque) { + for (&layer, state) in &mut self.selected_shape_state { let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else { continue; }; - for &delete in &state.selected_points { + for delete in std::mem::take(&mut state.selected_points) { let Some(point) = delete.get_anchor(&vector_data) else { continue }; // Delete point @@ -888,7 +888,7 @@ impl ShapeState { responses.add(GraphOperationMessage::Vector { layer, modification_type }); // Delete connected segments - for HandleId { segment, .. } in vector_data.segment_domain.all_connected(point) { + for HandleId { segment, .. } in vector_data.all_connected(point) { let modification_type = VectorModificationType::RemoveSegment { id: segment }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } @@ -905,7 +905,7 @@ impl ShapeState { for &point in &state.selected_points { if let ManipulatorPointId::Anchor(point) = point { - for connected in vector_data.segment_domain.all_connected(point) { + for connected in vector_data.all_connected(point) { if let Some(&handles) = vector_data.colinear_manipulators.iter().find(|target| target.iter().any(|&target| target == connected)) { let modification_type = VectorModificationType::SetG1Continuous { handles, enabled: false }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); @@ -1072,7 +1072,7 @@ impl ShapeState { } let (id, anchor) = result?; - let handles = vector_data.segment_domain.all_connected(id); + let handles = vector_data.all_connected(id); let mut positions = handles .filter_map(|handle| handle.to_manipulator_point().get_position(&vector_data)) .filter(|&handle| !anchor.abs_diff_eq(handle, 1e-5)); @@ -1083,7 +1083,7 @@ impl ShapeState { if already_sharp { self.convert_manipulator_handles_to_colinear(&vector_data, id, responses, layer); } else { - for handle in vector_data.segment_domain.all_connected(id) { + for handle in vector_data.all_connected(id) { let Some(bezier) = vector_data.segment_from_id(handle.segment) else { continue }; match bezier.handles { @@ -1141,13 +1141,13 @@ impl ShapeState { let Some(vector_data) = vector_data else { continue }; let transform = network_interface.document_metadata().transform_to_viewport(layer); - assert_eq!(vector_data.segment_domain.ids().len(), vector_data.segment_domain.start_point().len()); - assert_eq!(vector_data.segment_domain.ids().len(), vector_data.segment_domain.end_point().len()); - for start in vector_data.segment_domain.start_point() { - assert!(vector_data.point_domain.ids().contains(start)); + assert_eq!(vector_data.segment_domain.ids().len(), vector_data.start_point().count()); + assert_eq!(vector_data.segment_domain.ids().len(), vector_data.end_point().count()); + for start in vector_data.start_point() { + assert!(vector_data.point_domain.ids().contains(&start)); } - for end in vector_data.segment_domain.end_point() { - assert!(vector_data.point_domain.ids().contains(end)); + for end in vector_data.end_point() { + assert!(vector_data.point_domain.ids().contains(&end)); } for (id, bezier, _, _) in vector_data.segment_bezier_iter() { diff --git a/libraries/bezier-rs/src/subpath/core.rs b/libraries/bezier-rs/src/subpath/core.rs index 54269709..de3575fe 100644 --- a/libraries/bezier-rs/src/subpath/core.rs +++ b/libraries/bezier-rs/src/subpath/core.rs @@ -10,7 +10,7 @@ impl Subpath { /// A `Subpath` with less than 2 [ManipulatorGroup]s may not be closed. #[track_caller] pub fn new(manipulator_groups: Vec>, closed: bool) -> Self { - assert!(!closed || manipulator_groups.len() > 1, "A closed Subpath must contain more than 1 ManipulatorGroup."); + assert!(!closed || !manipulator_groups.is_empty(), "A closed Subpath must contain more than 0 ManipulatorGroups."); Self { manipulator_groups, closed } } diff --git a/node-graph/gcore/src/graphic_element/renderer.rs b/node-graph/gcore/src/graphic_element/renderer.rs index 481be53a..16f33d8a 100644 --- a/node-graph/gcore/src/graphic_element/renderer.rs +++ b/node-graph/gcore/src/graphic_element/renderer.rs @@ -364,9 +364,6 @@ impl GraphicElementRendered for VectorData { let transformed_bounds = self.bounding_box_with_transform(multiplied_transform).unwrap_or_default(); let mut path = String::new(); - for (_, subpath) in self.region_bezier_paths() { - let _ = subpath.subpath_to_svg(&mut path, multiplied_transform); - } for subpath in self.stroke_bezier_paths() { let _ = subpath.subpath_to_svg(&mut path, multiplied_transform); } @@ -431,10 +428,6 @@ impl GraphicElementRendered for VectorData { let kurbo_transform = kurbo::Affine::new(transform.to_cols_array()); let to_point = |p: DVec2| kurbo::Point::new(p.x, p.y); let mut path = kurbo::BezPath::new(); - // TODO: Is this correct and efficient? Deesn't this lead to us potentially rendering a path twice? - for (_, subpath) in self.region_bezier_paths() { - subpath.to_vello_path(self.transform, &mut path); - } for subpath in self.stroke_bezier_paths() { subpath.to_vello_path(self.transform, &mut path); } diff --git a/node-graph/gcore/src/vector/vector_data.rs b/node-graph/gcore/src/vector/vector_data.rs index ef04c1bf..7a0137bf 100644 --- a/node-graph/gcore/src/vector/vector_data.rs +++ b/node-graph/gcore/src/vector/vector_data.rs @@ -83,7 +83,7 @@ impl VectorData { point_id.next_id() }; self.point_domain.push(id, pair[0].anchor); - id + self.point_domain.ids().len() - 1 }); first_point = Some(first_point.unwrap_or(start)); let end = if preserve_id && !self.point_domain.ids().contains(&pair[1].id) { @@ -91,14 +91,15 @@ impl VectorData { } else { point_id.next_id() }; + let end_index = self.point_domain.ids().len(); self.point_domain.push(end, pair[1].anchor); let id = segment_id.next_id(); first_seg = Some(first_seg.unwrap_or(id)); last_seg = Some(id); - self.segment_domain.push(id, start, end, handles(&pair[0], &pair[1]), stroke_id); + self.segment_domain.push(id, start, end_index, handles(&pair[0], &pair[1]), stroke_id); - last_point = Some(end); + last_point = Some(end_index); } let fill_id = FillId::ZERO; @@ -169,9 +170,83 @@ impl VectorData { self.transform.transform_point2(self.layerspace_pivot(normalized_pivot)) } + pub fn start_point(&self) -> impl Iterator + '_ { + self.segment_domain.start_point().iter().map(|&index| self.point_domain.ids()[index]) + } + + pub fn end_point(&self) -> impl Iterator + '_ { + self.segment_domain.end_point().iter().map(|&index| self.point_domain.ids()[index]) + } + + pub fn push(&mut self, id: SegmentId, start: PointId, end: PointId, handles: bezier_rs::BezierHandles, stroke: StrokeId) { + let [Some(start), Some(end)] = [start, end].map(|id| self.point_domain.resolve_id(id)) else { + return; + }; + self.segment_domain.push(id, start, end, handles, stroke) + } + + pub fn handles_mut(&mut self) -> impl Iterator { + self.segment_domain + .handles_mut() + .map(|(id, handles, start, end)| (id, handles, self.point_domain.ids()[start], self.point_domain.ids()[end])) + } + + pub fn segment_start_from_id(&self, segment: SegmentId) -> Option { + self.segment_domain.segment_start_from_id(segment).map(|index| self.point_domain.ids()[index]) + } + + pub fn segment_end_from_id(&self, segment: SegmentId) -> Option { + self.segment_domain.segment_end_from_id(segment).map(|index| self.point_domain.ids()[index]) + } + + /// Returns an array for the start and end points of a segment. + pub fn points_from_id(&self, segment: SegmentId) -> Option<[PointId; 2]> { + self.segment_domain.points_from_id(segment).map(|val| val.map(|index| self.point_domain.ids()[index])) + } + + /// Attempts to find another point in the segment that is not the one passed in. + pub fn other_point(&self, segment: SegmentId, current: PointId) -> Option { + let index = self.point_domain.resolve_id(current); + index.and_then(|index| self.segment_domain.other_point(segment, index)).map(|index| self.point_domain.ids()[index]) + } + + /// Gets all points connected to the current one but not including the current one. + pub fn connected_points(&self, current: PointId) -> impl Iterator + '_ { + let index = [self.point_domain.resolve_id(current)].into_iter().flatten(); + index.flat_map(|index| self.segment_domain.connected_points(index).map(|index| self.point_domain.ids()[index])) + } + + /// Enumerate all segments that start at the point. + pub fn start_connected(&self, point: PointId) -> impl Iterator + '_ { + let index = [self.point_domain.resolve_id(point)].into_iter().flatten(); + index.flat_map(|index| self.segment_domain.start_connected(index)) + } + + /// Enumerate all segments that end at the point. + pub fn end_connected(&self, point: PointId) -> impl Iterator + '_ { + let index = [self.point_domain.resolve_id(point)].into_iter().flatten(); + index.flat_map(|index| self.segment_domain.end_connected(index)) + } + + /// Enumerate all segments that start or end at a point, converting them to [`HandleId`s]. Note that the handles may not exist e.g. for a linear segment. + pub fn all_connected(&self, point: PointId) -> impl Iterator + '_ { + let index = [self.point_domain.resolve_id(point)].into_iter().flatten(); + index.flat_map(|index| self.segment_domain.all_connected(index)) + } + + /// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice. + pub fn connected_count(&self, point: PointId) -> usize { + self.point_domain.resolve_id(point).map_or(0, |point| self.segment_domain.connected_count(point)) + } + /// Points connected to a single segment pub fn single_connected_points(&self) -> impl Iterator + '_ { - self.point_domain.ids().iter().copied().filter(|&point| self.segment_domain.connected_count(point) == 1) + self.point_domain + .ids() + .iter() + .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. @@ -179,7 +254,7 @@ impl VectorData { let has_handle = |target| self.colinear_manipulators.iter().flatten().any(|&handle| handle == target); match point { ManipulatorPointId::Anchor(id) => { - self.segment_domain.start_connected(id).all(|segment| has_handle(HandleId::primary(segment))) && self.segment_domain.end_connected(id).all(|segment| has_handle(HandleId::end(segment))) + self.start_connected(id).all(|segment| has_handle(HandleId::primary(segment))) && self.end_connected(id).all(|segment| has_handle(HandleId::end(segment))) } ManipulatorPointId::PrimaryHandle(segment) => has_handle(HandleId::primary(segment)), ManipulatorPointId::EndHandle(segment) => has_handle(HandleId::end(segment)), @@ -218,6 +293,7 @@ pub enum ManipulatorPointId { impl ManipulatorPointId { /// Attempt to retrieve the manipulator position in layer space (no transformation applied). #[must_use] + #[track_caller] pub fn get_position(&self, vector_data: &VectorData) -> Option { match self { ManipulatorPointId::Anchor(id) => vector_data.point_domain.position_from_id(*id), @@ -230,7 +306,7 @@ impl ManipulatorPointId { #[must_use] pub fn get_handle_pair(self, vector_data: &VectorData) -> Option<[HandleId; 2]> { match self { - ManipulatorPointId::Anchor(point) => vector_data.segment_domain.all_connected(point).take(2).collect::>().try_into().ok(), + ManipulatorPointId::Anchor(point) => vector_data.all_connected(point).take(2).collect::>().try_into().ok(), ManipulatorPointId::PrimaryHandle(segment) => { let point = vector_data.segment_domain.segment_start_from_id(segment)?; let current = HandleId::primary(segment); @@ -251,8 +327,8 @@ impl ManipulatorPointId { pub fn get_anchor(self, vector_data: &VectorData) -> Option { match self { ManipulatorPointId::Anchor(point) => Some(point), - ManipulatorPointId::PrimaryHandle(segment) => vector_data.segment_domain.segment_start_from_id(segment), - ManipulatorPointId::EndHandle(segment) => vector_data.segment_domain.segment_end_from_id(segment), + ManipulatorPointId::PrimaryHandle(segment) => vector_data.segment_start_from_id(segment), + ManipulatorPointId::EndHandle(segment) => vector_data.segment_end_from_id(segment), } } diff --git a/node-graph/gcore/src/vector/vector_data/attributes.rs b/node-graph/gcore/src/vector/vector_data/attributes.rs index 5859d7de..b376566d 100644 --- a/node-graph/gcore/src/vector/vector_data/attributes.rs +++ b/node-graph/gcore/src/vector/vector_data/attributes.rs @@ -95,9 +95,26 @@ impl PointDomain { self.positions.clear(); } - pub fn retain(&mut self, f: impl Fn(&PointId) -> bool) { + pub fn retain(&mut self, segment_domain: &mut SegmentDomain, f: impl Fn(&PointId) -> bool) { let mut keep = self.id.iter().map(&f); self.positions.retain(|_| keep.next().unwrap_or_default()); + + // TODO(TrueDoctor): Consider using a prefix sum to avoid this Vec allocation (https://github.com/GraphiteEditor/Graphite/pull/1949#discussion_r1741711562) + let mut id_map = Vec::with_capacity(self.ids().len()); + let mut new_index = 0; + for id in self.ids() { + if f(id) { + id_map.push(new_index); + new_index += 1; + } else { + id_map.push(usize::MAX); // A placeholder for invalid ids. This is checked after the segment domain is modified. + } + } + + let update_index = |index: &mut usize| *index = id_map[*index]; + segment_domain.start_point.iter_mut().for_each(update_index); + segment_domain.end_point.iter_mut().for_each(update_index); + self.id.retain(f); } @@ -126,6 +143,7 @@ impl PointDomain { self.ids().iter().copied().max_by(|a, b| a.0.cmp(&b.0)).map(|mut id| id.next_id()).unwrap_or(PointId::ZERO) } + #[track_caller] pub fn position_from_id(&self, id: PointId) -> Option { let pos = self.resolve_id(id).map(|index| self.positions[index]); if pos.is_none() { @@ -134,7 +152,7 @@ impl PointDomain { pos } - fn resolve_id(&self, id: PointId) -> Option { + pub(crate) fn resolve_id(&self, id: PointId) -> Option { self.id.iter().position(|&check_id| check_id == id) } @@ -155,8 +173,8 @@ impl PointDomain { /// Stores data which is per-segment. A segment is a bézier curve between two end points with a stroke. In future this will be extendable at runtime with custom attributes. pub struct SegmentDomain { ids: Vec, - start_point: Vec, - end_point: Vec, + start_point: Vec, + end_point: Vec, handles: Vec, stroke: Vec, } @@ -200,11 +218,11 @@ impl SegmentDomain { self.ids().iter().copied().max_by(|a, b| a.0.cmp(&b.0)).map(|mut id| id.next_id()).unwrap_or(SegmentId::ZERO) } - pub fn start_point(&self) -> &[PointId] { + pub(crate) fn start_point(&self) -> &[usize] { &self.start_point } - pub fn end_point(&self) -> &[PointId] { + pub(crate) fn end_point(&self) -> &[usize] { &self.end_point } @@ -216,35 +234,34 @@ impl SegmentDomain { &self.stroke } - pub fn push(&mut self, id: SegmentId, start: PointId, end: PointId, handles: bezier_rs::BezierHandles, stroke: StrokeId) { + pub(crate) fn push(&mut self, id: SegmentId, start: usize, end: usize, handles: bezier_rs::BezierHandles, stroke: StrokeId) { if self.ids.contains(&id) { - warn!("Duplicate segment"); return; } // Attempt to keep line joins? - let after = self.end_point.iter().copied().position(|other_end| other_end == start || other_end == end); - let before = self.start_point.iter().copied().position(|other_start| other_start == start || other_start == end); - let (index, flip) = match (before, after) { - (_, Some(after)) => (after + 1, self.end_point[after] == end), - (Some(before), _) => (before, self.start_point[before] == start), - (None, None) => (self.ids.len(), false), + let after = self.end_point.iter().copied().position(|other_end| other_end == start); + let before = self.start_point.iter().copied().position(|other_start| other_start == end); + let index = match (before, after) { + (_, Some(after)) => after + 1, + (Some(before), _) => before, + (None, None) => self.ids.len(), }; self.ids.insert(index, id); - self.start_point.insert(index, if flip { end } else { start }); - self.end_point.insert(index, if flip { start } else { end }); - self.handles.insert(index, if flip { handles.flipped() } else { handles }); + self.start_point.insert(index, start); + self.end_point.insert(index, end); + self.handles.insert(index, handles); self.stroke.insert(index, stroke); } - pub fn start_point_mut(&mut self) -> impl Iterator { + pub(crate) fn start_point_mut(&mut self) -> impl Iterator { self.ids.iter().copied().zip(self.start_point.iter_mut()) } - pub fn end_point_mut(&mut self) -> impl Iterator { + pub(crate) fn end_point_mut(&mut self) -> impl Iterator { self.ids.iter().copied().zip(self.end_point.iter_mut()) } - pub fn handles_mut(&mut self) -> impl Iterator { + pub(crate) fn handles_mut(&mut self) -> impl Iterator { let nested = self.ids.iter().zip(&mut self.handles).zip(&self.start_point).zip(&self.end_point); nested.map(|(((&a, b), &c), &d)| (a, b, c, d)) } @@ -253,26 +270,26 @@ impl SegmentDomain { self.ids.iter().copied().zip(self.stroke.iter_mut()) } - pub fn segment_start_from_id(&self, segment: SegmentId) -> Option { + pub(crate) fn segment_start_from_id(&self, segment: SegmentId) -> Option { self.id_to_index(segment).and_then(|index| self.start_point.get(index)).copied() } - pub fn segment_end_from_id(&self, segment: SegmentId) -> Option { + pub(crate) fn segment_end_from_id(&self, segment: SegmentId) -> Option { self.id_to_index(segment).and_then(|index| self.end_point.get(index)).copied() } /// Returns an array for the start and end points of a segment. - pub fn points_from_id(&self, segment: SegmentId) -> Option<[PointId; 2]> { + pub(crate) fn points_from_id(&self, segment: SegmentId) -> Option<[usize; 2]> { self.segment_start_from_id(segment).and_then(|start| self.segment_end_from_id(segment).map(|end| [start, end])) } /// Attempts to find another point in the segment that is not the one passed in. - pub fn other_point(&self, segment: SegmentId, current: PointId) -> Option { + pub(crate) fn other_point(&self, segment: SegmentId, current: usize) -> Option { self.points_from_id(segment).and_then(|points| points.into_iter().find(|&point| point != current)) } /// Gets all points connected to the current one but not including the current one. - pub fn connected_points(&self, current: PointId) -> impl Iterator + '_ { + pub(crate) fn connected_points(&self, current: usize) -> impl Iterator + '_ { self.start_point.iter().zip(&self.end_point).filter_map(move |(&a, &b)| match (a == current, b == current) { (true, false) => Some(b), (false, true) => Some(a), @@ -299,8 +316,8 @@ impl SegmentDomain { fn concat(&mut self, other: &Self, transform: DAffine2, id_map: &IdMap) { self.ids.extend(other.ids.iter().map(|id| *id_map.segment_map.get(id).unwrap_or(id))); - self.start_point.extend(other.start_point.iter().map(|id| *id_map.point_map.get(id).unwrap_or(id))); - self.end_point.extend(other.end_point.iter().map(|id| *id_map.point_map.get(id).unwrap_or(id))); + self.start_point.extend(other.start_point.iter().map(|&index| id_map.point_offset + index)); + self.end_point.extend(other.end_point.iter().map(|&index| id_map.point_offset + index)); self.handles.extend(other.handles.iter().map(|handles| handles.apply_transformation(|p| transform.transform_point2(p)))); self.stroke.extend(&other.stroke); } @@ -312,22 +329,22 @@ impl SegmentDomain { } /// Enumerate all segments that start at the point. - pub fn start_connected(&self, point: PointId) -> impl Iterator + '_ { + pub(crate) fn start_connected(&self, point: usize) -> impl Iterator + '_ { self.start_point.iter().zip(&self.ids).filter(move |&(&found_point, _)| found_point == point).map(|(_, &seg)| seg) } /// Enumerate all segments that end at the point. - pub fn end_connected(&self, point: PointId) -> impl Iterator + '_ { + pub(crate) fn end_connected(&self, point: usize) -> impl Iterator + '_ { self.end_point.iter().zip(&self.ids).filter(move |&(&found_point, _)| found_point == point).map(|(_, &seg)| seg) } /// Enumerate all segments that start or end at a point, converting them to [`HandleId`s]. Note that the handles may not exist e.g. for a linear segment. - pub fn all_connected(&self, point: PointId) -> impl Iterator + '_ { + pub(crate) fn all_connected(&self, point: usize) -> impl Iterator + '_ { self.start_connected(point).map(HandleId::primary).chain(self.end_connected(point).map(HandleId::end)) } /// Enumerate the number of segments connected to a point. If a segment starts and ends at a point then it is counted twice. - pub fn connected_count(&self, point: PointId) -> usize { + pub(crate) fn connected_count(&self, point: usize) -> usize { self.all_connected(point).count() } } @@ -415,11 +432,11 @@ impl RegionDomain { } impl super::VectorData { - /// Construct a [`bezier_rs::Bezier`] curve spanning from the resolved position of the start and end points with the specified handles. Returns [`None`] if either ID is invalid. - fn segment_to_bezier(&self, start: PointId, end: PointId, handles: bezier_rs::BezierHandles) -> Option { - let start = self.point_domain.position_from_id(start)?; - let end = self.point_domain.position_from_id(end)?; - Some(bezier_rs::Bezier { start, end, handles }) + /// Construct a [`bezier_rs::Bezier`] curve spanning from the resolved position of the start and end points with the specified handles. + fn segment_to_bezier_with_index(&self, start: usize, end: usize, handles: bezier_rs::BezierHandles) -> bezier_rs::Bezier { + let start = self.point_domain.positions()[start]; + let end = self.point_domain.positions()[end]; + bezier_rs::Bezier { start, end, handles } } /// Tries to convert a segment with the specified id to a [`bezier_rs::Bezier`], returning None if the id is invalid. @@ -432,26 +449,28 @@ impl super::VectorData { let index: usize = self.segment_domain.id_to_index(id)?; let start = self.segment_domain.start_point[index]; let end = self.segment_domain.end_point[index]; - Some((start, end, self.segment_to_bezier(start, end, self.segment_domain.handles[index])?)) + let start_id = self.point_domain.ids()[start]; + let end_id = self.point_domain.ids()[end]; + Some((start_id, end_id, self.segment_to_bezier_with_index(start, end, self.segment_domain.handles[index]))) } /// Iterator over all of the [`bezier_rs::Bezier`] following the order that they are stored in the segment domain, skipping invalid segments. pub fn segment_bezier_iter(&self) -> impl Iterator + '_ { - let to_bezier = |(((&handles, &id), &start), &end)| self.segment_to_bezier(start, end, handles).map(|bezier| (id, bezier, start, end)); + let to_bezier = |(((&handles, &id), &start), &end)| (id, self.segment_to_bezier_with_index(start, end, handles), self.point_domain.ids()[start], self.point_domain.ids()[end]); self.segment_domain .handles .iter() .zip(&self.segment_domain.ids) - .zip(&self.segment_domain.start_point) - .zip(&self.segment_domain.end_point) - .filter_map(to_bezier) + .zip(self.segment_domain.start_point()) + .zip(self.segment_domain.end_point()) + .map(to_bezier) } /// Construct a [`bezier_rs::Bezier`] curve from an iterator of segments with (handles, start point, end point). Returns None if any ids are invalid or if the segments are not continuous. - fn subpath_from_segments(&self, segments: impl Iterator) -> Option> { + fn subpath_from_segments(&self, segments: impl Iterator) -> Option> { let mut first_point = None; let mut groups = Vec::new(); - let mut last: Option<(PointId, bezier_rs::BezierHandles)> = None; + let mut last: Option<(usize, bezier_rs::BezierHandles)> = None; for (handle, start, end) in segments { if last.is_some_and(|(previous_end, _)| previous_end != start) { @@ -461,10 +480,10 @@ impl super::VectorData { first_point = Some(first_point.unwrap_or(start)); groups.push(bezier_rs::ManipulatorGroup { - anchor: self.point_domain.position_from_id(start)?, + anchor: self.point_domain.positions()[start], in_handle: last.and_then(|(_, handle)| handle.end()), out_handle: handle.start(), - id: start, + id: self.point_domain.ids()[start], }); last = Some((end, handle)); @@ -477,10 +496,10 @@ impl super::VectorData { groups[0].in_handle = last_handle.end(); } else { groups.push(bezier_rs::ManipulatorGroup { - anchor: self.point_domain.position_from_id(end)?, + anchor: self.point_domain.positions()[end], in_handle: last_handle.end(), out_handle: None, - id: end, + id: self.point_domain.ids()[end], }); } } @@ -510,7 +529,18 @@ impl super::VectorData { /// Construct a [`bezier_rs::Bezier`] curve for stroke. pub fn stroke_bezier_paths(&self) -> StrokePathIter<'_> { - StrokePathIter { vector_data: self, segment_index: 0 } + let mut points = vec![StrokePathIterPointMetadata::default(); self.point_domain.ids().len()]; + for (segment_index, (&start, &end)) in self.segment_domain.start_point.iter().zip(&self.segment_domain.end_point).enumerate() { + points[start].set(StrokePathIterPointSegmentMetadata::new(segment_index, false)); + points[end].set(StrokePathIterPointSegmentMetadata::new(segment_index, true)); + } + + StrokePathIter { + vector_data: self, + points, + skip: 0, + done_one: false, + } } /// Construct an iterator [`bezier_rs::ManipulatorGroup`] for stroke. @@ -531,37 +561,121 @@ impl super::VectorData { } } +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +struct StrokePathIterPointSegmentMetadata { + segment_index: usize, + start_from_end: bool, +} + +impl StrokePathIterPointSegmentMetadata { + #[must_use] + const fn new(segment_index: usize, start_from_end: bool) -> Self { + Self { segment_index, start_from_end } + } + #[must_use] + const fn flipped(&self) -> Self { + Self { + segment_index: self.segment_index, + start_from_end: !self.start_from_end, + } + } +} + +#[derive(Clone, Default)] +struct StrokePathIterPointMetadata([Option; 2]); + +impl StrokePathIterPointMetadata { + fn set(&mut self, value: StrokePathIterPointSegmentMetadata) { + if self.0[0].is_none() { + self.0[0] = Some(value) + } else if self.0[1].is_none() { + self.0[1] = Some(value); + } else { + panic!("Mesh networks are not supported"); + } + } + #[must_use] + fn connected(&self) -> usize { + self.0.iter().filter(|val| val.is_some()).count() + } + #[must_use] + fn take_first(&mut self) -> Option { + self.0[0].take().or_else(|| self.0[1].take()) + } + fn take_eq(&mut self, target: StrokePathIterPointSegmentMetadata) -> bool { + self.0[0].take_if(|&mut value| value == target).or_else(|| self.0[1].take_if(|&mut value| value == target)).is_some() + } +} + #[derive(Clone)] pub struct StrokePathIter<'a> { vector_data: &'a super::VectorData, - segment_index: usize, + points: Vec, + skip: usize, + done_one: bool, } impl<'a> Iterator for StrokePathIter<'a> { type Item = bezier_rs::Subpath; fn next(&mut self) -> Option { - let segments = &self.vector_data.segment_domain; - if self.segment_index >= segments.end_point.len() { - return None; - } - let mut old_end = None; - let mut count = 0; - let segments_iter = segments.handles[self.segment_index..] - .iter() - .zip(&segments.start_point[self.segment_index..]) - .zip(&segments.end_point[self.segment_index..]) - .map(|((&handles, &start), &end)| (handles, start, end)) - .take_while(|&(_, start, end)| { - let continuous = old_end.is_none() || old_end.is_some_and(|old_end| old_end == start); - old_end = Some(end); - continuous - }) - .inspect(|_| count += 1); + let current_start = if let Some((index, _)) = self.points.iter().enumerate().skip(self.skip).find(|(_, val)| val.connected() == 1) { + index + } else { + if !self.done_one { + self.done_one = true; + self.skip = 0; + } + self.points.iter().enumerate().skip(self.skip).find(|(_, val)| val.connected() > 0)?.0 + }; + self.skip = current_start + 1; - let subpath = self.vector_data.subpath_from_segments(segments_iter); - self.segment_index += count; - subpath + // There will always be one (seeing as we checked above) + let mut point_index = current_start; + let mut groups = Vec::new(); + let mut in_handle = None; + let mut closed = false; + loop { + let Some(val) = self.points[point_index].take_first() else { + // Dead end + groups.push(bezier_rs::ManipulatorGroup { + anchor: self.vector_data.point_domain.positions()[point_index], + in_handle, + out_handle: None, + id: self.vector_data.point_domain.ids()[point_index], + }); + + break; + }; + + let mut handles = self.vector_data.segment_domain.handles()[val.segment_index]; + if val.start_from_end { + handles = handles.flipped(); + } + let next_point_index = if val.start_from_end { + self.vector_data.segment_domain.start_point()[val.segment_index] + } else { + self.vector_data.segment_domain.end_point()[val.segment_index] + }; + groups.push(bezier_rs::ManipulatorGroup { + anchor: self.vector_data.point_domain.positions()[point_index], + in_handle, + out_handle: handles.start(), + id: self.vector_data.point_domain.ids()[point_index], + }); + + in_handle = handles.end(); + + point_index = next_point_index; + self.points[next_point_index].take_eq(val.flipped()); + if next_point_index == current_start { + closed = true; + groups[0].in_handle = in_handle; + break; + } + } + + Some(bezier_rs::Subpath::new(groups, closed)) } } @@ -584,7 +698,12 @@ impl crate::vector::ConcatElement for super::VectorData { let segment_map = new_ids.collect::>(); let new_ids = other.region_domain.ids.iter().filter(|id| self.region_domain.ids.contains(id)).map(|&old| (old, RegionId::generate())); let region_map = new_ids.collect::>(); - let id_map = IdMap { point_map, segment_map, region_map }; + let id_map = IdMap { + point_offset: self.point_domain.ids().len(), + point_map, + segment_map, + region_map, + }; self.point_domain.concat(&other.point_domain, transform * other.transform, &id_map); self.segment_domain.concat(&other.segment_domain, transform * other.transform, &id_map); self.region_domain.concat(&other.region_domain, transform * other.transform, &id_map); @@ -597,6 +716,7 @@ impl crate::vector::ConcatElement for super::VectorData { /// Represents the conversion of ids used when concatenating vector data with conflicting ids. struct IdMap { + point_offset: usize, point_map: HashMap, segment_map: HashMap, region_map: HashMap, diff --git a/node-graph/gcore/src/vector/vector_data/modification.rs b/node-graph/gcore/src/vector/vector_data/modification.rs index 41dcadd1..b3eb23ed 100644 --- a/node-graph/gcore/src/vector/vector_data/modification.rs +++ b/node-graph/gcore/src/vector/vector_data/modification.rs @@ -27,9 +27,9 @@ impl Hash for PointModification { impl PointModification { /// Apply this modification to the specified [`PointDomain`]. pub fn apply(&self, point_domain: &mut PointDomain, segment_domain: &mut SegmentDomain) { - point_domain.retain(|id| !self.remove.contains(id)); + point_domain.retain(segment_domain, |id| !self.remove.contains(id)); - for (id, position) in point_domain.positions_mut() { + for (index, (id, position)) in point_domain.positions_mut().enumerate() { let Some(&delta) = self.delta.get(&id) else { continue }; if !delta.is_finite() { warn!("Invalid delta when applying a point modification"); @@ -39,10 +39,10 @@ impl PointModification { *position += delta; for (_, handles, start, end) in segment_domain.handles_mut() { - if start == id { + if start == index { handles.move_start(delta); } - if end == id { + if end == index { handles.move_end(delta); } } @@ -105,27 +105,27 @@ impl SegmentModification { for (id, point) in segment_domain.start_point_mut() { let Some(&new) = self.start_point.get(&id) else { continue }; - if !point_domain.ids().contains(&new) { + let Some(index) = point_domain.resolve_id(new) else { warn!("Invalid start ID when applying a segment modification"); continue; - } + }; - *point = new; + *point = index; } for (id, point) in segment_domain.end_point_mut() { let Some(&new) = self.end_point.get(&id) else { continue }; - if !point_domain.ids().contains(&new) { + let Some(index) = point_domain.resolve_id(new) else { warn!("Invalid end ID when applying a segment modification"); continue; - } + }; - *point = new; + *point = index; } for (id, handles, start, end) in segment_domain.handles_mut() { - let Some(start) = point_domain.position_from_id(start) else { continue }; - let Some(end) = point_domain.position_from_id(end) else { continue }; + let Some(&start) = point_domain.positions().get(start) else { continue }; + let Some(&end) = point_domain.positions().get(end) else { continue }; // Compute the actual start and end position based on the offset from the anchor let start = self.handle_primary.get(&id).copied().map(|handle| handle.map(|handle| handle + start)); @@ -178,17 +178,17 @@ impl SegmentModification { let Some(&handle_end) = self.handle_end.get(&add_id) else { continue }; let Some(&stroke) = self.stroke.get(&add_id) else { continue }; - if !point_domain.ids().contains(&start) { + let Some(start_index) = point_domain.resolve_id(start) else { warn!("invalid start id"); continue; - } - if !point_domain.ids().contains(&end) { + }; + let Some(end_index) = point_domain.resolve_id(end) else { warn!("invalid end id"); continue; - } + }; - let Some(start_position) = point_domain.position_from_id(start) else { continue }; - let Some(end_position) = point_domain.position_from_id(end) else { continue }; + let start_position = point_domain.positions()[start_index]; + let end_position = point_domain.positions()[end_index]; let handles = match (handle_start, handle_end) { (Some(handle_start), Some(handle_end)) => BezierHandles::Cubic { handle_start: handle_start + start_position, @@ -203,17 +203,21 @@ impl SegmentModification { continue; } - segment_domain.push(add_id, start, end, handles, stroke); + segment_domain.push(add_id, start_index, end_index, handles, stroke); } + + assert!(segment_domain.start_point().iter().all(|&index| index < point_domain.ids().len()), "index should be in range"); + assert!(segment_domain.end_point().iter().all(|&index| index < point_domain.ids().len()), "index should be in range"); } /// Create a new modification that will convert an empty [`VectorData`] into the target [`VectorData`]. pub fn create_from_vector(vector_data: &VectorData) -> Self { + let point_id = |(&segment, &index)| (segment, vector_data.point_domain.ids()[index]); Self { add: vector_data.segment_domain.ids().to_vec(), remove: HashSet::new(), - start_point: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.start_point().iter().cloned()).collect(), - end_point: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.end_point().iter().cloned()).collect(), + start_point: vector_data.segment_domain.ids().iter().zip(vector_data.segment_domain.start_point()).map(point_id).collect(), + end_point: vector_data.segment_domain.ids().iter().zip(vector_data.segment_domain.end_point()).map(point_id).collect(), handle_primary: vector_data.segment_bezier_iter().map(|(id, b, _, _)| (id, b.handle_start().map(|handle| handle - b.start))).collect(), handle_end: vector_data.segment_bezier_iter().map(|(id, b, _, _)| (id, b.handle_end().map(|handle| handle - b.end))).collect(), stroke: vector_data.segment_domain.ids().iter().copied().zip(vector_data.segment_domain.stroke().iter().cloned()).collect(), diff --git a/node-graph/gcore/src/vector/vector_nodes.rs b/node-graph/gcore/src/vector/vector_nodes.rs index 251ec8ef..556499bf 100644 --- a/node-graph/gcore/src/vector/vector_nodes.rs +++ b/node-graph/gcore/src/vector/vector_nodes.rs @@ -520,9 +520,7 @@ fn splines_from_points(mut vector_data: VectorData) -> VectorData { let handle_end = points.positions()[end_index] * 2. - first_handles[end_index]; let handles = bezier_rs::BezierHandles::Cubic { handle_start, handle_end }; - vector_data - .segment_domain - .push(SegmentId::generate(), points.ids()[start_index], points.ids()[end_index], handles, stroke_id) + vector_data.segment_domain.push(SegmentId::generate(), start_index, end_index, handles, stroke_id) } vector_data