diff --git a/docs/dev/debug_env_vars.md b/docs/dev/debug_env_vars.md index ecaf7fd4..72de08b0 100644 --- a/docs/dev/debug_env_vars.md +++ b/docs/dev/debug_env_vars.md @@ -84,6 +84,7 @@ This matters for large-scale investigations that need to run under |---|---|---|---| | `DELAUNAY_DEBUG_NEIGHBORS` | presence | `incremental_insertion.rs`, `triangulation.rs` | Neighbor symmetry checks after cavity wiring and hull extension | | `DELAUNAY_DEBUG_RIDGE_LINK` | presence | `incremental_insertion.rs`, `triangulation.rs` | Ridge-link validation after wiring; skipped external facet matches | +| `DELAUNAY_FORCE_GLOBAL_NEIGHBOR_REBUILD` | presence | `triangulation.rs` | `[release]` Bypass local repair; use global rebuild for A/B isolation. | ## Flip Repair diff --git a/src/core/algorithms/incremental_insertion.rs b/src/core/algorithms/incremental_insertion.rs index ed546ba2..e544e5cf 100644 --- a/src/core/algorithms/incremental_insertion.rs +++ b/src/core/algorithms/incremental_insertion.rs @@ -542,21 +542,37 @@ impl From for InitialSimplexConstructionError { } } -/// Structured reason why a global neighbor rebuild failed after cavity repair. +/// Structured reason why neighbor repair failed after cavity repair. +/// +/// # Examples +/// +/// ```rust +/// use delaunay::prelude::tds::CellKey; +/// use delaunay::prelude::triangulation::insertion::{ +/// NeighborRebuildError, NeighborWiringError, +/// }; +/// use slotmap::KeyData; +/// +/// let cell_key = CellKey::from(KeyData::from_ffi(11)); +/// let err = NeighborRebuildError::Wiring { +/// reason: NeighborWiringError::MissingCell { cell_key }, +/// }; +/// assert!(matches!(err, NeighborRebuildError::Wiring { .. })); +/// ``` #[derive(Debug, Clone, thiserror::Error, PartialEq, Eq)] #[non_exhaustive] pub enum NeighborRebuildError { - /// Neighbor wiring failed during the rebuild. - #[error("neighbor wiring failed during rebuild: {reason}")] + /// Neighbor wiring failed while repairing neighbor pointers. + #[error("neighbor wiring failed while repairing neighbors: {reason}")] Wiring { /// Structured wiring failure. #[source] reason: NeighborWiringError, }, - /// The rebuilt facet incidence is non-manifold. + /// The repaired facet incidence is non-manifold. #[error( - "non-manifold topology during neighbor rebuild: facet {facet_hash:#x} shared by {cell_count} cells" + "non-manifold topology while repairing neighbors: facet {facet_hash:#x} shared by {cell_count} cells" )] NonManifoldTopology { /// Hash of the over-shared facet. @@ -565,19 +581,20 @@ pub enum NeighborRebuildError { cell_count: usize, }, - /// TDS validation failed during neighbor rebuild. - #[error("TDS validation failed during neighbor rebuild: {reason}")] + /// TDS validation failed while repairing neighbor pointers. + #[error("TDS validation failed while repairing neighbors: {reason}")] TopologyValidation { /// Underlying TDS validation error. #[source] reason: TdsValidationFailure, }, - /// Another insertion error escaped the neighbor-rebuild helper. - #[error("unexpected neighbor rebuild error: {message}")] + /// Another insertion error escaped the neighbor repair helper. + #[error("unexpected neighbor repair error: {source}")] Unexpected { - /// Display form of the unexpected insertion error. - message: String, + /// Insertion-layer error that did not map to a neighbor repair category. + #[source] + source: Box, }, } @@ -695,8 +712,8 @@ pub enum CavityFillingError { stage: CavityRepairStage, }, - /// Rebuilding neighbor pointers after cavity repair failed. - #[error("failed to rebuild neighbors after insertion repairs: {reason}")] + /// Repairing neighbor pointers after cavity repair failed. + #[error("failed to repair neighbors after insertion repairs: {reason}")] NeighborRebuild { /// Underlying insertion-layer error from the neighbor rebuild. #[source] @@ -852,6 +869,24 @@ pub enum NeighborWiringError { neighbor_key: CellKey, }, + /// A neighbor pointer does not point back through the matching mirror facet. + #[error( + "cell {cell_key:?} facet {facet_index} points to {neighbor_key:?}, \ + but the mirror facet {mirror_facet_index:?} points back to {neighbor_back:?}" + )] + AsymmetricNeighbor { + /// Cell containing the neighbor pointer. + cell_key: CellKey, + /// Facet index in `cell_key`. + facet_index: usize, + /// Neighbor referenced by `cell_key`. + neighbor_key: CellKey, + /// Matching facet index in `neighbor_key`, if one could be found. + mirror_facet_index: Option, + /// Neighbor's pointer back through the mirror facet. + neighbor_back: Option, + }, + /// Neighbor traversal discovered more cells than the TDS contains. #[error("neighbor walk visited {visited} unique cells but triangulation contains {total}")] NeighborWalkExceededCellCount { @@ -1033,7 +1068,7 @@ impl From for NeighborRebuildError { reason: source.into(), }, other => Self::Unexpected { - message: other.to_string(), + source: Box::new(other), }, } } @@ -2050,7 +2085,6 @@ fn set_neighbor( neighbor: Option, ) -> Result<(), InsertionError> where - T: CoordinateScalar, U: DataType, V: DataType, { @@ -2081,6 +2115,323 @@ fn compute_facet_hash(sorted_vkeys: &[VertexKey]) -> u64 { hasher.finish() } +/// Compute a canonical hash for one cell facet so local repair can match +/// newly exposed facets without scanning the full triangulation. +fn facet_hash_for_cell(cell: &Cell, facet_idx: usize) -> u64 +where + U: DataType, + V: DataType, +{ + let mut facet_vkeys = SmallBuffer::::new(); + for (i, &vkey) in cell.vertices().iter().enumerate() { + if i != facet_idx { + facet_vkeys.push(vkey); + } + } + facet_vkeys.sort_unstable(); + compute_facet_hash(&facet_vkeys) +} + +/// Return whether `neighbor_key` is the cell incident across `cell`'s `facet_idx`. +fn neighbor_slot_points_across_facet( + tds: &Tds, + cell: &Cell, + facet_idx: usize, + neighbor_key: CellKey, +) -> bool +where + U: DataType, + V: DataType, +{ + tds.cell(neighbor_key) + .is_some_and(|neighbor_cell| cell.mirror_facet_index(facet_idx, neighbor_cell).is_some()) +} + +/// Repair neighbor pointers for a locally affected cell set. +/// +/// This is the cold-path counterpart to [`repair_neighbor_pointers`]. It avoids a +/// triangulation-wide facet scan after a small repair removes cells by: +/// - seeding the affected set from `seeds` and `optional_external_cells`, +/// - adding one-hop neighbors reachable through current pointers, +/// - building facet incidence only for that affected set, and +/// - filling only empty, dangling, or facet-incompatible neighbor slots that can be matched locally. +/// +/// Existing valid neighbor pointers are preserved, including pointers into cells outside +/// the affected set. +/// +/// This helper is intentionally scoped: it only considers `seeds`, +/// `optional_external_cells`, and one-hop neighbors reachable from those cells. +/// Damage outside that local region is ignored. Use [`repair_neighbor_pointers`] +/// when callers need a full triangulation-wide rebuild, or run validation after +/// local repair when the affected region is not known precisely. +/// +/// # Arguments +/// +/// - `tds` - triangulation data structure whose neighbor slots may be repaired. +/// - `seeds` - cells that mark the local region touched by cavity repair. +/// - `optional_external_cells` - outside cells that share facets with the repaired region. +/// +/// # Returns +/// +/// `Ok(n)` where `n` is the number of neighbor-pointer slots whose values changed. +/// +/// # Errors +/// +/// Returns [`InsertionError::NeighborWiring`] if an affected cell is malformed, a facet index +/// cannot fit in `u8`, or debug-only local validation finds a dangling or asymmetric neighbor +/// pointer. Returns [`InsertionError::NonManifoldTopology`] if local facet incidence has more +/// than two incident cells for one facet. +/// +/// # Examples +/// +/// ```rust +/// use delaunay::prelude::triangulation::{DelaunayTriangulation, vertex}; +/// use delaunay::prelude::triangulation::DelaunayTriangulationConstructionError; +/// use delaunay::prelude::triangulation::insertion::{ +/// InsertionError, TdsMutationError, repair_neighbor_pointers_local, +/// }; +/// +/// # #[derive(Debug, thiserror::Error)] +/// # enum ExampleError { +/// # #[error(transparent)] +/// # Construction(#[from] DelaunayTriangulationConstructionError), +/// # #[error(transparent)] +/// # Mutation(#[from] TdsMutationError), +/// # #[error(transparent)] +/// # Insertion(#[from] InsertionError), +/// # } +/// # fn main() -> Result<(), ExampleError> { +/// let vertices = [ +/// vertex!([0.0, 0.0]), +/// vertex!([1.0, 0.0]), +/// vertex!([0.0, 1.0]), +/// vertex!([1.0, 1.1]), +/// ]; +/// let dt: DelaunayTriangulation<_, (), (), 2> = DelaunayTriangulation::new(&vertices)?; +/// let mut tds = dt.tds().clone(); +/// +/// let (cell_key, facet_idx, neighbor_key) = tds +/// .cells() +/// .find_map(|(cell_key, cell)| { +/// cell.neighbors()?.iter().enumerate().find_map(|(facet_idx, neighbor)| { +/// neighbor.map(|neighbor_key| (cell_key, facet_idx, neighbor_key)) +/// }) +/// }) +/// .expect("test triangulation should contain adjacent cells"); +/// +/// let mut neighbors: Vec<_> = tds +/// .cell(cell_key) +/// .expect("cell key came from the TDS") +/// .neighbors() +/// .expect("neighbor slot exists") +/// .iter() +/// .copied() +/// .collect(); +/// neighbors[facet_idx] = None; +/// tds.set_neighbors_by_key(cell_key, &neighbors)?; +/// +/// let repaired = repair_neighbor_pointers_local(&mut tds, &[cell_key], Some(&[neighbor_key]))?; +/// assert_eq!(repaired, 1); +/// assert_eq!( +/// tds.cell(cell_key) +/// .and_then(|cell| cell.neighbors()) +/// .and_then(|neighbors| neighbors.get(facet_idx)) +/// .copied() +/// .flatten(), +/// Some(neighbor_key) +/// ); +/// # Ok(()) +/// # } +/// ``` +#[expect( + clippy::too_many_lines, + reason = "Local neighbor repair keeps affected-set construction, facet indexing, and slot application together" +)] +pub fn repair_neighbor_pointers_local( + tds: &mut Tds, + seeds: &[CellKey], + optional_external_cells: Option<&[CellKey]>, +) -> Result +where + U: DataType, + V: DataType, +{ + type FacetIncidents = SmallBuffer<(CellKey, u8), 2>; + + let mut affected_cells: FastHashSet = FastHashSet::default(); + for &cell_key in seeds { + if tds.contains_cell(cell_key) { + affected_cells.insert(cell_key); + } + } + if let Some(external_cells) = optional_external_cells { + for &cell_key in external_cells { + if tds.contains_cell(cell_key) { + affected_cells.insert(cell_key); + } + } + } + + if affected_cells.is_empty() { + return Ok(0); + } + + // Expand once through existing pointers. This keeps the repair local while + // including the surviving cells whose facets may now match a repaired seed. + let seed_snapshot: Vec = affected_cells.iter().copied().collect(); + for cell_key in seed_snapshot { + let Some(cell) = tds.cell(cell_key) else { + continue; + }; + let Some(neighbors) = cell.neighbors() else { + continue; + }; + for &neighbor_key in neighbors.iter().flatten() { + if tds.contains_cell(neighbor_key) { + affected_cells.insert(neighbor_key); + } + } + } + + let mut facet_map: FastHashMap = FastHashMap::default(); + + for &cell_key in &affected_cells { + let cell = tds + .cell(cell_key) + .ok_or(NeighborWiringError::MissingCell { cell_key })?; + + let vertex_count = cell.number_of_vertices(); + let expected = D + 1; + if vertex_count != expected { + return Err(NeighborWiringError::WrongCellArity { + cell_key, + expected, + found: vertex_count, + } + .into()); + } + + for facet_idx in 0..vertex_count { + let facet_hash = facet_hash_for_cell(cell, facet_idx); + let facet_idx_u8 = + u8::try_from(facet_idx).map_err(|_| NeighborWiringError::FacetIndexOverflow { + facet_index: facet_idx, + max: u8::MAX, + })?; + + let entry = facet_map.entry(facet_hash).or_default(); + entry.push((cell_key, facet_idx_u8)); + + if entry.len() > 2 { + return Err(InsertionError::NonManifoldTopology { + facet_hash, + cell_count: entry.len(), + }); + } + } + } + + let mut total_neighbor_slots_fixed = 0usize; + + for &cell_key in &affected_cells { + let (vertex_count, old_neighbors, replacement_by_facet, current_usable_by_facet) = { + let cell = tds + .cell(cell_key) + .ok_or(NeighborWiringError::MissingCell { cell_key })?; + let vertex_count = cell.number_of_vertices(); + let old_neighbors: SmallBuffer, MAX_PRACTICAL_DIMENSION_SIZE> = + cell.neighbors().map_or_else( + || SmallBuffer::from_elem(None, vertex_count), + |neighbors| { + let mut copied: SmallBuffer, MAX_PRACTICAL_DIMENSION_SIZE> = + neighbors.iter().copied().collect(); + copied.resize(vertex_count, None); + copied + }, + ); + + let mut replacement_by_facet = + SmallBuffer::, MAX_PRACTICAL_DIMENSION_SIZE>::new(); + replacement_by_facet.resize(vertex_count, None); + + let mut current_usable_by_facet = + SmallBuffer::::new(); + current_usable_by_facet.resize(vertex_count, false); + + for facet_idx in 0..vertex_count { + let current_neighbor = old_neighbors.get(facet_idx).copied().flatten(); + let current_usable = current_neighbor.is_some_and(|neighbor_key| { + neighbor_slot_points_across_facet(tds, cell, facet_idx, neighbor_key) + }); + current_usable_by_facet[facet_idx] = current_usable; + if current_usable { + continue; + } + + let facet_hash = facet_hash_for_cell(cell, facet_idx); + let Some(incidents) = facet_map.get(&facet_hash) else { + continue; + }; + let [(c1, _), (c2, _)] = incidents.as_slice() else { + continue; + }; + + replacement_by_facet[facet_idx] = if *c1 == cell_key { + Some(*c2) + } else if *c2 == cell_key { + Some(*c1) + } else { + None + }; + } + + ( + vertex_count, + old_neighbors, + replacement_by_facet, + current_usable_by_facet, + ) + }; + + let mut rebuilt_neighbors = old_neighbors; + let mut changed = false; + for facet_idx in 0..vertex_count { + let current_neighbor = rebuilt_neighbors.get(facet_idx).copied().flatten(); + if current_usable_by_facet[facet_idx] { + continue; + } + + let replacement = replacement_by_facet[facet_idx]; + if current_neighbor != replacement { + rebuilt_neighbors[facet_idx] = replacement; + total_neighbor_slots_fixed = total_neighbor_slots_fixed.saturating_add(1); + changed = true; + } + } + + if !changed { + continue; + } + + let cell = tds + .cell_mut(cell_key) + .ok_or(NeighborWiringError::MissingCell { cell_key })?; + if rebuilt_neighbors.iter().all(Option::is_none) { + cell.neighbors = None; + } else { + cell.neighbors = Some(rebuilt_neighbors); + } + } + + #[cfg(debug_assertions)] + { + validate_no_neighbor_cycles_for_cells(tds, &affected_cells)?; + validate_neighbor_symmetry_for_cells(tds, &affected_cells)?; + } + + Ok(total_neighbor_slots_fixed) +} + /// Repair neighbor pointers using a global facet-incidence rebuild. /// /// This performs a **global** reconstruction of the cell-neighbor graph: @@ -2267,7 +2618,6 @@ fn validate_no_neighbor_cycles( tds: &Tds, ) -> Result<(), InsertionError> where - T: CoordinateScalar, U: DataType, V: DataType, { @@ -2324,6 +2674,127 @@ where Ok(()) } +/// Validate neighbor walks from a local affected set after partial pointer repair. +#[cfg(debug_assertions)] +fn validate_no_neighbor_cycles_for_cells( + tds: &Tds, + sample_cells: &FastHashSet, +) -> Result<(), InsertionError> +where + U: DataType, + V: DataType, +{ + let sample_cells: Vec = sample_cells.iter().copied().take(10).collect(); + let max_cells = tds.number_of_cells(); + + for &start_cell in &sample_cells { + let mut visited: FastHashSet = FastHashSet::default(); + let mut to_visit = vec![start_cell]; + visited.insert(start_cell); + + while let Some(current) = to_visit.pop() { + let cell = tds + .cell(current) + .ok_or(NeighborWiringError::MissingCell { cell_key: current })?; + + let Some(neighbors) = cell.neighbors() else { + continue; + }; + + for &neighbor_opt in neighbors { + let Some(neighbor_key) = neighbor_opt else { + continue; + }; + + if neighbor_key == current { + return Err(NeighborWiringError::SelfNeighbor { cell_key: current }.into()); + } + + if !tds.contains_cell(neighbor_key) { + return Err(NeighborWiringError::MissingNeighborTarget { + cell_key: current, + neighbor_key, + } + .into()); + } + + if visited.insert(neighbor_key) { + to_visit.push(neighbor_key); + if visited.len() > max_cells { + return Err(NeighborWiringError::NeighborWalkExceededCellCount { + visited: visited.len(), + total: max_cells, + } + .into()); + } + } + } + } + } + + Ok(()) +} + +/// Check mirror-facet symmetry for neighbor slots touched by local repair. +#[cfg(debug_assertions)] +fn validate_neighbor_symmetry_for_cells( + tds: &Tds, + affected_cells: &FastHashSet, +) -> Result<(), InsertionError> +where + U: DataType, + V: DataType, +{ + for &cell_key in affected_cells { + let cell = tds + .cell(cell_key) + .ok_or(NeighborWiringError::MissingCell { cell_key })?; + let Some(neighbors) = cell.neighbors() else { + continue; + }; + + for (facet_idx, &neighbor_opt) in neighbors.iter().enumerate() { + let Some(neighbor_key) = neighbor_opt else { + continue; + }; + + if neighbor_key == cell_key { + return Err(NeighborWiringError::SelfNeighbor { cell_key }.into()); + } + + let Some(neighbor_cell) = tds.cell(neighbor_key) else { + return Err(NeighborWiringError::MissingNeighborTarget { + cell_key, + neighbor_key, + } + .into()); + }; + + let mirror_facet_index = cell.mirror_facet_index(facet_idx, neighbor_cell); + let neighbor_back = mirror_facet_index.and_then(|mirror_idx| { + neighbor_cell + .neighbors() + .and_then(|neighbor_neighbors| neighbor_neighbors.get(mirror_idx)) + .copied() + .flatten() + }); + + if neighbor_back != Some(cell_key) { + return Err(NeighborWiringError::AsymmetricNeighbor { + cell_key, + facet_index: facet_idx, + neighbor_key, + mirror_facet_index, + neighbor_back, + } + .into()); + } + } + } + + Ok(()) +} + /// Extend the convex hull by connecting an exterior vertex to visible boundary facets. /// /// This function is used when a vertex is outside the current convex hull. @@ -3203,6 +3674,34 @@ mod tests { use crate::vertex; use slotmap::KeyData; + /// Return one mutual neighbor pair from a test TDS. + fn first_neighbor_pair( + tds: &Tds, + ) -> Option<(CellKey, u8, CellKey, u8)> + where + U: DataType, + V: DataType, + { + for (cell_key, cell) in tds.cells() { + let Some(neighbors) = cell.neighbors() else { + continue; + }; + for (facet_idx, &neighbor_opt) in neighbors.iter().enumerate() { + let Some(neighbor_key) = neighbor_opt else { + continue; + }; + let Some(neighbor_cell) = tds.cell(neighbor_key) else { + continue; + }; + let mirror_idx = cell.mirror_facet_index(facet_idx, neighbor_cell)?; + let facet_idx = u8::try_from(facet_idx).ok()?; + let mirror_idx = u8::try_from(mirror_idx).ok()?; + return Some((cell_key, facet_idx, neighbor_key, mirror_idx)); + } + } + None + } + /// Macro to generate cavity filling tests for different dimensions macro_rules! test_fill_cavity { ($dim:literal, $initial_vertices:expr, $new_vertex:expr, $expected_facets:literal) => { @@ -3663,7 +4162,7 @@ mod tests { let v_d = tds .insert_vertex_with_mapping(vertex!([0.0, -1.0])) .unwrap(); - let v_e = tds.insert_vertex_with_mapping(vertex!([2.0, 0.0])).unwrap(); + let v_e = tds.insert_vertex_with_mapping(vertex!([1.0, 1.0])).unwrap(); let c1 = tds .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_c], None).unwrap()) @@ -3759,6 +4258,22 @@ mod tests { assert!(err.to_string().contains("has 2 vertices")); } + #[test] + fn test_neighbor_rebuild_error_unexpected_preserves_source_error() { + let source = InsertionError::DuplicateCoordinates { + coordinates: "[0.0, 0.0]".to_string(), + }; + + let err = NeighborRebuildError::from(source.clone()); + + match err { + NeighborRebuildError::Unexpected { source: boxed } => { + assert_eq!(*boxed, source); + } + other => panic!("expected unexpected neighbor repair error, got {other:?}"), + } + } + #[test] fn test_cavity_filling_retryability_inspects_construction_payloads() { let geometry_failure = TdsValidationFailure::Geometric { @@ -4286,6 +4801,269 @@ mod tests { assert!(tds.is_valid().is_ok()); } + macro_rules! test_repair_neighbor_pointers_local_dimensions { + ( + $dim:literal, + $initial_vertices:expr, + $shared_facet_vertices:expr, + $opposite_vertices:expr + ) => { + pastey::paste! { + #[test] + fn []() { + let vertices = $initial_vertices; + let mut dt = DelaunayTriangulation::<_, (), (), $dim>::new(&vertices).unwrap(); + let tds = dt.tds_mut(); + let (cell_key, facet_idx, neighbor_key, _) = + first_neighbor_pair(tds).expect("test triangulation should have adjacent cells"); + + set_neighbor(tds, cell_key, facet_idx, None).unwrap(); + let repaired = repair_neighbor_pointers_local(tds, &[cell_key], Some(&[neighbor_key])) + .expect("local repair should reconstruct the missing slot"); + + assert_eq!(repaired, 1); + let cell = tds.cell(cell_key).unwrap(); + assert_eq!( + cell.neighbors() + .and_then(|neighbors| neighbors.get(usize::from(facet_idx))) + .copied() + .flatten(), + Some(neighbor_key) + ); + assert!(tds.is_valid().is_ok()); + } + + #[test] + fn []() { + let mut tds: Tds = Tds::empty(); + let shared_vertices = $shared_facet_vertices; + let opposite_vertices = $opposite_vertices; + + let mut shared_keys = Vec::new(); + for vertex in shared_vertices { + shared_keys.push(tds.insert_vertex_with_mapping(vertex).unwrap()); + } + + let mut cell_keys = Vec::new(); + for vertex in opposite_vertices { + let opposite_key = tds.insert_vertex_with_mapping(vertex).unwrap(); + let mut vertices = shared_keys.clone(); + vertices.push(opposite_key); + cell_keys.push( + tds.insert_cell_with_mapping(Cell::new(vertices, None).unwrap()) + .unwrap(), + ); + } + + let err = repair_neighbor_pointers_local(&mut tds, &cell_keys, None).unwrap_err(); + + assert!(matches!( + err, + InsertionError::NonManifoldTopology { cell_count: 3, .. } + )); + } + } + }; + } + + test_repair_neighbor_pointers_local_dimensions!( + 2, + vec![ + vertex!([0.0, 0.0]), + vertex!([1.0, 0.0]), + vertex!([0.0, 1.0]), + vertex!([1.0, 1.1]), + ], + vec![vertex!([0.0, 0.0]), vertex!([1.0, 0.0])], + vec![ + vertex!([0.0, 1.0]), + vertex!([0.0, -1.0]), + vertex!([2.0, 0.0]), + ] + ); + + test_repair_neighbor_pointers_local_dimensions!( + 3, + vec![ + vertex!([0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0]), + vertex!([0.0, 0.0, 1.0]), + vertex!([0.25, 0.25, 0.25]), + ], + vec![ + vertex!([0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0]), + ], + vec![ + vertex!([0.0, 0.0, 1.0]), + vertex!([0.0, 0.0, -1.0]), + vertex!([1.0, 1.0, 1.0]), + ] + ); + + test_repair_neighbor_pointers_local_dimensions!( + 4, + vec![ + vertex!([0.0, 0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 1.0, 0.0]), + vertex!([0.0, 0.0, 0.0, 1.0]), + vertex!([0.2, 0.2, 0.2, 0.2]), + ], + vec![ + vertex!([0.0, 0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 1.0, 0.0]), + ], + vec![ + vertex!([0.0, 0.0, 0.0, 1.0]), + vertex!([0.0, 0.0, 0.0, -1.0]), + vertex!([1.0, 1.0, 1.0, 1.0]), + ] + ); + + test_repair_neighbor_pointers_local_dimensions!( + 5, + vec![ + vertex!([0.0, 0.0, 0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 1.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 0.0, 1.0, 0.0]), + vertex!([0.0, 0.0, 0.0, 0.0, 1.0]), + vertex!([0.15, 0.15, 0.15, 0.15, 0.15]), + ], + vec![ + vertex!([0.0, 0.0, 0.0, 0.0, 0.0]), + vertex!([1.0, 0.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 1.0, 0.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 1.0, 0.0, 0.0]), + vertex!([0.0, 0.0, 0.0, 1.0, 0.0]), + ], + vec![ + vertex!([0.0, 0.0, 0.0, 0.0, 1.0]), + vertex!([0.0, 0.0, 0.0, 0.0, -1.0]), + vertex!([1.0, 1.0, 1.0, 1.0, 1.0]), + ] + ); + + #[test] + fn test_repair_neighbor_pointers_local_replaces_stale_neighbor_slot() { + let vertices = vec![ + vertex!([0.0, 0.0]), + vertex!([1.0, 0.0]), + vertex!([0.0, 1.0]), + vertex!([1.0, 1.1]), + ]; + let mut dt = DelaunayTriangulation::<_, (), (), 2>::new(&vertices).unwrap(); + let tds = dt.tds_mut(); + let (cell_key, facet_idx, neighbor_key, _) = + first_neighbor_pair(tds).expect("test triangulation should have adjacent cells"); + let stale_neighbor = CellKey::from(KeyData::from_ffi(u64::MAX - 7)); + assert!(!tds.contains_cell(stale_neighbor)); + + set_neighbor(tds, cell_key, facet_idx, Some(stale_neighbor)).unwrap(); + let repaired = repair_neighbor_pointers_local(tds, &[cell_key], Some(&[neighbor_key])) + .expect("local repair should replace a stale neighbor slot"); + + assert_eq!(repaired, 1); + let cell = tds.cell(cell_key).unwrap(); + assert_eq!( + cell.neighbors() + .and_then(|neighbors| neighbors.get(usize::from(facet_idx))) + .copied() + .flatten(), + Some(neighbor_key) + ); + assert!(tds.is_valid().is_ok()); + } + + #[test] + fn test_repair_neighbor_pointers_local_replaces_wrong_live_neighbor_slot() { + let mut tds: Tds = Tds::empty(); + + let v_a = tds.insert_vertex_with_mapping(vertex!([0.0, 0.0])).unwrap(); + let v_b = tds.insert_vertex_with_mapping(vertex!([1.0, 0.0])).unwrap(); + let v_c = tds.insert_vertex_with_mapping(vertex!([0.0, 1.0])).unwrap(); + let v_d = tds + .insert_vertex_with_mapping(vertex!([0.0, -1.0])) + .unwrap(); + let v_e = tds.insert_vertex_with_mapping(vertex!([2.0, 0.0])).unwrap(); + let v_f = tds.insert_vertex_with_mapping(vertex!([2.0, 1.0])).unwrap(); + + let c1 = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_c], None).unwrap()) + .unwrap(); + let c2 = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_d], None).unwrap()) + .unwrap(); + let wrong_live_neighbor = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_e, v_f], None).unwrap()) + .unwrap(); + + let shared_facet_idx = 2usize; + let shared_facet_idx_u8 = u8::try_from(shared_facet_idx).unwrap(); + assert!( + tds.cell(c1) + .unwrap() + .mirror_facet_index(shared_facet_idx, tds.cell(wrong_live_neighbor).unwrap()) + .is_none() + ); + + set_neighbor(&mut tds, c1, shared_facet_idx_u8, Some(wrong_live_neighbor)).unwrap(); + set_neighbor(&mut tds, c2, shared_facet_idx_u8, Some(c1)).unwrap(); + + let repaired = repair_neighbor_pointers_local(&mut tds, &[c1], Some(&[c2])) + .expect("local repair should replace a live neighbor across the wrong facet"); + + assert_eq!(repaired, 1); + let cell = tds.cell(c1).unwrap(); + assert_eq!( + cell.neighbors() + .and_then(|neighbors| neighbors.get(shared_facet_idx)) + .copied() + .flatten(), + Some(c2) + ); + } + + #[test] + fn test_repair_neighbor_pointers_local_does_not_scan_unseeded_cells() { + let vertices = vec![ + vertex!([0.0, 0.0]), + vertex!([1.0, 0.0]), + vertex!([0.0, 1.0]), + vertex!([1.0, 1.1]), + vertex!([0.5, 0.35]), + ]; + let mut dt = DelaunayTriangulation::<_, (), (), 2>::new(&vertices).unwrap(); + let tds = dt.tds_mut(); + let (cell_key, facet_idx, _neighbor_key, _) = + first_neighbor_pair(tds).expect("test triangulation should have adjacent cells"); + + set_neighbor(tds, cell_key, facet_idx, None).unwrap(); + let repaired = repair_neighbor_pointers_local(tds, &[], None) + .expect("local repair should ignore unseeded damage"); + + assert_eq!(repaired, 0); + let cell = tds.cell(cell_key).unwrap(); + assert_eq!( + cell.neighbors() + .and_then(|neighbors| neighbors.get(usize::from(facet_idx))) + .copied() + .flatten(), + None + ); + + // The global repair still sees the missing slot, proving the local path was scoped. + assert!(repair_neighbor_pointers(tds).unwrap() > 0); + assert!(tds.is_valid().is_ok()); + } + #[test] fn test_extend_hull_adds_cells_for_exterior_vertex() { let vertices = vec![ diff --git a/src/core/triangulation.rs b/src/core/triangulation.rs index fc581f28..ce7eab58 100644 --- a/src/core/triangulation.rs +++ b/src/core/triangulation.rs @@ -110,7 +110,8 @@ use crate::core::adjacency::{AdjacencyIndex, AdjacencyIndexBuildError}; use crate::core::algorithms::incremental_insertion::{ CavityFillingError, CavityRepairStage, HullExtensionReason, InsertionError, extend_hull, - external_facets_for_boundary, fill_cavity, repair_neighbor_pointers, wire_cavity_neighbors, + external_facets_for_boundary, fill_cavity, repair_neighbor_pointers, + repair_neighbor_pointers_local, wire_cavity_neighbors, }; #[cfg(debug_assertions)] use crate::core::algorithms::locate::locate_with_stats; @@ -195,6 +196,7 @@ static DUPLICATE_DETECTION_GRID_CANDIDATES: AtomicU64 = AtomicU64::new(0); static DUPLICATE_DETECTION_ENABLED: OnceLock = OnceLock::new(); static RETRYABLE_SKIP_TRACE_ENABLED: OnceLock = OnceLock::new(); static CAVITY_REDUCTION_TRACE_ENABLED: OnceLock = OnceLock::new(); +static FORCE_GLOBAL_NEIGHBOR_REBUILD_ENABLED: OnceLock = OnceLock::new(); static CAVITY_REDUCTION_TRACE_EMITTED: AtomicBool = AtomicBool::new(false); #[cfg(test)] @@ -244,6 +246,12 @@ fn cavity_reduction_trace_enabled() -> bool { .get_or_init(|| env::var_os("DELAUNAY_DEBUG_CAVITY_REDUCTION_ONCE").is_some()) } +/// Returns whether local neighbor repair should be bypassed for regression isolation. +fn force_global_neighbor_rebuild_enabled() -> bool { + *FORCE_GLOBAL_NEIGHBOR_REBUILD_ENABLED + .get_or_init(|| env::var_os("DELAUNAY_FORCE_GLOBAL_NEIGHBOR_REBUILD").is_some()) +} + /// Extracts a compact one-line summary for retryable conflict-region failures. /// /// These summaries are designed for the large-scale debug harness logs, where we want @@ -836,6 +844,24 @@ struct TryInsertImplOk { repair_seed_cells: CellKeyBuffer, } +/// Internal result from over-shared-facet repair, including the surviving frontier +/// that should seed local neighbor-pointer repair. +struct LocalFacetRepairOutcome { + /// Number of cells actually removed from the TDS. + removed_count: usize, + /// Cells selected for removal before they were deleted. + #[cfg_attr( + not(debug_assertions), + expect( + dead_code, + reason = "Removed-cell keys are retained for debug logging and future local repair diagnostics" + ) + )] + removed_cells: CellKeyBuffer, + /// Surviving one-hop neighbors whose back-references may have been cleared. + frontier_cells: CellKeyBuffer, +} + enum InsertionSite<'a> { Interior { start_cell: CellKey, @@ -4125,6 +4151,71 @@ where } } + /// Repair neighbor pointers after local cell removal without scanning the full TDS. + fn repair_neighbors_after_local_cell_removal( + &mut self, + new_cells: &CellKeyBuffer, + frontier_cells: &[CellKey], + ) -> Result { + #[cfg(debug_assertions)] + tracing::debug!( + cells = self.tds.number_of_cells(), + surviving_new_cell_seeds = new_cells + .iter() + .filter(|&&cell_key| self.tds.contains_cell(cell_key)) + .count(), + frontier_cell_seeds = frontier_cells + .iter() + .filter(|&&cell_key| self.tds.contains_cell(cell_key)) + .count(), + "Before local neighbor-pointer repair" + ); + + if force_global_neighbor_rebuild_enabled() { + #[cfg(debug_assertions)] + tracing::debug!( + "DELAUNAY_FORCE_GLOBAL_NEIGHBOR_REBUILD set; using global neighbor rebuild" + ); + return repair_neighbor_pointers(&mut self.tds).map_err(|source| { + CavityFillingError::NeighborRebuild { + reason: source.into(), + } + .into() + }); + } + + #[cfg(debug_assertions)] + { + match repair_neighbor_pointers_local(&mut self.tds, new_cells, Some(frontier_cells)) { + Ok(repaired) => Ok(repaired), + Err(local_error) => { + tracing::warn!( + error = %local_error, + "Local neighbor-pointer repair failed; falling back to global rebuild in debug mode" + ); + repair_neighbor_pointers(&mut self.tds).map_err(|source| { + CavityFillingError::NeighborRebuild { + reason: source.into(), + } + .into() + }) + } + } + } + + #[cfg(not(debug_assertions))] + { + repair_neighbor_pointers_local(&mut self.tds, new_cells, Some(frontier_cells)).map_err( + |source| { + CavityFillingError::NeighborRebuild { + reason: source.into(), + } + .into() + }, + ) + } + } + /// Attempt an insertion, and if Level 3 validation fails, roll back and try a /// conservative star-split fallback of the containing cell. fn try_insert_with_topology_safety_net( @@ -4908,6 +4999,8 @@ where // Iteratively repair non-manifold topology until facet sharing is valid let mut total_removed = 0; + let mut facet_sharing_known_valid = true; + let mut neighbor_repair_frontier = CellKeyBuffer::new(); #[cfg_attr( not(debug_assertions), expect( @@ -4935,7 +5028,8 @@ where issues.len() ); - let removed = self.repair_local_facet_issues(&issues)?; + let repair = self.repair_local_facet_issues_with_frontier(&issues)?; + let removed = repair.removed_count; // Early exit if repair made no progress if removed == 0 { @@ -4955,16 +5049,21 @@ where } total_removed += removed; + neighbor_repair_frontier.extend(repair.frontier_cells); if removed > 0 { suspicion.cells_removed = true; } #[cfg(debug_assertions)] - tracing::debug!("Removed {removed} cells (total: {total_removed})"); + tracing::debug!( + removed_cells = ?repair.removed_cells, + "Removed {removed} cells (total: {total_removed})" + ); // Early exit if repair succeeded - if self.tds.validate_facet_sharing().is_ok() { + facet_sharing_known_valid = self.tds.validate_facet_sharing().is_ok(); + if facet_sharing_known_valid { break; } } else { @@ -4977,34 +5076,21 @@ where #[cfg(debug_assertions)] tracing::debug!("After repair loop: total_removed={total_removed}"); + if !facet_sharing_known_valid { + return Err(CavityFillingError::InvalidFacetSharingAfterRepair { + stage: CavityRepairStage::PrimaryInsertion, + } + .into()); + } + // Global neighbor rebuild is expensive. In the common case (no cells removed during the // local facet-repair loop), `wire_cavity_neighbors` has already glued the cavity locally. // - // If we *did* remove cells during the repair loop, neighbor pointers may no longer match - // facet incidence, so we rebuild globally. + // If we *did* remove cells during the repair loop, repair only the new-cell/frontier + // neighborhood unless the force-rebuild diagnostic environment variable is set. if total_removed > 0 { - // Double-check that facet sharing is actually valid. - let facet_valid = self.tds.validate_facet_sharing().is_ok(); - #[cfg(debug_assertions)] - tracing::debug!( - "Before repair_neighbor_pointers: facet_sharing_valid={facet_valid}, cells={}", - self.tds.number_of_cells() - ); - - if !facet_valid { - return Err(CavityFillingError::InvalidFacetSharingAfterRepair { - stage: CavityRepairStage::PrimaryInsertion, - } - .into()); - } - - // Rebuild neighbor pointers from facet incidence (global O(n·D) pass). - // This is only needed after we removed cells in the local repair loop. - let repaired = repair_neighbor_pointers(&mut self.tds).map_err(|source| { - CavityFillingError::NeighborRebuild { - reason: source.into(), - } - })?; + let repaired = self + .repair_neighbors_after_local_cell_removal(&new_cells, &neighbor_repair_frontier)?; suspicion.neighbor_pointers_rebuilt = repaired > 0; } @@ -5584,6 +5670,8 @@ where // Iteratively repair non-manifold topology until facet sharing is valid let mut total_removed = 0; + let mut facet_sharing_known_valid = true; + let mut neighbor_repair_frontier = CellKeyBuffer::new(); #[cfg_attr( not(debug_assertions), expect( @@ -5612,7 +5700,8 @@ where issues.len() ); - let removed = self.repair_local_facet_issues(&issues)?; + let repair = self.repair_local_facet_issues_with_frontier(&issues)?; + let removed = repair.removed_count; // Early exit if repair made no progress if removed == 0 { @@ -5632,15 +5721,20 @@ where } total_removed += removed; + neighbor_repair_frontier.extend(repair.frontier_cells); if removed > 0 { suspicion.cells_removed = true; } #[cfg(debug_assertions)] - tracing::debug!("Removed {removed} cells (total: {total_removed})"); + tracing::debug!( + removed_cells = ?repair.removed_cells, + "Removed {removed} cells (total: {total_removed})" + ); // Early exit if repair succeeded - if self.tds.validate_facet_sharing().is_ok() { + facet_sharing_known_valid = self.tds.validate_facet_sharing().is_ok(); + if facet_sharing_known_valid { break; } } else { @@ -5649,30 +5743,19 @@ where } } - // Rebuild neighbor pointers now that topology is manifold - if total_removed > 0 { - // Double-check that facet sharing is actually valid - let facet_valid = self.tds.validate_facet_sharing().is_ok(); - #[cfg(debug_assertions)] - tracing::debug!( - "Before repair_neighbor_pointers: facet_sharing_valid={facet_valid}, cells={}", - self.tds.number_of_cells() - ); - - if !facet_valid { - return Err(CavityFillingError::InvalidFacetSharingAfterRepair { - stage: CavityRepairStage::FanTriangulation, - } - .into()); + // Repair neighbor pointers now that topology is manifold. + if !facet_sharing_known_valid { + return Err(CavityFillingError::InvalidFacetSharingAfterRepair { + stage: CavityRepairStage::FanTriangulation, } + .into()); + } - // Rebuild neighbor pointers from facet incidence (global O(n·D) pass). - // This is only needed after we removed cells in the local repair loop. - let repaired = repair_neighbor_pointers(&mut self.tds).map_err(|source| { - CavityFillingError::NeighborRebuild { - reason: source.into(), - } - })?; + if total_removed > 0 { + let repaired = self.repair_neighbors_after_local_cell_removal( + &new_cells, + &neighbor_repair_frontier, + )?; suspicion.neighbor_pointers_rebuilt = repaired > 0; } @@ -6079,56 +6162,11 @@ where } } - /// Repairs over-shared facets by removing lower-quality cells. - /// - /// Uses geometric quality metrics (`radius_ratio`) to select which cells to keep - /// when a facet is shared by more than 2 cells. UUID ordering is used as a tie-breaker - /// when cells have equal quality. Errors if quality computation or conversion fails. - /// - /// # Performance - /// - /// - **Complexity**: O(m * q) where m = number of problematic facets, q = quality computation cost - /// - **Localized**: Only processes cells involved in detected issues - /// - /// # Arguments - /// - /// * `issues` - Detected facet issues map from `detect_local_facet_issues()` - /// - /// # Returns - /// - /// Number of cells removed during repair. - /// - /// # Errors - /// - /// Returns error if quality evaluation or facet bookkeeping fails while - /// selecting cells to remove. This function itself does not rebuild neighbors; - /// callers are responsible for repairing or validating topology after removal - /// (e.g., via `repair_neighbor_pointers` or a validation pass). - /// - /// # Examples - /// - /// ```rust - /// use delaunay::prelude::collections::FacetIssuesMap; - /// use delaunay::prelude::triangulation::*; - /// - /// // Start with a valid 2D simplex. - /// let vertices = vec![ - /// vertex!([0.0, 0.0]), - /// vertex!([1.0, 0.0]), - /// vertex!([0.0, 1.0]), - /// ]; - /// let mut dt: DelaunayTriangulation<_, (), (), 2> = DelaunayTriangulation::new(&vertices).unwrap(); - /// - /// // Empty issues map => nothing to remove. - /// let mut tri = dt.as_triangulation().clone(); - /// let removed = tri.repair_local_facet_issues(&FacetIssuesMap::default()).unwrap(); - /// assert_eq!(removed, 0); - /// ``` - /// - /// In practice, this method is typically called with issues detected by - /// [`detect_local_facet_issues`](Self::detect_local_facet_issues) after insertion/removal - /// operations. See `insert_transactional` for a typical usage pattern. - pub fn repair_local_facet_issues(&mut self, issues: &FacetIssuesMap) -> Result + /// Select cells to remove for over-shared-facet repair without mutating the TDS. + fn cells_for_local_facet_issue_repair( + &self, + issues: &FacetIssuesMap, + ) -> Result where K::Scalar: Div, { @@ -6136,11 +6174,9 @@ where // For each over-shared facet, select cells to remove for cell_facet_pairs in issues.values() { - let involved_cells: Vec = cell_facet_pairs.iter().map(|(ck, _)| *ck).collect(); - // Compute quality for each cell - propagate errors from quality evaluation let mut cell_qualities: Vec<(CellKey, f64, Uuid)> = Vec::new(); - for &cell_key in &involved_cells { + for &(cell_key, _) in cell_facet_pairs { let cell = self .tds .cell(cell_key) @@ -6189,12 +6225,147 @@ where } } - // Remove the selected cells - do NOT rebuild neighbors here - // Neighbor wiring should happen AFTER all non-manifold issues are resolved - let to_remove: Vec = cells_to_remove.into_iter().collect(); + Ok(cells_to_remove.into_iter().collect()) + } + + /// Collect surviving neighbor cells that will have removed-cell back-references cleared. + fn removal_frontier_for_cells(&self, cells_to_remove: &[CellKey]) -> CellKeyBuffer { + if cells_to_remove.is_empty() { + return CellKeyBuffer::new(); + } + + let removal_set: CellKeySet = cells_to_remove.iter().copied().collect(); + let mut frontier = CellKeyBuffer::new(); + let mut seen = FastHashSet::default(); + + for &cell_key in cells_to_remove { + let Some(cell) = self.tds.cell(cell_key) else { + continue; + }; + let Some(neighbors) = cell.neighbors() else { + continue; + }; + for &neighbor_key in neighbors.iter().flatten() { + if removal_set.contains(&neighbor_key) || !self.tds.contains_cell(neighbor_key) { + continue; + } + if seen.insert(neighbor_key) { + frontier.push(neighbor_key); + } + } + } + + frontier + } + + /// Add surviving cells from the facet-issue incidence map to the local repair frontier. + fn add_issue_survivors_to_frontier( + &self, + issues: &FacetIssuesMap, + cells_to_remove: &[CellKey], + frontier: &mut CellKeyBuffer, + ) { + let removal_set: CellKeySet = cells_to_remove.iter().copied().collect(); + let mut seen: FastHashSet = frontier.iter().copied().collect(); + + for cell_facet_pairs in issues.values() { + for &(cell_key, _) in cell_facet_pairs { + if removal_set.contains(&cell_key) || !self.tds.contains_cell(cell_key) { + continue; + } + if seen.insert(cell_key) { + frontier.push(cell_key); + } + } + } + } + + /// Repair over-shared facets and return the local frontier for neighbor repair. + fn repair_local_facet_issues_with_frontier( + &mut self, + issues: &FacetIssuesMap, + ) -> Result + where + K::Scalar: Div, + { + let to_remove = self.cells_for_local_facet_issue_repair(issues)?; + let mut frontier_cells = self.removal_frontier_for_cells(&to_remove); + self.add_issue_survivors_to_frontier(issues, &to_remove, &mut frontier_cells); let removed_count = self.tds.remove_cells_by_keys(&to_remove); - Ok(removed_count) + Ok(LocalFacetRepairOutcome { + removed_count, + removed_cells: to_remove, + frontier_cells, + }) + } + + /// Repairs over-shared facets by removing lower-quality cells. + /// + /// Uses geometric quality metrics (`radius_ratio`) to select which cells to keep + /// when a facet is shared by more than 2 cells. UUID ordering is used as a tie-breaker + /// when cells have equal quality. Errors if quality computation or conversion fails. + /// + /// # Performance + /// + /// - **Complexity**: O(m * q) where m = number of problematic facets, q = quality computation cost + /// - **Localized**: Only processes cells involved in detected issues + /// + /// # Arguments + /// + /// * `issues` - Detected facet issues map from `detect_local_facet_issues()` + /// + /// # Returns + /// + /// Number of cells removed during repair. + /// + /// # Errors + /// + /// Returns error if quality evaluation or facet bookkeeping fails while + /// selecting cells to remove. This function itself does not rebuild neighbors; + /// callers are responsible for repairing or validating topology after removal + /// (e.g., via local or global neighbor-pointer repair, or a validation pass). + /// + /// # Examples + /// + /// ```rust + /// use delaunay::prelude::collections::FacetIssuesMap; + /// use delaunay::prelude::tds::TdsError; + /// use delaunay::prelude::triangulation::*; + /// + /// # #[derive(Debug, thiserror::Error)] + /// # enum ExampleError { + /// # #[error(transparent)] + /// # Construction(#[from] DelaunayTriangulationConstructionError), + /// # #[error(transparent)] + /// # Tds(#[from] TdsError), + /// # } + /// # fn main() -> Result<(), ExampleError> { + /// // Start with a valid 2D simplex. + /// let vertices = vec![ + /// vertex!([0.0, 0.0]), + /// vertex!([1.0, 0.0]), + /// vertex!([0.0, 1.0]), + /// ]; + /// let dt: DelaunayTriangulation<_, (), (), 2> = DelaunayTriangulation::new(&vertices)?; + /// + /// // Empty issues map => nothing to remove. + /// let mut tri = dt.as_triangulation().clone(); + /// let removed = tri.repair_local_facet_issues(&FacetIssuesMap::default())?; + /// assert_eq!(removed, 0); + /// # Ok(()) + /// # } + /// ``` + /// + /// In practice, this method is typically called with issues detected by + /// [`detect_local_facet_issues`](Self::detect_local_facet_issues) after insertion/removal + /// operations. See `insert_transactional` for a typical usage pattern. + pub fn repair_local_facet_issues(&mut self, issues: &FacetIssuesMap) -> Result + where + K::Scalar: Div, + { + self.repair_local_facet_issues_with_frontier(issues) + .map(|outcome| outcome.removed_count) } } @@ -10582,6 +10753,129 @@ mod tests { assert!(removed > 0, "Should remove at least one duplicate cell"); } + /// Return the facet index opposite the vertex not on the tested shared edge. + fn shared_edge_facet_index( + cell: &Cell, + v_a: VertexKey, + v_b: VertexKey, + ) -> usize { + cell.vertices() + .iter() + .position(|&vertex_key| vertex_key != v_a && vertex_key != v_b) + .expect("test cells should contain the shared edge") + } + + /// Read the neighbor slot across the tested shared edge in a 2D repair fixture. + fn neighbor_across_shared_edge( + tri: &Triangulation, (), (), 2>, + cell_key: CellKey, + v_a: VertexKey, + v_b: VertexKey, + ) -> Option { + let cell = tri.tds.cell(cell_key).unwrap(); + let facet_idx = shared_edge_facet_index(cell, v_a, v_b); + cell.neighbors() + .and_then(|neighbors| neighbors.get(facet_idx)) + .copied() + .flatten() + } + + #[test] + fn test_local_repair_uses_removal_frontier() { + let mut tds: Tds = Tds::empty(); + + let v_a = tds.insert_vertex_with_mapping(vertex!([0.0, 0.0])).unwrap(); + let v_b = tds.insert_vertex_with_mapping(vertex!([1.0, 0.0])).unwrap(); + let v_c = tds.insert_vertex_with_mapping(vertex!([0.0, 1.0])).unwrap(); + let v_d = tds + .insert_vertex_with_mapping(vertex!([0.0, -1.0])) + .unwrap(); + let v_e = tds.insert_vertex_with_mapping(vertex!([1.0, 1.0])).unwrap(); + + let c1 = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_c], None).unwrap()) + .unwrap(); + let c2 = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_d], None).unwrap()) + .unwrap(); + let c3 = tds + .insert_cell_with_mapping(Cell::new(vec![v_a, v_b, v_e], None).unwrap()) + .unwrap(); + + for (cell_key, neighbor_key) in [(c1, c2), (c2, c3), (c3, c1)] { + let cell = tds.cell_mut(cell_key).unwrap(); + let mut neighbors = NeighborBuffer::>::new(); + neighbors.resize(3, None); + neighbors[2] = Some(neighbor_key); + cell.neighbors = Some(neighbors); + } + tds.assign_incident_cells().unwrap(); + + let mut tri = + Triangulation::, (), (), 2>::new_with_tds(FastKernel::new(), tds); + let original_cells = [c1, c2, c3]; + let issues = tri + .detect_local_facet_issues(&original_cells) + .unwrap() + .expect("three cells sharing one edge should be detected as over-shared"); + + let repair = tri + .repair_local_facet_issues_with_frontier(&issues) + .unwrap(); + assert_eq!(repair.removed_count, 1); + assert!( + !repair.frontier_cells.is_empty(), + "removed-cell neighbors should seed the local repair frontier" + ); + + let survivors: Vec<_> = original_cells + .into_iter() + .filter(|cell_key| tri.tds.contains_cell(*cell_key)) + .collect(); + assert_eq!(survivors.len(), 2); + let [first_survivor, second_survivor] = survivors.as_slice() else { + panic!("fixture should leave exactly two surviving cells"); + }; + for &survivor in &survivors { + assert!( + repair.frontier_cells.contains(&survivor), + "facet-issue survivors should seed the local repair frontier" + ); + } + let survivor_pairs = [ + (*first_survivor, *second_survivor), + (*second_survivor, *first_survivor), + ]; + + let missing_shared_slots_before = survivor_pairs + .iter() + .filter(|&&(cell_key, other)| { + neighbor_across_shared_edge(&tri, cell_key, v_a, v_b) != Some(other) + }) + .count(); + assert!( + missing_shared_slots_before > 0, + "cell removal should leave at least one survivor slot needing local repair" + ); + + let mut new_cells = CellKeyBuffer::new(); + new_cells.extend(original_cells); + let repaired = tri + .repair_neighbors_after_local_cell_removal(&new_cells, &repair.frontier_cells) + .unwrap(); + + assert!(repaired > 0); + for (cell_key, other) in survivor_pairs { + assert_eq!( + neighbor_across_shared_edge(&tri, cell_key, v_a, v_b), + Some(other), + "surviving cells should be rewired across the formerly over-shared edge" + ); + } + assert!(tri.tds.validate_facet_sharing().is_ok()); + assert!(tri.detect_local_facet_issues(&survivors).unwrap().is_none()); + } + // ========================================================================= // DUPLICATE COORDINATES ERROR: LINEAR FALLBACK (NO INDEX) // ========================================================================= diff --git a/src/lib.rs b/src/lib.rs index 6300eed1..5c0d5c14 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -50,6 +50,7 @@ //! | Task | Import | //! |---|---| //! | Build a triangulation, insert/remove vertices | `use delaunay::prelude::triangulation::*` | +//! | Low-level incremental insertion building blocks | `use delaunay::prelude::triangulation::insertion::*` | //! | Read-only queries, traversal, convex hull | `use delaunay::prelude::query::*` | //! | Point location and conflict-region algorithms | `use delaunay::prelude::algorithms::*` | //! | Geometry helpers, predicates, points | `use delaunay::prelude::geometry::*` | @@ -996,14 +997,14 @@ pub mod prelude { CavityFillingError, CavityRepairStage, HullExtensionReason, InitialSimplexConstructionError, InsertionError, NeighborRebuildError, NeighborWiringError, TdsConstructionFailure, TdsValidationFailure, extend_hull, - fill_cavity, wire_cavity_neighbors, + fill_cavity, repair_neighbor_pointers_local, wire_cavity_neighbors, }; pub use crate::core::collections::CellKeyBuffer; pub use crate::core::facet::FacetHandle; pub use crate::core::operations::{ InsertionOutcome, InsertionResult, InsertionStatistics, }; - pub use crate::core::tds::{CellKey, Tds, VertexKey}; + pub use crate::core::tds::{CellKey, Tds, TdsMutationError, VertexKey}; } /// Topological operation telemetry and repair decisions. diff --git a/tests/delaunay_incremental_insertion.rs b/tests/delaunay_incremental_insertion.rs index 7a3cdd4d..3a7a4122 100644 --- a/tests/delaunay_incremental_insertion.rs +++ b/tests/delaunay_incremental_insertion.rs @@ -8,8 +8,118 @@ //! - Different kernels (Fast vs Robust) use approx::assert_relative_eq; +use delaunay::prelude::algorithms::{LocateResult, find_conflict_region, locate}; +use delaunay::prelude::collections::MAX_PRACTICAL_DIMENSION_SIZE; +use delaunay::prelude::geometry::{AdaptiveKernel, Coordinate, Point}; +use delaunay::prelude::tds::{Cell, CellKey, SmallBuffer, VertexKey, facet_key_from_vertices}; use delaunay::prelude::triangulation::*; +/// Build the canonical facet key used to compare neighbor mirror facets in tests. +fn facet_key_for_cell(cell: &Cell, facet_idx: usize) -> u64 { + let mut facet_vertices = SmallBuffer::::new(); + for (idx, &vertex_key) in cell.vertices().iter().enumerate() { + if idx != facet_idx { + facet_vertices.push(vertex_key); + } + } + facet_key_from_vertices(&facet_vertices) +} + +/// Assert that every populated neighbor slot references an existing cell that points back. +fn assert_neighbors_valid_and_symmetric( + dt: &DelaunayTriangulation, (), (), D>, +) { + for (cell_key, cell) in dt.cells() { + let Some(neighbors) = cell.neighbors() else { + continue; + }; + for (facet_idx, &neighbor_opt) in neighbors.iter().enumerate() { + let Some(neighbor_key) = neighbor_opt else { + continue; + }; + let neighbor_cell = dt + .tds() + .cell(neighbor_key) + .unwrap_or_else(|| panic!("neighbor {neighbor_key:?} for {cell_key:?} is missing")); + let facet_key = facet_key_for_cell(cell, facet_idx); + let mirror_idx = (0..neighbor_cell.number_of_vertices()) + .find(|&idx| facet_key_for_cell(neighbor_cell, idx) == facet_key) + .unwrap_or_else(|| { + panic!( + "neighbor {neighbor_key:?} does not share facet {facet_idx} with {cell_key:?}" + ) + }); + let neighbor_back = neighbor_cell + .neighbors() + .and_then(|neighbor_neighbors| neighbor_neighbors.get(mirror_idx)) + .copied() + .flatten(); + assert_eq!( + neighbor_back, + Some(cell_key), + "neighbor symmetry failed for {cell_key:?} facet {facet_idx}" + ); + } + } +} + +/// Return a query point strictly inside the first cell so locate traversal has a stable target. +fn centroid_of_first_cell( + dt: &DelaunayTriangulation, (), (), D>, +) -> (CellKey, Point) { + let (cell_key, cell) = dt + .cells() + .next() + .expect("test triangulation should contain at least one cell"); + let mut coords = [0.0; D]; + for &vertex_key in cell.vertices() { + let vertex = dt + .tds() + .vertex(vertex_key) + .expect("cell vertex should exist"); + for (coord, &value) in coords.iter_mut().zip(vertex.point().coords()) { + *coord += value; + } + } + let mut vertex_count = 0.0; + for _ in cell.vertices() { + vertex_count += 1.0; + } + let scale = 1.0 / vertex_count; + for coord in &mut coords { + *coord *= scale; + } + (cell_key, Point::new(coords)) +} + +/// Verify repaired neighbor pointers support hinted locate and conflict-region traversal. +fn assert_locate_and_conflict_traversal( + dt: &DelaunayTriangulation, (), (), D>, +) { + let kernel = AdaptiveKernel::::new(); + let (hint_cell, query) = centroid_of_first_cell(dt); + + let no_hint = locate(dt.tds(), &kernel, &query, None).expect("locate without hint failed"); + let with_hint = + locate(dt.tds(), &kernel, &query, Some(hint_cell)).expect("locate with hint failed"); + + let start_cell = match with_hint { + LocateResult::InsideCell(cell_key) => cell_key, + other => panic!("centroid should locate inside a cell with hint, got {other:?}"), + }; + assert!( + matches!(no_hint, LocateResult::InsideCell(_)), + "centroid should locate inside a cell without hint, got {no_hint:?}" + ); + + let conflict_cells = find_conflict_region(dt.tds(), &kernel, &query, start_cell) + .expect("conflict traversal failed"); + assert!(!conflict_cells.is_empty()); + for &cell_key in &conflict_cells { + assert!(dt.tds().contains_cell(cell_key)); + } +} + // ========================================================================= // Basic Incremental Insertion Tests (using macros for 2D-5D) // ========================================================================= @@ -177,6 +287,98 @@ test_insert_5_points!( ] ); +macro_rules! test_local_neighbor_repair_guardrails { + ($dim:expr, [$($simplex:expr),+ $(,)?], [$($point:expr),+ $(,)?]) => { + pastey::paste! { + #[test] + fn []() { + let vertices = vec![ + $(vertex!($simplex)),+ + ]; + let mut dt: DelaunayTriangulation, (), (), $dim> = + DelaunayTriangulation::with_topology_guarantee( + &AdaptiveKernel::new(), + &vertices, + TopologyGuarantee::PLManifold, + ) + .unwrap(); + + for point in [$(vertex!($point)),+] { + dt.insert(point).unwrap(); + assert_neighbors_valid_and_symmetric(&dt); + assert_locate_and_conflict_traversal(&dt); + } + } + } + }; +} + +test_local_neighbor_repair_guardrails!( + 2, + [[0.0, 0.0], [4.0, 0.0], [0.0, 4.0]], + [ + [0.8, 0.8], + [1.6, 0.7], + [0.7, 1.5], + [1.2, 1.2], + [1.0, 1.0e-9] + ] +); + +test_local_neighbor_repair_guardrails!( + 3, + [ + [0.0, 0.0, 0.0], + [4.0, 0.0, 0.0], + [0.0, 4.0, 0.0], + [0.0, 0.0, 4.0] + ], + [ + [0.8, 0.8, 0.8], + [1.2, 0.6, 0.7], + [0.7, 1.1, 0.9], + [1.0, 0.9, 1.2], + [1.0e-9, 0.8, 0.8] + ] +); + +test_local_neighbor_repair_guardrails!( + 4, + [ + [0.0, 0.0, 0.0, 0.0], + [4.0, 0.0, 0.0, 0.0], + [0.0, 4.0, 0.0, 0.0], + [0.0, 0.0, 4.0, 0.0], + [0.0, 0.0, 0.0, 4.0] + ], + [ + [0.5, 0.5, 0.5, 0.5], + [0.8, 0.4, 0.5, 0.6], + [0.4, 0.8, 0.6, 0.5], + [0.6, 0.5, 0.8, 0.4], + [1.0e-9, 0.6, 0.6, 0.6] + ] +); + +test_local_neighbor_repair_guardrails!( + 5, + [ + [0.0, 0.0, 0.0, 0.0, 0.0], + [4.0, 0.0, 0.0, 0.0, 0.0], + [0.0, 4.0, 0.0, 0.0, 0.0], + [0.0, 0.0, 4.0, 0.0, 0.0], + [0.0, 0.0, 0.0, 4.0, 0.0], + [0.0, 0.0, 0.0, 0.0, 4.0] + ], + [ + [0.4, 0.4, 0.4, 0.4, 0.4], + [0.7, 0.3, 0.4, 0.5, 0.4], + [0.3, 0.7, 0.5, 0.4, 0.4], + [0.4, 0.5, 0.7, 0.3, 0.4], + [1.0e-9, 0.5, 0.5, 0.5, 0.5] + ] +); + // ========================================================================= // Kernel Comparison Tests // ========================================================================= diff --git a/tests/prelude_exports.rs b/tests/prelude_exports.rs index ed88f16a..4b0774d0 100644 --- a/tests/prelude_exports.rs +++ b/tests/prelude_exports.rs @@ -30,11 +30,14 @@ use delaunay::prelude::ordering::{ use delaunay::prelude::query::ConvexHull; #[cfg(feature = "diagnostics")] use delaunay::prelude::tds::Tds; -use delaunay::prelude::tds::TdsMutationError; use delaunay::prelude::triangulation::delaunayize::{ DelaunayizeConfig, DelaunayizeError, DelaunayizeOutcome, delaunayize_by_flips, }; use delaunay::prelude::triangulation::flips::{BistellarFlips, TopologyGuarantee}; +use delaunay::prelude::triangulation::insertion::{ + InsertionError, NeighborRebuildError, Tds as InsertionTds, TdsMutationError, + repair_neighbor_pointers_local, +}; use delaunay::prelude::triangulation::repair::{ DelaunayCheckPolicy, DelaunayRepairDiagnostics, DelaunayRepairError, DelaunayRepairOutcome, DelaunayRepairPolicy, DelaunayRepairStats, FlipEdgeAdjacencyError, FlipError, @@ -60,6 +63,8 @@ enum PreludeExportTestError { DelaunayRepair(#[from] DelaunayRepairError), #[error(transparent)] Delaunayize(#[from] DelaunayizeError), + #[error(transparent)] + Insertion(#[from] InsertionError), } /// Proves the focused flips prelude exports the trait bound expected by benchmarks. @@ -87,6 +92,11 @@ fn preludes_cover_bench_apis() -> Result<(), PreludeExportTestError> { assert!(dt.validate().is_ok()); assert_bistellar_flips(&dt); + let mut empty_tds: InsertionTds = InsertionTds::empty(); + assert_eq!( + repair_neighbor_pointers_local(&mut empty_tds, &[], None)?, + 0 + ); assert!(matches!( DelaunayConstructionFailure::GeometricDegeneracy { message: "synthetic".to_string(), @@ -95,6 +105,7 @@ fn preludes_cover_bench_apis() -> Result<(), PreludeExportTestError> { )); assert!(matches!(LocateResult::Outside, LocateResult::Outside)); assert_send_sync_unpin::(); + assert_send_sync_unpin::(); Ok(()) }