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

Add custom data type to represent edge vertices #667

Merged
merged 13 commits into from
Jun 3, 2022
25 changes: 9 additions & 16 deletions crates/fj-kernel/src/algorithms/approx/edges.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use fj_math::Point;

use crate::{geometry, shape::LocalForm, topology::Vertex};
use crate::{geometry, topology::VerticesOfEdge};

pub fn approximate_edge(
vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
vertices: VerticesOfEdge,
points: &mut Vec<geometry::Point<1, 3>>,
) {
// Insert the exact vertices of this edge into the approximation. This means
Expand All @@ -14,13 +12,8 @@ pub fn approximate_edge(
// would lead to bugs in the approximation, as points that should refer to
// the same vertex would be understood to refer to very close, but distinct
// vertices.
let vertices = vertices.map(|vertices| {
vertices.map(|vertex| {
geometry::Point::new(
*vertex.local(),
vertex.canonical().get().point(),
)
})
let vertices = vertices.convert(|vertex| {
geometry::Point::new(*vertex.local(), vertex.canonical().get().point())
});
if let Some([a, b]) = vertices {
points.insert(0, a);
Expand All @@ -44,7 +37,7 @@ mod test {
use crate::{
geometry,
shape::{LocalForm, Shape},
topology::Vertex,
topology::{Vertex, VerticesOfEdge},
};

#[test]
Expand All @@ -59,10 +52,10 @@ mod test {
let v1 = Vertex::builder(&mut shape).build_from_point(a)?;
let v2 = Vertex::builder(&mut shape).build_from_point(d)?;

let vertices = [
let vertices = VerticesOfEdge::from_vertices([
LocalForm::new(Point::from([0.]), v1),
LocalForm::new(Point::from([1.]), v2),
];
]);

let a = geometry::Point::new([0.0], a);
let b = geometry::Point::new([0.25], b);
Expand All @@ -71,12 +64,12 @@ mod test {

// Regular edge
let mut points = vec![b, c];
super::approximate_edge(Some(vertices), &mut points);
super::approximate_edge(vertices, &mut points);
assert_eq!(points, vec![a, b, c, d]);

// Continuous edge
let mut points = vec![b, c];
super::approximate_edge(None, &mut points);
super::approximate_edge(VerticesOfEdge::none(), &mut points);
assert_eq!(points, vec![b, c, b]);

Ok(())
Expand Down
7 changes: 2 additions & 5 deletions crates/fj-kernel/src/algorithms/sweep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,8 @@ fn create_side_edges(
) -> Result<[Handle<Edge<3>>; 2], ValidationError> {
// Can't panic. We already ruled out the "continuous edge" case above, so
// these edges must have vertices.
let [vertices_bottom, vertices_top] = [edge_bottom, edge_top].map(|edge| {
edge.get()
.vertices
.expect("Expected vertices on non-continuous edge")
});
let [vertices_bottom, vertices_top] = [edge_bottom, edge_top]
.map(|edge| edge.get().vertices.expect_vertices());

// Can be simplified, once `zip` is stabilized:
// https://doc.rust-lang.org/std/primitive.array.html#method.zip
Expand Down
12 changes: 8 additions & 4 deletions crates/fj-kernel/src/shape/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ mod tests {
use crate::{
geometry::{Curve, Surface},
shape::{Handle, LocalForm, Shape, ValidationError, ValidationResult},
topology::{Cycle, Edge, Face, Vertex},
topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge},
};

const MIN_DISTANCE: f64 = 5e-7;
Expand All @@ -340,7 +340,7 @@ mod tests {
assert!(shape.get_handle(&surface.get()).as_ref() == Some(&surface));

let vertex = Vertex { point };
let edge = Edge::new(curve, None);
let edge = Edge::new(curve, VerticesOfEdge::none());

assert!(shape.get_handle(&vertex).is_none());
assert!(shape.get_handle(&edge).is_none());
Expand Down Expand Up @@ -429,7 +429,10 @@ mod tests {

// Shouldn't work. Nothing has been added to `shape`.
let err = shape
.insert(Edge::new(curve.clone(), Some([a.clone(), b.clone()])))
.insert(Edge::new(
curve.clone(),
VerticesOfEdge::from_vertices([a.clone(), b.clone()]),
))
.unwrap_err();
assert!(err.missing_curve(&curve));
assert!(err.missing_vertex(&a.canonical()));
Expand All @@ -443,7 +446,8 @@ mod tests {
let b = LocalForm::new(Point::from([2.]), b);

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

Ok(())
}
Expand Down
22 changes: 9 additions & 13 deletions crates/fj-kernel/src/shape/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use fj_math::Point;

use crate::{
geometry::{Curve, Surface},
topology::{Cycle, Edge, Face, Vertex},
topology::{Cycle, Edge, Face, Vertex, VerticesOfEdge},
};

use super::{
Expand Down Expand Up @@ -117,26 +117,22 @@ impl Object for Edge<3> {
mapping,
)?;

// Can be cleaned up using `try_map`, once that is stable:
// https://doc.rust-lang.org/std/primitive.array.html#method.try_map
let vertices: Option<[Result<_, ValidationError>; 2]> =
self.vertices.map(|vertices| {
vertices.map(|vertex| {
let vertices =
self.vertices
.try_convert::<_, _, ValidationError>(|vertex| {
let canonical = vertex.canonical();
let canonical = canonical.get().merge_into(
Some(canonical),
shape,
mapping,
)?;
Ok(LocalForm::new(*vertex.local(), canonical))
})
});
let vertices = match vertices {
Some([a, b]) => Some([a?, b?]),
None => None,
};
})?;

let merged = shape.get_handle_or_insert(Edge::new(curve, vertices))?;
let merged = shape.get_handle_or_insert(Edge::new(
curve,
VerticesOfEdge::new(vertices),
))?;

if let Some(handle) = handle {
mapping.edges.insert(handle, merged.clone());
Expand Down
53 changes: 25 additions & 28 deletions crates/fj-kernel/src/shape/validate/geometric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,31 @@ pub fn validate_edge(
// Validate that the local and canonical forms of the vertices match. As a
// side effect, this also happens to validate that the canonical forms of
// the vertices lie on the curve.
if let Some(vertices) = &edge.vertices {
let mut edge_vertex_mismatches = Vec::new();

for vertex in vertices {
let local = *vertex.local();
let local_3d = edge.curve().point_from_curve_coords(local);
let canonical = vertex.canonical().get().point();
let distance = (local_3d - canonical).magnitude();

if distance > max_distance {
edge_vertex_mismatches.push(EdgeVertexMismatch {
local,
local_3d,
canonical,
distance,
});
}
}

if !edge_vertex_mismatches.is_empty() {
return Err(GeometricIssues {
edge_vertex_mismatches,
let mut edge_vertex_mismatches = Vec::new();

for vertex in edge.vertices.iter() {
let local = *vertex.local();
let local_3d = edge.curve().point_from_curve_coords(local);
let canonical = vertex.canonical().get().point();
let distance = (local_3d - canonical).magnitude();

if distance > max_distance {
edge_vertex_mismatches.push(EdgeVertexMismatch {
local,
local_3d,
canonical,
distance,
});
}
}

if !edge_vertex_mismatches.is_empty() {
return Err(GeometricIssues {
edge_vertex_mismatches,
});
}

Ok(())
}

Expand Down Expand Up @@ -116,13 +115,11 @@ mod tests {
.build_line_segment_from_points([[0., 0., 0.], [1., 0., 0.]])?
.get();
let edge = Edge {
vertices: edge.vertices.clone().map(|vertices| {
vertices.map(|vertex| {
LocalForm::new(
*vertex.local() + [deviation],
vertex.canonical(),
)
})
vertices: edge.vertices.map(|vertex| {
LocalForm::new(
*vertex.local() + [deviation],
vertex.canonical(),
)
}),
..edge
};
Expand Down
8 changes: 3 additions & 5 deletions crates/fj-kernel/src/shape/validate/structural.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ pub fn validate_edge(
if !stores.curves.contains(&edge.curve.canonical()) {
missing_curve = Some(edge.curve.canonical());
}
for vertices in &edge.vertices {
for vertex in vertices {
if !stores.vertices.contains(&vertex.canonical()) {
missing_vertices.insert(vertex.canonical().clone());
}
for vertex in edge.vertices.iter() {
if !stores.vertices.contains(&vertex.canonical()) {
missing_vertices.insert(vertex.canonical().clone());
}
}

Expand Down
11 changes: 8 additions & 3 deletions crates/fj-kernel/src/topology/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
shape::{Handle, LocalForm, Shape, ValidationResult},
};

use super::{Cycle, Edge, Face, Vertex};
use super::{Cycle, Edge, Face, Vertex, VerticesOfEdge};

/// API for building a [`Vertex`]
#[must_use]
Expand Down Expand Up @@ -53,7 +53,9 @@ impl<'r> EdgeBuilder<'r> {
a: Vector::from([radius, Scalar::ZERO, Scalar::ZERO]),
b: Vector::from([Scalar::ZERO, radius, Scalar::ZERO]),
}))?;
let edge = self.shape.insert(Edge::new(curve, None))?;
let edge = self
.shape
.insert(Edge::new(curve, VerticesOfEdge::none()))?;

Ok(edge)
}
Expand Down Expand Up @@ -93,7 +95,10 @@ impl<'r> EdgeBuilder<'r> {
LocalForm::new(Point::from([1.]), b),
];

let edge = self.shape.insert(Edge::new(curve, Some(vertices)))?;
let edge = self.shape.insert(Edge::new(
curve,
VerticesOfEdge::from_vertices(vertices),
))?;

Ok(edge)
}
Expand Down
87 changes: 82 additions & 5 deletions crates/fj-kernel/src/topology/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,12 @@ pub struct Edge<const D: usize> {
///
/// If there are no such vertices, that means that both the curve and the
/// edge are continuous (i.e. connected to themselves).
pub vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
pub vertices: VerticesOfEdge,
}

impl Edge<3> {
/// Construct an instance of `Edge`
pub fn new(
curve: Handle<Curve<3>>,
vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>,
) -> Self {
pub fn new(curve: Handle<Curve<3>>, vertices: VerticesOfEdge) -> Self {
let curve = LocalForm::canonical_only(curve);
Self { curve, vertices }
}
Expand All @@ -68,6 +65,7 @@ impl<const D: usize> Edge<D> {
/// [`Handle`]s.
pub fn vertices(&self) -> Option<[Vertex; 2]> {
self.vertices
.0
.as_ref()
.map(|[a, b]| [a.canonical().get(), b.canonical().get()])
}
Expand All @@ -88,3 +86,82 @@ impl<const D: usize> fmt::Display for Edge<D> {
Ok(())
}
}

/// The vertices that bound an edge
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct VerticesOfEdge(Option<[LocalForm<Point<1>, Vertex>; 2]>);

impl VerticesOfEdge {
/// Construct an instance of `VerticesOfEdge` from zero or two vertices
pub fn new(vertices: Option<[LocalForm<Point<1>, Vertex>; 2]>) -> Self {
Self(vertices)
}

/// Construct an instance of `VerticesOfEdge` from two vertices
pub fn from_vertices(vertices: [LocalForm<Point<1>, Vertex>; 2]) -> Self {
Self(Some(vertices))
}

/// Construct an instance of `VerticesOfEdge` without vertices
pub fn none() -> Self {
Self(None)
}

/// Access the two vertices
///
/// # Panics
///
/// Panics, if the edge has no vertices.
pub fn expect_vertices(self) -> [LocalForm<Point<1>, Vertex>; 2] {
self.0.expect("Expected edge to have vertices")
}

/// Iterate over the vertices, if any
pub fn iter(&self) -> impl Iterator<Item = &LocalForm<Point<1>, Vertex>> {
self.0.iter().flatten()
}

/// Reverse the order of vertices
///
/// Makes sure that the local coordinates are still correct.
pub fn reverse(self) -> Self {
Self(self.0.map(|[a, b]| {
[
LocalForm::new(-(*b.local()), b.canonical()),
LocalForm::new(-(*a.local()), a.canonical()),
]
}))
}

/// Map each vertex using the provided function
pub fn map<F>(self, f: F) -> Self
where
F: FnMut(LocalForm<Point<1>, Vertex>) -> LocalForm<Point<1>, Vertex>,
{
Self(self.convert(f))
}

/// Convert each vertex using the provided function
pub fn convert<F, T>(self, f: F) -> Option<[T; 2]>
where
F: FnMut(LocalForm<Point<1>, Vertex>) -> T,
{
self.0.map(|vertices| vertices.map(f))
}

/// Convert each vertex using the provided fallible function
pub fn try_convert<F, T, E>(self, f: F) -> Result<Option<[T; 2]>, E>
where
F: FnMut(LocalForm<Point<1>, Vertex>) -> Result<T, E>,
{
// Can be cleaned up using `try_map`, once that is stable:
// https://doc.rust-lang.org/std/primitive.array.html#method.try_map
let vertices: Option<[Result<_, E>; 2]> = self.convert(f);
let vertices = match vertices {
Some([a, b]) => Some([a?, b?]),
None => None,
};

Ok(vertices)
}
}
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/topology/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ mod vertex;
pub use self::{
builder::{CycleBuilder, EdgeBuilder, FaceBuilder, VertexBuilder},
cycle::Cycle,
edge::Edge,
edge::{Edge, VerticesOfEdge},
face::{CyclesInFace, Face},
vertex::Vertex,
};
Loading