From 3c43f7dd6d28c6968dc6eaf580ddb062ccb2368c Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:33:15 +0100 Subject: [PATCH 1/3] Remove outdated documentation Vertex uniqueness is now checked in `Vertices::add`, so the caller doesn't have to keep track of this during construction. --- src/kernel/topology/vertices.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/kernel/topology/vertices.rs b/src/kernel/topology/vertices.rs index ae47fe927..7ea566467 100644 --- a/src/kernel/topology/vertices.rs +++ b/src/kernel/topology/vertices.rs @@ -8,22 +8,6 @@ use crate::math::Point; /// /// Points, on the other hand, might be used to approximate a shape for various /// purposes, without presenting any deeper truth about the shape's structure. -/// -/// # Uniqueness -/// -/// You **MUST NOT** construct a new instance of `Vertex` that represents an -/// already existing vertex. If there already exists a vertex and you need a -/// `Vertex` instance to refer to it, acquire one by copying or converting the -/// existing `Vertex` instance. -/// -/// Every time you create a `Vertex` instance, you might do so using a point you -/// have computed. When doing this for an existing vertex, you run the risk of -/// computing a slightly different point, due to floating point accuracy issues. -/// The resulting `Vertex` will then no longer be equal to the existing `Vertex` -/// instance that refers to the same vertex, which will cause bugs. -/// -/// This can be prevented outright by never creating a new `Vertex` instance -/// for an existing vertex. Hence why this is strictly forbidden. #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] pub struct Vertex(Point<3>); From f3da496e9f48983a7c0bd354f2b90d9190d8c789 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:36:00 +0100 Subject: [PATCH 2/3] Refactor --- src/kernel/algorithms/sweep.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index cad604db5..6a81b1a92 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -125,13 +125,15 @@ mod tests { .edges() .add(Edge::line_segment([c.clone(), a.clone()])); + let cycles = Cycle { + edges: vec![ab, bc, ca], + }; + let abc = Face::Face { surface: Surface::Swept(Swept::plane_from_points( [a, b, c].map(|vertex| vertex.point()), )), - cycles: vec![Cycle { - edges: vec![ab, bc, ca], - }], + cycles: vec![cycles], }; let face = shape.faces().add(abc); From a16ebef63891d26e203b31bc2ec1adf3498bafa4 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Tue, 8 Mar 2022 14:44:45 +0100 Subject: [PATCH 3/3] Make sure cycles that bound face are part of shape --- src/kernel/algorithms/approximation.rs | 4 ++-- src/kernel/algorithms/sweep.rs | 4 ++-- src/kernel/algorithms/transform.rs | 4 ++-- src/kernel/shapes/circle.rs | 6 +----- src/kernel/shapes/sketch.rs | 6 +----- src/kernel/topology/faces.rs | 3 ++- 6 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/kernel/algorithms/approximation.rs b/src/kernel/algorithms/approximation.rs index c1a7217f5..6b01c7912 100644 --- a/src/kernel/algorithms/approximation.rs +++ b/src/kernel/algorithms/approximation.rs @@ -262,9 +262,9 @@ mod tests { let cd = shape.edges().add(Edge::line_segment([v3, v4.clone()])); let da = shape.edges().add(Edge::line_segment([v4, v1])); - let abcd = Cycle { + let abcd = shape.cycles().add(Cycle { edges: vec![ab, bc, cd, da], - }; + }); let face = Face::Face { surface: Surface::x_y_plane(), diff --git a/src/kernel/algorithms/sweep.rs b/src/kernel/algorithms/sweep.rs index 6a81b1a92..b5e123b01 100644 --- a/src/kernel/algorithms/sweep.rs +++ b/src/kernel/algorithms/sweep.rs @@ -125,9 +125,9 @@ mod tests { .edges() .add(Edge::line_segment([c.clone(), a.clone()])); - let cycles = Cycle { + let cycles = shape.cycles().add(Cycle { edges: vec![ab, bc, ca], - }; + }); let abc = Face::Face { surface: Surface::Swept(Swept::plane_from_points( diff --git a/src/kernel/algorithms/transform.rs b/src/kernel/algorithms/transform.rs index f737dce1e..31ab73d17 100644 --- a/src/kernel/algorithms/transform.rs +++ b/src/kernel/algorithms/transform.rs @@ -43,7 +43,7 @@ pub fn transform_face( for cycle in cycles { let mut edges = Vec::new(); - for edge in cycle.edges { + for edge in &cycle.edges { let vertices = edge.vertices.clone().map(|vertices| { vertices.map(|vertex| { let point = @@ -62,7 +62,7 @@ pub fn transform_face( edges.push(edge); } - cycles_trans.push(Cycle { edges }); + cycles_trans.push(shape.cycles().add(Cycle { edges })); } Face::Face { diff --git a/src/kernel/shapes/circle.rs b/src/kernel/shapes/circle.rs index 3048af1c8..3cf0a37bc 100644 --- a/src/kernel/shapes/circle.rs +++ b/src/kernel/shapes/circle.rs @@ -25,11 +25,7 @@ impl ToShape for fj::Circle { .add(Edge::circle(Scalar::from_f64(self.radius))); shape.cycles().add(Cycle { edges: vec![edge] }); - let cycles = shape - .cycles() - .all() - .map(|handle| (*handle).clone()) - .collect(); + let cycles = shape.cycles().all().collect(); shape.faces().add(Face::Face { cycles, surface: Surface::x_y_plane(), diff --git a/src/kernel/shapes/sketch.rs b/src/kernel/shapes/sketch.rs index 296604b4f..de13b0a1e 100644 --- a/src/kernel/shapes/sketch.rs +++ b/src/kernel/shapes/sketch.rs @@ -48,11 +48,7 @@ impl ToShape for fj::Sketch { }; let face = Face::Face { - cycles: shape - .cycles() - .all() - .map(|handle| (*handle).clone()) - .collect(), + cycles: shape.cycles().all().collect(), surface: Surface::x_y_plane(), }; shape.faces().add(face); diff --git a/src/kernel/topology/faces.rs b/src/kernel/topology/faces.rs index 3f51e1ac9..6aebcfcf2 100644 --- a/src/kernel/topology/faces.rs +++ b/src/kernel/topology/faces.rs @@ -10,6 +10,7 @@ use crate::{ approximation::Approximation, triangulation::triangulate, }, geometry::Surface, + shape::handle::Handle, }, math::{Aabb, Scalar, Segment, Triangle}, }; @@ -37,7 +38,7 @@ pub enum Face { /// /// It might be less error-prone to specify the edges in surface /// coordinates. - cycles: Vec, + cycles: Vec>, }, /// The triangles of the face