From 7682fad9f5976485e35f80a0ae4863a35e315d42 Mon Sep 17 00:00:00 2001 From: Adam Getchell Date: Sun, 21 Jun 2026 02:50:58 -0700 Subject: [PATCH] refactor(api)!: borrow topology views from canonical storage (#472) - Return validated borrowed simplex vertex slices instead of owned or optional detached snapshots. - Split convex hull facet access into detached `facet_handles()` and borrowed `facets(triangulation)` views with freshness checks. - Make vertex and simplex payload setters checked mutations that report typed stale-key errors. - Preserve fallback rebuild payload restoration through typed simplex-data restore errors. BREAKING CHANGE: `Tds::simplex_vertices`, `Triangulation::simplex_vertices`, and `DelaunayTriangulation::simplex_vertices` now return `Result<&[VertexKey], TdsError>`-style borrowed views instead of the previous owned/optional forms. `ConvexHull::facets()` has been split into `facet_handles()` for detached handles and `facets(triangulation)` for borrowed `FacetView` access. `set_vertex_data` and `set_simplex_data` now return checked `Result, TdsMutationError>` values instead of conflating missing keys with empty payloads. --- benches/allocation_hot_paths.rs | 2 +- docs/api_design.md | 51 ++++- docs/dev/rust.md | 24 +++ docs/workflows.md | 10 +- src/core/algorithms/pl_manifold_repair.rs | 8 +- src/core/query.rs | 24 ++- src/core/simplex.rs | 2 +- src/core/tds/mutation.rs | 236 ++++++++++++++-------- src/core/tds/storage.rs | 29 ++- src/core/tds/validation.rs | 4 +- src/core/triangulation.rs | 62 ++++-- src/core/util/jaccard.rs | 29 +-- src/delaunay/construction.rs | 20 +- src/delaunay/delaunayize.rs | 134 +++++++++--- src/delaunay/query.rs | 76 ++++--- src/geometry/algorithms/convex_hull.rs | 227 ++++++++++++++------- src/geometry/quality.rs | 2 +- src/lib.rs | 3 +- src/topology/manifold.rs | 8 +- tests/prelude_exports.rs | 16 +- tests/public_topology_api.rs | 12 +- tests/trait_bound_ergonomics.rs | 7 +- 22 files changed, 686 insertions(+), 300 deletions(-) diff --git a/benches/allocation_hot_paths.rs b/benches/allocation_hot_paths.rs index 05bed0bd..edc5236c 100644 --- a/benches/allocation_hot_paths.rs +++ b/benches/allocation_hot_paths.rs @@ -350,7 +350,7 @@ mod allocation_contracts { |b| { b.iter(|| { let (vertex_count, info) = measure_with_result(|| { - tds.simplex_vertices(simplex_key).map(|keys| keys.len()) + tds.simplex_vertices(simplex_key).map(<[VertexKey]>::len) }); assert_eq!(vertex_count.or_abort(), D + 1); assert_zero_allocations(&info, "Tds::simplex_vertices"); diff --git a/docs/api_design.md b/docs/api_design.md index 1e93d739..1d21cb15 100644 --- a/docs/api_design.md +++ b/docs/api_design.md @@ -152,8 +152,9 @@ for topology guarantee and validation policy details. Delaunay repair or orientation canonicalization fails, the triangulation and internal caches are restored to their pre-removal state. - **Auxiliary data**: Vertices and simplices carry optional user data (`U` / `V`). Read via `vertex.data()` / - `simplex.data()`, write via `dt.set_vertex_data(key, data)` / `dt.set_simplex_data(key, data)` (O(1), - invariant-preserving). See [`workflows.md`](workflows.md) for examples. + `simplex.data()`, write via checked `dt.set_vertex_data(key, data)?` / + `dt.set_simplex_data(key, data)?` calls (O(1), invariant-preserving, typed failure for stale keys). + See [`workflows.md`](workflows.md) for examples. - **Error handling**: Operations fail gracefully if they would violate invariants (see [`invariants.md`](invariants.md)). Mutating operations that invoke repair use typed repair diagnostics where available, for example @@ -383,6 +384,52 @@ let report = dt.validation_report(); - **Edit API**: Implemented in `delaunay::flips` (public trait) and `core::algorithms::flips` (internal implementation) - **Low-level primitives**: Context builders and flip application functions are `pub(crate)` in `core::algorithms::flips` +### Borrowed Views, Handles, Snapshots, And Rollback State + +Topology APIs use names to make ownership visible: + +- `*View` values borrow the canonical owner or are lifetime-bound to it, so they + cannot outlive the storage they observe. Examples include `FacetView<'tds>`, + `IncidenceView<'tds>`, `EdgeIndex<'tds>`, `SimplexNeighborIndex<'tds>`, and + `TriangulationAdjacency<'tds>`. +- Borrowed slices over canonical storage follow the same rule. For example, + `Tds::simplex_vertices(simplex_key)` validates the key relation, then returns + the simplex's stored `&[VertexKey]` instead of copying detached keys into a + buffer. +- `*Handle` and `*Key` values are detached, copyable runtime references. They + may be queued, stored, or returned from snapshots, but callers must validate + them against a live owner before reading through them. Examples include + `VertexKey`, `SimplexKey`, `FacetHandle`, `RidgeHandle`, `EdgeKey`, and + `TriangleHandle`. +- Owned snapshots are allowed only when the data must cross a persistence, + detached-analysis, or cache boundary. `TdsSnapshot`/`RawTdsSnapshot` are the + durable UUID persistence boundary. `ConvexHull` is a logically immutable hull + snapshot that stores `FacetHandle`s, while `ConvexHull::facets(triangulation)` + returns borrowed `FacetView` values and `ConvexHull::facet_handles()` exposes + the detached handles explicitly. +- Transactional rollback state may own cloned topology or exact mutation + records while an operation is in flight. `Tds::clone_for_rollback`, + `Tds::clone_from_for_rollback`, `SimplexIncidenceRemoval`, and flip trial + workspaces are rollback state, not long-lived public views. Replacing + full-TDS clone rollback with a journaled or localized design remains tracked + by #364. + +Runtime generation or identity checks remain appropriate for detached handles, +owned snapshots, serialization boundaries, persistent performance caches, and +tests that intentionally construct inconsistent topology. They should not be +used as a substitute for lifetimes when a value is truly a view over live +canonical storage. + +Algorithms follow the same phase split. Read-only traversal, classification, +and validation should work through borrowed views or lifetime-bound indexes +where practical. Mutating topology APIs should take `&mut Tds`/`&mut +Triangulation` directly, or execute behind a transaction guard that holds that +mutable borrow for the mutation or rollback window. Handles and keys may appear +inside that guard as short-lived, validated commit identifiers; they are not +proof that topology still exists by themselves. Keep views in lexical scopes +that end before the mutation so Rust enforces both existence and mutable versus +immutable access. + ### Design Rationale The separation serves several purposes: diff --git a/docs/dev/rust.md b/docs/dev/rust.md index b603c568..f4aa660b 100644 --- a/docs/dev/rust.md +++ b/docs/dev/rust.md @@ -212,6 +212,30 @@ and either borrow canonical relations for `'tds` or carry a lifetime tie to the source snapshot for derived maps, so mutation through the same owner is impossible while the view is alive. +Names should match ownership. A `*View` type or a method described as returning +views must borrow the canonical owner, or return values lifetime-bound to that +owner, so the view cannot outlive the data it observes. Detached, copyable +runtime references should be named `*Handle` or `*Key` instead, and APIs that +turn handles back into views must revalidate the handle against a live owner at +the conversion boundary. For example, `ConvexHull::facets(triangulation)` +returns borrowed `FacetView<'_>` values, while `ConvexHull::facet_handles()` +exposes the stored `FacetHandle`s explicitly. + +Borrowed slices over canonical topology storage follow the same convention: +return `&[Key]` when the slice lives in the owner and the caller should not keep +it across mutation. For example, `Tds::simplex_vertices(simplex_key)` validates +the relation and lends the simplex's stored `&[VertexKey]`. + +Algorithm implementations should use borrowed views for read-only observation, +classification, and validation phases. Mutation APIs that change canonical +topology should take `&mut Tds`/`&mut Triangulation` directly, or expose a guard +that holds that mutable borrow for the whole mutation or rollback window. This +ties existence and aliasing to the real owner: missing topology fails at view or +guard construction, and Rust prevents mutation while immutable views remain +live. Inside the mutable scope, collapse short-lived views into validated +`*Handle`/`*Key` commit identifiers before mutating; a live view must not span a +topology mutation. + Keep runtime identity or generation checks for detached handles, separately supplied indexes, serialization boundaries, and tests that intentionally corrupt metadata. Those checks complement lifetimes at API boundaries where Rust cannot diff --git a/docs/workflows.md b/docs/workflows.md index af9fb940..65dc558d 100644 --- a/docs/workflows.md +++ b/docs/workflows.md @@ -327,21 +327,23 @@ fn main() -> DelaunayResult<()> { let Some((key, _)) = dt.vertices().next() else { return Ok(()); }; - let prev = dt.set_vertex_data(key, Some(99)); + let prev = dt.set_vertex_data(key, Some(99))?; assert!(prev.is_some()); // returns the old Option // Simplex data works the same way let Some((simplex_key, _)) = dt.simplices().next() else { return Ok(()); }; - dt.set_simplex_data(simplex_key, Some(42)); + dt.set_simplex_data(simplex_key, Some(42))?; assert_eq!(dt.tds().simplex(simplex_key).map(|s| s.data()), Some(Some(&42))); Ok(()) } ``` -`set_vertex_data` and `set_simplex_data` are safe O(1) operations — they modify only the -user-data field and do not invalidate geometry, topology, or Delaunay invariants. +`set_vertex_data` and `set_simplex_data` are checked O(1) operations — they modify only the +user-data field, return the previous payload on success, and fail with a typed mutation error +if the supplied key no longer exists. Successful calls do not invalidate geometry, topology, or +Delaunay invariants. For algorithm-local state keyed by existing vertices or simplices, prefer the caller-owned secondary-map aliases instead of mutating stored user data: diff --git a/src/core/algorithms/pl_manifold_repair.rs b/src/core/algorithms/pl_manifold_repair.rs index daf5be7f..722be976 100644 --- a/src/core/algorithms/pl_manifold_repair.rs +++ b/src/core/algorithms/pl_manifold_repair.rs @@ -676,8 +676,8 @@ mod tests { // Duplicate the first simplex → its facets go from degree 2 to degree 3. let simplex_key = tds.simplex_keys().next().unwrap(); - let vkeys = tds.simplex_vertices(simplex_key).unwrap(); - let dup_simplex = Simplex::try_new_with_data(vkeys.to_vec(), None).unwrap(); + let vkeys = tds.simplex_vertices(simplex_key).unwrap().to_vec(); + let dup_simplex = Simplex::try_new_with_data(vkeys, None).unwrap(); tds.insert_simplex_bypassing_topology_checks_for_test(dup_simplex) .unwrap(); @@ -695,10 +695,10 @@ mod tests { fn make_multi_duplicate_overshared_tds() -> Tds<(), (), 3> { let mut tds = make_overshared_tds(); let simplex_key = tds.simplex_keys().next().unwrap(); - let vkeys = tds.simplex_vertices(simplex_key).unwrap(); + let vkeys = tds.simplex_vertices(simplex_key).unwrap().to_vec(); for _ in 0..5 { - let dup_simplex = Simplex::try_new_with_data(vkeys.to_vec(), None).unwrap(); + let dup_simplex = Simplex::try_new_with_data(vkeys.clone(), None).unwrap(); tds.insert_simplex_bypassing_topology_checks_for_test(dup_simplex) .unwrap(); } diff --git a/src/core/query.rs b/src/core/query.rs index 2058634e..dbd5f5ad 100644 --- a/src/core/query.rs +++ b/src/core/query.rs @@ -498,10 +498,15 @@ impl Triangulation { /// Returns a slice view of a simplex's vertex keys. /// - /// This is a zero-allocation accessor. If `c` is not present, returns `None`. - #[must_use] - pub fn simplex_vertices(&self, c: SimplexKey) -> Option<&[VertexKey]> { - self.tds.simplex(c).map(Simplex::vertices) + /// This is a zero-allocation accessor that validates the simplex key and + /// referenced vertex keys before lending the canonical slice. + /// + /// # Errors + /// + /// Returns [`TdsError`] if `c` does not identify a simplex in this + /// triangulation, or if the simplex references a missing vertex key. + pub fn simplex_vertices(&self, c: SimplexKey) -> Result<&[VertexKey], TdsError> { + self.tds.simplex_vertices(c) } /// Returns a slice view of a vertex's coordinates. @@ -1082,7 +1087,10 @@ mod tests { neighbor_index.number_of_simplex_neighbors(missing_simplex_key), 0 ); - assert!(tri.simplex_vertices(missing_simplex_key).is_none()); + assert_matches!( + tri.simplex_vertices(missing_simplex_key), + Err(TdsError::SimplexNotFound { .. }) + ); } #[test] @@ -1464,9 +1472,9 @@ mod tests { assert_eq!(coords.len(), 3); } - assert!( - tri.simplex_vertices(SimplexKey::from(KeyData::from_ffi(0xDEAD))) - .is_none() + assert_matches!( + tri.simplex_vertices(SimplexKey::from(KeyData::from_ffi(0xDEAD))), + Err(TdsError::SimplexNotFound { .. }) ); assert!( tri.vertex_coords(VertexKey::from(KeyData::from_ffi(0xBEEF))) diff --git a/src/core/simplex.rs b/src/core/simplex.rs index d9471a43..dd9105de 100644 --- a/src/core/simplex.rs +++ b/src/core/simplex.rs @@ -4398,7 +4398,7 @@ mod tests { assert_eq!(dt.tds().simplex(key).unwrap().data(), None); // Set data and verify via accessor - dt.set_simplex_data(key, Some(99)); + dt.set_simplex_data(key, Some(99)).unwrap(); assert_eq!(dt.tds().simplex(key).unwrap().data(), Some(&99)); } } diff --git a/src/core/tds/mutation.rs b/src/core/tds/mutation.rs index d76c0785..d55c7002 100644 --- a/src/core/tds/mutation.rs +++ b/src/core/tds/mutation.rs @@ -224,7 +224,7 @@ impl Tds { for i in 0..vertices.len() { let facet_key = - Self::periodic_facet_key_from_simplex_vertices(simplex, &vertices, i)?; + Self::periodic_facet_key_from_simplex_vertices(simplex, vertices, i)?; let facet_entry = facet_map.entry(facet_key).or_default(); // Detect degenerate case early: more than 2 simplices sharing a facet // Note: Check happens before push, so len() reflects current sharing count @@ -542,7 +542,7 @@ impl Tds { for (existing_simplex_key, _existing_simplex) in &self.simplices { let vertices = self.simplex_vertices(existing_simplex_key)?; let existing_identity = - self.build_periodic_vertex_uuid_offsets(existing_simplex_key, &vertices)?; + self.build_periodic_vertex_uuid_offsets(existing_simplex_key, vertices)?; if existing_identity == candidate_identity { return Err(TdsError::DuplicateSimplices { @@ -573,7 +573,7 @@ impl Tds { for existing_facet_idx in 0..existing_vertices.len() { let existing_facet_key = Self::periodic_facet_key_from_simplex_vertices( existing_simplex, - &existing_vertices, + existing_vertices, existing_facet_idx, )?; if existing_facet_key == candidate_facet_key { @@ -623,7 +623,7 @@ impl Tds { self.insert_simplex_with_mapping_impl(simplex, SimplexInsertionTopologyCheck::Prechecked) } - /// Sets the auxiliary data on a returning the previous value. + /// Sets the auxiliary data on a vertex, returning the previous value. /// /// This is a safe O(1) operation that modifies only the user-data field. /// It does not affect geometry, topology, or Delaunay invariants. @@ -635,8 +635,13 @@ impl Tds { /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a vertex in this + /// TDS. Detached keys are validated at the mutation boundary so stale + /// handles cannot be mistaken for an existing vertex with no payload. /// /// # Examples /// @@ -669,8 +674,8 @@ impl Tds { /// }; /// /// // Replace existing data - /// let prev = tds.set_vertex_data(key, Some(99)); - /// assert!(prev.is_some()); // key was found + /// let prev = tds.set_vertex_data(key, Some(99))?; + /// assert!(prev.is_some()); /// /// // Verify new value /// let Some(vertex) = tds.vertex(key) else { @@ -679,18 +684,27 @@ impl Tds { /// assert_eq!(vertex.data(), Some(&99)); /// /// // Clear data - /// let prev = tds.set_vertex_data(key, None); - /// assert_eq!(prev, Some(Some(99))); + /// let prev = tds.set_vertex_data(key, None)?; + /// assert_eq!(prev, Some(99)); /// assert_eq!(tds.vertex(key).and_then(|vertex| vertex.data()), None); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_vertex_data(&mut self, key: VertexKey, data: Option) -> Option> { - let vertex = self.vertices.get_mut(key)?; + pub fn set_vertex_data( + &mut self, + key: VertexKey, + data: Option, + ) -> Result, TdsMutationError> { + let vertex = self.vertices.get_mut(key).ok_or_else(|| { + TdsMutationError::from(TdsError::VertexNotFound { + vertex_key: key, + context: "set_vertex_data".to_string(), + }) + })?; let previous = vertex.data.take(); vertex.data = data; - Some(previous) + Ok(previous) } /// Sets the auxiliary data on a simplex, returning the previous value. @@ -705,8 +719,13 @@ impl Tds { /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a simplex in + /// this TDS. Detached keys are validated at the mutation boundary so stale + /// handles cannot be mistaken for an existing simplex with no payload. /// /// # Examples /// @@ -739,8 +758,8 @@ impl Tds { /// }; /// /// // Set data on a simplex that had no data - /// let prev = tds.set_simplex_data(key, Some(42)); - /// assert_eq!(prev, Some(None)); // key found, previous was None + /// let prev = tds.set_simplex_data(key, Some(42))?; + /// assert_eq!(prev, None); /// /// // Verify new value /// let Some(simplex) = tds.simplex(key) else { @@ -749,18 +768,27 @@ impl Tds { /// assert_eq!(simplex.data(), Some(&42)); /// /// // Clear data - /// let prev = tds.set_simplex_data(key, None); - /// assert_eq!(prev, Some(Some(42))); + /// let prev = tds.set_simplex_data(key, None)?; + /// assert_eq!(prev, Some(42)); /// assert_eq!(tds.simplex(key).and_then(|simplex| simplex.data()), None); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_simplex_data(&mut self, key: SimplexKey, data: Option) -> Option> { - let simplex = self.simplices.get_mut(key)?; + pub fn set_simplex_data( + &mut self, + key: SimplexKey, + data: Option, + ) -> Result, TdsMutationError> { + let simplex = self.simplices.get_mut(key).ok_or_else(|| { + TdsMutationError::from(TdsError::SimplexNotFound { + simplex_key: key, + context: "set_simplex_data".to_string(), + }) + })?; let previous = simplex.data.take(); simplex.data = data; - Some(previous) + Ok(previous) } /// Removes multiple simplices by their keys in a batch operation. @@ -2109,7 +2137,7 @@ impl Tds { for simplex_key in self.simplices.keys() { let vertices = self.simplex_vertices(simplex_key)?; let vertex_uuid_offsets = - self.build_periodic_vertex_uuid_offsets(simplex_key, &vertices)?; + self.build_periodic_vertex_uuid_offsets(simplex_key, vertices)?; match unique_simplices.entry(vertex_uuid_offsets) { Entry::Occupied(_) => { @@ -4157,9 +4185,9 @@ mod tests { #[test] fn test_set_vertex_data_replaces_existing() { let vertices: [Vertex; 3] = [ - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 0.0], 10i32).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([1.0, 0.0], 20).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 1.0], 30).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 0.0], 10i32).unwrap(), + Vertex::<_, _>::try_new_with_data([1.0, 0.0], 20).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 1.0], 30).unwrap(), ]; let dt = DelaunayTriangulationBuilder::new(&vertices) .build::<()>() @@ -4167,8 +4195,8 @@ mod tests { let mut tds = dt.tds().clone(); let key = tds.vertex_keys().next().unwrap(); - let prev = tds.set_vertex_data(key, Some(99)); - assert!(prev.unwrap().is_some()); // had data before + let prev = tds.set_vertex_data(key, Some(99)).unwrap(); + assert!(prev.is_some()); // had data before assert_eq!(tds.vertex(key).unwrap().data, Some(99)); } @@ -4176,33 +4204,34 @@ mod tests { fn test_set_vertex_data_on_no_data_vertex() { // Vertices without data have U = (), so set_vertex_data sets (). let vertices = [ - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), ]; let dt = DelaunayTriangulation::try_new(&vertices).unwrap(); let mut tds = dt.tds().clone(); let key = tds.vertex_keys().next().unwrap(); - let prev = tds.set_vertex_data(key, Some(())); + let prev = tds.set_vertex_data(key, Some(())).unwrap(); // Vertices constructed without explicit data have data = None - assert_eq!(prev, Some(None)); + assert_eq!(prev, None); assert_eq!(tds.vertex(key).unwrap().data, Some(())); } #[test] - fn test_set_vertex_data_invalid_key_returns_none() { + fn test_set_vertex_data_invalid_key_returns_error() { let mut tds: Tds = Tds::empty(); let stale = VertexKey::from(KeyData::from_ffi(0xDEAD)); - assert!(tds.set_vertex_data(stale, Some(1)).is_none()); + let err = tds.set_vertex_data(stale, Some(1)).unwrap_err(); + assert_matches!(err.as_tds_error(), TdsError::VertexNotFound { .. }); } #[test] fn test_set_simplex_data_on_empty_simplex() { let vertices = [ - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), ]; let dt = DelaunayTriangulationBuilder::new(&vertices) .build::() @@ -4210,17 +4239,17 @@ mod tests { let mut tds = dt.tds().clone(); let key = tds.simplex_keys().next().unwrap(); - let prev = tds.set_simplex_data(key, Some(42)); - assert_eq!(prev, Some(None)); // key found, no previous data + let prev = tds.set_simplex_data(key, Some(42)).unwrap(); + assert_eq!(prev, None); // key found, no previous data assert_eq!(tds.simplex(key).unwrap().data, Some(42)); } #[test] fn test_set_simplex_data_replaces_existing() { let vertices = [ - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), ]; let dt = DelaunayTriangulationBuilder::new(&vertices) .build::() @@ -4228,25 +4257,26 @@ mod tests { let mut tds = dt.tds().clone(); let key = tds.simplex_keys().next().unwrap(); - tds.set_simplex_data(key, Some(1)); - let prev = tds.set_simplex_data(key, Some(2)); - assert_eq!(prev, Some(Some(1))); + tds.set_simplex_data(key, Some(1)).unwrap(); + let prev = tds.set_simplex_data(key, Some(2)).unwrap(); + assert_eq!(prev, Some(1)); assert_eq!(tds.simplex(key).unwrap().data, Some(2)); } #[test] - fn test_set_simplex_data_invalid_key_returns_none() { + fn test_set_simplex_data_invalid_key_returns_error() { let mut tds: Tds<(), i32, 2> = Tds::empty(); let stale = SimplexKey::from(KeyData::from_ffi(0xDEAD)); - assert!(tds.set_simplex_data(stale, Some(1)).is_none()); + let err = tds.set_simplex_data(stale, Some(1)).unwrap_err(); + assert_matches!(err.as_tds_error(), TdsError::SimplexNotFound { .. }); } #[test] fn test_set_vertex_data_preserves_triangulation_validity() { let vertices: [Vertex; 3] = [ - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 0.0], 1i32).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([1.0, 0.0], 2).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 1.0], 3).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 0.0], 1i32).unwrap(), + Vertex::<_, _>::try_new_with_data([1.0, 0.0], 2).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 1.0], 3).unwrap(), ]; let mut dt = DelaunayTriangulationBuilder::new(&vertices) .build::<()>() @@ -4255,7 +4285,7 @@ mod tests { // Mutate every vertex's data through the DT wrapper. let keys: Vec<_> = dt.vertices().map(|(k, _)| k).collect(); for (key, i) in keys.iter().zip(0i32..) { - dt.set_vertex_data(*key, Some(i * 100)); + dt.set_vertex_data(*key, Some(i * 100)).unwrap(); } // Triangulation must remain fully valid. @@ -4271,10 +4301,10 @@ mod tests { #[test] fn test_set_simplex_data_preserves_triangulation_validity() { let vertices = [ - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([0.5, 1.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.5, 0.5]).unwrap(), + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.5, 1.0]).unwrap(), + Vertex::<(), _>::try_new([1.5, 0.5]).unwrap(), ]; let mut dt = DelaunayTriangulationBuilder::new(&vertices) .build::() @@ -4284,7 +4314,7 @@ mod tests { // Mutate every simplex's data through the DT wrapper. let keys: Vec<_> = dt.simplices().map(|(k, _)| k).collect(); for (key, i) in keys.iter().zip(0i32..) { - dt.set_simplex_data(*key, Some(i)); + dt.set_simplex_data(*key, Some(i)).unwrap(); } // Triangulation must remain fully valid. @@ -4300,9 +4330,9 @@ mod tests { #[test] fn test_set_vertex_data_via_delaunay_wrapper() { let vertices: [Vertex; 3] = [ - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 0.0], 10i32).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([1.0, 0.0], 20).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 1.0], 30).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 0.0], 10i32).unwrap(), + Vertex::<_, _>::try_new_with_data([1.0, 0.0], 20).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 1.0], 30).unwrap(), ]; let mut dt = DelaunayTriangulationBuilder::new(&vertices) .build::<()>() @@ -4310,22 +4340,47 @@ mod tests { let key = dt.vertices().next().unwrap().0; // Set via Delaunay wrapper - let prev = dt.set_vertex_data(key, Some(99)); - assert!(prev.unwrap().is_some()); + let prev = dt.set_vertex_data(key, Some(99)).unwrap(); + assert!(prev.is_some()); assert_eq!(dt.tds().vertex(key).unwrap().data, Some(99)); // Clear via Delaunay wrapper - let prev = dt.set_vertex_data(key, None); - assert_eq!(prev, Some(Some(99))); + let prev = dt.set_vertex_data(key, None).unwrap(); + assert_eq!(prev, Some(99)); assert_eq!(dt.tds().vertex(key).unwrap().data, None); } + #[test] + fn test_set_vertex_data_via_delaunay_wrapper_invalid_key_returns_error() { + let vertices: [Vertex; 3] = [ + Vertex::<_, _>::try_new_with_data([0.0, 0.0], 10i32).unwrap(), + Vertex::<_, _>::try_new_with_data([1.0, 0.0], 20).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 1.0], 30).unwrap(), + ]; + let mut dt = DelaunayTriangulationBuilder::new(&vertices) + .build::<()>() + .unwrap(); + let live_key = dt.vertices().next().unwrap().0; + let stale = VertexKey::from(KeyData::from_ffi(0xFEED)); + + let err = dt.set_vertex_data(stale, Some(99)).unwrap_err(); + + assert_matches!( + err.as_tds_error(), + TdsError::VertexNotFound { + vertex_key, + .. + } if *vertex_key == stale + ); + assert_eq!(dt.tds().vertex(live_key).unwrap().data, Some(10)); + } + #[test] fn test_set_simplex_data_via_delaunay_wrapper() { let vertices = [ - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), - crate::core::vertex::Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), ]; let mut dt = DelaunayTriangulationBuilder::new(&vertices) .build::() @@ -4333,35 +4388,59 @@ mod tests { let key = dt.simplices().next().unwrap().0; // Set via Delaunay wrapper - let prev = dt.set_simplex_data(key, Some(42)); - assert_eq!(prev, Some(None)); + let prev = dt.set_simplex_data(key, Some(42)).unwrap(); + assert_eq!(prev, None); assert_eq!(dt.tds().simplex(key).unwrap().data, Some(42)); // Clear via Delaunay wrapper - let prev = dt.set_simplex_data(key, None); - assert_eq!(prev, Some(Some(42))); + let prev = dt.set_simplex_data(key, None).unwrap(); + assert_eq!(prev, Some(42)); assert_eq!(dt.tds().simplex(key).unwrap().data, None); } + #[test] + fn test_set_simplex_data_via_delaunay_wrapper_invalid_key_returns_error() { + let vertices = [ + Vertex::<(), _>::try_new([0.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([1.0, 0.0]).unwrap(), + Vertex::<(), _>::try_new([0.0, 1.0]).unwrap(), + ]; + let mut dt = DelaunayTriangulationBuilder::new(&vertices) + .build::() + .unwrap(); + let live_key = dt.simplices().next().unwrap().0; + let stale = SimplexKey::from(KeyData::from_ffi(0xFEED)); + + let err = dt.set_simplex_data(stale, Some(42)).unwrap_err(); + + assert_matches!( + err.as_tds_error(), + TdsError::SimplexNotFound { + simplex_key, + .. + } if *simplex_key == stale + ); + assert_eq!(dt.tds().simplex(live_key).unwrap().data, None); + } + #[test] fn test_set_data_via_dt_does_not_invalidate_locate_hint() { let vertices: [Vertex; 3] = [ - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 0.0], 0i32).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([1.0, 0.0], 0).unwrap(), - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.0, 1.0], 0).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 0.0], 0i32).unwrap(), + Vertex::<_, _>::try_new_with_data([1.0, 0.0], 0).unwrap(), + Vertex::<_, _>::try_new_with_data([0.0, 1.0], 0).unwrap(), ]; let mut dt = DelaunayTriangulationBuilder::new(&vertices) .build::<()>() .unwrap(); // Insert a new vertex so the locate hint is populated. - let extra = - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.25, 0.25], 0i32).unwrap(); + let extra = Vertex::<_, _>::try_new_with_data([0.25, 0.25], 0i32).unwrap(); dt.insert(extra).unwrap(); // Data mutation should NOT clear the insertion hint. let key = dt.vertices().next().unwrap().0; - let prev = dt.set_vertex_data(key, Some(999)); + let prev = dt.set_vertex_data(key, Some(999)).unwrap(); assert!(prev.is_some(), "set_vertex_data should find the key"); assert_eq!( dt.tds().vertex(key).unwrap().data, @@ -4370,8 +4449,7 @@ mod tests { ); // A subsequent insert should still succeed (hint not invalidated). - let another = - crate::core::vertex::Vertex::<_, _>::try_new_with_data([0.75, 0.1], 0i32).unwrap(); + let another = Vertex::<_, _>::try_new_with_data([0.75, 0.1], 0i32).unwrap(); assert!(dt.insert(another).is_ok()); assert!(dt.validate().is_ok()); } diff --git a/src/core/tds/storage.rs b/src/core/tds/storage.rs index 5778549f..9beb5d73 100644 --- a/src/core/tds/storage.rs +++ b/src/core/tds/storage.rs @@ -327,7 +327,7 @@ use crate::core::collections::{ MAX_PRACTICAL_DIMENSION_SIZE, SimplexKeySet, SmallBuffer, StorageMap, UuidToSimplexKeyMap, - UuidToVertexKeyMap, VertexKeyBuffer, + UuidToVertexKeyMap, }; use crate::core::tds::errors::{NeighborValidationError, TdsError, TriangulationConstructionState}; use crate::core::tds::incidence::VertexIncidenceIndex; @@ -771,7 +771,7 @@ impl Tds { simplex_key, context: format!("deriving facet key for index {facet_index}"), })?; - Self::periodic_facet_key_from_simplex_vertices(simplex, &vertices, facet_index) + Self::periodic_facet_key_from_simplex_vertices(simplex, vertices, facet_index) } /// Returns an iterator over all simplices in the triangulation. @@ -1566,16 +1566,17 @@ impl Tds { impl Tds {} impl Tds { - /// Gets validated vertex keys for a simplex. + /// Returns a validated borrowed view of a simplex's vertex keys. /// - /// This performs O(D) validation and copying of the requested simplex's - /// vertex keys into a stack-friendly buffer. + /// This performs O(D) validation of the requested simplex's vertex keys and + /// returns the canonical slice stored by the simplex. The returned view is + /// borrowed from this TDS, so it cannot outlive the storage it observes. /// /// This method provides: /// - O(1) simplex lookup via storage map key /// - O(D) validation that all vertex keys exist in the triangulation /// - Direct key access without UUID→key lookups - /// - Stack-allocated buffer for D ≤ 7 to avoid heap allocation + /// - Zero allocation on success /// /// # Arguments /// @@ -1583,19 +1584,19 @@ impl Tds { /// /// # Returns /// - /// A `Result` containing a `VertexKeyBuffer` if the simplex exists and all vertices are valid, - /// or a `TdsError` if the simplex doesn't exist or vertices are missing. + /// A borrowed [`VertexKey`] slice if the [`SimplexKey`] exists and all + /// referenced vertices are valid. /// /// # Errors /// - /// Returns a `TdsError` if: + /// Returns [`TdsError`] if: /// - The simplex with the given key doesn't exist /// - A vertex key from the simplex doesn't exist in the vertex storage (TDS corruption) /// /// # Performance /// /// This uses direct storage map access with O(1) key lookup for the simplex and O(D) - /// validation for vertex keys. Uses stack-allocated buffer for D ≤ 7 to avoid heap + /// validation for vertex keys. It returns the stored slice directly and performs no /// allocation in the hot path. /// /// # Examples @@ -1632,7 +1633,7 @@ impl Tds { /// # } /// ``` #[inline] - pub fn simplex_vertices(&self, simplex_key: SimplexKey) -> Result { + pub fn simplex_vertices(&self, simplex_key: SimplexKey) -> Result<&[VertexKey], TdsError> { let simplex = self .simplices .get(simplex_key) @@ -1641,9 +1642,8 @@ impl Tds { context: "simplex_vertices lookup".to_string(), })?; - // Validate and collect keys in one pass to avoid redundant iteration. + // Validate keys in one pass before lending the canonical slice. let simplex_vertices = simplex.vertices(); - let mut keys = VertexKeyBuffer::with_capacity(simplex_vertices.len()); for (idx, &vertex_key) in simplex_vertices.iter().enumerate() { if !self.vertices.contains_key(vertex_key) { return Err(TdsError::VertexNotFound { @@ -1654,9 +1654,8 @@ impl Tds { ), }); } - keys.push(vertex_key); } - Ok(keys) + Ok(simplex_vertices) } /// Helper function to get a simplex key from a simplex UUID using the optimized UUID→Key mapping. diff --git a/src/core/tds/validation.rs b/src/core/tds/validation.rs index 6fa4f7a6..c80f2593 100644 --- a/src/core/tds/validation.rs +++ b/src/core/tds/validation.rs @@ -91,7 +91,7 @@ impl Tds { for i in 0..vertices.len() { let facet_key = - Self::periodic_facet_key_from_simplex_vertices(simplex, &vertices, i)?; + Self::periodic_facet_key_from_simplex_vertices(simplex, vertices, i)?; let Ok(facet_index_u8) = usize_to_u8(i, vertices.len()) else { return Err(TdsError::IndexOutOfBounds { index: i, @@ -424,7 +424,7 @@ impl Tds { for (simplex_key, _simplex) in &self.simplices { let vertices = self.simplex_vertices(simplex_key)?; let vertex_uuid_offsets = - self.build_periodic_vertex_uuid_offsets(simplex_key, &vertices)?; + self.build_periodic_vertex_uuid_offsets(simplex_key, vertices)?; if let Some(existing_simplex_key) = unique_simplices.get(&vertex_uuid_offsets) { duplicates.push(( diff --git a/src/core/triangulation.rs b/src/core/triangulation.rs index 42b7fc1e..138a407b 100644 --- a/src/core/triangulation.rs +++ b/src/core/triangulation.rs @@ -12,7 +12,7 @@ #![forbid(unsafe_code)] -use crate::core::tds::{SimplexKey, Tds, VertexKey}; +use crate::core::tds::{SimplexKey, Tds, TdsMutationError, VertexKey}; use crate::core::validation::{TopologyGuarantee, ValidationPolicy}; use crate::geometry::kernel::Kernel; use crate::topology::traits::topological_space::GlobalTopology; @@ -94,15 +94,19 @@ where } } - /// Sets the auxiliary data on a returning the previous value. + /// Sets the auxiliary data on a vertex, returning the previous value. /// /// Delegates to [`Tds::set_vertex_data`]. This is a safe O(1) operation /// that does not affect geometry, topology, or Delaunay invariants. /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a vertex in the + /// underlying TDS. /// /// # Examples /// @@ -119,6 +123,8 @@ where /// # MissingVertex, /// # #[error(transparent)] /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), + /// # #[error(transparent)] + /// # TdsMutation(#[from] delaunay::prelude::tds::TdsMutationError), /// # } /// # fn main() -> Result<(), ExampleError> { /// let vertices: [Vertex; 3] = [ @@ -128,19 +134,23 @@ where /// ]; /// let mut dt = DelaunayTriangulationBuilder::new(&vertices).build::<()>()?; /// let key = dt.vertices().next().ok_or(ExampleError::MissingVertex)?.0; - /// let prev = dt.set_vertex_data(key, Some(99)); + /// let prev = dt.set_vertex_data(key, Some(99))?; /// assert!(prev.is_some()); /// /// // Clear data - /// let prev = dt.set_vertex_data(key, None); - /// assert_eq!(prev, Some(Some(99))); + /// let prev = dt.set_vertex_data(key, None)?; + /// assert_eq!(prev, Some(99)); /// let vertex = dt.tds().vertex(key).ok_or(ExampleError::MissingVertex)?; /// assert_eq!(vertex.data(), None); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_vertex_data(&mut self, key: VertexKey, data: Option) -> Option> { + pub fn set_vertex_data( + &mut self, + key: VertexKey, + data: Option, + ) -> Result, TdsMutationError> { self.tds.set_vertex_data(key, data) } @@ -151,8 +161,12 @@ where /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a simplex in + /// the underlying TDS. /// /// # Examples /// @@ -169,6 +183,8 @@ where /// # MissingSimplex, /// # #[error(transparent)] /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), + /// # #[error(transparent)] + /// # TdsMutation(#[from] delaunay::prelude::tds::TdsMutationError), /// # } /// # fn main() -> Result<(), ExampleError> { /// let vertices = [ @@ -178,19 +194,23 @@ where /// ]; /// let mut dt = DelaunayTriangulationBuilder::new(&vertices).build::()?; /// let key = dt.simplices().next().ok_or(ExampleError::MissingSimplex)?.0; - /// let prev = dt.set_simplex_data(key, Some(42)); - /// assert_eq!(prev, Some(None)); + /// let prev = dt.set_simplex_data(key, Some(42))?; + /// assert_eq!(prev, None); /// /// // Clear data - /// let prev = dt.set_simplex_data(key, None); - /// assert_eq!(prev, Some(Some(42))); + /// let prev = dt.set_simplex_data(key, None)?; + /// assert_eq!(prev, Some(42)); /// let simplex = dt.tds().simplex(key).ok_or(ExampleError::MissingSimplex)?; /// assert_eq!(simplex.data(), None); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_simplex_data(&mut self, key: SimplexKey, data: Option) -> Option> { + pub fn set_simplex_data( + &mut self, + key: SimplexKey, + data: Option, + ) -> Result, TdsMutationError> { self.tds.set_simplex_data(key, data) } } @@ -198,8 +218,10 @@ where #[cfg(test)] mod tests { use super::*; + use crate::core::tds::TdsError; use crate::geometry::kernel::FastKernel; use slotmap::KeyData; + use std::assert_matches; #[test] fn new_empty_sets_default_topology_and_validation_policy() { @@ -217,22 +239,24 @@ mod tests { } #[test] - fn set_vertex_data_returns_none_for_invalid_key() { + fn set_vertex_data_returns_error_for_invalid_key() { let mut tri: Triangulation, i32, (), 2> = Triangulation::new_empty(FastKernel::new()); let stale = VertexKey::from(KeyData::from_ffi(0xDEAD_BEEF)); - assert_eq!(tri.set_vertex_data(stale, Some(42)), None); + let err = tri.set_vertex_data(stale, Some(42)).unwrap_err(); + assert_matches!(err.as_tds_error(), TdsError::VertexNotFound { .. }); assert_eq!(tri.tds.number_of_vertices(), 0); } #[test] - fn set_simplex_data_returns_none_for_invalid_key() { + fn set_simplex_data_returns_error_for_invalid_key() { let mut tri: Triangulation, (), i32, 2> = Triangulation::new_empty(FastKernel::new()); let stale = SimplexKey::from(KeyData::from_ffi(0xDEAD_BEEF)); - assert_eq!(tri.set_simplex_data(stale, Some(42)), None); + let err = tri.set_simplex_data(stale, Some(42)).unwrap_err(); + assert_matches!(err.as_tds_error(), TdsError::SimplexNotFound { .. }); assert_eq!(tri.tds.number_of_simplices(), 0); } } diff --git a/src/core/util/jaccard.rs b/src/core/util/jaccard.rs index 9df800bc..448ee8a4 100644 --- a/src/core/util/jaccard.rs +++ b/src/core/util/jaccard.rs @@ -2,13 +2,12 @@ #![forbid(unsafe_code)] -use crate::core::facet::{FacetError, FacetView}; +use crate::core::facet::FacetError; use crate::core::tds::Tds; use crate::core::traits::boundary_analysis::BoundaryAnalysis; use crate::core::traits::data_type::DataType; use crate::core::triangulation::Triangulation; -use crate::geometry::algorithms::convex_hull::ConvexHull; -use crate::geometry::kernel::Kernel; +use crate::geometry::algorithms::convex_hull::{ConvexHull, ConvexHullConstructionError}; use crate::geometry::point::Point; use std::collections::HashSet; use std::fmt::Debug; @@ -416,11 +415,12 @@ where /// /// # Returns /// -/// A `Result` containing a `HashSet` of facet identifiers, or a `FacetError` +/// A `Result` containing a `HashSet` of facet identifiers. /// /// # Errors /// -/// Returns `FacetError` if facet views cannot be created or facet keys cannot be computed +/// Returns [`ConvexHullConstructionError`] if the hull is stale for `tri`, belongs to a +/// different triangulation identity, or if borrowed facet views cannot be created or keyed. /// /// # Examples /// @@ -458,21 +458,14 @@ where pub fn extract_hull_facet_set( hull: &ConvexHull, tri: &Triangulation, -) -> Result, FacetError> -where - K: Kernel, - U: DataType, - V: DataType, -{ - let tds = &tri.tds; +) -> Result, ConvexHullConstructionError> { let mut facet_ids = HashSet::new(); - for facet_handle in hull.facets() { - // Create FacetView using simplex_key() and facet_index() methods from FacetHandle - let facet_view = - FacetView::try_new(tds, facet_handle.simplex_key(), facet_handle.facet_index())?; - // Use the existing FacetView::key() method - let facet_id = facet_view.key()?; + for facet_view in hull.facets(tri)? { + let facet_id = facet_view + .map_err(|source| ConvexHullConstructionError::FacetDataAccessFailed { source })? + .key() + .map_err(|source| ConvexHullConstructionError::FacetDataAccessFailed { source })?; facet_ids.insert(facet_id); } diff --git a/src/delaunay/construction.rs b/src/delaunay/construction.rs index fba95f16..9f6fbda6 100644 --- a/src/delaunay/construction.rs +++ b/src/delaunay/construction.rs @@ -69,7 +69,7 @@ use crate::core::operations::{ InsertionTelemetryMode, RepairDecision, TopologicalOperation, }; use crate::core::simplex::SimplexValidationError; -use crate::core::tds::{InvariantError, SimplexKey}; +use crate::core::tds::{InvariantError, SimplexKey, TdsMutationError}; use crate::core::tds::{TdsConstructionError, TdsError, TriangulationConstructionState}; use crate::core::traits::data_type::DataType; use crate::core::triangulation::Triangulation; @@ -206,8 +206,9 @@ pub(crate) mod test_hooks { /// This convenience error covers the fallible path most examples use: /// converting caller coordinates into vertices, constructing a /// [`DelaunayTriangulation`], editing it through -/// the Delaunay insertion API, and validating its Delaunay invariants. More -/// specialized workflows such as convex hull extraction, bistellar flips, +/// the Delaunay insertion API, updating auxiliary vertex/simplex data through +/// checked keys, and validating its Delaunay invariants. More specialized +/// workflows such as convex hull extraction, bistellar flips, /// repair, and delaunayize continue to expose their narrower error types /// directly. /// @@ -215,7 +216,7 @@ pub(crate) mod test_hooks { /// /// Use [`DelaunayResult`] for examples, binaries, and quick workflows whose /// fallible operations stay inside coordinate conversion, construction, -/// insertion, and validation: +/// checked auxiliary-data mutation, insertion, and validation: /// /// ```rust /// use delaunay::prelude::construction::{ @@ -263,6 +264,14 @@ pub enum DelaunayError { source: InsertionError, }, + /// User-data mutation through a checked TDS key failed. + #[error(transparent)] + TdsMutation { + /// Underlying TDS mutation failure. + #[from] + source: TdsMutationError, + }, + /// Validation policy configuration failed. #[error(transparent)] ValidationConfiguration { @@ -292,7 +301,8 @@ pub enum DelaunayError { /// /// This is equivalent to `Result` with [`DelaunayError`] as /// the error type, and is intended for caller-facing examples and applications -/// that use the standard construction, insertion, and validation APIs. +/// that use the standard construction, checked auxiliary-data mutation, +/// insertion, and validation APIs. pub type DelaunayResult = Result; /// Errors that can occur during Delaunay triangulation construction. diff --git a/src/delaunay/delaunayize.rs b/src/delaunay/delaunayize.rs index 4615d01f..12678e7e 100644 --- a/src/delaunay/delaunayize.rs +++ b/src/delaunay/delaunayize.rs @@ -73,10 +73,10 @@ use crate::core::algorithms::pl_manifold_repair::{ }; use crate::core::collections::{Entry, FastHashMap, SimplexVertexUuidBuffer}; use crate::core::simplex::Simplex; -use crate::core::tds::{SimplexKey, Tds}; +use crate::core::tds::{SimplexKey, Tds, TdsMutationError}; use crate::core::traits::data_type::DataType; use crate::core::vertex::Vertex; -use crate::geometry::kernel::{ExactPredicates, Kernel}; +use crate::geometry::kernel::ExactPredicates; use crate::repair::DelaunayRepairHeuristicConfig; use crate::triangulation::DelaunayTriangulation; use thiserror::Error; @@ -313,7 +313,7 @@ pub enum DelaunayizeError { #[source] source: PlManifoldRepairError, /// The simplex-data restoration error from the rebuilt triangulation. - restore_error: SimplexValidationError, + restore_error: SimplexDataRestoreError, }, /// Delaunay flip repair failed; no fallback rebuild was attempted @@ -360,7 +360,7 @@ pub enum DelaunayizeError { #[source] source: DelaunayRepairError, /// The simplex-data restoration error from the rebuilt triangulation. - restore_error: SimplexValidationError, + restore_error: SimplexDataRestoreError, }, /// Fallback rebuild was enabled, but the pre-repair simplex-payload snapshot @@ -379,6 +379,66 @@ pub enum DelaunayizeError { // HELPERS // ============================================================================= +/// Errors that can occur while restoring simplex payloads after a fallback rebuild. +/// +/// Restoration first identifies rebuilt simplices by their sorted vertex UUID +/// set, then commits payloads through checked TDS mutation APIs. These are +/// separate failure modes so callers can distinguish corrupted simplex +/// identity data from stale mutation handles. +/// +/// # Examples +/// +/// ```rust +/// use delaunay::prelude::delaunayize::SimplexDataRestoreError; +/// use delaunay::prelude::tds::{ +/// SimplexKey, TdsError, TdsMutationError, VertexKey, SimplexValidationError, +/// }; +/// use slotmap::KeyData; +/// +/// let identity_error = SimplexDataRestoreError::SimplexIdentity { +/// source: SimplexValidationError::VertexKeyNotFound { +/// key: VertexKey::from(KeyData::from_ffi(0xBAD)), +/// }, +/// }; +/// std::assert_matches!( +/// identity_error, +/// SimplexDataRestoreError::SimplexIdentity { .. } +/// ); +/// +/// let mutation_error = TdsMutationError::from(TdsError::SimplexNotFound { +/// simplex_key: SimplexKey::from(KeyData::from_ffi(0xCAFE)), +/// context: "restore simplex payload".to_string(), +/// }); +/// let assignment_error = SimplexDataRestoreError::PayloadAssignment { +/// source: mutation_error, +/// }; +/// std::assert_matches!( +/// assignment_error, +/// SimplexDataRestoreError::PayloadAssignment { .. } +/// ); +/// ``` +#[derive(Clone, Debug, Error, PartialEq)] +#[non_exhaustive] +pub enum SimplexDataRestoreError { + /// A rebuilt simplex could not resolve its vertex UUID identity. + #[error("rebuilt simplex identity lookup failed: {source}")] + SimplexIdentity { + /// The simplex validation failure encountered while reading vertex UUIDs. + #[from] + #[source] + source: SimplexValidationError, + }, + + /// A rebuilt simplex payload could not be assigned through the checked setter. + #[error("rebuilt simplex payload assignment failed: {source}")] + PayloadAssignment { + /// The TDS mutation failure encountered while assigning simplex data. + #[from] + #[source] + source: TdsMutationError, + }, +} + #[derive(Clone, Debug, PartialEq, Eq)] enum SimplexDataMatch { Unique(Option), @@ -394,8 +454,8 @@ fn snapshot_rebuild_state( tds: &Tds, ) -> Result, SimplexValidationError> where - U: DataType, - V: DataType, + U: Copy, + V: Copy, { let vertices = tds .vertices() @@ -411,8 +471,7 @@ fn collect_simplex_data( tds: &Tds, ) -> Result, SimplexValidationError> where - U: DataType, - V: DataType, + V: Copy, { let mut simplex_data = FastHashMap::default(); for (_, simplex) in tds.simplices() { @@ -434,11 +493,7 @@ where fn simplex_vertex_uuids( tds: &Tds, simplex: &Simplex, -) -> Result -where - U: DataType, - V: DataType, -{ +) -> Result { let mut vertex_uuids = simplex .vertex_uuid_iter(tds) .collect::>()?; @@ -451,11 +506,9 @@ where fn restore_simplex_data( rebuilt: &mut DelaunayTriangulation, original_simplex_data: &SimplexDataByVertexUuids, -) -> Result<(), SimplexValidationError> +) -> Result<(), SimplexDataRestoreError> where - K: Kernel, - U: DataType, - V: DataType, + V: Copy, { let mut assignments: Vec<(SimplexKey, V)> = Vec::new(); for (simplex_key, simplex) in rebuilt.simplices() { @@ -468,7 +521,7 @@ where } for (simplex_key, data) in assignments { - rebuilt.set_simplex_data(simplex_key, Some(data)); + rebuilt.set_simplex_data(simplex_key, Some(data))?; } Ok(()) @@ -486,7 +539,7 @@ enum FallbackRebuildError { Restore { #[from] #[source] - source: SimplexValidationError, + source: SimplexDataRestoreError, }, } @@ -767,7 +820,7 @@ mod tests { use super::*; use crate::geometry::kernel::AdaptiveKernel; use crate::geometry::point::Point; - use crate::tds::VertexKey; + use crate::tds::{TdsError, VertexKey}; use crate::try_vertices_from_points; use crate::{DelaunayTriangulationBuilder, TriangulationConstructionError}; use slotmap::KeyData; @@ -1078,8 +1131,10 @@ mod tests { let delaunay_source = DelaunayRepairError::PostconditionFailed { reason: Box::new(DelaunayRepairPostconditionFailure::Disconnected { simplex_count: 1 }), }; - let restore_error = SimplexValidationError::VertexKeyNotFound { - key: VertexKey::from(KeyData::from_ffi(0xBAD)), + let restore_error = SimplexDataRestoreError::SimplexIdentity { + source: SimplexValidationError::VertexKeyNotFound { + key: VertexKey::from(KeyData::from_ffi(0xBAD)), + }, }; let topology_err = DelaunayizeError::TopologyRepairFailedWithRebuildRestore { @@ -1111,6 +1166,27 @@ mod tests { ); } + #[test] + fn test_payload_assignment_restore_error_source() { + let source = TdsMutationError::from(TdsError::SimplexNotFound { + simplex_key: SimplexKey::from(KeyData::from_ffi(0xCAFE)), + context: "restore simplex payload".to_string(), + }); + let err = SimplexDataRestoreError::PayloadAssignment { + source: source.clone(), + }; + + assert_eq!( + err, + SimplexDataRestoreError::PayloadAssignment { + source: source.clone() + } + ); + assert!(err.to_string().contains("payload assignment failed")); + let error_source = StdError::source(&err).unwrap(); + assert_eq!(error_source.to_string(), source.to_string()); + } + #[test] fn test_topology_rebuild_error_mapping() { let source = PlManifoldRepairError::NoProgress { @@ -1119,8 +1195,10 @@ mod tests { simplices_removed: 4, }; let rebuild_error = construction_error(); - let restore_error = SimplexValidationError::VertexKeyNotFound { - key: VertexKey::from(KeyData::from_ffi(0xBAD)), + let restore_error = SimplexDataRestoreError::SimplexIdentity { + source: SimplexValidationError::VertexKeyNotFound { + key: VertexKey::from(KeyData::from_ffi(0xBAD)), + }, }; let rebuild_err = topology_rebuild_error( @@ -1168,8 +1246,10 @@ mod tests { reason: Box::new(DelaunayRepairPostconditionFailure::Disconnected { simplex_count: 1 }), }; let rebuild_error = construction_error(); - let restore_error = SimplexValidationError::VertexKeyNotFound { - key: VertexKey::from(KeyData::from_ffi(0xBAD)), + let restore_error = SimplexDataRestoreError::SimplexIdentity { + source: SimplexValidationError::VertexKeyNotFound { + key: VertexKey::from(KeyData::from_ffi(0xBAD)), + }, }; let rebuild_err = delaunay_rebuild_error( @@ -1332,7 +1412,7 @@ mod tests { .build::() .unwrap(); let original_simplex_key = dt.simplices().next().unwrap().0; - dt.set_simplex_data(original_simplex_key, Some(42)); + dt.set_simplex_data(original_simplex_key, Some(42)).unwrap(); let tds = &dt.as_triangulation().tds; let vertices: Vec<_> = tds diff --git a/src/delaunay/query.rs b/src/delaunay/query.rs index 5ebdd476..2e812ccc 100644 --- a/src/delaunay/query.rs +++ b/src/delaunay/query.rs @@ -14,7 +14,7 @@ use crate::core::edge::EdgeKey; use crate::core::facet::{AllFacetsIter, BoundaryFacetsIter}; use crate::core::query::QueryError; use crate::core::simplex::Simplex; -use crate::core::tds::{SimplexKey, Tds, VertexKey}; +use crate::core::tds::{SimplexKey, Tds, TdsError, TdsMutationError, VertexKey}; use crate::core::triangulation::Triangulation; use crate::core::validation::{TopologyGuarantee, ValidationConfigurationError, ValidationPolicy}; use crate::core::vertex::Vertex; @@ -220,7 +220,7 @@ impl DelaunayTriangulation { self.tri.vertices() } - /// Sets the auxiliary data on a returning the previous value. + /// Sets the auxiliary data on a vertex, returning the previous value. /// /// This is a safe O(1) operation that modifies only the user-data field. /// It does not affect geometry, topology, or Delaunay invariants, so @@ -228,8 +228,12 @@ impl DelaunayTriangulation { /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a vertex in the + /// underlying TDS. /// /// # Examples /// @@ -244,6 +248,8 @@ impl DelaunayTriangulation { /// # Source(#[from] delaunay::DelaunayTriangulationConstructionError), /// # #[error(transparent)] /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), + /// # #[error(transparent)] + /// # TdsMutation(#[from] delaunay::prelude::tds::TdsMutationError), /// # } /// # fn main() -> Result<(), ExampleError> { /// let vertices: [Vertex; 3] = [ @@ -256,18 +262,22 @@ impl DelaunayTriangulation { /// return Ok(()); /// }; /// - /// let prev = dt.set_vertex_data(key, Some(99)); + /// let prev = dt.set_vertex_data(key, Some(99))?; /// assert!(prev.is_some()); /// /// // Clear data - /// let prev = dt.set_vertex_data(key, None); - /// assert_eq!(prev, Some(Some(99))); + /// let prev = dt.set_vertex_data(key, None)?; + /// assert_eq!(prev, Some(99)); /// assert_eq!(dt.tds().vertex(key).map(|v| v.data()), Some(None)); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_vertex_data(&mut self, key: VertexKey, data: Option) -> Option> { + pub fn set_vertex_data( + &mut self, + key: VertexKey, + data: Option, + ) -> Result, TdsMutationError> { self.tri.tds.set_vertex_data(key, data) } @@ -279,8 +289,12 @@ impl DelaunayTriangulation { /// /// # Returns /// - /// `None` if the key is not found. `Some(previous)` where `previous` is - /// the old `Option` value if the key exists. + /// The old `Option` value when the key exists. + /// + /// # Errors + /// + /// Returns [`TdsMutationError`] if `key` does not identify a simplex in + /// the underlying TDS. /// /// # Examples /// @@ -293,6 +307,8 @@ impl DelaunayTriangulation { /// # Source(#[from] delaunay::DelaunayTriangulationConstructionError), /// # #[error(transparent)] /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), + /// # #[error(transparent)] + /// # TdsMutation(#[from] delaunay::prelude::tds::TdsMutationError), /// # } /// # fn main() -> Result<(), ExampleError> { /// let vertices = [ @@ -305,18 +321,22 @@ impl DelaunayTriangulation { /// return Ok(()); /// }; /// - /// let prev = dt.set_simplex_data(key, Some(42)); - /// assert_eq!(prev, Some(None)); + /// let prev = dt.set_simplex_data(key, Some(42))?; + /// assert_eq!(prev, None); /// /// // Clear data - /// let prev = dt.set_simplex_data(key, None); - /// assert_eq!(prev, Some(Some(42))); + /// let prev = dt.set_simplex_data(key, None)?; + /// assert_eq!(prev, Some(42)); /// assert_eq!(dt.tds().simplex(key).map(|s| s.data()), Some(None)); /// # Ok(()) /// # } /// ``` #[inline] - pub fn set_simplex_data(&mut self, key: SimplexKey, data: Option) -> Option> { + pub fn set_simplex_data( + &mut self, + key: SimplexKey, + data: Option, + ) -> Result, TdsMutationError> { self.tri.tds.set_simplex_data(key, data) } @@ -482,7 +502,7 @@ impl DelaunayTriangulation { /// /// Returns [`QueryError::TriangulationCorrupted`] if facet-map construction /// detects invalid simplex or facet bookkeeping. The variant preserves the - /// lower-level [`TdsError`](crate::tds::TdsError) for diagnostics. + /// lower-level [`TdsError`] for diagnostics. /// Individual iterator items return [`FacetError`](crate::prelude::tds::FacetError) /// if a boundary facet cannot be created or keyed from the simplices. pub fn boundary_facets(&self) -> Result, QueryError> { @@ -1171,11 +1191,17 @@ impl DelaunayTriangulation { /// Returns a slice view of a simplex's vertex keys. /// - /// This is a zero-allocation accessor. If `c` is not present, returns `None`. + /// This is a zero-allocation accessor that validates the simplex key and + /// referenced vertex keys before lending the canonical slice. /// /// This is a convenience wrapper around /// [`Triangulation::simplex_vertices`](crate::Triangulation::simplex_vertices). /// + /// # Errors + /// + /// Returns [`TdsError`] if `c` does not identify a simplex in this + /// triangulation, or if the simplex references a missing vertex key. + /// /// # Examples /// /// ```rust @@ -1187,6 +1213,8 @@ impl DelaunayTriangulation { /// # #[error(transparent)] /// # Source(#[from] delaunay::DelaunayTriangulationConstructionError), /// # #[error(transparent)] + /// # Tds(#[from] delaunay::prelude::tds::TdsError), + /// # #[error(transparent)] /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), /// # } /// # fn main() -> Result<(), ExampleError> { @@ -1200,15 +1228,12 @@ impl DelaunayTriangulation { /// let Some((simplex_key, _)) = dt.simplices().next() else { /// return Ok(()); /// }; - /// let Some(simplex_vertices) = dt.simplex_vertices(simplex_key) else { - /// return Ok(()); - /// }; + /// let simplex_vertices = dt.simplex_vertices(simplex_key)?; /// assert_eq!(simplex_vertices.len(), 3); // D+1 for a 2D simplex /// # Ok(()) /// # } /// ``` - #[must_use] - pub fn simplex_vertices(&self, c: SimplexKey) -> Option<&[VertexKey]> { + pub fn simplex_vertices(&self, c: SimplexKey) -> Result<&[VertexKey], TdsError> { self.as_triangulation().simplex_vertices(c) } @@ -1263,7 +1288,7 @@ mod tests { use crate::core::operations::DelaunayInsertionState; use crate::core::tds::TdsError; use crate::geometry::kernel::{AdaptiveKernel, FastKernel}; - use std::{collections::HashSet, num::NonZeroUsize, sync::Once}; + use std::{assert_matches, collections::HashSet, num::NonZeroUsize, sync::Once}; struct Payload; @@ -1773,6 +1798,9 @@ mod tests { // Missing keys should behave the same as on `Triangulation`. assert!(dt.vertex_coords(VertexKey::default()).is_none()); - assert!(dt.simplex_vertices(SimplexKey::default()).is_none()); + assert_matches!( + dt.simplex_vertices(SimplexKey::default()), + Err(TdsError::SimplexNotFound { .. }) + ); } } diff --git a/src/geometry/algorithms/convex_hull.rs b/src/geometry/algorithms/convex_hull.rs index 71272268..ded2fc58 100644 --- a/src/geometry/algorithms/convex_hull.rs +++ b/src/geometry/algorithms/convex_hull.rs @@ -433,7 +433,8 @@ pub struct ConvexHull { /// Use `is_valid_for_triangulation()` to check validity before use. /// /// This field is private to prevent external mutation. Use the provided read-only - /// accessors (`facets()`, `facet()`, `number_of_facets()`) to access hull facets. + /// accessors (`facets(triangulation)`, `facet_handles()`, `facet()`, `number_of_facets()`) + /// to access hull facets. hull_facets: Vec, /// Cache for the facet-to-simplices mapping to avoid rebuilding it for each facet check /// Uses `ArcSwapOption` for lock-free atomic updates when cache needs invalidation @@ -555,7 +556,11 @@ impl ConvexHull { self.hull_facets.get(index) } - /// Returns an iterator over the hull facets + /// Returns an iterator over the hull facet handles. + /// + /// These handles are detached, runtime-local references into the TDS state + /// captured when the hull was built. Use [`Self::facets`] when callers need + /// borrowed [`FacetView`] access with hull freshness checked at the boundary. /// /// # Examples /// @@ -592,24 +597,86 @@ impl ConvexHull { /// ConvexHull::try_from_triangulation(dt.as_triangulation())?; /// /// // Iterate over all hull facets - /// let facet_count = hull.facets().count(); + /// let facet_count = hull.facet_handles().count(); /// assert_eq!(facet_count, 4); // Tetrahedron has 4 faces /// - /// // Check that all facets have the expected number of vertices - /// // Note: facets() returns FacetHandle structs - need to create FacetView to access vertices - /// use delaunay::prelude::tds::FacetView; - /// for facet_handle in hull.facets() { - /// if let Ok(facet_view) = FacetView::try_new(&dt.tds(), facet_handle.simplex_key(), facet_handle.facet_index()) { - /// assert_eq!(facet_view.vertices()?.count(), 3); // 3D facets have 3 vertices - /// } + /// // Check that all facets have the expected number of vertices. + /// for facet_view in hull.facets(dt.as_triangulation())? { + /// assert_eq!(facet_view?.vertices()?.count(), 3); // 3D facets have 3 vertices /// } /// # Ok(()) /// # } /// ``` - pub fn facets(&self) -> std::slice::Iter<'_, FacetHandle> { + pub fn facet_handles(&self) -> std::slice::Iter<'_, FacetHandle> { self.hull_facets.iter() } + /// Returns borrowed facet views for this hull in a live triangulation. + /// + /// This is the borrowed-view counterpart to [`Self::facet_handles`]. It first + /// verifies that the hull still belongs to `tri` and that the triangulation + /// generation matches the hull's creation generation, then yields + /// [`FacetView`] values lifetime-bound to the supplied triangulation. Holding + /// any returned view therefore keeps the source triangulation immutably + /// borrowed. + /// + /// # Errors + /// + /// Returns [`ConvexHullConstructionError::StaleHull`] or + /// [`ConvexHullConstructionError::IdentityMismatch`] if the hull no longer + /// matches `tri`. Individual iterator items return [`FacetError`] if a + /// stored handle no longer resolves to a valid facet despite the freshness + /// check. + /// + /// # Examples + /// + /// ```rust + /// use delaunay::prelude::{DelaunayTriangulation, DelaunayTriangulationBuilder}; + /// use delaunay::prelude::query::ConvexHull; + /// + /// # #[derive(Debug, thiserror::Error)] + /// # enum ExampleError { + /// # #[error(transparent)] + /// # Construction(#[from] delaunay::prelude::DelaunayTriangulationConstructionError), + /// # #[error(transparent)] + /// # Hull(#[from] delaunay::prelude::query::ConvexHullConstructionError), + /// # #[error(transparent)] + /// # Facet(#[from] delaunay::prelude::tds::FacetError), + /// # #[error(transparent)] + /// # Coordinate(#[from] delaunay::prelude::geometry::CoordinateConversionError), + /// # } + /// # fn main() -> Result<(), ExampleError> { + /// let vertices = vec![ + /// delaunay::vertex![0.0, 0.0, 0.0]?, + /// delaunay::vertex![1.0, 0.0, 0.0]?, + /// delaunay::vertex![0.0, 1.0, 0.0]?, + /// delaunay::vertex![0.0, 0.0, 1.0]?, + /// ]; + /// let dt: DelaunayTriangulation<_, (), (), 3> = + /// DelaunayTriangulationBuilder::new(&vertices).build::<()>()?; + /// let hull = ConvexHull::try_from_triangulation(dt.as_triangulation())?; + /// + /// for facet in hull.facets(dt.as_triangulation())? { + /// assert_eq!(facet?.vertices()?.count(), 3); + /// } + /// # Ok(()) + /// # } + /// ``` + pub fn facets<'tds, K>( + &'tds self, + tri: &'tds Triangulation, + ) -> Result< + impl Iterator, FacetError>> + 'tds, + ConvexHullConstructionError, + > { + self.ensure_current_for_construction(tri)?; + let tds = &tri.tds; + Ok(self + .hull_facets + .iter() + .map(move |handle| (*handle).view(tds))) + } + /// Returns true if the convex hull is empty (has no facets) /// /// # Examples @@ -768,10 +835,7 @@ impl ConvexHull { /// # } /// ``` #[must_use] - pub fn is_valid_for_triangulation(&self, tri: &Triangulation) -> bool - where - K: Kernel, - { + pub fn is_valid_for_triangulation(&self, tri: &Triangulation) -> bool { // Use creation_generation (immutable) for validity check, not cached_generation (mutable) // Empty hull (with creation_generation unset) is always valid - it has no facets to be stale let Some(&creation_generation) = self.creation_generation.get() else { @@ -784,85 +848,95 @@ impl ConvexHull { && Arc::ptr_eq(creation_identity, tri.tds.identity()) } + /// Returns the TDS identity captured by this hull, or nil for uninitialized empty hulls. + /// + /// This keeps validation and construction errors diagnosable without + /// exposing the private `Arc` identity token used for same-owner + /// freshness checks. fn hull_identity_uuid(&self) -> uuid::Uuid { self.creation_identity .get() .map_or_else(uuid::Uuid::nil, |identity| *identity.as_ref()) } - fn tds_identity_uuid(tri: &Triangulation) -> uuid::Uuid - where - K: Kernel, - { + /// Returns the runtime identity of the TDS currently owned by `tri`. + /// + /// Generation counters are meaningful only within a single TDS identity, so + /// public hull view conversion reports both identities when a detached hull + /// is supplied with the wrong triangulation. + fn tds_identity_uuid(tri: &Triangulation) -> uuid::Uuid { *tri.tds.identity().as_ref() } - /// Helper to construct a `StaleHull` error with generation info + /// Builds a validation stale-hull error from the immutable creation generation. /// - /// Centralizes the error construction pattern to avoid duplication. + /// Validation and borrowed-view entry points use the same generation + /// comparison, so this helper keeps their diagnostic payloads aligned. #[inline] - fn stale_hull_error(&self, tri: &Triangulation) -> ConvexHullValidationError - where - K: Kernel, - { + fn stale_hull_error(&self, tri: &Triangulation) -> ConvexHullValidationError { ConvexHullValidationError::StaleHull { hull_generation: self.creation_generation.get().copied().unwrap_or(0), tds_generation: tri.tds.generation(), } } + /// Builds a validation identity-mismatch error for a hull used with the wrong TDS. + /// + /// Borrowed hull views must be derived from the same canonical owner that + /// produced the detached handles. This diagnostic preserves both identities + /// when that same-owner condition fails. #[inline] fn identity_mismatch_error( &self, tri: &Triangulation, - ) -> ConvexHullValidationError - where - K: Kernel, - { + ) -> ConvexHullValidationError { ConvexHullValidationError::IdentityMismatch { hull_identity: self.hull_identity_uuid(), tds_identity: Self::tds_identity_uuid(tri), } } - /// Helper to construct a `StaleHull` construction error with generation info + /// Builds a construction stale-hull error from the immutable creation generation. /// - /// Centralizes the error construction pattern to avoid duplication. + /// Construction-facing APIs return [`ConvexHullConstructionError`], but they + /// enforce the same freshness invariant as validation. #[inline] fn stale_hull_construction_error( &self, tri: &Triangulation, - ) -> ConvexHullConstructionError - where - K: Kernel, - { + ) -> ConvexHullConstructionError { ConvexHullConstructionError::StaleHull { hull_generation: self.creation_generation.get().copied().unwrap_or(0), tds_generation: tri.tds.generation(), } } + /// Builds a construction identity-mismatch error for a hull used with the wrong TDS. + /// + /// This is the construction-error counterpart to + /// [`Self::identity_mismatch_error`] for APIs such as [`Self::facets`] that + /// return borrowed views after checking same-owner freshness. #[inline] fn identity_mismatch_construction_error( &self, tri: &Triangulation, - ) -> ConvexHullConstructionError - where - K: Kernel, - { + ) -> ConvexHullConstructionError { ConvexHullConstructionError::IdentityMismatch { hull_identity: self.hull_identity_uuid(), tds_identity: Self::tds_identity_uuid(tri), } } + /// Verifies that construction-facing borrowed views can safely resolve this hull. + /// + /// Empty synthetic hulls without recorded creation metadata are accepted. + /// Otherwise the live triangulation must match both the stored generation + /// and the stored TDS identity before detached handles are converted into + /// borrowed [`FacetView`] values. fn ensure_current_for_construction( &self, tri: &Triangulation, - ) -> Result<(), ConvexHullConstructionError> - where - K: Kernel, - { + ) -> Result<(), ConvexHullConstructionError> { if self.is_empty() && self.creation_generation.get().is_none() { return Ok(()); } @@ -879,13 +953,15 @@ impl ConvexHull { Ok(()) } + /// Verifies that validation-facing queries still observe the hull's source TDS. + /// + /// This mirrors [`Self::ensure_current_for_construction`] while preserving + /// the validation-specific error type used by + /// [`Self::is_valid_for_triangulation`]. fn ensure_current_for_validation( &self, tri: &Triangulation, - ) -> Result<(), ConvexHullValidationError> - where - K: Kernel, - { + ) -> Result<(), ConvexHullValidationError> { if self.is_empty() && self.creation_generation.get().is_none() { return Ok(()); } @@ -1078,8 +1154,8 @@ where ConvexHullConstructionError::BoundaryFacetExtractionFailed { source } })?; - // Collect facet handles (SimplexKey, facet_index) for storage - // These can be used to reconstruct FacetViews when needed + // Collect detached facet handles for storage. Borrowed FacetViews are + // reconstructed later through ConvexHull::facets after freshness checks. let hull_facets: Vec<_> = hull_facets_iter .map(|facet_view| { let facet_view = facet_view.map_err(|source| { @@ -2105,7 +2181,7 @@ mod tests { /// - `is_empty()` returns false for non-empty hull /// /// 2. **Facet access test** (via `pastey::paste`) - Tests: - /// - `facets()` iterator returns correct count + /// - `facet_handles()` iterator returns correct count /// - `facet(0)` returns Some for valid index /// - `facet(out_of_bounds)` returns None /// @@ -2179,11 +2255,11 @@ mod tests { let hull: ConvexHull<(), (), $dim> = ConvexHull::try_from_triangulation(dt.as_triangulation()).unwrap(); - // Test facet iterator + // Test detached facet-handle iterator assert_eq!( - hull.facets().count(), + hull.facet_handles().count(), $expected_facets, - "{}D facets iterator should return {} facets", + "{}D facet-handle iterator should return {} facets", $dim, $expected_facets ); @@ -2314,9 +2390,9 @@ mod tests { "Empty hull should not have facets" ); assert_eq!( - empty_hull.facets().count(), + empty_hull.facet_handles().count(), 0, - "Empty hull's facets iterator should be empty" + "Empty hull's facet-handle iterator should be empty" ); // Test cache behavior on empty hull @@ -2483,7 +2559,7 @@ mod tests { } // Test individual facet visibility for each facet - for (i, facet) in hull_3d.facets().enumerate() { + for (i, facet) in hull_3d.facet_handles().enumerate() { let visibility_result = hull_3d.is_facet_visible_from_point( facet, &far_outside_point, @@ -3409,22 +3485,17 @@ mod tests { "Very large index should return None" ); - // Test iterator - let facet_count_via_iter = hull.facets().count(); + // Test detached facet-handle iterator + let facet_count_via_iter = hull.facet_handles().count(); assert_eq!( facet_count_via_iter, hull.number_of_facets(), - "Iterator count should match facet_count" + "Handle iterator count should match facet_count" ); - // Verify all facets in iterator are valid - create FacetView to check vertices - for facet_handle in hull.facets() { - let facet_view = FacetView::try_new( - &dt.as_triangulation().tds, - facet_handle.simplex_key(), - facet_handle.facet_index(), - ) - .unwrap(); + // Verify all borrowed facet views are valid. + for facet_view in hull.facets(dt.as_triangulation()).unwrap() { + let facet_view = facet_view.unwrap(); let vertex_count = facet_view.vertices().unwrap().count(); assert!(vertex_count > 0, "Each facet should have vertices"); } @@ -3869,8 +3940,8 @@ mod tests { assert!(hull.facet(4).is_none()); assert!(hull.facet(100).is_none()); - // Test facets iterator - let facet_count = hull.facets().count(); + // Test detached facet-handle iterator + let facet_count = hull.facet_handles().count(); assert_eq!(facet_count, 4); } @@ -3965,8 +4036,8 @@ mod tests { let dt = create_triangulation(&vertices); let hull = ConvexHull::try_from_triangulation(dt.as_triangulation()).unwrap(); - // Test that facets() iterator produces the same results as facet - let iter_facets: Vec<_> = hull.facets().collect(); + // Test that facet_handles() iterator produces the same results as facet + let iter_facets: Vec<_> = hull.facet_handles().collect(); assert_eq!(iter_facets.len(), hull.number_of_facets()); @@ -3977,8 +4048,8 @@ mod tests { } // Test multiple iterations produce same results - let first_iteration: Vec<_> = hull.facets().collect(); - let second_iteration: Vec<_> = hull.facets().collect(); + let first_iteration: Vec<_> = hull.facet_handles().collect(); + let second_iteration: Vec<_> = hull.facet_handles().collect(); assert_eq!(first_iteration.len(), second_iteration.len()); for (i, (f1, f2)) in first_iteration @@ -7396,6 +7467,16 @@ mod tests { "is_point_outside should fail with StaleHull error" ); + test_debug!(" Testing facets..."); + let facets_result = hull.facets(dt.as_triangulation()); + assert!( + matches!( + facets_result, + Err(ConvexHullConstructionError::StaleHull { .. }) + ), + "facets should fail with StaleHull error" + ); + test_debug!(" Testing validate..."); let validate_result = hull.validate(dt.as_triangulation()); // validate() should fail with explicit StaleHull error diff --git a/src/geometry/quality.rs b/src/geometry/quality.rs index dcf52657..9df1172d 100644 --- a/src/geometry/quality.rs +++ b/src/geometry/quality.rs @@ -261,7 +261,7 @@ where // Use SmallBuffer to avoid heap allocation (simplices have D+1 vertices, D ≤ MAX_PRACTICAL_DIMENSION_SIZE) let mut points = SmallBuffer::new(); - for &vkey in &vertex_keys { + for &vkey in vertex_keys { let point = tri .tds .vertex(vkey) diff --git a/src/lib.rs b/src/lib.rs index 7bc7f62c..3faf9e4c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1803,7 +1803,7 @@ mod tests { prelude::delaunayize::{ DelaunayTriangulationConstructionError, DelaunayizeConfig, DelaunayizeError, DelaunayizeOutcome, PlManifoldRepairError, PlManifoldRepairStats, - SimplexValidationError, + SimplexDataRestoreError, SimplexValidationError, }, prelude::repair::{ DelaunayCheckPolicy, DelaunayRepairError, DelaunayRepairOutcome, DelaunayRepairPolicy, @@ -1840,6 +1840,7 @@ mod tests { assert!(is_normal::()); assert!(is_normal::()); assert!(is_normal::>()); + assert!(is_normal::()); assert!(is_normal::()); assert!(is_normal::()); assert!(is_normal::()); diff --git a/src/topology/manifold.rs b/src/topology/manifold.rs index 1f65629a..591692f8 100644 --- a/src/topology/manifold.rs +++ b/src/topology/manifold.rs @@ -1627,7 +1627,9 @@ pub fn validate_ridge_links(tds: &Tds) -> Result< Vec::with_capacity(star.star_simplices.len()); for &simplex_key in &star.star_simplices { match tds.simplex_vertices(simplex_key) { - Ok(vertices) => star_simplex_vertices.push((simplex_key, vertices)), + Ok(vertices) => { + star_simplex_vertices.push((simplex_key, vertices.into())); + } Err(_) => star_simplex_vertices.push((simplex_key, VertexKeyBuffer::new())), } } @@ -1712,7 +1714,9 @@ pub fn validate_ridge_links_for_simplices( Vec::with_capacity(star.star_simplices.len()); for &simplex_key in &star.star_simplices { match tds.simplex_vertices(simplex_key) { - Ok(vertices) => star_simplex_vertices.push((simplex_key, vertices)), + Ok(vertices) => { + star_simplex_vertices.push((simplex_key, vertices.into())); + } Err(_) => star_simplex_vertices.push((simplex_key, VertexKeyBuffer::new())), } } diff --git a/tests/prelude_exports.rs b/tests/prelude_exports.rs index 8c9a9544..0ad10bb1 100644 --- a/tests/prelude_exports.rs +++ b/tests/prelude_exports.rs @@ -44,7 +44,7 @@ use delaunay::prelude::construction::{ }; use delaunay::prelude::delaunayize::{ DelaunayTriangulationBuilder as DelaunayizeDelaunayTriangulationBuilder, DelaunayizeConfig, - DelaunayizeError, DelaunayizeOutcome, delaunayize_by_flips, + DelaunayizeError, DelaunayizeOutcome, SimplexDataRestoreError, delaunayize_by_flips, }; use delaunay::prelude::diagnostics::ConstructionTelemetry; #[cfg(feature = "diagnostics")] @@ -86,8 +86,8 @@ use delaunay::prelude::ordering::{ }; use delaunay::prelude::query::{ AllFacetsIter as QueryAllFacetsIter, BoundaryFacetsIter as QueryBoundaryFacetsIter, ConvexHull, - EdgeIndex as QueryEdgeIndex, IncidenceView as QueryIncidenceView, QueryError, - SimplexNeighborIndex as QuerySimplexNeighborIndex, TopologyIndexBuildError, + ConvexHullConstructionError, EdgeIndex as QueryEdgeIndex, IncidenceView as QueryIncidenceView, + QueryError, SimplexNeighborIndex as QuerySimplexNeighborIndex, TopologyIndexBuildError, TriangulationAdjacency as QueryTriangulationAdjacency, }; use delaunay::prelude::repair::{ @@ -191,6 +191,8 @@ enum PreludeExportTestError { #[error(transparent)] Query(#[from] QueryError), #[error(transparent)] + ConvexHull(#[from] ConvexHullConstructionError), + #[error(transparent)] TopologyIndex(#[from] TopologyIndexBuildError), #[error(transparent)] Facet(#[from] FacetError), @@ -606,7 +608,12 @@ fn preludes_cover_bench_apis() -> Result<(), PreludeExportTestError> { }) })?; assert!(boundary_facet_count > 0); - let _hull = ConvexHull::try_from_triangulation(dt.as_triangulation()).unwrap(); + let hull = ConvexHull::try_from_triangulation(dt.as_triangulation())?; + assert_eq!(hull.facet_handles().count(), boundary_facet_count); + let hull_facet_view_count = hull + .facets(dt.as_triangulation())? + .try_fold(0_usize, |count, facet| facet.map(|_| count + 1))?; + assert_eq!(hull_facet_view_count, boundary_facet_count); dt.validate().unwrap(); assert_bistellar_flips(&dt); @@ -1402,6 +1409,7 @@ fn diagnostic_preludes_cover_repair_apis() -> Result<(), PreludeExportTestError> assert!(!outcome.used_fallback_rebuild); let _typed_outcome: DelaunayizeOutcome<(), (), 3> = outcome; let _typed_error: Option = None; + assert_send_sync_unpin::(); Ok(()) } diff --git a/tests/public_topology_api.rs b/tests/public_topology_api.rs index fafdaecf..d7bb870b 100644 --- a/tests/public_topology_api.rs +++ b/tests/public_topology_api.rs @@ -10,6 +10,7 @@ use delaunay::prelude::DelaunayTriangulationConstructionError; use delaunay::prelude::TopologyGuarantee; use delaunay::prelude::geometry::CoordinateConversionError; use delaunay::prelude::query::*; +use delaunay::prelude::tds::TdsError; use std::collections::HashSet; #[derive(Debug, thiserror::Error)] @@ -26,8 +27,8 @@ enum PublicTopologyApiTestError { EmptySingleTetrahedronSimplices, #[error("single simplex triangulation has no simplices")] EmptySingleSimplexSimplices, - #[error("simplex key from triangulation has no vertices")] - MissingSimplexVertices, + #[error(transparent)] + Tds(#[from] TdsError), #[error("double tetrahedron did not contain the expected shared vertex")] MissingExpectedSharedVertex, } @@ -174,12 +175,7 @@ fn edges_and_incident_edges_on_single_tetrahedron() -> Result<(), PublicTopology dt.simplex_vertices(simplex_key), tri.simplex_vertices(simplex_key) ); - assert_eq!( - dt.simplex_vertices(simplex_key) - .ok_or(PublicTopologyApiTestError::MissingSimplexVertices)? - .len(), - 4 - ); + assert_eq!(dt.simplex_vertices(simplex_key)?.len(), 4); Ok(()) } diff --git a/tests/trait_bound_ergonomics.rs b/tests/trait_bound_ergonomics.rs index c0af04d7..86486a1a 100644 --- a/tests/trait_bound_ergonomics.rs +++ b/tests/trait_bound_ergonomics.rs @@ -8,7 +8,7 @@ use delaunay::prelude::construction::{GlobalTopology, TopologyGuarantee, Topolog use delaunay::prelude::geometry::{Coordinate, CoordinateValidationError, FastKernel, Point}; use delaunay::prelude::query::BoundaryAnalysis; use delaunay::prelude::tds::{ - Simplex, SimplexKey, Tds, Vertex, VertexKey, verify_facet_index_consistency, + Simplex, SimplexKey, Tds, TdsError, Vertex, VertexKey, verify_facet_index_consistency, }; use delaunay::prelude::topology::validation::validate_triangulation_euler; use delaunay::query::{QueryError, TopologyIndexBuildError}; @@ -162,7 +162,10 @@ fn delaunay_empty_query_wrappers_accept_non_datatype_payloads() assert_eq!(dt.edges().count(), 0); assert_eq!(dt.incident_edges(VertexKey::default()).count(), 0); assert_eq!(dt.simplex_neighbors(SimplexKey::default()).count(), 0); - assert_eq!(dt.simplex_vertices(SimplexKey::default()), None); + assert!(matches!( + dt.simplex_vertices(SimplexKey::default()), + Err(TdsError::SimplexNotFound { .. }) + )); assert_eq!(dt.vertex_coords(VertexKey::default()), None); let incidence = dt.incidence()?;