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

Require Handle<HalfEdge> in fewer places #1680

Merged
merged 11 commits into from
Mar 13, 2023
4 changes: 3 additions & 1 deletion crates/fj-kernel/src/algorithms/approx/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
//!
//! See [`CycleApprox`].

use std::ops::Deref;

use fj_math::Segment;

use crate::objects::{Cycle, Surface};
Expand All @@ -26,7 +28,7 @@ impl Approx for (&Cycle, &Surface) {
let half_edges = cycle
.half_edges()
.map(|half_edge| {
(half_edge, surface).approx_with_cache(tolerance, cache)
(half_edge.deref(), surface).approx_with_cache(tolerance, cache)
})
.collect();

Expand Down
43 changes: 12 additions & 31 deletions crates/fj-kernel/src/algorithms/approx/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::{

use super::{path::RangeOnPath, Approx, ApproxPoint, Tolerance};

impl Approx for (&Handle<HalfEdge>, &Surface) {
impl Approx for (&HalfEdge, &Surface) {
type Approximation = HalfEdgeApprox;
type Cache = EdgeCache;

Expand All @@ -43,8 +43,7 @@ impl Approx for (&Handle<HalfEdge>, &Surface) {
}
};

let first = ApproxPoint::new(position_surface, position_global)
.with_source((half_edge.clone(), half_edge.boundary()[0]));
let first = ApproxPoint::new(position_surface, position_global);

let points = {
let approx =
Expand Down Expand Up @@ -74,7 +73,6 @@ impl Approx for (&Handle<HalfEdge>, &Surface) {
.point_from_path_coords(point.local_form);

ApproxPoint::new(point_surface, point.global_form)
.with_source((half_edge.clone(), point.local_form))
})
.collect()
};
Expand Down Expand Up @@ -264,11 +262,10 @@ mod tests {

use crate::{
algorithms::approx::{path::RangeOnPath, Approx, ApproxPoint},
builder::{CycleBuilder, HalfEdgeBuilder},
builder::HalfEdgeBuilder,
geometry::{curve::GlobalPath, surface::SurfaceGeometry},
insert::Insert,
objects::Surface,
partial::{PartialCycle, PartialObject},
services::Services,
};

Expand All @@ -277,16 +274,9 @@ mod tests {
let mut services = Services::new();

let surface = services.objects.surfaces.xz_plane();
let half_edge = {
let mut cycle = PartialCycle::new(&mut services.objects);

let [half_edge, _, _] = cycle.update_as_polygon_from_points(
[[1., 1.], [2., 1.], [1., 2.]],
&mut services.objects,
);

half_edge
};
let half_edge =
HalfEdgeBuilder::line_segment([[1., 1.], [2., 1.]], None)
.build(&mut services.objects);

let tolerance = 1.;
let approx = (&half_edge, surface.deref()).approx(tolerance);
Expand All @@ -303,16 +293,9 @@ mod tests {
v: [0., 0., 1.].into(),
})
.insert(&mut services.objects);
let half_edge = {
let mut cycle = PartialCycle::new(&mut services.objects);

let [half_edge, _, _] = cycle.update_as_polygon_from_points(
[[1., 1.], [2., 1.], [1., 2.]],
&mut services.objects,
);

half_edge
};
let half_edge =
HalfEdgeBuilder::line_segment([[1., 1.], [2., 1.]], None)
.build(&mut services.objects);

let tolerance = 1.;
let approx = (&half_edge, surface.deref()).approx(tolerance);
Expand All @@ -336,8 +319,7 @@ mod tests {
[[0., 1.], [TAU, 1.]],
Some(range.boundary),
)
.build(&mut services.objects)
.insert(&mut services.objects);
.build(&mut services.objects);

let tolerance = 1.;
let approx = (&half_edge, surface.deref()).approx(tolerance);
Expand All @@ -361,9 +343,8 @@ mod tests {
let mut services = Services::new();

let surface = services.objects.surfaces.xz_plane();
let half_edge = HalfEdgeBuilder::circle(1.)
.build(&mut services.objects)
.insert(&mut services.objects);
let half_edge =
HalfEdgeBuilder::circle(1.).build(&mut services.objects);

let tolerance = 1.;
let approx = (&half_edge, surface.deref()).approx(tolerance);
Expand Down
6 changes: 2 additions & 4 deletions crates/fj-kernel/src/algorithms/approx/face.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ impl Approx for &FaceSet {
panic!(
"Invalid approximation: \
Distinct points are too close \
(a: {:?}, b: {:?}, distance: {distance})\n\
source of `a`: {:#?}\n\
source of `b`: {:#?}\n",
a.global_form, b.global_form, a.source, b.source
(a: {:?}, b: {:?}, distance: {distance})",
a.global_form, b.global_form,
);
}
}
Expand Down
21 changes: 0 additions & 21 deletions crates/fj-kernel/src/algorithms/approx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,13 @@ pub mod solid;
pub mod tolerance;

use std::{
any::Any,
cmp::Ordering,
fmt::Debug,
hash::{Hash, Hasher},
rc::Rc,
};

use fj_math::Point;

use crate::{objects::HalfEdge, storage::Handle};

pub use self::tolerance::{InvalidTolerance, Tolerance};

/// Approximate an object
Expand Down Expand Up @@ -59,9 +55,6 @@ pub struct ApproxPoint<const D: usize> {

/// The global form of the points
pub global_form: Point<3>,

/// The optional source of the point
pub source: Option<Rc<dyn Source>>,
}

impl<const D: usize> ApproxPoint<D> {
Expand All @@ -70,15 +63,6 @@ impl<const D: usize> ApproxPoint<D> {
Self {
local_form,
global_form,
source: None,
}
}

/// Attach a source to the point
pub fn with_source(self, source: impl Source) -> Self {
Self {
source: Some(Rc::new(source)),
..self
}
}
}
Expand Down Expand Up @@ -114,8 +98,3 @@ impl<const D: usize> PartialOrd for ApproxPoint<D> {
Some(self.cmp(other))
}
}

/// The source of an [`ApproxPoint`]
pub trait Source: Any + Debug {}

impl Source for (Handle<HalfEdge>, Point<1>) {}
8 changes: 3 additions & 5 deletions crates/fj-kernel/src/algorithms/sweep/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{

use super::{Sweep, SweepCache};

impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
impl Sweep for (&HalfEdge, &Handle<Vertex>, &Surface, Color) {
type Swept = (Handle<Face>, Handle<HalfEdge>);

fn sweep_with_cache(
Expand Down Expand Up @@ -101,12 +101,10 @@ impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
builder
};

builder.build(objects).insert(objects)
builder.build(objects)
};

face.exterior.write().add_half_edge(half_edge.clone());

half_edge
face.exterior.write().add_half_edge(half_edge, objects)
});

// And we're done creating the face! All that's left to do is build our
Expand Down
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 @@ -68,7 +68,7 @@ impl Sweep for Handle<Face> {
cycle.half_edges().cloned().circular_tuple_windows()
{
let (face, top_edge) = (
half_edge.clone(),
half_edge.deref(),
next.start_vertex(),
self.surface().deref(),
self.color(),
Expand Down
30 changes: 17 additions & 13 deletions crates/fj-kernel/src/builder/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ pub trait CycleBuilder {
///
/// If this is the first half-edge being added, it is connected to itself,
/// meaning its front and back vertices are the same.
fn add_half_edge(&mut self, half_edge: Handle<HalfEdge>);
fn add_half_edge(
&mut self,
half_edge: HalfEdge,
objects: &mut Service<Objects>,
) -> Handle<HalfEdge>;

/// Update cycle as a polygon from the provided points
fn update_as_polygon_from_points<O, P>(
Expand Down Expand Up @@ -50,8 +54,14 @@ pub trait CycleBuilder {
}

impl CycleBuilder for PartialCycle {
fn add_half_edge(&mut self, half_edge: Handle<HalfEdge>) {
self.half_edges.push(half_edge);
fn add_half_edge(
&mut self,
half_edge: HalfEdge,
objects: &mut Service<Objects>,
) -> Handle<HalfEdge> {
let half_edge = half_edge.insert(objects);
self.half_edges.push(half_edge.clone());
half_edge
}

fn update_as_polygon_from_points<O, P>(
Expand All @@ -65,12 +75,9 @@ impl CycleBuilder for PartialCycle {
{
points.map_with_next(|start, end| {
let half_edge = HalfEdgeBuilder::line_segment([start, end], None)
.build(objects)
.insert(objects);

self.add_half_edge(half_edge.clone());
.build(objects);

half_edge
self.add_half_edge(half_edge, objects)
})
}

Expand All @@ -85,12 +92,9 @@ impl CycleBuilder for PartialCycle {
edges.map_with_prev(|(_, curve, boundary), (prev, _, _)| {
let half_edge = HalfEdgeBuilder::new(curve, boundary)
.with_start_vertex(prev.start_vertex().clone())
.build(objects)
.insert(objects);

self.add_half_edge(half_edge.clone());
.build(objects);

half_edge
self.add_half_edge(half_edge, objects)
})
}
}
5 changes: 2 additions & 3 deletions crates/fj-operations/src/sketch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ impl Shape for fj::Sketch {
}
};

let half_edge =
half_edge.build(objects).insert(objects);
cycle.add_half_edge(half_edge);
let half_edge = half_edge.build(objects);
cycle.add_half_edge(half_edge, objects);
}

Partial::from_partial(cycle)
Expand Down