Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store local representation of edge vertices #574

Merged
merged 11 commits into from
May 12, 2022
30 changes: 11 additions & 19 deletions crates/fj-kernel/src/algorithms/sweep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,10 @@ pub fn sweep_shape(
let vertices_top = source_to_top.vertices_for_edge(&edge_source);

let edge_bottom = target
.insert(Edge {
curve: curve_bottom,
vertices: vertices_bottom,
})
.unwrap();
let edge_top = target
.insert(Edge {
curve: curve_top,
vertices: vertices_top,
})
.insert(Edge::new(curve_bottom, vertices_bottom))
.unwrap();
let edge_top =
target.insert(Edge::new(curve_top, vertices_top)).unwrap();

source_to_bottom
.edges
Expand Down Expand Up @@ -175,7 +168,7 @@ pub fn sweep_shape(
vertices_source.map(|vertex_source| {
let vertex_bottom = source_to_bottom
.vertices
.get(&vertex_source)
.get(&vertex_source.handle)
.unwrap()
.clone();

Expand All @@ -188,18 +181,15 @@ pub fn sweep_shape(

let vertex_top = source_to_top
.vertices
.get(&vertex_source)
.get(&vertex_source.handle)
.unwrap()
.clone();

target
.insert(Edge {
.insert(Edge::new(
curve,
vertices: Some([
vertex_bottom,
vertex_top,
]),
})
Some([vertex_bottom, vertex_top]),
))
.unwrap()
})
.clone()
Expand Down Expand Up @@ -266,7 +256,9 @@ impl Relation {
edge: &Handle<Edge>,
) -> Option<[Handle<Vertex>; 2]> {
edge.get().vertices.map(|vertices| {
vertices.map(|vertex| self.vertices.get(&vertex).unwrap().clone())
vertices.map(|vertex| {
self.vertices.get(&vertex.handle).unwrap().clone()
})
})
}

Expand Down
16 changes: 11 additions & 5 deletions crates/fj-kernel/src/geometry/curves/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ pub use self::circle::Circle;

use fj_math::{Line, Point, Transform, Vector};

use crate::geometry;

/// A one-dimensional shape
///
/// The word "curve" is used as an umbrella term for all one-dimensional shapes,
Expand Down Expand Up @@ -85,11 +87,15 @@ impl Curve {
pub fn point_to_curve_coords(
&self,
point: impl Into<Point<3>>,
) -> Point<1> {
match self {
Self::Circle(curve) => curve.point_to_circle_coords(point),
Self::Line(curve) => curve.point_to_line_coords(point),
}
) -> geometry::Point<1> {
let point_3d = point.into();

let point_1d = match self {
Self::Circle(curve) => curve.point_to_circle_coords(point_3d),
Self::Line(curve) => curve.point_to_line_coords(point_3d),
};

geometry::Point::new(point_1d, point_3d)
}

/// Convert a point on the curve into model coordinates
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/geometry/surfaces/swept.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl SweptCurve {
) -> Point<2> {
let point = point.into();

let u = self.curve.point_to_curve_coords(point).t;
let u = self.curve.point_to_curve_coords(point).native().t;
let v = self.path_to_line().point_to_line_coords(point).t;

Point::from([u, v])
Expand Down
10 changes: 2 additions & 8 deletions crates/fj-kernel/src/shape/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,7 @@ mod tests {

// Shouldn't work. Nothing has been added to `shape`.
let err = shape
.insert(Edge {
curve: curve.clone(),
vertices: Some([a.clone(), b.clone()]),
})
.insert(Edge::new(curve.clone(), Some([a.clone(), b.clone()])))
.unwrap_err();
assert!(err.missing_curve(&curve));
assert!(err.missing_vertex(&a));
Expand All @@ -315,10 +312,7 @@ mod tests {
let b = Vertex::builder(&mut shape).build_from_point([2., 0., 0.])?;

// Everything has been added to `shape` now. Should work!
shape.insert(Edge {
curve,
vertices: Some([a, b]),
})?;
shape.insert(Edge::new(curve, Some([a, b])))?;

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions crates/fj-kernel/src/shape/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ impl Validate for Edge {
}
for vertices in &self.vertices {
for vertex in vertices {
if !stores.vertices.contains(vertex) {
missing_vertices.insert(vertex.clone());
if !stores.vertices.contains(&vertex.handle) {
missing_vertices.insert(vertex.handle.clone());
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions crates/fj-kernel/src/topology/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,7 @@ impl<'r> EdgeBuilder<'r> {
let curve = self.shape.insert(Curve::Line(Line::from_points(
vertices.clone().map(|vertex| vertex.get().point()),
)))?;
let edge = self.shape.insert(Edge {
curve,
vertices: Some(vertices),
})?;
let edge = self.shape.insert(Edge::new(curve, Some(vertices)))?;

Ok(edge)
}
Expand Down
55 changes: 55 additions & 0 deletions crates/fj-kernel/src/topology/cycles.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
use std::hash::{Hash, Hasher};

use crate::shape::{Handle, Shape};

use super::{CycleBuilder, Edge};

/// A cycle of connected edges
///
/// The end of each edge in the cycle must connect to the beginning of the next
/// edge. The end of the last edge must connect to the beginning of the first
/// one.
///
/// # Equality
///
/// Please refer to [`crate::kernel::topology`] for documentation on the
/// equality of topological objects.
///
/// # Validation
///
/// A cycle that is part of a [`Shape`] must be structurally sound. That means
/// the edges it refers to, must be part of the same shape.
#[derive(Clone, Debug, Eq, Ord, PartialOrd)]
pub struct Cycle {
/// The edges that make up the cycle
pub edges: Vec<Handle<Edge>>,
}

impl Cycle {
/// Build a cycle using the [`CycleBuilder`] API
pub fn builder(shape: &mut Shape) -> CycleBuilder {
CycleBuilder::new(shape)
}

/// Access the edges that this cycle refers to
///
/// This is a convenience method that saves the caller from dealing with the
/// [`Handle`]s.
pub fn edges(&self) -> impl Iterator<Item = Edge> + '_ {
self.edges.iter().map(|handle| handle.get())
}
}

impl PartialEq for Cycle {
fn eq(&self, other: &Self) -> bool {
self.edges().eq(other.edges())
}
}

impl Hash for Cycle {
fn hash<H: Hasher>(&self, state: &mut H) {
for edge in self.edges() {
edge.hash(state);
}
}
}
98 changes: 34 additions & 64 deletions crates/fj-kernel/src/topology/edges.rs
Original file line number Diff line number Diff line change
@@ -1,61 +1,11 @@
use std::hash::{Hash, Hasher};

use crate::{
geometry::Curve,
geometry::{self, Curve},
shape::{Handle, Shape},
};

use super::{builder::CycleBuilder, vertices::Vertex, EdgeBuilder};

/// A cycle of connected edges
///
/// The end of each edge in the cycle must connect to the beginning of the next
/// edge. The end of the last edge must connect to the beginning of the first
/// one.
///
/// # Equality
///
/// Please refer to [`crate::kernel::topology`] for documentation on the
/// equality of topological objects.
///
/// # Validation
///
/// A cycle that is part of a [`Shape`] must be structurally sound. That means
/// the edges it refers to, must be part of the same shape.
#[derive(Clone, Debug, Eq, Ord, PartialOrd)]
pub struct Cycle {
/// The edges that make up the cycle
pub edges: Vec<Handle<Edge>>,
}

impl Cycle {
/// Build a cycle using the [`CycleBuilder`] API
pub fn builder(shape: &mut Shape) -> CycleBuilder {
CycleBuilder::new(shape)
}

/// Access the edges that this cycle refers to
///
/// This is a convenience method that saves the caller from dealing with the
/// [`Handle`]s.
pub fn edges(&self) -> impl Iterator<Item = Edge> + '_ {
self.edges.iter().map(|handle| handle.get())
}
}

impl PartialEq for Cycle {
fn eq(&self, other: &Self) -> bool {
self.edges().eq(other.edges())
}
}

impl Hash for Cycle {
fn hash<H: Hasher>(&self, state: &mut H) {
for edge in self.edges() {
edge.hash(state);
}
}
}
use super::{vertices::Vertex, EdgeBuilder};

/// An edge of a shape
///
Expand All @@ -82,20 +32,26 @@ pub struct Edge {
///
/// If there are no such vertices, that means that both the curve and the
/// edge are continuous (i.e. connected to themselves).
///
/// # Implementation note
///
/// Since these vertices bound the edge, they must lie on the curve. This
/// isn't enforced at all, however. It would make sense to store 1D vertices
/// here, and indeed, this was the case in the past.
///
/// It got in the way of some work, however, so it made sense to simplify
/// it by storing 3D vertices. It will probably make sense to revert this
/// and store 1D vertices again, at some point.
pub vertices: Option<[Handle<Vertex>; 2]>,
pub vertices: Option<[EdgeVertex; 2]>,
}

impl Edge {
/// Construct an instance of `Edge`
pub fn new(
curve: Handle<Curve>,
vertices: Option<[Handle<Vertex>; 2]>,
) -> Self {
let vertices = vertices.map(|vertices| {
vertices.map(|handle| {
let local =
curve.get().point_to_curve_coords(handle.get().point());
EdgeVertex { handle, local }
})
});

Self { curve, vertices }
}

/// Build an edge using the [`EdgeBuilder`] API
pub fn builder(shape: &mut Shape) -> EdgeBuilder {
EdgeBuilder::new(shape)
Expand All @@ -114,7 +70,9 @@ impl Edge {
/// This is a convenience method that saves the caller from dealing with the
/// [`Handle`]s.
pub fn vertices(&self) -> Option<[Vertex; 2]> {
self.vertices.as_ref().map(|[a, b]| [a.get(), b.get()])
self.vertices
.as_ref()
.map(|[a, b]| [a.handle.get(), b.handle.get()])
}
}

Expand All @@ -130,3 +88,15 @@ impl Hash for Edge {
self.vertices().hash(state);
}
}

/// A vertex of an edge
#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub struct EdgeVertex {
/// The handle to the vertex
pub handle: Handle<Vertex>,

/// The local representation of the vertex
///
/// Represents the vertex in terms of the coordinates of the edge's curve.
pub local: geometry::Point<1>,
}
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/topology/faces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
shape::{Handle, Shape},
};

use super::{builder::FaceBuilder, edges::Cycle};
use super::{Cycle, FaceBuilder};

/// A face of a shape
///
Expand Down
6 changes: 4 additions & 2 deletions crates/fj-kernel/src/topology/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@
//! refer to objects in the same memory location.

mod builder;
mod cycles;
mod edges;
mod faces;
mod vertices;

pub use self::{
builder::{EdgeBuilder, VertexBuilder},
edges::{Cycle, Edge},
builder::{CycleBuilder, EdgeBuilder, FaceBuilder, VertexBuilder},
cycles::Cycle,
edges::{Edge, EdgeVertex},
faces::Face,
vertices::Vertex,
};
2 changes: 1 addition & 1 deletion crates/fj-operations/src/difference_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn add_cycle(
vs
});

let edge = shape.insert(Edge { curve, vertices }).unwrap();
let edge = shape.insert(Edge::new(curve, vertices)).unwrap();
edges.push(edge);
}

Expand Down
14 changes: 6 additions & 8 deletions crates/fj-operations/src/group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,12 @@ fn copy_shape(orig: Shape, target: &mut Shape) {
vertices.insert(vertex_orig, vertex);
}
for edge_orig in orig.edges() {
let edge = target
.insert(Edge {
curve: curves[&edge_orig.get().curve].clone(),
vertices: edge_orig.get().vertices.as_ref().map(|vs| {
vs.clone().map(|vertex| vertices[&vertex].clone())
}),
})
.unwrap();
let curve = curves[&edge_orig.get().curve].clone();
let vertices = edge_orig.get().vertices.as_ref().map(|vs| {
vs.clone().map(|vertex| vertices[&vertex.handle].clone())
});

let edge = target.insert(Edge::new(curve, vertices)).unwrap();
edges.insert(edge_orig, edge);
}
for cycle_orig in orig.cycles() {
Expand Down