diff --git a/crates/fj-kernel/src/algorithms/approx/cycle.rs b/crates/fj-kernel/src/algorithms/approx/cycle.rs index 9d398f2bc..b819de3e3 100644 --- a/crates/fj-kernel/src/algorithms/approx/cycle.rs +++ b/crates/fj-kernel/src/algorithms/approx/cycle.rs @@ -2,6 +2,8 @@ //! //! See [`CycleApprox`]. +use std::ops::Deref; + use fj_math::Segment; use crate::objects::Cycle; @@ -23,7 +25,10 @@ impl Approx for &Cycle { let half_edges = self .half_edges() - .map(|half_edge| half_edge.approx_with_cache(tolerance, cache)) + .map(|half_edge| { + (half_edge, self.surface().deref()) + .approx_with_cache(tolerance, cache) + }) .collect(); CycleApprox { half_edges } diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 2d74e66d5..cefa0cee0 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -5,9 +5,10 @@ //! approximations are usually used to build cycle approximations, and this way, //! the caller doesn't have to call with duplicate vertices. -use std::ops::Deref; - -use crate::{objects::HalfEdge, storage::Handle}; +use crate::{ + objects::{HalfEdge, Surface}, + storage::Handle, +}; use super::{ curve::{CurveApprox, CurveCache}, @@ -15,7 +16,7 @@ use super::{ Approx, ApproxPoint, Tolerance, }; -impl Approx for &Handle { +impl Approx for (&Handle, &Surface) { type Approximation = HalfEdgeApprox; type Cache = CurveCache; @@ -24,15 +25,17 @@ impl Approx for &Handle { tolerance: impl Into, cache: &mut Self::Cache, ) -> Self::Approximation { - let boundary = self.boundary(); + let (half_edge, surface) = self; + + let boundary = half_edge.boundary(); let range = RangeOnPath { boundary }; let first = ApproxPoint::new( - self.start_vertex().position(), - self.start_vertex().global_form().position(), + half_edge.start_vertex().position(), + half_edge.start_vertex().global_form().position(), ) - .with_source((self.clone(), self.boundary()[0])); - let curve_approx = (self.curve(), self.surface().deref(), range) + .with_source((half_edge.clone(), half_edge.boundary()[0])); + let curve_approx = (half_edge.curve(), surface, range) .approx_with_cache(tolerance, cache); HalfEdgeApprox { diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index d7ca55ac7..bec4fa99f 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -76,7 +76,7 @@ mod tests { use crate::{ builder::{CurveBuilder, HalfEdgeBuilder}, - partial::{Partial, PartialCurve, PartialHalfEdge, PartialObject}, + partial::{PartialCurve, PartialHalfEdge, PartialObject}, services::Services, }; @@ -86,16 +86,14 @@ mod tests { fn compute_edge_in_front_of_curve_origin() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface, - [[1., -1.], [1., 1.]], - ); + half_edge.update_as_line_segment_from_points([[1., -1.], [1., 1.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -114,16 +112,15 @@ mod tests { fn compute_edge_behind_curve_origin() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface, - [[-1., -1.], [-1., 1.]], - ); + half_edge + .update_as_line_segment_from_points([[-1., -1.], [-1., 1.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -142,16 +139,15 @@ mod tests { fn compute_edge_parallel_to_curve() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface, - [[-1., -1.], [1., -1.]], - ); + half_edge + .update_as_line_segment_from_points([[-1., -1.], [1., -1.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -165,16 +161,14 @@ mod tests { fn compute_edge_on_curve() { let mut services = Services::new(); - let surface = Partial::from(services.objects.surfaces.xy_plane()); + let surface = services.objects.surfaces.xy_plane(); let mut curve = PartialCurve::default(); curve.update_as_u_axis(); let curve = curve.build(&mut services.objects); let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface, - [[-1., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[-1., 0.], [1., 0.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/algorithms/reverse/cycle.rs b/crates/fj-kernel/src/algorithms/reverse/cycle.rs index 23f517430..fd08da75a 100644 --- a/crates/fj-kernel/src/algorithms/reverse/cycle.rs +++ b/crates/fj-kernel/src/algorithms/reverse/cycle.rs @@ -17,6 +17,6 @@ impl Reverse for Handle { edges.reverse(); - Cycle::new(edges).insert(objects) + Cycle::new(self.surface().clone(), edges).insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index 721f77dc8..1a9023356 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -19,7 +19,6 @@ impl Reverse for Handle { }; HalfEdge::new( - self.surface().clone(), self.curve().clone(), vertices, self.global_form().clone(), diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 488aeb75e..70a4c1cf1 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -1,12 +1,10 @@ -use std::ops::Deref; - use fj_interop::{ext::ArrayExt, mesh::Color}; use fj_math::{Point, Scalar, Vector}; use crate::{ builder::{CycleBuilder, HalfEdgeBuilder}, insert::Insert, - objects::{Face, HalfEdge, Objects}, + objects::{Face, HalfEdge, Objects, Surface}, partial::{Partial, PartialFace, PartialObject}, services::Service, storage::Handle, @@ -14,7 +12,7 @@ use crate::{ use super::{Sweep, SweepCache}; -impl Sweep for (Handle, Color) { +impl Sweep for (Handle, &Surface, Color) { type Swept = (Handle, Handle); fn sweep_with_cache( @@ -23,7 +21,7 @@ impl Sweep for (Handle, Color) { cache: &mut SweepCache, objects: &mut Service, ) -> Self::Swept { - let (edge, color) = self; + let (edge, surface, color) = self; let path = path.into(); // The result of sweeping an edge is a face. Let's create that. @@ -36,7 +34,7 @@ impl Sweep for (Handle, Color) { // be created by sweeping a curve, so let's sweep the curve of the edge // we're sweeping. face.exterior.write().surface = Partial::from( - (edge.curve().clone(), edge.surface().deref()) + (edge.curve().clone(), surface) .sweep_with_cache(path, cache, objects), ); @@ -150,6 +148,8 @@ impl Sweep for (Handle, Color) { #[cfg(test)] mod tests { + use std::ops::Deref; + use fj_interop::{ext::ArrayExt, mesh::Color}; use fj_math::Point; use pretty_assertions::assert_eq; @@ -168,38 +168,33 @@ mod tests { fn sweep() { let mut services = Services::new(); + let surface = services.objects.surfaces.xy_plane(); + let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) .insert(&mut services.objects) }; - let (face, _) = (half_edge, Color::default()) + let (face, _) = (half_edge, surface.deref(), Color::default()) .sweep([0., 0., 1.], &mut services.objects); let expected_face = { - let surface = Partial::from(services.objects.surfaces.xz_plane()); + let surface = services.objects.surfaces.xz_plane(); let bottom = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - surface.clone(), - [[0., 0.], [1., 0.]], - ); + half_edge + .update_as_line_segment_from_points([[0., 0.], [1., 0.]]); half_edge }; let side_up = { - let mut side_up = PartialHalfEdge { - surface: surface.clone(), - ..Default::default() - }; + let mut side_up = PartialHalfEdge::default(); { let [back, front] = side_up @@ -219,10 +214,7 @@ mod tests { side_up }; let top = { - let mut top = PartialHalfEdge { - surface: surface.clone(), - ..Default::default() - }; + let mut top = PartialHalfEdge::default(); { let [(back, back_surface), (front, front_surface)] = @@ -238,6 +230,7 @@ mod tests { top.infer_global_form(); top.update_as_line_segment(); + top.infer_vertex_positions_if_necessary(&surface.geometry()); Partial::from( top.build(&mut services.objects) @@ -247,10 +240,7 @@ mod tests { .clone() }; let side_down = { - let mut side_down = PartialHalfEdge { - surface, - ..Default::default() - }; + let mut side_down = PartialHalfEdge::default(); let [(back, back_surface), (front, front_surface)] = side_down.vertices.each_mut_ext(); @@ -263,6 +253,8 @@ mod tests { side_down.infer_global_form(); side_down.update_as_line_segment(); + side_down + .infer_vertex_positions_if_necessary(&surface.geometry()); Partial::from( side_down @@ -273,7 +265,10 @@ mod tests { .clone() }; - let mut cycle = PartialCycle::default(); + let mut cycle = PartialCycle { + surface: Partial::from(surface), + ..Default::default() + }; cycle.half_edges.extend( [bottom, side_up, top, side_down].map(Partial::from_partial), ); diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index db79e37f2..34a7cfb07 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -1,3 +1,5 @@ +use std::ops::Deref; + use fj_interop::ext::ArrayExt; use fj_math::{Scalar, Vector}; @@ -70,8 +72,9 @@ impl Sweep for Handle { let mut original_edges = Vec::new(); let mut top_edges = Vec::new(); for half_edge in cycle.half_edges().cloned() { - let (face, top_edge) = (half_edge.clone(), self.color()) - .sweep_with_cache(path, cache, objects); + let (face, top_edge) = + (half_edge.clone(), self.surface().deref(), self.color()) + .sweep_with_cache(path, cache, objects); faces.push(face); @@ -128,6 +131,8 @@ impl Sweep for Handle { #[cfg(test)] mod tests { + use std::ops::Deref; + use fj_interop::{ext::SliceExt, mesh::Color}; use crate::{ @@ -182,7 +187,7 @@ mod tests { .reverse(&mut services.objects); let mut top = PartialFace::default(); top.exterior.write().surface = - Partial::from(surface.translate(UP, &mut services.objects)); + Partial::from(surface.clone().translate(UP, &mut services.objects)); top.exterior.write().update_as_polygon_from_points(TRIANGLE); let top = top .build(&mut services.objects) @@ -195,17 +200,16 @@ mod tests { let side_faces = triangle.array_windows_ext().map(|&[a, b]| { let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [a, b], - ); + half_edge.update_as_line_segment_from_points([a, b]); + half_edge + .infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) .insert(&mut services.objects) }; - let (face, _) = - (half_edge, Color::default()).sweep(UP, &mut services.objects); + let (face, _) = (half_edge, surface.deref(), Color::default()) + .sweep(UP, &mut services.objects); face }); @@ -250,7 +254,7 @@ mod tests { .insert(&mut services.objects) .reverse(&mut services.objects); let mut top = PartialFace::default(); - top.exterior.write().surface = Partial::from(surface); + top.exterior.write().surface = Partial::from(surface.clone()); top.exterior.write().update_as_polygon_from_points(TRIANGLE); let top = top .build(&mut services.objects) @@ -263,17 +267,16 @@ mod tests { let side_faces = triangle.array_windows_ext().map(|&[a, b]| { let half_edge = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [a, b], - ); + half_edge.update_as_line_segment_from_points([a, b]); + half_edge + .infer_vertex_positions_if_necessary(&surface.geometry()); half_edge .build(&mut services.objects) .insert(&mut services.objects) .reverse(&mut services.objects) }; - let (face, _) = (half_edge, Color::default()) + let (face, _) = (half_edge, surface.deref(), Color::default()) .sweep(DOWN, &mut services.objects); face }); diff --git a/crates/fj-kernel/src/algorithms/transform/cycle.rs b/crates/fj-kernel/src/algorithms/transform/cycle.rs index 5fbc6eb32..e56793752 100644 --- a/crates/fj-kernel/src/algorithms/transform/cycle.rs +++ b/crates/fj-kernel/src/algorithms/transform/cycle.rs @@ -14,12 +14,16 @@ impl TransformObject for Cycle { objects: &mut Service, cache: &mut TransformCache, ) -> Self { + let surface = self + .surface() + .clone() + .transform_with_cache(transform, objects, cache); let half_edges = self.half_edges().map(|half_edge| { half_edge .clone() .transform_with_cache(transform, objects, cache) }); - Self::new(half_edges) + Self::new(surface, half_edges) } } diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 796769f5e..4c5b7b242 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -15,10 +15,6 @@ impl TransformObject for HalfEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let surface = self - .surface() - .clone() - .transform_with_cache(transform, objects, cache); let curve = self .curve() .clone() @@ -36,7 +32,7 @@ impl TransformObject for HalfEdge { .clone() .transform_with_cache(transform, objects, cache); - Self::new(surface, curve, boundary, global_form) + Self::new(curve, boundary, global_form) } } diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 3e7202905..3673b0ee8 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -149,7 +149,6 @@ impl CycleBuilder for PartialCycle { let [_, vertex] = &mut new_half_edge.vertices; vertex.1 = shared_surface_vertex; - new_half_edge.surface = self.surface.clone(); new_half_edge.infer_global_form(); } @@ -247,7 +246,8 @@ impl CycleBuilder for PartialCycle { let mut this = half_edges.pop_front().expect( "Pushed correct number of half-edges; should be able to pop", ); - this.write().update_from_other_edge(&other); + this.write() + .update_from_other_edge(&other, &self.surface.read().geometry); this }) } @@ -261,7 +261,8 @@ impl CycleBuilder for PartialCycle { { edges.map(|other| { let mut this = self.add_half_edge(); - this.write().update_from_other_edge(&other); + this.write() + .update_from_other_edge(&other, &self.surface.read().geometry); this }) } diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 78f85e921..c4f5b7402 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -2,8 +2,11 @@ use fj_interop::ext::ArrayExt; use fj_math::{Point, Scalar}; use crate::{ - geometry::path::{GlobalPath, SurfacePath}, - objects::{GlobalEdge, HalfEdge, Surface}, + geometry::{ + path::{GlobalPath, SurfacePath}, + surface::SurfaceGeometry, + }, + objects::{GlobalEdge, HalfEdge}, partial::{MaybeSurfacePath, Partial, PartialGlobalEdge, PartialHalfEdge}, }; @@ -25,7 +28,6 @@ pub trait HalfEdgeBuilder { /// Update partial half-edge to be a line segment, from the given points fn update_as_line_segment_from_points( &mut self, - surface: impl Into>, points: [impl Into>; 2], ); @@ -38,6 +40,12 @@ pub trait HalfEdgeBuilder { /// it. fn infer_global_form(&mut self) -> Partial; + /// Infer the vertex positions (surface and global), if not already set + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ); + /// Update this edge from another /// /// Infers as much information about this edge from the other, under the @@ -47,7 +55,11 @@ pub trait HalfEdgeBuilder { /// under various circumstances. As long as you're only dealing with lines /// and planes, you should be fine. Otherwise, please read the code of this /// method carefully, to make sure you don't run into trouble. - fn update_from_other_edge(&mut self, other: &Partial); + fn update_from_other_edge( + &mut self, + other: &Partial, + surface: &Option, + ); } impl HalfEdgeBuilder for PartialHalfEdge { @@ -111,11 +123,8 @@ impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_line_segment_from_points( &mut self, - surface: impl Into>, points: [impl Into>; 2], ) { - self.surface = surface.into(); - for (vertex, point) in self.vertices.each_mut_ext().zip_ext(points) { let mut surface_form = vertex.1.write(); surface_form.position = Some(point.into()); @@ -166,14 +175,62 @@ impl HalfEdgeBuilder for PartialHalfEdge { self.global_form.clone() } - fn update_from_other_edge(&mut self, other: &Partial) { + fn infer_vertex_positions_if_necessary( + &mut self, + surface: &SurfaceGeometry, + ) { + let path = self + .curve + .read() + .path + .expect("Can't infer vertex positions without curve"); + let MaybeSurfacePath::Defined(path) = path else { + panic!("Can't infer vertex positions with undefined path"); + }; + + for vertex in &mut self.vertices { + let position_curve = vertex + .0 + .expect("Can't infer surface position without curve position"); + + let position_surface = vertex.1.read().position; + + // Infer surface position, if not available. + let position_surface = match position_surface { + Some(position_surface) => position_surface, + None => { + let position_surface = + path.point_from_path_coords(position_curve); + + vertex.1.write().position = Some(position_surface); + + position_surface + } + }; + + // Infer global position, if not available. + let position_global = vertex.1.read().global_form.read().position; + if position_global.is_none() { + let position_global = + surface.point_from_surface_coords(position_surface); + vertex.1.write().global_form.write().position = + Some(position_global); + } + } + } + + fn update_from_other_edge( + &mut self, + other: &Partial, + surface: &Option, + ) { let global_curve = other.read().curve.read().global_form.clone(); self.curve.write().global_form = global_curve.clone(); self.global_form.write().curve = global_curve; self.curve.write().path = other.read().curve.read().path.as_ref().and_then(|path| { - match other.read().surface.read().geometry { + match surface { Some(surface) => { // We have information about the other edge's surface // available. We need to use that to interpret what the diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index 66e56a200..5fdfb4a4a 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -12,6 +12,7 @@ use crate::{ /// A cycle of connected half-edges #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Cycle { + surface: Handle, half_edges: Vec>, } @@ -21,7 +22,10 @@ impl Cycle { /// # Panics /// /// Panics, if `half_edges` does not yield at least one half-edge. - pub fn new(half_edges: impl IntoIterator>) -> Self { + pub fn new( + surface: Handle, + half_edges: impl IntoIterator>, + ) -> Self { let half_edges = half_edges.into_iter().collect::>(); // This is not a validation check, and thus not part of the validation @@ -33,18 +37,15 @@ impl Cycle { "Cycle must contain at least one half-edge" ); - Self { half_edges } + Self { + surface, + half_edges, + } } /// Access the surface that the cycle is in pub fn surface(&self) -> &Handle { - if let Some(half_edge) = self.half_edges.first() { - return half_edge.surface(); - } - - unreachable!( - "Cycle has no half-edges, which the constructor should prevent." - ) + &self.surface } /// Access the half-edges that make up the cycle diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index f7a77b378..5288eec25 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -4,14 +4,13 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{Curve, GlobalCurve, GlobalVertex, Surface, SurfaceVertex}, + objects::{Curve, GlobalCurve, GlobalVertex, SurfaceVertex}, storage::{Handle, HandleWrapper}, }; /// A half-edge #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - surface: Handle, curve: Handle, boundary: [(Point<1>, Handle); 2], global_form: Handle, @@ -20,24 +19,17 @@ pub struct HalfEdge { impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( - surface: Handle, curve: Handle, boundary: [(Point<1>, Handle); 2], global_form: Handle, ) -> Self { Self { - surface, curve, boundary, global_form, } } - /// Access the surface that the half-edge is defined in - pub fn surface(&self) -> &Handle { - &self.surface - } - /// Access the curve that defines the half-edge's geometry pub fn curve(&self) -> &Handle { &self.curve @@ -173,14 +165,15 @@ mod tests { let a_to_b = { let mut half_edge = PartialHalfEdge::default(); - half_edge - .update_as_line_segment_from_points(surface.clone(), [a, b]); + half_edge.update_as_line_segment_from_points([a, b]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; let b_to_a = { let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points(surface, [b, a]); + half_edge.update_as_line_segment_from_points([b, a]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index 3826d4cb2..4377536ae 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -46,7 +46,7 @@ impl PartialObject for PartialCurve { /// /// Can be a fully defined [`SurfacePath`], or just the type of path might be /// known. -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] pub enum MaybeSurfacePath { /// The surface path is fully defined Defined(SurfacePath), diff --git a/crates/fj-kernel/src/partial/objects/cycle.rs b/crates/fj-kernel/src/partial/objects/cycle.rs index e1d109555..386fce9e1 100644 --- a/crates/fj-kernel/src/partial/objects/cycle.rs +++ b/crates/fj-kernel/src/partial/objects/cycle.rs @@ -1,4 +1,5 @@ use crate::{ + builder::HalfEdgeBuilder, objects::{Cycle, HalfEdge, Objects, Surface}, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -29,11 +30,15 @@ impl PartialObject for PartialCycle { } fn build(self, objects: &mut Service) -> Self::Full { - let half_edges = self - .half_edges - .into_iter() - .map(|half_edge| half_edge.build(objects)); + let surface = self.surface.build(objects); + let surface_geometry = surface.geometry(); + let half_edges = self.half_edges.into_iter().map(|mut half_edge| { + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface_geometry); + half_edge.build(objects) + }); - Cycle::new(half_edges) + Cycle::new(surface, half_edges) } } diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index a0057c85a..db7ea206b 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -6,7 +6,7 @@ use fj_math::Point; use crate::{ objects::{ Curve, GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Objects, - Surface, SurfaceVertex, + SurfaceVertex, }, partial::{FullToPartialCache, Partial, PartialObject}, services::Service, @@ -15,9 +15,6 @@ use crate::{ /// A partial [`HalfEdge`] #[derive(Clone, Debug)] pub struct PartialHalfEdge { - /// The surface that the half-edge is defined in - pub surface: Partial, - /// The curve that the half-edge is defined in pub curve: Partial, @@ -36,7 +33,6 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - surface: Partial::from_full(half_edge.surface().clone(), cache), curve: Partial::from_full(half_edge.curve().clone(), cache), vertices: half_edge .boundary() @@ -55,53 +51,23 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let surface = self.surface.build(objects); let curve = self.curve.build(objects); - let vertices = self.vertices.map(|mut vertex| { - let position_surface = vertex.1.read().position; - - // Infer surface position, if not available. - let position_surface = match position_surface { - Some(position_surface) => position_surface, - None => { - let position_curve = vertex.0.expect( - "Can't infer surface position without curve position", - ); - let position_surface = - curve.path().point_from_path_coords(position_curve); - - vertex.1.write().position = Some(position_surface); - - position_surface - } - }; - - // Infer global position, if not available. - let position_global = vertex.1.read().global_form.read().position; - if position_global.is_none() { - let position_global = surface - .geometry() - .point_from_surface_coords(position_surface); - vertex.1.write().global_form.write().position = - Some(position_global); - } - - let position = - vertex.0.expect("Can't build `Vertex` without position"); + let vertices = self.vertices.map(|vertex| { + let position_curve = vertex + .0 + .expect("Can't build `HalfEdge` without boundary positions"); let surface_form = vertex.1.build(objects); - (position, surface_form) + (position_curve, surface_form) }); let global_form = self.global_form.build(objects); - HalfEdge::new(surface, curve, vertices, global_form) + HalfEdge::new(curve, vertices, global_form) } } impl Default for PartialHalfEdge { fn default() -> Self { - let surface = Partial::new(); - let curve = Partial::::new(); let vertices = array::from_fn(|_| { let surface_form = Partial::default(); @@ -123,7 +89,6 @@ impl Default for PartialHalfEdge { }); Self { - surface, curve, vertices, global_form, diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index c903e2ed0..264c4acf6 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -17,6 +17,7 @@ impl Validate for Cycle { ) { CycleValidationError::check_half_edge_connections(self, errors); CycleValidationError::check_half_edge_boundaries(self, config, errors); + CycleValidationError::check_vertex_positions(self, config, errors); // We don't need to check that all half-edges are defined in the same // surface. We already check that they are connected by identical @@ -70,6 +71,33 @@ pub enum CycleValidationError { /// The half-edge half_edge: Handle, }, + + /// Mismatch between [`SurfaceVertex`] and `GlobalVertex` positions + #[error( + "`SurfaceVertex` position doesn't match position of its global form\n\ + - Surface position: {surface_position:?}\n\ + - Surface position converted to global position: \ + {surface_position_as_global:?}\n\ + - Global position: {global_position:?}\n\ + - Distance between the positions: {distance}\n\ + - `SurfaceVertex`: {surface_vertex:#?}" + )] + VertexSurfacePositionMismatch { + /// The position of the surface vertex + surface_position: Point<2>, + + /// The surface position converted into a global position + surface_position_as_global: Point<3>, + + /// The position of the global vertex + global_position: Point<3>, + + /// The distance between the positions + distance: Scalar, + + /// The surface vertex + surface_vertex: Handle, + }, } impl CycleValidationError { @@ -130,15 +158,49 @@ impl CycleValidationError { } } } + + fn check_vertex_positions( + cycle: &Cycle, + config: &ValidationConfig, + errors: &mut Vec, + ) { + for half_edge in cycle.half_edges() { + for surface_vertex in half_edge.surface_vertices() { + let surface_position_as_global = cycle + .surface() + .geometry() + .point_from_surface_coords(surface_vertex.position()); + let global_position = surface_vertex.global_form().position(); + + let distance = + surface_position_as_global.distance_to(&global_position); + + if distance > config.identical_max_distance { + errors.push( + Self::VertexSurfacePositionMismatch { + surface_position: surface_vertex.position(), + surface_position_as_global, + global_position, + distance, + surface_vertex: surface_vertex.clone(), + } + .into(), + ); + } + } + } + } } #[cfg(test)] mod tests { - use fj_math::Point; + use fj_interop::ext::ArrayExt; + use fj_math::{Point, Scalar, Vector}; use crate::{ - builder::CycleBuilder, - objects::Cycle, + builder::{CycleBuilder, HalfEdgeBuilder}, + insert::Insert, + objects::{Cycle, HalfEdge, SurfaceVertex}, partial::{Partial, PartialCycle, PartialObject}, services::Services, validate::Validate, @@ -176,7 +238,7 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(half_edges) + Cycle::new(valid.surface().clone(), half_edges) }; valid.validate_and_return_first_error()?; @@ -212,7 +274,69 @@ mod tests { .into_iter() .map(|half_edge| half_edge.build(&mut services.objects)); - Cycle::new(half_edges) + Cycle::new(valid.surface().clone(), half_edges) + }; + + valid.validate_and_return_first_error()?; + assert!(invalid.validate_and_return_first_error().is_err()); + + Ok(()) + } + + #[test] + fn surface_vertex_position_mismatch() -> anyhow::Result<()> { + let mut services = Services::new(); + + let valid = { + let surface = services.objects.surfaces.xy_plane(); + + let mut cycle = PartialCycle { + surface: Partial::from(surface.clone()), + ..Default::default() + }; + + let mut half_edge = cycle.add_half_edge(); + half_edge.write().update_as_circle_from_radius(1.); + half_edge + .write() + .infer_vertex_positions_if_necessary(&surface.geometry()); + + cycle.build(&mut services.objects) + }; + let invalid = { + let half_edge = { + let half_edge = valid.half_edges().next().unwrap(); + + let boundary = half_edge + .boundary() + .map(|point| point + Vector::from([Scalar::PI / 2.])); + + let mut surface_vertices = + half_edge.surface_vertices().map(Clone::clone); + + let mut invalid = None; + for surface_vertex in surface_vertices.each_mut_ext() { + let invalid = invalid.get_or_insert_with(|| { + SurfaceVertex::new( + [0., 1.], + surface_vertex.global_form().clone(), + ) + .insert(&mut services.objects) + }); + *surface_vertex = invalid.clone(); + } + + let boundary = boundary.zip_ext(surface_vertices); + + HalfEdge::new( + half_edge.curve().clone(), + boundary, + half_edge.global_form().clone(), + ) + .insert(&mut services.objects) + }; + + Cycle::new(valid.surface().clone(), [half_edge]) }; valid.validate_and_return_first_error()?; diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index d5658975f..e0f7e933c 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -1,9 +1,7 @@ use fj_math::{Point, Scalar}; use crate::{ - objects::{ - GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface, SurfaceVertex, - }, + objects::{GlobalCurve, GlobalEdge, GlobalVertex, HalfEdge, Surface}, storage::Handle, }; @@ -18,7 +16,6 @@ impl Validate for HalfEdge { HalfEdgeValidationError::check_global_curve_identity(self, errors); HalfEdgeValidationError::check_global_vertex_identity(self, errors); HalfEdgeValidationError::check_vertex_coincidence(self, config, errors); - HalfEdgeValidationError::check_vertex_positions(self, config, errors); } } @@ -107,33 +104,6 @@ pub enum HalfEdgeValidationError { /// The half-edge half_edge: HalfEdge, }, - - /// Mismatch between [`SurfaceVertex`] and [`GlobalVertex`] positions - #[error( - "`SurfaceVertex` position doesn't match position of its global form\n\ - - Surface position: {surface_position:?}\n\ - - Surface position converted to global position: \ - {surface_position_as_global:?}\n\ - - Global position: {global_position:?}\n\ - - Distance between the positions: {distance}\n\ - - `SurfaceVertex`: {surface_vertex:#?}" - )] - VertexSurfacePositionMismatch { - /// The position of the surface vertex - surface_position: Point<2>, - - /// The surface position converted into a global position - surface_position_as_global: Point<3>, - - /// The position of the global vertex - global_position: Point<3>, - - /// The distance between the positions - distance: Scalar, - - /// The surface vertex - surface_vertex: Handle, - }, } impl HalfEdgeValidationError { @@ -206,36 +176,6 @@ impl HalfEdgeValidationError { ); } } - - fn check_vertex_positions( - half_edge: &HalfEdge, - config: &ValidationConfig, - errors: &mut Vec, - ) { - for surface_vertex in half_edge.surface_vertices() { - let surface_position_as_global = half_edge - .surface() - .geometry() - .point_from_surface_coords(surface_vertex.position()); - let global_position = surface_vertex.global_form().position(); - - let distance = - surface_position_as_global.distance_to(&global_position); - - if distance > config.identical_max_distance { - errors.push( - Box::new(Self::VertexSurfacePositionMismatch { - surface_position: surface_vertex.position(), - surface_position_as_global, - global_position, - distance, - surface_vertex: surface_vertex.clone(), - }) - .into(), - ); - } - } - } } #[cfg(test)] @@ -246,7 +186,7 @@ mod tests { use crate::{ builder::HalfEdgeBuilder, insert::Insert, - objects::{GlobalCurve, HalfEdge, SurfaceVertex}, + objects::{GlobalCurve, HalfEdge}, partial::{Partial, PartialHalfEdge, PartialObject}, services::Services, validate::Validate, @@ -257,11 +197,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -277,12 +217,7 @@ mod tests { .boundary() .zip_ext(valid.surface_vertices().map(Clone::clone)); - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - vertices, - global_form, - ) + HalfEdge::new(valid.curve().clone(), vertices, global_form) }; valid.validate_and_return_first_error()?; @@ -296,11 +231,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -324,12 +259,7 @@ mod tests { .boundary() .zip_ext(valid.surface_vertices().map(Clone::clone)); - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - vertices, - global_form, - ) + HalfEdge::new(valid.curve().clone(), vertices, global_form) }; valid.validate_and_return_first_error()?; @@ -343,11 +273,11 @@ mod tests { let mut services = Services::new(); let valid = { + let surface = services.objects.surfaces.xy_plane(); + let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); + half_edge.update_as_line_segment_from_points([[0., 0.], [1., 0.]]); + half_edge.infer_vertex_positions_if_necessary(&surface.geometry()); half_edge.build(&mut services.objects) }; @@ -357,7 +287,6 @@ mod tests { }); HalfEdge::new( - valid.surface().clone(), valid.curve().clone(), vertices, valid.global_form().clone(), @@ -369,44 +298,4 @@ mod tests { Ok(()) } - - #[test] - fn surface_vertex_position_mismatch() -> anyhow::Result<()> { - let mut services = Services::new(); - - let valid = { - let mut half_edge = PartialHalfEdge::default(); - half_edge.update_as_line_segment_from_points( - services.objects.surfaces.xy_plane(), - [[0., 0.], [1., 0.]], - ); - - half_edge.build(&mut services.objects) - }; - let invalid = { - let mut surface_vertices = - valid.surface_vertices().map(Clone::clone); - - let [_, surface_vertex] = surface_vertices.each_mut_ext(); - *surface_vertex = SurfaceVertex::new( - [2., 0.], - surface_vertex.global_form().clone(), - ) - .insert(&mut services.objects); - - let boundary = valid.boundary().zip_ext(surface_vertices); - - HalfEdge::new( - valid.surface().clone(), - valid.curve().clone(), - boundary, - valid.global_form().clone(), - ) - }; - - valid.validate_and_return_first_error()?; - assert!(invalid.validate_and_return_first_error().is_err()); - - Ok(()) - } } diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index a790d391c..f9e402aeb 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -27,19 +27,19 @@ impl Shape for fj::Sketch { let face = match self.chain() { fj::Chain::Circle(circle) => { - let half_edge = { - let surface = Partial::from(surface); + let surface = Partial::from(surface); - let mut half_edge = PartialHalfEdge { - surface, - ..Default::default() - }; + let half_edge = { + let mut half_edge = PartialHalfEdge::default(); half_edge.update_as_circle_from_radius(circle.radius()); Partial::from_partial(half_edge) }; let exterior = { - let mut cycle = PartialCycle::default(); + let mut cycle = PartialCycle { + surface, + ..Default::default() + }; cycle.half_edges.push(half_edge); Partial::from_partial(cycle) };