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

Remove redundant references to Vertex from GlobalEdge #1644

Merged
merged 14 commits into from
Mar 3, 2023
5 changes: 1 addition & 4 deletions crates/fj-kernel/src/algorithms/sweep/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
// even if the original edge was a circle, it's still going to be a line
// when projected into the new surface. For the side edges, because
// we're sweeping along a straight path.
for ((mut half_edge, start), (next_half_edge, end)) in [
for ((mut half_edge, start), (_, end)) in [
edge_bottom.clone(),
edge_up.clone(),
edge_top.clone(),
Expand All @@ -122,9 +122,6 @@ impl Sweep for (Handle<HalfEdge>, &Handle<Vertex>, &Surface, Color) {
.circular_tuple_windows()
{
half_edge.write().update_as_line_segment(start, end);
half_edge
.write()
.infer_global_form(next_half_edge.read().start_vertex.clone());
}

// Finally, we can make sure that all edges refer to the correct global
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/algorithms/sweep/vertex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl Sweep for Handle<Vertex> {
.clone();

let vertices = [a, b];
let global_edge = GlobalEdge::new(vertices.clone()).insert(objects);
let global_edge = GlobalEdge::new().insert(objects);

// The vertices of the returned `GlobalEdge` are in normalized order,
// which means the order can't be relied upon by the caller. Return the
Expand Down
16 changes: 7 additions & 9 deletions crates/fj-kernel/src/algorithms/transform/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,13 @@ impl TransformObject for HalfEdge {
impl TransformObject for GlobalEdge {
fn transform_with_cache(
self,
transform: &Transform,
objects: &mut Service<Objects>,
cache: &mut TransformCache,
_: &Transform,
_: &mut Service<Objects>,
_: &mut TransformCache,
) -> Self {
let vertices =
self.vertices().access_in_normalized_order().map(|vertex| {
vertex.transform_with_cache(transform, objects, cache)
});

Self::new(vertices)
// There's nothing to actually transform here, as `GlobalEdge` holds no
// data. We still need this implementation though, as a new `GlobalEdge`
// must be created to represent the new and transformed edge.
Self::new()
}
}
38 changes: 3 additions & 35 deletions crates/fj-kernel/src/builder/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,41 +54,9 @@ pub trait CycleBuilder {

impl CycleBuilder for PartialCycle {
fn add_half_edge(&mut self) -> Partial<HalfEdge> {
let mut new_half_edge = Partial::<HalfEdge>::new();

let (first_half_edge, mut last_half_edge) =
match self.half_edges.first() {
Some(first_half_edge) => {
let first_half_edge = first_half_edge.clone();
let last_half_edge = self
.half_edges
.last()
.cloned()
.unwrap_or_else(|| first_half_edge.clone());

(first_half_edge, last_half_edge)
}
None => (new_half_edge.clone(), new_half_edge.clone()),
};

{
let shared_surface_vertex =
new_half_edge.read().start_vertex.clone();
last_half_edge
.write()
.infer_global_form(shared_surface_vertex);
}

{
let shared_surface_vertex =
first_half_edge.read().start_vertex.clone();
new_half_edge
.write()
.infer_global_form(shared_surface_vertex);
}

self.half_edges.push(new_half_edge.clone());
new_half_edge
let half_edge = Partial::new();
self.half_edges.push(half_edge.clone());
half_edge
}

fn update_as_polygon_from_points<O, P>(
Expand Down
24 changes: 1 addition & 23 deletions crates/fj-kernel/src/builder/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
curve::{Curve, GlobalPath},
surface::SurfaceGeometry,
},
objects::{GlobalEdge, HalfEdge, Vertex},
objects::{HalfEdge, Vertex},
partial::{MaybeCurve, Partial, PartialGlobalEdge, PartialHalfEdge},
};

Expand Down Expand Up @@ -38,15 +38,6 @@ pub trait HalfEdgeBuilder {
end: Point<2>,
) -> Curve;

/// Infer the global form of the half-edge
///
/// Updates the global form referenced by this half-edge, and also returns
/// it.
fn infer_global_form(
&mut self,
next_vertex: Partial<Vertex>,
) -> Partial<GlobalEdge>;

/// Infer the vertex positions (surface and global), if not already set
fn infer_vertex_positions_if_necessary(
&mut self,
Expand Down Expand Up @@ -88,9 +79,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
*point_boundary = Some(point_curve);
}

let next_vertex = self.start_vertex.clone();
self.infer_global_form(next_vertex);

path
}

Expand Down Expand Up @@ -151,16 +139,6 @@ impl HalfEdgeBuilder for PartialHalfEdge {
path
}

fn infer_global_form(
&mut self,
next_vertex: Partial<Vertex>,
) -> Partial<GlobalEdge> {
self.global_form.write().vertices =
[&self.start_vertex, &next_vertex].map(|vertex| vertex.clone());

self.global_form.clone()
}

fn infer_vertex_positions_if_necessary(
&mut self,
surface: &SurfaceGeometry,
Expand Down
116 changes: 13 additions & 103 deletions crates/fj-kernel/src/objects/full/edge.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use fj_math::Point;

use crate::{geometry::curve::Curve, objects::Vertex, storage::Handle};
use crate::{
geometry::curve::Curve,
objects::Vertex,
storage::{Handle, HandleWrapper},
};

/// A directed edge, defined in a surface's 2D space
///
Expand Down Expand Up @@ -43,7 +47,7 @@ pub struct HalfEdge {
curve: Curve,
boundary: [Point<1>; 2],
start_vertex: Handle<Vertex>,
global_form: Handle<GlobalEdge>,
global_form: HandleWrapper<GlobalEdge>,
}

impl HalfEdge {
Expand All @@ -58,7 +62,7 @@ impl HalfEdge {
curve,
boundary,
start_vertex,
global_form,
global_form: global_form.into(),
}
}

Expand Down Expand Up @@ -96,115 +100,21 @@ impl HalfEdge {
/// An undirected edge, defined in global (3D) coordinates
///
/// In contrast to [`HalfEdge`], `GlobalEdge` is undirected, meaning it has no
/// defined direction, and its vertices have no defined order. This means it can
/// be used to determine whether two [`HalfEdge`]s map to the same `GlobalEdge`,
/// regardless of their direction.
/// defined direction. This means it can be used to determine whether two
/// [`HalfEdge`]s map to the same `GlobalEdge`, regardless of their direction.
///
/// See [`HalfEdge`]'s documentation for more information on the relationship
/// between [`HalfEdge`] and `GlobalEdge`.
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct GlobalEdge {
vertices: VerticesInNormalizedOrder,
}
#[derive(Clone, Debug, Default, Hash)]
pub struct GlobalEdge {}

impl GlobalEdge {
/// Create a new instance
///
/// The order of `vertices` is irrelevant. Two `GlobalEdge`s with the same
/// `curve` and `vertices` will end up being equal, regardless of the order
/// of `vertices` here.
pub fn new(vertices: [Handle<Vertex>; 2]) -> Self {
let (vertices, _) = VerticesInNormalizedOrder::new(vertices);

Self { vertices }
}

/// Access the vertices that bound the edge on the curve
///
/// As the name indicates, the order of the returned vertices is normalized
/// and might not match the order of the vertices that were passed to
/// [`GlobalEdge::new`]. You must not rely on the vertices being in any
/// specific order.
pub fn vertices(&self) -> &VerticesInNormalizedOrder {
&self.vertices
}
}

/// The vertices of a [`GlobalEdge`]
///
/// Since [`GlobalEdge`] is the single global representation of an edge in
/// global space, it must normalize the order of its vertices. Otherwise, it is
/// possible to construct two [`GlobalEdge`] instances that are meant to
/// represent the same edge, but aren't equal.
#[derive(Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)]
pub struct VerticesInNormalizedOrder([Handle<Vertex>; 2]);

impl VerticesInNormalizedOrder {
/// Construct a new instance of `VerticesInNormalizedOrder`
///
/// The provided vertices can be in any order. The returned `bool` value
/// indicates whether the normalization changed the order of the vertices.
pub fn new([a, b]: [Handle<Vertex>; 2]) -> (Self, bool) {
if a < b {
(Self([a, b]), false)
} else {
(Self([b, a]), true)
}
}

/// Access the vertices
///
/// The vertices in the returned array will be in normalized order.
pub fn access_in_normalized_order(&self) -> [Handle<Vertex>; 2] {
self.0.clone()
}
}

#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;

use crate::{
builder::{CycleBuilder, HalfEdgeBuilder},
partial::PartialCycle,
services::Services,
};

#[test]
fn global_edge_equality() {
let mut services = Services::new();

let surface = services.objects.surfaces.xy_plane();

let a = [0., 0.];
let b = [1., 0.];
let c = [0., 1.];

let a_to_b = {
let mut cycle = PartialCycle::default();

let [mut half_edge, next_half_edge, _] =
cycle.update_as_polygon_from_points([a, b, c]);
half_edge.write().infer_vertex_positions_if_necessary(
&surface.geometry(),
next_half_edge.read().start_vertex.clone(),
);

half_edge.build(&mut services.objects)
};
let b_to_a = {
let mut cycle = PartialCycle::default();

let [mut half_edge, next_half_edge, _] =
cycle.update_as_polygon_from_points([b, a, c]);
half_edge.write().infer_vertex_positions_if_necessary(
&surface.geometry(),
next_half_edge.read().start_vertex.clone(),
);

half_edge.build(&mut services.objects)
};

assert_eq!(a_to_b.global_form(), b_to_a.global_form());
pub fn new() -> Self {
Self::default()
}
}
2 changes: 1 addition & 1 deletion crates/fj-kernel/src/objects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ mod stores;
pub use self::{
full::{
cycle::{Cycle, HalfEdgesOfCycle},
edge::{GlobalEdge, HalfEdge, VerticesInNormalizedOrder},
edge::{GlobalEdge, HalfEdge},
face::{Face, FaceSet, Handedness},
shell::Shell,
sketch::Sketch,
Expand Down
28 changes: 6 additions & 22 deletions crates/fj-kernel/src/partial/objects/edge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,7 @@ impl Default for PartialHalfEdge {
fn default() -> Self {
let curve = None;
let start_vertex = Partial::default();
let end_vertex = Partial::default();

let global_form = Partial::from_partial(PartialGlobalEdge {
vertices: [start_vertex.clone(), end_vertex],
});
let global_form = Partial::default();

Self {
curve,
Expand All @@ -103,28 +99,16 @@ impl Default for PartialHalfEdge {

/// A partial [`GlobalEdge`]
#[derive(Clone, Debug, Default)]
pub struct PartialGlobalEdge {
/// The vertices that bound the edge on the curve
pub vertices: [Partial<Vertex>; 2],
}
pub struct PartialGlobalEdge {}

impl PartialObject for PartialGlobalEdge {
type Full = GlobalEdge;

fn from_full(
global_edge: &Self::Full,
cache: &mut FullToPartialCache,
) -> Self {
Self {
vertices: global_edge
.vertices()
.access_in_normalized_order()
.map(|vertex| Partial::from_full(vertex, cache)),
}
fn from_full(_: &Self::Full, _: &mut FullToPartialCache) -> Self {
Self {}
}

fn build(self, objects: &mut Service<Objects>) -> Self::Full {
let vertices = self.vertices.map(|vertex| vertex.build(objects));
GlobalEdge::new(vertices)
fn build(self, _: &mut Service<Objects>) -> Self::Full {
GlobalEdge::new()
}
}
Loading