From a2cca23e5e8d7d5a46c178ab22c55470c0024f7e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:48:19 +0200 Subject: [PATCH 01/14] Document module --- crates/fj-kernel/src/algorithms/reverse.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index d1d70acd4..5db619886 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -1,3 +1,5 @@ +//! Reverse the direction/orientation of objects + use fj_math::{Circle, Line, Point, Vector}; use crate::objects::{Curve, CurveKind, Cycle, Edge, Face}; From 55ec4b404ff0a3a30bb07942d2505f3e5da99f88 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:48:25 +0200 Subject: [PATCH 02/14] Make module public --- crates/fj-kernel/src/algorithms/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-kernel/src/algorithms/mod.rs b/crates/fj-kernel/src/algorithms/mod.rs index 631839911..a603f9247 100644 --- a/crates/fj-kernel/src/algorithms/mod.rs +++ b/crates/fj-kernel/src/algorithms/mod.rs @@ -3,11 +3,11 @@ //! Algorithmic code is collected in this module, to keep other modules focused //! on their respective purpose. -mod reverse; mod triangulate; pub mod approx; pub mod intersect; +pub mod reverse; pub mod sweep; pub mod transform; From 72e97d9ba8119a21483bf71eb776381b237a8ab3 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:49:29 +0200 Subject: [PATCH 03/14] Add trait `Reverse` --- crates/fj-kernel/src/algorithms/reverse.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 5db619886..529e45c79 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -4,6 +4,13 @@ use fj_math::{Circle, Line, Point, Vector}; use crate::objects::{Curve, CurveKind, Cycle, Edge, Face}; +/// Reverse the direction/orientation of an object +pub trait Reverse { + /// Reverse the direction/orientation of the object + #[must_use] + fn reverse(self) -> Self; +} + /// Reverse the direction of a face pub fn reverse_face(face: &Face) -> Face { if face.triangles().is_some() { From e08de818731c0ddfb13c6cb8ec5e3d573d139acb Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:52:24 +0200 Subject: [PATCH 04/14] Implement `Reverse` for `Face` --- crates/fj-kernel/src/algorithms/reverse.rs | 21 ++++++++++++------- crates/fj-kernel/src/algorithms/sweep/face.rs | 8 ++++--- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 529e45c79..e5a9670da 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -11,6 +11,12 @@ pub trait Reverse { fn reverse(self) -> Self; } +impl Reverse for Face { + fn reverse(self) -> Self { + reverse_face(&self) + } +} + /// Reverse the direction of a face pub fn reverse_face(face: &Face) -> Face { if face.triangles().is_some() { @@ -76,18 +82,19 @@ fn reverse_local_coordinates_in_cycle<'r>( mod tests { use pretty_assertions::assert_eq; - use crate::objects::{Face, Surface}; + use crate::{ + algorithms::reverse::Reverse, + objects::{Face, Surface}, + }; #[test] fn reverse_face() { let surface = Surface::xy_plane(); - let original = Face::build(surface).polygon_from_points([ - [0., 0.], - [1., 0.], - [0., 1.], - ]); + let original = Face::build(surface) + .polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]) + .into_face(); - let reversed = super::reverse_face(&original); + let reversed = original.reverse(); let surface = Surface::xy_plane().reverse(); let expected = Face::build(surface) diff --git a/crates/fj-kernel/src/algorithms/sweep/face.rs b/crates/fj-kernel/src/algorithms/sweep/face.rs index a84826c13..f34f47fd1 100644 --- a/crates/fj-kernel/src/algorithms/sweep/face.rs +++ b/crates/fj-kernel/src/algorithms/sweep/face.rs @@ -1,7 +1,9 @@ use fj_interop::mesh::Color; use crate::{ - algorithms::{approx::Tolerance, reverse_face, transform::TransformObject}, + algorithms::{ + approx::Tolerance, reverse::Reverse, transform::TransformObject, + }, objects::{Face, Shell}, }; @@ -46,7 +48,7 @@ fn create_bottom_face( if is_sweep_along_negative_direction { face.clone() } else { - reverse_face(face) + face.clone().reverse() } } @@ -54,7 +56,7 @@ fn create_top_face(face: Face, path: Path) -> Face { let mut face = face.translate(path.inner()); if path.is_negative_direction() { - face = reverse_face(&face); + face = face.reverse(); }; face From fefde33174ac77ca69687911af6491d03a79c09e Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:53:11 +0200 Subject: [PATCH 05/14] Inline function --- crates/fj-kernel/src/algorithms/mod.rs | 2 +- crates/fj-kernel/src/algorithms/reverse.rs | 27 +++++++++------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/mod.rs b/crates/fj-kernel/src/algorithms/mod.rs index a603f9247..1ec28c787 100644 --- a/crates/fj-kernel/src/algorithms/mod.rs +++ b/crates/fj-kernel/src/algorithms/mod.rs @@ -11,4 +11,4 @@ pub mod reverse; pub mod sweep; pub mod transform; -pub use self::{reverse::reverse_face, triangulate::triangulate}; +pub use self::triangulate::triangulate; diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index e5a9670da..4be9958ed 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -13,25 +13,20 @@ pub trait Reverse { impl Reverse for Face { fn reverse(self) -> Self { - reverse_face(&self) - } -} + if self.triangles().is_some() { + panic!("Reversing tri-rep faces is not supported"); + } -/// Reverse the direction of a face -pub fn reverse_face(face: &Face) -> Face { - if face.triangles().is_some() { - panic!("Reversing tri-rep faces is not supported"); - } + let surface = self.surface().reverse(); - let surface = face.surface().reverse(); + let exteriors = reverse_local_coordinates_in_cycle(self.exteriors()); + let interiors = reverse_local_coordinates_in_cycle(self.interiors()); - let exteriors = reverse_local_coordinates_in_cycle(face.exteriors()); - let interiors = reverse_local_coordinates_in_cycle(face.interiors()); - - Face::new(surface) - .with_exteriors(exteriors) - .with_interiors(interiors) - .with_color(face.color()) + Face::new(surface) + .with_exteriors(exteriors) + .with_interiors(interiors) + .with_color(self.color()) + } } fn reverse_local_coordinates_in_cycle<'r>( From fc23e1363ca2bd59f47f559766c40923f6600423 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:54:20 +0200 Subject: [PATCH 06/14] Move face reversal code to dedicate module --- crates/fj-kernel/src/algorithms/reverse.rs | 93 +----------------- .../fj-kernel/src/algorithms/reverse/face.rs | 94 +++++++++++++++++++ 2 files changed, 95 insertions(+), 92 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/reverse/face.rs diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse.rs index 4be9958ed..3bb9a0fda 100644 --- a/crates/fj-kernel/src/algorithms/reverse.rs +++ b/crates/fj-kernel/src/algorithms/reverse.rs @@ -1,8 +1,6 @@ //! Reverse the direction/orientation of objects -use fj_math::{Circle, Line, Point, Vector}; - -use crate::objects::{Curve, CurveKind, Cycle, Edge, Face}; +mod face; /// Reverse the direction/orientation of an object pub trait Reverse { @@ -10,92 +8,3 @@ pub trait Reverse { #[must_use] fn reverse(self) -> Self; } - -impl Reverse for Face { - fn reverse(self) -> Self { - if self.triangles().is_some() { - panic!("Reversing tri-rep faces is not supported"); - } - - let surface = self.surface().reverse(); - - let exteriors = reverse_local_coordinates_in_cycle(self.exteriors()); - let interiors = reverse_local_coordinates_in_cycle(self.interiors()); - - Face::new(surface) - .with_exteriors(exteriors) - .with_interiors(interiors) - .with_color(self.color()) - } -} - -fn reverse_local_coordinates_in_cycle<'r>( - cycles: impl IntoIterator + 'r, -) -> impl Iterator + 'r { - cycles.into_iter().map(|cycle| { - let surface = cycle.surface().reverse(); - - let edges = cycle.edges().map(|edge| { - let curve = { - let local = match edge.curve().kind() { - CurveKind::Circle(circle) => { - let center = Point::from([ - circle.center().u, - -circle.center().v, - ]); - - let a = Vector::from([circle.a().u, -circle.a().v]); - let b = Vector::from([circle.b().u, -circle.b().v]); - - CurveKind::Circle(Circle::new(center, a, b)) - } - CurveKind::Line(line) => { - let origin = - Point::from([line.origin().u, -line.origin().v]); - let direction = Vector::from([ - line.direction().u, - -line.direction().v, - ]); - - CurveKind::Line(Line::from_origin_and_direction( - origin, direction, - )) - } - }; - - Curve::new(local, *edge.curve().global()) - }; - - Edge::from_curve_and_vertices(curve, *edge.vertices()) - }); - - Cycle::new(surface).with_edges(edges) - }) -} - -#[cfg(test)] -mod tests { - use pretty_assertions::assert_eq; - - use crate::{ - algorithms::reverse::Reverse, - objects::{Face, Surface}, - }; - - #[test] - fn reverse_face() { - let surface = Surface::xy_plane(); - let original = Face::build(surface) - .polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]) - .into_face(); - - let reversed = original.reverse(); - - let surface = Surface::xy_plane().reverse(); - let expected = Face::build(surface) - .polygon_from_points([[0., 0.], [1., 0.], [0., -1.]]) - .into_face(); - - assert_eq!(expected, reversed); - } -} diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs new file mode 100644 index 000000000..b3ef3eacf --- /dev/null +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -0,0 +1,94 @@ +use fj_math::{Circle, Line, Point, Vector}; + +use crate::objects::{Curve, CurveKind, Cycle, Edge, Face}; + +use super::Reverse; + +impl Reverse for Face { + fn reverse(self) -> Self { + if self.triangles().is_some() { + panic!("Reversing tri-rep faces is not supported"); + } + + let surface = self.surface().reverse(); + + let exteriors = reverse_local_coordinates_in_cycle(self.exteriors()); + let interiors = reverse_local_coordinates_in_cycle(self.interiors()); + + Face::new(surface) + .with_exteriors(exteriors) + .with_interiors(interiors) + .with_color(self.color()) + } +} + +fn reverse_local_coordinates_in_cycle<'r>( + cycles: impl IntoIterator + 'r, +) -> impl Iterator + 'r { + cycles.into_iter().map(|cycle| { + let surface = cycle.surface().reverse(); + + let edges = cycle.edges().map(|edge| { + let curve = { + let local = match edge.curve().kind() { + CurveKind::Circle(circle) => { + let center = Point::from([ + circle.center().u, + -circle.center().v, + ]); + + let a = Vector::from([circle.a().u, -circle.a().v]); + let b = Vector::from([circle.b().u, -circle.b().v]); + + CurveKind::Circle(Circle::new(center, a, b)) + } + CurveKind::Line(line) => { + let origin = + Point::from([line.origin().u, -line.origin().v]); + let direction = Vector::from([ + line.direction().u, + -line.direction().v, + ]); + + CurveKind::Line(Line::from_origin_and_direction( + origin, direction, + )) + } + }; + + Curve::new(local, *edge.curve().global()) + }; + + Edge::from_curve_and_vertices(curve, *edge.vertices()) + }); + + Cycle::new(surface).with_edges(edges) + }) +} + +#[cfg(test)] +mod tests { + use pretty_assertions::assert_eq; + + use crate::{ + algorithms::reverse::Reverse, + objects::{Face, Surface}, + }; + + #[test] + fn reverse_face() { + let surface = Surface::xy_plane(); + let original = Face::build(surface) + .polygon_from_points([[0., 0.], [1., 0.], [0., 1.]]) + .into_face(); + + let reversed = original.reverse(); + + let surface = Surface::xy_plane().reverse(); + let expected = Face::build(surface) + .polygon_from_points([[0., 0.], [1., 0.], [0., -1.]]) + .into_face(); + + assert_eq!(expected, reversed); + } +} From a2359b4aed285756e1875b450d22790d7c17ddd0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 15:58:10 +0200 Subject: [PATCH 07/14] Restrict `reverse` module to one directory --- crates/fj-kernel/src/algorithms/{reverse.rs => reverse/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename crates/fj-kernel/src/algorithms/{reverse.rs => reverse/mod.rs} (100%) diff --git a/crates/fj-kernel/src/algorithms/reverse.rs b/crates/fj-kernel/src/algorithms/reverse/mod.rs similarity index 100% rename from crates/fj-kernel/src/algorithms/reverse.rs rename to crates/fj-kernel/src/algorithms/reverse/mod.rs From 6baec9587ce36a24c22296e47e20988000fc1171 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 16:01:44 +0200 Subject: [PATCH 08/14] Implement `Reverse` for `Surface` --- crates/fj-kernel/src/algorithms/reverse/mod.rs | 1 + crates/fj-kernel/src/algorithms/reverse/surface.rs | 11 +++++++++++ crates/fj-kernel/src/objects/surface.rs | 8 -------- 3 files changed, 12 insertions(+), 8 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/reverse/surface.rs diff --git a/crates/fj-kernel/src/algorithms/reverse/mod.rs b/crates/fj-kernel/src/algorithms/reverse/mod.rs index 3bb9a0fda..0e2f755f4 100644 --- a/crates/fj-kernel/src/algorithms/reverse/mod.rs +++ b/crates/fj-kernel/src/algorithms/reverse/mod.rs @@ -1,6 +1,7 @@ //! Reverse the direction/orientation of objects mod face; +mod surface; /// Reverse the direction/orientation of an object pub trait Reverse { diff --git a/crates/fj-kernel/src/algorithms/reverse/surface.rs b/crates/fj-kernel/src/algorithms/reverse/surface.rs new file mode 100644 index 000000000..23f3ad5ea --- /dev/null +++ b/crates/fj-kernel/src/algorithms/reverse/surface.rs @@ -0,0 +1,11 @@ +use crate::objects::Surface; + +use super::Reverse; + +impl Reverse for Surface { + fn reverse(self) -> Self { + match self { + Self::SweptCurve(surface) => Self::SweptCurve(surface.reverse()), + } + } +} diff --git a/crates/fj-kernel/src/objects/surface.rs b/crates/fj-kernel/src/objects/surface.rs index 9f0169f2c..76f1e42de 100644 --- a/crates/fj-kernel/src/objects/surface.rs +++ b/crates/fj-kernel/src/objects/surface.rs @@ -44,14 +44,6 @@ impl Surface { Self::SweptCurve(SweptCurve { curve, path }) } - /// Create a new instance that is reversed - #[must_use] - pub fn reverse(self) -> Self { - match self { - Self::SweptCurve(surface) => Self::SweptCurve(surface.reverse()), - } - } - /// Convert a point in surface coordinates to model coordinates pub fn point_from_surface_coords( &self, From cba92223a3731032477d04767c5cdd6562ce66bc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 30 Aug 2022 12:53:15 +0200 Subject: [PATCH 09/14] Implement `Reverse` for `GlobalCurve` --- crates/fj-kernel/src/algorithms/reverse/curve.rs | 9 +++++++++ crates/fj-kernel/src/algorithms/reverse/mod.rs | 1 + crates/fj-operations/src/difference_2d.rs | 12 ++++++------ 3 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/reverse/curve.rs diff --git a/crates/fj-kernel/src/algorithms/reverse/curve.rs b/crates/fj-kernel/src/algorithms/reverse/curve.rs new file mode 100644 index 000000000..fccce65da --- /dev/null +++ b/crates/fj-kernel/src/algorithms/reverse/curve.rs @@ -0,0 +1,9 @@ +use crate::objects::GlobalCurve; + +use super::Reverse; + +impl Reverse for GlobalCurve { + fn reverse(self) -> Self { + Self::from_kind(self.kind().reverse()) + } +} diff --git a/crates/fj-kernel/src/algorithms/reverse/mod.rs b/crates/fj-kernel/src/algorithms/reverse/mod.rs index 0e2f755f4..8196486b1 100644 --- a/crates/fj-kernel/src/algorithms/reverse/mod.rs +++ b/crates/fj-kernel/src/algorithms/reverse/mod.rs @@ -1,5 +1,6 @@ //! Reverse the direction/orientation of objects +mod curve; mod face; mod surface; diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index d00c0af77..fd9431fbb 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -1,8 +1,8 @@ use fj_interop::{debug::DebugInfo, mesh::Color}; use fj_kernel::{ - algorithms::approx::Tolerance, + algorithms::{approx::Tolerance, reverse::Reverse}, iter::ObjectIters, - objects::{Curve, Cycle, Edge, Face, GlobalCurve, Sketch}, + objects::{Curve, Cycle, Edge, Face, Sketch}, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -98,11 +98,11 @@ fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { *edge.curve().kind() }; - let curve_global = GlobalCurve::from_kind(if reverse { - edge.curve().global().kind().reverse() + let curve_global = if reverse { + edge.curve().global().reverse() } else { - *edge.curve().global().kind() - }); + *edge.curve().global() + }; let vertices = if reverse { edge.vertices().reverse() From a01ed0e7ff028c056cc90086623d588200208255 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 30 Aug 2022 12:55:36 +0200 Subject: [PATCH 10/14] Implement `Reverse` for `Curve` --- .../fj-kernel/src/algorithms/reverse/curve.rs | 8 +++++++- crates/fj-operations/src/difference_2d.rs | 19 +++++-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/fj-kernel/src/algorithms/reverse/curve.rs b/crates/fj-kernel/src/algorithms/reverse/curve.rs index fccce65da..3e120ffc6 100644 --- a/crates/fj-kernel/src/algorithms/reverse/curve.rs +++ b/crates/fj-kernel/src/algorithms/reverse/curve.rs @@ -1,7 +1,13 @@ -use crate::objects::GlobalCurve; +use crate::objects::{Curve, GlobalCurve}; use super::Reverse; +impl Reverse for Curve { + fn reverse(self) -> Self { + Curve::new(self.kind().reverse(), self.global().reverse()) + } +} + impl Reverse for GlobalCurve { fn reverse(self) -> Self { Self::from_kind(self.kind().reverse()) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index fd9431fbb..6b808d5f5 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -2,7 +2,7 @@ use fj_interop::{debug::DebugInfo, mesh::Color}; use fj_kernel::{ algorithms::{approx::Tolerance, reverse::Reverse}, iter::ObjectIters, - objects::{Curve, Cycle, Edge, Face, Sketch}, + objects::{Cycle, Edge, Face, Sketch}, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -92,16 +92,10 @@ impl Shape for fj::Difference2d { fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { let mut edges = Vec::new(); for edge in cycle.edges() { - let curve_local = if reverse { - edge.curve().kind().reverse() + let curve = if reverse { + edge.curve().reverse() } else { - *edge.curve().kind() - }; - - let curve_global = if reverse { - edge.curve().global().reverse() - } else { - *edge.curve().global() + *edge.curve() }; let vertices = if reverse { @@ -110,10 +104,7 @@ fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { *edge.vertices() }; - let edge = Edge::from_curve_and_vertices( - Curve::new(curve_local, curve_global), - vertices, - ); + let edge = Edge::from_curve_and_vertices(curve, vertices); edges.push(edge); } From 8a3ed05025c962601ca29d3dd3546ecb4a7c451c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 30 Aug 2022 12:58:07 +0200 Subject: [PATCH 11/14] Implement `Reverse` for `Edge` --- crates/fj-kernel/src/algorithms/reverse/edge.rs | 12 ++++++++++++ crates/fj-kernel/src/algorithms/reverse/mod.rs | 1 + crates/fj-operations/src/difference_2d.rs | 16 ++-------------- 3 files changed, 15 insertions(+), 14 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/reverse/edge.rs diff --git a/crates/fj-kernel/src/algorithms/reverse/edge.rs b/crates/fj-kernel/src/algorithms/reverse/edge.rs new file mode 100644 index 000000000..623a67c12 --- /dev/null +++ b/crates/fj-kernel/src/algorithms/reverse/edge.rs @@ -0,0 +1,12 @@ +use crate::objects::Edge; + +use super::Reverse; + +impl Reverse for Edge { + fn reverse(self) -> Self { + Edge::from_curve_and_vertices( + self.curve().reverse(), + self.vertices().reverse(), + ) + } +} diff --git a/crates/fj-kernel/src/algorithms/reverse/mod.rs b/crates/fj-kernel/src/algorithms/reverse/mod.rs index 8196486b1..b8c5e1790 100644 --- a/crates/fj-kernel/src/algorithms/reverse/mod.rs +++ b/crates/fj-kernel/src/algorithms/reverse/mod.rs @@ -1,6 +1,7 @@ //! Reverse the direction/orientation of objects mod curve; +mod edge; mod face; mod surface; diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index 6b808d5f5..ae827c835 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -2,7 +2,7 @@ use fj_interop::{debug::DebugInfo, mesh::Color}; use fj_kernel::{ algorithms::{approx::Tolerance, reverse::Reverse}, iter::ObjectIters, - objects::{Cycle, Edge, Face, Sketch}, + objects::{Cycle, Face, Sketch}, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -92,19 +92,7 @@ impl Shape for fj::Difference2d { fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { let mut edges = Vec::new(); for edge in cycle.edges() { - let curve = if reverse { - edge.curve().reverse() - } else { - *edge.curve() - }; - - let vertices = if reverse { - edge.vertices().reverse() - } else { - *edge.vertices() - }; - - let edge = Edge::from_curve_and_vertices(curve, vertices); + let edge = if reverse { edge.reverse() } else { *edge }; edges.push(edge); } From 7acbe369bc9998ea300807bc22ae578a42779ad0 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 30 Aug 2022 13:02:45 +0200 Subject: [PATCH 12/14] Implement `Reverse` for `Cycle` --- .../fj-kernel/src/algorithms/reverse/cycle.rs | 18 ++++++++++++++++++ crates/fj-kernel/src/algorithms/reverse/mod.rs | 1 + crates/fj-operations/src/difference_2d.rs | 13 +++---------- 3 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 crates/fj-kernel/src/algorithms/reverse/cycle.rs diff --git a/crates/fj-kernel/src/algorithms/reverse/cycle.rs b/crates/fj-kernel/src/algorithms/reverse/cycle.rs new file mode 100644 index 000000000..db73de4bb --- /dev/null +++ b/crates/fj-kernel/src/algorithms/reverse/cycle.rs @@ -0,0 +1,18 @@ +use crate::objects::Cycle; + +use super::Reverse; + +impl Reverse for Cycle { + fn reverse(self) -> Self { + let surface = *self.surface(); + + let mut edges = self + .into_edges() + .map(|edge| edge.reverse()) + .collect::>(); + + edges.reverse(); + + Cycle::new(surface).with_edges(edges) + } +} diff --git a/crates/fj-kernel/src/algorithms/reverse/mod.rs b/crates/fj-kernel/src/algorithms/reverse/mod.rs index b8c5e1790..2b736706f 100644 --- a/crates/fj-kernel/src/algorithms/reverse/mod.rs +++ b/crates/fj-kernel/src/algorithms/reverse/mod.rs @@ -1,6 +1,7 @@ //! Reverse the direction/orientation of objects mod curve; +mod cycle; mod edge; mod face; mod surface; diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index ae827c835..fa48e39e5 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -90,16 +90,9 @@ impl Shape for fj::Difference2d { } fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { - let mut edges = Vec::new(); - for edge in cycle.edges() { - let edge = if reverse { edge.reverse() } else { *edge }; - - edges.push(edge); - } - if reverse { - edges.reverse(); + cycle.reverse() + } else { + cycle } - - Cycle::new(*cycle.surface()).with_edges(edges) } From ec9cd297f5715911acecbffd628743b3133c8e26 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Thu, 25 Aug 2022 16:06:13 +0200 Subject: [PATCH 13/14] Add implementation notes --- .../fj-kernel/src/algorithms/reverse/curve.rs | 26 +++++++++++++++++++ .../fj-kernel/src/algorithms/reverse/face.rs | 13 ++++++++++ .../src/algorithms/reverse/surface.rs | 21 +++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/crates/fj-kernel/src/algorithms/reverse/curve.rs b/crates/fj-kernel/src/algorithms/reverse/curve.rs index 3e120ffc6..87048df73 100644 --- a/crates/fj-kernel/src/algorithms/reverse/curve.rs +++ b/crates/fj-kernel/src/algorithms/reverse/curve.rs @@ -3,12 +3,38 @@ use crate::objects::{Curve, GlobalCurve}; use super::Reverse; impl Reverse for Curve { + /// Reverse the curve + /// + /// # Implementation Note + /// + /// Right now, the orientation of a face is defined by the orientation of + /// its surface. By extension, the orientation of a surface is defined by + /// the orientation of the curve it was swept from. This leads to some + /// complications. See this issue for context: + /// + /// + /// It would probably be much better, if `Surface`s were without + /// orientation, which would then make it possible for curves to be without + /// direction. Then this implementation would not exist. fn reverse(self) -> Self { Curve::new(self.kind().reverse(), self.global().reverse()) } } impl Reverse for GlobalCurve { + /// Reverse the curve + /// + /// # Implementation Note + /// + /// Right now, the orientation of a face is defined by the orientation of + /// its surface. By extension, the orientation of a surface is defined by + /// the orientation of the curve it was swept from. This leads to some + /// complications. See this issue for context: + /// + /// + /// It would probably be much better, if `Surface`s were without + /// orientation, which would then make it possible for curves to be without + /// direction. Then this implementation would not exist. fn reverse(self) -> Self { Self::from_kind(self.kind().reverse()) } diff --git a/crates/fj-kernel/src/algorithms/reverse/face.rs b/crates/fj-kernel/src/algorithms/reverse/face.rs index b3ef3eacf..970041ee1 100644 --- a/crates/fj-kernel/src/algorithms/reverse/face.rs +++ b/crates/fj-kernel/src/algorithms/reverse/face.rs @@ -22,6 +22,19 @@ impl Reverse for Face { } } +/// Reverse local coordinates within the cycle, leaving global ones as-is +/// +/// # Implementation Note +/// +/// This is probably overly complicated. If the orientation of a face were +/// defined by the direction of the half-edges that bound it, we could reverse +/// the whole cycle with no weird distinction. The `Reverse` implementation of +/// `Face` could just use the `Reverse` implementation of `Cycle` then. +/// +/// Please note that, as of this writing, half-edges don't really exist as a +/// concept in the kernel. We kind of treat `Edge` as a half-edge, but in an +/// inconsistent way that causes problems. This issue has some context on that: +/// fn reverse_local_coordinates_in_cycle<'r>( cycles: impl IntoIterator + 'r, ) -> impl Iterator + 'r { diff --git a/crates/fj-kernel/src/algorithms/reverse/surface.rs b/crates/fj-kernel/src/algorithms/reverse/surface.rs index 23f3ad5ea..82652defd 100644 --- a/crates/fj-kernel/src/algorithms/reverse/surface.rs +++ b/crates/fj-kernel/src/algorithms/reverse/surface.rs @@ -3,6 +3,27 @@ use crate::objects::Surface; use super::Reverse; impl Reverse for Surface { + /// Reverse the surface + /// + /// # Implementation Note + /// + /// Right now, the orientation of a face is defined by the orientation of + /// its surface. This leads to some complications. See this issue for + /// context: + /// + /// + /// It would probably be much better, if `Surface`s were without + /// orientation, and the orientation of a face were defined by the direction + /// of the half-edges that bound it. + /// + /// Please note that, as of this writing, half-edges don't really exist as a + /// concept in the kernel. We kind of treat `Edge` as a half-edge, but in an + /// inconsistent way that causes problems. This issue has some context on + /// that: + /// + /// + /// So, in conclusion, in a perfect world, this implementation would not + /// exist. fn reverse(self) -> Self { match self { Self::SweptCurve(surface) => Self::SweptCurve(surface.reverse()), From 6bfb514c697032cddd8a48dd7cc3a8ca00284104 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 30 Aug 2022 13:05:00 +0200 Subject: [PATCH 14/14] Inline function --- crates/fj-operations/src/difference_2d.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/crates/fj-operations/src/difference_2d.rs b/crates/fj-operations/src/difference_2d.rs index fa48e39e5..d187a062f 100644 --- a/crates/fj-operations/src/difference_2d.rs +++ b/crates/fj-operations/src/difference_2d.rs @@ -2,7 +2,7 @@ use fj_interop::{debug::DebugInfo, mesh::Color}; use fj_kernel::{ algorithms::{approx::Tolerance, reverse::Reverse}, iter::ObjectIters, - objects::{Cycle, Face, Sketch}, + objects::{Face, Sketch}, validation::{validate, Validated, ValidationConfig, ValidationError}, }; use fj_math::Aabb; @@ -47,12 +47,10 @@ impl Shape for fj::Difference2d { ); for cycle in face.exteriors() { - let cycle = add_cycle(cycle.clone(), false); - exteriors.push(cycle); + exteriors.push(cycle.clone()); } for cycle in face.interiors() { - let cycle = add_cycle(cycle.clone(), true); - interiors.push(cycle); + interiors.push(cycle.clone().reverse()); } } @@ -64,8 +62,7 @@ impl Shape for fj::Difference2d { ); for cycle in face.exteriors() { - let cycle = add_cycle(cycle.clone(), true); - interiors.push(cycle); + interiors.push(cycle.clone().reverse()); } } @@ -88,11 +85,3 @@ impl Shape for fj::Difference2d { self.shapes()[0].bounding_volume() } } - -fn add_cycle(cycle: Cycle, reverse: bool) -> Cycle { - if reverse { - cycle.reverse() - } else { - cycle - } -}