diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 9f8c5602a..944e8544b 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -111,7 +111,7 @@ impl Sweep for (Handle, &Handle, &Surface, Color) { // even if the original edge was a circle, it's still going to be a line // when projected into the new surface. For the side edges, because // we're sweeping along a straight path. - for ((mut half_edge, start), (next_half_edge, end)) in [ + for ((mut half_edge, start), (_, end)) in [ edge_bottom.clone(), edge_up.clone(), edge_top.clone(), @@ -122,9 +122,6 @@ impl Sweep for (Handle, &Handle, &Surface, Color) { .circular_tuple_windows() { half_edge.write().update_as_line_segment(start, end); - half_edge - .write() - .infer_global_form(next_half_edge.read().start_vertex.clone()); } // Finally, we can make sure that all edges refer to the correct global diff --git a/crates/fj-kernel/src/algorithms/sweep/vertex.rs b/crates/fj-kernel/src/algorithms/sweep/vertex.rs index e0af7cbf2..9ba95787a 100644 --- a/crates/fj-kernel/src/algorithms/sweep/vertex.rs +++ b/crates/fj-kernel/src/algorithms/sweep/vertex.rs @@ -28,7 +28,7 @@ impl Sweep for Handle { .clone(); let vertices = [a, b]; - let global_edge = GlobalEdge::new(vertices.clone()).insert(objects); + let global_edge = GlobalEdge::new().insert(objects); // The vertices of the returned `GlobalEdge` are in normalized order, // which means the order can't be relied upon by the caller. Return the diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index a94fc8501..7ea2de376 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -34,15 +34,13 @@ impl TransformObject for HalfEdge { impl TransformObject for GlobalEdge { fn transform_with_cache( self, - transform: &Transform, - objects: &mut Service, - cache: &mut TransformCache, + _: &Transform, + _: &mut Service, + _: &mut TransformCache, ) -> Self { - let vertices = - self.vertices().access_in_normalized_order().map(|vertex| { - vertex.transform_with_cache(transform, objects, cache) - }); - - Self::new(vertices) + // There's nothing to actually transform here, as `GlobalEdge` holds no + // data. We still need this implementation though, as a new `GlobalEdge` + // must be created to represent the new and transformed edge. + Self::new() } } diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index feed2ae9d..ddec16daa 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -54,41 +54,9 @@ pub trait CycleBuilder { impl CycleBuilder for PartialCycle { fn add_half_edge(&mut self) -> Partial { - let mut new_half_edge = Partial::::new(); - - let (first_half_edge, mut last_half_edge) = - match self.half_edges.first() { - Some(first_half_edge) => { - let first_half_edge = first_half_edge.clone(); - let last_half_edge = self - .half_edges - .last() - .cloned() - .unwrap_or_else(|| first_half_edge.clone()); - - (first_half_edge, last_half_edge) - } - None => (new_half_edge.clone(), new_half_edge.clone()), - }; - - { - let shared_surface_vertex = - new_half_edge.read().start_vertex.clone(); - last_half_edge - .write() - .infer_global_form(shared_surface_vertex); - } - - { - let shared_surface_vertex = - first_half_edge.read().start_vertex.clone(); - new_half_edge - .write() - .infer_global_form(shared_surface_vertex); - } - - self.half_edges.push(new_half_edge.clone()); - new_half_edge + let half_edge = Partial::new(); + self.half_edges.push(half_edge.clone()); + half_edge } fn update_as_polygon_from_points( diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 35f2f6551..571878f22 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -6,7 +6,7 @@ use crate::{ curve::{Curve, GlobalPath}, surface::SurfaceGeometry, }, - objects::{GlobalEdge, HalfEdge, Vertex}, + objects::{HalfEdge, Vertex}, partial::{MaybeCurve, Partial, PartialGlobalEdge, PartialHalfEdge}, }; @@ -38,15 +38,6 @@ pub trait HalfEdgeBuilder { end: Point<2>, ) -> Curve; - /// Infer the global form of the half-edge - /// - /// Updates the global form referenced by this half-edge, and also returns - /// it. - fn infer_global_form( - &mut self, - next_vertex: Partial, - ) -> Partial; - /// Infer the vertex positions (surface and global), if not already set fn infer_vertex_positions_if_necessary( &mut self, @@ -88,9 +79,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { *point_boundary = Some(point_curve); } - let next_vertex = self.start_vertex.clone(); - self.infer_global_form(next_vertex); - path } @@ -151,16 +139,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { path } - fn infer_global_form( - &mut self, - next_vertex: Partial, - ) -> Partial { - self.global_form.write().vertices = - [&self.start_vertex, &next_vertex].map(|vertex| vertex.clone()); - - self.global_form.clone() - } - fn infer_vertex_positions_if_necessary( &mut self, surface: &SurfaceGeometry, diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 610731cb5..aa41304c4 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -1,6 +1,10 @@ use fj_math::Point; -use crate::{geometry::curve::Curve, objects::Vertex, storage::Handle}; +use crate::{ + geometry::curve::Curve, + objects::Vertex, + storage::{Handle, HandleWrapper}, +}; /// A directed edge, defined in a surface's 2D space /// @@ -43,7 +47,7 @@ pub struct HalfEdge { curve: Curve, boundary: [Point<1>; 2], start_vertex: Handle, - global_form: Handle, + global_form: HandleWrapper, } impl HalfEdge { @@ -58,7 +62,7 @@ impl HalfEdge { curve, boundary, start_vertex, - global_form, + global_form: global_form.into(), } } @@ -96,16 +100,13 @@ impl HalfEdge { /// An undirected edge, defined in global (3D) coordinates /// /// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no -/// defined direction, and its vertices have no defined order. This means it can -/// be used to determine whether two [`HalfEdge`]s map to the same `GlobalEdge`, -/// regardless of their direction. +/// defined direction. This means it can be used to determine whether two +/// [`HalfEdge`]s map to the same `GlobalEdge`, regardless of their direction. /// /// See [`HalfEdge`]'s documentation for more information on the relationship /// between [`HalfEdge`] and `GlobalEdge`. -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct GlobalEdge { - vertices: VerticesInNormalizedOrder, -} +#[derive(Clone, Debug, Default, Hash)] +pub struct GlobalEdge {} impl GlobalEdge { /// Create a new instance @@ -113,98 +114,7 @@ impl GlobalEdge { /// The order of `vertices` is irrelevant. Two `GlobalEdge`s with the same /// `curve` and `vertices` will end up being equal, regardless of the order /// of `vertices` here. - pub fn new(vertices: [Handle; 2]) -> Self { - let (vertices, _) = VerticesInNormalizedOrder::new(vertices); - - Self { vertices } - } - - /// Access the vertices that bound the edge on the curve - /// - /// As the name indicates, the order of the returned vertices is normalized - /// and might not match the order of the vertices that were passed to - /// [`GlobalEdge::new`]. You must not rely on the vertices being in any - /// specific order. - pub fn vertices(&self) -> &VerticesInNormalizedOrder { - &self.vertices - } -} - -/// The vertices of a [`GlobalEdge`] -/// -/// Since [`GlobalEdge`] is the single global representation of an edge in -/// global space, it must normalize the order of its vertices. Otherwise, it is -/// possible to construct two [`GlobalEdge`] instances that are meant to -/// represent the same edge, but aren't equal. -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct VerticesInNormalizedOrder([Handle; 2]); - -impl VerticesInNormalizedOrder { - /// Construct a new instance of `VerticesInNormalizedOrder` - /// - /// The provided vertices can be in any order. The returned `bool` value - /// indicates whether the normalization changed the order of the vertices. - pub fn new([a, b]: [Handle; 2]) -> (Self, bool) { - if a < b { - (Self([a, b]), false) - } else { - (Self([b, a]), true) - } - } - - /// Access the vertices - /// - /// The vertices in the returned array will be in normalized order. - pub fn access_in_normalized_order(&self) -> [Handle; 2] { - self.0.clone() - } -} - -#[cfg(test)] -mod tests { - use pretty_assertions::assert_eq; - - use crate::{ - builder::{CycleBuilder, HalfEdgeBuilder}, - partial::PartialCycle, - services::Services, - }; - - #[test] - fn global_edge_equality() { - let mut services = Services::new(); - - let surface = services.objects.surfaces.xy_plane(); - - let a = [0., 0.]; - let b = [1., 0.]; - let c = [0., 1.]; - - let a_to_b = { - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = - cycle.update_as_polygon_from_points([a, b, c]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - let b_to_a = { - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = - cycle.update_as_polygon_from_points([b, a, c]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - - assert_eq!(a_to_b.global_form(), b_to_a.global_form()); + pub fn new() -> Self { + Self::default() } } diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index 319425dbb..6c19fa57f 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -80,7 +80,7 @@ mod stores; pub use self::{ full::{ cycle::{Cycle, HalfEdgesOfCycle}, - edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder}, + edge::{GlobalEdge, HalfEdge}, face::{Face, FaceSet, Handedness}, shell::Shell, sketch::Sketch, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 592f4fd9a..4b49eb84d 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -86,11 +86,7 @@ impl Default for PartialHalfEdge { fn default() -> Self { let curve = None; let start_vertex = Partial::default(); - let end_vertex = Partial::default(); - - let global_form = Partial::from_partial(PartialGlobalEdge { - vertices: [start_vertex.clone(), end_vertex], - }); + let global_form = Partial::default(); Self { curve, @@ -103,28 +99,16 @@ impl Default for PartialHalfEdge { /// A partial [`GlobalEdge`] #[derive(Clone, Debug, Default)] -pub struct PartialGlobalEdge { - /// The vertices that bound the edge on the curve - pub vertices: [Partial; 2], -} +pub struct PartialGlobalEdge {} impl PartialObject for PartialGlobalEdge { type Full = GlobalEdge; - fn from_full( - global_edge: &Self::Full, - cache: &mut FullToPartialCache, - ) -> Self { - Self { - vertices: global_edge - .vertices() - .access_in_normalized_order() - .map(|vertex| Partial::from_full(vertex, cache)), - } + fn from_full(_: &Self::Full, _: &mut FullToPartialCache) -> Self { + Self {} } - fn build(self, objects: &mut Service) -> Self::Full { - let vertices = self.vertices.map(|vertex| vertex.build(objects)); - GlobalEdge::new(vertices) + fn build(self, _: &mut Service) -> Self::Full { + GlobalEdge::new() } } diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 8aba4e614..bacc0fd61 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,9 +1,6 @@ use fj_math::{Point, Scalar}; -use crate::{ - objects::{GlobalEdge, HalfEdge, Vertex}, - storage::Handle, -}; +use crate::objects::{GlobalEdge, HalfEdge}; use super::{Validate, ValidationConfig, ValidationError}; @@ -13,7 +10,6 @@ impl Validate for HalfEdge { config: &ValidationConfig, errors: &mut Vec, ) { - HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); } } @@ -30,24 +26,6 @@ impl Validate for GlobalEdge { /// [`HalfEdge`] validation failed #[derive(Clone, Debug, thiserror::Error)] pub enum HalfEdgeValidationError { - /// [`HalfEdge`]'s [`Vertex`] objects do not match - #[error( - "`HalfEdge` vertices don't match vertices of `HalfEdge`'s global form\n\ - - Start vertex: {vertex_from_half_edge:#?}\n\ - - Vertices from `GlobalEdge`: {vertices_from_global_form:#?}\n\ - - `HalfEdge`: {half_edge:#?}" - )] - VertexMismatch { - /// The [`Vertex`] from the [`HalfEdge`]'s start vertex - vertex_from_half_edge: Handle, - - /// The [`Vertex`] instances from the [`HalfEdge`]'s global form - vertices_from_global_form: [Handle; 2], - - /// The half-edge - half_edge: HalfEdge, - }, - /// [`HalfEdge`]'s vertices are coincident #[error( "Vertices of `HalfEdge` on curve are coincident\n\ @@ -71,33 +49,6 @@ pub enum HalfEdgeValidationError { } impl HalfEdgeValidationError { - fn check_global_vertex_identity( - half_edge: &HalfEdge, - errors: &mut Vec, - ) { - let vertex_from_half_edge = half_edge.start_vertex().clone(); - let vertices_from_global_form = half_edge - .global_form() - .vertices() - .access_in_normalized_order(); - - let matching_vertex = - vertices_from_global_form.iter().find(|global_vertex| { - global_vertex.id() == vertex_from_half_edge.id() - }); - - if matching_vertex.is_none() { - errors.push( - Self::VertexMismatch { - vertex_from_half_edge, - vertices_from_global_form, - half_edge: half_edge.clone(), - } - .into(), - ); - } - } - fn check_vertex_coincidence( half_edge: &HalfEdge, config: &ValidationConfig, @@ -127,65 +78,11 @@ mod tests { use crate::{ builder::{CycleBuilder, HalfEdgeBuilder}, objects::HalfEdge, - partial::{Partial, PartialCycle}, + partial::PartialCycle, services::Services, validate::{HalfEdgeValidationError, Validate, ValidationError}, }; - #[test] - fn vertex_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let surface = services.objects.surfaces.xy_plane(); - - let mut cycle = PartialCycle::default(); - - let [mut half_edge, next_half_edge, _] = cycle - .update_as_polygon_from_points([[0., 0.], [1., 0.], [1., 1.]]); - half_edge.write().infer_vertex_positions_if_necessary( - &surface.geometry(), - next_half_edge.read().start_vertex.clone(), - ); - - half_edge.build(&mut services.objects) - }; - let invalid = { - let global_form = { - let mut global_edge = - Partial::from(valid.global_form().clone()); - global_edge.write().vertices = valid - .global_form() - .vertices() - .access_in_normalized_order() - // Creating equal but not identical vertices here. - .map(|vertex| { - Partial::from_partial( - Partial::from(vertex).read().clone(), - ) - }); - global_edge.build(&mut services.objects) - }; - - HalfEdge::new( - valid.curve(), - valid.boundary(), - valid.start_vertex().clone(), - global_form, - ) - }; - - valid.validate_and_return_first_error()?; - assert!(matches!( - invalid.validate_and_return_first_error(), - Err(ValidationError::HalfEdge( - HalfEdgeValidationError::VertexMismatch { .. } - )) - )); - - Ok(()) - } - #[test] fn half_edge_vertices_are_coincident() -> anyhow::Result<()> { let mut services = Services::new();