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

Simplify curve handling in PartialHalfEdge #1660

Merged
merged 8 commits into from
Mar 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/algorithms/sweep/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Sweep for Handle<Face> {
.into_iter()
.zip(top_cycle.write().half_edges.iter_mut())
{
top.write().curve = Some(bottom.curve().into());
top.write().curve = Some(bottom.curve());

let boundary = bottom.boundary();

Expand Down
29 changes: 12 additions & 17 deletions crates/fj-kernel/src/builder/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
surface::SurfaceGeometry,
},
objects::HalfEdge,
partial::{MaybeCurve, Partial, PartialHalfEdge},
partial::{Partial, PartialHalfEdge},
};

/// Builder API for [`PartialHalfEdge`]
Expand Down Expand Up @@ -61,7 +61,7 @@ impl HalfEdgeBuilder for PartialHalfEdge {
radius: impl Into<Scalar>,
) -> Curve {
let path = Curve::circle_from_radius(radius);
self.curve = Some(path.into());
self.curve = Some(path);

let [a_curve, b_curve] =
[Scalar::ZERO, Scalar::TAU].map(|coord| Point::from([coord]));
Expand Down Expand Up @@ -89,7 +89,7 @@ impl HalfEdgeBuilder for PartialHalfEdge {
let arc = fj_math::Arc::from_endpoints_and_angle(start, end, angle_rad);

let path = Curve::circle_from_center_and_radius(arc.center, arc.radius);
self.curve = Some(path.into());
self.curve = Some(path);

let [a_curve, b_curve] =
[arc.start_angle, arc.end_angle].map(|coord| Point::from([coord]));
Expand All @@ -106,19 +106,18 @@ impl HalfEdgeBuilder for PartialHalfEdge {
start: Point<2>,
end: Point<2>,
) -> Curve {
let boundary = self.boundary;
let points_surface = [start, end];

let path = if let [Some(start), Some(end)] = boundary {
let path = if let [Some(start), Some(end)] = self.boundary {
let points = [start, end].zip_ext(points_surface);

let path = Curve::from_points_with_line_coords(points);
self.curve = Some(path.into());
let path = Curve::line_from_points_with_coords(points);
self.curve = Some(path);

path
} else {
let (path, _) = Curve::line_from_points(points_surface);
self.curve = Some(path.into());
self.curve = Some(path);

for (vertex, position) in
self.boundary.each_mut_ext().zip_ext([0., 1.])
Expand All @@ -143,14 +142,13 @@ impl HalfEdgeBuilder for PartialHalfEdge {
// need to use that to interpret what the other edge's curve path
// means for our curve path.
match surface.u {
GlobalPath::Circle(circle) => {
GlobalPath::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 {
MaybeCurve::Defined(Curve::Line(_))
| MaybeCurve::UndefinedLine => {
Curve::Line(_) => {
// We're dealing with a line on a rounded surface.
//
// Based on the current uses of this method, we can
Expand All @@ -177,9 +175,7 @@ impl HalfEdgeBuilder for PartialHalfEdge {
// I hope that I'll come up with a better curve/
// surface representation before this becomes a
// problem.
Some(MaybeCurve::UndefinedCircle {
radius: circle.radius(),
})
None
}
_ => {
// The other edge is a line segment in a curved
Expand All @@ -194,11 +190,10 @@ impl HalfEdgeBuilder for PartialHalfEdge {
GlobalPath::Line(_) => {
// The other edge is defined on a plane.
match path {
MaybeCurve::Defined(Curve::Line(_))
| MaybeCurve::UndefinedLine => {
Curve::Line(_) => {
// The other edge is a line segment on a plane. That
// means our edge must be a line segment too.
Some(MaybeCurve::UndefinedLine)
None
}
_ => {
// The other edge is a circle or arc on a plane. I'm
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/geometry/curve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Curve {
}

/// Create a line from two points that include line coordinates
pub fn from_points_with_line_coords(
pub fn line_from_points_with_coords(
points: [(impl Into<Point<1>>, impl Into<Point<2>>); 2],
) -> Self {
Self::Line(Line::from_points_with_line_coords(points))
Expand Down
8 changes: 2 additions & 6 deletions crates/fj-kernel/src/partial/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,8 @@ mod wrapper;

pub use self::{
objects::{
cycle::PartialCycle,
edge::{MaybeCurve, PartialHalfEdge},
face::PartialFace,
shell::PartialShell,
sketch::PartialSketch,
solid::PartialSolid,
cycle::PartialCycle, edge::PartialHalfEdge, face::PartialFace,
shell::PartialShell, sketch::PartialSketch, solid::PartialSolid,
},
traits::{HasPartial, PartialObject},
wrapper::{FullToPartialCache, Partial},
Expand Down
44 changes: 5 additions & 39 deletions crates/fj-kernel/src/partial/objects/edge.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use fj_math::{Point, Scalar};
use fj_math::Point;

use crate::{
geometry::curve::Curve,
Expand All @@ -13,7 +13,7 @@ use crate::{
#[derive(Clone, Debug)]
pub struct PartialHalfEdge {
/// The curve that the half-edge is defined in
pub curve: Option<MaybeCurve>,
pub curve: Option<Curve>,

/// The boundary of the half-edge on the curve
pub boundary: [Option<Point<1>>; 2],
Expand All @@ -35,12 +35,7 @@ impl PartialHalfEdge {
let [start, _] = self.boundary;
start.and_then(|start| {
let curve = self.curve?;

if let MaybeCurve::Defined(curve) = curve {
return Some(curve.point_from_path_coords(start));
}

None
Some(curve.point_from_path_coords(start))
})
}
}
Expand All @@ -59,48 +54,19 @@ impl PartialObject for PartialHalfEdge {

fn from_full(half_edge: &Self::Full, _: &mut FullToPartialCache) -> Self {
Self {
curve: Some(half_edge.curve().into()),
curve: Some(half_edge.curve()),
boundary: half_edge.boundary().map(Some),
start_vertex: half_edge.start_vertex().clone(),
global_form: half_edge.global_form().clone(),
}
}

fn build(self, _: &mut Service<Objects>) -> Self::Full {
let curve = match self.curve.expect("Need path to build curve") {
MaybeCurve::Defined(path) => path,
undefined => {
panic!(
"Trying to build curve with undefined path: {undefined:?}"
)
}
};
let curve = self.curve.expect("Need path to build curve");
let boundary = self.boundary.map(|point| {
point.expect("Can't build `HalfEdge` without boundary positions")
});

HalfEdge::new(curve, boundary, self.start_vertex, self.global_form)
}
}

/// A possibly undefined curve
#[derive(Clone, Copy, Debug)]
pub enum MaybeCurve {
/// The curve is fully defined
Defined(Curve),

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

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

impl From<Curve> for MaybeCurve {
fn from(path: Curve) -> Self {
Self::Defined(path)
}
}