From 755f74a022f41d03256a88eb2f4d5c0ed998a79c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 21 Mar 2023 15:41:38 +0100 Subject: [PATCH 01/14] Expand `Shell` validation test Also test the validation check with valid geometry. This is very verbose, as we're lacking the right APIs to build `Shell`s. I'm working on changing that now. --- crates/fj-kernel/src/validate/shell.rs | 147 ++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 46a9e186e..1f502658a 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -191,11 +191,14 @@ impl ShellValidationError { #[cfg(test)] mod tests { + use fj_math::Point; + use crate::{ assert_contains_err, - builder::{CycleBuilder, FaceBuilder}, + builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder}, + geometry::{curve::GlobalPath, surface::SurfaceGeometry}, insert::Insert, - objects::Shell, + objects::{Cycle, Face, Shell, Surface}, services::Services, validate::{shell::ShellValidationError, Validate, ValidationError}, }; @@ -203,6 +206,145 @@ mod tests { #[test] fn coincident_not_identical() -> anyhow::Result<()> { let mut services = Services::new(); + + let valid = { + let [_a, b, c, d] = + [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]] + .map(Point::from); + + let (bottom, [ab, bc, ca]) = { + let surface = services.objects.surfaces.xy_plane(); + let (exterior, global_edges) = { + let a = [0., 0.]; + let b = [1., 0.]; + let c = [0., 1.]; + + let half_edges = [[a, b], [b, c], [c, a]].map(|points| { + HalfEdgeBuilder::line_segment(points, None) + .build(&mut services.objects) + .insert(&mut services.objects) + }); + + let cycle = Cycle::new(half_edges.clone()) + .insert(&mut services.objects); + + let global_edges = half_edges + .map(|half_edge| half_edge.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None) + .insert(&mut services.objects); + + (face, global_edges) + }; + let (front, [_, bd, da]) = { + let surface = services.objects.surfaces.xz_plane(); + let (exterior, global_edges) = { + let a = [0., 0.]; + let b = [1., 0.]; + let d = [0., 1.]; + + let half_edges = + [([a, b], Some(ab)), ([b, d], None), ([d, a], None)] + .map(|(points, global_form)| { + let mut builder = + HalfEdgeBuilder::line_segment(points, None); + + if let Some(global_form) = global_form { + builder = + builder.with_global_form(global_form); + } + + builder + .build(&mut services.objects) + .insert(&mut services.objects) + }); + + let cycle = Cycle::new(half_edges.clone()) + .insert(&mut services.objects); + + let global_edges = half_edges + .map(|half_edges| half_edges.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None) + .insert(&mut services.objects); + + (face, global_edges) + }; + let (left, [_, _, dc]) = { + let surface = services.objects.surfaces.yz_plane(); + let (exterior, global_edges) = { + let c = [1., 0.]; + let a = [0., 0.]; + let d = [0., 1.]; + + let half_edges = [ + ([c, a], Some(ca)), + ([a, d], Some(da)), + ([d, c], None), + ] + .map(|(points, global_form)| { + let mut builder = + HalfEdgeBuilder::line_segment(points, None); + + if let Some(global_form) = global_form { + builder = builder.with_global_form(global_form); + } + + builder + .build(&mut services.objects) + .insert(&mut services.objects) + }); + + let cycle = Cycle::new(half_edges.clone()) + .insert(&mut services.objects); + + let global_edges = half_edges + .map(|half_edge| half_edge.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None) + .insert(&mut services.objects); + + (face, global_edges) + }; + let back_right = { + let surface = { + let geometry = SurfaceGeometry { + u: GlobalPath::line_from_points([b, c]).0, + v: d - b, + }; + Surface::new(geometry).insert(&mut services.objects) + }; + let exterior = { + let b = [0., 0.]; + let c = [1., 0.]; + let d = [0., 1.]; + + let half_edges = [([b, c], bc), ([c, d], dc), ([d, b], bd)] + .map(|(points, global_form)| { + HalfEdgeBuilder::line_segment(points, None) + .with_global_form(global_form) + .build(&mut services.objects) + .insert(&mut services.objects) + }); + + Cycle::new(half_edges).insert(&mut services.objects) + }; + + Face::new(surface, exterior, [], None) + .insert(&mut services.objects) + }; + + Shell::new([bottom, front, left, back_right]) + }; let invalid = { let face1 = FaceBuilder::new(services.objects.surfaces.xy_plane()) .with_exterior(CycleBuilder::polygon([ @@ -227,6 +369,7 @@ mod tests { Shell::new([face1, face2]) }; + valid.validate_and_return_first_error()?; assert_contains_err!( invalid, ValidationError::Shell( From d74e93c2218edfffe2dc3b71afea7b9ca96bd9ca Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:07:22 +0100 Subject: [PATCH 02/14] Add `SurfaceBuilder` --- crates/fj-kernel/src/builder/mod.rs | 6 +++++- crates/fj-kernel/src/builder/surface.rs | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 crates/fj-kernel/src/builder/surface.rs diff --git a/crates/fj-kernel/src/builder/mod.rs b/crates/fj-kernel/src/builder/mod.rs index 5349098a1..21b498be8 100644 --- a/crates/fj-kernel/src/builder/mod.rs +++ b/crates/fj-kernel/src/builder/mod.rs @@ -4,5 +4,9 @@ mod cycle; mod edge; mod face; +mod surface; -pub use self::{cycle::CycleBuilder, edge::HalfEdgeBuilder, face::FaceBuilder}; +pub use self::{ + cycle::CycleBuilder, edge::HalfEdgeBuilder, face::FaceBuilder, + surface::SurfaceBuilder, +}; diff --git a/crates/fj-kernel/src/builder/surface.rs b/crates/fj-kernel/src/builder/surface.rs new file mode 100644 index 000000000..1b595f4aa --- /dev/null +++ b/crates/fj-kernel/src/builder/surface.rs @@ -0,0 +1,23 @@ +use fj_math::Point; + +use crate::{ + geometry::{curve::GlobalPath, surface::SurfaceGeometry}, + objects::Surface, +}; + +/// Builder API for [`Surface`] +pub struct SurfaceBuilder {} + +impl SurfaceBuilder { + /// Create a plane from the provided points + pub fn plane_from_points(points: [impl Into>; 3]) -> Surface { + let [a, b, c] = points.map(Into::into); + + let geometry = SurfaceGeometry { + u: GlobalPath::line_from_points([a, b]).0, + v: c - a, + }; + + Surface::new(geometry) + } +} From df1522faf08792baec92fe03c536728a3a9d496a Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:11:28 +0100 Subject: [PATCH 03/14] Make use of `SurfaceBuilder` in test This is a bit overly complicated, as not all the code changed here is actually improved by the `SurfaceBuilder`. However, keeping those different cases consistent makes the next refactoring steps easier. --- crates/fj-kernel/src/validate/shell.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 1f502658a..71257ee02 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -195,10 +195,9 @@ mod tests { use crate::{ assert_contains_err, - builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder}, - geometry::{curve::GlobalPath, surface::SurfaceGeometry}, + builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder, SurfaceBuilder}, insert::Insert, - objects::{Cycle, Face, Shell, Surface}, + objects::{Cycle, Face, Shell}, services::Services, validate::{shell::ShellValidationError, Validate, ValidationError}, }; @@ -208,12 +207,13 @@ mod tests { let mut services = Services::new(); let valid = { - let [_a, b, c, d] = + let [a, b, c, d] = [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]] .map(Point::from); let (bottom, [ab, bc, ca]) = { - let surface = services.objects.surfaces.xy_plane(); + let surface = SurfaceBuilder::plane_from_points([a, b, c]) + .insert(&mut services.objects); let (exterior, global_edges) = { let a = [0., 0.]; let b = [1., 0.]; @@ -240,7 +240,8 @@ mod tests { (face, global_edges) }; let (front, [_, bd, da]) = { - let surface = services.objects.surfaces.xz_plane(); + let surface = SurfaceBuilder::plane_from_points([a, b, d]) + .insert(&mut services.objects); let (exterior, global_edges) = { let a = [0., 0.]; let b = [1., 0.]; @@ -277,7 +278,8 @@ mod tests { (face, global_edges) }; let (left, [_, _, dc]) = { - let surface = services.objects.surfaces.yz_plane(); + let surface = SurfaceBuilder::plane_from_points([a, c, d]) + .insert(&mut services.objects); let (exterior, global_edges) = { let c = [1., 0.]; let a = [0., 0.]; @@ -316,13 +318,8 @@ mod tests { (face, global_edges) }; let back_right = { - let surface = { - let geometry = SurfaceGeometry { - u: GlobalPath::line_from_points([b, c]).0, - v: d - b, - }; - Surface::new(geometry).insert(&mut services.objects) - }; + let surface = SurfaceBuilder::plane_from_points([b, c, d]) + .insert(&mut services.objects); let exterior = { let b = [0., 0.]; let c = [1., 0.]; From 0d0c8addb6485119ff8ec4c3c9584311899a8bb4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:25:47 +0100 Subject: [PATCH 04/14] Refactor to make similar code more consistent --- crates/fj-kernel/src/validate/shell.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 71257ee02..296e44462 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -278,11 +278,11 @@ mod tests { (face, global_edges) }; let (left, [_, _, dc]) = { - let surface = SurfaceBuilder::plane_from_points([a, c, d]) + let surface = SurfaceBuilder::plane_from_points([c, a, d]) .insert(&mut services.objects); let (exterior, global_edges) = { - let c = [1., 0.]; - let a = [0., 0.]; + let c = [0., 0.]; + let a = [1., 0.]; let d = [0., 1.]; let half_edges = [ From 7645e74ce95c8768217b23bcd3762b408eafe8be Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:31:22 +0100 Subject: [PATCH 05/14] Add new `HalfEdgeBuilder` method --- crates/fj-kernel/src/builder/edge.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/builder/edge.rs b/crates/fj-kernel/src/builder/edge.rs index 46e106423..b4bbccf64 100644 --- a/crates/fj-kernel/src/builder/edge.rs +++ b/crates/fj-kernel/src/builder/edge.rs @@ -4,7 +4,7 @@ use fj_math::{Arc, Point, Scalar}; use crate::{ geometry::curve::Curve, insert::Insert, - objects::{GlobalEdge, HalfEdge, Objects, Vertex}, + objects::{GlobalEdge, HalfEdge, Objects, Surface, Vertex}, services::Service, storage::Handle, }; @@ -76,6 +76,17 @@ impl HalfEdgeBuilder { Self::new(curve, boundary) } + /// Create a line segment from global points + pub fn line_segment_from_global_points( + points_global: [impl Into>; 2], + surface: &Surface, + boundary: Option<[Point<1>; 2]>, + ) -> Self { + let points_surface = points_global + .map(|point| surface.geometry().project_global_point(point)); + Self::line_segment(points_surface, boundary) + } + /// Build the half-edge with a specific start vertex pub fn with_start_vertex(mut self, start_vertex: Handle) -> Self { self.start_vertex = Some(start_vertex); From bbf504830f5b9473574dd112f8b43944436171e8 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:31:51 +0100 Subject: [PATCH 06/14] Make use of new `HalfEdgeBuilder` method --- crates/fj-kernel/src/validate/shell.rs | 42 +++++++++++--------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 296e44462..2477b1207 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -215,14 +215,12 @@ mod tests { let surface = SurfaceBuilder::plane_from_points([a, b, c]) .insert(&mut services.objects); let (exterior, global_edges) = { - let a = [0., 0.]; - let b = [1., 0.]; - let c = [0., 1.]; - let half_edges = [[a, b], [b, c], [c, a]].map(|points| { - HalfEdgeBuilder::line_segment(points, None) - .build(&mut services.objects) - .insert(&mut services.objects) + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ) + .build(&mut services.objects) + .insert(&mut services.objects) }); let cycle = Cycle::new(half_edges.clone()) @@ -243,15 +241,13 @@ mod tests { let surface = SurfaceBuilder::plane_from_points([a, b, d]) .insert(&mut services.objects); let (exterior, global_edges) = { - let a = [0., 0.]; - let b = [1., 0.]; - let d = [0., 1.]; - let half_edges = [([a, b], Some(ab)), ([b, d], None), ([d, a], None)] .map(|(points, global_form)| { let mut builder = - HalfEdgeBuilder::line_segment(points, None); + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ); if let Some(global_form) = global_form { builder = @@ -281,10 +277,6 @@ mod tests { let surface = SurfaceBuilder::plane_from_points([c, a, d]) .insert(&mut services.objects); let (exterior, global_edges) = { - let c = [0., 0.]; - let a = [1., 0.]; - let d = [0., 1.]; - let half_edges = [ ([c, a], Some(ca)), ([a, d], Some(da)), @@ -292,7 +284,9 @@ mod tests { ] .map(|(points, global_form)| { let mut builder = - HalfEdgeBuilder::line_segment(points, None); + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ); if let Some(global_form) = global_form { builder = builder.with_global_form(global_form); @@ -321,16 +315,14 @@ mod tests { let surface = SurfaceBuilder::plane_from_points([b, c, d]) .insert(&mut services.objects); let exterior = { - let b = [0., 0.]; - let c = [1., 0.]; - let d = [0., 1.]; - let half_edges = [([b, c], bc), ([c, d], dc), ([d, b], bd)] .map(|(points, global_form)| { - HalfEdgeBuilder::line_segment(points, None) - .with_global_form(global_form) - .build(&mut services.objects) - .insert(&mut services.objects) + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ) + .with_global_form(global_form) + .build(&mut services.objects) + .insert(&mut services.objects) }); Cycle::new(half_edges).insert(&mut services.objects) From c60e946f66b0bec52c61fe6510ea9045fd6dd07f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:35:57 +0100 Subject: [PATCH 07/14] Make `CycleBuilder` method more general --- crates/fj-kernel/src/builder/cycle.rs | 7 +++++-- crates/fj-kernel/src/validate/cycle.rs | 4 ++-- crates/fj-operations/src/sketch.rs | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/builder/cycle.rs b/crates/fj-kernel/src/builder/cycle.rs index 92fc63cd8..ee8ae6697 100644 --- a/crates/fj-kernel/src/builder/cycle.rs +++ b/crates/fj-kernel/src/builder/cycle.rs @@ -24,8 +24,11 @@ impl CycleBuilder { } /// Add a half-edge to the cycle - pub fn add_half_edge(mut self, half_edge: HalfEdgeBuilder) -> Self { - self.half_edges.push(half_edge); + pub fn add_half_edges( + mut self, + half_edges: impl IntoIterator, + ) -> Self { + self.half_edges.extend(half_edges); self } diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index 7b638f438..e4b21a812 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -118,8 +118,8 @@ mod tests { HalfEdgeBuilder::line_segment([[0., 0.], [1., 0.]], None); CycleBuilder::new() - .add_half_edge(first) - .add_half_edge(second) + .add_half_edges([first]) + .add_half_edges([second]) .build(&mut services.objects) }; diff --git a/crates/fj-operations/src/sketch.rs b/crates/fj-operations/src/sketch.rs index 8091de253..83b7ad565 100644 --- a/crates/fj-operations/src/sketch.rs +++ b/crates/fj-operations/src/sketch.rs @@ -68,7 +68,7 @@ impl Shape for fj::Sketch { } }; - cycle = cycle.add_half_edge(half_edge); + cycle = cycle.add_half_edges([half_edge]); } cycle.build(objects).insert(objects) From c0cf248960717494c3826d9ba3e151af02d591a5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 10:36:23 +0100 Subject: [PATCH 08/14] Refactor code to simplify it --- crates/fj-kernel/src/validate/cycle.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/fj-kernel/src/validate/cycle.rs b/crates/fj-kernel/src/validate/cycle.rs index e4b21a812..592dca4f3 100644 --- a/crates/fj-kernel/src/validate/cycle.rs +++ b/crates/fj-kernel/src/validate/cycle.rs @@ -118,8 +118,7 @@ mod tests { HalfEdgeBuilder::line_segment([[0., 0.], [1., 0.]], None); CycleBuilder::new() - .add_half_edges([first]) - .add_half_edges([second]) + .add_half_edges([first, second]) .build(&mut services.objects) }; From a58253cfcfcba63134faa740beb7b0d1ee122fc5 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 11:35:14 +0100 Subject: [PATCH 09/14] Add `ShellBuilder` --- crates/fj-kernel/src/builder/mod.rs | 3 +- crates/fj-kernel/src/builder/shell.rs | 131 +++++++++++++++++++++++ crates/fj-kernel/src/validate/shell.rs | 138 ++----------------------- 3 files changed, 139 insertions(+), 133 deletions(-) create mode 100644 crates/fj-kernel/src/builder/shell.rs diff --git a/crates/fj-kernel/src/builder/mod.rs b/crates/fj-kernel/src/builder/mod.rs index 21b498be8..1cbad97ac 100644 --- a/crates/fj-kernel/src/builder/mod.rs +++ b/crates/fj-kernel/src/builder/mod.rs @@ -4,9 +4,10 @@ mod cycle; mod edge; mod face; +mod shell; mod surface; pub use self::{ cycle::CycleBuilder, edge::HalfEdgeBuilder, face::FaceBuilder, - surface::SurfaceBuilder, + shell::ShellBuilder, surface::SurfaceBuilder, }; diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs new file mode 100644 index 000000000..dc043a095 --- /dev/null +++ b/crates/fj-kernel/src/builder/shell.rs @@ -0,0 +1,131 @@ +use fj_math::Point; + +use crate::{ + insert::Insert, + objects::{Cycle, Face, Objects, Shell}, + services::Service, +}; + +use super::{HalfEdgeBuilder, SurfaceBuilder}; + +/// Builder API for [`Shell`] +pub struct ShellBuilder {} + +impl ShellBuilder { + /// Create a tetrahedron from the provided points + pub fn tetrahedron( + points: [impl Into>; 4], + objects: &mut Service, + ) -> Shell { + let [a, b, c, d] = points.map(Into::into); + + let (bottom, [ab, bc, ca]) = { + let surface = + SurfaceBuilder::plane_from_points([a, b, c]).insert(objects); + let (exterior, global_edges) = { + let half_edges = [[a, b], [b, c], [c, a]].map(|points| { + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ) + .build(objects) + .insert(objects) + }); + + let cycle = Cycle::new(half_edges.clone()).insert(objects); + + let global_edges = + half_edges.map(|half_edge| half_edge.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None).insert(objects); + + (face, global_edges) + }; + let (front, [_, bd, da]) = { + let surface = + SurfaceBuilder::plane_from_points([a, b, d]).insert(objects); + let (exterior, global_edges) = { + let half_edges = + [([a, b], Some(ab)), ([b, d], None), ([d, a], None)].map( + |(points, global_form)| { + let mut builder = + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ); + + if let Some(global_form) = global_form { + builder = builder.with_global_form(global_form); + } + + builder.build(objects).insert(objects) + }, + ); + + let cycle = Cycle::new(half_edges.clone()).insert(objects); + + let global_edges = half_edges + .map(|half_edges| half_edges.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None).insert(objects); + + (face, global_edges) + }; + let (left, [_, _, dc]) = { + let surface = + SurfaceBuilder::plane_from_points([c, a, d]).insert(objects); + let (exterior, global_edges) = { + let half_edges = + [([c, a], Some(ca)), ([a, d], Some(da)), ([d, c], None)] + .map(|(points, global_form)| { + let mut builder = + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ); + + if let Some(global_form) = global_form { + builder = builder.with_global_form(global_form); + } + + builder.build(objects).insert(objects) + }); + + let cycle = Cycle::new(half_edges.clone()).insert(objects); + + let global_edges = + half_edges.map(|half_edge| half_edge.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None).insert(objects); + + (face, global_edges) + }; + let back_right = { + let surface = + SurfaceBuilder::plane_from_points([b, c, d]).insert(objects); + let exterior = { + let half_edges = [([b, c], bc), ([c, d], dc), ([d, b], bd)] + .map(|(points, global_form)| { + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ) + .with_global_form(global_form) + .build(objects) + .insert(objects) + }); + + Cycle::new(half_edges).insert(objects) + }; + + Face::new(surface, exterior, [], None).insert(objects) + }; + + Shell::new([bottom, front, left, back_right]) + } +} diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 2477b1207..1ab1bd964 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -191,13 +191,11 @@ impl ShellValidationError { #[cfg(test)] mod tests { - use fj_math::Point; - use crate::{ assert_contains_err, - builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder, SurfaceBuilder}, + builder::{CycleBuilder, FaceBuilder, ShellBuilder}, insert::Insert, - objects::{Cycle, Face, Shell}, + objects::Shell, services::Services, validate::{shell::ShellValidationError, Validate, ValidationError}, }; @@ -206,134 +204,10 @@ mod tests { fn coincident_not_identical() -> anyhow::Result<()> { let mut services = Services::new(); - let valid = { - let [a, b, c, d] = - [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]] - .map(Point::from); - - let (bottom, [ab, bc, ca]) = { - let surface = SurfaceBuilder::plane_from_points([a, b, c]) - .insert(&mut services.objects); - let (exterior, global_edges) = { - let half_edges = [[a, b], [b, c], [c, a]].map(|points| { - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ) - .build(&mut services.objects) - .insert(&mut services.objects) - }); - - let cycle = Cycle::new(half_edges.clone()) - .insert(&mut services.objects); - - let global_edges = half_edges - .map(|half_edge| half_edge.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None) - .insert(&mut services.objects); - - (face, global_edges) - }; - let (front, [_, bd, da]) = { - let surface = SurfaceBuilder::plane_from_points([a, b, d]) - .insert(&mut services.objects); - let (exterior, global_edges) = { - let half_edges = - [([a, b], Some(ab)), ([b, d], None), ([d, a], None)] - .map(|(points, global_form)| { - let mut builder = - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ); - - if let Some(global_form) = global_form { - builder = - builder.with_global_form(global_form); - } - - builder - .build(&mut services.objects) - .insert(&mut services.objects) - }); - - let cycle = Cycle::new(half_edges.clone()) - .insert(&mut services.objects); - - let global_edges = half_edges - .map(|half_edges| half_edges.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None) - .insert(&mut services.objects); - - (face, global_edges) - }; - let (left, [_, _, dc]) = { - let surface = SurfaceBuilder::plane_from_points([c, a, d]) - .insert(&mut services.objects); - let (exterior, global_edges) = { - let half_edges = [ - ([c, a], Some(ca)), - ([a, d], Some(da)), - ([d, c], None), - ] - .map(|(points, global_form)| { - let mut builder = - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ); - - if let Some(global_form) = global_form { - builder = builder.with_global_form(global_form); - } - - builder - .build(&mut services.objects) - .insert(&mut services.objects) - }); - - let cycle = Cycle::new(half_edges.clone()) - .insert(&mut services.objects); - - let global_edges = half_edges - .map(|half_edge| half_edge.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None) - .insert(&mut services.objects); - - (face, global_edges) - }; - let back_right = { - let surface = SurfaceBuilder::plane_from_points([b, c, d]) - .insert(&mut services.objects); - let exterior = { - let half_edges = [([b, c], bc), ([c, d], dc), ([d, b], bd)] - .map(|(points, global_form)| { - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ) - .with_global_form(global_form) - .build(&mut services.objects) - .insert(&mut services.objects) - }); - - Cycle::new(half_edges).insert(&mut services.objects) - }; - - Face::new(surface, exterior, [], None) - .insert(&mut services.objects) - }; - - Shell::new([bottom, front, left, back_right]) - }; + let valid = ShellBuilder::tetrahedron( + [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]], + &mut services.objects, + ); let invalid = { let face1 = FaceBuilder::new(services.objects.surfaces.xy_plane()) .with_exterior(CycleBuilder::polygon([ From 7b59700dd221b2b3f7eb4d5b1f62c9a5f07e5f1e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 11:36:47 +0100 Subject: [PATCH 10/14] Test valid case in other `Shell` validation test --- crates/fj-kernel/src/validate/shell.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/fj-kernel/src/validate/shell.rs b/crates/fj-kernel/src/validate/shell.rs index 1ab1bd964..7deba2f69 100644 --- a/crates/fj-kernel/src/validate/shell.rs +++ b/crates/fj-kernel/src/validate/shell.rs @@ -246,6 +246,10 @@ mod tests { fn shell_not_watertight() -> anyhow::Result<()> { let mut services = Services::new(); + let valid = ShellBuilder::tetrahedron( + [[0., 0., 0.], [1., 0., 0.], [0., 1., 0.], [0., 0., 1.]], + &mut services.objects, + ); let invalid = { // Shell with single face is not watertight let face = FaceBuilder::new(services.objects.surfaces.xy_plane()) @@ -260,6 +264,7 @@ mod tests { Shell::new([face]) }; + valid.validate_and_return_first_error()?; assert_contains_err!( invalid, ValidationError::Shell(ShellValidationError::NotWatertight) From 7960891e723e228c74284146cbc3ddbd24d2a518 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 11:56:18 +0100 Subject: [PATCH 11/14] Add `FaceBuilder::triangle` --- crates/fj-kernel/src/builder/face.rs | 36 +++++++++++++++++++++++++-- crates/fj-kernel/src/builder/shell.rs | 27 ++------------------ 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index 667dc4a62..dcb7b0ec8 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -1,13 +1,14 @@ use fj_interop::mesh::Color; +use fj_math::Point; use crate::{ insert::Insert, - objects::{Face, Objects, Surface}, + objects::{Cycle, Face, GlobalEdge, Objects, Surface}, services::Service, storage::Handle, }; -use super::CycleBuilder; +use super::{CycleBuilder, HalfEdgeBuilder, SurfaceBuilder}; /// Builder API for [`Face`] pub struct FaceBuilder { @@ -27,6 +28,37 @@ impl FaceBuilder { } } + /// Create a triangle + pub fn triangle( + points: [impl Into>; 3], + objects: &mut Service, + ) -> (Handle, [Handle; 3]) { + let [a, b, c] = points.map(Into::into); + + let surface = + SurfaceBuilder::plane_from_points([a, b, c]).insert(objects); + let (exterior, global_edges) = { + let half_edges = [[a, b], [b, c], [c, a]].map(|points| { + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ) + .build(objects) + .insert(objects) + }); + + let cycle = Cycle::new(half_edges.clone()).insert(objects); + + let global_edges = + half_edges.map(|half_edge| half_edge.global_form().clone()); + + (cycle, global_edges) + }; + + let face = Face::new(surface, exterior, [], None).insert(objects); + + (face, global_edges) + } + /// Replace the face's exterior cycle pub fn with_exterior(mut self, exterior: CycleBuilder) -> Self { self.exterior = exterior; diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index dc043a095..b0e6b7efd 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -6,7 +6,7 @@ use crate::{ services::Service, }; -use super::{HalfEdgeBuilder, SurfaceBuilder}; +use super::{FaceBuilder, HalfEdgeBuilder, SurfaceBuilder}; /// Builder API for [`Shell`] pub struct ShellBuilder {} @@ -19,30 +19,7 @@ impl ShellBuilder { ) -> Shell { let [a, b, c, d] = points.map(Into::into); - let (bottom, [ab, bc, ca]) = { - let surface = - SurfaceBuilder::plane_from_points([a, b, c]).insert(objects); - let (exterior, global_edges) = { - let half_edges = [[a, b], [b, c], [c, a]].map(|points| { - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ) - .build(objects) - .insert(objects) - }); - - let cycle = Cycle::new(half_edges.clone()).insert(objects); - - let global_edges = - half_edges.map(|half_edge| half_edge.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None).insert(objects); - - (face, global_edges) - }; + let (bottom, [ab, bc, ca]) = FaceBuilder::triangle([a, b, c], objects); let (front, [_, bd, da]) = { let surface = SurfaceBuilder::plane_from_points([a, b, d]).insert(objects); From 3f948e60434d5c3cf7953b926c6d44e6fdad54ad Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 11:59:19 +0100 Subject: [PATCH 12/14] Accept edges in `FaceBuilder::triangle` --- crates/fj-kernel/src/builder/face.rs | 24 ++++++++++++++++-------- crates/fj-kernel/src/builder/shell.rs | 3 ++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/fj-kernel/src/builder/face.rs b/crates/fj-kernel/src/builder/face.rs index dcb7b0ec8..c450d5f37 100644 --- a/crates/fj-kernel/src/builder/face.rs +++ b/crates/fj-kernel/src/builder/face.rs @@ -1,4 +1,4 @@ -use fj_interop::mesh::Color; +use fj_interop::{ext::ArrayExt, mesh::Color}; use fj_math::Point; use crate::{ @@ -31,6 +31,7 @@ impl FaceBuilder { /// Create a triangle pub fn triangle( points: [impl Into>; 3], + edges: [Option>; 3], objects: &mut Service, ) -> (Handle, [Handle; 3]) { let [a, b, c] = points.map(Into::into); @@ -38,13 +39,20 @@ impl FaceBuilder { let surface = SurfaceBuilder::plane_from_points([a, b, c]).insert(objects); let (exterior, global_edges) = { - let half_edges = [[a, b], [b, c], [c, a]].map(|points| { - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ) - .build(objects) - .insert(objects) - }); + let half_edges = [[a, b], [b, c], [c, a]].zip_ext(edges).map( + |(points, global_form)| { + let mut builder = + HalfEdgeBuilder::line_segment_from_global_points( + points, &surface, None, + ); + + if let Some(global_form) = global_form { + builder = builder.with_global_form(global_form); + } + + builder.build(objects).insert(objects) + }, + ); let cycle = Cycle::new(half_edges.clone()).insert(objects); diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index b0e6b7efd..83c1a1d03 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -19,7 +19,8 @@ impl ShellBuilder { ) -> Shell { let [a, b, c, d] = points.map(Into::into); - let (bottom, [ab, bc, ca]) = FaceBuilder::triangle([a, b, c], objects); + let (bottom, [ab, bc, ca]) = + FaceBuilder::triangle([a, b, c], [None, None, None], objects); let (front, [_, bd, da]) = { let surface = SurfaceBuilder::plane_from_points([a, b, d]).insert(objects); From bdddf7100a462bcfcc3b10106f517db95f8bd7e6 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 12:00:56 +0100 Subject: [PATCH 13/14] Make more use of `FaceBuilder::triangle` --- crates/fj-kernel/src/builder/shell.rs | 99 ++++----------------------- 1 file changed, 14 insertions(+), 85 deletions(-) diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index 83c1a1d03..bab7f1d63 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -1,12 +1,11 @@ use fj_math::Point; use crate::{ - insert::Insert, - objects::{Cycle, Face, Objects, Shell}, + objects::{Objects, Shell}, services::Service, }; -use super::{FaceBuilder, HalfEdgeBuilder, SurfaceBuilder}; +use super::FaceBuilder; /// Builder API for [`Shell`] pub struct ShellBuilder {} @@ -21,88 +20,18 @@ impl ShellBuilder { let (bottom, [ab, bc, ca]) = FaceBuilder::triangle([a, b, c], [None, None, None], objects); - let (front, [_, bd, da]) = { - let surface = - SurfaceBuilder::plane_from_points([a, b, d]).insert(objects); - let (exterior, global_edges) = { - let half_edges = - [([a, b], Some(ab)), ([b, d], None), ([d, a], None)].map( - |(points, global_form)| { - let mut builder = - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ); - - if let Some(global_form) = global_form { - builder = builder.with_global_form(global_form); - } - - builder.build(objects).insert(objects) - }, - ); - - let cycle = Cycle::new(half_edges.clone()).insert(objects); - - let global_edges = half_edges - .map(|half_edges| half_edges.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None).insert(objects); - - (face, global_edges) - }; - let (left, [_, _, dc]) = { - let surface = - SurfaceBuilder::plane_from_points([c, a, d]).insert(objects); - let (exterior, global_edges) = { - let half_edges = - [([c, a], Some(ca)), ([a, d], Some(da)), ([d, c], None)] - .map(|(points, global_form)| { - let mut builder = - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ); - - if let Some(global_form) = global_form { - builder = builder.with_global_form(global_form); - } - - builder.build(objects).insert(objects) - }); - - let cycle = Cycle::new(half_edges.clone()).insert(objects); - - let global_edges = - half_edges.map(|half_edge| half_edge.global_form().clone()); - - (cycle, global_edges) - }; - - let face = Face::new(surface, exterior, [], None).insert(objects); - - (face, global_edges) - }; - let back_right = { - let surface = - SurfaceBuilder::plane_from_points([b, c, d]).insert(objects); - let exterior = { - let half_edges = [([b, c], bc), ([c, d], dc), ([d, b], bd)] - .map(|(points, global_form)| { - HalfEdgeBuilder::line_segment_from_global_points( - points, &surface, None, - ) - .with_global_form(global_form) - .build(objects) - .insert(objects) - }); - - Cycle::new(half_edges).insert(objects) - }; - - Face::new(surface, exterior, [], None).insert(objects) - }; + let (front, [_, bd, da]) = + FaceBuilder::triangle([a, b, d], [Some(ab), None, None], objects); + let (left, [_, _, dc]) = FaceBuilder::triangle( + [c, a, d], + [Some(ca), Some(da), None], + objects, + ); + let (back_right, _) = FaceBuilder::triangle( + [b, c, d], + [Some(bc), Some(dc), Some(bd)], + objects, + ); Shell::new([bottom, front, left, back_right]) } From b55ed4dd8accafa28345fd7b844588af5a96deae Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 22 Mar 2023 12:03:36 +0100 Subject: [PATCH 14/14] Update variable names The old names were inherited from the test were this code was originally written, and they no longer fit this more generic context. --- crates/fj-kernel/src/builder/shell.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/fj-kernel/src/builder/shell.rs b/crates/fj-kernel/src/builder/shell.rs index bab7f1d63..81f14bf95 100644 --- a/crates/fj-kernel/src/builder/shell.rs +++ b/crates/fj-kernel/src/builder/shell.rs @@ -18,21 +18,21 @@ impl ShellBuilder { ) -> Shell { let [a, b, c, d] = points.map(Into::into); - let (bottom, [ab, bc, ca]) = + let (base, [ab, bc, ca]) = FaceBuilder::triangle([a, b, c], [None, None, None], objects); - let (front, [_, bd, da]) = + let (side_a, [_, bd, da]) = FaceBuilder::triangle([a, b, d], [Some(ab), None, None], objects); - let (left, [_, _, dc]) = FaceBuilder::triangle( + let (side_b, [_, _, dc]) = FaceBuilder::triangle( [c, a, d], [Some(ca), Some(da), None], objects, ); - let (back_right, _) = FaceBuilder::triangle( + let (side_c, _) = FaceBuilder::triangle( [b, c, d], [Some(bc), Some(dc), Some(bd)], objects, ); - Shell::new([bottom, front, left, back_right]) + Shell::new([base, side_a, side_b, side_c]) } }