Skip to content

Commit 4a65ad2

Browse files
KeavonKeavon
committed
Fix 'Boolean Operation' node merge-by-distance post-processing
Fixes #2750
1 parent 11ba2cc commit 4a65ad2

File tree

3 files changed

+123
-125
lines changed

3 files changed

+123
-125
lines changed

‎node-graph/gcore/src/vector/algorithms/merge_by_distance.rs

Lines changed: 117 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,18 @@
1-
use crate::vector::{PointId, VectorData, VectorDataIndex};
2-
use glam::DVec2;
1+
use crate::vector::{PointDomain, PointId, SegmentDomain, VectorData, VectorDataIndex};
2+
use glam::{DAffine2, DVec2};
33
use petgraph::prelude::UnGraphMap;
44
use rustc_hash::FxHashSet;
55

66
impl VectorData {
77
/// Collapse all points with edges shorter than the specified distance
8-
pub fn merge_by_distance(&mut self, distance: f64) {
8+
pub fn merge_by_distance_topological(&mut self, distance: f64) {
99
// Treat self as an undirected graph
1010
let indices = VectorDataIndex::build_from(self);
1111

12+
// TODO: We lose information on the winding order by using an undirected graph. Switch to a directed graph and fix the algorithm to handle that.
1213
// Graph containing only short edges, referencing the data graph
1314
let mut short_edges = UnGraphMap::new();
15+
1416
for segment_id in self.segment_ids().iter().copied() {
1517
let length = indices.segment_chord_length(segment_id);
1618
if length < distance {
@@ -92,4 +94,116 @@ impl VectorData {
9294
self.segment_domain.retain(|id| !segments_to_delete.contains(id), usize::MAX);
9395
self.point_domain.retain(&mut self.segment_domain, |id| !points_to_delete.contains(id));
9496
}
97+
98+
pub fn merge_by_distance_spatial(&mut self, transform: DAffine2, distance: f64) {
99+
let point_count = self.point_domain.positions().len();
100+
101+
// Find min x and y for grid cell normalization
102+
let mut min_x = f64::MAX;
103+
let mut min_y = f64::MAX;
104+
105+
// Calculate mins without collecting all positions
106+
for &pos in self.point_domain.positions() {
107+
let transformed_pos = transform.transform_point2(pos);
108+
min_x = min_x.min(transformed_pos.x);
109+
min_y = min_y.min(transformed_pos.y);
110+
}
111+
112+
// Create a spatial grid with cell size of 'distance'
113+
use std::collections::HashMap;
114+
let mut grid: HashMap<(i32, i32), Vec<usize>> = HashMap::new();
115+
116+
// Add points to grid cells without collecting all positions first
117+
for i in 0..point_count {
118+
let pos = transform.transform_point2(self.point_domain.positions()[i]);
119+
let grid_x = ((pos.x - min_x) / distance).floor() as i32;
120+
let grid_y = ((pos.y - min_y) / distance).floor() as i32;
121+
122+
grid.entry((grid_x, grid_y)).or_default().push(i);
123+
}
124+
125+
// Create point index mapping for merged points
126+
let mut point_index_map = vec![None; point_count];
127+
let mut merged_positions = Vec::new();
128+
let mut merged_indices = Vec::new();
129+
130+
// Process each point
131+
for i in 0..point_count {
132+
// Skip points that have already been processed
133+
if point_index_map[i].is_some() {
134+
continue;
135+
}
136+
137+
let pos_i = transform.transform_point2(self.point_domain.positions()[i]);
138+
let grid_x = ((pos_i.x - min_x) / distance).floor() as i32;
139+
let grid_y = ((pos_i.y - min_y) / distance).floor() as i32;
140+
141+
let mut group = vec![i];
142+
143+
// Check only neighboring cells (3x3 grid around current cell)
144+
for dx in -1..=1 {
145+
for dy in -1..=1 {
146+
let neighbor_cell = (grid_x + dx, grid_y + dy);
147+
148+
if let Some(indices) = grid.get(&neighbor_cell) {
149+
for &j in indices {
150+
if j > i && point_index_map[j].is_none() {
151+
let pos_j = transform.transform_point2(self.point_domain.positions()[j]);
152+
if pos_i.distance(pos_j) <= distance {
153+
group.push(j);
154+
}
155+
}
156+
}
157+
}
158+
}
159+
}
160+
161+
// Create merged point - calculate positions as needed
162+
let merged_position = group
163+
.iter()
164+
.map(|&idx| transform.transform_point2(self.point_domain.positions()[idx]))
165+
.fold(DVec2::ZERO, |sum, pos| sum + pos)
166+
/ group.len() as f64;
167+
168+
let merged_position = transform.inverse().transform_point2(merged_position);
169+
let merged_index = merged_positions.len();
170+
171+
merged_positions.push(merged_position);
172+
merged_indices.push(self.point_domain.ids()[group[0]]);
173+
174+
// Update mapping for all points in the group
175+
for &idx in &group {
176+
point_index_map[idx] = Some(merged_index);
177+
}
178+
}
179+
180+
// Create new point domain with merged points
181+
let mut new_point_domain = PointDomain::new();
182+
for (idx, pos) in merged_indices.into_iter().zip(merged_positions) {
183+
new_point_domain.push(idx, pos);
184+
}
185+
186+
// Update segment domain
187+
let mut new_segment_domain = SegmentDomain::new();
188+
for segment_idx in 0..self.segment_domain.ids().len() {
189+
let id = self.segment_domain.ids()[segment_idx];
190+
let start = self.segment_domain.start_point()[segment_idx];
191+
let end = self.segment_domain.end_point()[segment_idx];
192+
let handles = self.segment_domain.handles()[segment_idx];
193+
let stroke = self.segment_domain.stroke()[segment_idx];
194+
195+
// Get new indices for start and end points
196+
let new_start = point_index_map[start].unwrap();
197+
let new_end = point_index_map[end].unwrap();
198+
199+
// Skip segments where start and end points were merged
200+
if new_start != new_end {
201+
new_segment_domain.push(id, new_start, new_end, handles, stroke);
202+
}
203+
}
204+
205+
// Create new vector data
206+
self.point_domain = new_point_domain;
207+
self.segment_domain = new_segment_domain;
208+
}
95209
}

‎node-graph/gcore/src/vector/vector_nodes.rs

Lines changed: 5 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -555,138 +555,22 @@ pub fn merge_by_distance(
555555
vector_data: VectorDataTable,
556556
#[default(0.1)]
557557
#[hard_min(0.0001)]
558-
distance: Length,
558+
distance: PixelLength,
559559
algorithm: MergeByDistanceAlgorithm,
560560
) -> VectorDataTable {
561561
let mut result_table = VectorDataTable::default();
562562

563563
match algorithm {
564564
MergeByDistanceAlgorithm::Spatial => {
565565
for mut vector_data_instance in vector_data.instance_iter() {
566-
let vector_data_transform = vector_data_instance.transform;
567-
let vector_data = vector_data_instance.instance;
568-
569-
let point_count = vector_data.point_domain.positions().len();
570-
571-
// Find min x and y for grid cell normalization
572-
let mut min_x = f64::MAX;
573-
let mut min_y = f64::MAX;
574-
575-
// Calculate mins without collecting all positions
576-
for &pos in vector_data.point_domain.positions() {
577-
let transformed_pos = vector_data_transform.transform_point2(pos);
578-
min_x = min_x.min(transformed_pos.x);
579-
min_y = min_y.min(transformed_pos.y);
580-
}
581-
582-
// Create a spatial grid with cell size of 'distance'
583-
use std::collections::HashMap;
584-
let mut grid: HashMap<(i32, i32), Vec<usize>> = HashMap::new();
585-
586-
// Add points to grid cells without collecting all positions first
587-
for i in 0..point_count {
588-
let pos = vector_data_transform.transform_point2(vector_data.point_domain.positions()[i]);
589-
let grid_x = ((pos.x - min_x) / distance).floor() as i32;
590-
let grid_y = ((pos.y - min_y) / distance).floor() as i32;
591-
592-
grid.entry((grid_x, grid_y)).or_default().push(i);
593-
}
594-
595-
// Create point index mapping for merged points
596-
let mut point_index_map = vec![None; point_count];
597-
let mut merged_positions = Vec::new();
598-
let mut merged_indices = Vec::new();
599-
600-
// Process each point
601-
for i in 0..point_count {
602-
// Skip points that have already been processed
603-
if point_index_map[i].is_some() {
604-
continue;
605-
}
606-
607-
let pos_i = vector_data_transform.transform_point2(vector_data.point_domain.positions()[i]);
608-
let grid_x = ((pos_i.x - min_x) / distance).floor() as i32;
609-
let grid_y = ((pos_i.y - min_y) / distance).floor() as i32;
610-
611-
let mut group = vec![i];
612-
613-
// Check only neighboring cells (3x3 grid around current cell)
614-
for dx in -1..=1 {
615-
for dy in -1..=1 {
616-
let neighbor_cell = (grid_x + dx, grid_y + dy);
617-
618-
if let Some(indices) = grid.get(&neighbor_cell) {
619-
for &j in indices {
620-
if j > i && point_index_map[j].is_none() {
621-
let pos_j = vector_data_transform.transform_point2(vector_data.point_domain.positions()[j]);
622-
if pos_i.distance(pos_j) <= distance {
623-
group.push(j);
624-
}
625-
}
626-
}
627-
}
628-
}
629-
}
630-
631-
// Create merged point - calculate positions as needed
632-
let merged_position = group
633-
.iter()
634-
.map(|&idx| vector_data_transform.transform_point2(vector_data.point_domain.positions()[idx]))
635-
.fold(DVec2::ZERO, |sum, pos| sum + pos)
636-
/ group.len() as f64;
637-
638-
let merged_position = vector_data_transform.inverse().transform_point2(merged_position);
639-
let merged_index = merged_positions.len();
640-
641-
merged_positions.push(merged_position);
642-
merged_indices.push(vector_data.point_domain.ids()[group[0]]);
643-
644-
// Update mapping for all points in the group
645-
for &idx in &group {
646-
point_index_map[idx] = Some(merged_index);
647-
}
648-
}
649-
650-
// Create new point domain with merged points
651-
let mut new_point_domain = PointDomain::new();
652-
for (idx, pos) in merged_indices.into_iter().zip(merged_positions) {
653-
new_point_domain.push(idx, pos);
654-
}
655-
656-
// Update segment domain
657-
let mut new_segment_domain = SegmentDomain::new();
658-
for segment_idx in 0..vector_data.segment_domain.ids().len() {
659-
let id = vector_data.segment_domain.ids()[segment_idx];
660-
let start = vector_data.segment_domain.start_point()[segment_idx];
661-
let end = vector_data.segment_domain.end_point()[segment_idx];
662-
let handles = vector_data.segment_domain.handles()[segment_idx];
663-
let stroke = vector_data.segment_domain.stroke()[segment_idx];
664-
665-
// Get new indices for start and end points
666-
let new_start = point_index_map[start].unwrap();
667-
let new_end = point_index_map[end].unwrap();
668-
669-
// Skip segments where start and end points were merged
670-
if new_start != new_end {
671-
new_segment_domain.push(id, new_start, new_end, handles, stroke);
672-
}
673-
}
674-
675-
// Create new vector data
676-
let mut result = vector_data.clone();
677-
result.point_domain = new_point_domain;
678-
result.segment_domain = new_segment_domain;
679-
680-
// Create and return the result
681-
vector_data_instance.instance = result;
682-
vector_data_instance.source_node_id = None;
566+
vector_data_instance.instance.merge_by_distance_spatial(vector_data_instance.transform, distance);
683567
result_table.push(vector_data_instance);
684568
}
685569
}
686570
MergeByDistanceAlgorithm::Topological => {
687-
for mut source_instance in vector_data.instance_iter() {
688-
source_instance.instance.merge_by_distance(distance);
689-
result_table.push(source_instance);
571+
for mut vector_data_instance in vector_data.instance_iter() {
572+
vector_data_instance.instance.merge_by_distance_topological(distance);
573+
result_table.push(vector_data_instance);
690574
}
691575
}
692576
}

‎node-graph/gstd/src/vector.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ async fn boolean_operation<I: Into<GraphicGroupTable> + 'n + Send + Clone>(
4343
result_vector_data.instance.upstream_graphic_group = Some(group_of_paths.clone());
4444

4545
// Clean up the boolean operation result by merging duplicated points
46-
result_vector_data.instance.merge_by_distance(0.001);
46+
result_vector_data.instance.merge_by_distance_spatial(*result_vector_data.transform, 0.0001);
4747
}
4848

4949
result_vector_data_table

0 commit comments

Comments
 (0)