From d5bb1396ec24a8b620fe18d05820da52342df246 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 15 Sep 2026 22:36:59 +0200 Subject: [PATCH] Delete the vestigial region domain, reverting to naive logic for mesh region fill determination (#4463) --- .../document/data_panel/data_panel_message.rs | 1 - .../data_panel/data_panel_message_handler.rs | 11 +- .../graphic-types/src/graphic/glue.rs | 6 +- node-graph/libraries/graphic-types/src/lib.rs | 4 +- .../libraries/rendering/src/renderer.rs | 1 - .../vector/algorithms/merge_by_distance.rs | 4 - .../src/vector/vector_attributes.rs | 123 +----------------- .../src/vector/vector_modification.rs | 68 +--------- .../vector-types/src/vector/vector_types.rs | 30 +---- node-graph/nodes/gstd/src/lib.rs | 2 +- .../nodes/vector/src/generator_nodes.rs | 7 +- node-graph/nodes/vector/src/vector_nodes.rs | 74 ++++------- 12 files changed, 38 insertions(+), 293 deletions(-) diff --git a/editor/src/messages/portfolio/document/data_panel/data_panel_message.rs b/editor/src/messages/portfolio/document/data_panel/data_panel_message.rs index cf35abca53..9efc67eba8 100644 --- a/editor/src/messages/portfolio/document/data_panel/data_panel_message.rs +++ b/editor/src/messages/portfolio/document/data_panel/data_panel_message.rs @@ -39,6 +39,5 @@ pub enum VectorTableTab { #[default] Points, Segments, - Regions, Handles, } diff --git a/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs b/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs index 248d6b8259..07f07c4fe1 100644 --- a/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs +++ b/editor/src/messages/portfolio/document/data_panel/data_panel_message_handler.rs @@ -585,7 +585,7 @@ impl TableItemLayout for Vector { ) } fn value_page(&self, data: &mut LayoutData) -> Vec { - let table_tab_entries = [VectorTableTab::Points, VectorTableTab::Segments, VectorTableTab::Regions, VectorTableTab::Handles] + let table_tab_entries = [VectorTableTab::Points, VectorTableTab::Segments, VectorTableTab::Handles] .into_iter() .map(|tab| { RadioEntryData::new(format!("{tab:?}")) @@ -618,15 +618,6 @@ impl TableItemLayout for Vector { ] })); } - VectorTableTab::Regions => { - table_rows.push(column_headings(&["", "segment_range"])); - table_rows.extend(self.region_domain.iter().map(|(id, segment_range)| { - vec![ - TextLabel::new(format!("{}", id.inner())).narrow(true).widget_instance(), - TextLabel::new(format!("{segment_range:?}")).narrow(true).widget_instance(), - ] - })); - } VectorTableTab::Handles => { table_rows.push(column_headings(&["", "colinear_manipulators[0]", "colinear_manipulators[1]"])); table_rows.extend(self.colinear_manipulators.iter().enumerate().map(|(index, [a, b])| { diff --git a/node-graph/libraries/graphic-types/src/graphic/glue.rs b/node-graph/libraries/graphic-types/src/graphic/glue.rs index 311f2cfacd..7b41026e9e 100644 --- a/node-graph/libraries/graphic-types/src/graphic/glue.rs +++ b/node-graph/libraries/graphic-types/src/graphic/glue.rs @@ -224,11 +224,7 @@ fn graphic_retained_heap(graphic: &Graphic<'_>) -> usize { /// The heap a vector's domain columns own, summed over the columns it /// exposes, so the segment domain's private parallel columns are undercounted. fn vector_retained_heap(vector: &Vector) -> usize { - size_of_val(vector.point_domain.ids()) - + size_of_val(vector.point_domain.positions()) - + size_of_val(vector.segment_domain.ids()) - + size_of_val(vector.region_domain.ids()) - + size_of_val(vector.colinear_manipulators.as_slice()) + size_of_val(vector.point_domain.ids()) + size_of_val(vector.point_domain.positions()) + size_of_val(vector.segment_domain.ids()) + size_of_val(vector.colinear_manipulators.as_slice()) } /// Whether any group is reachable from the graphic, so it does not own all of diff --git a/node-graph/libraries/graphic-types/src/lib.rs b/node-graph/libraries/graphic-types/src/lib.rs index ed817c5e86..902b8c019b 100644 --- a/node-graph/libraries/graphic-types/src/lib.rs +++ b/node-graph/libraries/graphic-types/src/lib.rs @@ -27,7 +27,7 @@ pub mod migrations { use core_types::Color; use dyn_any::DynAny; use glam::{DAffine2, DVec2}; - use vector_types::vector::{PointDomain, RegionDomain, SegmentDomain, misc::HandleId, style::Stroke}; + use vector_types::vector::{PointDomain, SegmentDomain, misc::HandleId, style::Stroke}; use vector_types::{GradientRamp, Vector, vector}; #[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash, DynAny, serde::Serialize, serde::Deserialize)] @@ -114,7 +114,6 @@ pub mod migrations { pub colinear_manipulators: Vec<[HandleId; 2]>, pub point_domain: PointDomain, pub segment_domain: SegmentDomain, - pub region_domain: RegionDomain, } #[derive(serde::Deserialize)] @@ -145,7 +144,6 @@ pub mod migrations { colinear_manipulators: old.colinear_manipulators, point_domain: old.point_domain, segment_domain: old.segment_domain, - region_domain: old.region_domain, }), VectorFormat::Vector(vector) => Some(vector), VectorFormat::List(list) => list.element.into_iter().next(), diff --git a/node-graph/libraries/rendering/src/renderer.rs b/node-graph/libraries/rendering/src/renderer.rs index dfcfe68645..8457670354 100644 --- a/node-graph/libraries/rendering/src/renderer.rs +++ b/node-graph/libraries/rendering/src/renderer.rs @@ -1865,7 +1865,6 @@ fn render_vector_item_vello>( } }; - // Branching vectors without regions (e.g. mesh grids) need face-by-face fill rendering. let use_face_fill = element.use_face_fill(); let do_fill = |scene: &mut Scene, context: &mut RenderContext| { if use_face_fill { diff --git a/node-graph/libraries/vector-types/src/vector/algorithms/merge_by_distance.rs b/node-graph/libraries/vector-types/src/vector/algorithms/merge_by_distance.rs index 63271baa9d..e4cf8f25a5 100644 --- a/node-graph/libraries/vector-types/src/vector/algorithms/merge_by_distance.rs +++ b/node-graph/libraries/vector-types/src/vector/algorithms/merge_by_distance.rs @@ -94,10 +94,6 @@ impl MergeByDistanceExt for Vector { points_to_delete.extend(collapse_set) } - // Remove faces whose start or end segments are removed - // TODO: Adjust faces and only delete if all (or all but one) segments are removed - self.region_domain - .retain_with_region(|_, segment_range| segments_to_delete.contains(segment_range.start()) || segments_to_delete.contains(segment_range.end())); self.segment_domain.retain(|id| !segments_to_delete.contains(id), usize::MAX); self.point_domain.retain(&mut self.segment_domain, |id| !points_to_delete.contains(id)); } diff --git a/node-graph/libraries/vector-types/src/vector/vector_attributes.rs b/node-graph/libraries/vector-types/src/vector/vector_attributes.rs index a4cd04a03e..f87d238d82 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_attributes.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_attributes.rs @@ -48,7 +48,7 @@ macro_rules! create_ids { }; } -create_ids! { PointId, SegmentId, RegionId } +create_ids! { PointId, SegmentId } /// A no-op hasher that allows writing u64s (the id type). #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -550,110 +550,6 @@ impl SegmentDomain { } } -#[derive(Clone, Debug, Default, PartialEq, Hash, graphene_hash::CacheHash, DynAny)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -/// Stores data which is per-region. A region is an enclosed area composed of a range of segments from the -/// [`SegmentDomain`]. In future this will be extendable at runtime with custom attributes. -pub struct RegionDomain { - #[cfg_attr(feature = "serde", serde(alias = "ids"))] - id: Vec, - segment_range: Vec>, -} - -impl RegionDomain { - pub const fn new() -> Self { - Self { - id: Vec::new(), - segment_range: Vec::new(), - } - } - - #[inline(always)] - pub fn reserve(&mut self, additional: usize) { - self.id.reserve(additional); - self.segment_range.reserve(additional); - } - - pub(crate) fn retain(&mut self, f: impl Fn(&RegionId) -> bool) { - let mut keep = self.id.iter().map(&f); - self.segment_range.retain(|_| keep.next().unwrap_or_default()); - self.id.retain(&f); - } - - /// Like [`Self::retain`] but also gives the function access to the segment range. - /// - /// Note that this function requires an allocation that `retain` avoids. - pub(crate) fn retain_with_region(&mut self, f: impl Fn(&RegionId, &std::ops::RangeInclusive) -> bool) { - let keep = self.id.iter().zip(self.segment_range.iter()).map(|(id, range)| f(id, range)).collect::>(); - let mut iter = keep.iter().copied(); - self.segment_range.retain(|_| iter.next().unwrap()); - let mut iter = keep.iter().copied(); - self.id.retain(|_| iter.next().unwrap()); - } - - pub fn push(&mut self, id: RegionId, segment_range: std::ops::RangeInclusive) { - #[cfg(debug_assertions)] - if self.id.contains(&id) { - warn!("Tried to push a duplicate region to a region domain"); - return; - } - - self.push_unchecked(id, segment_range); - } - - #[inline(always)] - pub fn push_unchecked(&mut self, id: RegionId, segment_range: std::ops::RangeInclusive) { - self.id.push(id); - self.segment_range.push(segment_range); - } - - fn _resolve_id(&self, id: RegionId) -> Option { - self.id.iter().position(|&check_id| check_id == id) - } - - pub fn next_id(&self) -> RegionId { - self.id.iter().copied().max_by(|a, b| a.0.cmp(&b.0)).map(|mut id| id.next_id()).unwrap_or(RegionId::ZERO) - } - - pub(crate) fn segment_range_mut(&mut self) -> impl Iterator)> { - self.id.iter().copied().zip(self.segment_range.iter_mut()) - } - - pub fn ids(&self) -> &[RegionId] { - &self.id - } - - pub(crate) fn segment_range(&self) -> &[std::ops::RangeInclusive] { - &self.segment_range - } - - pub(crate) fn concat(&mut self, other: &Self, _transform: DAffine2, id_map: &IdMap) { - self.id.extend(other.id.iter().map(|id| *id_map.region_map.get(id).unwrap_or(id))); - self.segment_range.extend( - other - .segment_range - .iter() - .map(|range| *id_map.segment_map.get(range.start()).unwrap_or(range.start())..=*id_map.segment_map.get(range.end()).unwrap_or(range.end())), - ); - } - - pub(crate) fn map_ids(&mut self, id_map: &IdMap) { - self.id.iter_mut().for_each(|id| *id = *id_map.region_map.get(id).unwrap_or(id)); - self.segment_range - .iter_mut() - .for_each(|range| *range = *id_map.segment_map.get(range.start()).unwrap_or(range.start())..=*id_map.segment_map.get(range.end()).unwrap_or(range.end())); - } - - /// Iterates over regions in the domain. - /// - /// Tuple is: (id, segment_range) - pub fn iter(&self) -> impl Iterator)> + '_ { - let ids = self.id.iter().copied(); - let segment_range = self.segment_range.iter().cloned(); - zip(ids, segment_range) - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct HalfEdge { pub id: SegmentId, @@ -1022,18 +918,15 @@ impl Vector { pub fn vector_new_ids_from_hash(&mut self, node_id: u64) { let point_map = self.point_domain.ids().iter().map(|&old| (old, old.generate_from_hash(node_id))).collect::>(); let segment_map = self.segment_domain.ids().iter().map(|&old| (old, old.generate_from_hash(node_id))).collect::>(); - let region_map = self.region_domain.ids().iter().map(|&old| (old, old.generate_from_hash(node_id))).collect::>(); let id_map = IdMap { point_offset: self.point_domain.ids().len(), point_map, segment_map, - region_map, }; self.point_domain.map_ids(&id_map); self.segment_domain.map_ids(&id_map); - self.region_domain.map_ids(&id_map); } pub fn is_branching(&self) -> bool { @@ -1048,17 +941,10 @@ impl Vector { false } - fn has_regions(&self) -> bool { - !self.region_domain.id.is_empty() - } - - /// Determines if face-by-face fill rendering should be used. - /// Branching vectors without regions (e.g. mesh grids) need face-by-face fill rendering. - /// Branching vectors with regions (e.g. boolean operation results) use even-odd fill - /// on the main stroke path instead, since face decomposition can't determine which - /// bounded faces should vs. shouldn't be filled in boolean results. + /// Determines if face-by-face fill rendering should be used. Branching vectors are meshes, whose + /// bounded faces are found and filled individually rather than filling the stroke path directly. pub fn use_face_fill(&self) -> bool { - self.is_branching() && !self.has_regions() + self.is_branching() } pub fn construct_faces(&self) -> FaceIterator<'_> { @@ -1265,5 +1151,4 @@ pub(crate) struct IdMap { pub point_offset: usize, pub point_map: HashMap, pub segment_map: HashMap, - pub region_map: HashMap, } 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 3a9aa1ba55..fc7f6f498a 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_modification.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_modification.rs @@ -247,50 +247,12 @@ impl SegmentModification { } } -/// Represents a procedural change to the [`RegionDomain`] in [`Vector`]. -#[derive(Clone, Debug, Default, PartialEq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub(crate) struct RegionModification { - add: Vec, - #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashset"))] - remove: HashSet, - #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashmap", deserialize_with = "deserialize_hashmap"))] - segment_range: HashMap>, -} - -impl RegionModification { - /// Apply this modification to the specified [`RegionDomain`]. - pub fn apply(&self, region_domain: &mut RegionDomain) { - region_domain.retain(|id| !self.remove.contains(id)); - - for (id, segment_range) in region_domain.segment_range_mut() { - let Some(new) = self.segment_range.get(&id) else { continue }; - *segment_range = new.clone(); // Range inclusive is not copy - } - - for &add_id in &self.add { - let Some(segment_range) = self.segment_range.get(&add_id) else { continue }; - region_domain.push(add_id, segment_range.clone()); - } - } - - /// Create a new modification that will convert an empty [`Vector`] into the target [`Vector`]. - pub fn create_from_vector(vector: &Vector) -> Self { - Self { - add: vector.region_domain.ids().to_vec(), - remove: HashSet::new(), - segment_range: vector.region_domain.ids().iter().copied().zip(vector.region_domain.segment_range().iter().cloned()).collect(), - } - } -} - /// Represents a procedural change to the [`Vector`]. #[derive(Clone, Debug, Default, PartialEq, DynAny)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct VectorModification { points: PointModification, segments: SegmentModification, - regions: RegionModification, #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashset"))] add_g1_continuous: HashSet<[HandleId; 2]>, #[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashset"))] @@ -323,7 +285,6 @@ pub enum VectorModificationType { struct ModificationCategoryCounts { points: [usize; 3], segments: [usize; 3], - regions: [usize; 3], smooth_handles: [usize; 3], } @@ -331,7 +292,7 @@ impl ModificationCategoryCounts { /// Returns the `[added, removed, modified]` totals across all categories. fn totals(&self) -> [usize; 3] { let mut totals = [0; 3]; - for [a, r, m] in [self.points, self.segments, self.regions, self.smooth_handles] { + for [a, r, m] in [self.points, self.segments, self.smooth_handles] { totals[0] += a; totals[1] += r; totals[2] += m; @@ -341,7 +302,7 @@ impl ModificationCategoryCounts { /// Iterates over each named category and its `[added, removed, modified]` counts. fn iter_categories(&self) -> impl Iterator { - [("Points", self.points), ("Segments", self.segments), ("Regions", self.regions), ("Smooth Handles", self.smooth_handles)].into_iter() + [("Points", self.points), ("Segments", self.segments), ("Smooth Handles", self.smooth_handles)].into_iter() } } @@ -351,7 +312,6 @@ impl VectorModification { // Build sets of added IDs so we can distinguish true modifications from initial values stored for newly added items let add_points: HashSet<_> = self.points.add.iter().copied().collect(); let add_segments: HashSet<_> = self.segments.add.iter().copied().collect(); - let add_regions: HashSet<_> = self.regions.add.iter().copied().collect(); let point_modifications = self.points.delta.keys().filter(|id| !add_points.contains(id)).count(); @@ -363,15 +323,9 @@ impl VectorModification { modified_segments.extend(self.segments.handle_primary.keys().filter(not_added_segment)); modified_segments.extend(self.segments.handle_end.keys().filter(not_added_segment)); - // Count unique modified region IDs across all field maps - let mut modified_regions: HashSet<&RegionId> = HashSet::with_capacity(self.regions.segment_range.len()); - let not_added_region = |id: &&RegionId| !add_regions.contains(id); - modified_regions.extend(self.regions.segment_range.keys().filter(not_added_region)); - ModificationCategoryCounts { points: [self.points.add.len(), self.points.remove.len(), point_modifications], segments: [self.segments.add.len(), self.segments.remove.len(), modified_segments.len()], - regions: [self.regions.add.len(), self.regions.remove.len(), modified_regions.len()], smooth_handles: [self.add_g1_continuous.len(), self.remove_g1_continuous.len(), 0], } } @@ -423,7 +377,6 @@ impl VectorModification { pub fn apply(&self, vector: &mut Vector) { self.points.apply(&mut vector.point_domain, &mut vector.segment_domain); self.segments.apply(&mut vector.segment_domain, &vector.point_domain); - self.regions.apply(&mut vector.region_domain); let valid = |val: &[HandleId; 2]| vector.segment_domain.ids().contains(&val[0].segment) && vector.segment_domain.ids().contains(&val[1].segment); vector @@ -497,7 +450,6 @@ impl VectorModification { Self { points: PointModification::create_from_vector(vector), segments: SegmentModification::create_from_vector(vector), - regions: RegionModification::create_from_vector(vector), add_g1_continuous: vector.colinear_manipulators.iter().copied().collect(), remove_g1_continuous: HashSet::new(), } @@ -622,8 +574,6 @@ pub(crate) struct AppendBezpath<'a> { last_point: Option, first_point_index: Option, last_point_index: Option, - first_segment_id: Option, - last_segment_id: Option, point_id: PointId, segment_id: SegmentId, vector: &'a mut Vector, @@ -636,8 +586,6 @@ impl<'a> AppendBezpath<'a> { last_point: None, first_point_index: None, last_point_index: None, - first_segment_id: None, - last_segment_id: None, point_id: vector.point_domain.next_id(), segment_id: vector.segment_domain.next_id(), vector, @@ -664,13 +612,6 @@ impl<'a> AppendBezpath<'a> { self.vector .segment_domain .push(next_segment_id, self.last_point_index.unwrap(), self.first_point_index.unwrap(), handle); - - // Create a new region. - let next_region_id = self.vector.region_domain.next_id(); - let first_segment_id = self.first_segment_id.unwrap_or(next_segment_id); - let last_segment_id = next_segment_id; - - self.vector.region_domain.push(next_region_id, first_segment_id..=last_segment_id); } fn append_segment(&mut self, end_point: Point, handle: BezierHandles) { @@ -687,9 +628,6 @@ impl<'a> AppendBezpath<'a> { // Update the states. self.last_point = Some(end_point); self.last_point_index = Some(next_point_index); - - self.first_segment_id = Some(self.first_segment_id.unwrap_or(next_segment_id)); - self.last_segment_id = Some(next_segment_id); } fn append_first_point(&mut self, point: Point) { @@ -710,8 +648,6 @@ impl<'a> AppendBezpath<'a> { self.last_point = None; self.first_point_index = None; self.last_point_index = None; - self.first_segment_id = None; - self.last_segment_id = None; } pub fn append_bezpath(vector: &'a mut Vector, bezpath: BezPath) { 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 24ddb37008..0eada69ba3 100644 --- a/node-graph/libraries/vector-types/src/vector/vector_types.rs +++ b/node-graph/libraries/vector-types/src/vector/vector_types.rs @@ -20,7 +20,6 @@ pub struct Vector { pub point_domain: PointDomain, pub segment_domain: SegmentDomain, - pub region_domain: RegionDomain, } impl Default for Vector { @@ -29,7 +28,6 @@ impl Default for Vector { colinear_manipulators: Vec::new(), point_domain: PointDomain::new(), segment_domain: SegmentDomain::new(), - region_domain: RegionDomain::new(), } } } @@ -38,7 +36,6 @@ impl graphene_hash::CacheHash for Vector { fn cache_hash(&self, state: &mut H) { self.point_domain.cache_hash(state); self.segment_domain.cache_hash(state); - self.region_domain.cache_hash(state); self.colinear_manipulators.cache_hash(state); } } @@ -86,7 +83,6 @@ impl Vector { (Some(handle), None) | (None, Some(handle)) => BezierHandles::Quadratic { handle }, (Some(handle_start), Some(handle_end)) => BezierHandles::Cubic { handle_start, handle_end }, }; - let [mut first_seg, mut last_seg] = [None, None]; let mut segment_id = self.segment_domain.next_id(); let mut last_point = None; let mut first_point = None; @@ -112,24 +108,14 @@ impl Vector { 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_index, handles(&pair[0], &pair[1])); last_point = Some(end_index); } - if closed { - if let (Some(last), Some(first), Some(first_id), Some(last_id)) = (manipulator_groups.last(), manipulator_groups.first(), first_point, last_point) { - let id = segment_id.next_id(); - first_seg = Some(first_seg.unwrap_or(id)); - last_seg = Some(id); - self.segment_domain.push(id, last_id, first_id, handles(last, first)); - } - - if let [Some(first_seg), Some(last_seg)] = [first_seg, last_seg] { - self.region_domain.push(self.region_domain.next_id(), first_seg..=last_seg); - } + if closed && let (Some(last), Some(first), Some(first_id), Some(last_id)) = (manipulator_groups.last(), manipulator_groups.first(), first_point, last_point) { + let id = segment_id.next_id(); + self.segment_domain.push(id, last_id, first_id, handles(last, first)); } } @@ -454,24 +440,14 @@ impl Vector { .map(|&old| (old, old.generate_from_hash(collision_hash_seed))) .collect::>(); - let region_map = additional - .region_domain - .ids() - .iter() - .filter(|id| self.region_domain.ids().contains(id)) - .map(|&old| (old, old.generate_from_hash(collision_hash_seed))) - .collect::>(); - let id_map = IdMap { point_offset: self.point_domain.ids().len(), point_map, segment_map, - region_map, }; self.point_domain.concat(&additional.point_domain, transform_of_additional, &id_map); self.segment_domain.concat(&additional.segment_domain, transform_of_additional, &id_map); - self.region_domain.concat(&additional.region_domain, transform_of_additional, &id_map); self.colinear_manipulators.extend(additional.colinear_manipulators.iter().copied()); } diff --git a/node-graph/nodes/gstd/src/lib.rs b/node-graph/nodes/gstd/src/lib.rs index d338493155..88ba9428c2 100644 --- a/node-graph/nodes/gstd/src/lib.rs +++ b/node-graph/nodes/gstd/src/lib.rs @@ -32,7 +32,7 @@ pub mod vector { pub use vector_types::vector::algorithms; pub use vector_types::vector::click_target; pub use vector_types::vector::misc::HandleId; - pub use vector_types::vector::{PointId, RegionId, SegmentId}; + pub use vector_types::vector::{PointId, SegmentId}; pub use vector_types::vector::{deserialize_hashmap, serialize_hashmap, serialize_hashmap_as_sorted_object}; // Re-export HandleExt trait and NoHashBuilder diff --git a/node-graph/nodes/vector/src/generator_nodes.rs b/node-graph/nodes/vector/src/generator_nodes.rs index c7b07db841..b3fa3281bc 100644 --- a/node-graph/nodes/vector/src/generator_nodes.rs +++ b/node-graph/nodes/vector/src/generator_nodes.rs @@ -408,10 +408,9 @@ mod tests { #[test] fn grid_disconnected_cells_test() { - // A 3x3 rectangular grid has a 2x2 arrangement of cells, each its own closed quad subpath with a fillable region. - let grid = grid(&(), (), GridType::Rectangular, 10., 3_u32, 3_u32, (30., 30.).into(), false); - let vector = grid; - assert_eq!(vector.region_domain.ids().len(), 4); + // A 3x3 rectangular grid has a 2x2 arrangement of cells, each its own closed quad subpath. + let vector = grid(&(), (), GridType::Rectangular, 10., 3_u32, 3_u32, (30., 30.).into(), false); + assert_eq!(vector.stroke_manipulator_groups().filter(|(_, closed)| *closed).count(), 4); assert_eq!(vector.point_domain.ids().len(), 4 * 4); assert_eq!(vector.segment_domain.ids().len(), 4 * 4); diff --git a/node-graph/nodes/vector/src/vector_nodes.rs b/node-graph/nodes/vector/src/vector_nodes.rs index 5638e7861c..c08e2af34e 100644 --- a/node-graph/nodes/vector/src/vector_nodes.rs +++ b/node-graph/nodes/vector/src/vector_nodes.rs @@ -42,7 +42,7 @@ use vector_types::vector::misc::{ bezpath_from_manipulator_groups, bezpath_to_manipulator_groups, handles_to_segment, is_linear, point_to_dvec2, segment_to_handles, }; use vector_types::vector::style::{DashPattern, Gradient, Stroke, StrokeAlign, StrokeCap, StrokeJoin}; -use vector_types::vector::{PointDomain, PointId, RegionDomain, RegionId, SegmentDomain, SegmentId, VectorExt}; +use vector_types::vector::{PointDomain, PointId, SegmentDomain, SegmentId, VectorExt}; /// The gradient color for one assign-colors position, replaying the /// randomized draws up to it. @@ -1314,7 +1314,7 @@ fn as_vector>(_: impl Ctx, #[implementations(Vector, DVec2)] val value.into() } -/// Creates a polyline from a series of vector points, replacing any existing segments and regions that may already exist. +/// Creates a polyline from a series of vector points, replacing any existing segments that may already exist. #[node_macro::node(category("Vector"), name("Points to Polyline"), path(core_types::vector))] fn points_to_polyline<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'static>( ctx: impl Ctx + ExtractArena<'e>, @@ -1334,8 +1334,6 @@ fn points_to_polyline<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash if closed && points_count != 2 { segment_domain.push(next_id.next_id(), points_count - 1, 0, BezierHandles::Linear); - - points.region_domain.push(RegionId::generate(), segment_domain.ids()[0]..=*segment_domain.ids().last().unwrap()); } } @@ -1373,7 +1371,7 @@ fn relax_points<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'sta /// Builds a Voronoi diagram from the anchor points. Each point claims the region of space closest to it, and those regions tessellate the plane. Cells around the outside are clipped to the convex hull of the points so the diagram stays finite. /// -/// When Connect Cells is off, every cell becomes its own closed, fillable subpath. When on, the cells share their common points and segments, forming a single connected mesh with no fillable regions. +/// When Connect Cells is off, every cell becomes its own closed, fillable subpath. When on, the cells share their common points and segments, forming a single connected mesh. #[node_macro::node(category("Vector"), path(core_types::vector))] fn voronoi_cells<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'static>( ctx: impl Ctx + ExtractArena<'e>, @@ -1395,7 +1393,7 @@ fn voronoi_cells<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'st /// Builds a Delaunay triangulation connecting the anchor points. It is the geometric dual of the **Voronoi** node: a mesh of triangles in which no point lies inside any triangle's circumscribed circle. /// -/// When Connect Cells is off, every triangle becomes its own closed, fillable subpath. When on, the triangles share their common points and segments, forming a single connected mesh with no fillable regions. +/// When Connect Cells is off, every triangle becomes its own closed, fillable subpath. When on, the triangles share their common points and segments, forming a single connected mesh. #[node_macro::node(category("Vector"), path(core_types::vector))] fn triangulate<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'static>( ctx: impl Ctx + ExtractArena<'e>, @@ -1418,17 +1416,15 @@ fn triangulate<'e, V: MapVectorContent + Clone + Send + Sync + CacheHash + 'stat Ok(content) } -/// Replaces a vector's geometry (points, segments, and regions) with the given closed polygons, preserving its style. +/// Replaces a vector's geometry (points and segments) with the given closed polygons, preserving its style. /// -/// Without `connect_cells`, each polygon becomes its own closed subpath with a fillable region. -/// With it, coincident vertices are welded and each shared edge is emitted once, producing a connected mesh with no regions. +/// Without `connect_cells`, each polygon becomes its own closed subpath. +/// With it, coincident vertices are welded and each shared edge is emitted once, producing a connected mesh. pub(crate) fn replace_with_polygons(vector: &mut Vector, polygons: Vec>, connect_cells: bool) { let mut point_domain = PointDomain::new(); let mut segment_domain = SegmentDomain::new(); - let mut region_domain = RegionDomain::new(); let mut next_point = PointId::ZERO; let mut next_segment = SegmentId::ZERO; - let mut next_region = RegionId::ZERO; if !connect_cells { for polygon in &polygons { @@ -1442,19 +1438,10 @@ pub(crate) fn replace_with_polygons(vector: &mut Vector, polygons: Vec, snapshot: List>, progres } } - /// Pushes a subpath (list of manipulators) directly into a Vector's point, segment, and region domains, + /// Pushes a subpath (list of manipulators) directly into a Vector's point and segment domains, /// bypassing the BezPath intermediate representation used by `append_bezpath`. fn push_manipulators_to_vector(vector: &mut Vector, manips: &[ManipulatorGroup], closed: bool, point_id: &mut PointId, segment_id: &mut SegmentId) { let Some(first) = manips.first() else { return }; @@ -2943,28 +2928,20 @@ fn morph_core(flattened: List, snapshot: List>, progres let first_point_index = vector.point_domain.ids().len(); vector.point_domain.push_unchecked(point_id.next_id(), first.anchor); let mut prev_point_index = first_point_index; - let mut first_segment_id = None; for manip_window in manips.windows(2) { let point_index = vector.point_domain.ids().len(); vector.point_domain.push_unchecked(point_id.next_id(), manip_window[1].anchor); let handles = handles_from_manips(manip_window[0].out_handle, manip_window[1].in_handle); - let seg_id = segment_id.next_id(); - first_segment_id.get_or_insert(seg_id); - vector.segment_domain.push_unchecked(seg_id, prev_point_index, point_index, handles); + vector.segment_domain.push_unchecked(segment_id.next_id(), prev_point_index, point_index, handles); prev_point_index = point_index; } if closed && manips.len() > 1 { let handles = handles_from_manips(manips.last().unwrap().out_handle, manips[0].in_handle); - let closing_seg_id = segment_id.next_id(); - first_segment_id.get_or_insert(closing_seg_id); - vector.segment_domain.push_unchecked(closing_seg_id, prev_point_index, first_point_index, handles); - - let region_id = vector.region_domain.next_id(); - vector.region_domain.push_unchecked(region_id, first_segment_id.unwrap()..=closing_seg_id); + vector.segment_domain.push_unchecked(segment_id.next_id(), prev_point_index, first_point_index, handles); } } @@ -3352,7 +3329,6 @@ fn morph_core(flattened: List, snapshot: List>, progres // Pre-allocate domain storage based on total manipulator counts across all subpaths let mut total_points = 0; let mut total_segments = 0; - let mut total_regions = 0; for ((source_manips, source_closed), (target_manips, _)) in source_subpaths.iter().zip(target_subpaths.iter()) { if source_manips.is_empty() || target_manips.is_empty() { continue; @@ -3360,20 +3336,13 @@ fn morph_core(flattened: List, snapshot: List>, progres let manip_count = source_manips.len().max(target_manips.len()); total_points += manip_count; total_segments += if *source_closed { manip_count } else { manip_count.saturating_sub(1) }; - if *source_closed { - total_regions += 1; - } } for (manips, closed) in extra_source.iter().chain(extra_target.iter()) { total_points += manips.len(); total_segments += if *closed { manips.len() } else { manips.len().saturating_sub(1) }; - if *closed { - total_regions += 1; - } } vector.point_domain.reserve(total_points); vector.segment_domain.reserve(total_segments); - vector.region_domain.reserve(total_regions); let mut point_id = PointId::ZERO; let mut segment_id = SegmentId::ZERO; @@ -4082,12 +4051,13 @@ mod test { } #[test] - fn delaunay_disconnected_cells_make_one_region_per_triangle() { + fn delaunay_disconnected_cells_make_one_subpath_per_triangle() { let vector = with_ctx(|ctx| super::triangulate(ctx, vector_from_points(&SQUARE_WITH_CENTER), false).unwrap()); // The square plus its center tessellates into four triangles, each its own closed subpath. - assert_eq!(vector.region_domain.ids().len(), 4); + assert_eq!(vector.stroke_manipulator_groups().filter(|(_, closed)| *closed).count(), 4); assert_eq!(vector.segment_domain.ids().len(), 4 * 3); assert_eq!(vector.point_domain.ids().len(), 4 * 3); + assert!(!vector.use_face_fill()); } #[test] @@ -4124,19 +4094,19 @@ mod test { #[test] fn delaunay_shared_mesh_welds_points_and_shares_edges() { let vector = with_ctx(|ctx| super::triangulate(ctx, vector_from_points(&SQUARE_WITH_CENTER), true).unwrap()); - // The connected mesh reuses the five input points and shares edges, with no fillable regions. - assert_eq!(vector.region_domain.ids().len(), 0); + // The connected mesh reuses the five input points and shares edges, so it fills face by face. + assert!(vector.use_face_fill()); assert_eq!(vector.point_domain.ids().len(), 5); // Four hull edges plus four spokes to the center, each emitted once. assert_eq!(vector.segment_domain.ids().len(), 8); } #[test] - fn voronoi_disconnected_cells_make_a_region_per_cell() { + fn voronoi_disconnected_cells_make_a_subpath_per_cell() { let vector = with_ctx(|ctx| super::voronoi_cells(ctx, vector_from_points(&SQUARE_WITH_CENTER), false).unwrap()); - let regions = vector.region_domain.ids().len(); - assert!(regions > 0, "expected at least one Voronoi region"); - // Every region is a closed subpath, so segments and points come in matched per-region loops. + let cells = vector.stroke_manipulator_groups().filter(|(_, closed)| *closed).count(); + assert!(cells > 0, "expected at least one Voronoi cell"); + // Every cell is a closed subpath, so segments and points come in matched per-cell loops. assert_eq!(vector.segment_domain.ids().len(), vector.point_domain.ids().len()); // Clipping to the convex hull keeps all cell vertices within the input bounds. @@ -4147,9 +4117,9 @@ mod test { } #[test] - fn voronoi_shared_mesh_has_no_regions() { + fn voronoi_shared_mesh_uses_face_fill() { let vector = with_ctx(|ctx| super::voronoi_cells(ctx, vector_from_points(&SQUARE_WITH_CENTER), true).unwrap()); - assert_eq!(vector.region_domain.ids().len(), 0); + assert!(vector.use_face_fill()); assert!(!vector.segment_domain.ids().is_empty()); }