Skip to content

Commit

Permalink
Merge pull request #1572 from hannobraun/builder
Browse files Browse the repository at this point in the history 
Make various fixes and small updates in builder API
  • Loading branch information
@hannobraun
hannobraun authored Feb 8, 2023
2 parents ed08c4c + 419921a commit 4c8b72c
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 95 deletions.
68 changes: 68 additions & 0 deletions crates/fj-kernel/src/builder/cycle.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::collections::VecDeque;

use fj_interop::ext::ArrayExt;
use fj_math::Point;

Expand Down Expand Up @@ -76,6 +78,33 @@ pub trait CycleBuilder {
&mut self,
points: [impl Into<Point<3>>; 3],
) -> [Partial<HalfEdge>; 3];

/// Connect the cycle to the provided half-edges
///
/// Assumes that the provided half-edges, once translated into local
/// equivalents of this cycle, will not form a cycle themselves.
///
/// Returns the local equivalents of the provided half-edges and, as the
/// last entry, an additional half-edge that closes the cycle.
fn connect_to_open_edges<O>(
&mut self,
edges: O,
) -> O::SizePlusOne<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>;

/// Connect the cycles to the provided half-edges
///
/// Assumes that the provided half-edges, once translated into local
/// equivalents of this cycle, form a cycle themselves.
///
/// Returns the local equivalents of the provided half-edges.
fn connect_to_closed_edges<O>(
&mut self,
edges: O,
) -> O::SameSize<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>;
}

impl CycleBuilder for PartialCycle {
Expand Down Expand Up @@ -197,4 +226,43 @@ impl CycleBuilder for PartialCycle {

half_edges
}

fn connect_to_open_edges<O>(
&mut self,
edges: O,
) -> O::SizePlusOne<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>,
{
// We need to create the additional half-edge last, but at the same time
// need to provide it to the `map_plus_one` method first. Really no
// choice but to create them all in one go, as we do here.
let mut half_edges = VecDeque::new();
for _ in 0..edges.num_objects() {
half_edges.push_back(self.add_half_edge());
}
let additional_half_edge = self.add_half_edge();

edges.map_plus_one(additional_half_edge, |other| {
let mut this = half_edges.pop_front().expect(
"Pushed correct number of half-edges; should be able to pop",
);
this.write().update_from_other_edge(&other);
this
})
}

fn connect_to_closed_edges<O>(
&mut self,
edges: O,
) -> O::SameSize<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>,
{
edges.map(|other| {
let mut this = self.add_half_edge();
this.write().update_from_other_edge(&other);
this
})
}
}
115 changes: 108 additions & 7 deletions crates/fj-kernel/src/builder/edge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use fj_interop::ext::ArrayExt;
use fj_math::{Point, Scalar};

use crate::{
geometry::path::{GlobalPath, SurfacePath},
objects::{GlobalEdge, HalfEdge, Surface},
partial::{MaybeSurfacePath, Partial, PartialGlobalEdge, PartialHalfEdge},
};
Expand Down Expand Up @@ -51,6 +52,11 @@ pub trait HalfEdgeBuilder {
///
/// Infers as much information about this edge from the other, under the
/// assumption that the other edge is on a different surface.
///
/// This method is quite fragile. It might panic, or even silently fail,
/// under various circumstances. As long as you're only dealing with lines
/// and planes, you should be fine. Otherwise, please read the code of this
/// method carefully, to make sure you don't run into trouble.
fn update_from_other_edge(&mut self, other: &Partial<HalfEdge>);
}

Expand Down Expand Up @@ -196,13 +202,108 @@ impl HalfEdgeBuilder for PartialHalfEdge {
self.curve.write().global_form = global_curve.clone();
self.global_form.write().curve = global_curve;

self.curve.write().path = other
.read()
.curve
.read()
.path
.as_ref()
.map(MaybeSurfacePath::to_undefined);
self.curve.write().path =
other.read().curve.read().path.as_ref().and_then(|path| {
match other.read().curve.read().surface.read().geometry {
Some(surface) => {
// We have information about the other edge's surface
// available. We need to use that to interpret what the
// other edge's curve path means for our curve path.
match surface.u {
GlobalPath::Circle(circle) => {
// The other surface is curved. We're entering
// some dodgy territory here, as only some edge
// cases can be represented using our current
// curve/surface representation.
match path {
MaybeSurfacePath::Defined(
SurfacePath::Line(_),
)
| MaybeSurfacePath::UndefinedLine => {
// We're dealing with a line on a
// rounded surface.
//
// Based on the current uses of this
// method, we can make some assumptions:
//
// 1. The line is parallel to the u-axis
// of the other surface.
// 2. The surface that *our* edge is in
// is a plane that is parallel to the
// the plane of the circle that
// defines the curvature of the other
// surface.
//
// These assumptions are necessary
// preconditions for the following code
// to work. But unfortunately, I see no
// way to check those preconditions
// here, as neither the other line nor
// our surface is necessarily defined
// yet.
//
// Handling this case anyway feels like
// a grave sin, but I don't know what
// else to do. If you tracked some
// extremely subtle and annoying bug
// back to this code, I apologize.
//
// I hope that I'll come up with a
// better curve/surface representation
// before this becomes a problem.
Some(
MaybeSurfacePath::UndefinedCircle {
radius: circle.radius(),
},
)
}
_ => {
// The other edge is a line segment in a
// curved surface. No idea how to deal
// with this.
todo!(
"Can't connect edge to circle on \
curved surface"
)
}
}
}
GlobalPath::Line(_) => {
// The other edge is defined on a plane.
match path {
MaybeSurfacePath::Defined(
SurfacePath::Line(_),
)
| MaybeSurfacePath::UndefinedLine => {
// The other edge is a line segment on
// a plane. That means our edge must be
// a line segment too.
Some(MaybeSurfacePath::UndefinedLine)
}
_ => {
// The other edge is a circle or arc on
// a plane. I'm actually not sure what
// that means for our edge. We might be
// able to represent it somehow, but
// let's leave that as an exercise for
// later.
todo!(
"Can't connect edge to circle on \
plane"
)
}
}
}
}
}
None => {
// We know nothing about the surface the other edge is
// on. This means we can't infer anything about our
// curve from the other curve.
None
}
}
});

for (this, other) in self
.vertices
Expand Down
76 changes: 5 additions & 71 deletions crates/fj-kernel/src/builder/face.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,14 @@ use fj_interop::ext::ArrayExt;

use crate::{
geometry::path::SurfacePath,
objects::{Cycle, HalfEdge, Surface},
objects::{Cycle, Surface},
partial::{MaybeSurfacePath, Partial, PartialCycle, PartialFace},
};

use super::{CycleBuilder, HalfEdgeBuilder, ObjectArgument, SurfaceBuilder};
use super::SurfaceBuilder;

/// Builder API for [`PartialFace`]
pub trait FaceBuilder {
/// Connect the face to other faces at the provided half-edges
///
/// Assumes that the provided half-edges, once translated into local
/// equivalents of this face, will not form a cycle.
///
/// Returns the local equivalents of the provided half-edges and, as the
/// last entry, an additional half-edge that closes the cycle.
fn connect_to_open_edges<O>(
&mut self,
edges: O,
) -> O::SizePlusOne<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>;

/// Connect the face to other faces at the provided half-edges
///
/// Assumes that the provided half-edges, once translated into local
/// equivalents of this face, form a cycle.
///
/// Returns the local equivalents of the provided half-edges.
fn connect_to_closed_edges<O>(
&mut self,
edges: O,
) -> O::SameSize<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>;

/// Add an interior cycle
fn add_interior(&mut self) -> Partial<Cycle>;

Expand All @@ -59,45 +32,6 @@ pub trait FaceBuilder {
}

impl FaceBuilder for PartialFace {
fn connect_to_open_edges<O>(
&mut self,
edges: O,
) -> O::SizePlusOne<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>,
{
// We need to create the additional half-edge last, but at the same time
// need to provide it to the `map_plus_one` method first. Really no
// choice but to create them all in one go, as we do here.
let mut half_edges = VecDeque::new();
for _ in 0..edges.num_objects() {
half_edges.push_back(self.exterior.write().add_half_edge());
}
let additional_half_edge = self.exterior.write().add_half_edge();

edges.map_plus_one(additional_half_edge, |other| {
let mut this = half_edges.pop_front().expect(
"Pushed correct number of half-edges; should be able to pop",
);
this.write().update_from_other_edge(&other);
this
})
}

fn connect_to_closed_edges<O>(
&mut self,
edges: O,
) -> O::SameSize<Partial<HalfEdge>>
where
O: ObjectArgument<Partial<HalfEdge>>,
{
edges.map(|other| {
let mut this = self.exterior.write().add_half_edge();
this.write().update_from_other_edge(&other);
this
})
}

fn add_interior(&mut self) -> Partial<Cycle> {
let cycle = Partial::from_partial(PartialCycle {
surface: self.exterior.read().surface.clone(),
Expand Down Expand Up @@ -125,7 +59,7 @@ impl FaceBuilder for PartialFace {
})
.collect::<VecDeque<_>>();

let (first_three_vertices, surface) = {
let (significant_vertices, surface) = {
let first_three_vertices = array::from_fn(|_| {
vertices
.pop_front()
Expand Down Expand Up @@ -153,7 +87,7 @@ impl FaceBuilder for PartialFace {
});

for (mut surface_vertex, point) in
first_three_vertices.into_iter().chain(rest_of_vertices)
significant_vertices.into_iter().chain(rest_of_vertices)
{
surface_vertex.write().position = Some(point);
}
Expand All @@ -173,7 +107,7 @@ impl FaceBuilder for PartialFace {
MaybeSurfacePath::Defined(_) => {
// Path is already defined. Nothing to infer.
}
MaybeSurfacePath::UndefinedCircle => todo!(
MaybeSurfacePath::UndefinedCircle { .. } => todo!(
"Inferring undefined circles is not supported yet"
),
MaybeSurfacePath::UndefinedLine => {
Expand Down
23 changes: 6 additions & 17 deletions crates/fj-kernel/src/partial/objects/curve.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use fj_math::Scalar;

use crate::{
geometry::path::SurfacePath,
objects::{Curve, GlobalCurve, Objects, Surface},
Expand Down Expand Up @@ -55,28 +57,15 @@ pub enum MaybeSurfacePath {
Defined(SurfacePath),

/// The surface path is undefined, but we know it is a circle
UndefinedCircle,
UndefinedCircle {
/// The radius of the undefined circle
radius: Scalar,
},

/// The surface path is undefined, but we know it is a line
UndefinedLine,
}

impl MaybeSurfacePath {
/// Convert into an undefined variant
///
/// If `self` is defined, it is converted into the applicable undefined
/// variant. If it is undefined, a copy is returned.
pub fn to_undefined(&self) -> Self {
match self {
Self::Defined(path) => match path {
SurfacePath::Circle(_) => Self::UndefinedCircle,
SurfacePath::Line(_) => Self::UndefinedLine,
},
undefined => undefined.clone(),
}
}
}

impl From<SurfacePath> for MaybeSurfacePath {
fn from(path: SurfacePath) -> Self {
Self::Defined(path)
Expand Down

0 comments on commit 4c8b72c

Please sign in to comment.