fork/Graphite
1
0
Fork 0
mirror of https://github.com/GraphiteEditor/Graphite.git synced 2026-09-15 14:18:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

Delete the vestigial region domain, reverting to naive logic for mesh region fill determination (#4463)

Browse Source 
Delete the vestigial region domain, reverting the face fill decision to the bare branching test
This commit is contained in:
Keavon Chambers
2026-08-18 23:50:16 -07:00
committed by GitHub
parent 39425ca7ea
commit daa8b6a5a4
11 changed files with 36 additions and 287 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
node-graph/libraries/graphic-types/src/lib.rs
View File

@@ -24,7 +24,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)]
@@ -111,7 +111,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)]
@@ -142,7 +141,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(),

1
node-graph/libraries/rendering/src/renderer.rs
View File

@@ -1848,7 +1848,6 @@ fn render_vector_item_to_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 {

4
node-graph/libraries/vector-types/src/vector/algorithms/merge_by_distance.rs
View File

@@ -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));
}

123
node-graph/libraries/vector-types/src/vector/vector_attributes.rs
View File

@@ -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<RegionId>,
segment_range: Vec<std::ops::RangeInclusive<SegmentId>>,
}
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<SegmentId>) -> bool) {
let keep = self.id.iter().zip(self.segment_range.iter()).map(|(id, range)| f(id, range)).collect::<Vec<_>>();
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<SegmentId>) {
#[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<SegmentId>) {
self.id.push(id);
self.segment_range.push(segment_range);
}
fn _resolve_id(&self, id: RegionId) -> Option<usize> {
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<Item = (RegionId, &mut std::ops::RangeInclusive<SegmentId>)> {
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<SegmentId>] {
&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<Item = (RegionId, std::ops::RangeInclusive<SegmentId>)> + '_ {
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::<HashMap<_, _>>();
let segment_map = self.segment_domain.ids().iter().map(|&old| (old, old.generate_from_hash(node_id))).collect::<HashMap<_, _>>();
let region_map = self.region_domain.ids().iter().map(|&old| (old, old.generate_from_hash(node_id))).collect::<HashMap<_, _>>();
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<PointId, PointId>,
pub segment_map: HashMap<SegmentId, SegmentId>,
pub region_map: HashMap<RegionId, RegionId>,
}

68
node-graph/libraries/vector-types/src/vector/vector_modification.rs
View File

@@ -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<RegionId>,
#[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashset"))]
remove: HashSet<RegionId>,
#[cfg_attr(feature = "serde", serde(serialize_with = "serialize_hashmap", deserialize_with = "deserialize_hashmap"))]
segment_range: HashMap<RegionId, std::ops::RangeInclusive<SegmentId>>,
}
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<Item = (&str, [usize; 3])> {
[("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<Point>,
first_point_index: Option<usize>,
last_point_index: Option<usize>,
first_segment_id: Option<SegmentId>,
last_segment_id: Option<SegmentId>,
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) {

30
node-graph/libraries/vector-types/src/vector/vector_types.rs
View File

@@ -21,7 +21,6 @@ pub struct Vector {
pub point_domain: PointDomain,
pub segment_domain: SegmentDomain,
pub region_domain: RegionDomain,
}
unsafe impl StaticType for Vector {
type Static = Self;
@@ -33,7 +32,6 @@ impl Default for Vector {
colinear_manipulators: Vec::new(),
point_domain: PointDomain::new(),
segment_domain: SegmentDomain::new(),
region_domain: RegionDomain::new(),
}
}
}
@@ -42,7 +40,6 @@ impl graphene_hash::CacheHash for Vector {
fn cache_hash<H: core::hash::Hasher>(&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);
}
}
@@ -82,7 +79,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;
@@ -108,24 +104,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));
}
}
@@ -450,24 +436,14 @@ impl Vector {
.map(|&old| (old, old.generate_from_hash(collision_hash_seed)))
.collect::<HashMap<_, _>>();
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::<HashMap<_, _>>();
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());
}