From 66a0226251279208c541e0aba5cf30f65d28fd61 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 14:28:51 +0100 Subject: [PATCH 01/17] Derive `Copy` for `Curve` --- crates/fj-kernel/src/objects/full/curve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/objects/full/curve.rs b/crates/fj-kernel/src/objects/full/curve.rs index c291ce6ff..e2c359a2a 100644 --- a/crates/fj-kernel/src/objects/full/curve.rs +++ b/crates/fj-kernel/src/objects/full/curve.rs @@ -1,7 +1,7 @@ use crate::geometry::path::SurfacePath; /// A curve, defined in local surface coordinates -#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Curve { path: SurfacePath, } From e5ae9b1be41463a9d7356b4626d50e1e20c74cda Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 14:30:32 +0100 Subject: [PATCH 02/17] Avoid using `Handle` --- crates/fj-kernel/src/algorithms/approx/edge.rs | 2 +- crates/fj-kernel/src/algorithms/approx/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index db834a594..4b40ff268 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -61,7 +61,7 @@ impl Approx for (&Handle, &Surface) { ApproxPoint::new(point_surface, point.global_form) .with_source(( - half_edge.curve().clone(), + half_edge.curve().clone_object(), point.local_form, )) }) diff --git a/crates/fj-kernel/src/algorithms/approx/mod.rs b/crates/fj-kernel/src/algorithms/approx/mod.rs index cc8ae0208..dad7da54e 100644 --- a/crates/fj-kernel/src/algorithms/approx/mod.rs +++ b/crates/fj-kernel/src/algorithms/approx/mod.rs @@ -121,5 +121,5 @@ impl PartialOrd for ApproxPoint { /// The source of an [`ApproxPoint`] pub trait Source: Any + Debug {} -impl Source for (Handle, Point<1>) {} +impl Source for (Curve, Point<1>) {} impl Source for (Handle, Point<1>) {} From 1c6feea431cb19680b07ee4b953001ee2fe07f37 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 14:32:16 +0100 Subject: [PATCH 03/17] Avoid using `Handle` --- crates/fj-kernel/src/algorithms/sweep/curve.rs | 2 +- crates/fj-kernel/src/algorithms/sweep/edge.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/curve.rs b/crates/fj-kernel/src/algorithms/sweep/curve.rs index 34d6a706b..523bc42d4 100644 --- a/crates/fj-kernel/src/algorithms/sweep/curve.rs +++ b/crates/fj-kernel/src/algorithms/sweep/curve.rs @@ -12,7 +12,7 @@ use crate::{ use super::{Sweep, SweepCache}; -impl Sweep for (Handle, &Surface) { +impl Sweep for (Curve, &Surface) { type Swept = Handle; fn sweep_with_cache( diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 5fc50f4c5..663a766a4 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -35,7 +35,7 @@ impl Sweep for (Handle, &Surface, Color) { // we're sweeping. { let surface = Partial::from( - (edge.curve().clone(), surface) + (edge.curve().clone_object(), surface) .sweep_with_cache(path, cache, objects), ); face.surface = surface; From 25131a1c0a6c6f77b90f1cdb0781a3fd3df674be Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 14:37:19 +0100 Subject: [PATCH 04/17] Use `PartialCurve` directly in `PartialHalfEdge` --- crates/fj-kernel/src/algorithms/sweep/face.rs | 3 +- crates/fj-kernel/src/builder/edge.rs | 154 +++++++++--------- crates/fj-kernel/src/builder/face.rs | 1 - crates/fj-kernel/src/partial/objects/edge.rs | 15 +- 4 files changed, 84 insertions(+), 89 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index d55d0337a..c1b8bf7b7 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -109,8 +109,7 @@ impl Sweep for Handle { .into_iter() .zip(top_cycle.write().half_edges.iter_mut()) { - top.write().curve.write().path = - Some(bottom.curve().path().into()); + top.write().curve.path = Some(bottom.curve().path().into()); let boundary = bottom.boundary(); diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 8812b6810..533d4217e 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -76,13 +76,13 @@ pub trait HalfEdgeBuilder { impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_u_axis(&mut self) -> SurfacePath { let path = SurfacePath::u_axis(); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); path } fn update_as_v_axis(&mut self) -> SurfacePath { let path = SurfacePath::v_axis(); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); path } @@ -91,7 +91,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { radius: impl Into, ) -> SurfacePath { let path = SurfacePath::circle_from_radius(radius); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); let [a_curve, b_curve] = [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); @@ -133,7 +133,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let path = SurfacePath::circle_from_center_and_radius(arc.center, arc.radius); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); let [a_curve, b_curve] = [arc.start_angle, arc.end_angle].map(|coord| Point::from([coord])); @@ -175,12 +175,12 @@ impl HalfEdgeBuilder for PartialHalfEdge { let points = [start, end].zip_ext(points_surface); let path = SurfacePath::from_points_with_line_coords(points); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); path } else { let (path, _) = SurfacePath::line_from_points(points_surface); - self.curve.write().path = Some(path.into()); + self.curve.path = Some(path.into()); for (vertex, position) in self.vertices.each_mut_ext().zip_ext([0., 1.]) @@ -211,7 +211,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { ) { let path = self .curve - .read() .path .expect("Can't infer vertex positions without curve"); let MaybeSurfacePath::Defined(path) = path else { @@ -254,83 +253,82 @@ impl HalfEdgeBuilder for PartialHalfEdge { other: &Partial, surface: &SurfaceGeometry, ) { - self.curve.write().path = - other.read().curve.read().path.as_ref().and_then(|path| { - // We have information about the other edge's surface available. - // We need to use that to interpret what the other edge's curve - // path means for our curve path. - match surface.u { - GlobalPath::Circle(circle) => { - // The other surface is curved. We're entering some - // dodgy territory here, as only some edge cases can be - // represented using our current curve/surface - // representation. - match path { - MaybeSurfacePath::Defined(SurfacePath::Line(_)) - | MaybeSurfacePath::UndefinedLine => { - // We're dealing with a line on a rounded - // surface. - // - // Based on the current uses of this method, we - // can make some assumptions: - // - // 1. The line is parallel to the u-axis of the - // other surface. - // 2. The surface that *our* edge is in is a - // plane that is parallel to the the plane of - // the circle that defines the curvature of - // the other surface. - // - // These assumptions are necessary preconditions - // for the following code to work. But - // unfortunately, I see no way to check those - // preconditions here, as neither the other line - // nor our surface is necessarily defined yet. - // - // Handling this case anyway feels like a grave - // sin, but I don't know what else to do. If you - // tracked some extremely subtle and annoying - // bug back to this code, I apologize. - // - // I hope that I'll come up with a better curve/ - // surface representation before this becomes a - // problem. - Some(MaybeSurfacePath::UndefinedCircle { - radius: circle.radius(), - }) - } - _ => { - // The other edge is a line segment in a curved - // surface. No idea how to deal with this. - todo!( - "Can't connect edge to circle on curved \ + self.curve.path = other.read().curve.path.as_ref().and_then(|path| { + // We have information about the other edge's surface available. + // We need to use that to interpret what the other edge's curve + // path means for our curve path. + match surface.u { + GlobalPath::Circle(circle) => { + // The other surface is curved. We're entering some + // dodgy territory here, as only some edge cases can be + // represented using our current curve/surface + // representation. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // We're dealing with a line on a rounded + // surface. + // + // Based on the current uses of this method, we + // can make some assumptions: + // + // 1. The line is parallel to the u-axis of the + // other surface. + // 2. The surface that *our* edge is in is a + // plane that is parallel to the the plane of + // the circle that defines the curvature of + // the other surface. + // + // These assumptions are necessary preconditions + // for the following code to work. But + // unfortunately, I see no way to check those + // preconditions here, as neither the other line + // nor our surface is necessarily defined yet. + // + // Handling this case anyway feels like a grave + // sin, but I don't know what else to do. If you + // tracked some extremely subtle and annoying + // bug back to this code, I apologize. + // + // I hope that I'll come up with a better curve/ + // surface representation before this becomes a + // problem. + Some(MaybeSurfacePath::UndefinedCircle { + radius: circle.radius(), + }) + } + _ => { + // The other edge is a line segment in a curved + // surface. No idea how to deal with this. + todo!( + "Can't connect edge to circle on curved \ surface" - ) - } + ) } } - GlobalPath::Line(_) => { - // The other edge is defined on a plane. - match path { - MaybeSurfacePath::Defined(SurfacePath::Line(_)) - | MaybeSurfacePath::UndefinedLine => { - // The other edge is a line segment on a plane. - // That means our edge must be a line segment - // too. - Some(MaybeSurfacePath::UndefinedLine) - } - _ => { - // The other edge is a circle or arc on a plane. - // I'm actually not sure what that means for our - // edge. We might be able to represent it - // somehow, but let's leave that as an exercise - // for later. - todo!("Can't connect edge to circle on plane") - } + } + GlobalPath::Line(_) => { + // The other edge is defined on a plane. + match path { + MaybeSurfacePath::Defined(SurfacePath::Line(_)) + | MaybeSurfacePath::UndefinedLine => { + // The other edge is a line segment on a plane. + // That means our edge must be a line segment + // too. + Some(MaybeSurfacePath::UndefinedLine) + } + _ => { + // The other edge is a circle or arc on a plane. + // I'm actually not sure what that means for our + // edge. We might be able to represent it + // somehow, but let's leave that as an exercise + // for later. + todo!("Can't connect edge to circle on plane") } } } - }); + } + }); for (this, other) in self .vertices diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 277de3be5..c586a6e41 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -97,7 +97,6 @@ impl FaceBuilder for PartialFace { let mut half_edge = half_edge.write(); let mut curve = half_edge.curve.clone(); - let mut curve = curve.write(); if let Some(path) = &mut curve.path { match path { diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index ffaece70d..df233cfc7 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,10 +4,9 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{ - Curve, GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex, - }, - partial::{FullToPartialCache, Partial, PartialObject}, + insert::Insert, + objects::{GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex}, + partial::{FullToPartialCache, Partial, PartialCurve, PartialObject}, services::Service, }; @@ -15,7 +14,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct PartialHalfEdge { /// The curve that the half-edge is defined in - pub curve: Partial, + pub curve: PartialCurve, /// The vertices that bound the half-edge on the curve pub vertices: [(Option>, Partial); 2], @@ -32,7 +31,7 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - curve: Partial::from_full(half_edge.curve().clone(), cache), + curve: PartialCurve::from_full(half_edge.curve(), cache), vertices: half_edge .boundary() .zip_ext(half_edge.surface_vertices()) @@ -50,7 +49,7 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = self.curve.build(objects); + let curve = self.curve.build(objects).insert(objects); let vertices = self.vertices.map(|vertex| { let position_curve = vertex .0 @@ -67,7 +66,7 @@ impl PartialObject for PartialHalfEdge { impl Default for PartialHalfEdge { fn default() -> Self { - let curve = Partial::::new(); + let curve = PartialCurve::default(); let vertices = array::from_fn(|_| { let surface_form = Partial::default(); (None, surface_form) From 92feeb7b5ef950c4d3fd5647c2550605e6703547 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Feb 2023 14:40:10 +0100 Subject: [PATCH 05/17] Update formatting --- crates/fj-kernel/src/builder/edge.rs | 57 +++++++++++++--------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 533d4217e..0a42dc7c7 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -254,41 +254,40 @@ impl HalfEdgeBuilder for PartialHalfEdge { surface: &SurfaceGeometry, ) { self.curve.path = other.read().curve.path.as_ref().and_then(|path| { - // We have information about the other edge's surface available. - // We need to use that to interpret what the other edge's curve - // path means for our curve path. + // We have information about the other edge's surface available. We + // need to use that to interpret what the other edge's curve path + // means for our curve path. match surface.u { GlobalPath::Circle(circle) => { - // The other surface is curved. We're entering some - // dodgy territory here, as only some edge cases can be + // The other surface is curved. We're entering some dodgy + // territory here, as only some edge cases can be // represented using our current curve/surface // representation. match path { MaybeSurfacePath::Defined(SurfacePath::Line(_)) | MaybeSurfacePath::UndefinedLine => { - // We're dealing with a line on a rounded - // surface. + // We're dealing with a line on a rounded surface. // - // Based on the current uses of this method, we - // can make some assumptions: + // Based on the current uses of this method, we can + // make some assumptions: // // 1. The line is parallel to the u-axis of the // other surface. - // 2. The surface that *our* edge is in is a - // plane that is parallel to the the plane of - // the circle that defines the curvature of - // the other surface. + // 2. The surface that *our* edge is in is a plane + // that is parallel to the the plane of the + // circle that defines the curvature of the other + // surface. // - // These assumptions are necessary preconditions - // for the following code to work. But - // unfortunately, I see no way to check those - // preconditions here, as neither the other line - // nor our surface is necessarily defined yet. + // These assumptions are necessary preconditions for + // the following code to work. But unfortunately, I + // see no way to check those preconditions here, as + // neither the other line nor our surface is + // necessarily defined yet. // - // Handling this case anyway feels like a grave - // sin, but I don't know what else to do. If you - // tracked some extremely subtle and annoying - // bug back to this code, I apologize. + // Handling this case anyway feels like a grave sin, + // but I don't know what else to do. If you tracked + // some extremely subtle and annoying bug back to + // this code, I apologize. // // I hope that I'll come up with a better curve/ // surface representation before this becomes a @@ -312,17 +311,15 @@ impl HalfEdgeBuilder for PartialHalfEdge { match path { MaybeSurfacePath::Defined(SurfacePath::Line(_)) | MaybeSurfacePath::UndefinedLine => { - // The other edge is a line segment on a plane. - // That means our edge must be a line segment - // too. + // The other edge is a line segment on a plane. That + // means our edge must be a line segment too. Some(MaybeSurfacePath::UndefinedLine) } _ => { - // The other edge is a circle or arc on a plane. - // I'm actually not sure what that means for our - // edge. We might be able to represent it - // somehow, but let's leave that as an exercise - // for later. + // The other edge is a circle or arc on a plane. I'm + // actually not sure what that means for our edge. + // We might be able to represent it somehow, but + // let's leave that as an exercise for later. todo!("Can't connect edge to circle on plane") } } From 858a752ad9b550cdc934c6f278ec9bd9b9ed4c14 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:07:40 +0100 Subject: [PATCH 06/17] Relax requirements of `Approx` implementation --- crates/fj-kernel/src/algorithms/approx/edge.rs | 4 ++-- crates/fj-kernel/src/algorithms/approx/path.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 4b40ff268..337181a56 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -113,7 +113,7 @@ fn approx_edge( ) } (SurfacePath::Circle(_), GlobalPath::Line(_)) => { - (curve.path(), range) + (&curve.path(), range) .approx_with_cache(tolerance, &mut ()) .into_iter() .map(|(point_curve, point_surface)| { @@ -355,7 +355,7 @@ mod tests { let approx = (&half_edge, surface.deref()).approx(tolerance); let expected_approx = - (half_edge.curve().path(), RangeOnPath::from([[0.], [TAU]])) + (&half_edge.curve().path(), RangeOnPath::from([[0.], [TAU]])) .approx(tolerance) .into_iter() .map(|(_, point_surface)| { diff --git a/crates/fj-kernel/src/algorithms/approx/path.rs b/crates/fj-kernel/src/algorithms/approx/path.rs index 00c981152..0c7c1d579 100644 --- a/crates/fj-kernel/src/algorithms/approx/path.rs +++ b/crates/fj-kernel/src/algorithms/approx/path.rs @@ -36,7 +36,7 @@ use crate::geometry::path::{GlobalPath, SurfacePath}; use super::{Approx, Tolerance}; -impl Approx for (SurfacePath, RangeOnPath) { +impl Approx for (&SurfacePath, RangeOnPath) { type Approximation = Vec<(Point<1>, Point<2>)>; type Cache = (); @@ -49,7 +49,7 @@ impl Approx for (SurfacePath, RangeOnPath) { match path { SurfacePath::Circle(circle) => { - approx_circle(&circle, range, tolerance.into()) + approx_circle(circle, range, tolerance.into()) } SurfacePath::Line(_) => vec![], } From 5b7a76dc93d5b61387eb3b856942577db9bd0a5d Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:08:56 +0100 Subject: [PATCH 07/17] Avoid using `Curve` in approx code --- crates/fj-kernel/src/algorithms/approx/edge.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 337181a56..53d8be71b 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; use crate::{ geometry::path::{GlobalPath, SurfacePath}, - objects::{Curve, GlobalEdge, HalfEdge, Surface}, + objects::{GlobalEdge, HalfEdge, Surface}, storage::{Handle, ObjectId}, }; @@ -41,7 +41,7 @@ impl Approx for (&Handle, &Surface) { Some(approx) => approx, None => { let approx = approx_edge( - half_edge.curve(), + &half_edge.curve().path(), surface, range, tolerance, @@ -95,7 +95,7 @@ impl HalfEdgeApprox { } fn approx_edge( - curve: &Curve, + curve: &SurfacePath, surface: &Surface, range: RangeOnPath, tolerance: impl Into, @@ -106,14 +106,14 @@ fn approx_edge( // This will probably all be unified eventually, as `SurfacePath` and // `GlobalPath` grow APIs that are better suited to implementing this code // in a more abstract way. - let points = match (curve.path(), surface.geometry().u) { + let points = match (curve, surface.geometry().u) { (SurfacePath::Circle(_), GlobalPath::Circle(_)) => { todo!( "Approximating a circle on a curved surface not supported yet." ) } (SurfacePath::Circle(_), GlobalPath::Line(_)) => { - (&curve.path(), range) + (curve, range) .approx_with_cache(tolerance, &mut ()) .into_iter() .map(|(point_curve, point_surface)| { @@ -142,7 +142,7 @@ fn approx_edge( (SurfacePath::Line(line), _) => { let range_u = RangeOnPath::from(range.boundary.map(|point_curve| { - [curve.path().point_from_path_coords(point_curve).u] + [curve.point_from_path_coords(point_curve).u] })); let approx_u = (surface.geometry().u, range_u) @@ -151,7 +151,7 @@ fn approx_edge( let mut points = Vec::new(); for (u, _) in approx_u { let t = (u.t - line.origin().u) / line.direction().u; - let point_surface = curve.path().point_from_path_coords([t]); + let point_surface = curve.point_from_path_coords([t]); let point_global = surface.geometry().point_from_surface_coords(point_surface); points.push((u, point_global)); From 586e51a385e0a2533c450c8e438889a17fb71daa Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:32:56 +0100 Subject: [PATCH 08/17] Use `Curve` in `HalfEdge` directly --- crates/fj-kernel/src/algorithms/approx/edge.rs | 5 +---- crates/fj-kernel/src/algorithms/reverse/edge.rs | 8 ++------ crates/fj-kernel/src/algorithms/sweep/edge.rs | 3 +-- crates/fj-kernel/src/algorithms/transform/edge.rs | 6 ++---- crates/fj-kernel/src/objects/full/edge.rs | 8 ++++---- crates/fj-kernel/src/partial/objects/edge.rs | 6 ++++-- crates/fj-kernel/src/validate/edge.rs | 8 ++------ crates/fj-kernel/src/validate/face.rs | 2 +- 8 files changed, 17 insertions(+), 29 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 53d8be71b..6a1875455 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -60,10 +60,7 @@ impl Approx for (&Handle, &Surface) { .point_from_path_coords(point.local_form); ApproxPoint::new(point_surface, point.global_form) - .with_source(( - half_edge.curve().clone_object(), - point.local_form, - )) + .with_source((half_edge.curve(), point.local_form)) }) .collect() }; diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs index 1a9023356..c5f8bca6a 100644 --- a/crates/fj-kernel/src/algorithms/reverse/edge.rs +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -18,11 +18,7 @@ impl Reverse for Handle { [b, a] }; - HalfEdge::new( - self.curve().clone(), - vertices, - self.global_form().clone(), - ) - .insert(objects) + HalfEdge::new(self.curve(), vertices, self.global_form().clone()) + .insert(objects) } } diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index 663a766a4..a2c6ca704 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -35,8 +35,7 @@ impl Sweep for (Handle, &Surface, Color) { // we're sweeping. { let surface = Partial::from( - (edge.curve().clone_object(), surface) - .sweep_with_cache(path, cache, objects), + (edge.curve(), surface).sweep_with_cache(path, cache, objects), ); face.surface = surface; } diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 1b3be7b98..12f60995a 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -15,10 +15,8 @@ impl TransformObject for HalfEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let curve = self - .curve() - .clone() - .transform_with_cache(transform, objects, cache); + let curve = + self.curve().transform_with_cache(transform, objects, cache); let boundary = self.boundary().zip_ext(self.surface_vertices()).map( |(point, surface_vertex)| { let surface_vertex = surface_vertex diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 0f5025e13..0d263dc3c 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -44,7 +44,7 @@ use crate::{ /// #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - curve: Handle, + curve: Curve, boundary: [(Point<1>, Handle); 2], global_form: Handle, } @@ -52,7 +52,7 @@ pub struct HalfEdge { impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( - curve: Handle, + curve: Curve, boundary: [(Point<1>, Handle); 2], global_form: Handle, ) -> Self { @@ -64,8 +64,8 @@ impl HalfEdge { } /// Access the curve that defines the half-edge's geometry - pub fn curve(&self) -> &Handle { - &self.curve + pub fn curve(&self) -> Curve { + self.curve } /// Access the boundary points of the half-edge on the curve diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index df233cfc7..950d24255 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -31,7 +31,9 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - curve: PartialCurve::from_full(half_edge.curve(), cache), + curve: PartialCurve { + path: Some(half_edge.curve().path().into()), + }, vertices: half_edge .boundary() .zip_ext(half_edge.surface_vertices()) @@ -49,7 +51,7 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = self.curve.build(objects).insert(objects); + let curve = self.curve.build(objects).insert(objects).clone_object(); let vertices = self.vertices.map(|vertex| { let position_curve = vertex .0 diff --git a/crates/fj-kernel/src/validate/edge.rs b/crates/fj-kernel/src/validate/edge.rs index 811358843..9cbe21fdf 100644 --- a/crates/fj-kernel/src/validate/edge.rs +++ b/crates/fj-kernel/src/validate/edge.rs @@ -184,7 +184,7 @@ mod tests { .boundary() .zip_ext(valid.surface_vertices().map(Clone::clone)); - HalfEdge::new(valid.curve().clone(), vertices, global_form) + HalfEdge::new(valid.curve(), vertices, global_form) }; valid.validate_and_return_first_error()?; @@ -211,11 +211,7 @@ mod tests { (Point::from([0.]), surface_vertex.clone()) }); - HalfEdge::new( - valid.curve().clone(), - vertices, - valid.global_form().clone(), - ) + HalfEdge::new(valid.curve(), vertices, valid.global_form().clone()) }; valid.validate_and_return_first_error()?; diff --git a/crates/fj-kernel/src/validate/face.rs b/crates/fj-kernel/src/validate/face.rs index 6fde9dfb9..c2d0e7f41 100644 --- a/crates/fj-kernel/src/validate/face.rs +++ b/crates/fj-kernel/src/validate/face.rs @@ -291,7 +291,7 @@ mod tests { let boundary = boundary.zip_ext(surface_vertices); HalfEdge::new( - half_edge.curve().clone(), + half_edge.curve(), boundary, half_edge.global_form().clone(), ) From d1e17de3edc53beb73335cb5fb29b2fdea460313 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:36:10 +0100 Subject: [PATCH 09/17] Bypass `PartialCurve` --- crates/fj-kernel/src/algorithms/sweep/face.rs | 2 +- crates/fj-kernel/src/builder/edge.rs | 15 +++++----- crates/fj-kernel/src/builder/face.rs | 4 +-- crates/fj-kernel/src/partial/objects/edge.rs | 28 +++++++++++++------ 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index c1b8bf7b7..3c3e3504d 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -109,7 +109,7 @@ impl Sweep for Handle { .into_iter() .zip(top_cycle.write().half_edges.iter_mut()) { - top.write().curve.path = Some(bottom.curve().path().into()); + top.write().curve = Some(bottom.curve().path().into()); let boundary = bottom.boundary(); diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 0a42dc7c7..0ed9d4e35 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -76,13 +76,13 @@ pub trait HalfEdgeBuilder { impl HalfEdgeBuilder for PartialHalfEdge { fn update_as_u_axis(&mut self) -> SurfacePath { let path = SurfacePath::u_axis(); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); path } fn update_as_v_axis(&mut self) -> SurfacePath { let path = SurfacePath::v_axis(); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); path } @@ -91,7 +91,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { radius: impl Into, ) -> SurfacePath { let path = SurfacePath::circle_from_radius(radius); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); let [a_curve, b_curve] = [Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord])); @@ -133,7 +133,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { let path = SurfacePath::circle_from_center_and_radius(arc.center, arc.radius); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); let [a_curve, b_curve] = [arc.start_angle, arc.end_angle].map(|coord| Point::from([coord])); @@ -175,12 +175,12 @@ impl HalfEdgeBuilder for PartialHalfEdge { let points = [start, end].zip_ext(points_surface); let path = SurfacePath::from_points_with_line_coords(points); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); path } else { let (path, _) = SurfacePath::line_from_points(points_surface); - self.curve.path = Some(path.into()); + self.curve = Some(path.into()); for (vertex, position) in self.vertices.each_mut_ext().zip_ext([0., 1.]) @@ -211,7 +211,6 @@ impl HalfEdgeBuilder for PartialHalfEdge { ) { let path = self .curve - .path .expect("Can't infer vertex positions without curve"); let MaybeSurfacePath::Defined(path) = path else { panic!("Can't infer vertex positions with undefined path"); @@ -253,7 +252,7 @@ impl HalfEdgeBuilder for PartialHalfEdge { other: &Partial, surface: &SurfaceGeometry, ) { - self.curve.path = other.read().curve.path.as_ref().and_then(|path| { + self.curve = other.read().curve.as_ref().and_then(|path| { // We have information about the other edge's surface available. We // need to use that to interpret what the other edge's curve path // means for our curve path. diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index c586a6e41..1a6a1f520 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -96,9 +96,9 @@ impl FaceBuilder for PartialFace { for half_edge in &mut self.exterior.write().half_edges { let mut half_edge = half_edge.write(); - let mut curve = half_edge.curve.clone(); + let mut curve = half_edge.curve; - if let Some(path) = &mut curve.path { + if let Some(path) = &mut curve { match path { MaybeSurfacePath::Defined(_) => { // Path is already defined. Nothing to infer. diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 950d24255..064f3c9e2 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,9 +4,10 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - insert::Insert, - objects::{GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex}, - partial::{FullToPartialCache, Partial, PartialCurve, PartialObject}, + objects::{ + Curve, GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex, + }, + partial::{FullToPartialCache, MaybeSurfacePath, Partial, PartialObject}, services::Service, }; @@ -14,7 +15,7 @@ use crate::{ #[derive(Clone, Debug)] pub struct PartialHalfEdge { /// The curve that the half-edge is defined in - pub curve: PartialCurve, + pub curve: Option, /// The vertices that bound the half-edge on the curve pub vertices: [(Option>, Partial); 2], @@ -31,9 +32,7 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - curve: PartialCurve { - path: Some(half_edge.curve().path().into()), - }, + curve: Some(half_edge.curve().path().into()), vertices: half_edge .boundary() .zip_ext(half_edge.surface_vertices()) @@ -51,7 +50,18 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = self.curve.build(objects).insert(objects).clone_object(); + let curve = { + let path = match self.curve.expect("Need path to build curve") { + MaybeSurfacePath::Defined(path) => path, + undefined => { + panic!( + "Trying to build curve with undefined path: {undefined:?}" + ) + } + }; + + Curve::new(path) + }; let vertices = self.vertices.map(|vertex| { let position_curve = vertex .0 @@ -68,7 +78,7 @@ impl PartialObject for PartialHalfEdge { impl Default for PartialHalfEdge { fn default() -> Self { - let curve = PartialCurve::default(); + let curve = None; let vertices = array::from_fn(|_| { let surface_form = Partial::default(); (None, surface_form) From 9b4e574acc13ef12a4113e751c908f99b6813134 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 16:09:30 +0100 Subject: [PATCH 10/17] Remove reference to `PartialCurve` from comment --- crates/fj-kernel/src/partial/objects/curve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index baa43c241..ed8500b1f 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -37,7 +37,7 @@ impl PartialObject for PartialCurve { } } -/// The definition of a surface path within [`PartialCurve`] +/// The definition of a surface path within a partial object /// /// Can be a fully defined [`SurfacePath`], or just the type of path might be /// known. From 7df6d4407e326d3090f4d8e5fa3c8e7a361ea714 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:50:45 +0100 Subject: [PATCH 11/17] Remove unused code --- crates/fj-kernel/src/builder/curve.rs | 9 ----- crates/fj-kernel/src/builder/mod.rs | 2 - crates/fj-kernel/src/partial/mod.rs | 2 +- crates/fj-kernel/src/partial/objects/curve.rs | 37 +------------------ crates/fj-kernel/src/partial/traits.rs | 1 - 5 files changed, 2 insertions(+), 49 deletions(-) delete mode 100644 crates/fj-kernel/src/builder/curve.rs diff --git a/crates/fj-kernel/src/builder/curve.rs b/crates/fj-kernel/src/builder/curve.rs deleted file mode 100644 index dc35072ca..000000000 --- a/crates/fj-kernel/src/builder/curve.rs +++ /dev/null @@ -1,9 +0,0 @@ -use crate::partial::PartialCurve; - -/// Builder API for [`PartialCurve`] -pub trait CurveBuilder { - // No methods are currently defined. This trait serves as a placeholder, to - // make it clear where to add such methods, once necessary. -} - -impl CurveBuilder for PartialCurve {} diff --git a/crates/fj-kernel/src/builder/mod.rs b/crates/fj-kernel/src/builder/mod.rs index 9568fd64f..751476eb5 100644 --- a/crates/fj-kernel/src/builder/mod.rs +++ b/crates/fj-kernel/src/builder/mod.rs @@ -1,7 +1,6 @@ //! API for building objects // These are new-style builders that build on top of the partial object API. -mod curve; mod cycle; mod edge; mod face; @@ -14,7 +13,6 @@ mod vertex; use std::array; pub use self::{ - curve::CurveBuilder, cycle::CycleBuilder, edge::{GlobalEdgeBuilder, HalfEdgeBuilder}, face::FaceBuilder, diff --git a/crates/fj-kernel/src/partial/mod.rs b/crates/fj-kernel/src/partial/mod.rs index cb6469170..96b2d0a55 100644 --- a/crates/fj-kernel/src/partial/mod.rs +++ b/crates/fj-kernel/src/partial/mod.rs @@ -16,7 +16,7 @@ mod wrapper; pub use self::{ objects::{ - curve::{MaybeSurfacePath, PartialCurve}, + curve::MaybeSurfacePath, cycle::PartialCycle, edge::{PartialGlobalEdge, PartialHalfEdge}, face::PartialFace, diff --git a/crates/fj-kernel/src/partial/objects/curve.rs b/crates/fj-kernel/src/partial/objects/curve.rs index ed8500b1f..f0df764e0 100644 --- a/crates/fj-kernel/src/partial/objects/curve.rs +++ b/crates/fj-kernel/src/partial/objects/curve.rs @@ -1,41 +1,6 @@ use fj_math::Scalar; -use crate::{ - geometry::path::SurfacePath, - objects::{Curve, Objects}, - partial::{FullToPartialCache, PartialObject}, - services::Service, -}; - -/// A partial [`Curve`] -#[derive(Clone, Debug, Default)] -pub struct PartialCurve { - /// The path that defines the curve - pub path: Option, -} - -impl PartialObject for PartialCurve { - type Full = Curve; - - fn from_full(curve: &Self::Full, _: &mut FullToPartialCache) -> Self { - Self { - path: Some(curve.path().into()), - } - } - - fn build(self, _: &mut Service) -> Self::Full { - let path = match self.path.expect("Need path to build curve") { - MaybeSurfacePath::Defined(path) => path, - undefined => { - panic!( - "Trying to build curve with undefined path: {undefined:?}" - ) - } - }; - - Curve::new(path) - } -} +use crate::geometry::path::SurfacePath; /// The definition of a surface path within a partial object /// diff --git a/crates/fj-kernel/src/partial/traits.rs b/crates/fj-kernel/src/partial/traits.rs index 33da816c3..93670c1a3 100644 --- a/crates/fj-kernel/src/partial/traits.rs +++ b/crates/fj-kernel/src/partial/traits.rs @@ -33,7 +33,6 @@ macro_rules! impl_trait { } impl_trait!( - Curve, PartialCurve; Cycle, PartialCycle; Face, PartialFace; GlobalEdge, PartialGlobalEdge; From c3a3ceb024841c7bc20ba635d7a282b7ff042010 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:54:55 +0100 Subject: [PATCH 12/17] Avoid using `Curve` in approx code --- crates/fj-kernel/src/algorithms/approx/edge.rs | 2 +- crates/fj-kernel/src/algorithms/approx/mod.rs | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 6a1875455..7ed2ec6a3 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -60,7 +60,7 @@ impl Approx for (&Handle, &Surface) { .point_from_path_coords(point.local_form); ApproxPoint::new(point_surface, point.global_form) - .with_source((half_edge.curve(), point.local_form)) + .with_source((half_edge.clone(), point.local_form)) }) .collect() }; diff --git a/crates/fj-kernel/src/algorithms/approx/mod.rs b/crates/fj-kernel/src/algorithms/approx/mod.rs index dad7da54e..52018efd0 100644 --- a/crates/fj-kernel/src/algorithms/approx/mod.rs +++ b/crates/fj-kernel/src/algorithms/approx/mod.rs @@ -19,10 +19,7 @@ use std::{ use fj_math::Point; -use crate::{ - objects::{Curve, HalfEdge}, - storage::Handle, -}; +use crate::{objects::HalfEdge, storage::Handle}; pub use self::tolerance::{InvalidTolerance, Tolerance}; @@ -121,5 +118,4 @@ impl PartialOrd for ApproxPoint { /// The source of an [`ApproxPoint`] pub trait Source: Any + Debug {} -impl Source for (Curve, Point<1>) {} impl Source for (Handle, Point<1>) {} From cacbff30eb21f11214b8375c694a015bfacf2bb9 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:56:59 +0100 Subject: [PATCH 13/17] Remove unused code --- crates/fj-kernel/src/insert.rs | 3 +-- crates/fj-kernel/src/objects/object.rs | 3 +-- crates/fj-kernel/src/objects/stores.rs | 7 ++----- crates/fj-kernel/src/validate/curve.rs | 12 ------------ crates/fj-kernel/src/validate/mod.rs | 1 - 5 files changed, 4 insertions(+), 22 deletions(-) delete mode 100644 crates/fj-kernel/src/validate/curve.rs diff --git a/crates/fj-kernel/src/insert.rs b/crates/fj-kernel/src/insert.rs index 8cd1d03bc..65e1dadf2 100644 --- a/crates/fj-kernel/src/insert.rs +++ b/crates/fj-kernel/src/insert.rs @@ -4,7 +4,7 @@ use crate::{ objects::{ - Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, + Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, }, services::{Service, ServiceObjectsExt}, @@ -34,7 +34,6 @@ macro_rules! impl_insert { } impl_insert!( - Curve, curves; Cycle, cycles; Face, faces; GlobalEdge, global_edges; diff --git a/crates/fj-kernel/src/objects/object.rs b/crates/fj-kernel/src/objects/object.rs index 869403ea3..0a3162e41 100644 --- a/crates/fj-kernel/src/objects/object.rs +++ b/crates/fj-kernel/src/objects/object.rs @@ -2,7 +2,7 @@ use std::any::Any; use crate::{ objects::{ - Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, + Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Objects, Shell, Sketch, Solid, Surface, SurfaceVertex, }, storage::{Handle, ObjectId}, @@ -108,7 +108,6 @@ macro_rules! object { } object!( - Curve, "curve", curves; Cycle, "cycle", cycles; Face, "face", faces; GlobalEdge, "global edge", global_edges; diff --git a/crates/fj-kernel/src/objects/stores.rs b/crates/fj-kernel/src/objects/stores.rs index bcb2dcd88..6471fd8e8 100644 --- a/crates/fj-kernel/src/objects/stores.rs +++ b/crates/fj-kernel/src/objects/stores.rs @@ -6,16 +6,13 @@ use crate::{ }; use super::{ - Curve, Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Shell, Sketch, - Solid, Surface, SurfaceVertex, + Cycle, Face, GlobalEdge, GlobalVertex, HalfEdge, Shell, Sketch, Solid, + Surface, SurfaceVertex, }; /// The available object stores #[derive(Debug, Default)] pub struct Objects { - /// Store for [`Curve`]s - pub curves: Store, - /// Store for [`Cycle`]s pub cycles: Store, diff --git a/crates/fj-kernel/src/validate/curve.rs b/crates/fj-kernel/src/validate/curve.rs deleted file mode 100644 index 2f40a7820..000000000 --- a/crates/fj-kernel/src/validate/curve.rs +++ /dev/null @@ -1,12 +0,0 @@ -use crate::objects::Curve; - -use super::{Validate, ValidationConfig, ValidationError}; - -impl Validate for Curve { - fn validate_with_config( - &self, - _: &ValidationConfig, - _: &mut Vec, - ) { - } -} diff --git a/crates/fj-kernel/src/validate/mod.rs b/crates/fj-kernel/src/validate/mod.rs index 2a7dbc6b1..082a0b527 100644 --- a/crates/fj-kernel/src/validate/mod.rs +++ b/crates/fj-kernel/src/validate/mod.rs @@ -1,6 +1,5 @@ //! Infrastructure for validating objects -mod curve; mod cycle; mod edge; mod face; From e697c3ff6ab59243921487ee7cdde2e7f5be545b Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 15:58:28 +0100 Subject: [PATCH 14/17] Avoid using `Curve` in sweep code --- crates/fj-kernel/src/algorithms/sweep/curve.rs | 6 +++--- crates/fj-kernel/src/algorithms/sweep/edge.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/sweep/curve.rs b/crates/fj-kernel/src/algorithms/sweep/curve.rs index 523bc42d4..6eaed3c74 100644 --- a/crates/fj-kernel/src/algorithms/sweep/curve.rs +++ b/crates/fj-kernel/src/algorithms/sweep/curve.rs @@ -4,7 +4,7 @@ use crate::{ builder::SurfaceBuilder, geometry::path::{GlobalPath, SurfacePath}, insert::Insert, - objects::{Curve, Objects, Surface}, + objects::{Objects, Surface}, partial::{PartialObject, PartialSurface}, services::Service, storage::Handle, @@ -12,7 +12,7 @@ use crate::{ use super::{Sweep, SweepCache}; -impl Sweep for (Curve, &Surface) { +impl Sweep for (SurfacePath, &Surface) { type Swept = Handle; fn sweep_with_cache( @@ -47,7 +47,7 @@ impl Sweep for (Curve, &Surface) { } } - let u = match curve.path() { + let u = match curve { SurfacePath::Circle(circle) => { let center = surface .geometry() diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index a2c6ca704..db656768f 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -35,7 +35,8 @@ impl Sweep for (Handle, &Surface, Color) { // we're sweeping. { let surface = Partial::from( - (edge.curve(), surface).sweep_with_cache(path, cache, objects), + (edge.curve().path(), surface) + .sweep_with_cache(path, cache, objects), ); face.surface = surface; } From 2322ab9ddcc118ea6271d982c4155ef03d975e3e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 16:00:25 +0100 Subject: [PATCH 15/17] Remove `Curve`'s `TransformObject` implementation --- .../src/algorithms/transform/curve.rs | 23 ------------------- .../src/algorithms/transform/edge.rs | 5 ++-- .../fj-kernel/src/algorithms/transform/mod.rs | 1 - 3 files changed, 3 insertions(+), 26 deletions(-) delete mode 100644 crates/fj-kernel/src/algorithms/transform/curve.rs diff --git a/crates/fj-kernel/src/algorithms/transform/curve.rs b/crates/fj-kernel/src/algorithms/transform/curve.rs deleted file mode 100644 index b12f0c1d8..000000000 --- a/crates/fj-kernel/src/algorithms/transform/curve.rs +++ /dev/null @@ -1,23 +0,0 @@ -use fj_math::Transform; - -use crate::{ - objects::{Curve, Objects}, - services::Service, -}; - -use super::{TransformCache, TransformObject}; - -impl TransformObject for Curve { - fn transform_with_cache( - self, - _: &Transform, - _: &mut Service, - _: &mut TransformCache, - ) -> Self { - // Don't need to transform path, as that's defined in surface - // coordinates, and thus transforming `surface` takes care of it. - let path = self.path(); - - Self::new(path) - } -} diff --git a/crates/fj-kernel/src/algorithms/transform/edge.rs b/crates/fj-kernel/src/algorithms/transform/edge.rs index 12f60995a..ac0e64e6b 100644 --- a/crates/fj-kernel/src/algorithms/transform/edge.rs +++ b/crates/fj-kernel/src/algorithms/transform/edge.rs @@ -15,8 +15,9 @@ impl TransformObject for HalfEdge { objects: &mut Service, cache: &mut TransformCache, ) -> Self { - let curve = - self.curve().transform_with_cache(transform, objects, cache); + // Don't need to transform curve, as that's defined in surface + // coordinates. + let curve = self.curve(); let boundary = self.boundary().zip_ext(self.surface_vertices()).map( |(point, surface_vertex)| { let surface_vertex = surface_vertex diff --git a/crates/fj-kernel/src/algorithms/transform/mod.rs b/crates/fj-kernel/src/algorithms/transform/mod.rs index e980d5685..c56b78d22 100644 --- a/crates/fj-kernel/src/algorithms/transform/mod.rs +++ b/crates/fj-kernel/src/algorithms/transform/mod.rs @@ -1,6 +1,5 @@ //! API for transforming objects -mod curve; mod cycle; mod edge; mod face; From 34d251ce8f9c01bfe6a24583dc9edacb5ce77eaf Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 16:05:28 +0100 Subject: [PATCH 16/17] Merge `Curve` into `HalfEdge` --- .../fj-kernel/src/algorithms/approx/edge.rs | 11 ++++------ .../src/algorithms/intersect/curve_edge.rs | 2 +- .../src/algorithms/intersect/ray_edge.rs | 2 +- crates/fj-kernel/src/algorithms/sweep/edge.rs | 3 +-- crates/fj-kernel/src/algorithms/sweep/face.rs | 2 +- crates/fj-kernel/src/objects/full/curve.rs | 19 ------------------ crates/fj-kernel/src/objects/full/cycle.rs | 2 +- crates/fj-kernel/src/objects/full/edge.rs | 9 +++++---- crates/fj-kernel/src/objects/full/mod.rs | 1 - crates/fj-kernel/src/objects/mod.rs | 1 - crates/fj-kernel/src/partial/objects/edge.rs | 20 +++++++------------ crates/fj-kernel/src/validate/cycle.rs | 6 ++---- 12 files changed, 23 insertions(+), 55 deletions(-) delete mode 100644 crates/fj-kernel/src/objects/full/curve.rs diff --git a/crates/fj-kernel/src/algorithms/approx/edge.rs b/crates/fj-kernel/src/algorithms/approx/edge.rs index 7ed2ec6a3..906a059c3 100644 --- a/crates/fj-kernel/src/algorithms/approx/edge.rs +++ b/crates/fj-kernel/src/algorithms/approx/edge.rs @@ -41,7 +41,7 @@ impl Approx for (&Handle, &Surface) { Some(approx) => approx, None => { let approx = approx_edge( - &half_edge.curve().path(), + &half_edge.curve(), surface, range, tolerance, @@ -56,7 +56,6 @@ impl Approx for (&Handle, &Surface) { .map(|point| { let point_surface = half_edge .curve() - .path() .point_from_path_coords(point.local_form); ApproxPoint::new(point_surface, point.global_form) @@ -320,10 +319,8 @@ mod tests { .approx(tolerance) .into_iter() .map(|(point_local, _)| { - let point_surface = half_edge - .curve() - .path() - .point_from_path_coords(point_local); + let point_surface = + half_edge.curve().point_from_path_coords(point_local); let point_global = surface.geometry().point_from_surface_coords(point_surface); ApproxPoint::new(point_surface, point_global) @@ -352,7 +349,7 @@ mod tests { let approx = (&half_edge, surface.deref()).approx(tolerance); let expected_approx = - (&half_edge.curve().path(), RangeOnPath::from([[0.], [TAU]])) + (&half_edge.curve(), RangeOnPath::from([[0.], [TAU]])) .approx(tolerance) .into_iter() .map(|(_, point_surface)| { diff --git a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs index 337e08f13..495718be1 100644 --- a/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/curve_edge.rs @@ -35,7 +35,7 @@ impl CurveEdgeIntersection { }; let edge_as_segment = { - let edge_curve_as_line = match half_edge.curve().path() { + let edge_curve_as_line = match half_edge.curve() { SurfacePath::Line(line) => line, _ => { todo!("Curve-edge intersection only supports line segments") diff --git a/crates/fj-kernel/src/algorithms/intersect/ray_edge.rs b/crates/fj-kernel/src/algorithms/intersect/ray_edge.rs index 15cfa6be7..faf7c4c47 100644 --- a/crates/fj-kernel/src/algorithms/intersect/ray_edge.rs +++ b/crates/fj-kernel/src/algorithms/intersect/ray_edge.rs @@ -17,7 +17,7 @@ impl Intersect for (&HorizontalRayToTheRight<2>, &Handle) { fn intersect(self) -> Option { let (ray, edge) = self; - let line = match edge.curve().path() { + let line = match edge.curve() { SurfacePath::Line(line) => line, SurfacePath::Circle(_) => { todo!("Casting rays against circles is not supported yet") diff --git a/crates/fj-kernel/src/algorithms/sweep/edge.rs b/crates/fj-kernel/src/algorithms/sweep/edge.rs index db656768f..a2c6ca704 100644 --- a/crates/fj-kernel/src/algorithms/sweep/edge.rs +++ b/crates/fj-kernel/src/algorithms/sweep/edge.rs @@ -35,8 +35,7 @@ impl Sweep for (Handle, &Surface, Color) { // we're sweeping. { let surface = Partial::from( - (edge.curve().path(), surface) - .sweep_with_cache(path, cache, objects), + (edge.curve(), surface).sweep_with_cache(path, cache, objects), ); face.surface = surface; } diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index 3c3e3504d..b061ddb68 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -109,7 +109,7 @@ impl Sweep for Handle { .into_iter() .zip(top_cycle.write().half_edges.iter_mut()) { - top.write().curve = Some(bottom.curve().path().into()); + top.write().curve = Some(bottom.curve().into()); let boundary = bottom.boundary(); diff --git a/crates/fj-kernel/src/objects/full/curve.rs b/crates/fj-kernel/src/objects/full/curve.rs deleted file mode 100644 index e2c359a2a..000000000 --- a/crates/fj-kernel/src/objects/full/curve.rs +++ /dev/null @@ -1,19 +0,0 @@ -use crate::geometry::path::SurfacePath; - -/// A curve, defined in local surface coordinates -#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct Curve { - path: SurfacePath, -} - -impl Curve { - /// Construct a new instance of `Curve` - pub fn new(path: SurfacePath) -> Self { - Self { path } - } - - /// Access the path that defines the curve - pub fn path(&self) -> SurfacePath { - self.path - } -} diff --git a/crates/fj-kernel/src/objects/full/cycle.rs b/crates/fj-kernel/src/objects/full/cycle.rs index a8f429553..99a83b67a 100644 --- a/crates/fj-kernel/src/objects/full/cycle.rs +++ b/crates/fj-kernel/src/objects/full/cycle.rs @@ -55,7 +55,7 @@ impl Cycle { let [a, b] = first.boundary(); let edge_direction_positive = a < b; - let circle = match first.curve().path() { + let circle = match first.curve() { SurfacePath::Circle(circle) => circle, SurfacePath::Line(_) => unreachable!( "Invalid cycle: less than 3 edges, but not all are circles" diff --git a/crates/fj-kernel/src/objects/full/edge.rs b/crates/fj-kernel/src/objects/full/edge.rs index 0d263dc3c..1245d702a 100644 --- a/crates/fj-kernel/src/objects/full/edge.rs +++ b/crates/fj-kernel/src/objects/full/edge.rs @@ -2,7 +2,8 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{Curve, GlobalVertex, SurfaceVertex}, + geometry::path::SurfacePath, + objects::{GlobalVertex, SurfaceVertex}, storage::Handle, }; @@ -44,7 +45,7 @@ use crate::{ /// #[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct HalfEdge { - curve: Curve, + curve: SurfacePath, boundary: [(Point<1>, Handle); 2], global_form: Handle, } @@ -52,7 +53,7 @@ pub struct HalfEdge { impl HalfEdge { /// Create an instance of `HalfEdge` pub fn new( - curve: Curve, + curve: SurfacePath, boundary: [(Point<1>, Handle); 2], global_form: Handle, ) -> Self { @@ -64,7 +65,7 @@ impl HalfEdge { } /// Access the curve that defines the half-edge's geometry - pub fn curve(&self) -> Curve { + pub fn curve(&self) -> SurfacePath { self.curve } diff --git a/crates/fj-kernel/src/objects/full/mod.rs b/crates/fj-kernel/src/objects/full/mod.rs index 1c2e5d303..2261d4905 100644 --- a/crates/fj-kernel/src/objects/full/mod.rs +++ b/crates/fj-kernel/src/objects/full/mod.rs @@ -1,4 +1,3 @@ -pub mod curve; pub mod cycle; pub mod edge; pub mod face; diff --git a/crates/fj-kernel/src/objects/mod.rs b/crates/fj-kernel/src/objects/mod.rs index e957c10ab..0e7e590e2 100644 --- a/crates/fj-kernel/src/objects/mod.rs +++ b/crates/fj-kernel/src/objects/mod.rs @@ -79,7 +79,6 @@ mod stores; pub use self::{ full::{ - curve::Curve, cycle::{Cycle, HalfEdgesOfCycle}, edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder}, face::{Face, FaceSet, Handedness}, diff --git a/crates/fj-kernel/src/partial/objects/edge.rs b/crates/fj-kernel/src/partial/objects/edge.rs index 064f3c9e2..0940a2356 100644 --- a/crates/fj-kernel/src/partial/objects/edge.rs +++ b/crates/fj-kernel/src/partial/objects/edge.rs @@ -4,9 +4,7 @@ use fj_interop::ext::ArrayExt; use fj_math::Point; use crate::{ - objects::{ - Curve, GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex, - }, + objects::{GlobalEdge, GlobalVertex, HalfEdge, Objects, SurfaceVertex}, partial::{FullToPartialCache, MaybeSurfacePath, Partial, PartialObject}, services::Service, }; @@ -32,7 +30,7 @@ impl PartialObject for PartialHalfEdge { cache: &mut FullToPartialCache, ) -> Self { Self { - curve: Some(half_edge.curve().path().into()), + curve: Some(half_edge.curve().into()), vertices: half_edge .boundary() .zip_ext(half_edge.surface_vertices()) @@ -50,17 +48,13 @@ impl PartialObject for PartialHalfEdge { } fn build(self, objects: &mut Service) -> Self::Full { - let curve = { - let path = match self.curve.expect("Need path to build curve") { - MaybeSurfacePath::Defined(path) => path, - undefined => { - panic!( + let curve = match self.curve.expect("Need path to build curve") { + MaybeSurfacePath::Defined(path) => path, + undefined => { + panic!( "Trying to build curve with undefined path: {undefined:?}" ) - } - }; - - Curve::new(path) + } }; let vertices = self.vertices.map(|vertex| { let position_curve = vertex diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index d8898820d..60d6c23c8 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -101,10 +101,8 @@ impl CycleValidationError { .boundary() .zip_ext([half_edge.start_vertex(), next.start_vertex()]); for (position_on_curve, surface_vertex) in boundary_and_vertices { - let curve_position_on_surface = half_edge - .curve() - .path() - .point_from_path_coords(position_on_curve); + let curve_position_on_surface = + half_edge.curve().point_from_path_coords(position_on_curve); let surface_position_from_vertex = surface_vertex.position(); let distance = curve_position_on_surface From 1bd42dc6939ee3d890e3421f4a86b4348e0db6c4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Fri, 24 Feb 2023 16:10:41 +0100 Subject: [PATCH 17/17] Remove outdated implementation note --- crates/fj-kernel/src/geometry/path.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/crates/fj-kernel/src/geometry/path.rs b/crates/fj-kernel/src/geometry/path.rs index e8866e0f9..6ccd18ae2 100644 --- a/crates/fj-kernel/src/geometry/path.rs +++ b/crates/fj-kernel/src/geometry/path.rs @@ -1,21 +1,6 @@ //! Paths through 2D and 3D space //! //! See [`SurfacePath`] and [`GlobalPath`]. -//! -//! # Implementation Note -//! -//! This is a bit of an in-between module. It is closely associated with -//! [`Curve`] and [`Surface`]s, but paths are not really objects themselves, as -//! logically speaking, they are owned and not referenced. -//! -//! On the other hand, the types in this module don't follow the general style -//! of types in `fj-math`. -//! -//! We'll see how it shakes out. Maybe it will stay its own thing, maybe it will -//! move to `fj-math`, maybe something else entirely will happen. -//! -//! [`Curve`]: crate::objects::Curve -//! [`Surface`]: crate::objects::Surface use fj_math::{Circle, Line, Point, Scalar, Transform, Vector};